Add NOT param support to Merge Requests API

We added NOT functionality to the IssuableFinder and now expose that
through the Web UI. This MR adds the same negation functionality to the
Merge Request API.

changelogs/unreleased/31099-MR-API-allow-NOT-params.yml

+---
+title: Add 'not' params to MergeRequests API endpoint
+merge_request: 35391
+author:
+type: added

doc/api/issues.md

@@ -70,7 +70,7 @@ GET /issues?confidential=true
 | `updated_after`     | datetime         | no         | Return issues updated on or after the given time                                                                                                    |
 | `updated_before`    | datetime         | no         | Return issues updated on or before the given time                                                                                                   |
 | `confidential`      | boolean          | no         | Filter confidential or public issues.                                                                                                               |
-| `not`               | Hash             | no         | Return issues that do not match the parameters supplied. Accepts: `labels`, `milestone`, `author_id`, `author_username`, `assignee_id`, `assignee_username`, `my_reaction_emoji`, `search`, `in` |
+| `not`               | Hash             | no         | Return issues that do not match the parameters supplied. Accepts: `labels`, `milestone`, `author_id`, `author_username`, `assignee_id`, `assignee_username`, `my_reaction_emoji` |
 | `non_archived`      | boolean          | no         | Return issues only from non-archived projects. If `false`, response will return issues from both archived and non-archived projects. Default is `true`. _(Introduced in [GitLab 13.0](https://gitlab.com/gitlab-org/gitlab/-/issues/197170))_ |
 
 ```shell

doc/api/merge_requests.md

@@ -64,6 +64,7 @@ Parameters:
 | `search`                        | string         | no       | Search merge requests against their `title` and `description`                                                          |
 | `in`                            | string         | no       | Modify the scope of the `search` attribute. `title`, `description`, or a string joining them with comma. Default is `title,description` |
 | `wip`                           | string         | no       | Filter merge requests against their `wip` status. `yes` to return *only* WIP merge requests, `no` to return *non* WIP merge requests |
+| `not`                           | Hash           | no       | Return merge requests that do not match the parameters supplied. Accepts: `labels`, `milestone`, `author_id`, `author_username`, `assignee_id`, `assignee_username`, `my_reaction_emoji` |
 
 NOTE: **Note:**
 [Starting in GitLab 13.0](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/31890),

lib/api/helpers/merge_requests_helpers.rb

@@ -5,7 +5,30 @@ module API
     module MergeRequestsHelpers
       extend Grape::API::Helpers
 
+      params :merge_requests_negatable_params do
+        optional :author_id, type: Integer, desc: 'Return merge requests which are authored by the user with the given ID'
+        optional :author_username, type: String, desc: 'Return merge requests which are authored by the user with the given username'
+        mutually_exclusive :author_id, :author_username
+
+        optional :assignee_id,
+                 types: [Integer, String],
+                 integer_none_any: true,
+                 desc: 'Return merge requests which are assigned to the user with the given ID'
+        optional :assignee_username, type: Array[String], check_assignees_count: true,
+                 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 :labels,
+                 type: Array[String],
+                 coerce_with: Validations::Types::CommaSeparatedToArray.coerce,
+                 desc: 'Comma-separated list of label names'
+        optional :milestone, type: String, desc: 'Return merge requests for a specific milestone'
+        optional :my_reaction_emoji, type: String, desc: 'Return issues reacted by the authenticated user by the given emoji'
+      end
+
       params :merge_requests_base_params do
+        use :merge_requests_negatable_params
         optional :state,
                  type: String,
                  values: %w[opened closed locked merged all],
@@ -21,11 +44,6 @@ module API
                  values: %w[asc desc],
                  default: 'desc',
                  desc: 'Return merge requests sorted in `asc` or `desc` order.'
-        optional :milestone, type: String, desc: 'Return merge requests for a specific milestone'
-        optional :labels,
-                 type: Array[String],
-                 coerce_with: Validations::Types::CommaSeparatedToArray.coerce,
-                 desc: 'Comma-separated list of label names'
         optional :with_labels_details, type: Boolean, desc: 'Return titles of labels and other details', default: false
         optional :with_merge_status_recheck, type: Boolean, desc: 'Request that stale merge statuses be rechecked asynchronously', default: false
         optional :created_after, type: DateTime, desc: 'Return merge requests created after the specified time'
@@ -37,19 +55,10 @@ module API
                  values: %w[simple],
                  desc: 'If simple, returns the `iid`, URL, title, description, and basic state of merge request'
 
-        optional :author_id, type: Integer, desc: 'Return merge requests which are authored by the user with the given ID'
-        optional :author_username, type: String, desc: 'Return merge requests which are authored by the user with the given username'
-        mutually_exclusive :author_id, :author_username
-
-        optional :assignee_id,
-                 types: [Integer, String],
-                 integer_none_any: true,
-                 desc: 'Return merge requests which are assigned to the user with the given ID'
         optional :scope,
                  type: String,
                  values: %w[created-by-me assigned-to-me created_by_me assigned_to_me all],
                  desc: 'Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`'
-        optional :my_reaction_emoji, type: String, desc: 'Return issues reacted by the authenticated user by the given emoji'
         optional :source_branch, type: String, desc: 'Return merge requests with the given source branch'
         optional :source_project_id, type: Integer, desc: 'Return merge requests with the given source project id'
         optional :target_branch, type: String, desc: 'Return merge requests with the given target branch'
@@ -58,6 +67,9 @@ module API
                  desc: 'Search merge requests for text present in the title, description, or any combination of these'
         optional :in, type: String, desc: '`title`, `description`, or a string joining them with comma'
         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
+        end
       end
 
       params :optional_scope_param do

lib/api/merge_requests.rb

@@ -44,7 +44,9 @@ module API
       def find_merge_requests(args = {})
         args = declared_params.merge(args)
         args[:milestone_title] = args.delete(:milestone)
+        args[:not][:milestone_title] = args[:not]&.delete(:milestone)
         args[:label_name] = args.delete(:labels)
+        args[:not][:label_name] = args[:not]&.delete(:labels)
         args[:scope] = args[:scope].underscore if args[:scope]
 
         merge_requests = MergeRequestsFinder.new(current_user, args).execute

spec/finders/merge_requests_finder_spec.rb

@@ -53,6 +53,21 @@ RSpec.describe MergeRequestsFinder do
         expect(merge_requests).to be_empty
       end
 
+      context 'filtering by not author ID' do
+        let(:params) { { not: { author_id: user2.id } } }
+
+        before do
+          merge_request2.update!(author: user2)
+          merge_request3.update!(author: user2)
+        end
+
+        it 'returns merge requests not created by that user' do
+          merge_requests = described_class.new(user, params).execute
+
+          expect(merge_requests).to contain_exactly(merge_request1, merge_request4, merge_request5)
+        end
+      end
+
       it 'filters by projects' do
         params = { projects: [project2.id, project3.id] }
 
@@ -258,6 +273,11 @@ RSpec.describe MergeRequestsFinder do
           let(:expected_issuables) { [merge_request1, merge_request2] }
         end
 
+        it_behaves_like 'assignee NOT ID filter' do
+          let(:params) { { not: { assignee_id: user.id } } }
+          let(:expected_issuables) { [merge_request3, merge_request4, merge_request5] }
+        end
+
         it_behaves_like 'assignee username filter' do
           before do
             project2.add_developer(user3)
@@ -269,6 +289,15 @@ RSpec.describe MergeRequestsFinder do
           let(:expected_issuables) { [merge_request3] }
         end
 
+        it_behaves_like 'assignee NOT username filter' do
+          before do
+            merge_request2.assignees = [user2]
+          end
+
+          let(:params) { { not: { assignee_username: [user.username, user2.username] } } }
+          let(:expected_issuables) { [merge_request4, merge_request5] }
+        end
+
         it_behaves_like 'no assignee filter' do
           let_it_be(:user3) { create(:user) }
           let(:expected_issuables) { [merge_request4, merge_request5] }
@@ -294,6 +323,16 @@ RSpec.describe MergeRequestsFinder do
 
             expect(merge_requests).to contain_exactly(merge_request2, merge_request3)
           end
+
+          context 'using NOT' do
+            let(:params) { { not: { milestone_title: group_milestone.title } } }
+
+            it 'returns MRs not assigned to that group milestone' do
+              merge_requests = described_class.new(user, params).execute
+
+              expect(merge_requests).to contain_exactly(merge_request1, merge_request4, merge_request5)
+            end
+          end
         end
       end
 

spec/requests/api/merge_requests_spec.rb

@@ -425,6 +425,73 @@ RSpec.describe API::MergeRequests do
         end
       end
 
+      context 'NOT params' do
+        let(:merge_request2) do
+          create(
+            :merge_request,
+            :simple,
+            milestone: milestone,
+            author: user,
+            assignees: [user],
+            merge_request_context_commits: [merge_request_context_commit],
+            source_project: project,
+            target_project: project,
+            source_branch: 'what',
+            title: "What",
+            created_at: base_time
+          )
+        end
+
+        before do
+          create(:label_link, label: label, target: merge_request)
+          create(:label_link, label: label2, target: merge_request2)
+        end
+
+        it 'returns merge requests without any of the labels given', :aggregate_failures do
+          get api(endpoint_path, user), params: { not: { labels: ["#{label.title}, #{label2.title}"] } }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response).to be_an(Array)
+          expect(json_response.length).to eq(3)
+          json_response.each do |mr|
+            expect(mr['labels']).not_to include(label2.title, label.title)
+          end
+        end
+
+        it 'returns merge requests without any of the milestones given', :aggregate_failures do
+          get api(endpoint_path, user), params: { not: { milestone: milestone.title } }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response).to be_an(Array)
+          expect(json_response.length).to eq(4)
+          json_response.each do |mr|
+            expect(mr['milestone']).not_to eq(milestone.title)
+          end
+        end
+
+        it 'returns merge requests without the author given', :aggregate_failures do
+          get api(endpoint_path, user), params: { not: { author_id: user2.id } }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response).to be_an(Array)
+          expect(json_response.length).to eq(5)
+          json_response.each do |mr|
+            expect(mr['author']['id']).not_to eq(user2.id)
+          end
+        end
+
+        it 'returns merge requests without the assignee given', :aggregate_failures do
+          get api(endpoint_path, user), params: { not: { assignee_id: user2.id } }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response).to be_an(Array)
+          expect(json_response.length).to eq(5)
+          json_response.each do |mr|
+            expect(mr['assignee']['id']).not_to eq(user2.id)
+          end
+        end
+      end
+
       context 'source_branch param' do
         it 'returns merge requests with the given source branch' do
           get api(endpoint_path, user), params: { source_branch: merge_request_closed.source_branch, state: 'all' }

spec/support/shared_contexts/finders/merge_requests_finder_shared_contexts.rb

@@ -23,12 +23,12 @@ RSpec.shared_context 'MergeRequestsFinder multiple projects with merge requests
   # We cannot use `let_it_be` here otherwise we get:
   #   Failure/Error: allow(RepositoryForkWorker).to receive(:perform_async).and_return(true)
   #   The use of doubles or partial doubles from rspec-mocks outside of the per-test lifecycle is not supported.
-  let(:project2) do
+  let!(:project2) do
     allow_gitaly_n_plus_1 do
       fork_project(project1, user)
     end
   end
-  let(:project3) do
+  let!(:project3) do
     allow_gitaly_n_plus_1 do
       fork_project(project1, user).tap do |project|
         project.update!(archived: true)
@@ -45,6 +45,9 @@ RSpec.shared_context 'MergeRequestsFinder multiple projects with merge requests
     allow_gitaly_n_plus_1 { create(:project, group: subgroup) }
   end
 
+  let!(:label) { create(:label, project: project1) }
+  let!(:label2) { create(:label, project: project1) }
+
   let!(:merge_request1) do
     create(:merge_request, assignees: [user], author: user,
            source_project: project2, target_project: project1,
@@ -72,6 +75,9 @@ RSpec.shared_context 'MergeRequestsFinder multiple projects with merge requests
            title: '[WIP]')
   end
 
+  let!(:label_link) { create(:label_link, label: label, target: merge_request2) }
+  let!(:label_link2) { create(:label_link, label: label2, target: merge_request3) }
+
   before do
     project1.add_maintainer(user)
     project2.add_developer(user)