diff options
author | Timothy Andrew <mail@timothyandrew.net> | 2017-04-06 20:03:07 +0300 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2017-04-06 20:09:40 +0300 |
commit | 1c42505b026d922df50c59d5f9e85073b5f5345f (patch) | |
tree | b94259c6aa9c4a6e2e24d00d91c4d548a7bd6953 /spec/services/users | |
parent | e152f3f3daf09b25e5a5952a0e62580b3ef96c50 (diff) |
Implement review comments from @DouweM for !10467.
1. Have `MigrateToGhostUser` be a service rather than a mixed-in module, to keep
things explicit. Specs testing the behavior of this class are moved into a
separate service spec file.
2. Add a `user.reported_abuse_reports` association to make the
`migrate_abuse_reports` method more consistent with the other `migrate_`
methods.
Diffstat (limited to 'spec/services/users')
-rw-r--r-- | spec/services/users/destroy_service_spec.rb | 91 | ||||
-rw-r--r-- | spec/services/users/migrate_to_ghost_user_service_spec.rb | 64 |
2 files changed, 87 insertions, 68 deletions
diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index d3463185ebe..43c18992d1a 100644 --- a/spec/services/users/destroy_service_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 @@ -142,60 +146,11 @@ describe Users::DestroyService, services: true do 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 - - context 'notes' do - include_examples "migrating a deleted user's associated records to the ghost user", Note, { skip_assignee_specs: true } 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, { skip_assignee_specs: true } do - let(:created_record) { create(:abuse_report, reporter: user, user: create(:user)) } - 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 - context 'award emoji' do - include_examples "migrating a deleted user's associated records to the ghost user", AwardEmoji, { skip_assignee_specs: true } 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(user) - - 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(user) - - migrated_record = AwardEmoji.find_by_id(award_emoji.id) - - expect(migrated_record).to be_valid - end - end - end + service.execute(user) 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 |