user deletions: fix error when given incorrect password.

Use validations instead of raising an exception when the password is
incorrect so that the controller can display errors sensibly.

Also fix users being logged out even when the deletion attempt failed
due to an incorrect password.

app/controllers/maintenance/user/deletions_controller.rb

@@ -1,14 +1,23 @@
 module Maintenance
   module User
     class DeletionsController < ApplicationController
+      respond_to :html, :json, :xml
+
       def show
       end
 
       def destroy
         deletion = UserDeletion.new(CurrentUser.user, params.dig(:user, :password))
         deletion.delete!
-        session.delete(:user_id)
-        redirect_to(posts_path, :notice => "You are now logged out")
+
+        if deletion.errors.none?
+          session.delete(:user_id)
+          flash[:notice] = "Your account has been deactivated"
+          respond_with(deletion, location: posts_path)
+        else
+          flash[:notice] = deletion.errors.full_messages.join("; ")
+          redirect_to maintenance_user_deletion_path
+        end
       end
     end
   end

app/logical/user_deletion.rb

@@ -1,7 +1,8 @@
 class UserDeletion
-  class ValidationError < StandardError; end
+  include ActiveModel::Validations
 
   attr_reader :user, :password
+  validate :validate_deletion
 
   def initialize(user, password)
     @user = user
@@ -9,13 +10,14 @@ class UserDeletion
   end
 
   def delete!
-    validate
+    return false if invalid?
     clear_user_settings
     remove_favorites
     clear_saved_searches
     rename
     reset_password
     create_mod_action
+    user
   end
 
   private
@@ -56,13 +58,13 @@ class UserDeletion
     request.save!(validate: false) # XXX don't validate so that the 1 name change per week rule doesn't interfere
   end
 
-  def validate
+  def validate_deletion
     if !user.authenticate_password(password)
-      raise ValidationError.new("Password is incorrect")
+      errors[:base] << "Password is incorrect"
     end
 
     if user.level >= User::Levels::ADMIN
-      raise ValidationError.new("Admins cannot delete their account")
+      errors[:base] << "Admins cannot delete their account"
     end
   end
 end

test/functional/maintenance/user/deletions_controller_test.rb

@@ -16,9 +16,20 @@ module Maintenance
         end
 
         context "#destroy" do
-          should "render" do
-            delete_auth maintenance_user_deletion_path, @user, params: {:password => "password"}
-            assert_redirected_to(posts_path)
+          should "delete the user when given the correct password" do
+            delete_auth maintenance_user_deletion_path, @user, params: { user: { password: "password" }}
+            assert_redirected_to posts_path
+            assert_equal(true, @user.reload.is_deleted?)
+            assert_equal("Your account has been deactivated", flash[:notice])
+            assert_nil(session[:user_id])
+          end
+
+          should "not delete the user when given an incorrect password" do
+            delete_auth maintenance_user_deletion_path, @user, params: { user: { password: "hunter2" }}
+            assert_redirected_to maintenance_user_deletion_path
+            assert_equal(false, @user.reload.is_deleted?)
+            assert_equal("Password is incorrect", flash[:notice])
+            assert_equal(@user.id, session[:user_id])
           end
         end
       end

test/unit/user_deletion_test.rb

@@ -6,10 +6,8 @@ class UserDeletionTest < ActiveSupport::TestCase
       should "fail" do
         @user = create(:user)
         @deletion = UserDeletion.new(@user, "wrongpassword")
-
-        assert_raise(UserDeletion::ValidationError) do
-          @deletion.delete!
-        end
+        @deletion.delete!
+        assert_includes(@deletion.errors[:base], "Password is incorrect")
       end
     end
 
@@ -17,10 +15,8 @@ class UserDeletionTest < ActiveSupport::TestCase
       should "fail" do
         @user = create(:admin_user)
         @deletion = UserDeletion.new(@user, "password")
-
-        assert_raise(UserDeletion::ValidationError) do
-          @deletion.delete!
-        end
+        @deletion.delete!
+        assert_includes(@deletion.errors[:base], "Admins cannot delete their account")
       end
     end
   end