Merge pull request #1678 from codidact/0valt/1668/suggestions

Suggestions for PR #1668

Author: ArtOfCode-

app/controllers/users/registrations_controller.rb

@@ -32,13 +32,13 @@ def delete
   def do_delete
     @user = current_user
     if @user.admin?
-      @user.errors.add(:base, 'Admin accounts cannot be self-deleted. Contact support.')
+      @user.errors.add(:base, I18n.t('users.errors.no_admin_self_delete'))
       render :delete
     elsif @user.moderator?
-      @user.errors.add(:base, 'Moderator accounts cannot be self-deleted. Contact support.')
+      @user.errors.add(:base, I18n.t('users.errors.no_mod_self_delete'))
       render :delete
     elsif params[:username] != @user.username
-      @user.errors.add(:base, 'The username you entered was incorrect.')
+      @user.errors.add(:base, I18n.t('users.errors.self_delete_wrong_username'))
       render :delete
     else
       @user.do_soft_delete(@user)

config/locales/strings/en.users.yml

@@ -0,0 +1,9 @@
+en:
+  users:
+    errors:
+      no_admin_self_delete: >
+        Admin accounts cannot be self-deleted. Contact support.
+      no_mod_self_delete: >
+        Moderator accounts cannot be self-deleted. Contact support.
+      self_delete_wrong_username: >
+        The username you entered was incorrect.

test/controllers/sudo_controller_test.rb

@@ -17,7 +17,8 @@ class SudoControllerTest < ActionController::TestCase
 
   test 'should fail sudo mode with wrong password' do
     sign_in users(:standard_user)
-    post :enter_sudo, params: { password: 'wrong' }
+    try_enter_sudo('wrong')
+
     assert_response(:success)
     assert_equal 'The password you entered was incorrect.', flash[:danger]
   end
@@ -26,7 +27,8 @@ class SudoControllerTest < ActionController::TestCase
     set_password(users(:standard_user), 'test1234')
     sign_in users(:standard_user)
     session[:sudo_return] = users_me_path
-    post :enter_sudo, params: { password: 'test1234' }
+    try_enter_sudo('test1234')
+
     assert_response(:found)
     assert_redirected_to users_me_path
     assert_not_nil session[:sudo]
@@ -37,6 +39,12 @@ class SudoControllerTest < ActionController::TestCase
 
   private
 
+  # Attempts to enter sudo mode for the current user
+  # @param password [String] password of the user entering sudo mode
+  def try_enter_sudo(password)
+    post :enter_sudo, params: { password: password }
+  end
+
   def set_password(user, password)
     user.password = password
     user.skip_reconfirmation!

test/controllers/users/registrations_controller_test.rb

@@ -8,6 +8,7 @@ class Users::RegistrationsControllerTest < ActionController::TestCase
 
   test 'should register user' do
     try_register_user('test', 'test@example.com', 'testtest')
+
     assert_response(:found)
     assert_not_nil assigns(:user).id
     assert_redirected_to root_path
@@ -16,6 +17,7 @@ class Users::RegistrationsControllerTest < ActionController::TestCase
   test 'should prevent rapid registrations from same IP' do
     User.create(username: 'test', email: 'test2@example.com', password: 'testtest', current_sign_in_ip: '0.0.0.0')
     try_register_user('test', 'test@example.com', 'testtest')
+
     assert_response(:found)
     assert_redirected_to users_path
     assert_not_nil flash[:danger]
@@ -24,6 +26,7 @@ class Users::RegistrationsControllerTest < ActionController::TestCase
   test 'ensure Devise errors are handled properly' do
     existing_user = users(:standard_user)
     try_register_user(existing_user.username, existing_user.email, 'testtest')
+
     assert_response(:success)
     assert_not_empty assigns(:user).errors
   end
@@ -51,7 +54,8 @@ class Users::RegistrationsControllerTest < ActionController::TestCase
   test 'should delete user account' do
     sign_in users(:standard_user)
     session[:sudo] = DateTime.now.iso8601
-    post :do_delete, params: { username: users(:standard_user).username }
+    try_do_delete_user(users(:standard_user))
+
     assert_response(:found)
     assert_redirected_to root_path
     assert_equal 'Sorry to see you go!', flash[:info]
@@ -60,13 +64,15 @@ class Users::RegistrationsControllerTest < ActionController::TestCase
 
   test 'should require authentication to delete user account' do
     post :do_delete, params: { username: 'anything' }
+
     assert_response(:found)
     assert_redirected_to new_user_session_path
   end
 
   test 'should require sudo to delete user account' do
     sign_in users(:standard_user)
     post :do_delete, params: { username: 'anything' }
+
     assert_response(:found)
     assert_redirected_to user_sudo_path
   end
@@ -75,31 +81,38 @@ class Users::RegistrationsControllerTest < ActionController::TestCase
     sign_in users(:standard_user)
     session[:sudo] = DateTime.now.iso8601
     post :do_delete, params: { username: 'wrong' }
-    assert_response(:success)
-    assert_equal ['The username you entered was incorrect.'], assigns(:user).errors.full_messages
-    assert_not assigns(:user).deleted
-  end
 
-  test 'should prevent deletion of moderators' do
-    sign_in users(:moderator)
-    session[:sudo] = DateTime.now.iso8601
-    post :do_delete, params: { username: users(:moderator).username }
     assert_response(:success)
-    assert_equal ['Moderator accounts cannot be self-deleted. Contact support.'], assigns(:user).errors.full_messages
+    assert_equal [I18n.t('users.errors.self_delete_wrong_username')], assigns(:user).errors.full_messages
     assert_not assigns(:user).deleted
   end
 
-  test 'should prevent deletion of admins' do
-    sign_in users(:admin)
-    session[:sudo] = DateTime.now.iso8601
-    post :do_delete, params: { username: users(:admin).username }
-    assert_response(:success)
-    assert_equal ['Admin accounts cannot be self-deleted. Contact support.'], assigns(:user).errors.full_messages
-    assert_not assigns(:user).deleted
+  test 'should prevent self-deletion if the user is at least a moderator' do
+    locale_string_map = {
+      moderator: 'users.errors.no_mod_self_delete',
+      admin: 'users.errors.no_admin_self_delete'
+    }
+
+    [:moderator, :admin].each do |name|
+      sign_in users(name)
+      session[:sudo] = DateTime.now.iso8601
+
+      try_do_delete_user(users(name))
+
+      assert_response(:success)
+      assert_equal [I18n.t(locale_string_map[name])], assigns(:user).errors.full_messages
+      assert_not assigns(:user).deleted
+    end
   end
 
   private
 
+  # Attempts to sudo delete a given user
+  # @param user [User] user to delete
+  def try_do_delete_user(user)
+    post :do_delete, params: { username: user.username }
+  end
+
   def try_register_user(username, email, password)
     post :create, params: { user: { username: username, email: email, password: password,
                                     password_confirmation: password } }