From 72580f07af5a2c1e4df6bbc339ad804b5f5bb9ed Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Wed, 5 Apr 2017 12:50:53 +0530 Subject: Move a user's merge requests to the ghost user. 1. When the user is deleted. 2. Refactor out code relating to "migrating records to the ghost user" into a `MigrateToGhostUser` concern, which is tested using a shared example. --- spec/services/users/destroy_service_spec.rb | 161 ++++++++++++++++++++++++++++ spec/services/users/destroy_spec.rb | 145 ------------------------- 2 files changed, 161 insertions(+), 145 deletions(-) create mode 100644 spec/services/users/destroy_service_spec.rb delete mode 100644 spec/services/users/destroy_spec.rb (limited to 'spec/services') diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb new file mode 100644 index 00000000000..a5b69e6ddf4 --- /dev/null +++ b/spec/services/users/destroy_service_spec.rb @@ -0,0 +1,161 @@ +require 'spec_helper' + +describe Users::DestroyService, services: true do + describe "Deletes a user and all their personal projects" do + let!(:user) { create(:user) } + let!(:admin) { create(:admin) } + let!(:namespace) { create(:namespace, owner: user) } + let!(:project) { create(:empty_project, namespace: namespace) } + let(:service) { described_class.new(admin) } + + context 'no options are given' do + it 'deletes the user' do + user_data = service.execute(user) + + expect { user_data['email'].to eq(user.email) } + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Namespace.with_deleted.find(user.namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'will delete the project' do + expect_any_instance_of(Projects::DestroyService).to receive(:execute).once + + service.execute(user) + end + end + + context 'projects in pending_delete' do + before do + project.pending_delete = true + project.save + end + + it 'destroys a project in pending_delete' do + expect_any_instance_of(Projects::DestroyService).to receive(:execute).once + + service.execute(user) + + expect { Project.find(project.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context "a deleted user's issues" do + let(:project) { create(:project) } + + before do + project.add_developer(user) + end + + context "for an issue the user has created" do + let!(:issue) { create(:issue, project: project, author: user) } + + before do + service.execute(user) + end + + it 'does not delete the issue' 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 + migrated_issue = Issue.find_by_id(issue.id) + + expect(migrated_issue.author).to eq(User.ghost) + end + + it 'blocks the user before migrating issues to the "Ghost User' do + expect(user).to be_blocked + end + end + + 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 issues the user is assigned to' do + expect(Issue.find_by_id(issue.id)).to be_present + end + + it 'migrates the issue so that it is "Unassigned"' do + migrated_issue = Issue.find_by_id(issue.id) + + expect(migrated_issue.assignee).to be_nil + end + end + end + + context "solo owned groups present" do + let(:solo_owned) { create(:group) } + let(:member) { create(:group_member) } + let(:user) { member.user } + + before do + solo_owned.group_members = [member] + service.execute(user) + end + + it 'does not delete the user' do + expect(User.find(user.id)).to eq user + end + end + + context "deletions with solo owned groups" do + let(:solo_owned) { create(:group) } + let(:member) { create(:group_member) } + let(:user) { member.user } + + before do + solo_owned.group_members = [member] + service.execute(user, delete_solo_owned_groups: true) + end + + it 'deletes solo owned groups' do + expect { Project.find(solo_owned.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'deletes the user' do + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context "deletion permission checks" do + it 'does not delete the user when user is not an admin' do + other_user = create(:user) + + expect { described_class.new(other_user).execute(user) }.to raise_error(Gitlab::Access::AccessDeniedError) + expect(User.exists?(user.id)).to be(true) + end + + it 'allows admins to delete anyone' do + described_class.new(admin).execute(user) + + expect(User.exists?(user.id)).to be(false) + end + + it 'allows users to delete their own account' do + described_class.new(user).execute(user) + + expect(User.exists?(user.id)).to be(false) + 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 + end + end +end diff --git a/spec/services/users/destroy_spec.rb b/spec/services/users/destroy_spec.rb deleted file mode 100644 index 66c61b7f8ff..00000000000 --- a/spec/services/users/destroy_spec.rb +++ /dev/null @@ -1,145 +0,0 @@ -require 'spec_helper' - -describe Users::DestroyService, services: true do - describe "Deletes a user and all their personal projects" do - let!(:user) { create(:user) } - let!(:admin) { create(:admin) } - let!(:namespace) { create(:namespace, owner: user) } - let!(:project) { create(:empty_project, namespace: namespace) } - let(:service) { described_class.new(admin) } - - context 'no options are given' do - it 'deletes the user' do - user_data = service.execute(user) - - expect { user_data['email'].to eq(user.email) } - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - expect { Namespace.with_deleted.find(user.namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) - end - - it 'will delete the project' do - expect_any_instance_of(Projects::DestroyService).to receive(:execute).once - - service.execute(user) - end - end - - context 'projects in pending_delete' do - before do - project.pending_delete = true - project.save - end - - it 'destroys a project in pending_delete' do - expect_any_instance_of(Projects::DestroyService).to receive(:execute).once - - service.execute(user) - - expect { Project.find(project.id) }.to raise_error(ActiveRecord::RecordNotFound) - end - end - - context "a deleted user's issues" do - let(:project) { create(:project) } - - before do - project.add_developer(user) - end - - context "for an issue the user has created" do - let!(:issue) { create(:issue, project: project, author: user) } - - before do - service.execute(user) - end - - it 'does not delete the issue' 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 - migrated_issue = Issue.find_by_id(issue.id) - - expect(migrated_issue.author).to eq(User.ghost) - end - - it 'blocks the user before migrating issues to the "Ghost User' do - expect(user).to be_blocked - end - end - - 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 issues the user is assigned to' do - expect(Issue.find_by_id(issue.id)).to be_present - end - - it 'migrates the issue so that it is "Unassigned"' do - migrated_issue = Issue.find_by_id(issue.id) - - expect(migrated_issue.assignee).to be_nil - end - end - end - - context "solo owned groups present" do - let(:solo_owned) { create(:group) } - let(:member) { create(:group_member) } - let(:user) { member.user } - - before do - solo_owned.group_members = [member] - service.execute(user) - end - - it 'does not delete the user' do - expect(User.find(user.id)).to eq user - end - end - - context "deletions with solo owned groups" do - let(:solo_owned) { create(:group) } - let(:member) { create(:group_member) } - let(:user) { member.user } - - before do - solo_owned.group_members = [member] - service.execute(user, delete_solo_owned_groups: true) - end - - it 'deletes solo owned groups' do - expect { Project.find(solo_owned.id) }.to raise_error(ActiveRecord::RecordNotFound) - end - - it 'deletes the user' do - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - end - end - - context "deletion permission checks" do - it 'does not delete the user when user is not an admin' do - other_user = create(:user) - - expect { described_class.new(other_user).execute(user) }.to raise_error(Gitlab::Access::AccessDeniedError) - expect(User.exists?(user.id)).to be(true) - end - - it 'allows admins to delete anyone' do - described_class.new(admin).execute(user) - - expect(User.exists?(user.id)).to be(false) - end - - it 'allows users to delete their own account' do - described_class.new(user).execute(user) - - expect(User.exists?(user.id)).to be(false) - end - end - end -end -- cgit v1.2.3