diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-04-06 18:54:57 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-04-06 18:54:57 +0000 |
commit | bef1aca8837f2306ecbd8ff70deb2204bf1da539 (patch) | |
tree | 8b8e1e2edaca871756f98bfa991aeff0a69bb106 /spec/services/users | |
parent | 9205caa3361fb38233588c8a18d27288b1e076ee (diff) | |
parent | 1c42505b026d922df50c59d5f9e85073b5f5345f (diff) | |
download | gitlab-ce-bef1aca8837f2306ecbd8ff70deb2204bf1da539.tar.gz |
Merge branch '28695-move-all-associated-records-to-ghost-user' into 'master'
Resolve "Deleting a user shouldn't delete associated records"
Closes #28695 and #30514
See merge request !10467
Diffstat (limited to 'spec/services/users')
-rw-r--r-- | spec/services/users/destroy_service_spec.rb (renamed from spec/services/users/destroy_spec.rb) | 42 | ||||
-rw-r--r-- | spec/services/users/migrate_to_ghost_user_service_spec.rb | 64 |
2 files changed, 91 insertions, 15 deletions
diff --git a/spec/services/users/destroy_spec.rb b/spec/services/users/destroy_service_spec.rb index 66c61b7f8ff..43c18992d1a 100644 --- a/spec/services/users/destroy_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -46,43 +46,47 @@ describe Users::DestroyService, services: true do project.add_developer(user) end - context "for an issue the user has created" do - let!(:issue) { create(:issue, project: project, author: user) } + context "for an issue the user was assigned to" do + let!(:issue) { create(:issue, project: project, assignee: user) } before do service.execute(user) end - it 'does not delete the issue' do + it 'does not delete issues the user is assigned to' do expect(Issue.find_by_id(issue.id)).to be_present end - it 'migrates the issue so that the "Ghost User" is the issue owner' do + it 'migrates the issue so that it is "Unassigned"' do migrated_issue = Issue.find_by_id(issue.id) - expect(migrated_issue.author).to eq(User.ghost) + expect(migrated_issue.assignee).to be_nil end + end + end - it 'blocks the user before migrating issues to the "Ghost User' do - expect(user).to be_blocked - end + context "a deleted user's merge_requests" do + let(:project) { create(:project) } + + before do + project.add_developer(user) end - context "for an issue the user was assigned to" do - let!(:issue) { create(:issue, project: project, assignee: user) } + context "for an merge request the user was assigned to" do + let!(:merge_request) { create(:merge_request, source_project: project, assignee: user) } before do service.execute(user) end - it 'does not delete issues the user is assigned to' do - expect(Issue.find_by_id(issue.id)).to be_present + it 'does not delete merge requests the user is assigned to' do + expect(MergeRequest.find_by_id(merge_request.id)).to be_present end - it 'migrates the issue so that it is "Unassigned"' do - migrated_issue = Issue.find_by_id(issue.id) + it 'migrates the merge request so that it is "Unassigned"' do + migrated_merge_request = MergeRequest.find_by_id(merge_request.id) - expect(migrated_issue.assignee).to be_nil + expect(migrated_merge_request.assignee).to be_nil end end end @@ -141,5 +145,13 @@ describe Users::DestroyService, services: true do expect(User.exists?(user.id)).to be(false) end end + + context "migrating associated records" 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 + + service.execute(user) + end + end end end diff --git a/spec/services/users/migrate_to_ghost_user_service_spec.rb b/spec/services/users/migrate_to_ghost_user_service_spec.rb new file mode 100644 index 00000000000..8c5b7e41c15 --- /dev/null +++ b/spec/services/users/migrate_to_ghost_user_service_spec.rb @@ -0,0 +1,64 @@ +require 'spec_helper' + +describe Users::MigrateToGhostUserService, services: true do + let!(:user) { create(:user) } + let!(:project) { create(:project) } + let(:service) { described_class.new(user) } + + context "migrating a user's 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 + + context 'notes' do + include_examples "migrating a deleted user's associated records to the ghost user", Note do + let(:created_record) { create(:note, project: project, author: user) } + end + end + + context 'abuse reports' do + include_examples "migrating a deleted user's associated records to the ghost user", AbuseReport do + let(:created_record) { create(:abuse_report, reporter: user, user: create(:user)) } + end + end + + context 'award emoji' do + include_examples "migrating a deleted user's associated records to the ghost user", AwardEmoji do + let(:created_record) { create(:award_emoji, user: user) } + let(:author_alias) { :user } + + context "when the awardable already has an award emoji of the same name assigned to the ghost user" do + let(:awardable) { create(:issue) } + let!(:existing_award_emoji) { create(:award_emoji, user: User.ghost, name: "thumbsup", awardable: awardable) } + let!(:award_emoji) { create(:award_emoji, user: user, name: "thumbsup", awardable: awardable) } + + it "migrates the award emoji regardless" do + service.execute + + migrated_record = AwardEmoji.find_by_id(award_emoji.id) + + expect(migrated_record.user).to eq(User.ghost) + end + + it "does not leave the migrated award emoji in an invalid state" do + service.execute + + migrated_record = AwardEmoji.find_by_id(award_emoji.id) + + expect(migrated_record).to be_valid + end + end + end + end + end +end |