Add a new feature flag remove_non_gma_memberships

This feature flag is to allow the release of GMAs without including
the side of effects or removing other groups or project memberships
while converting accounts to GMAs, which could be disruptive.
parent 99b3ee82
...@@ -51,9 +51,11 @@ module GroupSaml ...@@ -51,9 +51,11 @@ module GroupSaml
end end
def leave_non_gma_memberships def leave_non_gma_memberships
return true unless ::Feature.enabled?(:remove_non_gma_memberships)
current_user.members.all? do |member| current_user.members.all? do |member|
next true if member.source == group next true if member.source == group
next true if member.source.owner == current_user next true if member.source.owned_by?(current_user)
Members::DestroyService.new(current_user).execute(member) Members::DestroyService.new(current_user).execute(member)
member.destroyed? member.destroyed?
......
...@@ -11,7 +11,7 @@ describe GroupSaml::GroupManagedAccounts::TransferMembershipService do ...@@ -11,7 +11,7 @@ describe GroupSaml::GroupManagedAccounts::TransferMembershipService do
let(:session) { { 'oauth_data' => oauth_data } } let(:session) { { 'oauth_data' => oauth_data } }
before do before do
stub_feature_flags(convert_user_to_group_managed_accounts: true) stub_feature_flags(convert_user_to_group_managed_accounts: true, remove_non_gma_memberships: false)
allow(Gitlab::Auth::GroupSaml::IdentityLinker) allow(Gitlab::Auth::GroupSaml::IdentityLinker)
.to receive(:new).with(current_user, oauth_data, session, group.saml_provider) .to receive(:new).with(current_user, oauth_data, session, group.saml_provider)
...@@ -26,23 +26,57 @@ describe GroupSaml::GroupManagedAccounts::TransferMembershipService do ...@@ -26,23 +26,57 @@ describe GroupSaml::GroupManagedAccounts::TransferMembershipService do
expect(service.execute).to eq true expect(service.execute).to eq true
end end
it 'reduces the amount of memberships' do it 'does not reduce the amount of memberships' do
create(:project_member, :developer, user: current_user) create(:project_member, :developer, user: current_user)
create(:group_member, :developer, user: current_user) create(:group_member, :developer, user: current_user)
expect { service.execute }.to change { current_user.members.count }.by(-2) expect { service.execute }.not_to change { current_user.members.count }
end end
context 'when at least one non-owner member was not removed' do context 'when remove_non_gma_memberships flag is enable' do
before do before do
member = create(:group_member, :developer, user: current_user) stub_feature_flags(remove_non_gma_memberships: true)
allow_next_instance_of(Members::DestroyService) do |service| end
allow(service).to receive(:execute).with(member).and_return(member)
it 'reduces the amount of memberships' do
create(:project_member, :developer, user: current_user)
create(:group_member, :developer, user: current_user)
expect { service.execute }.to change { current_user.members.count }.by(-2)
end
context 'when at least one non-owner member was not removed' do
before do
member = create(:group_member, :developer, user: current_user)
allow_next_instance_of(Members::DestroyService) do |service|
allow(service).to receive(:execute).with(member).and_return(member)
end
end
it 'returns a falsy value' do
expect(service.execute).to be_falsy
end end
end end
it 'returns a falsy value' do context 'when the user changes are not saved' do
expect(service.execute).to be_falsy before do
allow(current_user).to receive(:save).and_return(false)
end
it "doesn't remove account's members" do
create(:group_member, :developer, user: current_user)
expect { service.execute }.not_to change { current_user.members.count }
end
end
context 'when the user is owner of a group' do
it 'does not reduce the amount of memberships' do
other_group = create(:group)
other_group.add_owner(current_user)
expect { service.execute }.not_to change { current_user.members.count }
end
end end
end end
...@@ -51,12 +85,6 @@ describe GroupSaml::GroupManagedAccounts::TransferMembershipService do ...@@ -51,12 +85,6 @@ describe GroupSaml::GroupManagedAccounts::TransferMembershipService do
allow(current_user).to receive(:save).and_return(false) allow(current_user).to receive(:save).and_return(false)
end end
it "doesn't remove account's members" do
create(:group_member, :developer, user: current_user)
expect { service.execute }.not_to change { current_user.members.count }
end
it "doesn't remove account's identities" do it "doesn't remove account's identities" do
create(:identity, user: current_user) create(:identity, user: current_user)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment