Merge branch '31099-MR-API-allow-NOT-params' into 'master'

Add NOT param support to Merge Requests API

See merge request gitlab-org/gitlab!35391

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)