From c2426b4eac330a9bfabc7f28f4c7063c69129b3f Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Fri, 27 Jan 2017 15:34:15 -0200 Subject: Unify issues search behavior by always filtering when ALL labels matches --- .../26315-unify-labels-filter-behavior.yml | 4 ++ doc/api/issues.md | 4 +- doc/api/v3_to_v4.md | 2 + lib/api/issues.rb | 14 ++----- spec/requests/api/issues_spec.rb | 47 +++++++++++++++++++--- 5 files changed, 52 insertions(+), 19 deletions(-) create mode 100644 changelogs/unreleased/26315-unify-labels-filter-behavior.yml diff --git a/changelogs/unreleased/26315-unify-labels-filter-behavior.yml b/changelogs/unreleased/26315-unify-labels-filter-behavior.yml new file mode 100644 index 00000000000..cd2f40c94fe --- /dev/null +++ b/changelogs/unreleased/26315-unify-labels-filter-behavior.yml @@ -0,0 +1,4 @@ +--- +title: Unify issues search behavior by always filtering when ALL labels matches +merge_request: 8849 +author: diff --git a/doc/api/issues.md b/doc/api/issues.md index 7c0a444d4fa..f40e0938b0f 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -30,7 +30,7 @@ GET /issues?milestone=1.0.0&state=opened | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | | `state` | string | no | Return all issues or just those that are `opened` or `closed`| -| `labels` | string | no | Comma-separated list of label names, issues with any of the labels will be returned | +| `labels` | string | no | Comma-separated list of label names, issues must have all labels to be returned | | `milestone` | string| no | The milestone title | | `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` | @@ -188,7 +188,7 @@ GET /projects/:id/issues?milestone=1.0.0&state=opened | `id` | integer | yes | The ID of a project | | `iid` | integer | no | Return the issue having the given `iid` | | `state` | string | no | Return all issues or just those that are `opened` or `closed`| -| `labels` | string | no | Comma-separated list of label names, issues with any of the labels will be returned | +| `labels` | string | no | Comma-separated list of label names, issues must have all labels to be returned | | `milestone` | string| no | The milestone title | | `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` | diff --git a/doc/api/v3_to_v4.md b/doc/api/v3_to_v4.md index 2f82b6e97cc..95a78323242 100644 --- a/doc/api/v3_to_v4.md +++ b/doc/api/v3_to_v4.md @@ -27,3 +27,5 @@ changes are in V4: - Return pagination headers for all endpoints that return an array - Removed `DELETE projects/:id/deploy_keys/:key_id/disable`. Use `DELETE projects/:id/deploy_keys/:key_id` instead - Moved `PUT /users/:id/(block|unblock)` to `POST /users/:id/(block|unblock)` +- Labels filter on `projects/:id/issues` and `/issues` now matches only issues containing all labels (i.e.: Logical AND, not OR) + diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 90fca20d4fa..2b946bfd349 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -10,17 +10,9 @@ module API args.delete(:id) args[:milestone_title] = args.delete(:milestone) + args[:label_name] = args.delete(:labels) - match_all_labels = args.delete(:match_all_labels) - labels = args.delete(:labels) - args[:label_name] = labels if match_all_labels - - issues = IssuesFinder.new(current_user, args).execute.inc_notes_with_associations - - # TODO: Remove in 9.0 pass `label_name: args.delete(:labels)` to IssuesFinder - if !match_all_labels && labels.present? - issues = issues.includes(:labels).where('labels.title' => labels.split(',')) - end + issues = IssuesFinder.new(current_user, args).execute issues.reorder(args[:order_by] => args[:sort]) end @@ -77,7 +69,7 @@ module API get ":id/issues" do group = find_group!(params[:id]) - issues = find_issues(group_id: group.id, state: params[:state] || 'opened', match_all_labels: true) + issues = find_issues(group_id: group.id, state: params[:state] || 'opened') present paginate(issues), with: Entities::Issue, current_user: current_user end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 74ac7955cb8..ece1b43567d 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -117,14 +117,20 @@ describe API::Issues, api: true do expect(json_response.first['labels']).to eq([label.title]) end - it 'returns an array of labeled issues when at least one label matches' do - get api("/issues?labels=#{label.title},foo,bar", user) + it 'returns an array of labeled issues when all labels matches' do + label_b = create(:label, title: 'foo', project: project) + label_c = create(:label, title: 'bar', project: project) + + create(:label_link, label: label_b, target: issue) + create(:label_link, label: label_c, target: issue) + + get api("/issues", user), labels: "#{label.title},#{label_b.title},#{label_c.title}" expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) - expect(json_response.first['labels']).to eq([label.title]) + expect(json_response.first['labels']).to eq([label_c.title, label_b.title, label.title]) end it 'returns an empty array if no issue matches labels' do @@ -356,6 +362,21 @@ describe API::Issues, api: true do expect(json_response.length).to eq(0) end + it 'returns an array of labeled issues when all labels matches' do + label_b = create(:label, title: 'foo', project: group_project) + label_c = create(:label, title: 'bar', project: group_project) + + create(:label_link, label: label_b, target: group_issue) + create(:label_link, label: label_c, target: group_issue) + + get api("#{base_url}", user), labels: "#{group_label.title},#{label_b.title},#{label_c.title}" + + 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_c.title, label_b.title, group_label.title]) + end + it 'returns an empty array if no group issue matches labels' do get api("#{base_url}?labels=foo,bar", user) @@ -549,14 +570,28 @@ describe API::Issues, api: true do expect(json_response.first['labels']).to eq([label.title]) end - it 'returns an array of labeled project issues where all labels match' do - get api("#{base_url}/issues?labels=#{label.title},foo,bar", user) + it 'returns an array of labeled issues when all labels matches' do + label_b = create(:label, title: 'foo', project: project) + label_c = create(:label, title: 'bar', project: project) + + create(:label_link, label: label_b, target: issue) + create(:label_link, label: label_c, target: issue) + + get api("#{base_url}/issues", user), labels: "#{label.title},#{label_b.title},#{label_c.title}" expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) - expect(json_response.first['labels']).to eq([label.title]) + expect(json_response.first['labels']).to eq([label_c.title, label_b.title, label.title]) + end + + it 'returns an empty array if not all labels matches' do + get api("#{base_url}/issues?labels=#{label.title},foo", 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 project issue matches labels' do -- cgit v1.2.1