diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb index 1025e0ebc9b1885fa1a4a7af22231dd1f1df76aa..639db58b00d2718095d045e601a245da9294ab9a 100644 --- a/app/finders/group_members_finder.rb +++ b/app/finders/group_members_finder.rb @@ -86,11 +86,6 @@ def all_group_members(groups, shared_from_groups) end def members_of_groups(groups, shared_from_groups) - if Feature.disabled?(:members_with_shared_group_access, @group.root_ancestor) - groups << shared_from_groups unless shared_from_groups.nil? - return GroupMember.non_request.of_groups(find_union(groups, Group)) - end - members = GroupMember.non_request.of_groups(find_union(groups, Group)) return members if shared_from_groups.nil? diff --git a/config/feature_flags/development/members_with_shared_group_access.yml b/config/feature_flags/development/members_with_shared_group_access.yml deleted file mode 100644 index f9d9a4fede44634657a15f2d09e7a8ebd1faf117..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/members_with_shared_group_access.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: members_with_shared_group_access -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115346 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/398621 -milestone: '15.11' -type: development -group: group::tenant scale -default_enabled: true diff --git a/spec/finders/group_members_finder_spec.rb b/spec/finders/group_members_finder_spec.rb index 4ac4dc3ba3728de5491327c8b20e7176feaf8ec0..18473a5e70bb4872d52e79bf747132d2bb5f2732 100644 --- a/spec/finders/group_members_finder_spec.rb +++ b/spec/finders/group_members_finder_spec.rb @@ -51,55 +51,51 @@ user4_sub_group: create(:group_member, :developer, group: sub_group, user: user4, expires_at: 1.day.from_now), user4_group: create(:group_member, :developer, group: group, user: user4, expires_at: 2.days.from_now), user4_public_shared_group: create(:group_member, :developer, group: public_shared_group, user: user4), - user4_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user4), - user5_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user5_2fa) + user4_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user4), + user5_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user5_2fa) } end - shared_examples 'member relations' do - it 'raises an error if a non-supported relation type is used' do - expect do - described_class.new(group).execute(include_relations: [:direct, :invalid_relation_type]) - end.to raise_error(ArgumentError, "invalid_relation_type is not a valid relation type. Valid relation types are direct, inherited, descendants, shared_from_groups.") - end + it 'raises an error if a non-supported relation type is used' do + expect do + described_class.new(group).execute(include_relations: [:direct, :invalid_relation_type]) + end.to raise_error(ArgumentError, "invalid_relation_type is not a valid relation type. Valid relation types are direct, inherited, descendants, shared_from_groups.") + end - using RSpec::Parameterized::TableSyntax - - where(:subject_relations, :subject_group, :expected_members) do - [] | :group | [] - GroupMembersFinder::DEFAULT_RELATIONS | :group | [:user1_group, :user2_group, :user3_group, :user4_group] - [:direct] | :group | [:user1_group, :user2_group, :user3_group, :user4_group] - [:inherited] | :group | [] - [:descendants] | :group | [:user1_sub_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] - [:shared_from_groups] | :group | [:user1_public_shared_group, :user2_public_shared_group, :user3_public_shared_group, :user4_public_shared_group] - [:direct, :inherited, :descendants, :shared_from_groups] | :group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_public_shared_group] - [] | :sub_group | [] - GroupMembersFinder::DEFAULT_RELATIONS | :sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:direct] | :sub_group | [:user1_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] - [:inherited] | :sub_group | [:user1_group, :user2_group, :user3_group, :user4_group] - [:descendants] | :sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] - [:shared_from_groups] | :sub_group | [:user1_public_shared_group, :user2_public_shared_group, :user3_public_shared_group, :user4_public_shared_group] - [:direct, :inherited, :descendants, :shared_from_groups] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_public_shared_group] - [] | :sub_sub_group | [] - GroupMembersFinder::DEFAULT_RELATIONS | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:direct] | :sub_sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] - [:inherited] | :sub_sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:descendants] | :sub_sub_group | [] - [:shared_from_groups] | :sub_sub_group | [:user1_public_shared_group, :user2_public_shared_group, :user3_public_shared_group, :user4_public_shared_group] - [:direct, :inherited, :descendants, :shared_from_groups] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_public_shared_group] - end + using RSpec::Parameterized::TableSyntax + + where(:subject_relations, :subject_group, :expected_members) do + [] | :group | [] + GroupMembersFinder::DEFAULT_RELATIONS | :group | [:user1_group, :user2_group, :user3_group, :user4_group] + [:direct] | :group | [:user1_group, :user2_group, :user3_group, :user4_group] + [:inherited] | :group | [] + [:descendants] | :group | [:user1_sub_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] + [:shared_from_groups] | :group | [:user1_public_shared_group, :user2_public_shared_group, :user3_public_shared_group, :user4_public_shared_group] + [:direct, :inherited, :descendants, :shared_from_groups] | :group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_public_shared_group] + [] | :sub_group | [] + GroupMembersFinder::DEFAULT_RELATIONS | :sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] + [:direct] | :sub_group | [:user1_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] + [:inherited] | :sub_group | [:user1_group, :user2_group, :user3_group, :user4_group] + [:descendants] | :sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] + [:shared_from_groups] | :sub_group | [:user1_public_shared_group, :user2_public_shared_group, :user3_public_shared_group, :user4_public_shared_group] + [:direct, :inherited, :descendants, :shared_from_groups] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_public_shared_group] + [] | :sub_sub_group | [] + GroupMembersFinder::DEFAULT_RELATIONS | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] + [:direct] | :sub_sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] + [:inherited] | :sub_sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] + [:descendants] | :sub_sub_group | [] + [:shared_from_groups] | :sub_sub_group | [:user1_public_shared_group, :user2_public_shared_group, :user3_public_shared_group, :user4_public_shared_group] + [:direct, :inherited, :descendants, :shared_from_groups] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_public_shared_group] + end - with_them do - it 'returns correct members' do - result = described_class.new(groups[subject_group]).execute(include_relations: subject_relations) + with_them do + it 'returns correct members' do + result = described_class.new(groups[subject_group]).execute(include_relations: subject_relations) - expect(result.to_a).to match_array(expected_members.map { |name| members[name] }) - end + expect(result.to_a).to match_array(expected_members.map { |name| members[name] }) end end - it_behaves_like 'member relations' - it 'returns the correct access level of the members shared through group sharing' do shared_members_access = described_class .new(groups[:group]) @@ -110,14 +106,6 @@ correct_access_levels = ([Gitlab::Access::DEVELOPER] * 3) << Gitlab::Access::REPORTER expect(shared_members_access).to match_array(correct_access_levels) end - - context 'when members_with_shared_group_access feature flag is disabled' do - before do - stub_feature_flags(members_with_shared_group_access: false) - end - - it_behaves_like 'member relations' - end end context 'search' do