Kay Media Ranker #37
Kay Media Ranker #37
Conversation
…moved votes column from works
… they should be downcase
…dit method which finds the work that needs to be edited
…e and created user and vote models and controllers although i just realised vote doesn't need controller
… be kept track of via session
…r #upvote in the work controller
…its category, and created vote validation so a user can only vote on a single work once
…fer to and created validations for vote
…ed from Works child model Votes..pretty sure its working
…otes belonging to works, did this by creating new migration to add votes_count column and added counter_cache option in votes belongs to relationship
…ser who has voted on this work
…irects back to the same back
Media RankerWhat We're Looking For
|
| belongs_to :user | ||
| belongs_to :work, counter_cache: true | ||
| #vote can only be created on the same work by the same user once aka #each user can only upvote a work once | ||
| validates :user_id, uniqueness: {scope: :work_id, message:"You can only vote once"} |
There was a problem hiding this comment.
Good work getting these validations and relations figured out; they're tricky!
| class Work < ApplicationRecord | ||
| validates :title, presence: true | ||
| validates :title, uniqueness: { scope: :category } | ||
| validates_inclusion_of :category, in: %w(book album movie) |
There was a problem hiding this comment.
Good job figuring out validate_inclusion_of here, if you want to make it even more useful, consider putting those categories in a constant! That way, you can leverage the categories outside of this validation!
|
|
||
| def self.movies | ||
| return Work.where(category: 'movie').order(Arel.sql('votes_count IS NULL, votes_count DESC'), title: :asc) | ||
| end |
There was a problem hiding this comment.
These are not particularly DRY, we are essentially doing the same set of operations with slightly different names for those categories. Consider using a method with a single parameter category that selects the works by that parameter.
One more suggestion: While they aren't technically necessary, using intermediate variables to help break up these search parameters would make it easier for the reader, rather than having to parse a single line of code with lots of stuff going on.
| # def find_user | ||
| # @current_user = User.find_by(id: session[:user_id]) | ||
| #this finds the current user in the session so i can display it where i need | ||
| # end |
There was a problem hiding this comment.
This is commented out, but I feel like it shouldn't be. You've created a way of doing the job of a controller filter, but the result is less readable and less DRY.
| @books = Work.books | ||
| @movies = Work.movies | ||
|
|
||
| @current_user = User.find_by(id: session[:user_id]) |
There was a problem hiding this comment.
This makes the display for a user inconsistent; we see "Welcome {Name}" on any page where we are fetching @current_user, but the majority of your routes don't display who is logged in.
Putting find_user in a controller filter (as it seems you originally may have intended) would be the better solution.
|
|
||
| vote10: | ||
| user: user3 | ||
| user: work5 |
| end | ||
|
|
||
| describe 'Validations' do | ||
|
|
There was a problem hiding this comment.
You should be testing the uniqueness constraint here! Specific test cases I'd want to see:
- A user can have votes for two different works
- A work can have votes from two different users
- A user cannot vote for the same work twice
| albums_array = Work.albums #array of works that are albums | ||
| albums_array2 = Work.albums.sort | ||
|
|
||
| compare = albums_array.zip(albums_array2).map {|album, album2| album == album2} |
There was a problem hiding this comment.
This is a nice way to make sure that you aren't missing anything, and that they are correctly sorted!
| movies_array.each do |work| | ||
| expect(work.category).must_equal 'movie' | ||
| end | ||
| end |
There was a problem hiding this comment.
There's a lot of interesting test cases for your custom Work methods missing here. I would wonder:
- What happens if there are no works?
- What happens if there are works but no votes?
- What happens if two works have the same number of votes?
| <% flash.each do |name, message| %> | ||
| <div class="alert"-<%=name %> <%=message %> </div> | ||
| <% end %> | ||
| </section> |
There was a problem hiding this comment.
Your auto-indenter is not catching ERB very well. Your job is to help other developers read your code when you aren't there to explain it, so checking indentation should be a mandate for any serious developer!
Media Ranker
Congratulations! You're submitting your assignment!
Comprehension Questions
Describe how you approached testing that model method. What edge cases did you come up with? |
In general checking what happens when there is nil, one or many. I actually need to write more edge cases :/
What are
sessionandflash? What is the difference between them? |Flash and Session are both hashes. Flash tells the user something happened(success or error), session stores a piece of information(in my app its the user_id) in the browser in the cookies file which persists as the user goes to different pages in a website, while everything else gets re-set.
Describe a controller filter you wrote. |
I didn’t incorporate any controller filters yet.
What was one thing that you gained more clarity on through this assignment? |
I gained more clarity about Rails in terms of how the the routes, model, view and controller work together practically and where to look when an error occurred, I felt very confused about it still while working on RideShareRails. I also gained more clarity on how to write tests for the model.
What is the Heroku URL of your deployed application? |
https://kay-media-ranker.herokuapp.com/main/index
Do you have any recommendations on how we could improve this project for the next cohort? |
I could have used more time for this project, I felt like it was the first project I really started to understand how Rails works. Maybe doing this project before RideShare with a partner would help because each person gets to take their time to do extra learning in areas that they don’t understand yet, whereas when you’re working on a pair project it is more rushed.