Ever more tests...

.rubocop_todo.yml

@@ -37,7 +37,7 @@ Metrics/BlockNesting:
 # Offense count: 60
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 1628
+  Max: 1653
 
 # Offense count: 68
 Metrics/CyclomaticComplexity:

app/controllers/changeset_controller.rb

@@ -442,6 +442,8 @@ class ChangesetController < ApplicationController
     respond_to do |format|
       format.rss
     end
+  rescue OSM::APIBadUserInput
+    render :text => "", :status => :bad_request
   end
 
   private

app/controllers/user_controller.rb

@@ -466,8 +466,8 @@ class UserController < ApplicationController
     if request.post?
       ids = params[:user].keys.collect(&:to_i)
 
-      User.update_all("status = 'confirmed'", :id => ids) if params[:confirm]
+      User.where(:id => ids).update_all(:status => "confirmed") if params[:confirm]
-      User.update_all("status = 'deleted'", :id => ids) if params[:hide]
+      User.where(:id => ids).update_all(:status => "deleted") if params[:hide]
 
       redirect_to url_for(:status => params[:status], :ip => params[:ip], :page => params[:page])
     else

test/controllers/changeset_controller_test.rb

@@ -1816,7 +1816,7 @@ EOF
     assert_response :success
     assert_template "history"
 
-    get :list, :format => "html", :display_name => user.display_name, :list => "1"
+    xhr :get, :list, :format => "html", :display_name => user.display_name, :list => "1"
     assert_response :success
     assert_template "list"
 
@@ -1832,7 +1832,7 @@ EOF
     assert_response :success
     assert_template "history"
 
-    get :list, :format => "html", :display_name => user.display_name, :list => "1"
+    xhr :get, :list, :format => "html", :display_name => user.display_name, :list => "1"
     assert_response :success
     assert_template "list"
 
@@ -1846,7 +1846,7 @@ EOF
     assert_response :not_found
     assert_template "user/no_such_user"
 
-    get :list, :format => "html", :display_name => "Some random user", :list => "1"
+    xhr :get, :list, :format => "html", :display_name => "Some random user", :list => "1"
     assert_response :not_found
     assert_template "user/no_such_user"
   end
@@ -1866,7 +1866,7 @@ EOF
     assert_response :success
     assert_template "history"
 
-    get :list, :friends => true, :list => "1"
+    xhr :get, :list, :friends => true, :list => "1"
     assert_response :success
     assert_template "list"
 
@@ -1888,13 +1888,29 @@ EOF
     assert_response :success
     assert_template "history"
 
-    get :list, :nearby => true, :list => "1"
+    xhr :get, :list, :nearby => true, :list => "1"
     assert_response :success
     assert_template "list"
 
     check_list_result(Changeset.where(:user => user.nearby))
   end
 
+  ##
+  # Check that we can't request later pages of the changesets list
+  def test_list_max_id
+    xhr :get, :list, :format => "html", :max_id => 4
+    assert_response :success
+    assert_template "history"
+    assert_template :layout => "xhr"
+    assert_select "h2", :text => "Changesets", :count => 1
+
+    xhr :get, :list, :format => "html", :list => "1", :max_id => 4
+    assert_response :success
+    assert_template "list"
+
+    check_list_result(Changeset.where("id <= 4"))
+  end
+
   ##
   # This should display the last 20 changesets closed
   def test_feed
@@ -2138,75 +2154,77 @@ EOF
   def test_hide_comment_fail
     # unauthorized
     comment = changeset_comments(:normal_comment_1)
-    assert("comment.visible") do
+    assert_equal true, comment.visible
-      post :hide_comment, :id => comment.id
+
-      assert_response :unauthorized
+    post :hide_comment, :id => comment.id
-    end
+    assert_response :unauthorized
+    assert_equal true, comment.reload.visible
 
     basic_authorization(users(:public_user).email, "test")
 
     # not a moderator
-    assert("comment.visible") do
+    post :hide_comment, :id => comment.id
-      post :hide_comment, :id => comment.id
+    assert_response :forbidden
-      assert_response :forbidden
+    assert_equal true, comment.reload.visible
-    end
 
     basic_authorization(users(:moderator_user).email, "test")
 
     # bad comment id
     post :hide_comment, :id => 999111
     assert_response :not_found
+    assert_equal true, comment.reload.visible
   end
 
   ##
   # test hide comment succes
   def test_hide_comment_success
     comment = changeset_comments(:normal_comment_1)
+    assert_equal true, comment.visible
 
     basic_authorization(users(:moderator_user).email, "test")
 
-    assert("!comment.visible") do
+    post :hide_comment, :id => comment.id
-      post :hide_comment, :id => comment.id
-    end
     assert_response :success
+    assert_equal false, comment.reload.visible
   end
 
   ##
   # test unhide comment fail
   def test_unhide_comment_fail
     # unauthorized
-    comment = changeset_comments(:normal_comment_1)
+    comment = changeset_comments(:hidden_comment)
-    assert("comment.visible") do
+    assert_equal false, comment.visible
-      post :unhide_comment, :id => comment.id
+
-      assert_response :unauthorized
+    post :unhide_comment, :id => comment.id
-    end
+    assert_response :unauthorized
+    assert_equal false, comment.reload.visible
 
     basic_authorization(users(:public_user).email, "test")
 
     # not a moderator
-    assert("comment.visible") do
+    post :unhide_comment, :id => comment.id
-      post :unhide_comment, :id => comment.id
+    assert_response :forbidden
-      assert_response :forbidden
+    assert_equal false, comment.reload.visible
-    end
 
     basic_authorization(users(:moderator_user).email, "test")
 
     # bad comment id
     post :unhide_comment, :id => 999111
     assert_response :not_found
+    assert_equal false, comment.reload.visible
   end
 
   ##
   # test unhide comment succes
   def test_unhide_comment_success
-    comment = changeset_comments(:normal_comment_1)
+    comment = changeset_comments(:hidden_comment)
+    assert_equal false, comment.visible
 
     basic_authorization(users(:moderator_user).email, "test")
 
-    assert("!comment.visible") do
+    post :unhide_comment, :id => comment.id
-      post :unhide_comment, :id => comment.id
-    end
     assert_response :success
+    assert_equal true, comment.reload.visible
   end
 
   ##
@@ -2221,6 +2239,15 @@ EOF
       end
     end
 
+    get :comments_feed, :format => "rss", :limit => 2
+    assert_response :success
+    assert_equal "application/rss+xml", @response.content_type
+    assert_select "rss", :count => 1 do
+      assert_select "channel", :count => 1 do
+        assert_select "item", :count => 2
+      end
+    end
+
     get :comments_feed, :id => changesets(:normal_user_closed_change), :format => "rss"
     assert_response :success
     assert_equal "application/rss+xml", @response.content_type
@@ -2231,6 +2258,16 @@ EOF
     end
   end
 
+  ##
+  # test comments feed
+  def test_comments_feed_bad_limit
+    get :comments_feed, :format => "rss", :limit => 0
+    assert_response :bad_request
+
+    get :comments_feed, :format => "rss", :limit => 100001
+    assert_response :bad_request
+  end
+
   private
 
   ##

test/controllers/user_controller_test.rb

@@ -636,26 +636,78 @@ class UserControllerTest < ActionController::TestCase
     assert_redirected_to :controller => :user, :action => "login", :referer => "/user/test/account"
 
     # Make sure that you are blocked when not logged in as the right user
-    get :account, { :display_name => user.display_name }, { "user" => users(:public_user).id }
+    get :account, { :display_name => user.display_name }, { :user => users(:public_user).id }
     assert_response :forbidden
 
     # Make sure we get the page when we are logged in as the right user
-    get :account, { :display_name => user.display_name }, { "user" => user }
+    get :account, { :display_name => user.display_name }, { :user => user }
     assert_response :success
     assert_template :account
 
     # Updating the description should work
     user.description = "new description"
-    post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+    post :account, { :display_name => user.display_name, :user => user.attributes }, { :user => user.id }
     assert_response :success
     assert_template :account
     assert_select "div#errorExplanation", false
     assert_select ".notice", /^User information updated successfully/
     assert_select "form#accountForm > fieldset > div.form-row > div#user_description_container > div#user_description_content > textarea#user_description", user.description
 
+    # Changing to a invalid editor should fail
+    user.preferred_editor = "unknown"
+    post :account, { :display_name => user.display_name, :user => user.attributes }, { :user => user.id }
+    assert_response :success
+    assert_template :account
+    assert_select ".notice", false
+    assert_select "div#errorExplanation"
+    assert_select "form#accountForm > fieldset > div.form-row > select#user_preferred_editor > option[selected]", false
+
+    # Changing to a valid editor should work
+    user.preferred_editor = "potlatch2"
+    post :account, { :display_name => user.display_name, :user => user.attributes }, { :user => user.id }
+    assert_response :success
+    assert_template :account
+    assert_select "div#errorExplanation", false
+    assert_select ".notice", /^User information updated successfully/
+    assert_select "form#accountForm > fieldset > div.form-row > select#user_preferred_editor > option[selected][value=?]", "potlatch2"
+
+    # Changing to the default editor should work
+    user.preferred_editor = "default"
+    post :account, { :display_name => user.display_name, :user => user.attributes }, { :user => user.id }
+    assert_response :success
+    assert_template :account
+    assert_select "div#errorExplanation", false
+    assert_select ".notice", /^User information updated successfully/
+    assert_select "form#accountForm > fieldset > div.form-row > select#user_preferred_editor > option[selected]", false
+
+    # Changing to an uploaded image should work
+    image = Rack::Test::UploadedFile.new("test/traces/1.gif", "image/gif")
+    post :account, { :display_name => user.display_name, :image_action => "new", :user => user.attributes.merge(:image => image) }, { :user => user.id }
+    assert_response :success
+    assert_template :account
+    assert_select "div#errorExplanation", false
+    assert_select ".notice", /^User information updated successfully/
+    assert_select "form#accountForm > fieldset > div.form-row.accountImage input[name=image_action][checked][value=?]", "keep"
+
+    # Changing to a gravatar image should work
+    post :account, { :display_name => user.display_name, :image_action => "gravatar", :user => user.attributes }, { :user => user.id }
+    assert_response :success
+    assert_template :account
+    assert_select "div#errorExplanation", false
+    assert_select ".notice", /^User information updated successfully/
+    assert_select "form#accountForm > fieldset > div.form-row.accountImage input[name=image_action][checked][value=?]", "gravatar"
+
+    # Removing the image should work
+    post :account, { :display_name => user.display_name, :image_action => "delete", :user => user.attributes }, { :user => user.id }
+    assert_response :success
+    assert_template :account
+    assert_select "div#errorExplanation", false
+    assert_select ".notice", /^User information updated successfully/
+    assert_select "form#accountForm > fieldset > div.form-row.accountImage input[name=image_action][checked]", false
+
     # Changing name to one that exists should fail
     new_attributes = user.attributes.dup.merge(:display_name => users(:public_user).display_name)
-    post :account, { :display_name => user.display_name, :user => new_attributes }, { "user" => user.id }
+    post :account, { :display_name => user.display_name, :user => new_attributes }, { :user => user.id }
     assert_response :success
     assert_template :account
     assert_select ".notice", false
@@ -664,7 +716,7 @@ class UserControllerTest < ActionController::TestCase
 
     # Changing name to one that exists should fail, regardless of case
     new_attributes = user.attributes.dup.merge(:display_name => users(:public_user).display_name.upcase)
-    post :account, { :display_name => user.display_name, :user => new_attributes }, { "user" => user.id }
+    post :account, { :display_name => user.display_name, :user => new_attributes }, { :user => user.id }
     assert_response :success
     assert_template :account
     assert_select ".notice", false
@@ -673,7 +725,7 @@ class UserControllerTest < ActionController::TestCase
 
     # Changing name to one that doesn't exist should work
     new_attributes = user.attributes.dup.merge(:display_name => "new tester")
-    post :account, { :display_name => user.display_name, :user => new_attributes }, { "user" => user.id }
+    post :account, { :display_name => user.display_name, :user => new_attributes }, { :user => user.id }
     assert_response :success
     assert_template :account
     assert_select "div#errorExplanation", false
@@ -686,7 +738,7 @@ class UserControllerTest < ActionController::TestCase
     # Changing email to one that exists should fail
     user.new_email = users(:public_user).email
     assert_no_difference "ActionMailer::Base.deliveries.size" do
-      post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+      post :account, { :display_name => user.display_name, :user => user.attributes }, { :user => user.id }
     end
     assert_response :success
     assert_template :account
@@ -697,7 +749,7 @@ class UserControllerTest < ActionController::TestCase
     # Changing email to one that exists should fail, regardless of case
     user.new_email = users(:public_user).email.upcase
     assert_no_difference "ActionMailer::Base.deliveries.size" do
-      post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+      post :account, { :display_name => user.display_name, :user => user.attributes }, { :user => user.id }
     end
     assert_response :success
     assert_template :account
@@ -708,7 +760,7 @@ class UserControllerTest < ActionController::TestCase
     # Changing email to one that doesn't exist should work
     user.new_email = "new_tester@example.com"
     assert_difference "ActionMailer::Base.deliveries.size", 1 do
-      post :account, { :display_name => user.display_name, :user => user.attributes }, { "user" => user.id }
+      post :account, { :display_name => user.display_name, :user => user.attributes }, { :user => user.id }
     end
     assert_response :success
     assert_template :account
@@ -1126,6 +1178,166 @@ class UserControllerTest < ActionController::TestCase
     assert_equal "deleted", user.status
   end
 
+  def test_list_get
+    # Shouldn't work when not logged in
+    get :list
+    assert_response :redirect
+    assert_redirected_to :action => :login, :referer => users_path
+
+    session[:user] = users(:normal_user).id
+
+    # Shouldn't work when logged in as a normal user
+    get :list
+    assert_response :redirect
+    assert_redirected_to :action => :login, :referer => users_path
+
+    session[:user] = users(:moderator_user).id
+
+    # Shouldn't work when logged in as a moderator
+    get :list
+    assert_response :redirect
+    assert_redirected_to :action => :login, :referer => users_path
+
+    session[:user] = users(:administrator_user).id
+
+    # Should work when logged in as an administrator
+    get :list
+    assert_response :success
+    assert_template :list
+    assert_select "table#user_list tr", :count => User.count + 1
+
+    # Should be able to limit by status
+    get :list, :status => "suspended"
+    assert_response :success
+    assert_template :list
+    assert_select "table#user_list tr", :count => User.where(:status => "suspended").count + 1
+
+    # Should be able to limit by IP address
+    get :list, :ip => "1.2.3.4"
+    assert_response :success
+    assert_template :list
+    assert_select "table#user_list tr", :count => User.where(:creation_ip => "1.2.3.4").count + 1
+  end
+
+  def test_list_get_paginated
+    1.upto(100).each do |n|
+      User.create(:display_name => "extra_#{n}",
+                  :email => "extra#{n}@example.com",
+                  :pass_crypt => "extraextra")
+    end
+
+    session[:user] = users(:administrator_user).id
+
+    get :list
+    assert_response :success
+    assert_template :list
+    assert_select "table#user_list tr", :count => 51
+
+    get :list, :page => 2
+    assert_response :success
+    assert_template :list
+    assert_select "table#user_list tr", :count => 51
+
+    get :list, :page => 3
+    assert_response :success
+    assert_template :list
+    assert_select "table#user_list tr", :count => 19
+  end
+
+  def test_list_post_confirm
+    inactive_user = users(:inactive_user)
+    suspended_user = users(:suspended_user)
+
+    # Shouldn't work when not logged in
+    assert_no_difference "User.active.count" do
+      post :list, :confirm => 1, :user => { inactive_user.id => 1, suspended_user.id => 1 }
+    end
+    assert_response :redirect
+    assert_redirected_to :action => :login, :referer => users_path(:confirm => 1, :user => { inactive_user.id => 1, suspended_user.id => 1 })
+    assert_equal "pending", inactive_user.reload.status
+    assert_equal "suspended", suspended_user.reload.status
+
+    session[:user] = users(:normal_user).id
+
+    # Shouldn't work when logged in as a normal user
+    assert_no_difference "User.active.count" do
+      post :list, :confirm => 1, :user => { inactive_user.id => 1, suspended_user.id => 1 }
+    end
+    assert_response :redirect
+    assert_redirected_to :action => :login, :referer => users_path(:confirm => 1, :user => { inactive_user.id => 1, suspended_user.id => 1 })
+    assert_equal "pending", inactive_user.reload.status
+    assert_equal "suspended", suspended_user.reload.status
+
+    session[:user] = users(:moderator_user).id
+
+    # Shouldn't work when logged in as a moderator
+    assert_no_difference "User.active.count" do
+      post :list, :confirm => 1, :user => { inactive_user.id => 1, suspended_user.id => 1 }
+    end
+    assert_response :redirect
+    assert_redirected_to :action => :login, :referer => users_path(:confirm => 1, :user => { inactive_user.id => 1, suspended_user.id => 1 })
+    assert_equal "pending", inactive_user.reload.status
+    assert_equal "suspended", suspended_user.reload.status
+
+    session[:user] = users(:administrator_user).id
+
+    # Should work when logged in as an administrator
+    assert_difference "User.active.count", 2 do
+      post :list, :confirm => 1, :user => { inactive_user.id => 1, suspended_user.id => 1 }
+    end
+    assert_response :redirect
+    assert_redirected_to :action => :list
+    assert_equal "confirmed", inactive_user.reload.status
+    assert_equal "confirmed", suspended_user.reload.status
+  end
+
+  def test_list_post_hide
+    normal_user = users(:normal_user)
+    confirmed_user = users(:confirmed_user)
+
+    # Shouldn't work when not logged in
+    assert_no_difference "User.active.count" do
+      post :list, :hide => 1, :user => { normal_user.id => 1, confirmed_user.id => 1 }
+    end
+    assert_response :redirect
+    assert_redirected_to :action => :login, :referer => users_path(:hide => 1, :user => { normal_user.id => 1, confirmed_user.id => 1 })
+    assert_equal "active", normal_user.reload.status
+    assert_equal "confirmed", confirmed_user.reload.status
+
+    session[:user] = users(:normal_user).id
+
+    # Shouldn't work when logged in as a normal user
+    assert_no_difference "User.active.count" do
+      post :list, :hide => 1, :user => { normal_user.id => 1, confirmed_user.id => 1 }
+    end
+    assert_response :redirect
+    assert_redirected_to :action => :login, :referer => users_path(:hide => 1, :user => { normal_user.id => 1, confirmed_user.id => 1 })
+    assert_equal "active", normal_user.reload.status
+    assert_equal "confirmed", confirmed_user.reload.status
+
+    session[:user] = users(:moderator_user).id
+
+    # Shouldn't work when logged in as a moderator
+    assert_no_difference "User.active.count" do
+      post :list, :hide => 1, :user => { normal_user.id => 1, confirmed_user.id => 1 }
+    end
+    assert_response :redirect
+    assert_redirected_to :action => :login, :referer => users_path(:hide => 1, :user => { normal_user.id => 1, confirmed_user.id => 1 })
+    assert_equal "active", normal_user.reload.status
+    assert_equal "confirmed", confirmed_user.reload.status
+
+    session[:user] = users(:administrator_user).id
+
+    # Should work when logged in as an administrator
+    assert_difference "User.active.count", -2 do
+      post :list, :hide => 1, :user => { normal_user.id => 1, confirmed_user.id => 1 }
+    end
+    assert_response :redirect
+    assert_redirected_to :action => :list
+    assert_equal "deleted", normal_user.reload.status
+    assert_equal "deleted", confirmed_user.reload.status
+  end
+
   private
 
   def new_user

test/fixtures/users.yml

@@ -14,7 +14,8 @@ normal_user:
   terms_agreed: "2010-01-01 11:22:33"
   terms_seen: true
   languages: en
-  
+  creation_ip: "1.2.3.4"
+
 public_user:
   id: 2
   email: test@example.com
@@ -30,7 +31,8 @@ public_user:
   terms_agreed: "2010-01-01 11:22:33"
   terms_seen: true
   languages: en
-  
+  creation_ip: "4.5.6.7"
+
 inactive_user:
   id: 3
   email: inactive@openstreetmap.org
@@ -63,6 +65,7 @@ second_public_user:
   home_zoom: 12
   terms_agreed: "2010-01-01 11:22:33"
   terms_seen: true
+  creation_ip: "1.2.3.4"
 
 moderator_user:
   id: 5

test/integration/user_login_test.rb

@@ -17,17 +17,17 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
 
-    post "/login", "username" => user.email, "password" => "wrong", :referer => "/history"
+    post "/login", :username => user.email, :password => "wrong", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "login"
 
-    post "/login", "username" => user.email, "password" => "test", :referer => "/history"
+    post "/login", :username => user.email, :password => "test", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
@@ -40,17 +40,17 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
 
-    post "/login", "username" => user.email.upcase, "password" => "wrong", :referer => "/history"
+    post "/login", :username => user.email.upcase, :password => "wrong", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "login"
 
-    post "/login", "username" => user.email.upcase, "password" => "test", :referer => "/history"
+    post "/login", :username => user.email.upcase, :password => "test", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
@@ -63,17 +63,17 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
 
-    post "/login", "username" => user.email.titlecase, "password" => "wrong", :referer => "/history"
+    post "/login", :username => user.email.titlecase, :password => "wrong", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "login"
 
-    post "/login", "username" => user.email.titlecase, "password" => "test", :referer => "/history"
+    post "/login", :username => user.email.titlecase, :password => "test", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
@@ -86,17 +86,17 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
 
-    post "/login", "username" => user.email, "password" => "wrong", :referer => "/history"
+    post "/login", :username => user.email, :password => "wrong", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "login"
 
-    post "/login", "username" => user.email, "password" => "test", :referer => "/history"
+    post "/login", :username => user.email, :password => "test", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
@@ -109,17 +109,17 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
 
-    post "/login", "username" => user.email.upcase, "password" => "wrong", :referer => "/history"
+    post "/login", :username => user.email.upcase, :password => "wrong", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "login"
 
-    post "/login", "username" => user.email.upcase, "password" => "test", :referer => "/history"
+    post "/login", :username => user.email.upcase, :password => "test", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
@@ -132,17 +132,17 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
 
-    post "/login", "username" => user.email.titlecase, "password" => "wrong", :referer => "/history"
+    post "/login", :username => user.email.titlecase, :password => "wrong", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "login"
 
-    post "/login", "username" => user.email.titlecase, "password" => "test", :referer => "/history"
+    post "/login", :username => user.email.titlecase, :password => "test", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
@@ -150,22 +150,157 @@ class UserLoginTest < ActionDispatch::IntegrationTest
     assert_select "span.username", "test2"
   end
 
-  def test_login_username_password_normal
+  def test_login_email_password_inactive
-    user = users(:normal_user)
+    user = users(:inactive_user)
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
 
-    post "/login", "username" => user.display_name, "password" => "wrong", :referer => "/history"
+    post "/login", :username => user.email, :password => "wrong", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "login"
 
-    post "/login", "username" => user.display_name, "password" => "test", :referer => "/history"
+    post "/login", :username => user.email, :password => "test2", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "confirm"
+  end
+
+  def test_login_email_password_inactive_upcase
+    user = users(:inactive_user)
+
+    get "/login"
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
+    follow_redirect!
+    assert_response :success
+
+    post "/login", :username => user.email.upcase, :password => "wrong", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    post "/login", :username => user.email.upcase, :password => "test2", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "confirm"
+  end
+
+  def test_login_email_password_inactive_titlecase
+    user = users(:inactive_user)
+
+    get "/login"
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
+    follow_redirect!
+    assert_response :success
+
+    post "/login", :username => user.email.titlecase, :password => "wrong", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    post "/login", :username => user.email.titlecase, :password => "test2", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "confirm"
+  end
+
+  def test_login_email_password_suspended
+    user = users(:suspended_user)
+
+    get "/login"
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
+    follow_redirect!
+    assert_response :success
+
+    post "/login", :username => user.email, :password => "wrong", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    post "/login", :username => user.email, :password => "test", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+    assert_select "div.flash.error", /your account has been suspended/
+  end
+
+  def test_login_email_password_suspended_upcase
+    user = users(:suspended_user)
+
+    get "/login"
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
+    follow_redirect!
+    assert_response :success
+
+    post "/login", :username => user.email.upcase, :password => "wrong", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    post "/login", :username => user.email.upcase, :password => "test", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+    assert_select "div.flash.error", /your account has been suspended/
+  end
+
+  def test_login_email_password_suspended_titlecase
+    user = users(:suspended_user)
+
+    get "/login"
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
+    follow_redirect!
+    assert_response :success
+
+    post "/login", :username => user.email.titlecase, :password => "wrong", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    post "/login", :username => user.email.titlecase, :password => "test", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+    assert_select "div.flash.error", /your account has been suspended/
+  end
+
+  def test_login_username_password_normal
+    user = users(:normal_user)
+
+    get "/login"
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
+    follow_redirect!
+    assert_response :success
+
+    post "/login", :username => user.display_name, :password => "wrong", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    post "/login", :username => user.display_name, :password => "test", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
@@ -178,17 +313,17 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
 
-    post "/login", "username" => user.display_name.upcase, "password" => "wrong", :referer => "/history"
+    post "/login", :username => user.display_name.upcase, :password => "wrong", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "login"
 
-    post "/login", "username" => user.display_name.upcase, "password" => "test", :referer => "/history"
+    post "/login", :username => user.display_name.upcase, :password => "test", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
@@ -201,17 +336,17 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
 
-    post "/login", "username" => user.display_name.titlecase, "password" => "wrong", :referer => "/history"
+    post "/login", :username => user.display_name.titlecase, :password => "wrong", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "login"
 
-    post "/login", "username" => user.display_name.titlecase, "password" => "test", :referer => "/history"
+    post "/login", :username => user.display_name.titlecase, :password => "test", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
@@ -224,17 +359,17 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
 
-    post "/login", "username" => user.display_name, "password" => "wrong", :referer => "/history"
+    post "/login", :username => user.display_name, :password => "wrong", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "login"
 
-    post "/login", "username" => user.display_name, "password" => "test", :referer => "/history"
+    post "/login", :username => user.display_name, :password => "test", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
@@ -247,17 +382,17 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
 
-    post "/login", "username" => user.display_name.upcase, "password" => "wrong", :referer => "/history"
+    post "/login", :username => user.display_name.upcase, :password => "wrong", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "login"
 
-    post "/login", "username" => user.display_name.upcase, "password" => "test", :referer => "/history"
+    post "/login", :username => user.display_name.upcase, :password => "test", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
@@ -270,17 +405,17 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
 
-    post "/login", "username" => user.display_name.titlecase, "password" => "wrong", :referer => "/history"
+    post "/login", :username => user.display_name.titlecase, :password => "wrong", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "login"
 
-    post "/login", "username" => user.display_name.titlecase, "password" => "test", :referer => "/history"
+    post "/login", :username => user.display_name.titlecase, :password => "test", :referer => "/history"
     assert_response :redirect
     follow_redirect!
     assert_response :success
@@ -288,15 +423,150 @@ class UserLoginTest < ActionDispatch::IntegrationTest
     assert_select "span.username", "test2"
   end
 
+  def test_login_username_password_inactive
+    user = users(:inactive_user)
+
+    get "/login"
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
+    follow_redirect!
+    assert_response :success
+
+    post "/login", :username => user.display_name, :password => "wrong", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    post "/login", :username => user.display_name, :password => "test2", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "confirm"
+  end
+
+  def test_login_username_password_inactive_upcase
+    user = users(:inactive_user)
+
+    get "/login"
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
+    follow_redirect!
+    assert_response :success
+
+    post "/login", :username => user.display_name.upcase, :password => "wrong", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    post "/login", :username => user.display_name.upcase, :password => "test2", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "confirm"
+  end
+
+  def test_login_username_password_inactive_titlecase
+    user = users(:inactive_user)
+
+    get "/login"
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
+    follow_redirect!
+    assert_response :success
+
+    post "/login", :username => user.display_name.titlecase, :password => "wrong", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    post "/login", :username => user.display_name.titlecase, :password => "test2", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "confirm"
+  end
+
+  def test_login_username_password_suspended
+    user = users(:suspended_user)
+
+    get "/login"
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
+    follow_redirect!
+    assert_response :success
+
+    post "/login", :username => user.display_name, :password => "wrong", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    post "/login", :username => user.display_name, :password => "test", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+    assert_select "div.flash.error", /your account has been suspended/
+  end
+
+  def test_login_username_password_suspended_upcase
+    user = users(:suspended_user)
+
+    get "/login"
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
+    follow_redirect!
+    assert_response :success
+
+    post "/login", :username => user.display_name.upcase, :password => "wrong", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    post "/login", :username => user.display_name.upcase, :password => "test", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+    assert_select "div.flash.error", /your account has been suspended/
+  end
+
+  def test_login_username_password_suspended_titlecase
+    user = users(:suspended_user)
+
+    get "/login"
+    assert_response :redirect
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
+    follow_redirect!
+    assert_response :success
+
+    post "/login", :username => user.display_name.titlecase, :password => "wrong", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    post "/login", :username => user.display_name.titlecase, :password => "test", :referer => "/history"
+    assert_response :redirect
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+    assert_select "div.flash.error", /your account has been suspended/
+  end
+
   def test_login_openid_success
     OmniAuth.config.add_mock(:openid, :uid => "http://localhost:1123/john.doe")
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
-    post "/login", "openid_url" => "http://localhost:1123/john.doe", :referer => "/history"
+    post "/login", :openid_url => "http://localhost:1123/john.doe", :referer => "/history"
     assert_response :redirect
     assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login")
     follow_redirect!
@@ -315,10 +585,10 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
-    post "/login", "openid_url" => "http://localhost:1123/john.doe", :referer => "/history"
+    post "/login", :openid_url => "http://localhost:1123/john.doe", :referer => "/history"
     assert_response :redirect
     assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login")
     follow_redirect!
@@ -341,10 +611,10 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
-    post "/login", "openid_url" => "http://localhost:1123/john.doe", :referer => "/history"
+    post "/login", :openid_url => "http://localhost:1123/john.doe", :referer => "/history"
     assert_response :redirect
     assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login")
     follow_redirect!
@@ -367,10 +637,10 @@ class UserLoginTest < ActionDispatch::IntegrationTest
 
     get "/login"
     assert_response :redirect
-    assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
+    assert_redirected_to :controller => :user, :action => :login, :cookie_test => true
     follow_redirect!
     assert_response :success
-    post "/login", "openid_url" => "http://localhost:1123/fred.bloggs", :referer => "/diary"
+    post "/login", :openid_url => "http://localhost:1123/fred.bloggs", :referer => "/diary"
     assert_response :redirect
     assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/fred.bloggs", :origin => "/login")
     follow_redirect!