Commit f83a596d authored by Douwe Maan's avatar Douwe Maan

Merge branch 'worker-for-user-deletion' into 'master'

A worker deletes a user, so the request doesn't time out

Fixes #13261


See merge request !2855
parents 64d0dd18 29a43373
...@@ -39,6 +39,7 @@ v 8.6.0 (unreleased) ...@@ -39,6 +39,7 @@ v 8.6.0 (unreleased)
- Add ability to show archived projects on dashboard, explore and group pages - Add ability to show archived projects on dashboard, explore and group pages
- Move group activity to separate page - Move group activity to separate page
- Continue parameters are checked to ensure redirection goes to the same instance - Continue parameters are checked to ensure redirection goes to the same instance
- User deletion is now done in the background so the request can not time out
v 8.5.7 v 8.5.7
- Bump Git version requirement to 2.7.3 - Bump Git version requirement to 2.7.3
......
...@@ -6,7 +6,7 @@ class Admin::AbuseReportsController < Admin::ApplicationController ...@@ -6,7 +6,7 @@ class Admin::AbuseReportsController < Admin::ApplicationController
def destroy def destroy
abuse_report = AbuseReport.find(params[:id]) abuse_report = AbuseReport.find(params[:id])
abuse_report.remove_user if params[:remove_user] abuse_report.remove_user(deleted_by: current_user) if params[:remove_user]
abuse_report.destroy abuse_report.destroy
render nothing: true render nothing: true
......
...@@ -119,10 +119,10 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -119,10 +119,10 @@ class Admin::UsersController < Admin::ApplicationController
end end
def destroy def destroy
DeleteUserService.new(current_user).execute(user) DeleteUserWorker.perform_async(current_user.id, user.id)
respond_to do |format| respond_to do |format|
format.html { redirect_to admin_users_path } format.html { redirect_to admin_users_path, notice: "The user is being deleted." }
format.json { head :ok } format.json { head :ok }
end end
end end
......
...@@ -19,9 +19,9 @@ class AbuseReport < ActiveRecord::Base ...@@ -19,9 +19,9 @@ class AbuseReport < ActiveRecord::Base
validates :message, presence: true validates :message, presence: true
validates :user_id, uniqueness: { message: 'has already been reported' } validates :user_id, uniqueness: { message: 'has already been reported' }
def remove_user def remove_user(deleted_by:)
user.block user.block
user.destroy DeleteUserWorker.perform_async(deleted_by.id, user.id, delete_solo_owned_groups: true)
end end
def notify def notify
......
...@@ -5,11 +5,16 @@ class DeleteUserService ...@@ -5,11 +5,16 @@ class DeleteUserService
@current_user = current_user @current_user = current_user
end end
def execute(user) def execute(user, options = {})
if user.solo_owned_groups.present? if !options[: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'
user return user
else end
user.solo_owned_groups.each do |group|
DestroyGroupService.new(group, current_user).execute
end
user.personal_projects.each do |project| user.personal_projects.each do |project|
# Skip repository removal because we remove directory with namespace # Skip repository removal because we remove directory with namespace
# that contain all this repositories # that contain all this repositories
...@@ -18,5 +23,4 @@ class DeleteUserService ...@@ -18,5 +23,4 @@ class DeleteUserService
user.destroy user.destroy
end end
end
end end
...@@ -6,12 +6,12 @@ class DestroyGroupService ...@@ -6,12 +6,12 @@ class DestroyGroupService
end end
def execute def execute
@group.projects.each do |project| group.projects.each do |project|
# Skip repository removal because we remove directory with namespace # Skip repository removal because we remove directory with namespace
# that contain all this repositories # that contain all this repositories
::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete! ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete!
end end
@group.destroy group.destroy
end end
end end
class DeleteUserWorker
include Sidekiq::Worker
def perform(current_user_id, delete_user_id, options = {})
delete_user = User.find(delete_user_id)
current_user = User.find(current_user_id)
DeleteUserService.new(current_user).execute(delete_user, options.symbolize_keys)
end
end
...@@ -14,6 +14,7 @@ require 'rails_helper' ...@@ -14,6 +14,7 @@ require 'rails_helper'
RSpec.describe AbuseReport, type: :model do RSpec.describe AbuseReport, type: :model do
subject { create(:abuse_report) } subject { create(:abuse_report) }
let(:user) { create(:user) }
it { expect(subject).to be_valid } it { expect(subject).to be_valid }
...@@ -31,17 +32,14 @@ RSpec.describe AbuseReport, type: :model do ...@@ -31,17 +32,14 @@ RSpec.describe AbuseReport, type: :model do
describe '#remove_user' do describe '#remove_user' do
it 'blocks the user' do it 'blocks the user' do
report = build(:abuse_report) expect { subject.remove_user(deleted_by: user) }.to change { subject.user.blocked? }.to(true)
allow(report.user).to receive(:destroy)
expect { report.remove_user }.to change { report.user.blocked? }.to(true)
end end
it 'removes the user' do it 'lets a worker delete the user' do
report = build(:abuse_report) expect(DeleteUserWorker).to receive(:perform_async).with(user.id, subject.user.id,
delete_solo_owned_groups: true)
expect { report.remove_user }.to change { User.count }.by(-1) subject.remove_user(deleted_by: user)
end end
end end
......
require 'spec_helper'
describe DeleteUserService, services: true do
describe "Deletes a user and all their personal projects" do
let!(:user) { create(:user) }
let!(:current_user) { create(:user) }
let!(:namespace) { create(:namespace, owner: user) }
let!(:project) { create(:project, namespace: namespace) }
context 'no options are given' do
it 'deletes the user' do
DeleteUserService.new(current_user).execute(user)
expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound)
end
it 'will delete the project in the near future' do
expect_any_instance_of(Projects::DestroyService).to receive(:pending_delete!).once
DeleteUserService.new(current_user).execute(user)
end
end
context "solo owned groups present" do
let(:solo_owned) { create(:group) }
let(:member) { create(:group_member) }
let(:user) { member.user }
before do
solo_owned.group_members = [member]
DeleteUserService.new(current_user).execute(user)
end
it 'does not delete the user' do
expect(User.find(user.id)).to eq user
end
end
context "deletions with solo owned groups" do
let(:solo_owned) { create(:group) }
let(:member) { create(:group_member) }
let(:user) { member.user }
before do
solo_owned.group_members = [member]
DeleteUserService.new(current_user).execute(user, delete_solo_owned_groups: true)
end
it 'deletes solo owned groups' do
expect { Project.find(solo_owned.id) }.to raise_error(ActiveRecord::RecordNotFound)
end
it 'deletes the user' do
expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound)
end
end
end
end
require 'spec_helper'
describe DeleteUserWorker do
let!(:user) { create(:user) }
let!(:current_user) { create(:user) }
it "calls the DeleteUserWorker with the params it was given" do
expect_any_instance_of(DeleteUserService).to receive(:execute).
with(user, {})
DeleteUserWorker.new.perform(current_user.id, user.id)
end
it "uses symbolized keys" do
expect_any_instance_of(DeleteUserService).to receive(:execute).
with(user, test: "test")
DeleteUserWorker.new.perform(current_user.id, user.id, "test" => "test")
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment