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

Sockets - Cyndi #34

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

Sockets - Cyndi #34

wants to merge 60 commits into from

Conversation

cyndilopez
Copy link

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a custom model method you wrote. I wrote a class method media_votes(category), where given a category, a where query is called on the records of Work to return an array of works associated with that category.
Describe how you approached testing that model method. What edge cases did you come up with? I tested a nominal case where a category exists and works associated with that category also exist. I tested edge cases where a category is passed in that doesn't exist or works don't exist associated with that category. Both these cases are tested to return an empty array.
What are session and flash? What is the difference between them? Session is a type of hash that stores data during one request that can be read during other requests. A flash is essentially the same except that data only lasts through one request.
What was one thing that you gained more clarity on through this assignment? I gained more clarity on routes that are not part of RESTful routing.
What is the Heroku URL of your deployed application? https://cyndi-media-ranker.herokuapp.com/
Do you have any recommendations on how we could improve this project for the next cohort?

… vote count. added buttons at bottom page for works_path
…html code in work details page to display votes per work and table of users and date voted
…model method that returns user associated with session user id
… invalid work and in the update action, and to return a not found if the work doesn't exist in the destroy action
@CheezItMan
Copy link

Media Ranker

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits and good commit messages
Comprehension questions Check
General
Rails fundamentals (RESTful routing, use of named paths) Check your routes.rb file, you doubly created routes for workscontroller
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 Pretty good, I'd suggest an upvote method for the work model, to pull out some of the code you have in the controller and put it in the model.
Models are thoroughly tested, including relations, validations and any custom logic See my notes on testing, you've got some broken tests and some things to work on.
Controllers are thoroughly tested, including the login/logout process and multi-step workflows like voting for a work See my inline notes, the homepage controller tests are weirdly doing model testing, and you have some things to work on.
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 Looks good.
Overall You hit the learning goals here, but you had trouble with testing. See my inline notes and let me know what questions you have.

puts "Created work: #{media.inspect}"
end
end
puts "Added #{media.count} work records"

Choose a reason for hiding this comment

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

Notice that this variable was instantiated inside the forEach loop, and so doesn't have scope here at the puts line.

expect {
post work_votes_path(work.id)
}.must_change "Vote.count", 1
must_redirect_to root_path

Choose a reason for hiding this comment

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

This test is failing because creating a new vote redirects you to the work's show path.

list_all_movies = Work.media_votes("movie")
expect(list_all_movies[0]).must_equal monkey
expect(list_all_movies[1]).must_equal works(:avengers)
expect(list_all_movies[2]).must_equal works(:random8)

Choose a reason for hiding this comment

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

This is apparently the wrong work


describe "#user" do
it "finds the user associated with a user id" do
expect(User.user(user_valid.id)).must_equal user_valid

Choose a reason for hiding this comment

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

I'm not sure what this user method is doing that you couldn't do with User.find

end

describe "#vote_count" do
it "returns the number of votes taken by a given user" do

Choose a reason for hiding this comment

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

It would also be good to have a test when the user has no votes.

end
end

describe "update" do

Choose a reason for hiding this comment

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

Good tests for update

@@ -0,0 +1,16 @@
Rails.application.routes.draw do
# For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html
resources :works, only: [:index, :new, :create, :show, :edit, :update, :destroy]

Choose a reason for hiding this comment

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

Why do only:, you use all routes.


resources :users, only: [:index, :show]

resources :works do

Choose a reason for hiding this comment

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

Another set of routes for works?

<th scope ="col">Voted On</th>
</thead>
<tbody>
<%@user.works.each do |work|%>

Choose a reason for hiding this comment

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

Could you use this kind of table in a partial?

has_many :votes
has_many :works, through: :votes

def self.user(user_id)

Choose a reason for hiding this comment

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

I'm not sure the point of this method.

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