Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Branches - Linnea #39

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

Branches - Linnea #39

wants to merge 30 commits into from

Conversation

llinneaa
Copy link

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a custom model method you wrote. I wrote a 'top ten' method in the work model which returns the top works for each category and displays them on the homepage.
Describe how you approached testing that model method. What edge cases did you come up with? I tested whether the method would return fewer than 10 works if there were not 10 in the db, whether it would return zero works if there were no works in that category in the db, and whether it would return only 10 in the case of there being greater than 10 works in that category in the db. Because I did not want to write ~15 fixtures for one category, I chose instead to do a times loop to create 15 works and used Faker to produce unique titles for each work. This approach worked well for my tests.
What are session and flash? What is the difference between them? Session tracks the user status and is present throughout the entire web session. Flash displays messages to the user from the server and only lasts for one request/response cycle.
What was one thing that you gained more clarity on through this assignment? Routes.
What is the Heroku URL of your deployed application? https://linnea-media-ranker.herokuapp.com/
Do you have any recommendations on how we could improve this project for the next cohort? I thought this was a good assignment overall and don't have suggestions for improvement.

…tlight to include vote count and description
@tildeee
Copy link

tildeee commented Nov 5, 2019

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 yes
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 yes, not sorted by vote on view all media page
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
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) yes
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 yes
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 yes
Routes file uses resources for works yes
Routes file shows intention in limiting routes for voting, log-in functionality, and users yes
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 yes, there are more opportunities for partials
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
Some controller, likely the ApplicationController, has a controller filter for finding a logged in user work in progress
Some controller, likely the WorksController, has a controller filter for finding a work no
The WorksController uses strong params yes
The WorksController's code style is clean, and focused on working with requests, responses, params, session, flash yes

Targed Test Review

Area yes / no
There are valid fixtures files used for users, votes, and works yoshi!!! mitski!!!! spiderverse!!!!!
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) yes, nice validation tests!
Work model has tests with sections on validations (valid and invalid) and relationships (has votes) yes
Work model has tests with a section on all business logic methods in the model, including their edge cases yes!! very thorough!

Overall Feedback

Linnea!! This project submission is great! Your test code is impeccable, and the functionality and webapp all look really good. I'm really happy with how you wrote your tests!

If you had time to do more refactoring, it'd be interesting to see:

  • Moving the tables of listing works into a partial
  • Creating controller filters

That being said, you did a great work with this project. I'm really sorry that this feedback is so late, so let me know what questions you have. Keep up the great, hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants