Merge branch 'more-mr-filters' into 'master'

app/finders/merge_requests_finder.rb

@@ -6,7 +6,7 @@
 #   current_user - which user use
 #   params:
 #     scope: 'created-by-me' or 'assigned-to-me' or 'all'
-#     state: 'open' or 'closed' or 'all'
+#     state: 'open', 'closed', 'merged', or 'all'
 #     group_id: integer
 #     project_id: integer
 #     milestone_title: string

changelogs/unreleased/more-mr-filters.yml

+---
+title: 'API: Filter merge requests by milestone and labels'
+merge_request: Robert Schilling
+author: 10924 

doc/api/merge_requests.md

@@ -11,15 +11,21 @@ GET /projects/:id/merge_requests
 GET /projects/:id/merge_requests?state=opened
 GET /projects/:id/merge_requests?state=all
 GET /projects/:id/merge_requests?iids[]=42&iids[]=43
+GET /projects/:id/merge_requests?milestone=release
+GET /projects/:id/merge_requests?labels=bug,reproduced
 ```
 
 Parameters:
 
-- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user
-- `iid` (optional) - Return the request having the given `iid`
-- `state` (optional) - Return `all` requests or just those that are `merged`, `opened` or `closed`
-- `order_by` (optional) - Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at`
-- `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc`
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | integer | yes | The ID of a project |
+| `iids` | Array[integer] | no | Return the request having the given `iid` |
+| `state`   | string  | no    | Return all merge requests or just those that are `opened`, `closed`, or `merged`|
+| `order_by`| string  | no    | Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at` |
+| `sort`    | string  | no    | Return requests sorted in `asc` or `desc` order. Default is `desc`  |
+| `milestone`  | string  | no | Return merge requests for a specific milestone |
+| `labels`  | string  | no | Return merge requests matching a comma separated list of labels |
 
 ```json
 [

lib/api/merge_requests.rb

@@ -33,6 +33,17 @@ module API
           end
         end
 
+        def find_merge_requests(args = {})
+          args = params.merge(args)
+
+          args[:milestone_title] = args.delete(:milestone)
+          args[:label_name] = args.delete(:labels)
+
+          merge_requests = MergeRequestsFinder.new(current_user, args).execute.inc_notes_with_associations
+
+          merge_requests.reorder(args[:order_by] => args[:sort])
+        end
+
         params :optional_params_ce do
           optional :description, type: String, desc: 'The description of the merge request'
           optional :assignee_id, type: Integer, desc: 'The ID of a user to assign the merge request'
@@ -57,23 +68,15 @@ module API
         optional :sort, type: String, values: %w[asc desc], default: 'desc',
                         desc: 'Return merge requests sorted in `asc` or `desc` order.'
         optional :iids, type: Array[Integer], desc: 'The IID array of merge requests'
+        optional :milestone, type: String, desc: 'Return merge requests for a specific milestone'
+        optional :labels, type: String, desc: 'Comma-separated list of label names'
         use :pagination
       end
       get ":id/merge_requests" do
         authorize! :read_merge_request, user_project
 
-        merge_requests = user_project.merge_requests.inc_notes_with_associations
-        merge_requests = filter_by_iid(merge_requests, params[:iids]) if params[:iids].present?
-
-        merge_requests =
-          case params[:state]
-          when 'opened' then merge_requests.opened
-          when 'closed' then merge_requests.closed
-          when 'merged' then merge_requests.merged
-          else merge_requests
-          end
+        merge_requests = find_merge_requests(project_id: user_project.id)
 
-        merge_requests = merge_requests.reorder(params[:order_by] => params[:sort])
         present paginate(merge_requests), with: Entities::MergeRequestBasic, current_user: current_user, project: user_project
       end
 

spec/requests/api/merge_requests_spec.rb

@@ -6,10 +6,17 @@ describe API::MergeRequests do
   let(:admin)       { create(:user, :admin) }
   let(:non_member)  { create(:user) }
   let!(:project)    { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) }
-  let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, title: "Test", created_at: base_time) }
-  let!(:merge_request_closed) { create(:merge_request, state: "closed", author: user, assignee: user, source_project: project, title: "Closed test", created_at: base_time + 1.second) }
-  let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, title: "Merged test", created_at: base_time + 2.seconds, merge_commit_sha: '9999999999999999999999999999999999999999') }
   let(:milestone)   { create(:milestone, title: '1.0.0', project: project) }
+  let(:milestone1)   { create(:milestone, title: '0.9', project: project) }
+  let!(:merge_request) { create(:merge_request, :simple, milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) }
+  let!(:merge_request_closed) { create(:merge_request, state: "closed", milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.second) }
+  let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, target_project: project, title: "Merged test", created_at: base_time + 2.seconds, merge_commit_sha: '9999999999999999999999999999999999999999') }
+  let!(:note)       { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") }
+  let!(:note2)      { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") }
+  let!(:label) do
+    create(:label, title: 'label', color: '#FFAABB', project: project)
+  end
+  let!(:label_link) { create(:label_link, label: label, target: merge_request) }
 
   before do
     project.team << [user, :reporter]
@@ -99,6 +106,63 @@ describe API::MergeRequests do
         expect(response).to match_response_schema('public_api/v4/merge_requests')
       end
 
+      it 'returns an empty array if no issue matches milestone' do
+        get api("/projects/#{project.id}/merge_requests", user), milestone: '1.0.0'
+
+        expect(response).to have_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(0)
+      end
+
+      it 'returns an empty array if milestone does not exist' do
+        get api("/projects/#{project.id}/merge_requests", user), milestone: 'foo'
+
+        expect(response).to have_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(0)
+      end
+
+      it 'returns an array of merge requests in given milestone' do
+        get api("/projects/#{project.id}/merge_requests", user), milestone: '0.9'
+
+        expect(json_response.first['title']).to eq merge_request_closed.title
+        expect(json_response.first['id']).to eq merge_request_closed.id
+      end
+
+      it 'returns an array of merge requests matching state in milestone' do
+        get api("/projects/#{project.id}/merge_requests", user), milestone: '0.9', state: 'closed'
+
+        expect(response).to have_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(1)
+        expect(json_response.first['id']).to eq(merge_request_closed.id)
+      end
+
+      it 'returns an array of labeled merge requests' do
+        get api("/projects/#{project.id}/merge_requests?labels=#{label.title}", user)
+
+        expect(response).to have_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(1)
+        expect(json_response.first['labels']).to eq([label.title])
+      end
+
+      it 'returns an array of labeled merge requests where all labels match' do
+        get api("/projects/#{project.id}/merge_requests?labels=#{label.title},foo,bar", user)
+
+        expect(response).to have_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(0)
+      end
+
+      it 'returns an empty array if no merge request matches labels' do
+        get api("/projects/#{project.id}/merge_requests?labels=foo,bar", user)
+
+        expect(response).to have_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(0)
+      end
+
       context "with ordering" do
         before do
           @mr_later = mr_with_later_created_and_updated_at_time
@@ -166,7 +230,7 @@ describe API::MergeRequests do
       expect(json_response['created_at']).to be_present
       expect(json_response['updated_at']).to be_present
       expect(json_response['labels']).to eq(merge_request.label_names)
-      expect(json_response['milestone']).to be_nil
+      expect(json_response['milestone']).to be_a Hash
       expect(json_response['assignee']).to be_a Hash
       expect(json_response['author']).to be_a Hash
       expect(json_response['target_branch']).to eq(merge_request.target_branch)