Good work, here are my notes on your code:
- Watch out: your post controller is allowing
user_id
to be passed as a param. This means a client could easily spoof themselves as another user and create posts as someone else (by passing someone else's user_id in the params, perhaps via a command-line http tool like curl). You are going to want to removeuser_id
from yourpost_params
method and instead hard code the adding ofuser_id
to the post viasession.user_id
. You always want to get your user id from thesession.user_id
, sincesession
cannot be spoofed. - Instead on using
find_by
when searching by a unique id number (like you do here) it would be better to utilizefind
since it is designed specifically to search by a unique id number. Here are the docs on find for your reference. - Watch out: Your update and destroy methods in your post controller do not check if the current user is the owner of the post that is being deleted/updated. You should add logic that confirms the post is owned by
session.user_id
and only delete/update if it is. - It looks like you do not have any validations on your User model, which means someone could create a user with the same username as an existing user. This is bad news since your login looks for the first matching username. Thus, the first person that creates username X will have no issue logging in, but the second person with username X will never be able to log in. Uh oh!
- It looks like each user has_many sites, and each site has_many users, but there is no many-to-many join table between them. Do these assocations work properly?
- You don't want any logic happening in your views. But what constitutes logic? Here is a general rule: you should not chain two methods on each other in your views; that would constitute logic, and should happen only in your models or controllers (preferably in your models). For example, here you are pulling a username from a post by chaining
@post.creator.username
. It would be better to create acreator_username
method on yourPost
model, so that all you would need to do in your view is@post.creator_username
. This way, the logic happens in the model, not in the view.
Good work, you are getting this! Keep at it! Any questions let me know.
Phil