Skip to content

Conversation

@lebaongoc
Copy link

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a custom model method you wrote. I wrote a custom method called "find_top_ten" in the Work model to return an array of 10 works for a particular category(book, album, or movie) and that array is sorted by the number of votes.
Describe how you approached testing that model method. What edge cases did you come up with? I started with generating 11 works in the yml file to use as the test database. After that I started with the output of the "find_top_ten" custom method and wrote test to verify that the method return an array with 10 items. One edge case that I came up with is when the work in a particular category(book) does not exist, I would expect the method return an empty array as output.
What are session and flash? What is the difference between them? Session is similar to flash, it's a hash that stores key, value pairs of info that could be used to track a user or display a message to a user. The different is in how long the info lasts. Session hash last until the user closes a browser or log out while flash only last until the next request.
What was one thing that you gained more clarity on through this assignment? I gained more clarify on how session hash works , how are the different models linked together with the foreign key and the methods available after ActiveRecord relationships were established
What is the Heroku URL of your deployed application? https://ngoc-media-ranker.herokuapp.com/
https://trello.com/b/aXbTcPwK/media-ranker-week-11
Do you have any recommendations on how we could improve this project for the next cohort? I would find a more guidance of the custom model to display top-ten media helpful.

@CheezItMan
Copy link

Media Ranker

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits and good commit messages. I will say however that debugged isn't very informative as a commit message.
Comprehension questions Check, for the custom model methods, I would take a look at the submissions from Steph or Sav and see how they did them. You can definitely take a look after the project is submitted
General
Rails fundamentals (RESTful routing, use of named paths) Check
Views are well-organized (DRY, use of semantic HTML, use of partials) Check
Errors are reported to the user Check
Business logic lives in the models Check, but see my notes
Models are thoroughly tested, including relations, validations and any custom logic Check, see my inline notes for specific feedback here
Controllers are thoroughly tested, including the login/logout process and multi-step workflows like voting for a work Check, see my
Wave 1 - Media
Splash page shows the three media categories Check
Basic CRUD operations on media are present and functional Check
Wave 2 - Users and Votes
Users can log in and log out Check
The ID of the current user is stored in the session Check
A user cannot vote for the same media more than once Check
All media lists are ordered by vote count Check
Splash page contains a media spotlight Check
Wave 3 - Users and Votes
Media pages contain lists of voting users Check
Individual user pages and the user list are present Check
Optional - Styling
Bootstrap is used appropriately Check
Look and feel is similar to the original Check, nice work!
Overall This was very well done! Great work. You hit all the learning goals. It's not perfect, but take a look at my inline feedback and let me know what questions you have.

album.destroy

# Act
get work_path(invalid_album_id)

Choose a reason for hiding this comment

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

This test is failing

end

def show
@votes = @work.votes.order(created_at: :desc)

Choose a reason for hiding this comment

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

You need to check to see if you could not find @work.

require "test_helper"

describe VotesController do
before do

Choose a reason for hiding this comment

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

Nice use of a before block

post work_votes_path(@work.id)
}.must_change "Vote.count", 1

expect(flash[:success]).must_equal "Successfully upvoted!"

Choose a reason for hiding this comment

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

You should also test redirection

expect(album_to_update.creator).must_equal @input_creator
expect(album_to_update.publication_year).must_equal @input_publication_year
expect(album_to_update.description).must_equal @input_description
must_respond_with :redirect

Choose a reason for hiding this comment

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

You should also test where they redirect to.

@@ -0,0 +1,4 @@
require "test_helper"

Choose a reason for hiding this comment

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

no validations for votes? You could make one to require a unique combination of user-work.

def self.find_most_voted
max_votes = 0
most_voted = nil
Work.all.each do |work|

Choose a reason for hiding this comment

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

You could do this with .order or .sort_by instead

let(:books) { Work.where(category: "book") }

it "should return a list of works that belong to book category" do
expect(Work.find_top_ten("book")).must_be_kind_of Array

Choose a reason for hiding this comment

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

You should also verify that each element is a Work and is in the right category.

end

it "should return an empty list if no books exist" do
books.each do |book|

Choose a reason for hiding this comment

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

There is also a destroy_all method.

@@ -0,0 +1,30 @@
class VotesController < ApplicationController
def create
if session[:user_id]

Choose a reason for hiding this comment

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

There's a lot of business logic here, I suggest moving some of it to the Work model so you could just call: work.upvote(user). You could also put the upvote method in the User model

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