Amber Lynn | Media Ranker Revisited - Edges #40
Conversation
…:edit, status: :bad_request.
…h and without users
MediaRanker RevistedWhat We're Looking For
|
| flash[:status] = :success | ||
| flash[:result_text] = "Successfully logged in as existing user #{user.username}" | ||
| flash[:messages] = user.errors.messages | ||
| flash[:result_text] = "Logged in as returning user #{user.name}" |
There was a problem hiding this comment.
You shouldn't need to set error messages if nothing went wrong
| user = User.new(username: username) | ||
| # User doesn't match anything in the DB | ||
| # TODO: Attempt to create a new user | ||
| user = User.new(username: auth_hash[:info][:nickname], uid: auth_hash[:uid], provider: 'github') |
| def destroy | ||
| session[:user_id] = nil | ||
| flash[:success] = "Successfully logged out!" | ||
|
|
There was a problem hiding this comment.
Looks like this and the logout path below are the same. You should only keep one or the other.
| redirect_to root_path | ||
| end | ||
|
|
||
| def login_form | ||
| end |
There was a problem hiding this comment.
Since we're using OAuth, do we still need the login form?
| # get root_path | ||
|
|
||
| # Precondition: there is at least one media of each category | ||
| # must_respond_with :success |
There was a problem hiding this comment.
Why is all of this commented out? These tests look good to me.
Remember that you can specify which test file to run:
rails test test/controllers/users_controller_test.rb
| get users_path | ||
|
|
||
| expect( response.body ).must_include("dan") | ||
| expect( response.body ).must_include("kari") |
There was a problem hiding this comment.
This is more work than we typically need to do for controller tests - you're looking at the rendered HTML, and making sure it includes these two values. If your fixture data or view code changes, this test will also need to change.
Needed more help understanding session controller testing and tests involving OAuth.