Commit 004f78ac authored by Brett Walker's avatar Brett Walker Committed by Stan Hu

Set resoure_iteration_events to ghost user

when the user is deleted

Changelog: fixed
EE: true
parent c308510a
...@@ -391,7 +391,7 @@ class User < ApplicationRecord ...@@ -391,7 +391,7 @@ class User < ApplicationRecord
# rubocop: disable CodeReuse/ServiceClass # rubocop: disable CodeReuse/ServiceClass
# Ideally we should not call a service object here but user.block # Ideally we should not call a service object here but user.block
# is also bcalled by Users::MigrateToGhostUserService which references # is also called by Users::MigrateToGhostUserService which references
# this state transition object in order to do a rollback. # this state transition object in order to do a rollback.
# For this reason the tradeoff is to disable this cop. # For this reason the tradeoff is to disable this cop.
after_transition any => :blocked do |user| after_transition any => :blocked do |user|
......
...@@ -54,7 +54,7 @@ module Users ...@@ -54,7 +54,7 @@ module Users
yield(user) if block_given? yield(user) if block_given?
MigrateToGhostUserService.new(user).execute unless options[:hard_delete] MigrateToGhostUserService.new(user).execute(hard_delete: options[:hard_delete])
response = Snippets::BulkDestroyService.new(current_user, user.snippets).execute(options) response = Snippets::BulkDestroyService.new(current_user, user.snippets).execute(options)
raise DestroyError, response.message if response.error? raise DestroyError, response.message if response.error?
......
...@@ -10,14 +10,21 @@ module Users ...@@ -10,14 +10,21 @@ module Users
class MigrateToGhostUserService class MigrateToGhostUserService
extend ActiveSupport::Concern extend ActiveSupport::Concern
attr_reader :ghost_user, :user attr_reader :ghost_user, :user, :hard_delete
def initialize(user) def initialize(user)
@user = user @user = user
@ghost_user = User.ghost @ghost_user = User.ghost
end end
def execute # If an admin attempts to hard delete a user, in some cases associated
# records may have a NOT NULL constraint on the user ID that prevent that record
# from being destroyed. In such situations we must assign the record to the ghost user.
# Passing in `hard_delete: true` will ensure these records get assigned to
# the ghost user before the user is destroyed. Other associated records will be destroyed.
# letting the other associated records be destroyed.
def execute(hard_delete: false)
@hard_delete = hard_delete
transition = user.block_transition transition = user.block_transition
# Block the user before moving records to prevent a data race. # Block the user before moving records to prevent a data race.
...@@ -46,6 +53,8 @@ module Users ...@@ -46,6 +53,8 @@ module Users
private private
def migrate_records def migrate_records
return if hard_delete
migrate_issues migrate_issues
migrate_merge_requests migrate_merge_requests
migrate_notes migrate_notes
......
# frozen_string_literal: true # frozen_string_literal: true
class ResourceIterationEvent < ResourceTimeboxEvent class ResourceIterationEvent < ResourceTimeboxEvent
include EachBatch
belongs_to :iteration belongs_to :iteration
scope :with_api_entity_associations, -> { preload(:iteration, :user) } scope :with_api_entity_associations, -> { preload(:iteration, :user) }
scope :by_user, -> (user) { where(user_id: user ) }
end end
...@@ -3,9 +3,16 @@ ...@@ -3,9 +3,16 @@
module EE module EE
module Users module Users
module MigrateToGhostUserService module MigrateToGhostUserService
BATCH_SIZE = 1000
private private
def migrate_records def migrate_records
# these should always be ghosted
migrate_resource_iteration_events
return super if hard_delete
migrate_epics migrate_epics
migrate_requirements_management_requirements migrate_requirements_management_requirements
migrate_vulnerabilities_feedback migrate_vulnerabilities_feedback
...@@ -27,6 +34,12 @@ module EE ...@@ -27,6 +34,12 @@ module EE
user.vulnerability_feedback.update_all(author_id: ghost_user.id) user.vulnerability_feedback.update_all(author_id: ghost_user.id)
user.commented_vulnerability_feedback.update_all(comment_author_id: ghost_user.id) user.commented_vulnerability_feedback.update_all(comment_author_id: ghost_user.id)
end end
def migrate_resource_iteration_events
ResourceIterationEvent.by_user(user).each_batch(of: BATCH_SIZE) do |batch|
batch.update_all(user_id: ghost_user.id)
end
end
end end
end end
end end
...@@ -42,6 +42,20 @@ RSpec.describe Users::DestroyService do ...@@ -42,6 +42,20 @@ RSpec.describe Users::DestroyService do
end end
end end
context 'migrating associated records' do
context 'when hard_delete option is given' do
let!(:resource_iteration_event) { create(:resource_iteration_event, user: user) }
it 'will ghost certain records' do
expect_any_instance_of(Users::MigrateToGhostUserService).to receive(:execute).once.and_call_original
service.execute(user, hard_delete: true)
expect(resource_iteration_event.reload.user).to be_ghost
end
end
end
context 'when user has oncall rotations' do context 'when user has oncall rotations' do
let(:schedule) { create(:incident_management_oncall_schedule, project: project) } let(:schedule) { create(:incident_management_oncall_schedule, project: project) }
let(:rotation) { create(:incident_management_oncall_rotation, schedule: schedule) } let(:rotation) { create(:incident_management_oncall_rotation, schedule: schedule) }
......
...@@ -3,10 +3,11 @@ ...@@ -3,10 +3,11 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Users::MigrateToGhostUserService do RSpec.describe Users::MigrateToGhostUserService do
context 'epics' do let!(:user) { create(:user) }
let!(:user) { create(:user) } let(:service) { described_class.new(user) }
let(:service) { described_class.new(user) } let(:always_ghost) { false }
context 'epics' do
context 'deleted user is present as both author and edited_user' do context 'deleted user is present as both author and edited_user' do
include_examples "migrating a deleted user's associated records to the ghost user", Epic, [:author, :last_edited_by] do include_examples "migrating a deleted user's associated records to the ghost user", Epic, [:author, :last_edited_by] do
let(:created_record) do let(:created_record) do
...@@ -23,29 +24,28 @@ RSpec.describe Users::MigrateToGhostUserService do ...@@ -23,29 +24,28 @@ RSpec.describe Users::MigrateToGhostUserService do
end end
context 'vulnerability_feedback author' do context 'vulnerability_feedback author' do
let!(:user) { create(:user) }
let(:service) { described_class.new(user) }
include_examples "migrating a deleted user's associated records to the ghost user", Vulnerabilities::Feedback, [:author] do include_examples "migrating a deleted user's associated records to the ghost user", Vulnerabilities::Feedback, [:author] do
let(:created_record) { create(:vulnerability_feedback, author: user) } let(:created_record) { create(:vulnerability_feedback, author: user) }
end end
end end
context 'vulnerability_feedback comment author' do context 'vulnerability_feedback comment author' do
let!(:user) { create(:user) }
let(:service) { described_class.new(user) }
include_examples "migrating a deleted user's associated records to the ghost user", Vulnerabilities::Feedback, [:comment_author] do include_examples "migrating a deleted user's associated records to the ghost user", Vulnerabilities::Feedback, [:comment_author] do
let(:created_record) { create(:vulnerability_feedback, comment_author: user) } let(:created_record) { create(:vulnerability_feedback, comment_author: user) }
end end
end end
context 'requirements' do context 'requirements' do
let!(:user) { create(:user) }
let(:service) { described_class.new(user) }
include_examples "migrating a deleted user's associated records to the ghost user", RequirementsManagement::Requirement, [:author] do include_examples "migrating a deleted user's associated records to the ghost user", RequirementsManagement::Requirement, [:author] do
let(:created_record) { create(:requirement, author: user) } let(:created_record) { create(:requirement, author: user) }
end end
end end
context 'resource_iteration_events' do
let(:always_ghost) { true }
include_examples "migrating a deleted user's associated records to the ghost user", ResourceIterationEvent, [:user] do
let(:created_record) { create(:resource_iteration_event, issue: create(:issue), user: user, iteration: create(:iteration)) }
end
end
end end
...@@ -215,8 +215,8 @@ RSpec.describe Users::DestroyService do ...@@ -215,8 +215,8 @@ RSpec.describe Users::DestroyService do
end end
end end
context "migrating associated records" do context 'migrating associated records' do
let!(:issue) { create(:issue, author: user) } let!(:issue) { create(:issue, author: user) }
it 'delegates to the `MigrateToGhostUser` service to move associated records to the ghost user' do it 'delegates to the `MigrateToGhostUser` service to move associated records to the ghost user' do
expect_any_instance_of(Users::MigrateToGhostUserService).to receive(:execute).once.and_call_original expect_any_instance_of(Users::MigrateToGhostUserService).to receive(:execute).once.and_call_original
...@@ -226,12 +226,14 @@ RSpec.describe Users::DestroyService do ...@@ -226,12 +226,14 @@ RSpec.describe Users::DestroyService do
expect(issue.reload.author).to be_ghost expect(issue.reload.author).to be_ghost
end end
it 'does not run `MigrateToGhostUser` if hard_delete option is given' do context 'when hard_delete option is given' do
expect_any_instance_of(Users::MigrateToGhostUserService).not_to receive(:execute) it 'will not ghost certain records' do
expect_any_instance_of(Users::MigrateToGhostUserService).to receive(:execute).once.and_call_original
service.execute(user, hard_delete: true) service.execute(user, hard_delete: true)
expect(Issue.exists?(issue.id)).to be_falsy expect(Issue.exists?(issue.id)).to be_falsy
end
end end
end end
......
...@@ -3,9 +3,10 @@ ...@@ -3,9 +3,10 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Users::MigrateToGhostUserService do RSpec.describe Users::MigrateToGhostUserService do
let!(:user) { create(:user) } let!(:user) { create(:user) }
let!(:project) { create(:project, :repository) } let!(:project) { create(:project, :repository) }
let(:service) { described_class.new(user) } let(:service) { described_class.new(user) }
let(:always_ghost) { false }
context "migrating a user's associated records to the ghost user" do context "migrating a user's associated records to the ghost user" do
context 'issues' do context 'issues' do
......
...@@ -42,6 +42,18 @@ RSpec.shared_examples "migrating a deleted user's associated records to the ghos ...@@ -42,6 +42,18 @@ RSpec.shared_examples "migrating a deleted user's associated records to the ghos
end end
end end
it 'will only migrate specific records during a hard_delete' do
service.execute(hard_delete: true)
migrated_record = record_class.find_by_id(record.id)
check_user = always_ghost ? User.ghost : user
migrated_fields.each do |field|
expect(migrated_record.public_send(field)).to eq(check_user)
end
end
context "race conditions" do context "race conditions" do
context "when #{record_class_name} migration fails and is rolled back" do context "when #{record_class_name} migration fails and is rolled back" do
before do before do
......
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