Skip to content

Conversation

@pmanni02
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 to figure out which work to put into the media spotlight. The method uses an each do loop to determine which work has the max number of votes then returns that work.
Describe how you approached testing that model method. What edge cases did you come up with? For the spotlight method I used the edge case of if the input array is empty. An empty array would mean there are no works in the db. I tested to make sure the method returns nil. In a future iteration of this project, I would refactor the code to raise and rescue an Argument Error.
What are session and flash? What is the difference between them? A session is hash that is stored in the browser via a cookie. Flash displays messages.
Describe a controller filter you wrote. I wrote a filter to return the current session
What was one thing that you gained more clarity on through this assignment? Model relationships
What is the Heroku URL of your deployed application? https://media-pm.herokuapp.com/
Do you have any recommendations on how we could improve this project for the next cohort? Provide a few more examples of how to test model relationships.

@CheezItMan
Copy link

Media Ranker

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Check,
Comprehension questions Check
General
Rails fundamentals (RESTful routing, use of named paths) You have some unnecessary routes for votes with upvote
Semantic HTML
Errors are reported to the user for the most part, if I try to login with a blank username it tells me "Welcome," but I'm not logged in.
Business logic lives in the models Check, but see my notes in your code.
Models are thoroughly tested, including relations, validations and any custom logic You wrote some very nice tests here!
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, see my note in the code however.
Individual user pages and the user list are present 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
Media pages contain lists of voting users Check
Wave 3 - Styling
Foundation is used appropriately Nicely done!
Look and feel is similar to the original Looks very similar, nice work!
Overall Really nice work, you hit all the learning goals. I left some notes in your code over minor things. Slack me if you have questions.


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

resources :votes, only: [:new, :create]

Choose a reason for hiding this comment

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

Do you need a form or a create action for votes? Wouldn't upvote do just fine?

session[:user_id] = @user.id
flash[:success] = "Welcome back #{@user.name}!"
else
@user = User.create(user_params)

Choose a reason for hiding this comment

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

You should check to see if create here worked. Check to make sure validations passed!

belongs_to :work
belongs_to :user

validates :work_id, uniqueness: {scope: :user_id}

Choose a reason for hiding this comment

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

👍

validates :title, uniqueness: { scope: :category }
validates :published, format: { with: /\d{4}/ }, numericality: { less_than_or_equal_to: Time.now.strftime("%Y").to_i }

def self.make_category_hash

Choose a reason for hiding this comment

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

This isn't making a hash, but an array instead.

end

def self.spotlight
if Work.all != []

Choose a reason for hiding this comment

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

You should consider using an ennumerable like max_by or using ActiveRecord's .order method to do this.

or just use sort_by_vote you made below.

@@ -0,0 +1,38 @@
require "test_helper"

describe Vote do

Choose a reason for hiding this comment

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

🥇

work2.valid?.must_equal true
end

it "is not valid when published field is not 4 digit integer <= current year" do

Choose a reason for hiding this comment

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

🥇

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