lindsay - edges - mediaranker by elle-terch · Pull Request #41 · Ada-C10/MediaRanker

elle-terch · 2018-10-15T18:04:52Z

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question	Answer
Describe a custom model method you wrote.	In Work, I created a class method to tally total votes by work and then rank it from most votes to least. I used a query to take the votes table, join it with the works table and then group it by work.id. From there I used order to sort it from most votes to least (and had to troubleshoot some weird SQL stuff).
Describe how you approached testing that model method. What edge cases did you come up with?	I wrote the methods first. Once I had them working locally, I went through each method and created tests for each of them. I did not come up with edge cases.
What are `session` and `flash`? What is the difference between them?	Session and flash are special rails hashes. Flash displays a message to a user whereas session assigns a session id each time a user logs in (it persists until the user logs out).
Describe a controller filter you wrote.	I didn't get around to writing controller filters.
What was one thing that you gained more clarity on through this assignment?	In general, I gained more understanding of how all of the rails pieces fit together.
What is the Heroku URL of your deployed application?	https://terch-mediaranker.herokuapp.com/
Do you have any recommendations on how we could improve this project for the next cohort?	Maybe some more guidance on how much testing is expected for models. I did appreciate the open-ended nature of the project because it forced me to figure out what things to prioritize.

droberts-sea · 2018-10-27T00:44:40Z

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)	some extra routes
Views are well-organized (DRY, use of semantic HTML, use of partials)	semantic HTML is good, but views are not DRY - make sure to review partials
Errors are reported to the user	yes
Business logic lives in the models	yes
Models are thoroughly tested, including relations, validations and any custom logic	good start, missing some edge cases
Wave 1 - Media
Splash page shows the three media categories	yes
Basic CRUD operations on media are present and functional	Can't delete a work with votes!
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
A user cannot vote for the same media more than once	no
All media lists are ordered by vote count	yes
Splash page contains a media spotlight	yes
Wave 3 - Users and Votes
Media pages contain lists of voting users	yes
Individual user pages and the user list are present	yes
Optional - Styling
Bootstrap is used appropriately	nope - make sure to get some practice with this on API muncher if you didn't on bEtsy
Look and feel is similar to the original	workflow is similar
Overall	Great job overall! Your implementation matches the demo site very closely, and I would say the learning goals for this assignment were definitely met. There are a few places where things could be cleaned up or your test coverage could be expanded, which I've tried to outline below, but in general I am quite happy with this submission. Keep up the hard work!

droberts-sea · 2018-10-27T00:34:13Z

app/controllers/pages_controller.rb

+    @top_movies = Work.top_ten("movie")
+    @top_albums = Work.top_ten("album")
+
+    @top_media = Work.top_media


Your custom model methods make this controller action very clean - good work!

droberts-sea · 2018-10-27T00:36:37Z

app/models/user.rb

+  def media_votes
+    combined_works = []
+
+    self.votes.each do |vote|


Why not use a has_many :through relationship for this?

droberts-sea · 2018-10-27T00:37:45Z

app/models/vote.rb

+class Vote < ApplicationRecord
+  belongs_to :user
+  belongs_to :work
+


If you wanted to make it so that each user can only vote for a given work once, you could do it with a validation here.

droberts-sea · 2018-10-27T00:39:29Z

app/models/work.rb

+  def self.top_ten(category)
+    top_ten = self.ranked_media(category)
+    top_ten.first(10)
+  end


I love the way the methods in this model build on each other little by little - this is a great example of functional decomposition.

droberts-sea · 2018-10-27T00:40:31Z

app/views/works/index.html.erb

+<section class = "media-table">
+  <h4>Books</h4>
+  <table>
+    <thead>


You have the same code to show a list of works repeated 3 times. Could you use a view partial or a loop to DRY this up?

droberts-sea · 2018-10-27T00:41:07Z

app/views/works/edit.html.erb

+<h2>Edit this Work</h2>
+
+
+<%= form_with model: @work do |f| %>


You have the same form code here and in new.html.erb. You should be using a partial to DRY this up.

droberts-sea · 2018-10-27T00:41:52Z

config/routes.rb

+  end
+
+  resources :users
+


Do you need all 7 RESTful routes for user?

droberts-sea · 2018-10-27T00:43:29Z

test/models/work_test.rb

+
+      50.times do
+        book = Work.new(title: "Title#{x}", category: "Book")
+        book.save


I like this idea - test it by making a bunch of books in a loop. Good work!

droberts-sea · 2018-10-27T00:43:41Z

test/models/work_test.rb

+  describe 'top_ten' do
+
+    it 'returns only 10 entries of each category' do
+


I would also be interested in the following cases:

What if there are no works of that category?

What if there's a tie for last place, e.g. works 9, 10 and 11 all have 0 votes?

droberts-sea · 2018-10-27T00:43:59Z

test/models/work_test.rb

+  describe 'top_media' do
+
+    it 'has the most votes out of all media' do
+


I would also be interested in:

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?

elle-terch added 30 commits October 8, 2018 15:29

initial rails setup

03e4a0e

created model for users

d8d0ca7

created model for votes w/ foreign key for user

4dba26f

created model for work with corresponding foreign keys

c0d218b

added a creator column to works

7b2753b

created controller, route and view for the home page

a5c56b0

created route, view and controller for works

4c79280

created route, view and controller for users

753fe94

created show view and controller method for users

732ab5a

created show action for works

6d57833

started create action for works

a513e07

added action_view.rb for form_with

e0fb56a

created new form page and post action for new work

07d8979

created validation test for the work model

d2ce02c

created test for work-votes relationship

cad92f7

added fake data to yml files

278555c

refactored works test with yml data

c862444

changed data type of publication_year

2c21e8e

created view helpers for the homepage & called them from the index page

582fb97

created bootstrap form for a user to input a new work

18046c6

fixed edit work functionality

5bacc7e

fixed new work functionality

6164d0e

added delete functionality for a work

90d676a

added drop-down for category

009ab54

added flash messages for creating, updating and destroying media

f149128

added routes and controller actions for a user to log in

0b6005c

created a route, view and action for a user to log out

ff7fc21

changed name of login method

1c03c9a

started route, method and view for upvoting

0fbff86

fixed ability to add a new work

ac9627c

elle-terch added 23 commits October 12, 2018 16:02

fixed upvote functionality

8787ace

created logic to rank media from most voted to least

52cb2f1

cleaned up class methods to make them DRY

e41a380

added nav bar links to main application html

863e800

added basic layout css to nav bar

a683292

fixed validation error messages

137c002

added error message if dupicate work is created

d01c03b

rewrote the logic for sorting media by votes

6e1c840

wrote html for works index page

ba57ca2

pushing changes for testing

8d2ea73

wrote code to seed the database

8bda9b9

added model tests

6aecd9f

wrote relations tests for vote

e94fd51

created layout for index page

4ed378a

created method to iterate over each user vote

af5879c

updated styling to give tables more padding

978a56e

overall cleanup

4d55f4a

formatting changes

0aba933

updated seed files

0beac73

updated seeds.rb

c53aa46

made sure to downcase all media

92904ca

fixed validation

45155a7

removed id from seed data

9f33023

droberts-sea reviewed Oct 27, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lindsay - edges - mediaranker#41

lindsay - edges - mediaranker#41
elle-terch wants to merge 53 commits intoAda-C10:masterfrom
elle-terch:master

elle-terch commented Oct 15, 2018

Uh oh!

droberts-sea commented Oct 27, 2018

Uh oh!

droberts-sea Oct 27, 2018

Uh oh!

droberts-sea Oct 27, 2018

Uh oh!

droberts-sea Oct 27, 2018

Uh oh!

droberts-sea Oct 27, 2018

Uh oh!

droberts-sea Oct 27, 2018

Uh oh!

droberts-sea Oct 27, 2018

Uh oh!

droberts-sea Oct 27, 2018

Uh oh!

droberts-sea Oct 27, 2018

Uh oh!

droberts-sea Oct 27, 2018

Uh oh!

droberts-sea Oct 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		describe 'top_ten' do

		it 'returns only 10 entries of each category' do

		describe 'top_media' do

		it 'has the most votes out of all media' do

Conversation

elle-terch commented Oct 15, 2018

Media Ranker

Comprehension Questions

Uh oh!

droberts-sea commented Oct 27, 2018

Media Ranker

What We're Looking For

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants