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

Pipes - Sarah Read-Brown - MediaRanker #43

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

Conversation

SRBusiness
Copy link

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a custom model method you wrote. I wrote a method called sorted_works which returns an array of works for a given category, in descending order based on the number of votes each work has. This method was useful for the works index view and could be paired with the top_ten_works method for using on the home page.
Describe how you approached testing that model method. What edge cases did you come up with? I tried my best to follow along with the guidelines provides in the lecture for testing methods (2 tests per validation, at least one test per custom method). As I got more comfortable with the model tests I went back and refactored them using fixtures to dry things up. I wouldn't necessarily say I edge cased my model tests... I'm not really sure what that would look like in this context because the validations and strong params seem to already do some of that work for us.
Describe an edge case test you wrote for a controller I'm not sure if this counts but for the votes controller test I made sure that a user cannot vote for a work unless they are logged in.
What are session and flash? What is the difference between them? Session and flash are both special hash-like objects. Flash is used to make sure messages persist between request and is often used to give users feedback on the success or failure or their requests. Sessions is used to keep track of a users 'session' aka login information, unlike flash session data is stored indefinitely.
Describe a controller filter you wrote. I used the controller filter we build with dan in class in the works controller. It is a before_action that calls the find_works_by_params method on the show, update, edit, and destroy actions. These filters seem like they will be incredibly useful as we build larger scale projects.
What was one thing that you gained more clarity on through this assignment? Model methods and how to use them well to remove the burden of doing logic off of the controller and the view.
What is the Heroku URL of your deployed application https://srb-media-ranker.herokuapp.com/
Do you have any recommendations on how we could improve this project for the next cohort?

@droberts-sea
Copy link

Media Ranker

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes
Comprehension questions yes
General
Rails fundamentals (RESTful routing, use of named paths) yes
Semantic HTML yes
Errors are reported to the user yes
Business logic lives in the models yes - good work
Models are thoroughly tested, including relations, validations and any custom logic some missing cases around custom logic, but otherwise good. See inline comments
Controllers are thoroughly tested yes
Wave 1 - Media
Splash page shows the three media categories yes
Basic CRUD operations on media are present and functional yes
Wave 2 - Users and Votes
Users can log in and log out yes
The ID of the current user is stored in the session no - you store the ActiveRecord model instance itself. This appears to work, but is a bad idea for subtle reasons. In general it is better to store the user's ID than the user itself.
Individual user pages and the user list are present yes
A user cannot vote for the same media more than once yes
All media lists are ordered by vote count yes
Splash page contains a media spotlight yes
Media pages contain lists of voting users yes
Wave 3 - Styling
Foundation is used appropriately yes
Look and feel is similar to the original mostly - some layout pieces are different
Overall Great job overall! There's always some room for improvement around test coverage, but in general I am quite happy with what you've submitted. Keep up the hard work!

<section class="top-ten columns large-4 small-12">
<h3>Top <%= model.first.category.capitalize%>s</h3>
<ul>
<% model.each do |work| %>

Choose a reason for hiding this comment

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

Good use of a view partial here


it "returns works in order votes in descending order" do
sorted_albums.first.votes.count.must_be :>, sorted_albums.second.votes.count
sorted_albums.second.votes.count.must_be :>, sorted_albums.last.votes.count

Choose a reason for hiding this comment

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

You might want to use :>= here instead of :>. That way if two works have the same number of votes the test will still pass.

describe "top_ten_works" do
it "it will return up to ten works" do
Work.top_ten_works("album").length.must_be :<=, 10
end

Choose a reason for hiding this comment

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

I feel like there are more interesting edge cases here:

  • What if there are no works?
  • What if there are less than 10 works?
  • What if there are more than 10 works?

Work.sorted_works("albums").must_equal []
Work.sorted_works([1,2]).must_equal []
Work.sorted_works(2.3).must_equal []
end

Choose a reason for hiding this comment

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

I would also be interested to know whether this method works if there are no works.

end

describe "destroy" do
it "returns success, and destroys the work, if the work ID is valid " do

Choose a reason for hiding this comment

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

What if you attempt to destroy a work for which there are votes? Can you do it? What happens to the votes? This is exactly the sort of question that the end-to-end approach of a controller test is best for.

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