Merge branch '237938-merge-requests-filter-by-state' into 'master'

Search UI - Implement Merge Request scope results filter by state

See merge request gitlab-org/gitlab!41282

app/assets/javascripts/search/state_filter/components/state_filter.vue

 <script>
 import { GlDropdown, GlDropdownItem, GlDropdownDivider } from '@gitlab/ui';
-import { FILTER_STATES, FILTER_HEADER, FILTER_TEXT } from '../constants';
+import {
+  FILTER_STATES,
+  SCOPES,
+  FILTER_STATES_BY_SCOPE,
+  FILTER_HEADER,
+  FILTER_TEXT,
+} from '../constants';
 import { setUrlParams, visitUrl } from '~/lib/utils/url_utility';
 
 const FILTERS_ARRAY = Object.values(FILTER_STATES);
@@ -26,13 +32,15 @@ export default {
   },
   computed: {
     selectedFilterText() {
-      let filterText = FILTER_TEXT;
-      if (this.selectedFilter === FILTER_STATES.CLOSED.value) {
-        filterText = FILTER_STATES.CLOSED.label;
-      } else if (this.selectedFilter === FILTER_STATES.OPEN.value) {
-        filterText = FILTER_STATES.OPEN.label;
+      const filter = FILTERS_ARRAY.find(({ value }) => value === this.selectedFilter);
+      if (!filter || filter === FILTER_STATES.ANY) {
+        return FILTER_TEXT;
       }
-      return filterText;
+
+      return filter.label;
+    },
+    showDropdown() {
+      return Object.values(SCOPES).includes(this.scope);
     },
     selectedFilter: {
       get() {
@@ -63,29 +71,24 @@ export default {
   },
   filterStates: FILTER_STATES,
   filterHeader: FILTER_HEADER,
-  filtersArray: FILTERS_ARRAY,
+  filtersByScope: FILTER_STATES_BY_SCOPE,
 };
 </script>
 
 <template>
-  <gl-dropdown
-    v-if="scope === 'issues'"
-    :text="selectedFilterText"
-    class="col-sm-3 gl-pt-4 gl-pl-0"
-  >
+  <gl-dropdown v-if="showDropdown" :text="selectedFilterText" class="col-sm-3 gl-pt-4 gl-pl-0">
     <header class="gl-text-center gl-font-weight-bold gl-font-lg">
       {{ $options.filterHeader }}
     </header>
     <gl-dropdown-divider />
     <gl-dropdown-item
-      v-for="filter in $options.filtersArray"
+      v-for="filter in $options.filtersByScope[scope]"
       :key="filter.value"
       :is-check-item="true"
       :is-checked="isFilterSelected(filter.value)"
       :class="dropDownItemClass(filter)"
       @click="handleFilterChange(filter.value)"
+      >{{ filter.label }}</gl-dropdown-item
     >
-      {{ filter.label }}
-    </gl-dropdown-item>
   </gl-dropdown>
 </template>

app/assets/javascripts/search/state_filter/constants.js

@@ -17,4 +17,23 @@ export const FILTER_STATES = {
     label: __('Closed'),
     value: 'closed',
   },
+  MERGED: {
+    label: __('Merged'),
+    value: 'merged',
+  },
+};
+
+export const SCOPES = {
+  ISSUES: 'issues',
+  MERGE_REQUESTS: 'merge_requests',
+};
+
+export const FILTER_STATES_BY_SCOPE = {
+  [SCOPES.ISSUES]: [FILTER_STATES.ANY, FILTER_STATES.OPEN, FILTER_STATES.CLOSED],
+  [SCOPES.MERGE_REQUESTS]: [
+    FILTER_STATES.ANY,
+    FILTER_STATES.OPEN,
+    FILTER_STATES.MERGED,
+    FILTER_STATES.CLOSED,
+  ],
 };

changelogs/unreleased/237938-merge-requests-filter-by-state.yml

+---
+title: Search UI - Implement Merge Request scope results filter by state
+merge_request: 41282
+author:
+type: added

ee/lib/elastic/latest/issue_class_proxy.rb

@@ -3,6 +3,8 @@
 module Elastic
   module Latest
     class IssueClassProxy < ApplicationClassProxy
+      include StateFilter
+
       def elastic_search(query, options: {})
         query_hash =
           if query =~ /#(\d+)\z/
@@ -28,18 +30,6 @@ module Elastic
 
       private
 
-      def state_filter(query_hash, options)
-        state = options[:state]
-
-        return query_hash if state.blank? || state == 'all'
-        return query_hash unless %w(all opened closed).include?(state)
-
-        filter = { match: { state: state } }
-
-        query_hash[:query][:bool][:filter] << filter
-        query_hash
-      end
-
       def confidentiality_filter(query_hash, options)
         current_user = options[:current_user]
         project_ids = options[:project_ids]

ee/lib/elastic/latest/merge_request_class_proxy.rb

@@ -3,6 +3,8 @@
 module Elastic
   module Latest
     class MergeRequestClassProxy < ApplicationClassProxy
+      include StateFilter
+
       def elastic_search(query, options: {})
         query_hash =
           if query =~ /\!(\d+)\z/
@@ -20,6 +22,7 @@ module Elastic
 
         options[:features] = 'merge_requests'
         query_hash = project_ids_filter(query_hash, options)
+        query_hash = state_filter(query_hash, options)
 
         search(query_hash, options)
       end

ee/lib/elastic/latest/state_filter.rb

+# frozen_string_literal: true
+
+module Elastic
+  module Latest
+    module StateFilter
+      private
+
+      def state_filter(query_hash, options)
+        state = options[:state]
+
+        return query_hash if state.blank? || state == 'all'
+        return query_hash unless %w(all opened closed merged).include?(state)
+
+        filter = { match: { state: state } }
+
+        query_hash[:query][:bool][:filter] << filter
+        query_hash
+      end
+    end
+  end
+end

ee/lib/gitlab/elastic/search_results.rb

@@ -244,7 +244,10 @@ module Gitlab
 
       def merge_requests
         strong_memoize(:merge_requests) do
-          MergeRequest.elastic_search(query, options: base_options)
+          options = base_options
+          options[:state] = filters[:state] if filters.key?(:state)
+
+          MergeRequest.elastic_search(query, options: options)
         end
       end
 

ee/spec/lib/gitlab/elastic/group_search_results_spec.rb

@@ -17,11 +17,28 @@ RSpec.describe Gitlab::Elastic::GroupSearchResults, :elastic do
 
   context 'issues search', :sidekiq_inline do
     let!(:project) { create(:project, :public, group: group) }
-    let!(:closed_issue) { create(:issue, :closed, project: project, title: 'foo closed') }
-    let!(:opened_issue) { create(:issue, :opened, project: project, title: 'foo opened') }
+    let!(:closed_result) { create(:issue, :closed, project: project, title: 'foo closed') }
+    let!(:opened_result) { create(:issue, :opened, project: project, title: 'foo opened') }
+
+    let(:query) { 'foo' }
+    let(:scope) { 'issues' }
+
+    include_examples 'search results filtered by state' do
+      before do
+        ensure_elasticsearch_index!
+      end
+    end
+  end
+
+  context 'merge_requests search', :sidekiq_inline do
+    let!(:project) { create(:project, :public, group: group) }
+    let!(:opened_result) { create(:merge_request, :opened, source_project: project, title: 'foo opened') }
+    let!(:closed_result) { create(:merge_request, :closed, source_project: project, title: 'foo closed') }
+
     let(:query) { 'foo' }
+    let(:scope) { 'merge_requests' }
 
-    include_examples 'search issues scope filters by state' do
+    include_examples 'search results filtered by state' do
       before do
         ensure_elasticsearch_index!
       end

ee/spec/lib/gitlab/elastic/project_search_results_spec.rb

@@ -70,14 +70,31 @@ RSpec.describe Gitlab::Elastic::ProjectSearchResults, :elastic do
     end
 
     context 'filtering' do
-      include_examples 'search issues scope filters by state' do
+      context 'issues' do
         let!(:project) { create(:project, :public) }
-        let!(:closed_issue) { create(:issue, :closed, project: project, title: 'foo closed') }
-        let!(:opened_issue) { create(:issue, :opened, project: project, title: 'foo opened') }
+        let!(:closed_result) { create(:issue, :closed, project: project, title: 'foo closed') }
+        let!(:opened_result) { create(:issue, :opened, project: project, title: 'foo opened') }
         let(:query) { 'foo' }
+        let(:scope) { 'issues' }
 
-        before do
-          ensure_elasticsearch_index!
+        include_examples 'search results filtered by state' do
+          before do
+            ensure_elasticsearch_index!
+          end
+        end
+      end
+
+      context 'merge_requests' do
+        let!(:project) { create(:project, :public) }
+        let!(:opened_result) { create(:merge_request, :opened, source_project: project, title: 'foo opened') }
+        let!(:closed_result) { create(:merge_request, :closed, source_project: project, title: 'foo closed') }
+        let(:query) { 'foo' }
+        let(:scope) { 'merge_requests' }
+
+        include_examples 'search results filtered by state' do
+          before do
+            ensure_elasticsearch_index!
+          end
         end
       end
     end

ee/spec/lib/gitlab/elastic/search_results_spec.rb

@@ -174,12 +174,13 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
 
     context 'filtering' do
       let!(:project) { create(:project, :public) }
-      let!(:closed_issue) { create(:issue, :closed, project: project, title: 'foo closed') }
-      let!(:opened_issue) { create(:issue, :opened, project: project, title: 'foo opened') }
+      let!(:closed_result) { create(:issue, :closed, project: project, title: 'foo closed') }
+      let!(:opened_result) { create(:issue, :opened, project: project, title: 'foo opened') }
 
+      let(:scope) { 'issues' }
       let(:results) { described_class.new(user, 'foo', [project], filters: filters) }
 
-      include_examples 'search issues scope filters by state' do
+      include_examples 'search results filtered by state' do
         before do
           ensure_elasticsearch_index!
         end
@@ -493,6 +494,21 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
       expect(results.objects('merge_requests')).to be_empty
       expect(results.merge_requests_count).to eq 0
     end
+
+    context 'filtering' do
+      let!(:project) { create(:project, :public) }
+      let!(:opened_result) { create(:merge_request, :opened, source_project: project, title: 'foo opened') }
+      let!(:closed_result) { create(:merge_request, :closed, source_project: project, title: 'foo closed') }
+
+      let(:scope) { 'merge_requests' }
+      let(:results) { described_class.new(user, 'foo', [project], filters: filters) }
+
+      include_examples 'search results filtered by state' do
+        before do
+          ensure_elasticsearch_index!
+        end
+      end
+    end
   end
 
   describe 'project scoping' do

spec/frontend/search/components/state_filter_spec.js

 import { shallowMount } from '@vue/test-utils';
 import { GlDropdown, GlDropdownItem } from '@gitlab/ui';
 import StateFilter from '~/search/state_filter/components/state_filter.vue';
-import { FILTER_STATES } from '~/search/state_filter/constants';
+import {
+  FILTER_STATES,
+  SCOPES,
+  FILTER_STATES_BY_SCOPE,
+  FILTER_TEXT,
+} from '~/search/state_filter/constants';
 import * as urlUtils from '~/lib/utils/url_utility';
 
 jest.mock('~/lib/utils/url_utility', () => ({
@@ -38,10 +43,10 @@ describe('StateFilter', () => {
     describe.each`
       scope               | showStateDropdown
       ${'issues'}         | ${true}
+      ${'merge_requests'} | ${true}
       ${'projects'}       | ${false}
       ${'milestones'}     | ${false}
       ${'users'}          | ${false}
-      ${'merge_requests'} | ${false}
       ${'notes'}          | ${false}
       ${'wiki_blobs'}     | ${false}
       ${'blobs'}          | ${false}
@@ -55,11 +60,29 @@ describe('StateFilter', () => {
       });
     });
 
+    describe.each`
+      state                         | label
+      ${FILTER_STATES.ANY.value}    | ${FILTER_TEXT}
+      ${FILTER_STATES.OPEN.value}   | ${FILTER_STATES.OPEN.label}
+      ${FILTER_STATES.CLOSED.value} | ${FILTER_STATES.CLOSED.label}
+      ${FILTER_STATES.MERGED.value} | ${FILTER_STATES.MERGED.label}
+    `(`filter text`, ({ state, label }) => {
+      describe(`when state is ${state}`, () => {
+        beforeEach(() => {
+          wrapper = createComponent({ scope: 'issues', state });
+        });
+
+        it(`sets dropdown label to ${label}`, () => {
+          expect(findGlDropdown().attributes('text')).toBe(label);
+        });
+      });
+    });
+
     describe('Filter options', () => {
       it('renders a dropdown item for each filterOption', () => {
         expect(findDropdownItemsText()).toStrictEqual(
-          Object.keys(FILTER_STATES).map(key => {
-            return FILTER_STATES[key].label;
+          FILTER_STATES_BY_SCOPE[SCOPES.ISSUES].map(v => {
+            return v.label;
           }),
         );
       });

spec/lib/gitlab/group_search_results_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe Gitlab::GroupSearchResults do
   # before so expect(GroupsFinder) check works
   let_it_be(:group) { create(:group) }
   let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project, :public, group: group) }
   let(:filters) { {} }
   let(:limit_projects) { Project.all }
   let(:query) { 'gob' }
@@ -14,13 +15,27 @@ RSpec.describe Gitlab::GroupSearchResults do
   subject(:results) { described_class.new(user, query, limit_projects, group: group, filters: filters) }
 
   describe 'issues search' do
-    let_it_be(:project) { create(:project, :public, group: group) }
-    let_it_be(:opened_issue) { create(:issue, :opened, project: project, title: 'foo opened') }
-    let_it_be(:closed_issue) { create(:issue, :closed, project: project, title: 'foo closed') }
+    let_it_be(:opened_result) { create(:issue, :opened, project: project, title: 'foo opened') }
+    let_it_be(:closed_result) { create(:issue, :closed, project: project, title: 'foo closed') }
     let(:query) { 'foo' }
-    let(:filters) { { state: 'opened' } }
+    let(:scope) { 'issues' }
 
-    include_examples 'search issues scope filters by state'
+    include_examples 'search results filtered by state'
+  end
+
+  describe 'merge_requests search' do
+    let(:opened_result) { create(:merge_request, :opened, source_project: project, title: 'foo opened') }
+    let(:closed_result) { create(:merge_request, :closed, source_project: project, title: 'foo closed') }
+    let(:query) { 'foo' }
+    let(:scope) { 'merge_requests' }
+
+    before do
+      # we're creating those instances in before block because otherwise factory for MRs will fail on after(:build)
+      opened_result
+      closed_result
+    end
+
+    include_examples 'search results filtered by state'
   end
 
   describe 'user search' do

spec/lib/gitlab/project_search_results_spec.rb

@@ -249,8 +249,9 @@ RSpec.describe Gitlab::ProjectSearchResults do
   describe 'issues search' do
     let(:issue) { create(:issue, project: project) }
     let(:query) { issue.title }
+    let(:scope) { 'issues' }
 
-    subject(:objects) { results.objects('issues') }
+    subject(:objects) { results.objects(scope) }
 
     it 'does not list issues on private projects' do
       expect(objects).not_to include issue
@@ -262,20 +263,25 @@ RSpec.describe Gitlab::ProjectSearchResults do
 
     context 'filtering' do
       let_it_be(:project) { create(:project, :public) }
-      let_it_be(:closed_issue) { create(:issue, :closed, project: project, title: 'foo closed') }
-      let_it_be(:opened_issue) { create(:issue, :opened, project: project, title: 'foo opened') }
+      let_it_be(:closed_result) { create(:issue, :closed, project: project, title: 'foo closed') }
+      let_it_be(:opened_result) { create(:issue, :opened, project: project, title: 'foo opened') }
       let(:query) { 'foo' }
 
-      include_examples 'search issues scope filters by state'
+      include_examples 'search results filtered by state'
     end
+  end
 
-    it 'filters issues when state is provided', :aggregate_failures do
-      closed_issue = create(:issue, :closed, project: project, title: "Revert: #{issue.title}")
+  describe 'merge requests search' do
+    let(:scope) { 'merge_requests' }
+    let(:project) { create(:project, :public) }
 
-      results = described_class.new(project.creator, query, project: project, filters: { state: 'opened' })
+    context 'filtering' do
+      let!(:project) { create(:project, :public) }
+      let!(:opened_result) { create(:merge_request, :opened, source_project: project, title: 'foo opened') }
+      let!(:closed_result) { create(:merge_request, :closed, source_project: project, title: 'foo closed') }
+      let(:query) { 'foo' }
 
-      expect(results.objects('issues')).not_to include closed_issue
-      expect(results.objects('issues')).to include issue
+      include_examples 'search results filtered by state'
     end
   end
 

spec/lib/gitlab/search_results_spec.rb

@@ -150,6 +150,15 @@ RSpec.describe Gitlab::SearchResults do
 
         results.objects('merge_requests')
       end
+
+      context 'filtering' do
+        let!(:opened_result) { create(:merge_request, :opened, source_project: project, title: 'foo opened') }
+        let!(:closed_result) { create(:merge_request, :closed, source_project: project, title: 'foo closed') }
+        let(:scope) { 'merge_requests' }
+        let(:query) { 'foo' }
+
+        include_examples 'search results filtered by state'
+      end
     end
 
     describe '#issues' do
@@ -168,10 +177,12 @@ RSpec.describe Gitlab::SearchResults do
       end
 
       context 'filtering' do
-        let_it_be(:closed_issue) { create(:issue, :closed, project: project, title: 'foo closed') }
-        let_it_be(:opened_issue) { create(:issue, :opened, project: project, title: 'foo open') }
+        let(:scope) { 'issues' }
+
+        let_it_be(:closed_result) { create(:issue, :closed, project: project, title: 'foo closed') }
+        let_it_be(:opened_result) { create(:issue, :opened, project: project, title: 'foo open') }
 
-        include_examples 'search issues scope filters by state'
+        include_examples 'search results filtered by state'
       end
     end
 

spec/support/shared_examples/lib/gitlab/search_issue_state_filter_shared_examples.rb

 # frozen_string_literal: true
 
-RSpec.shared_examples 'search issues scope filters by state' do
+RSpec.shared_examples 'search results filtered by state' do
   context 'state not provided' do
     let(:filters) { {} }
 
-    it 'returns opened and closed issues', :aggregate_failures do
-      expect(results.objects('issues')).to include opened_issue
-      expect(results.objects('issues')).to include closed_issue
+    it 'returns opened and closed results', :aggregate_failures do
+      expect(results.objects(scope)).to include opened_result
+      expect(results.objects(scope)).to include closed_result
     end
   end
 
   context 'all state' do
     let(:filters) { { state: 'all' } }
 
-    it 'returns opened and closed issues', :aggregate_failures do
-      expect(results.objects('issues')).to include opened_issue
-      expect(results.objects('issues')).to include closed_issue
+    it 'returns opened and closed results', :aggregate_failures do
+      expect(results.objects(scope)).to include opened_result
+      expect(results.objects(scope)).to include closed_result
     end
   end
 
   context 'closed state' do
     let(:filters) { { state: 'closed' } }
 
-    it 'returns only closed issues', :aggregate_failures do
-      expect(results.objects('issues')).not_to include opened_issue
-      expect(results.objects('issues')).to include closed_issue
+    it 'returns only closed results', :aggregate_failures do
+      expect(results.objects(scope)).not_to include opened_result
+      expect(results.objects(scope)).to include closed_result
     end
   end
 
   context 'opened state' do
     let(:filters) { { state: 'opened' } }
 
-    it 'returns only opened issues', :aggregate_failures do
-      expect(results.objects('issues')).to include opened_issue
-      expect(results.objects('issues')).not_to include closed_issue
+    it 'returns only opened results', :aggregate_failures do
+      expect(results.objects(scope)).to include opened_result
+      expect(results.objects(scope)).not_to include closed_result
     end
   end
 
   context 'unsupported state' do
     let(:filters) { { state: 'hello' } }
 
-    it 'returns only opened issues', :aggregate_failures do
-      expect(results.objects('issues')).to include opened_issue
-      expect(results.objects('issues')).to include closed_issue
+    it 'returns only opened results', :aggregate_failures do
+      expect(results.objects(scope)).to include opened_result
+      expect(results.objects(scope)).to include closed_result
     end
   end
 end