Merge branch 'filter-by-review-merge-request-list-api' into 'master'

Filter by reviewer in MergeRequest List API

See merge request gitlab-org/gitlab!49341

lib/api/helpers/merge_requests_helpers.rb

@@ -21,6 +21,9 @@ module API
                  coerce_with: Validations::Validators::CheckAssigneesCount.coerce,
                  desc: 'Return merge requests which are assigned to the user with the given username'
         mutually_exclusive :assignee_id, :assignee_username
+        optional :reviewer_username,
+                 type: String,
+                 desc: 'Return merge requests which have the user as a reviewer with the given username'
 
         optional :labels,
                  type: Array[String],
@@ -32,6 +35,11 @@ module API
 
       params :merge_requests_base_params do
         use :merge_requests_negatable_params
+        optional :reviewer_id,
+                 types: [Integer, String],
+                 integer_none_any: true,
+                 desc: 'Return merge requests which have the user as a reviewer with the given ID'
+        mutually_exclusive :reviewer_id, :reviewer_username
         optional :state,
                  type: String,
                  values: %w[opened closed locked merged all],
@@ -72,6 +80,10 @@ module API
         optional :wip, type: String, values: %w[yes no], desc: 'Search merge requests for WIP in the title'
         optional :not, type: Hash, desc: 'Parameters to negate' do
           use :merge_requests_negatable_params
+          optional :reviewer_id,
+            types: Integer,
+            desc: 'Return merge requests which have the user as a reviewer with the given ID'
+          mutually_exclusive :reviewer_id, :reviewer_username
         end
 
         optional :deployed_before,

spec/requests/api/merge_requests_spec.rb

@@ -440,6 +440,7 @@ RSpec.describe API::MergeRequests do
             milestone: milestone,
             author: user,
             assignees: [user],
+            reviewers: [user2],
             source_project: project,
             target_project: project,
             source_branch: 'what',
@@ -498,6 +499,71 @@ RSpec.describe API::MergeRequests do
             expect(mr['assignee']['id']).not_to eq(user2.id)
           end
         end
+
+        context 'filter by reviewer' do
+          context 'with reviewer_id' do
+            context 'with an id' do
+              let(:params) { { not: { reviewer_id: user2.id } } }
+
+              it 'returns merge requests that do not have the given reviewer' do
+                get api(endpoint_path, user), params: { not: { reviewer_id: user2.id } }
+
+                expect(response).to have_gitlab_http_status(:ok)
+                expect(json_response).to be_an(Array)
+                expect(json_response.length).to eq(4)
+                expect(json_response.map { |mr| mr['id'] }).not_to include(merge_request2)
+              end
+            end
+
+            context 'with Any' do
+              let(:params) { { not: { reviewer_id: 'Any' } } }
+
+              it 'returns a 400' do
+                # Any is not supported for negated filter
+                get api(endpoint_path, user), params: params
+
+                expect(response).to have_gitlab_http_status(:bad_request)
+                expect(json_response['error']).to eq('not[reviewer_id] is invalid')
+              end
+            end
+
+            context 'with None' do
+              let(:params) { { not: { reviewer_id: 'None' } } }
+
+              it 'returns a 400' do
+                # None is not supported for negated filter
+                get api(endpoint_path, user), params: params
+
+                expect(response).to have_gitlab_http_status(:bad_request)
+                expect(json_response['error']).to eq('not[reviewer_id] is invalid')
+              end
+            end
+          end
+
+          context 'with reviewer_username' do
+            let(:params) { { not: { reviewer_username: user2.username } } }
+
+            it 'returns merge requests that do not have the given reviewer' do
+              get api(endpoint_path, user), params: params
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(json_response).to be_an(Array)
+              expect(json_response.length).to eq(4)
+              expect(json_response.map { |mr| mr['id'] }).not_to include(merge_request2)
+            end
+          end
+
+          context 'when both reviewer_id and reviewer_username' do
+            let(:params) { { not: { reviewer_id: user2.id, reviewer_username: user2.username } } }
+
+            it 'returns a 400' do
+              get api('/merge_requests', user), params: params
+
+              expect(response).to have_gitlab_http_status(:bad_request)
+              expect(json_response['error']).to eq('not[reviewer_id], not[reviewer_username] are mutually exclusive')
+            end
+          end
+        end
       end
 
       context 'source_branch param' do
@@ -666,6 +732,79 @@ RSpec.describe API::MergeRequests do
         end
       end
 
+      context 'filter by reviewer' do
+        let_it_be(:review_requested_mr1) do
+          create(:merge_request, :unique_branches, author: user, reviewers: [user2], source_project: project2, target_project: project2)
+        end
+
+        let_it_be(:review_requested_mr2) do
+          create(:merge_request, :unique_branches, author: user2, reviewers: [user], source_project: project2, target_project: project2)
+        end
+
+        let(:params) { { scope: :all } }
+
+        context 'with reviewer_id' do
+          let(:params) { super().merge(reviewer_id: reviewer_id) }
+
+          context 'with an id' do
+            let(:reviewer_id) { user2.id }
+
+            it 'returns review requested merge requests for the given user' do
+              get api('/merge_requests', user), params: params
+
+              expect_response_contain_exactly(review_requested_mr1.id)
+            end
+          end
+
+          context 'with Any' do
+            let(:reviewer_id) { 'Any' }
+
+            it 'returns review requested merge requests for any user' do
+              get api('/merge_requests', user), params: params
+
+              expect_response_contain_exactly(review_requested_mr1.id, review_requested_mr2.id)
+            end
+          end
+
+          context 'with None' do
+            let(:reviewer_id) { 'None' }
+
+            it 'returns merge requests that has no assigned reviewers' do
+              get api('/merge_requests', user), params: params
+
+              expect_response_contain_exactly(
+                merge_request.id,
+                merge_request_closed.id,
+                merge_request_merged.id,
+                merge_request_locked.id,
+                merge_request2.id
+              )
+            end
+          end
+        end
+
+        context 'with reviewer_username' do
+          let(:params) { super().merge(reviewer_username: user2.username) }
+
+          it 'returns review requested merge requests for the given user' do
+            get api('/merge_requests', user), params: params
+
+            expect_response_contain_exactly(review_requested_mr1.id)
+          end
+        end
+
+        context 'with both reviewer_id and reviewer_username' do
+          let(:params) { super().merge(reviewer_id: user2.id, reviewer_username: user2.username) }
+
+          it 'returns a 400' do
+            get api('/merge_requests', user), params: params
+
+            expect(response).to have_gitlab_http_status(:bad_request)
+            expect(json_response['error']).to eq('reviewer_id, reviewer_username are mutually exclusive')
+          end
+        end
+      end
+
       it 'returns an array of merge requests assigned to the given user' do
         merge_request3 = create(:merge_request, :simple, author: user, assignees: [user2], source_project: project2, target_project: project2, source_branch: 'other-branch')