Code review for branch mirascarvalone on challenge build-a-rails-blog 2/10/2016

code-review-mirascarvalone-build-a-rails-blog.md

This is good stuff, here are my notes on your code:

• Good job protecting against spoof posts by manually assigning the author_id to current_user.id and also restricting author_id from your post_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 use find since that method is designed to search on the id field. 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 current_user and only delete/update if it is. (Even if you have conditionals in your views that will only show your edit view if the current_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 an author_username method on your Post 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