Skip to content

G
gitlab-ce
Commit b684a009 authored by Lucas Zampieri's avatar Lucas Zampieri
Browse files
Options

Simplified Specs for new relations fetch 

Added tests for shared groups;
Simplified tests to check individual relations.
Signed-off-by: default avatarLucas Zampieri <lzampier@redhat.com>
parent d29c665b
Hide whitespace changes
Inline Side-by-side
Showing with 73 additions and 112 deletions
app/finders/group_members_finder.rb
View file @ b684a009
# frozen_string_literal: true
class GroupMembersFinder < UnionFinder
RELATIONS = %i(direct inherited descendants shared_with_groups).freeze
RELATIONS = %i(direct inherited descendants shared_from_groups).freeze
DEFAULT_RELATIONS = %i(direct inherited).freeze
INVALID_RELATION_TYPE_ERROR_MSG = "is not a valid relation type. Valid relation types are #{RELATIONS.join(', ')}."
......@@ -30,8 +30,6 @@ class GroupMembersFinder < UnionFinder
def execute(include_relations: DEFAULT_RELATIONS)
groups = groups_by_relations(include_relations)
return GroupMember.none unless groups
members = all_group_members(groups).distinct_on_user_with_max_access_level
filter_members(members)
......@@ -44,14 +42,14 @@ class GroupMembersFinder < UnionFinder
def groups_by_relations(include_relations)
check_relation_arguments!(include_relations)
members = []
related_groups = []
members << Group.where(id: group.id) if include_relations.include?(:direct)
members << group.ancestors if include_relations.include?(:inherited)
members << group.descendants if include_relations.include?(:descendants)
members << group.shared_with_groups.public_or_visible_to_user(user) if include_relations.include?(:shared_from_groups)
related_groups << Group.by_id(group.id) if include_relations&.include?(:direct)
related_groups << group.ancestors if include_relations&.include?(:inherited)
related_groups << group.descendants if include_relations&.include?(:descendants)
related_groups << group.shared_with_groups.public_or_visible_to_user(user) if include_relations&.include?(:shared_from_groups)
find_union(members, Group)
find_union(related_groups, Group)
end
def filter_members(members)
......
app/models/members/group_member.rb
View file @ b684a009
......@@ -18,7 +18,7 @@ class GroupMember < Member
default_scope { where(source_type: SOURCE_TYPE) } # rubocop:disable Cop/DefaultScope
scope :of_groups, ->(groups) { where(source_id: groups.select(:id)) }
scope :of_groups, ->(groups) { where(source_id: groups&.select(:id)) }
scope :of_ldap_type, -> { where(ldap: true) }
scope :count_users_by_group_id, -> { group(:source_id).count }
scope :with_user, -> (user) { where(user: user) }
......
doc/api/graphql/reference/index.md
View file @ b684a009
......@@ -16418,7 +16418,7 @@ Group member relation.
| <a id="groupmemberrelationdescendants"></a>`DESCENDANTS` | Members in the group's subgroups. |
| <a id="groupmemberrelationdirect"></a>`DIRECT` | Members in the group itself. |
| <a id="groupmemberrelationinherited"></a>`INHERITED` | Members in the group's ancestor groups. |
| <a id="groupmemberrelationshared_with_groups"></a>`SHARED_WITH_GROUPS` | Invited group's members. |
| <a id="groupmemberrelationshared_from_groups"></a>`SHARED_FROM_GROUPS` | Invited group's members. |
### `GroupPermission`
spec/finders/group_members_finder_spec.rb
View file @ b684a009
......@@ -3,83 +3,88 @@
require 'spec_helper'
RSpec.describe GroupMembersFinder, '#execute' do
let(:group) { create(:group) }
let(:sub_group) { create(:group, parent: group) }
let(:sub_sub_group) { create(:group, parent: sub_group) }
let(:shared_group) { create(:group, :public) }
let(:user1) { create(:user) }
let(:user2) { create(:user) }
let(:user3) { create(:user) }
let(:user4) { create(:user) }
let(:user5) { create(:user, :two_factor_via_otp) }
let_it_be(:group) { create(:group) }
let_it_be(:sub_group) { create(:group, parent: group) }
let_it_be(:sub_sub_group) { create(:group, parent: sub_group) }
let_it_be(:public_shared_group) { create(:group, :public) }
let_it_be(:private_shared_group) { create(:group, :private) }
let_it_be(:user1) { create(:user) }
let_it_be(:user2) { create(:user) }
let_it_be(:user3) { create(:user) }
let_it_be(:user4) { create(:user) }
let_it_be(:user5) { create(:user, :two_factor_via_otp) }
let!(:link) do
create(:group_group_link, shared_group: group, shared_with_group: public_shared_group)
create(:group_group_link, shared_group: sub_group, shared_with_group: private_shared_group)
end
let(:groups) do
{
group: group,
sub_group: sub_group,
sub_sub_group: sub_sub_group,
shared_group: shared_group
group: group,
sub_group: sub_group,
sub_sub_group: sub_sub_group,
public_shared_group: public_shared_group,
private_shared_group: private_shared_group
}
end
context 'relations' do
let!(:members) do
{
user1_sub_sub_group: create(:group_member, :maintainer, group: sub_sub_group, user: user1),
user1_sub_group: create(:group_member, :developer, group: sub_group, user: user1),
user1_group: create(:group_member, :reporter, group: group, user: user1),
user1_shared_group: create(:group_member, :maintainer, group: shared_group, user: user1),
user2_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user2),
user2_sub_group: create(:group_member, :developer, group: sub_group, user: user2),
user2_group: create(:group_member, :maintainer, group: group, user: user2),
user2_shared_group: create(:group_member, :developer, group: shared_group, user: user2),
user3_sub_sub_group: create(:group_member, :developer, group: sub_sub_group, user: user3, expires_at: 1.day.from_now),
user3_sub_group: create(:group_member, :developer, group: sub_group, user: user3, expires_at: 2.days.from_now),
user3_group: create(:group_member, :reporter, group: group, user: user3),
user3_shared_group: create(:group_member, :reporter, group: shared_group, user: user3),
user4_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user4),
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_shared_group: create(:group_member, :developer, group: shared_group, user: user4)
user1_sub_sub_group: create(:group_member, :maintainer, group: sub_sub_group, user: user1),
user1_sub_group: create(:group_member, :developer, group: sub_group, user: user1),
user1_group: create(:group_member, :reporter, group: group, user: user1),
user1_public_shared_group: create(:group_member, :maintainer, group: public_shared_group, user: user1),
user1_private_shared_group: create(:group_member, :maintainer, group: private_shared_group, user: user1),
user2_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user2),
user2_sub_group: create(:group_member, :developer, group: sub_group, user: user2),
user2_group: create(:group_member, :maintainer, group: group, user: user2),
user2_public_shared_group: create(:group_member, :developer, group: public_shared_group, user: user2),
user2_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user2),
user3_sub_sub_group: create(:group_member, :developer, group: sub_sub_group, user: user3, expires_at: 1.day.from_now),
user3_sub_group: create(:group_member, :developer, group: sub_group, user: user3, expires_at: 2.days.from_now),
user3_group: create(:group_member, :reporter, group: group, user: user3),
user3_public_shared_group: create(:group_member, :reporter, group: public_shared_group, user: user3),
user3_private_shared_group: create(:group_member, :reporter, group: private_shared_group, user: user3),
user4_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user4),
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)
}
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.")
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
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]
[:direct, :inherited] | :group | [:user1_group, :user2_group, :user3_group, :user4_group]
[:direct, :descendants] | :group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group]
[:descendants, :inherited] | :group | [:user1_sub_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group]
[:direct, :descendants, :inherited] | :group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group]
[:direct, :inherited, :shared_with_groups] | :group | [:user1_group, :user2_group, :user3_group, :user4_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]
[:direct, :inherited] | :sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group]
[:direct, :descendants] | :sub_group | [:user1_sub_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group]
[:descendants, :inherited] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_sub_group, :user4_group]
[:direct, :descendants, :inherited] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group]
[:direct, :inherited, :shared_with_groups] | :sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_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 | []
[:direct, :inherited] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group]
[:direct, :descendants] | :sub_sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group]
[:descendants, :inherited] | :sub_sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group]
[:direct, :descendants, :inherited] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group]
[:direct, :inherited, :shared_with_groups] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group]
[] | :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 | []
[:direct, :inherited, :descendants, :shared_from_groups] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_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 | []
[:direct, :inherited, :descendants, :shared_from_groups] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group]
end
with_them do
......@@ -190,46 +195,4 @@ RSpec.describe GroupMembersFinder, '#execute' do
expect(result.to_a).to match_array([member1])
end
end
context 'when :shared_with_groups is passed' do
let(:member1) { group.add_owner(user1) }
let(:member2) { shared_group.add_maintainer(user5) }
let(:member3) { shared_group.add_maintainer(user3) }
subject { described_class.new(group, user1).execute(include_relations: [:direct, :inherited, :shared_with_groups]) }
before do
create(:group_group_link, shared_group: group, shared_with_group: shared_group)
end
context "and shared_group is public" do
before do
shared_group.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
end
it 'includes all the shared_with_groups members' do
expect(subject).to match_array([member1, member2, member3])
end
end
context "and shared_group is private" do
before do
shared_group.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
end
it "does not return shared_with_groups members" do
expect(subject).to match_array([member1])
end
context "and current_user is on shared group" do
before do
shared_group.add_developer(user1)
end
it 'includes all the shared_with_groups members' do
expect(subject).to match_array([member1, member2, member3])
end
end
end
end
end
spec/graphql/types/group_member_relation_enum_spec.rb
View file @ b684a009
......@@ -6,6 +6,6 @@ RSpec.describe Types::GroupMemberRelationEnum do
specify { expect(described_class.graphql_name).to eq('GroupMemberRelation') }
it 'exposes all the existing group member relation type values' do
expect(described_class.values.keys).to contain_exactly('DIRECT', 'INHERITED', 'DESCENDANTS', 'SHARED_WITH_GROUPS')
expect(described_class.values.keys).to contain_exactly('DIRECT', 'INHERITED', 'DESCENDANTS', 'SHARED_FROM_GROUPS')
end
end
spec/requests/api/graphql/group/group_members_spec.rb
View file @ b684a009
......@@ -76,7 +76,7 @@ RSpec.describe 'getting group members information' do
end
it 'returns invited members plus inherited members' do
fetch_members(group: child_group, args: { relations: [:DIRECT, :INHERITED, :SHARED_WITH_GROUPS] })
fetch_members(group: child_group, args: { relations: [:DIRECT, :INHERITED, :SHARED_FROM_GROUPS] })
expect(graphql_errors).to be_nil
expect_array_response(invited_user, user_1, user_2, child_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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7