Conversation
…ror messaging to login
Media RankerWhat We're Looking For
|
|
|
||
| get "/users/current", to: "users#current_user", as: "current_user" | ||
|
|
||
| resources :users, except: [:new, :create] |
There was a problem hiding this comment.
This assignment should only need the index and show actions for users.
| def edit | ||
| if !@work | ||
| flash[:error] = "No work with that ID found." | ||
| redirect_to works_path |
There was a problem hiding this comment.
This error checking is repeated many times in this controller. You've already written the find_work controller filter, could you do this work there?
|
|
||
| def already_voted?(work) | ||
| return self.works.include?(work) | ||
| end |
There was a problem hiding this comment.
I like that you've made this a model method.
If you wanted to make the interface even simpler / more foolproof, you could define it as a validation on Vote, so that it's impossible to create a duplicate vote. Look up "rails validation uniqueness scope"
| unless all_works == [] | ||
| return all_works.max_by(10) { |work| work.votes.count } | ||
| else | ||
| return [] |
There was a problem hiding this comment.
I think that this conditional isn't needed - calling max_by(10) on an empty array should return an empty array.
| @@ -0,0 +1,37 @@ | |||
| <body> | |||
|
|
|||
| <header> | |||
There was a problem hiding this comment.
In general you shouldn't need to include a <body> tag in view templates.
| describe "index" do | ||
| it "can get the root path" do | ||
| get root_path | ||
| must_respond_with :success |
There was a problem hiding this comment.
The spotlight presents us with an interesting edge case when there are no works in the database. Do your controller and view handle that without crashing?
| describe "login" do | ||
| it "can login a user" do | ||
| user = perform_login | ||
|
|
There was a problem hiding this comment.
There are a couple of behavior modes you're not covering:
- Logging in a new user (should create that user)
- Attempting to log in with an invalid username (e.g. empty string) (should fail)
|
|
||
| it "won't let a user vote if they're not logged in" do | ||
| expect { post vote_work_path(@gump.id) }.wont_change "#{@gump.votes.count}" | ||
| expect(@gump.votes.count).must_equal 0 |
There was a problem hiding this comment.
Good work getting the three interesting cases for this action, especially since the testing logic is so complex.
| end | ||
|
|
||
| it "returns an empty array if there are no works in that category" do | ||
| top_cars = Work.top_works("car") |
There was a problem hiding this comment.
Good work finding the edge cases for top_works. Another interesting question is, how does it handle ties?
| describe "categories" do | ||
| it "must return a list of categories" do | ||
| categories = Work.categories | ||
|
|
There was a problem hiding this comment.
Does this still work if one of the categories has no works?
2 participants
Media Ranker
Congratulations! You're submitting your assignment!
Comprehension Questions
sessionandflash? What is the difference between them?