Merge branch '28694-hard-delete-user-from-admin-panel' into 'master'

Allow users to be hard-deleted from the admin panel

Closes #28694

See merge request !11874

8 files changed, 55 insertions, 12 deletions

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 563bcc65bd6..bace99dad58 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -138,7 +138,7 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def destroy
-    DeleteUserWorker.perform_async(current_user.id, user.id)
+    user.delete_async(deleted_by: current_user, params: params.permit(:hard_delete))
 
     respond_to do |format|
       format.html { redirect_to admin_users_path, notice: "The user is being deleted." }
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index 3ca14dee33c..cd2003586be 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -25,7 +25,7 @@ class RegistrationsController < Devise::RegistrationsController
   end
 
   def destroy
-    DeleteUserWorker.perform_async(current_user.id, current_user.id)
+    current_user.delete_async(deleted_by: current_user)
 
     respond_to do |format|
       format.html do
diff --git a/app/models/abuse_report.rb b/app/models/abuse_report.rb
index 0d7c2d20029..4cbd90c5817 100644
--- a/app/models/abuse_report.rb
+++ b/app/models/abuse_report.rb
@@ -15,8 +15,7 @@ class AbuseReport < ActiveRecord::Base
   alias_method :author, :reporter
 
   def remove_user(deleted_by:)
-    user.block
-    DeleteUserWorker.perform_async(deleted_by.id, user.id, delete_solo_owned_groups: true, hard_delete: true)
+    user.delete_async(deleted_by: deleted_by, params: { hard_delete: true })
   end
 
   def notify
diff --git a/app/models/spam_log.rb b/app/models/spam_log.rb
index dd21ee15c6c..56a115d1db4 100644
--- a/app/models/spam_log.rb
+++ b/app/models/spam_log.rb
@@ -4,8 +4,7 @@ class SpamLog < ActiveRecord::Base
   validates :user, presence: true
 
   def remove_user(deleted_by:)
-    user.block
-    DeleteUserWorker.perform_async(deleted_by.id, user.id, delete_solo_owned_groups: true, hard_delete: true)
+    user.delete_async(deleted_by: deleted_by, params: { hard_delete: true })
   end
 
   def text
diff --git a/app/models/user.rb b/app/models/user.rb
index e6eb9d09656..9ed42d6b6f5 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -809,6 +809,11 @@ class User < ActiveRecord::Base
     system_hook_service.execute_hooks_for(self, :destroy)
   end
 
+  def delete_async(deleted_by:, params: {})
+    block if params[:hard_delete]
+    DeleteUserWorker.perform_async(deleted_by.id, id, params)
+  end
+
   def notification_service
     NotificationService.new
   end
diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb
index 9eb6a600f6b..673afb8b5b9 100644
--- a/app/services/users/destroy_service.rb
+++ b/app/services/users/destroy_service.rb
@@ -6,12 +6,27 @@ module Users
       @current_user = current_user
     end
 
+    # Synchronously destroys +user+
+    #
+    # The operation will fail if the user is the sole owner of any groups. To
+    # force the groups to be destroyed, pass `delete_solo_owned_groups: true` in
+    # +options+.
+    #
+    # The user's contributions will be migrated to a global ghost user. To
+    # force the contributions to be destroyed, pass `hard_delete: true` in
+    # +options+.
+    #
+    # `hard_delete: true` implies `delete_solo_owned_groups: true`.  To perform
+    # a hard deletion without destroying solo-owned groups, pass
+    # `delete_solo_owned_groups: false, hard_delete: true` in +options+.
     def execute(user, options = {})
+      delete_solo_owned_groups = options.fetch(:delete_solo_owned_groups, options[:hard_delete])
+
       unless Ability.allowed?(current_user, :destroy_user, user)
         raise Gitlab::Access::AccessDeniedError, "#{current_user} tried to destroy user #{user}!"
       end
 
-      if !options[:delete_solo_owned_groups] && user.solo_owned_groups.present?
+      if !delete_solo_owned_groups && user.solo_owned_groups.present?
         user.errors[:base] << 'You must transfer ownership or delete groups before you can remove user'
         return user
       end
diff --git a/app/views/admin/users/_user.html.haml b/app/views/admin/users/_user.html.haml
index 46d2e3b3de1..4cf4a57ba18 100644
--- a/app/views/admin/users/_user.html.haml
+++ b/app/views/admin/users/_user.html.haml
@@ -34,9 +34,15 @@
             - if user.access_locked?
               %li
                 = link_to 'Unlock', unlock_admin_user_path(user), method: :put, class: 'btn-grouped btn btn-xs btn-success', data: { confirm: 'Are you sure?' }
-          - if user.can_be_removed? && can?(current_user, :destroy_user, user)
+          - if can?(current_user, :destroy_user, user)
             %li.divider
+            - if user.can_be_removed?
+              %li
+                = link_to 'Remove user', admin_user_path(user),
+                    data: { confirm: "USER #{user.name} WILL BE REMOVED! Are you sure?" },
+                    method: :delete
             %li
-              = link_to 'Delete user', [:admin, user], data: { confirm: "USER #{user.name} WILL BE REMOVED! All issues, merge requests and groups linked to this user will also be removed! Consider cancelling this deletion and blocking the user instead. Are you sure?" },
-                                                   class: 'btn btn-remove btn-block',
-                                                   method: :delete
+              = link_to 'Remove user and contributions', admin_user_path(user, hard_delete: true),
+                  data: { confirm: "USER #{user.name} WILL BE REMOVED! All issues, merge requests and comments authored by this user, and groups owned solely by them, will also be removed! Are you sure?" },
+                  class: 'btn btn-remove btn-block',
+                  method: :delete
diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml
index 89d0bbb7126..b556ff056c0 100644
--- a/app/views/admin/users/show.html.haml
+++ b/app/views/admin/users/show.html.haml
@@ -177,7 +177,7 @@
             %p Deleting a user has the following effects:
             = render 'users/deletion_guidance', user: @user
             %br
-            = link_to 'Remove user', [:admin, @user], data: { confirm: "USER #{@user.name} WILL BE REMOVED! Are you sure?" }, method: :delete, class: "btn btn-remove"
+            = link_to 'Remove user', admin_user_path(@user), data: { confirm: "USER #{@user.name} WILL BE REMOVED! Are you sure?" }, method: :delete, class: "btn btn-remove"
           - else
             - if @user.solo_owned_groups.present?
               %p
@@ -188,3 +188,22 @@
             - else
               %p
                 You don't have access to delete this user.
+
+      .panel.panel-danger
+        .panel-heading
+          Remove user and contributions
+        .panel-body
+          - if can?(current_user, :destroy_user, @user)
+            %p
+              This option deletes the user and any contributions that
+              would usually be moved to the
+              = succeed "." do
+                = link_to "system ghost user", help_page_path("user/profile/account/delete_account")
+              As well as the user's personal projects, groups owned solely by
+              the user, and projects in them, will also be removed. Commits
+              to other projects are unaffected.
+            %br
+            = link_to 'Remove user and contributions', admin_user_path(@user, hard_delete: true), data: { confirm: "USER #{@user.name} WILL BE REMOVED! Are you sure?" }, method: :delete, class: "btn btn-remove"
+          - else
+            %p
+              You don't have access to delete this user.