Commit 5984046c authored by Imre Farkas's avatar Imre Farkas

Merge branch '211374-move-group-project-removal-behind-a-new-feature-flag' into 'master'

Add a new feature flag remove_non_gma_memberships

Closes #211374

See merge request gitlab-org/gitlab!27581
parents e10e68da 030e02be
...@@ -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