Merge branch '39042-filter-merge-request-approved-by-user' into 'master'

Add "Approved By" in filtered MR search

Closes #39042

See merge request gitlab-org/gitlab!30335

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' } }

ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js

 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);
 };

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,

ee/app/assets/javascripts/filtered_search/constants.js

 /* 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'];

ee/app/views/shared/issuable/_approved_by_dropdown.html.haml

+#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}}' }

ee/changelogs/unreleased/39042-filter-merge-request-approved-by-user.yml

+---
+title: Add Approved By in filtered MR search
+merge_request: 30335
+author:
+type: added

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 @@ describe 'Merge Requests > User filters by approvers', :js do
     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

locale/gitlab.pot

@@ -2488,6 +2488,9 @@ msgstr ""
 msgid "Approved the current merge request."
 msgstr ""
 
+msgid "Approved-By"
+msgstr ""
+
 msgid "Approver"
 msgstr ""