Media Ranker Katherine #45

KatherineJF · 2018-10-15T19:23:21Z

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question	Answer
Describe a custom model method you wrote.	I didn't write custom methods in my models I created a scopes for my main page using Where to define categories based on media types.
Describe how you approached testing that model method. What edge cases did you come up with?	Create model tests that validate the correct media type by category. edge case is media type without a category.
What are `session` and `flash`? What is the difference between them?	Both used to persist data sessions throughout the actions of a user from one request to the next. Flash is different in that it only persists through a single request. Both are hashes
Describe a controller filter you wrote.	Wrote filter to track user currently logged in. Created a variable that matches user_id and session_id.
What was one thing that you gained more clarity on through this assignment?	Hard to narrow it down to one thing. How to use current user id to create a new object in this case vote and counter cache.
What is the Heroku URL of your deployed application?	https://mvoter.herokuapp.com/ - tad broken
Do you have any recommendations on how we could improve this project for the next cohort?	allow more time or make this the pair project. I was in coffee shops with classmates for 12 hours on Friday. Still didn't finish it all and I could have done the same amount of time Sunday and know people who did.

add new work

dHelmgren · 2018-10-29T04:41:24Z

Media Ranker

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene	Good!
Comprehension questions	Good!
General
Rails fundamentals (RESTful routing, use of named paths)	Need tuning up, see comments
Views are well-organized (DRY, use of semantic HTML, use of partials)	Mostly good, see comments
Errors are reported to the user	Mostly, see comments
Business logic lives in the models	No, see comments.
Models are thoroughly tested, including relations, validations and any custom logic	Mostly good, see comments
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
A user cannot vote for the same media more than once	Yes
All media lists are ordered by vote count	No, the All Media page is not, see comments
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	yes
Look and feel is similar to the original	yes
Overall	Your site works pretty well from a user's standpoint, but I'd like to see you leveraging the new tools we gave you for tracking user's data to improve the quality of your site overall. Also, the fact that you didn't write business logic in the model worries me, because I'm not sure if that was a decision by design (didn't feel it was necessary) or by necessity (couldn't get model methods working). This is a really good demo of the skills we've built so far, but I want to see you apply what you've learned in the last week.

dHelmgren · 2018-10-29T03:58:10Z

config/routes.rb

+  resources :works do
+  resources :votes, only: [:create, :new, :show]
+end
+resources :users


don't think you need all 7 restful routes for users - only index and show are needed for this project.

dHelmgren · 2018-10-29T04:01:31Z

config/routes.rb

+resources :users
+  resources :votes do
+    resources :works, only:[:create, :new, :show]
+    resources :users


Not sure what the use case is here? This gives you all 7 routes for users from a single vote, but you can never view a vote on its own. I'm not sure you need routes for votes period, because you can create a vote via a patch to a work (e.g. an upvote route).

dHelmgren · 2018-10-29T04:05:33Z

config/routes.rb

+
+  resources :works do
+  resources :votes, only: [:create, :new, :show]
+end


You need to keep an eye on your indentation. I know when you're writing code that it seems really unimportant, but the difference in readability between well indented code and poorly indented code is really significant.

dHelmgren · 2018-10-29T04:11:50Z

app/views/layouts/application.html.erb

+
+
+    <%= link_to "login", login_path %>
+    <%= link_to "logout", logout_path, :method => :delete, :data => {:confirm => "You Sure?"}%>


I encourage you to dig into how you could use session to select which option to show in HTML. Giving a user both a login and logout button is not only confusing in terms of workflow, but it's visual clutter.

dHelmgren · 2018-10-29T04:12:03Z

app/views/layouts/application.html.erb

+        <ul class="nav app-header__user-nav-container">
+
+          <li class="nav-item app-header__nav_item">
+          <%= link_to "login", login_path %>


duplicate login button

dHelmgren · 2018-10-29T04:25:14Z

app/controllers/main_controller.rb

+    @albums = Work.albums.order(votes_count: :desc).limit(10)
+    @movies = Work.movies.order(votes_count: :desc).limit(10)
+    @books = Work.books.order(votes_count: :desc).limit(10)
+    @spotlight = Work.order(votes_count: :desc).first


You make this same call 4 times, and while you can argue that this isn't "heavy lifting" I'd still say that it's business logic as the business of MediaRanker is sorting things by vote count. You can do the same sort of work with a method in your model, and in the future that's what I'd like to see you do.

dHelmgren · 2018-10-29T04:26:19Z

app/controllers/votes_controller.rb

+  end
+
+  def show
+  end


It's not a good practice to leave empty methods in your controller. If they aren't being used, delete these and the corresponding routes.

dHelmgren · 2018-10-29T04:30:32Z

app/controllers/works_controller.rb

+  def index
+      @albums = Work.where(category: "album")
+      @movies = Work.where(category: "movie")
+      @books = Work.where(category: "book")#paginate(page: params[:page], per_page: 10)


Your 'all works' page doesn't sort your media by votes because you don't call the sorting logic here. Again, you reduce the work you're doing and the likelihood of mistakes by creating code that relies on one method you use in many places.

dHelmgren · 2018-10-29T04:31:46Z

app/views/layouts/application.html.erb

+
+
+
+


This would be a good place to use Flash to show error messages to the user.

dHelmgren · 2018-10-29T04:32:48Z

test/models/vote_test.rb

+      # Assert
+      expect(user).must_be_instance_of User
+      expect(user.id).must_equal vote.user_id
+    end


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

KatherineJF added 19 commits October 9, 2018 18:11

Models Controllers

555c5bc

add new work

a509aa3

Merge pull request #1 from KatherineJF/add_new_work

479ca8d

add new work

user can login

5642758

session login and logout working

5d9a65c

Session working and vote

964e8b6

add work working

8be2a29

main styling

f18fb64

work show details done

4175e95

added counter cache

965605f

user summary working

bea2eb4

heroku issues

c83adc5

heroku troubleshooting

6314f38

main page styling

c82c099

heroku troubleshooting

dbd465e

fixed form layout and created work model test of categories

cbf891f

fix creator column on heroku

166dcdb

heroku fix attempt

aed6079

description and published year missing from heroku

bee07a2

dHelmgren reviewed Oct 30, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Media Ranker Katherine #45

Media Ranker Katherine #45

KatherineJF commented Oct 15, 2018

dHelmgren commented Oct 29, 2018

dHelmgren Oct 29, 2018

dHelmgren Oct 29, 2018

dHelmgren Oct 29, 2018

dHelmgren Oct 29, 2018

dHelmgren Oct 29, 2018

dHelmgren Oct 29, 2018

dHelmgren Oct 29, 2018

dHelmgren Oct 29, 2018

dHelmgren Oct 29, 2018

dHelmgren Oct 29, 2018



		<%= link_to "login", login_path %>
		<%= link_to "logout", logout_path, :method => :delete, :data => {:confirm => "You Sure?"}%>

Media Ranker Katherine #45

Are you sure you want to change the base?

Media Ranker Katherine #45

Conversation

KatherineJF commented Oct 15, 2018

Media Ranker

Comprehension Questions

dHelmgren commented Oct 29, 2018

Media Ranker

What We're Looking For

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment