From 227e30f7feb2072407a231a762835bf8d5103129 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Thu, 25 Oct 2018 15:02:28 +0800 Subject: Issues API: Add None/Any option to assignee_id --- app/finders/issuable_finder.rb | 35 +++++++++++++++++++++++------------ app/finders/issues_finder.rb | 9 +++++---- lib/api/issues.rb | 6 +++++- spec/finders/issues_finder_spec.rb | 18 +++++++++++++++++- spec/requests/api/issues_spec.rb | 15 +++++++++++++++ 5 files changed, 65 insertions(+), 18 deletions(-) diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 8abfe0c4c17..ec4472de0c4 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -14,7 +14,7 @@ # project_id: integer # milestone_title: string # author_id: integer -# assignee_id: integer +# assignee_id: integer or 'None' or 'Any' # search: string # label_name: string # sort: string @@ -34,6 +34,11 @@ class IssuableFinder requires_cross_project_access unless: -> { project? } + # This is used as a common filter for None / Any + FILTER_NONE = 'None'.freeze + FILTER_ANY = 'Any'.freeze + + # This is accepted as a deprecated filter and is also used in unassigning users NONE = '0'.freeze attr_accessor :current_user, :params @@ -236,16 +241,20 @@ class IssuableFinder # rubocop: enable CodeReuse/ActiveRecord def assignee_id? - params[:assignee_id].present? && params[:assignee_id].to_s != NONE + params[:assignee_id].present? end def assignee_username? - params[:assignee_username].present? && params[:assignee_username].to_s != NONE + params[:assignee_username].present? end - def no_assignee? + def filter_by_no_assignee? # Assignee_id takes precedence over assignee_username - params[:assignee_id].to_s == NONE || params[:assignee_username].to_s == NONE + [NONE, FILTER_NONE].include?(params[:assignee_id].to_s) || params[:assignee_username].to_s == NONE + end + + def filter_by_any_assignee? + params[:assignee_id].to_s == FILTER_ANY end # rubocop: disable CodeReuse/ActiveRecord @@ -399,15 +408,17 @@ class IssuableFinder # rubocop: disable CodeReuse/ActiveRecord def by_assignee(items) - if assignee - items = items.where(assignee_id: assignee.id) - elsif no_assignee? - items = items.where(assignee_id: nil) + if filter_by_no_assignee? + items.where(assignee_id: nil) + elsif filter_by_any_assignee? + items.where('assignee_id IS NOT NULL') + elsif assignee + items.where(assignee_id: assignee.id) elsif assignee_id? || assignee_username? # assignee not found - items = items.none + items.none + else + items end - - items end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index abdc47b9866..4a95eb44417 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -135,12 +135,13 @@ class IssuesFinder < IssuableFinder current_user.blank? end - # rubocop: disable CodeReuse/ActiveRecord def by_assignee(items) - if assignee - items.assigned_to(assignee) - elsif no_assignee? + if filter_by_no_assignee? items.unassigned + elsif filter_by_any_assignee? + items.assigned + elsif assignee + items.assigned_to(assignee) elsif assignee_id? || assignee_username? # assignee not found items.none else diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 25d78053c88..89232212bc7 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -40,7 +40,11 @@ module API optional :updated_after, type: DateTime, desc: 'Return issues updated after the specified time' optional :updated_before, type: DateTime, desc: 'Return issues updated before the specified time' optional :author_id, type: Integer, desc: 'Return issues which are authored by the user with the given ID' - optional :assignee_id, type: Integer, desc: 'Return issues which are assigned to the user with the given ID' + optional :assignee_id, types: [Integer, String], + values: -> (v) { + v.is_a?(Integer) or [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(v) + }, + desc: 'Return issues 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 issues 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' diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 0689c843104..7f4f613b406 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -59,11 +59,27 @@ describe IssuesFinder do context 'filtering by no assignee' do let(:params) { { assignee_id: 0 } } - it 'returns issues not assign to any assignee' do + it 'returns issues not assigned to any assignee' do expect(issues).to contain_exactly(issue4) end end + context 'filtering by no assignee' do + let(:params) { { assignee_id: 'None' } } + + it 'returns issues not assigned to any assignee' do + expect(issues).to contain_exactly(issue4) + end + end + + context 'filtering by any assignee' do + let(:params) { { assignee_id: 'Any' } } + + it 'returns issues assigned to any assignee' do + expect(issues).to contain_exactly(issue1, issue2, issue3) + end + end + context 'filtering by group_id' do let(:params) { { group_id: group.id } } diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 9f6cf12f9a7..090e8eac768 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -178,6 +178,21 @@ describe API::Issues do expect(first_issue['id']).to eq(issue2.id) end + it 'returns issues with no assignee' do + issue2 = create(:issue, author: user2, project: project) + + get api('/issues', user), assignee_id: 'None', scope: 'all' + + expect_paginated_array_response(size: 1) + expect(first_issue['id']).to eq(issue2.id) + end + + it 'returns issues with any assignee' do + get api('/issues', user), assignee_id: 'Any', scope: 'all' + + expect_paginated_array_response(size: 3) + end + it 'returns issues reacted by the authenticated user by the given emoji' do issue2 = create(:issue, project: project, author: user, assignees: [user]) award_emoji = create(:award_emoji, awardable: issue2, user: user2, name: 'star') -- cgit v1.2.1 From 006631f8823c1dfe43cc7b9ed7e47a11a3d3809c Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Thu, 25 Oct 2018 17:19:12 +0800 Subject: Apply similar change to MRs API --- app/finders/issues_finder.rb | 1 + lib/api/issues.rb | 2 +- lib/api/merge_requests.rb | 6 +++++- spec/requests/api/issues_spec.rb | 3 +++ spec/requests/api/merge_requests_spec.rb | 17 +++++++++++++++++ 5 files changed, 27 insertions(+), 2 deletions(-) diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index 4a95eb44417..cee57a83df4 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -135,6 +135,7 @@ class IssuesFinder < IssuableFinder current_user.blank? end + # rubocop: disable CodeReuse/ActiveRecord def by_assignee(items) if filter_by_no_assignee? items.unassigned diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 89232212bc7..d863a37238c 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -42,7 +42,7 @@ module API optional :author_id, type: Integer, desc: 'Return issues which are authored by the user with the given ID' optional :assignee_id, types: [Integer, String], values: -> (v) { - v.is_a?(Integer) or [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(v) + v.is_a?(Integer) || [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(v) }, desc: 'Return issues 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], diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 440d94ae186..5f209228105 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -89,7 +89,11 @@ module API optional :updated_before, type: DateTime, desc: 'Return merge requests updated before the specified time' optional :view, type: String, 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 :assignee_id, type: Integer, desc: 'Return merge requests which are assigned to the user with the given ID' + optional :assignee_id, types: [Integer, String], + values: -> (v) { + v.is_a?(Integer) || [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(v) + }, + 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' diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 090e8eac768..9cda39a569b 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -188,6 +188,9 @@ describe API::Issues do end it 'returns issues with any assignee' do + # This issue without assignee should not be returned + create(:issue, author: user2, project: project) + get api('/issues', user), assignee_id: 'Any', scope: 'all' expect_paginated_array_response(size: 3) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 07d19e3ad29..e4e0ca285e0 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -143,6 +143,23 @@ describe API::MergeRequests do expect_response_ordered_exactly(merge_request3) end + it 'returns an array of merge requests with no assignee' do + merge_request3 = create(:merge_request, :simple, author: user, source_project: project2, target_project: project2, source_branch: 'other-branch') + + get api('/merge_requests', user), assignee_id: 'None', scope: :all + + expect_response_ordered_exactly(merge_request3) + end + + it 'returns an array of merge requests with any assignee' do + # This MR with no assignee should not be returned + create(:merge_request, :simple, author: user, source_project: project2, target_project: project2, source_branch: 'other-branch') + + get api('/merge_requests', user), assignee_id: 'Any', scope: :all + + expect_response_contain_exactly(merge_request, merge_request2, merge_request_closed, merge_request_merged, merge_request_locked) + end + it 'returns an array of merge requests assigned to me' do merge_request3 = create(:merge_request, :simple, author: user, assignee: user2, source_project: project2, target_project: project2, source_branch: 'other-branch') -- cgit v1.2.1 From 31d333081023442828f07f80b52305472366834c Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Thu, 25 Oct 2018 16:54:21 +0800 Subject: Add documentation --- doc/api/issues.md | 6 +++--- doc/api/merge_requests.md | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/api/issues.md b/doc/api/issues.md index cc1d6834a20..57e861bc62e 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -40,7 +40,7 @@ GET /issues?my_reaction_emoji=star | `milestone` | string | no | The milestone title. `No+Milestone` lists all issues with no milestone. `Any+Milestone` lists all issues that have an assigned milestone | | `scope` | string | no | Return issues for the given scope: `created_by_me`, `assigned_to_me` or `all`. Defaults to `created_by_me`
For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.
_([Introduced][ce-13004] in GitLab 9.5. [Changed to snake_case][ce-18935] in GitLab 11.0)_ | | `author_id` | integer | no | Return issues created by the given user `id`. Combine with `scope=all` or `scope=assigned_to_me`. _([Introduced][ce-13004] in GitLab 9.5)_ | -| `assignee_id` | integer | no | Return issues assigned to the given user `id` _([Introduced][ce-13004] in GitLab 9.5)_ | +| `assignee_id` | integer | no | Return issues assigned to the given user `id`. `None` returns unassigned issues. `Any` returns issues with an assignee. _([Introduced][ce-13004] in GitLab 9.5)_ | | `my_reaction_emoji` | string | no | Return issues reacted by the authenticated user by the given `emoji` _([Introduced][ce-14016] in GitLab 10.0)_ | | `iids[]` | Array[integer] | 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` | @@ -154,7 +154,7 @@ GET /groups/:id/issues?my_reaction_emoji=star | `milestone` | string | no | The milestone title. `No+Milestone` lists all issues with no milestone | | `scope` | string | no | Return issues for the given scope: `created_by_me`, `assigned_to_me` or `all`.
For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.
_([Introduced][ce-13004] in GitLab 9.5. [Changed to snake_case][ce-18935] in GitLab 11.0)_ | | `author_id` | integer | no | Return issues created by the given user `id` _([Introduced][ce-13004] in GitLab 9.5)_ | -| `assignee_id` | integer | no | Return issues assigned to the given user `id` _([Introduced][ce-13004] in GitLab 9.5)_ | +| `assignee_id` | integer | no | Return issues assigned to the given user `id`. `None` returns unassigned issues. `Any` returns issues with an assignee. _([Introduced][ce-13004] in GitLab 9.5)_ | | `my_reaction_emoji` | string | no | Return issues reacted by the authenticated user by the given `emoji` _([Introduced][ce-14016] in GitLab 10.0)_ | | `order_by` | string | no | Return issues ordered by `created_at` or `updated_at` fields. Default is `created_at` | | `sort` | string | no | Return issues sorted in `asc` or `desc` order. Default is `desc` | @@ -268,7 +268,7 @@ GET /projects/:id/issues?my_reaction_emoji=star | `milestone` | string | no | The milestone title. `No+Milestone` lists all issues with no milestone | | `scope` | string | no | Return issues for the given scope: `created_by_me`, `assigned_to_me` or `all`.
For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.
_([Introduced][ce-13004] in GitLab 9.5. [Changed to snake_case][ce-18935] in GitLab 11.0)_ | | `author_id` | integer | no | Return issues created by the given user `id` _([Introduced][ce-13004] in GitLab 9.5)_ | -| `assignee_id` | integer | no | Return issues assigned to the given user `id` _([Introduced][ce-13004] in GitLab 9.5)_ | +| `assignee_id` | integer | no | Return issues assigned to the given user `id`. `None` returns unassigned issues. `Any` returns issues with an assignee. _([Introduced][ce-13004] in GitLab 9.5)_ | | `my_reaction_emoji` | string | no | Return issues reacted by the authenticated user by the given `emoji` _([Introduced][ce-14016] in GitLab 10.0)_ | | `order_by` | string | no | Return issues ordered by `created_at` or `updated_at` fields. Default is `created_at` | | `sort` | string | no | Return issues sorted in `asc` or `desc` order. Default is `desc` | diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 862ee398a84..0291b7e00c2 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -42,7 +42,7 @@ Parameters: | `updated_before` | datetime | no | Return merge requests updated on or before the given time | | `scope` | string | no | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`. Defaults to `created_by_me`
For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead. | | `author_id` | integer | no | Returns merge requests created by the given user `id`. Combine with `scope=all` or `scope=assigned_to_me` | -| `assignee_id` | integer | no | Returns merge requests assigned to the given user `id` | +| `assignee_id` | integer | no | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. | | `my_reaction_emoji` | string | no | Return merge requests reacted by the authenticated user by the given `emoji` _([Introduced][ce-14016] in GitLab 10.0)_ | | `source_branch` | string | no | Return merge requests with the given source branch | | `target_branch` | string | no | Return merge requests with the given target branch | @@ -166,7 +166,7 @@ Parameters: | `updated_before` | datetime | no | Return merge requests updated on or before the given time | | `scope` | string | no | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`.
For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.
_([Introduced][ce-13060] in GitLab 9.5. [Changed to snake_case][ce-18935] in GitLab 11.0)_ | | `author_id` | integer | no | Returns merge requests created by the given user `id` _([Introduced][ce-13060] in GitLab 9.5)_ | -| `assignee_id` | integer | no | Returns merge requests assigned to the given user `id` _([Introduced][ce-13060] in GitLab 9.5)_ | +| `assignee_id` | integer | no | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. _([Introduced][ce-13060] in GitLab 9.5)_ | | `my_reaction_emoji` | string | no | Return merge requests reacted by the authenticated user by the given `emoji` _([Introduced][ce-14016] in GitLab 10.0)_ | | `source_branch` | string | no | Return merge requests with the given source branch | | `target_branch` | string | no | Return merge requests with the given target branch | @@ -279,7 +279,7 @@ Parameters: | `updated_before` | datetime | no | Return merge requests updated on or before the given time | | `scope` | string | no | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`.
| | `author_id` | integer | no | Returns merge requests created by the given user `id` _([Introduced][ce-13060] in GitLab 9.5)_ | -| `assignee_id` | integer | no | Returns merge requests assigned to the given user `id` _([Introduced][ce-13060] in GitLab 9.5)_ | +| `assignee_id` | integer | no | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. _([Introduced][ce-13060] in GitLab 9.5)_ | | `my_reaction_emoji` | string | no | Return merge requests reacted by the authenticated user by the given `emoji` _([Introduced][ce-14016] in GitLab 10.0)_ | | `source_branch` | string | no | Return merge requests with the given source branch | | `target_branch` | string | no | Return merge requests with the given target branch | -- cgit v1.2.1 From d4e26636e72ef150a6f4446558c4d24d3bf21e69 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Thu, 25 Oct 2018 17:27:21 +0800 Subject: Add changelog entry --- changelogs/unreleased/52384-api-filter-assignee-none-any.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/52384-api-filter-assignee-none-any.yml diff --git a/changelogs/unreleased/52384-api-filter-assignee-none-any.yml b/changelogs/unreleased/52384-api-filter-assignee-none-any.yml new file mode 100644 index 00000000000..9acec04d946 --- /dev/null +++ b/changelogs/unreleased/52384-api-filter-assignee-none-any.yml @@ -0,0 +1,5 @@ +--- +title: Add None/Any option for assignee_id in Issues and Merge Requests API +merge_request: 22598 +author: Heinrich Lee Yu +type: added -- cgit v1.2.1 From bf1ed85a9d6a932a99d0a5fdf70e72ea36c2600c Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Thu, 25 Oct 2018 22:20:06 +0800 Subject: Refactor api validator to separate class --- lib/api/helpers/custom_validators.rb | 13 ++++++ lib/api/issues.rb | 5 +-- lib/api/merge_requests.rb | 5 +-- spec/lib/api/helpers/custom_validators_spec.rb | 62 ++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 8 deletions(-) create mode 100644 spec/lib/api/helpers/custom_validators_spec.rb diff --git a/lib/api/helpers/custom_validators.rb b/lib/api/helpers/custom_validators.rb index 23b1cd1ad45..e4af75f1971 100644 --- a/lib/api/helpers/custom_validators.rb +++ b/lib/api/helpers/custom_validators.rb @@ -10,8 +10,21 @@ module API raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:absence) end end + + class IntegerNoneAny < Grape::Validations::Base + def validate_param!(attr_name, params) + value = params[attr_name] + + return if value.is_a?(Integer) || + [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(value) + + raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], + message: "should be an integer, 'None' or 'Any'" + end + end end end end Grape::Validations.register_validator(:absence, ::API::Helpers::CustomValidators::Absence) +Grape::Validations.register_validator(:integer_none_any, ::API::Helpers::CustomValidators::IntegerNoneAny) diff --git a/lib/api/issues.rb b/lib/api/issues.rb index d863a37238c..405fc30a2ed 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -40,10 +40,7 @@ module API optional :updated_after, type: DateTime, desc: 'Return issues updated after the specified time' optional :updated_before, type: DateTime, desc: 'Return issues updated before the specified time' optional :author_id, type: Integer, desc: 'Return issues which are authored by the user with the given ID' - optional :assignee_id, types: [Integer, String], - values: -> (v) { - v.is_a?(Integer) || [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(v) - }, + optional :assignee_id, types: [Integer, String], integer_none_any: true, desc: 'Return issues 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 issues for the given scope: `created_by_me`, `assigned_to_me` or `all`' diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 5f209228105..a617efaaa4c 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -89,10 +89,7 @@ module API optional :updated_before, type: DateTime, desc: 'Return merge requests updated before the specified time' optional :view, type: String, 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 :assignee_id, types: [Integer, String], - values: -> (v) { - v.is_a?(Integer) || [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(v) - }, + 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`' diff --git a/spec/lib/api/helpers/custom_validators_spec.rb b/spec/lib/api/helpers/custom_validators_spec.rb new file mode 100644 index 00000000000..810d05c479c --- /dev/null +++ b/spec/lib/api/helpers/custom_validators_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +describe API::Helpers::CustomValidators do + let(:scope) do + Struct.new(:opts) do + def full_name(attr_name) + attr_name + end + end + end + + describe API::Helpers::CustomValidators::Absence do + subject do + described_class.new(['test'], {}, false, scope.new) + end + + context 'empty param' do + it 'does not raise a validation error' do + expect_no_validation_error({}) + end + end + + context 'invalid parameters' do + it 'should raise a validation error' do + expect_validation_error({ 'test' => 'some_value' }) + end + end + end + + describe API::Helpers::CustomValidators::IntegerNoneAny do + subject do + described_class.new(['test'], {}, false, scope.new) + end + + context 'valid parameters' do + it 'does not raise a validation error' do + expect_no_validation_error({ 'test' => 2 }) + expect_no_validation_error({ 'test' => 100 }) + expect_no_validation_error({ 'test' => 'None' }) + expect_no_validation_error({ 'test' => 'Any' }) + end + end + + context 'invalid parameters' do + it 'should raise a validation error' do + expect_validation_error({ 'test' => 'some_other_string' }) + end + end + end + + def expect_no_validation_error(params) + expect { validate_test_param!(params) }.not_to raise_error + end + + def expect_validation_error(params) + expect { validate_test_param!(params) }.to raise_error(Grape::Exceptions::Validation) + end + + def validate_test_param!(params) + subject.validate_param!('test', params) + end +end -- cgit v1.2.1 From e0f0c29b0cfe3c0c97191eeb96eae1299f3983d1 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Fri, 26 Oct 2018 10:47:14 +0800 Subject: Support lowercase none / any --- app/finders/issuable_finder.rb | 8 ++++---- lib/api/helpers/custom_validators.rb | 2 +- spec/finders/issues_finder_spec.rb | 18 ++++++++++++++---- spec/lib/api/helpers/custom_validators_spec.rb | 2 ++ 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index ec4472de0c4..eb3d2498830 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -35,8 +35,8 @@ class IssuableFinder requires_cross_project_access unless: -> { project? } # This is used as a common filter for None / Any - FILTER_NONE = 'None'.freeze - FILTER_ANY = 'Any'.freeze + FILTER_NONE = 'none'.freeze + FILTER_ANY = 'any'.freeze # This is accepted as a deprecated filter and is also used in unassigning users NONE = '0'.freeze @@ -250,11 +250,11 @@ class IssuableFinder def filter_by_no_assignee? # Assignee_id takes precedence over assignee_username - [NONE, FILTER_NONE].include?(params[:assignee_id].to_s) || params[:assignee_username].to_s == NONE + [NONE, FILTER_NONE].include?(params[:assignee_id].to_s.downcase) || params[:assignee_username].to_s == NONE end def filter_by_any_assignee? - params[:assignee_id].to_s == FILTER_ANY + params[:assignee_id].to_s.downcase == FILTER_ANY end # rubocop: disable CodeReuse/ActiveRecord diff --git a/lib/api/helpers/custom_validators.rb b/lib/api/helpers/custom_validators.rb index e4af75f1971..1058f4e8a5e 100644 --- a/lib/api/helpers/custom_validators.rb +++ b/lib/api/helpers/custom_validators.rb @@ -16,7 +16,7 @@ module API value = params[attr_name] return if value.is_a?(Integer) || - [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(value) + [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(value.to_s.downcase) raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: "should be an integer, 'None' or 'Any'" diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 7f4f613b406..2f164ffa8b0 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -57,17 +57,21 @@ describe IssuesFinder do end context 'filtering by no assignee' do - let(:params) { { assignee_id: 0 } } + let(:params) { { assignee_id: 'None' } } it 'returns issues not assigned to any assignee' do expect(issues).to contain_exactly(issue4) end - end - context 'filtering by no assignee' do - let(:params) { { assignee_id: 'None' } } + it 'returns issues not assigned to any assignee' do + params[:assignee_id] = 0 + + expect(issues).to contain_exactly(issue4) + end it 'returns issues not assigned to any assignee' do + params[:assignee_id] = 'none' + expect(issues).to contain_exactly(issue4) end end @@ -78,6 +82,12 @@ describe IssuesFinder do it 'returns issues assigned to any assignee' do expect(issues).to contain_exactly(issue1, issue2, issue3) end + + it 'returns issues assigned to any assignee' do + params[:assignee_id] = 'any' + + expect(issues).to contain_exactly(issue1, issue2, issue3) + end end context 'filtering by group_id' do diff --git a/spec/lib/api/helpers/custom_validators_spec.rb b/spec/lib/api/helpers/custom_validators_spec.rb index 810d05c479c..41e6fb47b11 100644 --- a/spec/lib/api/helpers/custom_validators_spec.rb +++ b/spec/lib/api/helpers/custom_validators_spec.rb @@ -38,6 +38,8 @@ describe API::Helpers::CustomValidators do expect_no_validation_error({ 'test' => 100 }) expect_no_validation_error({ 'test' => 'None' }) expect_no_validation_error({ 'test' => 'Any' }) + expect_no_validation_error({ 'test' => 'none' }) + expect_no_validation_error({ 'test' => 'any' }) end end -- cgit v1.2.1