Skip to content

Conversation

@Krashaune
Copy link

@Krashaune Krashaune commented Apr 14, 2018

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a custom model method you wrote. I created self.top_votes which takes all works votes and sorts them in reverse order.
Describe how you approached testing that model method. What edge cases did you come up with? I did not get around to testing this model method. however all the validations are tested.
What are session and flash? What is the difference between them? Flash allows you to display a message to the user until the page is refreshed and session is an ability to set an action such as login to persist throughout the various cycles until cancelled.
Describe a controller filter you wrote. I wrote one controller filter for user and works controller that helps to find either a user or a work by their id.
What was one thing that you gained more clarity on through this assignment? How to troubleshoot various error messages in regards to relations. I used the given space in the error message to further understand why the error was occurring. most of the time it was because I was referring to a collection or instance of a variable and not the specific element within in.
What is the Heroku URL of your deployed application? https://krt-media.herokuapp.com/
Do you have any recommendations on how we could improve this project for the next cohort? I think emphasizing that TDD will not fail you so make sure to start with it no matter how tempting it is to revisit it after you've built things. And to take one element at time the other elements will present themselves along the way. Also maybe a little more lecture time on foundation.

Krashaune added 30 commits April 9, 2018 14:15
…or works index page to display all works by category. Also updates works controller index with instance variables for albums, books, and movies.
…Added render partial for error messages within user log in form. Added fixtures and validation tests that are passing.
…orm saving the user and rendering errors.Added new routes for sessions and methods in the controller new, create, and destroy.
… top for home page.Updated application.html.erb for logout button.
… media content to top media page. Added header class for top of page to prep for CSS.
… to singular. Added upvote route to works post. Updated todo to update upvote links with correct path.
@Krashaune Krashaune changed the title Kiera - Media Kiera - MediaRanker - Octos Apr 14, 2018
@droberts-sea
Copy link

Media Ranker

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes - good work
Comprehension questions yes
General
Rails fundamentals (RESTful routing, use of named paths) yes, though resources is overused
Semantic HTML yes
Errors are reported to the user yes
Business logic lives in the models some - see inline
Models are thoroughly tested, including relations, validations and any custom logic some - several pieces are missing
Wave 1 - Media
Splash page shows the three media categories yes
Basic CRUD operations on media are present and functional yes
Wave 2 - Users and Votes
Users can log in and log out yes
The ID of the current user is stored in the session yes
Individual user pages and the user list are present yes
A user cannot vote for the same media more than once yes
All media lists are ordered by vote count top page yes, all media page no
Splash page contains a media spotlight no
Media pages contain lists of voting users yes
Wave 3 - Styling
Foundation is used appropriately yes
Look and feel is similar to the original good enough
Overall Good work overall! It's clear you've put in a lot of effort on this project, and I think it's paid off. There's a couple of pieces of functionality missing, but it seems like you've got most of the interesting bits of the assignment, and I would say the core learning goals have been met.

The one thing that's not quite where I want it is model testing, so you should make sure to get some practice on this (and controller testing) in bEtsy. I've also left some inline comments below - please review them and try to apply them in your next project, and keep up the hard work!

post 'works/:id/upvote', to: 'works#upvote', as: 'upvote'

resources :users

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need all 7 restful routes for users - only index and show are needed for this project.

<%= link_to 'Back to ranks',root_path, class:'button' %>
<%= link_to 'Edit',edit_work_path, class:'button' %>
<%= link_to 'Upvote', root_path, class:'button' %>
<%= link_to 'Delete',work_path, method: :delete, data: {confirm: "Are you sure?"}, class:'alert button'%>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your upvote link goes to the wrong place!

<section>
<h3>Top Books</h3>
<% @works.where(category: 'book').top_votes.take(10).each do |work| %>
<ul class="no-bullet">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 20 would be great as a model method! Maybe something like self.top_ten(category), so that here (or in the controller) you could say Work.top_ten('book').

<h3>Music Albums</h3>
<table>

<th>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

require "test_helper"

describe Vote do
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants