From d2cd9d96965722cca06792c63d76d2704366d7a5 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Thu, 18 Aug 2016 21:32:42 +0100
Subject: [PATCH] Ensure last group owner isn't removed on expiry
---
.../members/authorized_destroy_service.rb | 2 +
.../remove_expired_members_worker_spec.rb | 59 +++++++++++++++----
2 files changed, 48 insertions(+), 13 deletions(-)
diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb
index c23f90a6a10..ca9db59cac7 100644
--- a/app/services/members/authorized_destroy_service.rb
+++ b/app/services/members/authorized_destroy_service.rb
@@ -7,6 +7,8 @@ module Members
end
def execute
+ return false if member.is_a?(GroupMember) && member.source.last_owner?(member.user)
+
member.destroy
if member.request? && member.user != user
diff --git a/spec/workers/remove_expired_members_worker_spec.rb b/spec/workers/remove_expired_members_worker_spec.rb
index 75a5bee7ab4..a6ce926b925 100644
--- a/spec/workers/remove_expired_members_worker_spec.rb
+++ b/spec/workers/remove_expired_members_worker_spec.rb
@@ -1,25 +1,58 @@
require 'spec_helper'
describe RemoveExpiredMembersWorker do
- let!(:worker) { RemoveExpiredMembersWorker.new }
- let!(:expired_member) { create(:project_member, expires_at: 1.hour.ago) }
- let!(:member_expiring_in_future) { create(:project_member, expires_at: 10.days.from_now) }
- let!(:non_expiring_member) { create(:project_member, expires_at: nil) }
+ let(:worker) { RemoveExpiredMembersWorker.new }
describe '#perform' do
- it 'removes expired members' do
- expect { worker.perform }.to change { Member.count }.by(-1)
- expect(Member.find_by(id: expired_member.id)).to be_nil
+ context 'project members' do
+ let!(:expired_project_member) { create(:project_member, expires_at: 1.hour.ago, access_level: GroupMember::DEVELOPER) }
+ let!(:project_member_expiring_in_future) { create(:project_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) }
+ let!(:non_expiring_project_member) { create(:project_member, expires_at: nil, access_level: GroupMember::DEVELOPER) }
+
+ it 'removes expired members' do
+ expect { worker.perform }.to change { Member.count }.by(-1)
+ expect(Member.find_by(id: expired_project_member.id)).to be_nil
+ end
+
+ it 'leaves members who expire in the future' do
+ worker.perform
+ expect(project_member_expiring_in_future.reload).to be_present
+ end
+
+ it 'leaves members who do not expire at all' do
+ worker.perform
+ expect(non_expiring_project_member.reload).to be_present
+ end
end
- it 'leaves members who expire in the future' do
- worker.perform
- expect(member_expiring_in_future.reload).to be_present
+ context 'group members' do
+ let!(:expired_group_member) { create(:group_member, expires_at: 1.hour.ago, access_level: GroupMember::DEVELOPER) }
+ let!(:group_member_expiring_in_future) { create(:group_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) }
+ let!(:non_expiring_group_member) { create(:group_member, expires_at: nil, access_level: GroupMember::DEVELOPER) }
+
+ it 'removes expired members' do
+ expect { worker.perform }.to change { Member.count }.by(-1)
+ expect(Member.find_by(id: expired_group_member.id)).to be_nil
+ end
+
+ it 'leaves members who expire in the future' do
+ worker.perform
+ expect(group_member_expiring_in_future.reload).to be_present
+ end
+
+ it 'leaves members who do not expire at all' do
+ worker.perform
+ expect(non_expiring_group_member.reload).to be_present
+ end
end
- it 'leaves members who do not expire at all' do
- worker.perform
- expect(non_expiring_member.reload).to be_present
+ context 'when the last group owner expires' do
+ let!(:expired_group_owner) { create(:group_member, expires_at: 1.hour.ago, access_level: GroupMember::OWNER) }
+
+ it 'does not delete the owner' do
+ worker.perform
+ expect(expired_group_owner.reload).to be_present
+ end
end
end
end
--
GitLab