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

Kate N - Sockets #51

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

Conversation

KateAnnNichols
Copy link

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a custom model method you wrote. In the works controller, I used a custom model for the vote function.
Describe how you approached testing that model method. What edge cases did you come up with? An edge case example for this model method would be if the user tries to vote on the same media more than once.
What are session and flash? What is the difference between them? Session tracks a logged in user, and is not saved when the user leaves. Flash sends the user messages, such as success or failure.
What was one thing that you gained more clarity on through this assignment? Testing
What is the Heroku URL of your deployed application? https://media-ranker-kan.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 This still has a lot of room for improvement. You should be following a regular add/commit process - think of it as writing a story for the reader to follow. On a project of this size I would expect somewhere between 40 and 100 commits.
Comprehension questions I'm looking for a little more detail in your discussion of flash and session. You've accurately described what they're used for, but not what they are or how they work. Both of these are hash-like objects provided by Rails. Both of them use cookies to send extra data to the client, which is then sent along with future requests. You can store whatever data you want there, though you're right about how they're typically used. The big difference between them is that flash only lasts for one additional request-response cycle, whereas session will persist until the user closes the browser.
General
Rails fundamentals (RESTful routing, use of named paths) yes
Views are well-organized (DRY, use of semantic HTML, use of partials) see inline
Errors are reported to the user yes
Business logic lives in the models kind of - much of the interesting logic is handled by the acts_as_votable gem, so I'm worried that you may not have hit the learning goal here yourself
Models are thoroughly tested, including relations, validations and any custom logic no
Controllers are thoroughly tested, including the login/logout process and multi-step workflows like voting for a work no
Wave 1 - Media
Splash page shows the three media categories yes
Basic CRUD operations on media are present and functional yes - missing the works index (this is different than the splash page), but I'm less worried about that.
Wave 2 - Users and Votes
Users can log in and log out Log in yes. For log out, everything is set up to handle this but there's no link to log out so no way for the user to do so.
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
Splash page contains a media spotlight kind of - you display a list of the top media, but the spotlight is supposed to be only one
Wave 3 - Users and Votes
Media pages contain lists of voting users looks like you've got a start on this, but I see #<Vote::ActiveRecord_Relation:0x00007fef7867e2a0> instead of a nicely-formatted list of users. You should be looping through this relation in order to display a table.
Individual user pages and the user list are present yes, though the link to the list of users points at the wrong place
Optional - Styling
Bootstrap is used appropriately some
Look and feel is similar to the original though there's not any styling, you've done a pretty good job of matching the workflows and functionality of the original
Overall See email

@@ -0,0 +1,9 @@
Rails.application.routes.draw do
resources :works
resources :users, only: %i[index show]

Choose a reason for hiding this comment

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

Good work only generating the routes you need for users.

def create
@work = Work.new(work_params)
@work.save
redirect_to work_path(@work.id)

Choose a reason for hiding this comment

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

What happens if the work fails to save?


def edit
@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.

What if the user typed a bad ID into the URL bar?

<h3>MOVIES</h3>
<ul>
<% @movies.each do |movie| %>
<li> <%= link_to "#{movie.title}", work_path(movie.id) %> </li>

Choose a reason for hiding this comment

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

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?

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