Sigrid Benezra - Edges - MediaRanker #30
Conversation
…o db. Change works index page to list all works as list.
… Add link to edit page from works show page.
…_form.html.erb for better display. Make title on works index page a link to the work show page.
… to work show page.
…op vote getter and list works by category and vote total.
…ntroller method to flash message for missing user id.
Media RankerWhat We're Looking For
|
| else | ||
| user = User.create(name: name) | ||
| redirect_to root_path | ||
| flash[:success] = "#{user.name} is successfully logged in" |
There was a problem hiding this comment.
When you create a new user you're not saving their ID in the session. That means that a new user has to enter their name twice to log in: once to create the record, and once to set their user ID in the session.
| def update | ||
| @user = User.find(params[:id]) | ||
| if @user.update(user_params) | ||
| redirect_to user_path(@user) |
There was a problem hiding this comment.
You've implemented all 7 RESTful controller actions for User, but I think you only need index and show for this site.
|
|
||
| def new | ||
| user = @logged_in_user | ||
| work = Work.find_by(id: params[:work_id]) |
There was a problem hiding this comment.
Again, you've implemented the index and new actions for the VotesController, but they're not used by your site, and you don't have view templates for them.
| belongs_to :user | ||
|
|
||
| validates :user_id, presence: true, uniqueness: { scope: :work_id, | ||
| message: "can only vote for a work once" } |
There was a problem hiding this comment.
Good work getting this tricky uniqueness scope figured out!
|
|
||
| def self.top_vote_getter_votes | ||
| return Work.top_vote_getter.votes.size | ||
| 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.
| <h2><span><strong>Media Spotlight: </strong></span><span class="list-titles"><%= link_to Work.top_vote_getter.title, work_path(Work.top_vote_getter.id) %></span><span class="list-titles"> by <%= Work.top_vote_getter.creator %> </span></h2> | ||
| <p> | ||
| <%= Work.top_vote_getter_votes %> votes - <%= Work.top_vote_getter.description %> | ||
| </p> |
There was a problem hiding this comment.
Rather than having the views call model methods directly, these method should be invoked in the controller and stored in instance variables. See MediaRanker Revisited for an example of this.
| Top Albums | ||
| </h2> | ||
| <ul class="list-group"> | ||
| <% Work.top_works_by_category('album', 10).each do |work| %> |
There was a problem hiding this comment.
You have the same code to show a list of works repeated 3 times. Could you use a view partial or a loop to DRY this up?
| <% end %> | ||
| </section> | ||
|
|
||
| <body> |
There was a problem hiding this comment.
All HTML that renders on the page (including the <header> and flash <section>) should be inside the <body> tags.
|
|
||
| # describe Vote do | ||
| # let(:vote) { Vote.new } | ||
| # |
There was a problem hiding this comment.
You should be testing the uniqueness constraint here! Specific test cases I'd want to see:
- A user can have votes for two different works
- A work can have votes from two different users
- A user cannot vote for the same work twice
| describe 'list_works_by_category' do | ||
|
|
||
| it 'correctly filters the list for a category' do | ||
| book_list = Work.list_works_by_category('book') |
There was a problem hiding this comment.
There's a lot of interesting test cases for your custom Work methods missing here. For finding the media spotlight, I would wonder:
- What happens if there are no works?
- What happens if there are works but no votes?
- What happens if two works have the same number of votes?
Similarly for your category and top ten methods, I would ask:
- What if there are no works of that category?
- What if there are less than 10 works?
- What if there's a tie for last place, e.g. works 9, 10 and 11 all have 0 votes?
Media Ranker
Congratulations! You're submitting your assignment!
Comprehension Questions
sessionandflash? What is the difference between them?