Branches-Amal

[ashassan]

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question	Answer
Describe a custom model method you wrote.	I wrote a method that returns the top ten works for each category. It does this by organizing them in descending order of votes.
Describe how you approached testing that model method. What edge cases did you come up with?	I made a test for when the top ten has no works.
What are session and flash? What is the difference between them?	session is a hash-like object to keep track of data throughout a users session. Flash is a special type of hash that we can use for a one time message. The data for session does not go away for the next request-response cycle like flash.
What was one thing that you gained more clarity on through this assignment?	I gained more clarity on flash and session.
What is the Heroku URL of your deployed application?	https://stormy-garden-04556.herokuapp.com/
Do you have any recommendations on how we could improve this project for the next cohort?	more time

[CheezItMan]

Media Ranker

What We're Looking For

Manual Testing

Workflow	yes / no
Deployed to Heroku	Yes
Optional: Look and feel is similar to the original (consider styling and layout)	Yes
Before logging in
On index page, there are at most 10 pieces of media on three lists, and a Media Spotlight	Yes
Can go into a work's show page	Yes
Verify unable to vote on a work, and get a flash message	It crashes here if I try to vote, and then tells me (when I go back) that I have to be logged in to vote.
Can edit this work successfully, and get a flash message	Yes
Can go to "View all media" page and see three lists of works, sorted by vote	No, see my inline note on works#index
Verify unable to create a new work when the form is empty, and details about the validation errors are visible to the user through a flash message	Yes, but I only get one validation message
Can create a new work successfully. Note the URL for this work's show page	Yes
Can delete this work successfully	Yes
Going back to the URL of this deleted work's show page produces a 404 or some redirect behavior (and does not try to produce a broken view)	I get a 404 response and a blank page. Something nicer would be better, but this is ok.
Verify that the "View all users" page lists no users (except what is produced by db seeding)	Yes
Log in
Logging in with a valid name changes the UI to "Logged in as" and "Logout" buttons	Yes
Your username is listed in "View all users" page	Yes
Verify that number of votes determines the Media Spotlight	Yes
Voting on several different pieces of media affects the "Votes" tables shown in the work's show page and the user's show page	Yes
Voting on the same work twice produces an error/flash message, and there is no extra vote	I get an error and when I hit back I get the error message, but it says I have to be logged in. If I fix the login bug the error message occurs properly.
Log out
Logging out showed a flash message and changed the UI	Yes
Logging in as a new user creates a new user	Yes
Logging in as an already existing user has a specific flash message	Yes

Targeted Code Review

Area	yes / no
git commits were small and atomic, with useful messages	Good commit messages, you do often do multiple things in each commit. Something to watch.
Routes file uses resources for works	Yes
Routes file shows intention in limiting routes for voting, log-in functionality, and users	Yes, although you didn't use resources instead you used custom routes.
The homepage view, all media view, and new works view use semantic HTML	Yes
The homepage view, all media view, and new works view use partials when appropriate	You are only using 1 partial for the form to create/edit works. Take a look at how often you are listing media. You can dry up your views by creating a partial here.
The model for media (likely named work.rb) has_many votes	Yes
The model for media has methods to describe business logic, specifically for top ten and top media, possibly also for getting works by some category	Yes, although you are missing one, see my comments
Some controller, likely the ApplicationController, has a controller filter for finding a logged in user	NO There are no controller filters.
Some controller, likely the WorksController, has a controller filter for finding a work	No controller filters
The WorksController uses strong params	Yes
The WorksController's code style is clean, and focused on working with requests, responses, params, session, flash	You did a pretty good job here pulling out business logic into the models.

Targed Test Review

Area	yes / no
There are valid fixtures files used for users, votes, and works	Yes
User model has tests with sections on validations (valid and invalid) and relationships (has votes)	Yes
Vote model has tests with sections on validations (valid and invalid) and relationships (belongs to a user, belongs to a vote)	No tests for validations as you never created validations for the Vote model.
Work model has tests with sections on validations (valid and invalid) and relationships (has votes)	Yes, but you only have 1 validation.
Work model has tests with a section on all business logic methods in the model, including their edge cases	Yes, you do have tests for the business logic methods, but you are not covering the edge cases.

Overall Feedback

You are soooo close to having a fully functional Media Ranker project. I've left notes for you to find where there are bugs in the application and how to fix'em. You also have some decent testing here, although you don't cover all the edge-cases. It looks like you ran out of time.

[CheezItMan]

You do not have a class method .by_category in work.rb

[CheezItMan]

You are setting the session to return_user.id before you save the user, so the user model doesn't have an I yet!

[CheezItMan]

This find_by functionality should instead be in a controller filter.

[CheezItMan]

Normally you would simply do

bob.votes << votes(:nouservote)

[CheezItMan]

This is really assert-style testing. It works, but it's not the style we're using in class.

Suggested change

	assert(@album.valid?)
	expect(@album.valid?).must_equal true

[CheezItMan]

Only one validation?