-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Time Hannah-T #51
base: master
Are you sure you want to change the base?
Time Hannah-T #51
Conversation
Media RankerFunctional Requirements: Manual Testing
Major Learning Goals/Code Review
Previous Rails learning, Building Complex Model Logic, DRYing up Rails Code
Testing Rails Apps
Overall Feedback
Code Style Bonus AwardsWas the code particularly impressive in code style for any of these reasons (or more...?)
|
@work = Work.new(work_params) | ||
if @work.save | ||
flash.now[:success] = "Successfully created #{@work.category} #{@work.id}" | ||
render :show |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works! However, because you used render rather than redirect, the path is not updated to reflect the show action. Typically in a case like this it's better to use redirect instead.
if @work.nil? | ||
head :not_found | ||
return | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This follows the initial example that we gave for handling not found in the show action but in later lessons we moved from this to redirecting and presenting the user with a flash message explaining what went wrong.
This produces a more user-friendly flow than the default rails error, which doesn't allow the user to click anything to navigate back to the web application.
post "/login", to: "users#login" | ||
post "/logout", to: "users#logout", as: :logout | ||
patch "/works/:id/upvote", to: "votes#upvote", as: :upvote | ||
resources :votes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This creates the 7 restful routes for the VotesController even though none of those exist. Because the only action this controller has is the upvote action, this line is not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can run rails routes
from the terminal to see all the routes setup that are non-existent in the controller.
resources :works | ||
# For details on the DSL available within this file, see https://guides.rubyonrails.org/routing.html | ||
|
||
resources :users |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also creates all 7 restful routes when only index and show are actually setup in the controller.
resources :users | |
resources :users, only: [:index, :show] |
Media Ranker
Congratulations! You're submitting your assignment!
Comprehension Questions
session
andflash
? What is the difference between them?Assignment Submission: Media Ranker
Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.
Reflection
session
andflash
? What is the difference between them?