This is good stuff, here are my notes on your code:
- Good job protecting against spoof posts by manually assigning the
author_id
tocurrent_user.id
and also restricting author_id from yourpost_params
method. ๐ ๐ ๐ - Good work having
if logged_in?
logic on all of the appropriate views! ๐ - When searching by id number, instead of using
find_by
it is best to usefind
since that method is designed to search on the id field. Here are the docs onfind
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 bycurrent_user
and only delete/update if it is. (Even if you have conditionals in your views that will only show your edit view if thecurrent_user
created the post, any user could still edit any post via a http tool like curl.) - 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. Holy schnikies! - Tip: 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.author.username
. It would be better to create anauthor_username
method on yourPost
model, so that all you would need to do in your view is@post.author_username
. This way, the logic happens in the model, not in the view.
This is good stuff, you are getting this. Keep at it! ๐
Any questions let me know.
-Phil