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

Carets - Maria - Media Ranker #35

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

Carets - Maria - Media Ranker #35

wants to merge 52 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 16, 2017

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a custom model method you wrote. I don't have one unfortunately. If I got to the 10 top media, I would use one there.
Describe how you approached testing that model method. What edge cases did you come up with? For models I don't know that I tested an edge case...
Describe an edge case test you wrote for a controller I wanted to try testing that it didn't create a vote if a user wasn't assigned, but once I got it working in the browser, the tests oddly didn't pass. I think that either my test isn't testing what I think it is or my create action isn't doing what I think it is.
What are session and flash? What is the difference between them? Session is the current "session" and lasts until the browser is cleared. Flash is the message for the current request.
Describe a controller filter you wrote. I used the before_action for finding a particular work.
What was one thing that you gained more clarity on through this assignment? Why my app is so buggy. It feels like things kept breaking or I had to keep resetting the database. I'm not proud of this project. I feel like basic things kept breaking even though I feel I understand the concepts and how to build a Rails app. Blah. Most of the functionality is there, but I wish I made more time for styling.
What is the Heroku URL of your deployed application https://maria-media-ranker.herokuapp.com/
Do you have any recommendations on how we could improve this project for the next cohort? Not particularly.

@tildeee
Copy link

tildeee commented Oct 22, 2017

Media Ranker

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene x
Comprehension questions x
General
Rails fundamentals (RESTful routing, use of named paths) x
Semantic HTML x
Errors are reported to the user x
Business logic lives in the models no business logic
Models are thoroughly tested, including relations, validations and any custom logic votes should require work_id and user_id
Controllers are thoroughly tested
Wave 1 - Media
Splash page shows the three media categories x
Basic CRUD operations on media are present and functional x
Wave 2 - Users and Votes
Users can log in and log out x
The ID of the current user is stored in the session x
Individual user pages and the user list are present x
A user cannot vote for the same media more than once
All media lists are ordered by vote count
Splash page contains a media spotlight
Media pages contain lists of voting users x
Wave 3 - Styling
Foundation is used appropriately x
Look and feel is similar to the original halfway there! More than halfway there!
Overall

@tildeee
Copy link

tildeee commented Oct 22, 2017

Overall, you got the basic functionality of the site, and you got logging in, flash messages, and voting working for the most part. Good job!

There were just a few major bugs:

  • Cannot delete work that has votes on it
  • Can vote on same work multiple times
  • No way of handling URLs with params that don't lead anywhere
    (as well as the missing feature that you noted of no media spotlight)

In terms of testing:
The biggest thing you lacked was testing the negative case in controller testing. What if I tried to go to http://localhost:3000/users/12000 where that user id doesn't exist? or for a work that doesn't exist? You could have caught this with testing

Also, in order to make your VotesController test pass, I've added some comments about making that work.

describe VotesController do
it "should create a vote" do
proc {
post votes_path, params: { vote: {user_id: 2, work_id: 9 }}
Copy link

Choose a reason for hiding this comment

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

Instead of adding in user_id into params[:vote][:user_id], you need to pass it in through session. To get user_id into session, before line 5 add the following line:

    post '/login', params: {username: users(:bob).username}

and then modify this line (line 6) to the following

      post votes_path, params: { vote: {work_id: 9 }}

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