Merge branch 'allow-filtering-by-member-relations-in-graphql' into 'master'

Allow filtering by member relations in GraphQL

See merge request gitlab-org/gitlab!48372

app/finders/group_members_finder.rb

 # frozen_string_literal: true
 
 class GroupMembersFinder < UnionFinder
+  RELATIONS = %i(direct inherited descendants).freeze
+  DEFAULT_RELATIONS = %i(direct inherited).freeze
+
   include CreatedAtFilter
 
   # Params can be any of the following:
@@ -17,7 +20,7 @@ class GroupMembersFinder < UnionFinder
     @params = params
   end
 
-  def execute(include_relations: [:inherited, :direct])
+  def execute(include_relations: DEFAULT_RELATIONS)
     group_members = group_members_list
     relations = []
 

app/finders/members_finder.rb

 # frozen_string_literal: true
 
 class MembersFinder
+  RELATIONS = %i(direct inherited descendants invited_groups).freeze
+  DEFAULT_RELATIONS = %i(direct inherited).freeze
+
   # Params can be any of the following:
   #   sort:       string
   #   search:     string
@@ -13,7 +16,7 @@ class MembersFinder
     @params = params
   end
 
-  def execute(include_relations: [:inherited, :direct])
+  def execute(include_relations: DEFAULT_RELATIONS)
     members = find_members(include_relations)
 
     filter_members(members)
@@ -56,7 +59,7 @@ class MembersFinder
   def group_union_members(include_relations)
     [].tap do |members|
       members << direct_group_members(include_relations.include?(:descendants)) if group
-      members << project_invited_groups_members if include_relations.include?(:invited_groups_members)
+      members << project_invited_groups if include_relations.include?(:invited_groups)
     end
   end
 
@@ -66,7 +69,7 @@ class MembersFinder
     GroupMembersFinder.new(group).execute(include_relations: requested_relations).non_invite.non_minimal_access # rubocop: disable CodeReuse/Finder
   end
 
-  def project_invited_groups_members
+  def project_invited_groups
     invited_groups_ids_including_ancestors = Gitlab::ObjectHierarchy
       .new(project.invited_groups)
       .base_and_ancestors

app/graphql/resolvers/group_members_resolver.rb

@@ -6,6 +6,11 @@ module Resolvers
 
     authorize :read_group_member
 
+    argument :relations, [Types::GroupMemberRelationEnum],
+              description: 'Filter members by the given member relations',
+              required: false,
+              default_value: GroupMembersFinder::DEFAULT_RELATIONS
+
     private
 
     def preloads

app/graphql/resolvers/members_resolver.rb

@@ -14,7 +14,9 @@ module Resolvers
     def resolve_with_lookahead(**args)
       authorize!(object)
 
-      apply_lookahead(finder_class.new(object, current_user, params: args).execute)
+      relations = args.delete(:relations)
+
+      apply_lookahead(finder_class.new(object, current_user, params: args).execute(include_relations: relations))
     end
 
     private

app/graphql/resolvers/project_members_resolver.rb

@@ -5,6 +5,11 @@ module Resolvers
   class ProjectMembersResolver < MembersResolver
     authorize :read_project_member
 
+    argument :relations, [Types::ProjectMemberRelationEnum],
+              description: 'Filter members by the given member relations',
+              required: false,
+              default_value: MembersFinder::DEFAULT_RELATIONS
+
     private
 
     def finder_class

app/graphql/types/group_member_relation_enum.rb

+# frozen_string_literal: true
+
+module Types
+  class GroupMemberRelationEnum < BaseEnum
+    graphql_name 'GroupMemberRelation'
+    description 'Group member relation'
+
+    ::GroupMembersFinder::RELATIONS.each do |member_relation|
+      value member_relation.to_s.upcase, value: member_relation, description: "#{member_relation.to_s.titleize} members"
+    end
+  end
+end

app/graphql/types/project_member_relation_enum.rb

+# frozen_string_literal: true
+
+module Types
+  class ProjectMemberRelationEnum < BaseEnum
+    graphql_name 'ProjectMemberRelation'
+    description 'Project member relation'
+
+    ::MembersFinder::RELATIONS.each do |member_relation|
+      value member_relation.to_s.upcase, value: member_relation, description: "#{member_relation.to_s.titleize} members"
+    end
+  end
+end

changelogs/unreleased/allow-filtering-by-member-relations-in-graphql.yml

+---
+title: Allow filtering project and group members by relationship in GraphQL
+merge_request: 48372
+author:
+type: changed

doc/api/graphql/reference/gitlab_schema.graphql

@@ -9103,6 +9103,11 @@ type Group {
     """
     last: Int
 
+    """
+    Filter members by the given member relations
+    """
+    relations: [GroupMemberRelation!] = [DIRECT, INHERITED]
+
     """
     Search query
     """
@@ -10007,6 +10012,26 @@ type GroupMemberEdge {
   node: GroupMember
 }
 
+"""
+Group member relation
+"""
+enum GroupMemberRelation {
+  """
+  Descendants members
+  """
+  DESCENDANTS
+
+  """
+  Direct members
+  """
+  DIRECT
+
+  """
+  Inherited members
+  """
+  INHERITED
+}
+
 type GroupPermissions {
   """
   Indicates the user can perform `read_group` on this resource
@@ -16625,6 +16650,11 @@ type Project {
     """
     last: Int
 
+    """
+    Filter members by the given member relations
+    """
+    relations: [ProjectMemberRelation!] = [DIRECT, INHERITED]
+
     """
     Search query
     """
@@ -17262,6 +17292,31 @@ type ProjectMemberEdge {
   node: ProjectMember
 }
 
+"""
+Project member relation
+"""
+enum ProjectMemberRelation {
+  """
+  Descendants members
+  """
+  DESCENDANTS
+
+  """
+  Direct members
+  """
+  DIRECT
+
+  """
+  Inherited members
+  """
+  INHERITED
+
+  """
+  Invited Groups members
+  """
+  INVITED_GROUPS
+}
+
 type ProjectPermissions {
   """
   Indicates the user can perform `admin_operations` on this resource

doc/api/graphql/reference/gitlab_schema.json

@@ -25128,6 +25128,24 @@
                   },
                   "defaultValue": null
                 },
+                {
+                  "name": "relations",
+                  "description": "Filter members by the given member relations",
+                  "type": {
+                    "kind": "LIST",
+                    "name": null,
+                    "ofType": {
+                      "kind": "NON_NULL",
+                      "name": null,
+                      "ofType": {
+                        "kind": "ENUM",
+                        "name": "GroupMemberRelation",
+                        "ofType": null
+                      }
+                    }
+                  },
+                  "defaultValue": "[DIRECT, INHERITED]"
+                },
                 {
                   "name": "after",
                   "description": "Returns the elements in the list that come after the specified cursor.",
@@ -27424,6 +27442,35 @@
           "enumValues": null,
           "possibleTypes": null
         },
+        {
+          "kind": "ENUM",
+          "name": "GroupMemberRelation",
+          "description": "Group member relation",
+          "fields": null,
+          "inputFields": null,
+          "interfaces": null,
+          "enumValues": [
+            {
+              "name": "DIRECT",
+              "description": "Direct members",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "INHERITED",
+              "description": "Inherited members",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "DESCENDANTS",
+              "description": "Descendants members",
+              "isDeprecated": false,
+              "deprecationReason": null
+            }
+          ],
+          "possibleTypes": null
+        },
         {
           "kind": "OBJECT",
           "name": "GroupPermissions",
@@ -48635,6 +48682,24 @@
                   },
                   "defaultValue": null
                 },
+                {
+                  "name": "relations",
+                  "description": "Filter members by the given member relations",
+                  "type": {
+                    "kind": "LIST",
+                    "name": null,
+                    "ofType": {
+                      "kind": "NON_NULL",
+                      "name": null,
+                      "ofType": {
+                        "kind": "ENUM",
+                        "name": "ProjectMemberRelation",
+                        "ofType": null
+                      }
+                    }
+                  },
+                  "defaultValue": "[DIRECT, INHERITED]"
+                },
                 {
                   "name": "after",
                   "description": "Returns the elements in the list that come after the specified cursor.",
@@ -50363,6 +50428,41 @@
           "enumValues": null,
           "possibleTypes": null
         },
+        {
+          "kind": "ENUM",
+          "name": "ProjectMemberRelation",
+          "description": "Project member relation",
+          "fields": null,
+          "inputFields": null,
+          "interfaces": null,
+          "enumValues": [
+            {
+              "name": "DIRECT",
+              "description": "Direct members",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "INHERITED",
+              "description": "Inherited members",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "DESCENDANTS",
+              "description": "Descendants members",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "INVITED_GROUPS",
+              "description": "Invited Groups members",
+              "isDeprecated": false,
+              "deprecationReason": null
+            }
+          ],
+          "possibleTypes": null
+        },
         {
           "kind": "OBJECT",
           "name": "ProjectPermissions",

doc/api/graphql/reference/index.md

@@ -4078,6 +4078,16 @@ Epic ID wildcard values.
 | `ANY` | Any epic is assigned |
 | `NONE` | No epic is assigned |
 
+### GroupMemberRelation
+
+Group member relation.
+
+| Value | Description |
+| ----- | ----------- |
+| `DESCENDANTS` | Descendants members |
+| `DIRECT` | Direct members |
+| `INHERITED` | Inherited members |
+
 ### HealthStatus
 
 Health status of an issue or epic.
@@ -4366,6 +4376,17 @@ Values for sorting projects.
 | `SUCCESS` |  |
 | `WAITING_FOR_RESOURCE` |  |
 
+### ProjectMemberRelation
+
+Project member relation.
+
+| Value | Description |
+| ----- | ----------- |
+| `DESCENDANTS` | Descendants members |
+| `DIRECT` | Direct members |
+| `INHERITED` | Inherited members |
+| `INVITED_GROUPS` | Invited Groups members |
+
 ### RegistryState
 
 State of a Geo registry.

lib/api/helpers/members_helpers.rb

@@ -45,7 +45,7 @@ module API
       end
 
       def find_all_members_for_project(project)
-        MembersFinder.new(project, current_user).execute(include_relations: [:inherited, :direct, :invited_groups_members])
+        MembersFinder.new(project, current_user).execute(include_relations: [:inherited, :direct, :invited_groups])
       end
 
       def find_all_members_for_group(group)

spec/finders/members_finder_spec.rb

@@ -160,8 +160,8 @@ RSpec.describe MembersFinder, '#execute' do
     expect(result).to eq([member3, member2, member1])
   end
 
-  context 'when include_invited_groups_members == true' do
-    subject { described_class.new(project, user2).execute(include_relations: [:inherited, :direct, :invited_groups_members]) }
+  context 'when :invited_groups is passed' do
+    subject { described_class.new(project, user2).execute(include_relations: [:inherited, :direct, :invited_groups]) }
 
     let_it_be(:linked_group) { create(:group, :public) }
     let_it_be(:nested_linked_group) { create(:group, parent: linked_group) }

spec/graphql/types/group_member_relation_enum_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+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')
+  end
+end

spec/graphql/types/project_member_relation_enum_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Types::ProjectMemberRelationEnum do
+  specify { expect(described_class.graphql_name).to eq('ProjectMemberRelation') }
+
+  it 'exposes all the existing project member relation type values' do
+    expect(described_class.values.keys).to contain_exactly('DIRECT', 'INHERITED', 'DESCENDANTS', 'INVITED_GROUPS')
+  end
+end

spec/requests/api/graphql/group/group_members_spec.rb

@@ -5,44 +5,95 @@ require 'spec_helper'
 RSpec.describe 'getting group members information' do
   include GraphqlHelpers
 
-  let_it_be(:group) { create(:group, :public) }
+  let_it_be(:parent_group) { create(:group, :public) }
   let_it_be(:user) { create(:user) }
   let_it_be(:user_1) { create(:user, username: 'user') }
   let_it_be(:user_2) { create(:user, username: 'test') }
 
   let(:member_data) { graphql_data['group']['groupMembers']['edges'] }
 
-  before do
-    [user_1, user_2].each { |user| group.add_guest(user) }
+  before_all do
+    [user_1, user_2].each { |user| parent_group.add_guest(user) }
   end
 
   context 'when the request is correct' do
     it_behaves_like 'a working graphql query' do
-      before do
-        fetch_members(user)
+      before_all do
+        fetch_members
       end
     end
 
     it 'returns group members successfully' do
-      fetch_members(user)
+      fetch_members
 
       expect(graphql_errors).to be_nil
-      expect_array_response(user_1.to_global_id.to_s, user_2.to_global_id.to_s)
+      expect_array_response(user_1, user_2)
     end
 
     it 'returns members that match the search query' do
-      fetch_members(user, { search: 'test' })
+      fetch_members(args: { search: 'test' })
 
       expect(graphql_errors).to be_nil
-      expect_array_response(user_2.to_global_id.to_s)
+      expect_array_response(user_2)
     end
   end
 
-  def fetch_members(user = nil, args = {})
-    post_graphql(members_query(args), current_user: user)
+  context 'member relations' do
+    let_it_be(:child_group) { create(:group, :public, parent: parent_group) }
+    let_it_be(:grandchild_group) { create(:group, :public, parent: child_group) }
+    let_it_be(:child_user) { create(:user) }
+    let_it_be(:grandchild_user) { create(:user) }
+
+    before_all do
+      child_group.add_guest(child_user)
+      grandchild_group.add_guest(grandchild_user)
+    end
+
+    it 'returns direct members' do
+      fetch_members(group: child_group, args: { relations: [:DIRECT] })
+
+      expect(graphql_errors).to be_nil
+      expect_array_response(child_user)
+    end
+
+    it 'returns direct and inherited members' do
+      fetch_members(group: child_group, args: { relations: [:DIRECT, :INHERITED] })
+
+      expect(graphql_errors).to be_nil
+      expect_array_response(child_user, user_1, user_2)
+    end
+
+    it 'returns direct, inherited, and descendant members' do
+      fetch_members(group: child_group, args: { relations: [:DIRECT, :INHERITED, :DESCENDANTS] })
+
+      expect(graphql_errors).to be_nil
+      expect_array_response(child_user, user_1, user_2, grandchild_user)
+    end
+
+    it 'returns an error for an invalid member relation' do
+      fetch_members(group: child_group, args: { relations: [:OBLIQUE] })
+
+      expect(graphql_errors.first)
+        .to include('path' => %w[query group groupMembers relations],
+                    'message' => a_string_including('invalid value ([OBLIQUE])'))
+    end
+  end
+
+  context 'when unauthenticated' do
+    it 'returns nothing' do
+      fetch_members(current_user: nil)
+
+      expect(graphql_errors).to be_nil
+      expect(response).to have_gitlab_http_status(:success)
+      expect(member_data).to be_empty
+    end
+  end
+
+  def fetch_members(group: parent_group, current_user: user, args: {})
+    post_graphql(members_query(group.full_path, args), current_user: current_user)
   end
 
-  def members_query(args = {})
+  def members_query(group_path, args = {})
     members_node = <<~NODE
     edges {
       node {
@@ -54,7 +105,7 @@ RSpec.describe 'getting group members information' do
     NODE
 
     graphql_query_for("group",
-      { full_path: group.full_path },
+      { full_path: group_path },
       [query_graphql_field("groupMembers", args, members_node)]
     )
   end
@@ -62,6 +113,7 @@ RSpec.describe 'getting group members information' do
   def expect_array_response(*items)
     expect(response).to have_gitlab_http_status(:success)
     expect(member_data).to be_an Array
-    expect(member_data.map { |node| node["node"]["user"]["id"] }).to match_array(items)
+    expect(member_data.map { |node| node["node"]["user"]["id"] })
+      .to match_array(items.map { |u| global_id_of(u) })
   end
 end

spec/requests/api/graphql/project/project_members_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'getting project members information' do
+  include GraphqlHelpers
+
+  let_it_be(:parent_group) { create(:group, :public) }
+  let_it_be(:parent_project) { create(:project, :public, group: parent_group) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:user_1) { create(:user, username: 'user') }
+  let_it_be(:user_2) { create(:user, username: 'test') }
+
+  let(:member_data) { graphql_data['project']['projectMembers']['edges'] }
+
+  before_all do
+    [user_1, user_2].each { |user| parent_group.add_guest(user) }
+  end
+
+  context 'when the request is correct' do
+    it_behaves_like 'a working graphql query' do
+      before_all do
+        fetch_members(project: parent_project)
+      end
+    end
+
+    it 'returns project members successfully' do
+      fetch_members(project: parent_project)
+
+      expect(graphql_errors).to be_nil
+      expect_array_response(user_1, user_2)
+    end
+
+    it 'returns members that match the search query' do
+      fetch_members(project: parent_project, args: { search: 'test' })
+
+      expect(graphql_errors).to be_nil
+      expect_array_response(user_2)
+    end
+  end
+
+  context 'member relations' do
+    let_it_be(:child_group) { create(:group, :public, parent: parent_group) }
+    let_it_be(:child_project) { create(:project, :public, group: child_group) }
+    let_it_be(:invited_group) { create(:group, :public) }
+    let_it_be(:child_user) { create(:user) }
+    let_it_be(:invited_user) { create(:user) }
+    let_it_be(:group_link) { create(:project_group_link, project: child_project, group: invited_group) }
+
+    before_all do
+      child_project.add_guest(child_user)
+      invited_group.add_guest(invited_user)
+    end
+
+    it 'returns direct members' do
+      fetch_members(project: child_project, args: { relations: [:DIRECT] })
+
+      expect(graphql_errors).to be_nil
+      expect_array_response(child_user)
+    end
+
+    it 'returns invited members plus inherited members' do
+      fetch_members(project: child_project, args: { relations: [:INVITED_GROUPS] })
+
+      expect(graphql_errors).to be_nil
+      expect_array_response(invited_user, user_1, user_2)
+    end
+
+    it 'returns direct, inherited, descendant, and invited members' do
+      fetch_members(project: child_project, args: { relations: [:DIRECT, :INHERITED, :DESCENDANTS, :INVITED_GROUPS] })
+
+      expect(graphql_errors).to be_nil
+      expect_array_response(child_user, user_1, user_2, invited_user)
+    end
+
+    it 'returns an error for an invalid member relation' do
+      fetch_members(project: child_project, args: { relations: [:OBLIQUE] })
+
+      expect(graphql_errors.first)
+        .to include('path' => %w[query project projectMembers relations],
+                    'message' => a_string_including('invalid value ([OBLIQUE])'))
+    end
+  end
+
+  context 'when unauthenticated' do
+    it 'returns members' do
+      fetch_members(current_user: nil, project: parent_project)
+
+      expect(graphql_errors).to be_nil
+      expect_array_response(user_1, user_2)
+    end
+  end
+
+  def fetch_members(project:, current_user: user, args: {})
+    post_graphql(members_query(project.full_path, args), current_user: current_user)
+  end
+
+  def members_query(group_path, args = {})
+    members_node = <<~NODE
+    edges {
+      node {
+        user {
+          id
+        }
+      }
+    }
+    NODE
+
+    graphql_query_for('project',
+      { full_path: group_path },
+      [query_graphql_field('projectMembers', args, members_node)]
+    )
+  end
+
+  def expect_array_response(*items)
+    expect(response).to have_gitlab_http_status(:success)
+    expect(member_data).to be_an Array
+    expect(member_data.map { |node| node['node']['user']['id'] })
+      .to match_array(items.map { |u| global_id_of(u) })
+  end
+end

spec/support/shared_examples/graphql/members_shared_examples.rb

@@ -36,9 +36,10 @@ RSpec.shared_examples 'querying members with a group' do
   let_it_be(:group_2_member)    { create(:group_member, user: user_3, group: group_2) }
 
   let(:args) { {} }
+  let(:base_args) { { relations: described_class.arguments['relations'].default_value } }
 
   subject do
-    resolve(described_class, obj: resource, args: args, ctx: { current_user: user_4 })
+    resolve(described_class, obj: resource, args: base_args.merge(args), ctx: { current_user: user_4 })
   end
 
   describe '#resolve' do
@@ -72,7 +73,7 @@ RSpec.shared_examples 'querying members with a group' do
       let_it_be(:other_user) { create(:user) }
 
       subject do
-        resolve(described_class, obj: resource, args: args, ctx: { current_user: other_user })
+        resolve(described_class, obj: resource, args: base_args.merge(args), ctx: { current_user: other_user })
       end
 
       it 'raises an error' do