Sockets - Hana #33
Sockets - Hana #33
Conversation
Media RankerWhat We're Looking For
|
| <td><%= work.publication_year %></td> | ||
| <td> | ||
| <%= button_to "Upvote", new_vote_path(work), method: :post %> | ||
| </td> |
There was a problem hiding this comment.
This button doesn't seem to work. You might want create_vote_path(@work)
| root "homepage#index" | ||
| get "/", to: "homepages#index" | ||
|
|
||
| resources :works, :users, :votes |
There was a problem hiding this comment.
I don't think you need all 7 RESTful routes for users and votes.
| else | ||
| flash[:error] = "A problem occured" | ||
|
|
||
| redirect_back(fallback_location: root_path) |
There was a problem hiding this comment.
It would be good to give a little more information here about what went wrong, both for the user and for you as the engineer working on the product.
| def vote | ||
| if session[:user_id] | ||
| voter_id = session[:user_id] | ||
| vote = Vote.new(user_id: voter_id, work_id: @work.id) |
There was a problem hiding this comment.
You have controller actions for both votes#create and works#vote. Probably only one of these is needed (I think votes#create is the one used by the app).
| def create | ||
| @vote = Vote.new( | ||
| user_id: 1, | ||
| work_id: params[:work_id] |
There was a problem hiding this comment.
Instead of setting user_id to 1 here, you should set it to session[:user_id].
As is, every vote always comes from user 1 instead of whoever is logged in.
|
|
||
| def self.top_ten(category) | ||
| return entries(category)[0..9] | ||
| end |
There was a problem hiding this comment.
I love the way the methods in this file build on each other, each providing a small bit of functionality. This is an excellent example of functional decomposition.
| <% Work.categories.each do |category| %> | ||
| <div class="col"> | ||
| <p class="column head">Top <%= category.capitalize %>s</p> | ||
| <% Work.top_ten(category).each do |work| %> |
There was a problem hiding this comment.
Great work keeping this view DRY.
| describe HomepageController do | ||
| # it "must be a real test" do | ||
| # flunk "Need real tests" | ||
| # end |
There was a problem hiding this comment.
Looks like this was forgotten. Aside from the nominal case, there's an interesting edge case when there are no works, since the spotlight section might fail to render.
| it "will create new user" do | ||
| user_data = { | ||
| user: { | ||
| username: "new user", |
There was a problem hiding this comment.
Good work getting both of the success cases here. There is also a failure case you're missing: what if the user can't be created? For example, if the username is "".
| describe VotesController do | ||
| # it "must be a real test" do | ||
| # flunk "Need real tests" | ||
| # end |
There was a problem hiding this comment.
There are some interesting test cases around voting that I wish you had had time to address.
2 participants
Media Ranker
Congratulations! You're submitting your assignment!
Comprehension Questions
entrieswith an argumentcategorywhere it would return the array of records in the category sorted by highest vote count.sessionandflash? What is the difference between them?flashonly hangs around for 1 request cycle andsessionlasts until the browser closed (thesessionends), or until it’s set tonilmanually.sessionandflash) and then some lessons felt not necessarily immediately applicable to the project (application helpers). The whole lesson on prettifying time passed felt really disjointed from the project.