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 - Sara Frandsen - Media Ranker (late) #47

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

Conversation

frankienakama
Copy link

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a custom model method you wrote. In self.top_ten(category), it orders each work category by votes in descending order, and returns the first ten in the list. This gives us the top ten works for each category.
Describe how you approached testing that model method. What edge cases did you come up with? TBH I did not do any testing.
Describe an edge case test you wrote for a controller
What are session and flash? What is the difference between them? Sessions act like hashes that sort of stick around without needing to exist in the database. Flash objects are the same, but disappear after a single request.
Describe a controller filter you wrote.
What was one thing that you gained more clarity on through this assignment? Using the model the write methods in! USING GIT AND FIXING THE THINGS I BREAK
What is the Heroku URL of your deployed application https://sara-frandsen-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 - some extra routes
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 no
Controllers are thoroughly tested no
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 yes
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 some - seems like there's still a lot of ground to explore here
Look and feel is similar to the original some
Overall Good job overall. This is a big, difficult project, and while some of the pieces weren't quite there (particularly around styling and testing), you were able to get the core functionality working. This is no mean feat. Make sure you take the opportunity to practice these on the bEtsy project, and keep up the hard work!

root 'works#main'

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

Choose a reason for hiding this comment

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

I'm not sure if you need the full resources for :sessions here, especially since you've got login/logout listed explicitly below.

@@ -0,0 +1,9 @@
<%# partial view for top 10 lists %>
<article>

Choose a reason for hiding this comment

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

Great use of a partial to DRY this up!

def upvote(work_id)
work = Work.find(work_id)
if Vote.where(work: work, user: self).count > 0
return false, 'Cannot upvote more than once!'

Choose a reason for hiding this comment

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

I like this as a model method! Good work!

def show
# individual work
@work = Work.find(params[:id])
end

Choose a reason for hiding this comment

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

This would be a great bit of code to DRY up using a controller filter, especially if you extend the functionality to return 404 not_found if that model instance doesn't exist.

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