-
Notifications
You must be signed in to change notification settings - Fork 48
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
MediaRanker - Divya #26
base: master
Are you sure you want to change the base?
Conversation
Media RankerWhat We're Looking For
|
|
||
resources :works | ||
|
||
resources :users, only: [:show, :index, :new, :create] |
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.
Since the sessions controller is doing the creating and the login form, do you really need the :new
and :create
actions here?
post '/login', to: 'sessions#create' | ||
delete '/logout', to: 'sessions#destroy' | ||
|
||
post 'works/:id/upvote', to: 'votes#upvote', as: 'new_vote' |
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 could also consider just nesting the votes#create
action in the works
routes.
validates :user_id, presence: true, uniqueness: { | ||
scope: :work_id | ||
} | ||
validates :work_id, presence: true |
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.
Since a Vote
belongs_to a Work
, this requirement is there by default.
def self.top(num) | ||
self.most_voted[0..(num-1)] | ||
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.
Having a works_by_category
method would be very helpful as well.
class Work < ApplicationRecord | ||
has_many :votes | ||
|
||
validates :title, presence: true, uniqueness: true |
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.
no validations on categories, or publication date etc?
|
||
<header class="app-header__header"> | ||
<h1> | ||
<a href="/">Media Ranker</a> |
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.
Instead of using anchor tags, use link_to
and named routes. It makes things easier to adjust.
|
||
<% if session[:user_id] %> | ||
<li class="nav-item app-header__nav_item"> | ||
<a class="btn btn-primary" |
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 don't need the anchor tag here. Since link_to
generates an anchor tag, you can style it with Bootstrap like this:
<%= link_to "Logged in as #{@user.username}", user_path(@user.id), class: "btn btn-primary" %>
|
||
result = user2.valid? | ||
|
||
result.must_equal false |
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 should also verify that the username
field is the problem.
result.count.must_equal expected_count | ||
end | ||
|
||
it "it returns a smaller collection if the argument is bigger than the number of works available" do |
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.
If the test deals with the number of items, you should test the number of items.
You should also test if there are no works.
</thead> | ||
<tbody> | ||
<tr> | ||
<td><a href="/users/#{@work.id}"><%[email protected]%></a></td> |
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're printing the work title here not the user name.
Media Ranker
Congratulations! You're submitting your assignment!
Comprehension Questions
session
andflash
? What is the difference between them?