diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index cee8d34a881b3494ed1e5d757c688eecbe6ab5af..34be9291f1f9ef81e6766addeecd24841e3887b5 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -74,6 +74,7 @@ user: User.new(username: '{{username}}', name: '{{name}}'), avatar: { lazy: true, url: '{{avatar_url}}' } = render_if_exists 'shared/issuable/approver_dropdown' + = render_if_exists 'shared/issuable/approved_by_dropdown' #js-dropdown-milestone.filtered-search-input-dropdown-menu.dropdown-menu %ul{ data: { dropdown: true } } %li.filter-dropdown-item{ data: { value: 'None' } } diff --git a/ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js b/ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js index 86e1fa6d2705b51d67c25b1e44570efece5fada0..5f564123a020815aeef8a139936dad31ad4d7774 100644 --- a/ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js +++ b/ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js @@ -1,10 +1,8 @@ import addExtraTokensForMergeRequests from '~/filtered_search/add_extra_tokens_for_merge_requests'; import { __ } from '~/locale'; -export default IssuableTokenKeys => { - addExtraTokensForMergeRequests(IssuableTokenKeys); - - const approversConditions = [ +const approvers = { + condition: [ { url: 'approver_usernames[]=None', tokenKey: 'approver', @@ -29,9 +27,8 @@ export default IssuableTokenKeys => { value: __('Any'), operator: '!=', }, - ]; - - const approversToken = { + ], + token: { formattedKey: __('Approver'), key: 'approver', type: 'array', @@ -39,10 +36,54 @@ export default IssuableTokenKeys => { symbol: '@', icon: 'approval', tag: '@approver', - }; - const approversTokenPosition = 2; + }, +}; + +const approvedBy = { + condition: [ + { + url: 'approved_by_usernames[]=None', + tokenKey: 'approved-by', + value: __('None'), + operator: '=', + }, + { + url: 'not[approved_by_usernames][]=None', + tokenKey: 'approved-by', + value: __('None'), + operator: '!=', + }, + { + url: 'approved_by_usernames[]=Any', + tokenKey: 'approved-by', + value: __('Any'), + operator: '=', + }, + { + url: 'not[approved_by_usernames][]=Any', + tokenKey: 'approved-by', + value: __('Any'), + operator: '!=', + }, + ], + token: { + formattedKey: __('Approved-By'), + key: 'approved-by', + type: 'array', + param: 'usernames[]', + symbol: '@', + icon: 'approval', + tag: '@approved-by', + }, +}; + +export default IssuableTokenKeys => { + addExtraTokensForMergeRequests(IssuableTokenKeys); + const tokenPosition = 2; + const combinedTokens = [approvers.token, approvedBy.token]; + const combinedConditions = [approvers.condition, approvedBy.condition]; - IssuableTokenKeys.tokenKeys.splice(approversTokenPosition, 0, approversToken); - IssuableTokenKeys.tokenKeysWithAlternative.splice(approversTokenPosition, 0, approversToken); - IssuableTokenKeys.conditions.push(...approversConditions); + IssuableTokenKeys.tokenKeys.splice(tokenPosition, 0, ...combinedTokens); + IssuableTokenKeys.tokenKeysWithAlternative.splice(tokenPosition, 0, ...combinedTokens); + IssuableTokenKeys.conditions.push(...combinedConditions); }; diff --git a/ee/app/assets/javascripts/filtered_search/available_dropdown_mappings.js b/ee/app/assets/javascripts/filtered_search/available_dropdown_mappings.js index 98e2ec8164ce597822226659863d2ca8a4ccdd65..e23a1a94562d4b44e1f77f00c38f572beff0c71d 100644 --- a/ee/app/assets/javascripts/filtered_search/available_dropdown_mappings.js +++ b/ee/app/assets/javascripts/filtered_search/available_dropdown_mappings.js @@ -47,6 +47,12 @@ export default class AvailableDropdownMappings { element: this.container.querySelector('#js-dropdown-approver'), }; + ceMappings['approved-by'] = { + reference: null, + gl: DropdownUser, + element: this.container.querySelector('#js-dropdown-approved-by'), + }; + ceMappings.weight = { reference: null, gl: DropdownWeight, diff --git a/ee/app/assets/javascripts/filtered_search/constants.js b/ee/app/assets/javascripts/filtered_search/constants.js index 339099bba20898acb02de9f229af02120a6ad348..04524688e8690da4dea44a0c6d6ddcca146a026c 100644 --- a/ee/app/assets/javascripts/filtered_search/constants.js +++ b/ee/app/assets/javascripts/filtered_search/constants.js @@ -1,4 +1,4 @@ /* eslint-disable import/prefer-default-export */ import { USER_TOKEN_TYPES as CE_USER_TOKEN_TYPES } from '~/filtered_search/constants'; -export const USER_TOKEN_TYPES = [...CE_USER_TOKEN_TYPES, 'approver']; +export const USER_TOKEN_TYPES = [...CE_USER_TOKEN_TYPES, 'approver', 'approved-by']; diff --git a/ee/app/views/shared/issuable/_approved_by_dropdown.html.haml b/ee/app/views/shared/issuable/_approved_by_dropdown.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..eae1ae212d24749c0bcaf36eeb497b4beaf4e3d3 --- /dev/null +++ b/ee/app/views/shared/issuable/_approved_by_dropdown.html.haml @@ -0,0 +1,16 @@ +#js-dropdown-approved-by.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 + - if current_user + = render 'shared/issuable/user_dropdown_item', + user: current_user + %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } } + = render 'shared/issuable/user_dropdown_item', + user: User.new(username: '{{username}}', name: '{{name}}'), + avatar: { lazy: true, url: '{{avatar_url}}' } diff --git a/ee/changelogs/unreleased/39042-filter-merge-request-approved-by-user.yml b/ee/changelogs/unreleased/39042-filter-merge-request-approved-by-user.yml new file mode 100644 index 0000000000000000000000000000000000000000..ad056e0fb4ca3959ec1a9ea64d2cffe3a81bc3cf --- /dev/null +++ b/ee/changelogs/unreleased/39042-filter-merge-request-approved-by-user.yml @@ -0,0 +1,5 @@ +--- +title: Add Approved By in filtered MR search +merge_request: 30335 +author: +type: added diff --git a/ee/spec/features/merge_requests/user_filters_by_approvers_spec.rb b/ee/spec/features/merge_requests/user_filters_by_approvers_spec.rb index c84a76e88715b515a0605efee94caa771ec7ff2a..83f66086ff1eb8399b1f1dd22b5f79ee3b9e42c5 100644 --- a/ee/spec/features/merge_requests/user_filters_by_approvers_spec.rb +++ b/ee/spec/features/merge_requests/user_filters_by_approvers_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge Requests > User filters by approvers', :js do +describe 'Merge Requests > User filters', :js do include FilteredSearchHelpers let(:project) { create(:project, :public, :repository) } @@ -30,68 +30,132 @@ visit project_merge_requests_path(project) end - context 'filtering by approver:none' do - it 'applies the filter' do - input_filtered_search('approver:=none') + context 'by "approvers"' do + context 'filtering by approver:none' do + it 'applies the filter' do + input_filtered_search('approver:=none') - expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) - expect(page).not_to have_content 'Bugfix1' - expect(page).not_to have_content 'Bugfix2' - expect(page).not_to have_content 'Bugfix4' - expect(page).to have_content 'Bugfix3' + expect(page).not_to have_content 'Bugfix1' + expect(page).not_to have_content 'Bugfix2' + expect(page).not_to have_content 'Bugfix4' + expect(page).to have_content 'Bugfix3' + end end - end - context 'filtering by approver:any' do - it 'applies the filter' do - input_filtered_search('approver:=any') + context 'filtering by approver:any' do + it 'applies the filter' do + input_filtered_search('approver:=any') - expect(page).to have_issuable_counts(open: 3, closed: 0, all: 3) + expect(page).to have_issuable_counts(open: 3, closed: 0, all: 3) - expect(page).to have_content 'Bugfix1' - expect(page).to have_content 'Bugfix2' - expect(page).to have_content 'Bugfix4' - expect(page).not_to have_content 'Bugfix3' + expect(page).to have_content 'Bugfix1' + expect(page).to have_content 'Bugfix2' + expect(page).to have_content 'Bugfix4' + expect(page).not_to have_content 'Bugfix3' + end end - end - context 'filtering by approver:@username' do - it 'applies the filter' do - input_filtered_search("approver:=@#{first_user.username}") + context 'filtering by approver:@username' do + it 'applies the filter' do + input_filtered_search("approver:=@#{first_user.username}") - expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2) + expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2) - expect(page).to have_content 'Bugfix1' - expect(page).to have_content 'Bugfix2' - expect(page).not_to have_content 'Bugfix3' - expect(page).not_to have_content 'Bugfix4' + expect(page).to have_content 'Bugfix1' + expect(page).to have_content 'Bugfix2' + expect(page).not_to have_content 'Bugfix3' + expect(page).not_to have_content 'Bugfix4' + end + end + + context 'filtering by multiple approvers' do + it 'applies the filter' do + input_filtered_search("approver:=@#{first_user.username} approver:=@#{user.username}") + + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + + expect(page).to have_content 'Bugfix2' + expect(page).not_to have_content 'Bugfix1' + expect(page).not_to have_content 'Bugfix3' + expect(page).not_to have_content 'Bugfix4' + end end - end - context 'filtering by multiple approvers' do - it 'applies the filter' do - input_filtered_search("approver:=@#{first_user.username} approver:=@#{user.username}") + context 'filtering by an approver from a group' do + it 'applies the filter' do + input_filtered_search("approver:=@#{group_user.username}") - expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) - expect(page).to have_content 'Bugfix2' - expect(page).not_to have_content 'Bugfix1' - expect(page).not_to have_content 'Bugfix3' - expect(page).not_to have_content 'Bugfix4' + expect(page).to have_content 'Bugfix4' + expect(page).not_to have_content 'Bugfix1' + expect(page).not_to have_content 'Bugfix2' + expect(page).not_to have_content 'Bugfix3' + end end end - context 'filtering by an approver from a group' do - it 'applies the filter' do - input_filtered_search("approver:=@#{group_user.username}") + context 'by "approved by"' do + let!(:merge_request_with_first_user_approval) do + create(:merge_request, source_project: project, title: 'Bugfix5').tap do |mr| + create(:approval, merge_request: mr, user: first_user) + end + end + + let!(:merge_request_with_group_user_approved) do + group = create(:group) + group.add_developer(group_user) + + create(:merge_request, source_project: project, title: 'Bugfix6', approval_groups: [group], source_branch: 'bugfix6').tap do |mr| + create(:approval, merge_request: mr, user: group_user) + end + end + + context 'filtering by approved-by:none' do + it 'applies the filter' do + input_filtered_search('approved-by:=none') + + expect(page).to have_issuable_counts(open: 4, closed: 0, all: 4) + + expect(page).not_to have_content 'Bugfix5' + expect(page).to have_content 'Bugfix3' + end + end + + context 'filtering by approved-by:any' do + it 'applies the filter' do + input_filtered_search('approved-by:=any') + + expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2) + + expect(page).to have_content 'Bugfix5' + expect(page).not_to have_content 'Bugfix3' + end + end + + context 'filtering by approved-by:@username' do + it 'applies the filter' do + input_filtered_search("approved-by:=@#{first_user.username}") + + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + + expect(page).to have_content 'Bugfix5' + expect(page).not_to have_content 'Bugfix3' + end + end + + context 'filtering by an approver from a group' do + it 'applies the filter' do + input_filtered_search("approved-by:=@#{group_user.username}") - expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) - expect(page).to have_content 'Bugfix4' - expect(page).not_to have_content 'Bugfix1' - expect(page).not_to have_content 'Bugfix2' - expect(page).not_to have_content 'Bugfix3' + expect(page).to have_content 'Bugfix6' + expect(page).not_to have_content 'Bugfix5' + expect(page).not_to have_content 'Bugfix3' + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7cec0bbee5b2ed1e38b5af934881bfddf970a9d4..ed63596d5821343cfdb7ff275a8435aefd6d4d07 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2482,6 +2482,9 @@ msgstr "" msgid "Approved the current merge request." msgstr "" +msgid "Approved-By" +msgstr "" + msgid "Approver" msgstr ""