Conversation
models and validations
made a likes controller
user controller & show page, fixed post association
add post controller
made like and friends controller
changed some controller methods on likes
added links to nav and user login works
more post stuff
user stuff
philsof
left a comment
philsof
left a comment
There was a problem hiding this comment.
@jordanyryan @mercurom1 @LM-Towner See inline notes for tips and comments
|
|
||
| def update | ||
| @post = Post.find(params[:id]) | ||
| if @post.update(post_params) |
There was a problem hiding this comment.
Heads up: because of how you set up your post_params method, these two lines actually allow the logged in user to become the owner of any post in your database, including posts the logged in user did not create. You need to protect the update before calling it:
@post = Post.find(params[:id])
if @post.user == current_user
# update goes in here
# etc| private | ||
|
|
||
| def post_params | ||
| params.require(:post).permit(:content, :user_id => current_user.id) |
There was a problem hiding this comment.
merge may be a better choice here (i.e. merge the additional key/value pair into the hash that permit returns). But it would be even better if you did this merge inside your create action, since that is the only action in which you will want this merge to happen. (See note above in update action.)
|
|
||
| def destroy | ||
| @post = Post.find(params[:id]) | ||
| @post.destroy |
There was a problem hiding this comment.
Protect the destroy! This is very simple:
@post = Post.find(params[:id])
if @post.user == current_user
@post.destroy
#etc.| def show | ||
| if current_user | ||
| @user = User.find_by(id: params[:id]) | ||
| else |
| @@ -0,0 +1,35 @@ | |||
| class Friend < ApplicationRecord | |||
There was a problem hiding this comment.
The name of this class is throwing me off. This model is not of a friend, but rather of a friendship. The friend is what you call friend_user. Be sure to be clear in your naming.
| end | ||
|
|
||
| def find_friend | ||
| User.find(self.friend_user_id) |
There was a problem hiding this comment.
Let's not call User inside a Friend method. Instead, utilize your Friend associations to get to the desired User.
|
|
||
| <% @user.user_friends.each do |friend| %> | ||
| <div class="friends-list"> | ||
| <%= friend.find_friend.name %> |
There was a problem hiding this comment.
This is super confusing because of the naming
| def change | ||
| create_table :posts do |t| | ||
| t.text :content, null:false | ||
| t.references :user, null: false |
There was a problem hiding this comment.
FYI general best practice for foreign keys:
t.references :user, null: false, foreign_key: true
# this note applies to all of your foreign keys| def change | ||
| create_table :friends do |t| | ||
| t.references :user, null: false, foreign_key: true | ||
| t.references :friend_user, null: false, index: true |
There was a problem hiding this comment.
FYI references automatically creates an index for each of these fields
| def change | ||
| create_table :friend_requests do |t| | ||
| t.references :user, null: false, index: true | ||
| t.integer :friend_id, null: false, index: true |
There was a problem hiding this comment.
Is this referencing friends or another table?
4 participants
DO NOT MERGE