Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Save philsof/cf75e9aab3d7f47ec18a to your computer and use it in GitHub Desktop.
Save philsof/cf75e9aab3d7f47ec18a to your computer and use it in GitHub Desktop.
Code review pair-scottychou blog-1-anonymous-blog-challenge

Good work! Everything is solid except for what is mentioned below. So if it's not below, you are good. Here are my notes on your code:

##Views##

  • When I go to the edit form at http://localhost:9393/entries/:id/edit the body of the existing entry does not show in the corresponding input field. It looks like this is because you are utilizing a textarea tag in your entries/edit view. Heads up: It doesn't look like the textarea tag supports the value attribute, according to the docs. That would explain why the body doesn't show on the edit form. You will need to fix this, since the user will think the entry doesn't have a body at all (which is not true).
  • Watch out: Your entries/new view cannot handle a request from a user who is not logged in (because the view assumes that current_user is not nil). How can you adjust this view so that it can handle a request from a user who is not logged in (e.g. the user manually types http://localhost:9393/entries/new into their browser's address bar)?

#Authorization##

  • Watch out: Currently any user can edit or delete any entry, even entries the user did not create. (While you have successfully hidden the edit and delete links from the view of unauthorized users, you have not protected the put and delete route actions themselves, which means if I type http://localhost:9393/entries/1/edit into my browser's address bar, I can edit entry 1 even if I have not created it. Also if I use a simple http program like curl I could also delete entry 1 by sending a delete request.) How can you add controls to your edit and delete route actions, so that only the creator of an entry can edit or delete them?

##dependent: :destroy##

  • Your uses of dependent: :destroy are syntactically correct. But watch out: while it makes sense to automatically delete all of the entry_tags associated with an entry (since they would become orphaned records when their corresponding entry is deleted), it does not make sense to also automatically delete all of the tags associated with an entry, since other entry records may also be associated with those tag records. Challenge: can you figure out a way to delete the corresponding tag records only if they would be orphaned as a result of the destroy call on the entry object? Hint: it can be done.

##Users##

  • Watch out: You can currently create new users with no passwords. Not good.

##Instance Variables##

  • In this route action you utilize an instance variable @user but you don't use it outside of this action. (The redirect can't receive any state, and the users/login view doesn't utilize @user.) Thus you should use a local variable (user) instead of an instance variable (@user). Only use instance variables to utilize them outside of the current method.

##Error Handling##

  • If an edit of an entry is not successful, no error message shows because your put route action has no error handling. (But your entries/show view does have error handling! So all you need to do is pass @errors to the entries/show view.)
  • If the user tries to create a new profile, but does not enter any text in the username field, the error message that appears is username already exists which doesn't make any sense. Can you figure out how to assign a specific error message to the presence: true part of the validation, and another error message to the uniqueness: true part of the validation? Hint: it's already done for you. Another hint: read the entire section 1.4 in the previous link, especially the longer code example.

##Logout and Delete##

  • To answer your question: Your logout and delete forms are cool as-is. They work and are RESTful.
  • FYI you could also do logout and delete as links, although any HTML link would be sending a get request to your controller. The only way (that I know of) to have a link send a true delete request is to utilize event handlers and ajax. Note: I do not think you need to do this; your forms work and are sufficient (unless your instructors tell you otherwise; do what your instructors tell you to do).

##Missed Something?##

  • It doesn't look like users have the ability to edit tags on existing posts. (I'm not sure if this is part of the requirements of the challenge.)

Good work! 👍 Any questions let me know.

-Phil

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment