From f5b855546ed9b2c304b72e349af3f23c4eca549d Mon Sep 17 00:00:00 2001 From: Alexandru Croitor Date: Thu, 15 Aug 2019 13:56:33 +0300 Subject: [PATCH] Update sort options for issues list Increase sort options for issues list from updated_at and create_at, to include more options close to what is required in actual issue list UI. This helps us to use REST API for issues list with sorting capabilities https://gitlab.com/gitlab-org/gitlab-ce/issues/57402 --- app/models/concerns/issuable.rb | 19 +++++++++---------- app/models/concerns/sortable.rb | 6 +++++- app/models/issue.rb | 9 ++++----- .../57402-upate-issues-list-sort-options.yml | 5 +++++ doc/api/issues.md | 6 +++--- lib/api/helpers/issues_helpers.rb | 9 ++++++--- lib/api/issues.rb | 2 +- spec/requests/api/issues/issues_spec.rb | 16 ++++++++++++++++ 8 files changed, 49 insertions(+), 23 deletions(-) create mode 100644 changelogs/unreleased/57402-upate-issues-list-sort-options.yml diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index e60b6497cb7..db46d7afbb9 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -186,16 +186,15 @@ module Issuable def sort_by_attribute(method, excluded_labels: []) sorted = case method.to_s - when 'downvotes_desc' then order_downvotes_desc - when 'label_priority' then order_labels_priority(excluded_labels: excluded_labels) - when 'label_priority_desc' then order_labels_priority('DESC', excluded_labels: excluded_labels) - when 'milestone', 'milestone_due_asc' then order_milestone_due_asc - when 'milestone_due_desc' then order_milestone_due_desc - when 'popularity', 'popularity_desc' then order_upvotes_desc - when 'popularity_asc' then order_upvotes_asc - when 'priority', 'priority_asc' then order_due_date_and_labels_priority(excluded_labels: excluded_labels) - when 'priority_desc' then order_due_date_and_labels_priority('DESC', excluded_labels: excluded_labels) - when 'upvotes_desc' then order_upvotes_desc + when 'downvotes_desc' then order_downvotes_desc + when 'label_priority', 'label_priority_asc' then order_labels_priority(excluded_labels: excluded_labels) + when 'label_priority_desc' then order_labels_priority('DESC', excluded_labels: excluded_labels) + when 'milestone', 'milestone_due_asc' then order_milestone_due_asc + when 'milestone_due_desc' then order_milestone_due_desc + when 'popularity_asc' then order_upvotes_asc + when 'popularity', 'popularity_desc', 'upvotes_desc' then order_upvotes_desc + when 'priority', 'priority_asc' then order_due_date_and_labels_priority(excluded_labels: excluded_labels) + when 'priority_desc' then order_due_date_and_labels_priority('DESC', excluded_labels: excluded_labels) else order_by(method) end diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb index df1a9e3fe6e..c4af1b1fab2 100644 --- a/app/models/concerns/sortable.rb +++ b/app/models/concerns/sortable.rb @@ -27,14 +27,18 @@ module Sortable def simple_sorts { 'created_asc' => -> { order_created_asc }, + 'created_at_asc' => -> { order_created_asc }, 'created_date' => -> { order_created_desc }, 'created_desc' => -> { order_created_desc }, + 'created_at_desc' => -> { order_created_desc }, 'id_asc' => -> { order_id_asc }, 'id_desc' => -> { order_id_desc }, 'name_asc' => -> { order_name_asc }, 'name_desc' => -> { order_name_desc }, 'updated_asc' => -> { order_updated_asc }, - 'updated_desc' => -> { order_updated_desc } + 'updated_at_asc' => -> { order_updated_asc }, + 'updated_desc' => -> { order_updated_desc }, + 'updated_at_desc' => -> { order_updated_desc } } end diff --git a/app/models/issue.rb b/app/models/issue.rb index c5a18f0af0f..caea8eadd18 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -128,11 +128,10 @@ class Issue < ApplicationRecord def self.sort_by_attribute(method, excluded_labels: []) case method.to_s - when 'closest_future_date' then order_closest_future_date - when 'due_date' then order_due_date_asc - when 'due_date_asc' then order_due_date_asc - when 'due_date_desc' then order_due_date_desc - when 'relative_position' then order_relative_position_asc.with_order_id_desc + when 'closest_future_date', 'closest_future_date_asc' then order_closest_future_date + when 'due_date', 'due_date_asc' then order_due_date_asc + when 'due_date_desc' then order_due_date_desc + when 'relative_position', 'relative_position_asc' then order_relative_position_asc.with_order_id_desc else super end diff --git a/changelogs/unreleased/57402-upate-issues-list-sort-options.yml b/changelogs/unreleased/57402-upate-issues-list-sort-options.yml new file mode 100644 index 00000000000..900e71e3204 --- /dev/null +++ b/changelogs/unreleased/57402-upate-issues-list-sort-options.yml @@ -0,0 +1,5 @@ +--- +title: Updates issues REST API to allow extended sort options +merge_request: 31849 +author: +type: changed diff --git a/doc/api/issues.md b/doc/api/issues.md index 4f2b4a966c9..8313dd2c3bd 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -49,7 +49,7 @@ GET /issues?confidential=true | `my_reaction_emoji` | string | no | Return issues reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction. _([Introduced][ce-14016] in GitLab 10.0)_ | | `weight` **(STARTER)** | integer | no | Return issues with the specified `weight`. `None` returns issues with no weight assigned. `Any` returns issues with a weight assigned. | | `iids[]` | integer array | no | Return only the issues having the given `iid` | -| `order_by` | string | no | Return issues ordered by `created_at` or `updated_at` fields. Default is `created_at` | +| `order_by` | string | no | Return issues ordered by `created_at`, `updated_at`, `priority`, `due_date`, `relative_position`, `label_priority`, `milestone_due`, `popularity`, `weight` fields. Default is `created_at` | | `sort` | string | no | Return issues sorted in `asc` or `desc` order. Default is `desc` | | `search` | string | no | Search issues 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` | @@ -198,7 +198,7 @@ GET /groups/:id/issues?confidential=true | `assignee_username` | string array | no | Return issues assigned to the given `username`. Similar to `assignee_id` and mutually exclusive with `assignee_id`. In CE version `assignee_username` array should only contain a single value or an invalid param error will be returned otherwise. | | `my_reaction_emoji` | string | no | Return issues reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction. _([Introduced][ce-14016] in GitLab 10.0)_ | | `weight` **(STARTER)** | integer | no | Return issues with the specified `weight`. `None` returns issues with no weight assigned. `Any` returns issues with a weight assigned. | -| `order_by` | string | no | Return issues ordered by `created_at` or `updated_at` fields. Default is `created_at` | +| `order_by` | string | no | Return issues ordered by `created_at`, `updated_at`, `priority`, `due_date`, `relative_position`, `label_priority`, `milestone_due`, `popularity`, `weight` fields. Default is `created_at` | | `sort` | string | no | Return issues sorted in `asc` or `desc` order. Default is `desc` | | `search` | string | no | Search group issues against their `title` and `description` | | `created_after` | datetime | no | Return issues created on or after the given time | @@ -347,7 +347,7 @@ GET /projects/:id/issues?confidential=true | `assignee_username` | string array | no | Return issues assigned to the given `username`. Similar to `assignee_id` and mutually exclusive with `assignee_id`. In CE version `assignee_username` array should only contain a single value or an invalid param error will be returned otherwise. | | `my_reaction_emoji` | string | no | Return issues reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction. _([Introduced][ce-14016] in GitLab 10.0)_ | | `weight` **(STARTER)** | integer | no | Return issues with the specified `weight`. `None` returns issues with no weight assigned. `Any` returns issues with a weight assigned. | -| `order_by` | string | no | Return issues ordered by `created_at` or `updated_at` fields. Default is `created_at` | +| `order_by` | string | no | Return issues ordered by `created_at`, `updated_at`, `priority`, `due_date`, `relative_position`, `label_priority`, `milestone_due`, `popularity`, `weight` fields. Default is `created_at` | | `sort` | string | no | Return issues sorted in `asc` or `desc` order. Default is `desc` | | `search` | string | no | Search project issues against their `title` and `description` | | `created_after` | datetime | no | Return issues created on or after the given time | diff --git a/lib/api/helpers/issues_helpers.rb b/lib/api/helpers/issues_helpers.rb index 5b7199fddb0..a8480bb9339 100644 --- a/lib/api/helpers/issues_helpers.rb +++ b/lib/api/helpers/issues_helpers.rb @@ -27,6 +27,10 @@ module API ] end + def self.sort_options + %w[created_at updated_at priority due_date relative_position label_priority milestone_due popularity] + end + def issue_finder(args = {}) args = declared_params.merge(args) @@ -34,15 +38,14 @@ module API args[:milestone_title] ||= args.delete(:milestone) args[:label_name] ||= args.delete(:labels) args[:scope] = args[:scope].underscore if args[:scope] + args[:sort] = "#{args[:order_by]}_#{args[:sort]}" IssuesFinder.new(current_user, args) end def find_issues(args = {}) finder = issue_finder(args) - issues = finder.execute.with_api_entity_associations - - issues.reorder(order_options_with_tie_breaker) # rubocop: disable CodeReuse/ActiveRecord + finder.execute.with_api_entity_associations end def issues_statistics(args = {}) diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 7819c2de515..e16eeef202c 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -44,7 +44,7 @@ module API optional :with_labels_details, type: Boolean, desc: 'Return more label data than just lable title', default: false optional :state, type: String, values: %w[opened closed all], default: 'all', desc: 'Return opened, closed, or all issues' - optional :order_by, type: String, values: %w[created_at updated_at], default: 'created_at', + optional :order_by, type: String, values: Helpers::IssuesHelpers.sort_options, default: 'created_at', desc: 'Return issues ordered by `created_at` or `updated_at` fields.' optional :sort, type: String, values: %w[asc desc], default: 'desc', desc: 'Return issues sorted in `asc` or `desc` order.' diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index 27cf66629fe..f19c2dcc6fe 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -607,6 +607,22 @@ describe API::Issues do expect_paginated_array_response([closed_issue.id, issue.id]) end + context 'with issues list sort options' do + it 'accepts only predefined order by params' do + API::Helpers::IssuesHelpers.sort_options.each do |sort_opt| + get api('/issues', user), params: { order_by: sort_opt, sort: 'asc' } + expect(response).to have_gitlab_http_status(200) + end + end + + it 'fails to sort with non predefined options' do + %w(milestone title abracadabra).each do |sort_opt| + get api('/issues', user), params: { order_by: sort_opt, sort: 'asc' } + expect(response).to have_gitlab_http_status(400) + end + end + end + it 'matches V4 response schema' do get api('/issues', user)