-
Notifications
You must be signed in to change notification settings - Fork 47
Ports - Shamira #41
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
base: master
Are you sure you want to change the base?
Ports - Shamira #41
Conversation
Media RankerWhat We're Looking For
|
| fixtures :all | ||
| # Add more helper methods to be used by all tests here... | ||
|
|
||
| # def perform_login(user = nil) |
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.
Why comment this out?
| describe User do | ||
| let(:user) { User.new } | ||
|
|
||
| it "must be valid" 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.
No other tests for user?
| describe Vote do | ||
| let(:vote) { Vote.new } | ||
|
|
||
| it "must be valid" 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.
No other tests for vote? What about a validation to ensure that the combination work_id and user_id is unique.
| end | ||
| end | ||
|
|
||
| describe "validations" 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.
No other validations/
| end | ||
|
|
||
| describe "topten" do | ||
| it "will return top votes" 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.
What about spotlight?
| redirect_to root_path | ||
| end | ||
|
|
||
| def upvote |
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 method is rather long, and nonfunctional on the site, but I suggest adding a method to the Usermodel for upvoting. That can help you reduce the size of your controller method here.
| @work = Work.find_by(id: params[:id]) | ||
|
|
||
| if !@work | ||
| head :not_found |
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.
I suggest redirecting instead of head at this point.
|
|
||
| has_many :votes | ||
| has_many :users, through: :votes | ||
|
|
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.
I suggest a spotlight method as well.
| Rails.application.routes.draw do | ||
| # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html | ||
| root to: "homepage#index" | ||
| resources :works, :users |
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.
Do you have all the restful routes as a user?
|
|
||
| <%= f.submit action_name, class:"work-form-btn"%> | ||
|
|
||
| <% end%> No newline at end of file |
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.
indentation...
Media Ranker
Congratulations! You're submitting your assignment!
Comprehension Questions
sessionandflash? What is the difference between them?