Refactor the DeleteUserWorker

This commit is contained in:
Nick Thomas 2017-06-02 14:18:24 +01:00
parent c34107608e
commit 158581a447
8 changed files with 27 additions and 11 deletions

View file

@ -25,7 +25,7 @@ class RegistrationsController < Devise::RegistrationsController
end end
def destroy def destroy
DeleteUserWorker.perform_async(current_user.id, current_user.id) current_user.delete_async(deleted_by: current_user)
respond_to do |format| respond_to do |format|
format.html do format.html do

View file

@ -15,8 +15,7 @@ class AbuseReport < ActiveRecord::Base
alias_method :author, :reporter alias_method :author, :reporter
def remove_user(deleted_by:) def remove_user(deleted_by:)
user.block user.delete_async(deleted_by: deleted_by, params: { hard_delete: true })
DeleteUserWorker.perform_async(deleted_by.id, user.id, delete_solo_owned_groups: true, hard_delete: true)
end end
def notify def notify

View file

@ -4,8 +4,7 @@ class SpamLog < ActiveRecord::Base
validates :user, presence: true validates :user, presence: true
def remove_user(deleted_by:) def remove_user(deleted_by:)
user.block user.delete_async(deleted_by: deleted_by, params: { hard_delete: true })
DeleteUserWorker.perform_async(deleted_by.id, user.id, delete_solo_owned_groups: true, hard_delete: true)
end end
def text def text

View file

@ -809,6 +809,11 @@ class User < ActiveRecord::Base
system_hook_service.execute_hooks_for(self, :destroy) system_hook_service.execute_hooks_for(self, :destroy)
end end
def delete_async(deleted_by:, params: {})
block if params[:hard_delete]
DeleteUserWorker.perform_async(deleted_by.id, id, params)
end
def notification_service def notification_service
NotificationService.new NotificationService.new
end end

View file

@ -6,12 +6,27 @@ module Users
@current_user = current_user @current_user = current_user
end 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 = {}) 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) unless Ability.allowed?(current_user, :destroy_user, user)
raise Gitlab::Access::AccessDeniedError, "#{current_user} tried to destroy user #{user}!" raise Gitlab::Access::AccessDeniedError, "#{current_user} tried to destroy user #{user}!"
end 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' user.errors[:base] << 'You must transfer ownership or delete groups before you can remove user'
return user return user
end end

View file

@ -293,7 +293,7 @@ module API
user = User.find_by(id: params[:id]) user = User.find_by(id: params[:id])
not_found!('User') unless user not_found!('User') unless user
DeleteUserWorker.perform_async(current_user.id, user.id, hard_delete: params[:hard_delete]) user.delete_async(deleted_by: current_user, params: params)
end end
desc 'Block a user. Available only for admins.' desc 'Block a user. Available only for admins.'

View file

@ -77,7 +77,7 @@ describe RegistrationsController do
end end
it 'schedules the user for destruction' do it 'schedules the user for destruction' do
expect(DeleteUserWorker).to receive(:perform_async).with(user.id, user.id) expect(DeleteUserWorker).to receive(:perform_async).with(user.id, user.id, {})
post(:destroy) post(:destroy)

View file

@ -28,9 +28,7 @@ RSpec.describe AbuseReport, type: :model do
end end
it 'lets a worker delete the user' do it 'lets a worker delete the user' do
expect(DeleteUserWorker).to receive(:perform_async).with(user.id, subject.user.id, expect(DeleteUserWorker).to receive(:perform_async).with(user.id, subject.user.id, hard_delete: true)
delete_solo_owned_groups: true,
hard_delete: true)
subject.remove_user(deleted_by: user) subject.remove_user(deleted_by: user)
end end