Move a user's merge requests to the ghost user.
1. When the user is deleted. 2. Refactor out code relating to "migrating records to the ghost user" into a `MigrateToGhostUser` concern, which is tested using a shared example.
This commit is contained in:
parent
fa65b65b0f
commit
72580f07af
|
@ -0,0 +1,38 @@
|
|||
# When a user is destroyed, some of their associated records are
|
||||
# moved to a "Ghost User", to prevent these associated records from
|
||||
# being destroyed.
|
||||
#
|
||||
# For example, all the issues/MRs a user has created are _not_ destroyed
|
||||
# when the user is destroyed.
|
||||
module Users::MigrateToGhostUser
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
attr_reader :ghost_user
|
||||
|
||||
def move_associated_records_to_ghost_user(user)
|
||||
# Block the user before moving records to prevent a data race.
|
||||
# For example, if the user creates an issue after `move_issues_to_ghost_user`
|
||||
# runs and before the user is destroyed, the destroy will fail with
|
||||
# an exception.
|
||||
user.block
|
||||
|
||||
user.transaction do
|
||||
@ghost_user = User.ghost
|
||||
|
||||
move_issues_to_ghost_user(user)
|
||||
move_merge_requests_to_ghost_user(user)
|
||||
end
|
||||
|
||||
user.reload
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def move_issues_to_ghost_user(user)
|
||||
user.issues.update_all(author_id: ghost_user.id)
|
||||
end
|
||||
|
||||
def move_merge_requests_to_ghost_user(user)
|
||||
user.merge_requests.update_all(author_id: ghost_user.id)
|
||||
end
|
||||
end
|
|
@ -1,5 +1,7 @@
|
|||
module Users
|
||||
class DestroyService
|
||||
include MigrateToGhostUser
|
||||
|
||||
attr_accessor :current_user
|
||||
|
||||
def initialize(current_user)
|
||||
|
@ -26,7 +28,7 @@ module Users
|
|||
::Projects::DestroyService.new(project, current_user, skip_repo: true).execute
|
||||
end
|
||||
|
||||
move_issues_to_ghost_user(user)
|
||||
move_associated_records_to_ghost_user(user)
|
||||
|
||||
# Destroy the namespace after destroying the user since certain methods may depend on the namespace existing
|
||||
namespace = user.namespace
|
||||
|
@ -35,22 +37,5 @@ module Users
|
|||
|
||||
user_data
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def move_issues_to_ghost_user(user)
|
||||
# Block the user before moving issues to prevent a data race.
|
||||
# If the user creates an issue after `move_issues_to_ghost_user`
|
||||
# runs and before the user is destroyed, the destroy will fail with
|
||||
# an exception. We block the user so that issues can't be created
|
||||
# after `move_issues_to_ghost_user` runs and before the destroy happens.
|
||||
user.block
|
||||
|
||||
ghost_user = User.ghost
|
||||
|
||||
user.issues.update_all(author_id: ghost_user.id)
|
||||
|
||||
user.reload
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -141,5 +141,21 @@ describe Users::DestroyService, services: true do
|
|||
expect(User.exists?(user.id)).to be(false)
|
||||
end
|
||||
end
|
||||
|
||||
context 'migrating associated records to the ghost user' do
|
||||
context 'issues' do
|
||||
include_examples "migrating a deleted user's associated records to the ghost user", Issue do
|
||||
let(:created_record) { create(:issue, project: project, author: user) }
|
||||
let(:assigned_record) { create(:issue, project: project, assignee: user) }
|
||||
end
|
||||
end
|
||||
|
||||
context 'merge requests' do
|
||||
include_examples "migrating a deleted user's associated records to the ghost user", MergeRequest do
|
||||
let(:created_record) { create(:merge_request, source_project: project, author: user, target_branch: "first") }
|
||||
let(:assigned_record) { create(:merge_request, source_project: project, assignee: user, target_branch: 'second') }
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,53 @@
|
|||
require "spec_helper"
|
||||
|
||||
shared_examples "migrating a deleted user's associated records to the ghost user" do |record_class|
|
||||
record_class_name = record_class.to_s.titleize.downcase
|
||||
|
||||
let(:project) { create(:project) }
|
||||
|
||||
before do
|
||||
project.add_developer(user)
|
||||
end
|
||||
|
||||
context "for a #{record_class_name} the user has created" do
|
||||
let!(:record) { created_record }
|
||||
|
||||
it "does not delete the #{record_class_name}" do
|
||||
service.execute(user)
|
||||
|
||||
expect(record_class.find_by_id(record.id)).to be_present
|
||||
end
|
||||
|
||||
it "migrates the #{record_class_name} so that the 'Ghost User' is the #{record_class_name} owner" do
|
||||
service.execute(user)
|
||||
|
||||
migrated_record = record_class.find_by_id(record.id)
|
||||
|
||||
expect(migrated_record.author).to eq(User.ghost)
|
||||
end
|
||||
|
||||
it "blocks the user before migrating #{record_class_name}s to the 'Ghost User'" do
|
||||
service.execute(user)
|
||||
|
||||
expect(user).to be_blocked
|
||||
end
|
||||
end
|
||||
|
||||
context "for a #{record_class_name} the user was assigned to" do
|
||||
let!(:record) { assigned_record }
|
||||
|
||||
before do
|
||||
service.execute(user)
|
||||
end
|
||||
|
||||
it "does not delete #{record_class_name}s the user is assigned to" do
|
||||
expect(record_class.find_by_id(record.id)).to be_present
|
||||
end
|
||||
|
||||
it "migrates the #{record_class_name} so that it is 'Unassigned'" do
|
||||
migrated_record = record_class.find_by_id(record.id)
|
||||
|
||||
expect(migrated_record.assignee).to be_nil
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue