Merge branch 'ajk-gql-reviewer-merge-requests' into 'master'

Add support for filtering MRs by review in GraphQL

See merge request gitlab-org/gitlab!49464

app/graphql/resolvers/assigned_merge_requests_resolver.rb

@@ -4,6 +4,7 @@ module Resolvers
   class AssignedMergeRequestsResolver < UserMergeRequestsResolverBase
     type ::Types::MergeRequestType.connection_type, null: true
     accept_author
+    accept_reviewer
 
     def user_role
       :assignee

app/graphql/resolvers/authored_merge_requests_resolver.rb

@@ -4,6 +4,7 @@ module Resolvers
   class AuthoredMergeRequestsResolver < UserMergeRequestsResolverBase
     type ::Types::MergeRequestType.connection_type, null: true
     accept_assignee
+    accept_reviewer
 
     def user_role
       :author

app/graphql/resolvers/merge_requests_resolver.rb

@@ -18,6 +18,12 @@ module Resolvers
              description: 'Username of the author'
     end
 
+    def self.accept_reviewer
+      argument :reviewer_username, GraphQL::STRING_TYPE,
+             required: false,
+             description: 'Username of the reviewer'
+    end
+
     argument :iids, [GraphQL::STRING_TYPE],
               required: false,
               description: 'Array of IIDs of merge requests, for example `[1, 2]`'

app/graphql/resolvers/project_merge_requests_resolver.rb

@@ -5,5 +5,6 @@ module Resolvers
     type ::Types::MergeRequestType.connection_type, null: true
     accept_assignee
     accept_author
+    accept_reviewer
   end
 end

app/graphql/resolvers/review_requested_merge_requests_resolver.rb

+# frozen_string_literal: true
+
+module Resolvers
+  class ReviewRequestedMergeRequestsResolver < UserMergeRequestsResolverBase
+    type ::Types::MergeRequestType.connection_type, null: true
+    accept_author
+    accept_assignee
+
+    def user_role
+      :reviewer
+    end
+  end
+end

app/graphql/types/user_type.rb

@@ -48,13 +48,16 @@ module Types
           description: 'Projects starred by the user',
           resolver: Resolvers::UserStarredProjectsResolver
 
-    # Merge request field: MRs can be either authored or assigned:
+    # Merge request field: MRs can be authored, assigned, or assigned-for-review:
     field :authored_merge_requests,
           resolver: Resolvers::AuthoredMergeRequestsResolver,
           description: 'Merge Requests authored by the user'
     field :assigned_merge_requests,
           resolver: Resolvers::AssignedMergeRequestsResolver,
           description: 'Merge Requests assigned to the user'
+    field :review_requested_merge_requests,
+          resolver: Resolvers::ReviewRequestedMergeRequestsResolver,
+          description: 'Merge Requests assigned to the user for review'
 
     field :snippets,
           Types::SnippetType.connection_type,

changelogs/unreleased/ajk-gql-reviewer-merge-requests.yml

+---
+title: Support merge requests filtered by reviewer in GraphQL API
+merge_request: 49464
+author:
+type: changed

doc/api/graphql/reference/gitlab_schema.graphql

@@ -17052,6 +17052,11 @@ type Project {
     """
     milestoneTitle: String
 
+    """
+    Username of the reviewer
+    """
+    reviewerUsername: String
+
     """
     Sort merge requests by this criteria
     """
@@ -24018,6 +24023,11 @@ type User {
     """
     projectPath: String
 
+    """
+    Username of the reviewer
+    """
+    reviewerUsername: String
+
     """
     Sort merge requests by this criteria
     """
@@ -24103,6 +24113,11 @@ type User {
     """
     projectPath: String
 
+    """
+    Username of the reviewer
+    """
+    reviewerUsername: String
+
     """
     Sort merge requests by this criteria
     """
@@ -24209,6 +24224,96 @@ type User {
   """
   publicEmail: String
 
+  """
+  Merge Requests assigned to the user for review
+  """
+  reviewRequestedMergeRequests(
+    """
+    Returns the elements in the list that come after the specified cursor.
+    """
+    after: String
+
+    """
+    Username of the assignee
+    """
+    assigneeUsername: String
+
+    """
+    Username of the author
+    """
+    authorUsername: String
+
+    """
+    Returns the elements in the list that come before the specified cursor.
+    """
+    before: String
+
+    """
+    Returns the first _n_ elements from the list.
+    """
+    first: Int
+
+    """
+    Array of IIDs of merge requests, for example `[1, 2]`
+    """
+    iids: [String!]
+
+    """
+    Array of label names. All resolved merge requests will have all of these labels.
+    """
+    labels: [String!]
+
+    """
+    Returns the last _n_ elements from the list.
+    """
+    last: Int
+
+    """
+    Merge requests merged after this date
+    """
+    mergedAfter: Time
+
+    """
+    Merge requests merged before this date
+    """
+    mergedBefore: Time
+
+    """
+    Title of the milestone
+    """
+    milestoneTitle: String
+
+    """
+    The global ID of the project the authored merge requests should be in. Incompatible with projectPath.
+    """
+    projectId: ProjectID
+
+    """
+    The full-path of the project the authored merge requests should be in. Incompatible with projectId.
+    """
+    projectPath: String
+
+    """
+    Sort merge requests by this criteria
+    """
+    sort: MergeRequestSort = created_desc
+
+    """
+    Array of source branch names. All resolved merge requests will have one of these branches as their source.
+    """
+    sourceBranches: [String!]
+
+    """
+    A merge request state. If provided, all resolved merge requests will have this state.
+    """
+    state: MergeRequestState
+
+    """
+    Array of target branch names. All resolved merge requests will have one of these branches as their target.
+    """
+    targetBranches: [String!]
+  ): MergeRequestConnection
+
   """
   Snippets authored by the user
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -50196,6 +50196,16 @@
                   },
                   "defaultValue": null
                 },
+                {
+                  "name": "reviewerUsername",
+                  "description": "Username of the reviewer",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
                 {
                   "name": "after",
                   "description": "Returns the elements in the list that come after the specified cursor.",
@@ -70064,6 +70074,16 @@
                   },
                   "defaultValue": null
                 },
+                {
+                  "name": "reviewerUsername",
+                  "description": "Username of the reviewer",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
                 {
                   "name": "after",
                   "description": "Returns the elements in the list that come after the specified cursor.",
@@ -70269,6 +70289,16 @@
                   },
                   "defaultValue": null
                 },
+                {
+                  "name": "reviewerUsername",
+                  "description": "Username of the reviewer",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
                 {
                   "name": "after",
                   "description": "Returns the elements in the list that come after the specified cursor.",
@@ -70530,6 +70560,221 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "reviewRequestedMergeRequests",
+              "description": "Merge Requests assigned to the user for review",
+              "args": [
+                {
+                  "name": "iids",
+                  "description": "Array of IIDs of merge requests, for example `[1, 2]`",
+                  "type": {
+                    "kind": "LIST",
+                    "name": null,
+                    "ofType": {
+                      "kind": "NON_NULL",
+                      "name": null,
+                      "ofType": {
+                        "kind": "SCALAR",
+                        "name": "String",
+                        "ofType": null
+                      }
+                    }
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "sourceBranches",
+                  "description": "Array of source branch names. All resolved merge requests will have one of these branches as their source.",
+                  "type": {
+                    "kind": "LIST",
+                    "name": null,
+                    "ofType": {
+                      "kind": "NON_NULL",
+                      "name": null,
+                      "ofType": {
+                        "kind": "SCALAR",
+                        "name": "String",
+                        "ofType": null
+                      }
+                    }
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "targetBranches",
+                  "description": "Array of target branch names. All resolved merge requests will have one of these branches as their target.",
+                  "type": {
+                    "kind": "LIST",
+                    "name": null,
+                    "ofType": {
+                      "kind": "NON_NULL",
+                      "name": null,
+                      "ofType": {
+                        "kind": "SCALAR",
+                        "name": "String",
+                        "ofType": null
+                      }
+                    }
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "state",
+                  "description": "A merge request state. If provided, all resolved merge requests will have this state.",
+                  "type": {
+                    "kind": "ENUM",
+                    "name": "MergeRequestState",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "labels",
+                  "description": "Array of label names. All resolved merge requests will have all of these labels.",
+                  "type": {
+                    "kind": "LIST",
+                    "name": null,
+                    "ofType": {
+                      "kind": "NON_NULL",
+                      "name": null,
+                      "ofType": {
+                        "kind": "SCALAR",
+                        "name": "String",
+                        "ofType": null
+                      }
+                    }
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "mergedAfter",
+                  "description": "Merge requests merged after this date",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "Time",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "mergedBefore",
+                  "description": "Merge requests merged before this date",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "Time",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "milestoneTitle",
+                  "description": "Title of the milestone",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "sort",
+                  "description": "Sort merge requests by this criteria",
+                  "type": {
+                    "kind": "ENUM",
+                    "name": "MergeRequestSort",
+                    "ofType": null
+                  },
+                  "defaultValue": "created_desc"
+                },
+                {
+                  "name": "projectPath",
+                  "description": "The full-path of the project the authored merge requests should be in. Incompatible with projectId.",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "projectId",
+                  "description": "The global ID of the project the authored merge requests should be in. Incompatible with projectPath.",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "ProjectID",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "authorUsername",
+                  "description": "Username of the author",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "assigneeUsername",
+                  "description": "Username of the assignee",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "after",
+                  "description": "Returns the elements in the list that come after the specified cursor.",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "before",
+                  "description": "Returns the elements in the list that come before the specified cursor.",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "first",
+                  "description": "Returns the first _n_ elements from the list.",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "Int",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
+                {
+                  "name": "last",
+                  "description": "Returns the last _n_ elements from the list.",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "Int",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                }
+              ],
+              "type": {
+                "kind": "OBJECT",
+                "name": "MergeRequestConnection",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "snippets",
               "description": "Snippets authored by the user",

doc/api/graphql/reference/index.md

@@ -3660,6 +3660,7 @@ Autogenerated return type of UpdateSnippet.
 | `name` | String! | Human-readable name of the user |
 | `projectMemberships` | ProjectMemberConnection | Project memberships of the user |
 | `publicEmail` | String | User's public email |
+| `reviewRequestedMergeRequests` | MergeRequestConnection | Merge Requests assigned to the user for review |
 | `snippets` | SnippetConnection | Snippets authored by the user |
 | `starredProjects` | ProjectConnection | Projects starred by the user |
 | `state` | UserState! | State of the user |

spec/graphql/resolvers/project_merge_requests_resolver_spec.rb

@@ -8,14 +8,16 @@ RSpec.describe Resolvers::ProjectMergeRequestsResolver do
   let_it_be(:project) { create(:project, :repository) }
   let_it_be(:current_user) { create(:user) }
   let_it_be(:other_user) { create(:user) }
+  let_it_be(:reviewer) { create(:user) }
 
-  let_it_be(:merge_request_with_author_and_assignee) do
+  let_it_be(:merge_request) do
     create(:merge_request,
            :unique_branches,
            source_project: project,
            target_project: project,
            author: other_user,
-           assignee: other_user)
+           assignee: other_user,
+           reviewers: [reviewer])
   end
 
   before do
@@ -26,7 +28,7 @@ RSpec.describe Resolvers::ProjectMergeRequestsResolver do
     it 'filters merge requests by assignee username' do
       result = resolve_mr(project, assignee_username: other_user.username)
 
-      expect(result).to eq([merge_request_with_author_and_assignee])
+      expect(result).to eq([merge_request])
     end
 
     it 'does not find anything' do
@@ -40,7 +42,7 @@ RSpec.describe Resolvers::ProjectMergeRequestsResolver do
     it 'filters merge requests by author username' do
       result = resolve_mr(project, author_username: other_user.username)
 
-      expect(result).to eq([merge_request_with_author_and_assignee])
+      expect(result).to eq([merge_request])
     end
 
     it 'does not find anything' do
@@ -50,6 +52,20 @@ RSpec.describe Resolvers::ProjectMergeRequestsResolver do
     end
   end
 
+  context 'by reviewer' do
+    it 'filters merge requests by reviewer username' do
+      result = resolve_mr(project, reviewer_username: reviewer.username)
+
+      expect(result).to eq([merge_request])
+    end
+
+    it 'does not find anything' do
+      result = resolve_mr(project, reviewer_username: 'unknown-user')
+
+      expect(result).to be_empty
+    end
+  end
+
   def resolve_mr(project, resolver: described_class, user: current_user, **args)
     resolve(resolver, obj: project, args: args, ctx: { current_user: user })
   end

spec/graphql/types/project_type_spec.rb

@@ -79,6 +79,7 @@ RSpec.describe GitlabSchema.types['Project'] do
                                             :merged_before,
                                             :author_username,
                                             :assignee_username,
+                                            :reviewer_username,
                                             :milestone_title,
                                             :sort
                                            )

spec/graphql/types/user_type_spec.rb

@@ -25,6 +25,7 @@ RSpec.describe GitlabSchema.types['User'] do
       location
       authoredMergeRequests
       assignedMergeRequests
+      reviewRequestedMergeRequests
       groupMemberships
       groupCount
       projectMemberships

spec/requests/api/graphql/user_query_spec.rb

@@ -58,9 +58,25 @@ RSpec.describe 'getting user information' do
              source_project: project_b, author: user)
     end
 
+    let_it_be(:reviewed_mr) do
+      create(:merge_request, :unique_branches, :unique_author,
+             source_project: project_a, reviewers: [user])
+    end
+
+    let_it_be(:reviewed_mr_b) do
+      create(:merge_request, :unique_branches, :unique_author,
+             source_project: project_b, reviewers: [user])
+    end
+
+    let_it_be(:reviewed_mr_c) do
+      create(:merge_request, :unique_branches, :unique_author,
+             source_project: project_b, reviewers: [user])
+    end
+
     let(:current_user) { authorised_user }
     let(:authored_mrs) { graphql_data_at(:user, :authored_merge_requests, :nodes) }
     let(:assigned_mrs) { graphql_data_at(:user, :assigned_merge_requests, :nodes) }
+    let(:reviewed_mrs) { graphql_data_at(:user, :review_requested_merge_requests, :nodes) }
     let(:user_params) { { username: user.username } }
 
     before do
@@ -157,6 +173,23 @@ RSpec.describe 'getting user information' do
               )
             end
           end
+
+          context 'filtering by reviewer' do
+            let(:reviewer) { create(:user) }
+            let(:mr_args) { { reviewer_username: reviewer.username } }
+
+            it 'finds the assigned mrs' do
+              assigned_mr_b.reviewers << reviewer
+              assigned_mr_c.reviewers << reviewer
+
+              post_graphql(query, current_user: current_user)
+
+              expect(assigned_mrs).to contain_exactly(
+                a_hash_including('id' => global_id_of(assigned_mr_b)),
+                a_hash_including('id' => global_id_of(assigned_mr_c))
+              )
+            end
+          end
         end
 
         context 'the current user does not have access' do
@@ -168,6 +201,95 @@ RSpec.describe 'getting user information' do
         end
       end
 
+      describe 'reviewRequestedMergeRequests' do
+        let(:user_fields) do
+          query_graphql_field(:review_requested_merge_requests, mr_args, 'nodes { id }')
+        end
+
+        let(:mr_args) { nil }
+
+        it_behaves_like 'a working graphql query'
+
+        it 'can be found' do
+          expect(reviewed_mrs).to contain_exactly(
+            a_hash_including('id' => global_id_of(reviewed_mr)),
+            a_hash_including('id' => global_id_of(reviewed_mr_b)),
+            a_hash_including('id' => global_id_of(reviewed_mr_c))
+          )
+        end
+
+        context 'applying filters' do
+          context 'filtering by IID without specifying a project' do
+            let(:mr_args) do
+              { iids: [reviewed_mr_b.iid.to_s] }
+            end
+
+            it 'return an argument error that mentions the missing fields' do
+              expect_graphql_errors_to_include(/projectPath/)
+            end
+          end
+
+          context 'filtering by project path and IID' do
+            let(:mr_args) do
+              { project_path: project_b.full_path, iids: [reviewed_mr_b.iid.to_s] }
+            end
+
+            it 'selects the correct MRs' do
+              expect(reviewed_mrs).to contain_exactly(
+                a_hash_including('id' => global_id_of(reviewed_mr_b))
+              )
+            end
+          end
+
+          context 'filtering by project path' do
+            let(:mr_args) do
+              { project_path: project_b.full_path }
+            end
+
+            it 'selects the correct MRs' do
+              expect(reviewed_mrs).to contain_exactly(
+                a_hash_including('id' => global_id_of(reviewed_mr_b)),
+                a_hash_including('id' => global_id_of(reviewed_mr_c))
+              )
+            end
+          end
+
+          context 'filtering by author' do
+            let(:author) { reviewed_mr_b.author }
+            let(:mr_args) { { author_username: author.username } }
+
+            it 'finds the authored mrs' do
+              expect(reviewed_mrs).to contain_exactly(
+                a_hash_including('id' => global_id_of(reviewed_mr_b))
+              )
+            end
+          end
+
+          context 'filtering by assignee' do
+            let(:assignee) { create(:user) }
+            let(:mr_args) { { assignee_username: assignee.username } }
+
+            it 'finds the authored mrs' do
+              reviewed_mr_c.assignees << assignee
+
+              post_graphql(query, current_user: current_user)
+
+              expect(reviewed_mrs).to contain_exactly(
+                a_hash_including('id' => global_id_of(reviewed_mr_c))
+              )
+            end
+          end
+        end
+
+        context 'the current user does not have access' do
+          let(:current_user) { unauthorized_user }
+
+          it 'cannot be found' do
+            expect(reviewed_mrs).to be_empty
+          end
+        end
+      end
+
       describe 'authoredMergeRequests' do
         let(:user_fields) do
           query_graphql_field(:authored_merge_requests, mr_args, 'nodes { id }')
@@ -213,6 +335,23 @@ RSpec.describe 'getting user information' do
             end
           end
 
+          context 'filtering by reviewer' do
+            let(:reviewer) { create(:user) }
+            let(:mr_args) { { reviewer_username: reviewer.username } }
+
+            it 'finds the assigned mrs' do
+              authored_mr_b.reviewers << reviewer
+              authored_mr_c.reviewers << reviewer
+
+              post_graphql(query, current_user: current_user)
+
+              expect(authored_mrs).to contain_exactly(
+                a_hash_including('id' => global_id_of(authored_mr_b)),
+                a_hash_including('id' => global_id_of(authored_mr_c))
+              )
+            end
+          end
+
           context 'filtering by project path and IID' do
             let(:mr_args) do
               { project_path: project_b.full_path, iids: [authored_mr_b.iid.to_s] }