diff options
author | Sean McGivern <sean@gitlab.com> | 2018-11-01 11:07:09 +0000 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2018-11-01 11:07:09 +0000 |
commit | c49f57fe430edeac9a5f2423dac7afb80ff72576 (patch) | |
tree | 23c8e45711083748f18d81a9f40f3b464e4b62cb | |
parent | f7c0a18b8a061bfea650897b22dce24d712c6439 (diff) | |
parent | 8df7e6021b0da30e3b7550ca83cd9ab3f991c235 (diff) | |
download | gitlab-ce-c49f57fe430edeac9a5f2423dac7afb80ff72576.tar.gz |
Merge branch 'engwan/gitlab-ce-44012-filter-reactions-none-any'
-rw-r--r-- | app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js | 10 | ||||
-rw-r--r-- | app/finders/issuable_finder.rb | 35 | ||||
-rw-r--r-- | app/models/concerns/awardable.rb | 18 | ||||
-rw-r--r-- | app/views/shared/issuable/_search_bar.html.haml | 8 | ||||
-rw-r--r-- | changelogs/unreleased/44012-filter-reactions-none-any.yml | 5 | ||||
-rw-r--r-- | doc/api/issues.md | 6 | ||||
-rw-r--r-- | doc/api/merge_requests.md | 6 | ||||
-rw-r--r-- | spec/features/issues/filtered_search/dropdown_emoji_spec.rb | 20 | ||||
-rw-r--r-- | spec/finders/issues_finder_spec.rb | 16 | ||||
-rw-r--r-- | spec/models/concerns/awardable_spec.rb | 18 | ||||
-rw-r--r-- | spec/requests/api/issues_spec.rb | 20 | ||||
-rw-r--r-- | spec/support/helpers/filtered_search_helpers.rb | 8 |
12 files changed, 142 insertions, 28 deletions
diff --git a/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js b/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js index db115583165..bb0ecb8efe7 100644 --- a/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js +++ b/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js @@ -92,6 +92,16 @@ export const conditions = [ tokenKey: 'label', value: 'none', }, + { + url: 'my_reaction_emoji=None', + tokenKey: 'my-reaction', + value: 'none', + }, + { + url: 'my_reaction_emoji=Any', + tokenKey: 'my-reaction', + value: 'any', + }, ]; const IssuableFilteredSearchTokenKeys = new FilteredSearchTokenKeys( diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 419f55fe324..93bef592c65 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -240,15 +240,6 @@ class IssuableFinder params[:assignee_username].present? end - def filter_by_no_assignee? - # Assignee_id takes precedence over assignee_username - [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.downcase == FILTER_ANY - end - # rubocop: disable CodeReuse/ActiveRecord def assignee return @assignee if defined?(@assignee) @@ -414,6 +405,15 @@ class IssuableFinder end # rubocop: enable CodeReuse/ActiveRecord + def filter_by_no_assignee? + # Assignee_id takes precedence over assignee_username + [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.downcase == FILTER_ANY + end + # rubocop: disable CodeReuse/ActiveRecord def by_author(items) if author @@ -482,12 +482,27 @@ class IssuableFinder def by_my_reaction_emoji(items) if params[:my_reaction_emoji].present? && current_user - items = items.awarded(current_user, params[:my_reaction_emoji]) + items = + if filter_by_no_reaction? + items.not_awarded(current_user) + elsif filter_by_any_reaction? + items.awarded(current_user) + else + items.awarded(current_user, params[:my_reaction_emoji]) + end end items end + def filter_by_no_reaction? + params[:my_reaction_emoji].to_s.downcase == FILTER_NONE + end + + def filter_by_any_reaction? + params[:my_reaction_emoji].to_s.downcase == FILTER_ANY + end + def label_names if labels? params[:label_name].is_a?(String) ? params[:label_name].split(',') : params[:label_name] diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb index 6f29c92d176..60b7ec2815c 100644 --- a/app/models/concerns/awardable.rb +++ b/app/models/concerns/awardable.rb @@ -13,13 +13,13 @@ module Awardable end class_methods do - def awarded(user, name) + def awarded(user, name = nil) sql = <<~EOL EXISTS ( SELECT TRUE FROM award_emoji WHERE user_id = :user_id AND - name = :name AND + #{"name = :name AND" if name.present?} awardable_type = :awardable_type AND awardable_id = #{self.arel_table.name}.id ) @@ -28,6 +28,20 @@ module Awardable where(sql, user_id: user.id, name: name, awardable_type: self.name) end + def not_awarded(user) + sql = <<~EOL + NOT EXISTS ( + SELECT TRUE + FROM award_emoji + WHERE user_id = :user_id AND + awardable_type = :awardable_type AND + awardable_id = #{self.arel_table.name}.id + ) + EOL + + where(sql, user_id: user.id, awardable_type: self.name) + end + def order_upvotes_desc order_votes_desc(AwardEmoji::UPVOTE_NAME) end diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index 1d876cc4a5d..d27f79dc404 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -105,6 +105,14 @@ %span.label-title.js-data-value {{title}} #js-dropdown-my-reaction.filtered-search-input-dropdown-menu.dropdown-menu + %ul{ data: { dropdown: true } } + %li.filter-dropdown-item{ data: { value: 'none' } } + %button.btn.btn-link{ type: 'button' } + = _('None') + %li.filter-dropdown-item{ data: { value: 'any' } } + %button.btn.btn-link{ type: 'button' } + = _('Any') + %li.divider.droplab-item-ignore %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } } %li.filter-dropdown-item %button.btn.btn-link{ type: 'button' } diff --git a/changelogs/unreleased/44012-filter-reactions-none-any.yml b/changelogs/unreleased/44012-filter-reactions-none-any.yml new file mode 100644 index 00000000000..5d685010f8a --- /dev/null +++ b/changelogs/unreleased/44012-filter-reactions-none-any.yml @@ -0,0 +1,5 @@ +--- +title: Add None / Any options to reactions filter +merge_request: 22638 +author: Heinrich Lee Yu +type: added diff --git a/doc/api/issues.md b/doc/api/issues.md index 29c7a0bdbca..6b00ead94b0 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -41,7 +41,7 @@ GET /issues?my_reaction_emoji=star | `scope` | string | no | Return issues for the given scope: `created_by_me`, `assigned_to_me` or `all`. Defaults to `created_by_me`<br> For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.<br> _([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`. `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)_ | +| `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)_ | | `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` | | `sort` | string | no | Return issues sorted in `asc` or `desc` order. Default is `desc` | @@ -155,7 +155,7 @@ GET /groups/:id/issues?my_reaction_emoji=star | `scope` | string | no | Return issues for the given scope: `created_by_me`, `assigned_to_me` or `all`.<br> For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.<br> _([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`. `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)_ | +| `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)_ | | `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` | | `search` | string | no | Search group issues against their `title` and `description` | @@ -269,7 +269,7 @@ GET /projects/:id/issues?my_reaction_emoji=star | `scope` | string | no | Return issues for the given scope: `created_by_me`, `assigned_to_me` or `all`.<br> For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.<br> _([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`. `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)_ | +| `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)_ | | `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` | | `search` | string | no | Search project issues against their `title` and `description` | diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index ea326b663a4..f3cfe0ad218 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -43,7 +43,7 @@ Parameters: | `scope` | string | no | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`. Defaults to `created_by_me`<br> 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`. `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)_ | +| `my_reaction_emoji` | string | no | Return merge requests 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)_ | | `source_branch` | string | no | Return merge requests with the given source branch | | `target_branch` | string | no | Return merge requests with the given target branch | | `search` | string | no | Search merge requests against their `title` and `description` | @@ -167,7 +167,7 @@ Parameters: | `scope` | string | no | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`.<br> For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.<br> _([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`. `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)_ | +| `my_reaction_emoji` | string | no | Return merge requests 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)_ | | `source_branch` | string | no | Return merge requests with the given source branch | | `target_branch` | string | no | Return merge requests with the given target branch | | `search` | string | no | Search merge requests against their `title` and `description` | @@ -280,7 +280,7 @@ Parameters: | `scope` | string | no | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`.<br> | | `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`. `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)_ | +| `my_reaction_emoji` | string | no | Return merge requests 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)_ | | `source_branch` | string | no | Return merge requests with the given source branch | | `target_branch` | string | no | Return merge requests with the given target branch | | `search` | string | no | Search merge requests against their `title` and `description` | diff --git a/spec/features/issues/filtered_search/dropdown_emoji_spec.rb b/spec/features/issues/filtered_search/dropdown_emoji_spec.rb index be229e8aa7d..c42fcd92a36 100644 --- a/spec/features/issues/filtered_search/dropdown_emoji_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_emoji_spec.rb @@ -92,7 +92,7 @@ describe 'Dropdown emoji', :js do it 'shows the most populated emoji at top of dropdown' do send_keys_to_filtered_search('my-reaction:') - expect(first('#js-dropdown-my-reaction li')).to have_content(award_emoji_star.name) + expect(first('#js-dropdown-my-reaction .filter-dropdown li')).to have_content(award_emoji_star.name) end end @@ -121,13 +121,29 @@ describe 'Dropdown emoji', :js do send_keys_to_filtered_search(':') end + it 'selects `None`' do + find('#js-dropdown-my-reaction .filter-dropdown-item', text: 'None').click + + expect(page).to have_css(js_dropdown_emoji, visible: false) + expect_tokens([reaction_token('none', false)]) + expect_filtered_search_input_empty + end + + it 'selects `Any`' do + find('#js-dropdown-my-reaction .filter-dropdown-item', text: 'Any').click + + expect(page).to have_css(js_dropdown_emoji, visible: false) + expect_tokens([reaction_token('any', false)]) + expect_filtered_search_input_empty + end + it 'fills in the my-reaction name' do click_emoji('thumbsup') wait_for_requests expect(page).to have_css(js_dropdown_emoji, visible: false) - expect_tokens([emoji_token('thumbsup')]) + expect_tokens([reaction_token('thumbsup')]) expect_filtered_search_input_empty end end diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 0b3e2709ade..c0488c83bd8 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -372,6 +372,22 @@ describe IssuesFinder do end context 'filtering by reaction name' do + context 'user searches by no reaction' do + let(:params) { { my_reaction_emoji: 'None' } } + + it 'returns issues that the user did not react to' do + expect(issues).to contain_exactly(issue2, issue4) + end + end + + context 'user searches by any reaction' do + let(:params) { { my_reaction_emoji: 'Any' } } + + it 'returns issues that the user reacted to' do + expect(issues).to contain_exactly(issue1, issue3) + end + end + context 'user searches by "thumbsup" reaction' do let(:params) { { my_reaction_emoji: 'thumbsup' } } diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index 69083bdc125..debc02fa51f 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -24,13 +24,29 @@ describe Awardable do end end - describe ".awarded" do + describe "#awarded" do it "filters by user and emoji name" do expect(Issue.awarded(award_emoji.user, "thumbsup")).to be_empty expect(Issue.awarded(award_emoji.user, "thumbsdown")).to eq [issue] expect(Issue.awarded(award_emoji2.user, "thumbsup")).to eq [issue2] expect(Issue.awarded(award_emoji2.user, "thumbsdown")).to be_empty end + + it "filters by user and any emoji" do + issue3 = create(:issue) + create(:award_emoji, awardable: issue3, name: "star", user: award_emoji.user) + create(:award_emoji, awardable: issue3, name: "star", user: award_emoji2.user) + + expect(Issue.awarded(award_emoji.user)).to eq [issue, issue3] + expect(Issue.awarded(award_emoji2.user)).to eq [issue2, issue3] + end + end + + describe "#not_awarded" do + it "returns issues not awarded by user" do + expect(Issue.not_awarded(award_emoji.user)).to eq [issue2] + expect(Issue.not_awarded(award_emoji2.user)).to eq [issue] + end end end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 7813f6bc550..5dbe967e4fe 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -196,14 +196,24 @@ describe API::Issues do expect_paginated_array_response(size: 3) end - it 'returns issues reacted by the authenticated user by the given emoji' do + it 'returns issues reacted by the authenticated user' do issue2 = create(:issue, project: project, author: user, assignees: [user]) - award_emoji = create(:award_emoji, awardable: issue2, user: user2, name: 'star') + create(:award_emoji, awardable: issue2, user: user2, name: 'star') - get api('/issues', user2), my_reaction_emoji: award_emoji.name, scope: 'all' + create(:award_emoji, awardable: issue, user: user2, name: 'thumbsup') - expect_paginated_array_response(size: 1) - expect(first_issue['id']).to eq(issue2.id) + get api('/issues', user2), my_reaction_emoji: 'Any', scope: 'all' + + expect_paginated_array_response(size: 2) + end + + it 'returns issues not reacted by the authenticated user' do + issue2 = create(:issue, project: project, author: user, assignees: [user]) + create(:award_emoji, awardable: issue2, user: user2, name: 'star') + + get api('/issues', user2), my_reaction_emoji: 'None', scope: 'all' + + expect_paginated_array_response(size: 2) end it 'returns issues matching given search string for title' do diff --git a/spec/support/helpers/filtered_search_helpers.rb b/spec/support/helpers/filtered_search_helpers.rb index 5f42ff77fb2..6569feec39b 100644 --- a/spec/support/helpers/filtered_search_helpers.rb +++ b/spec/support/helpers/filtered_search_helpers.rb @@ -120,8 +120,12 @@ module FilteredSearchHelpers create_token('Label', label_name, symbol) end - def emoji_token(emoji_name = nil) - { name: 'My-Reaction', emoji_name: emoji_name } + def reaction_token(reaction_name = nil, is_emoji = true) + if is_emoji + { name: 'My-Reaction', emoji_name: reaction_name } + else + create_token('My-Reaction', reaction_name) + end end def default_placeholder |