Add latest changes from gitlab-org/gitlab@14-3-stable-eev14.3.0-rc42

7 files changed, 50 insertions, 21 deletions

diff --git a/app/services/users/ban_service.rb b/app/services/users/ban_service.rb
index 88e92ebff9b..959d4be3795 100644
--- a/app/services/users/ban_service.rb
+++ b/app/services/users/ban_service.rb
@@ -8,6 +8,10 @@ module Users
       user.ban
     end
 
+    def valid_state?(user)
+      user.active?
+    end
+
     def action
       :ban
     end
diff --git a/app/services/users/banned_user_base_service.rb b/app/services/users/banned_user_base_service.rb
index 16041075941..a582816283a 100644
--- a/app/services/users/banned_user_base_service.rb
+++ b/app/services/users/banned_user_base_service.rb
@@ -8,6 +8,7 @@ module Users
 
     def execute(user)
       return permission_error unless allowed?
+      return state_error(user) unless valid_state?(user)
 
       if update_user(user)
         log_event(user)
@@ -22,6 +23,10 @@ module Users
 
     attr_reader :current_user
 
+    def state_error(user)
+      error(_("You cannot %{action} %{state} users." % { action: action.to_s, state: user.state }), :forbidden)
+    end
+
     def allowed?
       can?(current_user, :admin_all_resources)
     end
diff --git a/app/services/users/dismiss_group_callout_service.rb b/app/services/users/dismiss_group_callout_service.rb
new file mode 100644
index 00000000000..8afee6a8187
--- /dev/null
+++ b/app/services/users/dismiss_group_callout_service.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module Users
+  class DismissGroupCalloutService < DismissUserCalloutService
+    private
+
+    def callout
+      current_user.find_or_initialize_group_callout(params[:feature_name], params[:group_id])
+    end
+  end
+end
diff --git a/app/services/users/dismiss_user_callout_service.rb b/app/services/users/dismiss_user_callout_service.rb
index f05c44186bb..96f3f3acb57 100644
--- a/app/services/users/dismiss_user_callout_service.rb
+++ b/app/services/users/dismiss_user_callout_service.rb
@@ -3,9 +3,15 @@
 module Users
   class DismissUserCalloutService < BaseContainerService
     def execute
-      current_user.find_or_initialize_callout(params[:feature_name]).tap do |callout|
-        callout.update(dismissed_at: Time.current) if callout.valid?
+      callout.tap do |record|
+        record.update(dismissed_at: Time.current) if record.valid?
       end
     end
+
+    private
+
+    def callout
+      current_user.find_or_initialize_callout(params[:feature_name])
+    end
   end
 end
diff --git a/app/services/users/migrate_to_ghost_user_service.rb b/app/services/users/migrate_to_ghost_user_service.rb
index a471f55e644..515d7821416 100644
--- a/app/services/users/migrate_to_ghost_user_service.rb
+++ b/app/services/users/migrate_to_ghost_user_service.rb
@@ -14,23 +14,30 @@ module Users
 
     def initialize(user)
       @user = user
+      @ghost_user = User.ghost
     end
 
     def execute
       transition = user.block_transition
 
-      user.transaction do
-        # Block the user before moving records to prevent a data race.
-        # For example, if the user creates an issue after `migrate_issues`
-        # runs and before the user is destroyed, the destroy will fail with
-        # an exception.
-        user.block
+      # Block the user before moving records to prevent a data race.
+      # For example, if the user creates an issue after `migrate_issues`
+      # runs and before the user is destroyed, the destroy will fail with
+      # an exception.
+      user.block
 
+      begin
+        user.transaction do
+          migrate_records
+        end
+      rescue Exception # rubocop:disable Lint/RescueException
         # Reverse the user block if record migration fails
-        if !migrate_records_in_transaction && transition
+        if transition
           transition.rollback
           user.save!
         end
+
+        raise
       end
 
       user.reset
@@ -38,14 +45,6 @@ module Users
 
     private
 
-    def migrate_records_in_transaction
-      user.transaction(requires_new: true) do
-        @ghost_user = User.ghost
-
-        migrate_records
-      end
-    end
-
     def migrate_records
       migrate_issues
       migrate_merge_requests
diff --git a/app/services/users/reject_service.rb b/app/services/users/reject_service.rb
index 833c30d9427..459dd81b74d 100644
--- a/app/services/users/reject_service.rb
+++ b/app/services/users/reject_service.rb
@@ -7,8 +7,8 @@ module Users
     end
 
     def execute(user)
-      return error(_('You are not allowed to reject a user')) unless allowed?
-      return error(_('This user does not have a pending request')) unless user.blocked_pending_approval?
+      return error(_('You are not allowed to reject a user'), :forbidden) unless allowed?
+      return error(_('User does not have a pending request'), :conflict) unless user.blocked_pending_approval?
 
       user.delete_async(deleted_by: current_user, params: { hard_delete: true })
 
@@ -18,7 +18,7 @@ module Users
 
       log_event(user)
 
-      success
+      success(message: 'Success', http_status: :ok)
     end
 
     private
diff --git a/app/services/users/unban_service.rb b/app/services/users/unban_service.rb
index 363783cf240..753a02fa752 100644
--- a/app/services/users/unban_service.rb
+++ b/app/services/users/unban_service.rb
@@ -5,7 +5,11 @@ module Users
     private
 
     def update_user(user)
-      user.activate
+      user.unban
+    end
+
+    def valid_state?(user)
+      user.banned?
     end
 
     def action