-
Notifications
You must be signed in to change notification settings - Fork 42
Ampers Sara S #45
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?
Ampers Sara S #45
Conversation
Media RankerWhat We're Looking For
|
|
|
||
| get '/login', to: 'sessions#new', as: 'login' | ||
| post '/login', to: 'sessions#create' | ||
| get '/logout', to: 'sessions#update', as: 'logout' |
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 would make this route a delete action instead of get.
| post '/logout', to: 'sessions#logout' | ||
|
|
||
| resources :works do | ||
| resources :upvotes, only: [:create, :new] |
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 need a new action for votes? Is there a form?
| session[:user_id] = @user.id | ||
| flash[:success] = "Successfully logged in as existing user #{@user.username}" | ||
| else | ||
| @user = User.create username: params[:user][:username] |
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 have some check to make sure that create worked properly.
| <ul class="errors"> | ||
| <% @work.errors.each do |column, message| %> | ||
| <li> | ||
| <strong><%= field.capitalize %></strong> <%= message %> |
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 crashing the app when I try to create a work without filling in any of the fields in the form.
field should be column
Lastly you should do this with flash notices.
| end | ||
|
|
||
| it 'must have a work' do | ||
| upvote = upvotes(:up_seventeen) |
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.
It's considered preferable to start with a valid model and then set one field to an invalid state and then check to see if that made the model invalid.
| require "test_helper" | ||
|
|
||
| describe Work do | ||
| let(:work) { Work.new } |
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.
Similar to the upvote tests above make the work valid and then take a way a specific field to make it invalid and then test that.
| work.get_upvotes_count.must_equal 0 | ||
| 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.
Missing tests for the custom methods.
Media Ranker
Congratulations! You're submitting your assignment!
Comprehension Questions
sessionandflash? What is the difference between them?