-
Notifications
You must be signed in to change notification settings - Fork 51
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
Leaves - Cloudy #53
base: master
Are you sure you want to change the base?
Leaves - Cloudy #53
Conversation
…nd models for work user abnd vote
…c for views show, new, index, and edit
…e homepages controller by adding index to it.
…ed 12 various works so i can ensure the top ten will work
…. checked validations
…. created custom method for top ten and spotlight.
…tml show the users who have voted.
root 'works#index' | ||
|
||
resources :works | ||
resources :homepages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unnecessary. You don't use the majority of these routes.
<h3>MOVIES</h3> | ||
<ul> | ||
<% movies = Work.topten(category: "movie") %> | ||
<% movies.each do |movie| %> | ||
<li> | ||
<%= link_to movie.title, work_path(movie.id) %> | ||
</li> | ||
</ul> | ||
<% end %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is repeated, you can use a view partial to DRY this up.
expect(book.save).must_equal false | ||
end | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need tests for your model methods.
|
||
<p><%= @work.description%></p> | ||
<p><%= link_to "Back to Media Ranks", works_path %></p> | ||
<p><%= link_to "Edit", works_path %></p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the wrong path. it should be edit_works_path
. You can get that information using rails routes
in the terminal
<p><%= @work.description%></p> | ||
<p><%= link_to "Back to Media Ranks", works_path %></p> | ||
<p><%= link_to "Edit", works_path %></p> | ||
<p><%= link_to "Upvote", vote_path(@work.id), method: :post %></p> | ||
<p><%= link_to "Delete", work_path(@work.id), method: :delete, data: {confirm: "Are you sure?"}%></p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a series of <p>
tags or a <ul>
with <li>
tags?
Media RankerWhat We're Looking ForManual Testing
Targeted Code Review
Targed Test Review
Overall FeedbackThis project is clearly still in-progress. It looks like a lot of the features got implemented most of the way, but there are some pretty clear problems outlined above. In the future, please tell your instructors about projects like this. Our goal is to help bridge the gap between where y'all are and where we need y'all to be, and part of that is being able to communicate about your progress. Let's take a minute to talk later this week. |
Media Ranker
Congratulations! You're submitting your assignment!
Comprehension Questions
session
andflash
? What is the difference between them?