diff --git a/app/models/user.rb b/app/models/user.rb index b2f75959f15b0deac4e107a92801d4d52ddc1b05..6c469c770e75c33685cd4cbb952b7439cfafca70 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -214,14 +214,15 @@ def update_tracked_fields!(request) has_many :spam_logs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :builds, class_name: 'Ci::Build' has_many :pipelines, class_name: 'Ci::Pipeline' - has_many :todos + has_many :todos, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :authored_todos, class_name: 'Todo', dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent has_many :notification_settings has_many :award_emoji, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :triggers, class_name: 'Ci::Trigger', foreign_key: :owner_id has_many :issue_assignees, inverse_of: :assignee - has_many :merge_request_assignees, inverse_of: :assignee - has_many :merge_request_reviewers, inverse_of: :reviewer + has_many :merge_request_assignees, inverse_of: :assignee, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :merge_request_reviewers, inverse_of: :reviewer, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :assigned_issues, class_name: "Issue", through: :issue_assignees, source: :issue has_many :assigned_merge_requests, class_name: "MergeRequest", through: :merge_request_assignees, source: :merge_request has_many :created_custom_emoji, class_name: 'CustomEmoji', inverse_of: :creator @@ -255,6 +256,8 @@ def update_tracked_fields!(request) has_many :timelogs has_many :resource_label_events, dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent + has_many :resource_state_events, dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent + has_many :authored_events, class_name: 'Event', dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent # # Validations diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index efaac06c1f28e927e94141b3488161d0bc49b559..03e1811c8a5c4b7202db0091b83f5a7da054b2ed 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -388,42 +388,95 @@ context 'batched nullify' do let(:other_user) { create(:user) } + # rubocop:disable Layout/LineLength + def nullify_in_batches_regexp(table, column, user, batch_size: 100) + %r{^UPDATE "#{table}" SET "#{column}" = NULL WHERE "#{table}"."id" IN \(SELECT "#{table}"."id" FROM "#{table}" WHERE "#{table}"."#{column}" = #{user.id} LIMIT #{batch_size}\)} + end + + def delete_in_batches_regexps(table, column, user, items, batch_size: 1000) + select_query = %r{^SELECT "#{table}".* FROM "#{table}" WHERE "#{table}"."#{column}" = #{user.id}.*ORDER BY "#{table}"."id" ASC LIMIT #{batch_size}} + + [select_query] + items.map { |item| %r{^DELETE FROM "#{table}" WHERE "#{table}"."id" = #{item.id}} } + end + # rubocop:enable Layout/LineLength + it 'nullifies related associations in batches' do expect(other_user).to receive(:nullify_dependent_associations_in_batches).and_call_original described_class.new(user).execute(other_user, skip_authorization: true) end - it 'nullifies last_updated_issues, closed_issues, resource_label_events' do + it 'nullifies issues and resource associations', :aggregate_failures do issue = create(:issue, closed_by: other_user, updated_by: other_user) resource_label_event = create(:resource_label_event, user: other_user) + resource_state_event = create(:resource_state_event, user: other_user) + todos = create_list(:todo, 2, project: issue.project, user: other_user, author: other_user, target: issue) + event = create(:event, project: issue.project, author: other_user) - described_class.new(user).execute(other_user, skip_authorization: true) + query_recorder = ActiveRecord::QueryRecorder.new do + described_class.new(user).execute(other_user, skip_authorization: true) + end issue.reload resource_label_event.reload + resource_state_event.reload expect(issue.closed_by).to be_nil expect(issue.updated_by).to be_nil expect(resource_label_event.user).to be_nil + expect(resource_state_event.user).to be_nil + expect(other_user.authored_todos).to be_empty + expect(other_user.todos).to be_empty + expect(other_user.authored_events).to be_empty + + expected_queries = [ + nullify_in_batches_regexp(:issues, :updated_by_id, other_user), + nullify_in_batches_regexp(:issues, :closed_by_id, other_user), + nullify_in_batches_regexp(:resource_label_events, :user_id, other_user), + nullify_in_batches_regexp(:resource_state_events, :user_id, other_user) + ] + + expected_queries += delete_in_batches_regexps(:todos, :user_id, other_user, todos) + expected_queries += delete_in_batches_regexps(:todos, :author_id, other_user, todos) + expected_queries += delete_in_batches_regexps(:events, :author_id, other_user, [event]) + + expect(query_recorder.log).to include(*expected_queries) end - it 'nullifies merge request associations' do + it 'nullifies merge request associations', :aggregate_failures do merge_request = create(:merge_request, source_project: project, target_project: project, assignee: other_user, updated_by: other_user, merge_user: other_user) merge_request.metrics.update!(merged_by: other_user, latest_closed_by: other_user) + merge_request.reviewers = [other_user] + merge_request.assignees = [other_user] - described_class.new(user).execute(other_user, skip_authorization: true) + query_recorder = ActiveRecord::QueryRecorder.new do + described_class.new(user).execute(other_user, skip_authorization: true) + end merge_request.reload - aggregate_failures do - expect(merge_request.updated_by).to be_nil - expect(merge_request.assignee).to be_nil - expect(merge_request.assignee_id).to be_nil - expect(merge_request.metrics.merged_by).to be_nil - expect(merge_request.metrics.latest_closed_by).to be_nil - end + expect(merge_request.updated_by).to be_nil + expect(merge_request.assignee).to be_nil + expect(merge_request.assignee_id).to be_nil + expect(merge_request.metrics.merged_by).to be_nil + expect(merge_request.metrics.latest_closed_by).to be_nil + expect(merge_request.reviewers).to be_empty + expect(merge_request.assignees).to be_empty + + expected_queries = [ + nullify_in_batches_regexp(:merge_requests, :updated_by_id, other_user), + nullify_in_batches_regexp(:merge_requests, :assignee_id, other_user), + nullify_in_batches_regexp(:merge_request_metrics, :merged_by_id, other_user), + nullify_in_batches_regexp(:merge_request_metrics, :latest_closed_by_id, other_user) + ] + + expected_queries += delete_in_batches_regexps(:merge_request_assignees, :user_id, other_user, + merge_request.assignees) + expected_queries += delete_in_batches_regexps(:merge_request_reviewers, :user_id, other_user, + merge_request.reviewers) + + expect(query_recorder.log).to include(*expected_queries) end end end