Merge branch '33215-fix-hard-delete-of-users' into 'master'

Fix hard-deleting users when they have authored issues

Closes #33215

See merge request !11855
This commit is contained in:
Douwe Maan 2017-06-02 15:10:40 +00:00
commit f123b65233
4 changed files with 13 additions and 7 deletions

View file

@ -101,6 +101,7 @@ class User < ActiveRecord::Base
has_many :snippets, dependent: :destroy, foreign_key: :author_id has_many :snippets, dependent: :destroy, foreign_key: :author_id
has_many :notes, dependent: :destroy, foreign_key: :author_id has_many :notes, dependent: :destroy, foreign_key: :author_id
has_many :issues, dependent: :destroy, foreign_key: :author_id
has_many :merge_requests, dependent: :destroy, foreign_key: :author_id has_many :merge_requests, dependent: :destroy, foreign_key: :author_id
has_many :events, dependent: :destroy, foreign_key: :author_id has_many :events, dependent: :destroy, foreign_key: :author_id
has_many :subscriptions, dependent: :destroy has_many :subscriptions, dependent: :destroy
@ -120,11 +121,6 @@ class User < ActiveRecord::Base
has_many :assigned_issues, class_name: "Issue", through: :issue_assignees, source: :issue has_many :assigned_issues, class_name: "Issue", through: :issue_assignees, source: :issue
has_many :assigned_merge_requests, dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest" has_many :assigned_merge_requests, dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest"
# Issues that a user owns are expected to be moved to the "ghost" user before
# the user is destroyed. If the user owns any issues during deletion, this
# should be treated as an exceptional condition.
has_many :issues, dependent: :restrict_with_exception, foreign_key: :author_id
# #
# Validations # Validations
# #

View file

@ -0,0 +1,4 @@
---
title: Fix hard-deleting users when they have authored issues
merge_request: 11855
author:

View file

@ -22,7 +22,7 @@ describe User, models: true do
it { is_expected.to have_many(:deploy_keys).dependent(:destroy) } it { is_expected.to have_many(:deploy_keys).dependent(:destroy) }
it { is_expected.to have_many(:events).dependent(:destroy) } it { is_expected.to have_many(:events).dependent(:destroy) }
it { is_expected.to have_many(:recent_events).class_name('Event') } it { is_expected.to have_many(:recent_events).class_name('Event') }
it { is_expected.to have_many(:issues).dependent(:restrict_with_exception) } it { is_expected.to have_many(:issues).dependent(:destroy) }
it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:notes).dependent(:destroy) }
it { is_expected.to have_many(:merge_requests).dependent(:destroy) } it { is_expected.to have_many(:merge_requests).dependent(:destroy) }
it { is_expected.to have_many(:identities).dependent(:destroy) } it { is_expected.to have_many(:identities).dependent(:destroy) }

View file

@ -147,16 +147,22 @@ describe Users::DestroyService, services: true do
end end
context "migrating associated records" do context "migrating associated records" do
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 expect_any_instance_of(Users::MigrateToGhostUserService).to receive(:execute).once.and_call_original
service.execute(user) service.execute(user)
expect(issue.reload.author).to be_ghost
end end
it 'does not run `MigrateToGhostUser` if hard_delete option is given' do it 'does not run `MigrateToGhostUser` if hard_delete option is given' do
expect_any_instance_of(Users::MigrateToGhostUserService).not_to receive(:execute) expect_any_instance_of(Users::MigrateToGhostUserService).not_to receive(:execute)
service.execute(user, hard_delete: true) service.execute(user, hard_delete: true)
expect(Issue.exists?(issue.id)).to be_falsy
end end
end end
end end