From 76080aaaf90f1f7cef1f04543f385e845f0a11b7 Mon Sep 17 00:00:00 2001 From: maddievn <mvanniekerk@gitlab.com> Date: Tue, 11 Jul 2023 15:13:09 +0200 Subject: [PATCH 1/3] Exclude archived projects from merge request search Changelog: changed --- ..._merge_requests_hide_archived_projects.yml | 8 +++++ ee/app/models/ee/project.rb | 2 ++ ...10142700_add_archived_to_merge_requests.rb | 19 +++++++++++ ...500_backfill_archived_on_merge_requests.rb | 22 +++++++++++++ .../elastic/latest/application_class_proxy.rb | 7 ++++ ee/lib/elastic/latest/issue_class_proxy.rb | 5 --- .../latest/merge_request_class_proxy.rb | 7 ++++ ee/lib/elastic/latest/merge_request_config.rb | 1 + .../latest/merge_request_instance_proxy.rb | 4 +++ ee/lib/gitlab/elastic/search_results.rb | 2 +- ...700_add_archived_to_merge_requests_spec.rb | 11 +++++++ ...ackfill_archived_on_merge_requests_spec.rb | 19 +++++++++++ .../latest/merge_request_class_proxy_spec.rb | 31 +++++++++++++++++ .../elastic/group_search_results_spec.rb | 20 +++++++---- .../lib/gitlab/elastic/search_results_spec.rb | 28 ++++++++++------ .../concerns/elastic/merge_request_spec.rb | 10 ++++-- .../support/elastic_query_name_inspector.rb | 4 +++ .../support/helpers/elasticsearch_helpers.rb | 6 ++-- lib/gitlab/search_results.rb | 8 ++++- spec/lib/gitlab/group_search_results_spec.rb | 11 +++++-- spec/lib/gitlab/search_results_spec.rb | 26 +++++---------- .../search_archived_filter_shared_examples.rb | 33 +++++++++++++------ 22 files changed, 225 insertions(+), 59 deletions(-) create mode 100644 config/feature_flags/development/search_merge_requests_hide_archived_projects.yml create mode 100644 ee/elastic/migrate/20230710142700_add_archived_to_merge_requests.rb create mode 100644 ee/elastic/migrate/20230711140500_backfill_archived_on_merge_requests.rb create mode 100644 ee/spec/elastic/migrate/20230710142700_add_archived_to_merge_requests_spec.rb create mode 100644 ee/spec/elastic/migrate/20230711140500_backfill_archived_on_merge_requests_spec.rb diff --git a/config/feature_flags/development/search_merge_requests_hide_archived_projects.yml b/config/feature_flags/development/search_merge_requests_hide_archived_projects.yml new file mode 100644 index 0000000000000000..a33d6605644d7e3f --- /dev/null +++ b/config/feature_flags/development/search_merge_requests_hide_archived_projects.yml @@ -0,0 +1,8 @@ +--- +name: search_merge_requests_hide_archived_projects +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/126024 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/417595 +milestone: '16.2' +type: development +group: group::global search +default_enabled: false diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 004e86f11df70f90..f25eb61d00576bcc 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -155,6 +155,8 @@ def lock_for_confirmation!(id) elastic_index_dependant_association :issues, on_change: :visibility_level elastic_index_dependant_association :issues, on_change: :archived, depends_on_finished_migration: :add_archived_to_issues elastic_index_dependant_association :merge_requests, on_change: :visibility_level + elastic_index_dependant_association :merge_requests, on_change: :archived, + depends_on_finished_migration: :add_archived_to_merge_requests elastic_index_dependant_association :notes, on_change: :visibility_level elastic_index_dependant_association :milestones, on_change: :visibility_level diff --git a/ee/elastic/migrate/20230710142700_add_archived_to_merge_requests.rb b/ee/elastic/migrate/20230710142700_add_archived_to_merge_requests.rb new file mode 100644 index 0000000000000000..f79e4aff69ded860 --- /dev/null +++ b/ee/elastic/migrate/20230710142700_add_archived_to_merge_requests.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddArchivedToMergeRequests < Elastic::Migration + include Elastic::MigrationUpdateMappingsHelper + + private + + def index_name + MergeRequest.__elasticsearch__.index_name + end + + def new_mappings + { + archived: { + type: 'boolean' + } + } + end +end diff --git a/ee/elastic/migrate/20230711140500_backfill_archived_on_merge_requests.rb b/ee/elastic/migrate/20230711140500_backfill_archived_on_merge_requests.rb new file mode 100644 index 0000000000000000..93643cdd0f5e3e4c --- /dev/null +++ b/ee/elastic/migrate/20230711140500_backfill_archived_on_merge_requests.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class BackfillArchivedOnMergeRequests < Elastic::Migration + include Elastic::MigrationBackfillHelper + + batched! + batch_size 9_000 + throttle_delay 1.minute + + DOCUMENT_TYPE = MergeRequest + UPDATE_BATCH_SIZE = 100 + + private + + def index_name + DOCUMENT_TYPE.__elasticsearch__.index_name + end + + def field_name + 'archived' + end +end diff --git a/ee/lib/elastic/latest/application_class_proxy.rb b/ee/lib/elastic/latest/application_class_proxy.rb index fd139baa659d9112..d6faadc3d361663a 100644 --- a/ee/lib/elastic/latest/application_class_proxy.rb +++ b/ee/lib/elastic/latest/application_class_proxy.rb @@ -241,6 +241,13 @@ def remove_fields_boost(fields) fields.map { |m| m.split('^').first } end + def archived_filter(query_hash) + exclude_archived_query = { term: { archived: { _name: context.name(:non_archived), value: false } } } + query_hash[:query][:bool][:filter] << exclude_archived_query + + query_hash + end + # Builds an elasticsearch query that will select projects the user is # granted access to. # diff --git a/ee/lib/elastic/latest/issue_class_proxy.rb b/ee/lib/elastic/latest/issue_class_proxy.rb index 42b34d3b1ee68c42..35b87a1d1bf8dc5b 100644 --- a/ee/lib/elastic/latest/issue_class_proxy.rb +++ b/ee/lib/elastic/latest/issue_class_proxy.rb @@ -165,11 +165,6 @@ def hidden_filter(query_hash) query_hash end - def archived_filter(query_hash) - query_hash[:query][:bool][:filter] << { term: { archived: { _name: context.name(:non_archived), value: false } } } - query_hash - end - def label_ids_filter(query_hash, options) labels = [options[:labels]].flatten return query_hash unless labels.any? diff --git a/ee/lib/elastic/latest/merge_request_class_proxy.rb b/ee/lib/elastic/latest/merge_request_class_proxy.rb index 4f074f3be1d25201..386d10ac570d390f 100644 --- a/ee/lib/elastic/latest/merge_request_class_proxy.rb +++ b/ee/lib/elastic/latest/merge_request_class_proxy.rb @@ -24,6 +24,7 @@ def elastic_search(query, options: {}) context.name(:merge_request) do query_hash = context.name(:authorized) { project_ids_filter(query_hash, options) } query_hash = context.name(:match) { state_filter(query_hash, options) } + query_hash = context.name(:archived) { archived_filter(query_hash) } if archived_filter_applicable?(options) if hidden_filter_applicable?(options[:current_user]) query_hash = context.name(:hidden) { hidden_filter(query_hash) } end @@ -69,6 +70,12 @@ def hidden_filter_applicable?(user) Feature.enabled?(:hide_merge_requests_from_banned_users) && !user&.can_admin_all_resources? end + def archived_filter_applicable?(options) + Feature.enabled?(:search_merge_requests_hide_archived_projects) && + ::Elastic::DataMigrationService.migration_has_finished?(:backfill_archived_on_merge_requests) && + !options[:include_archived] + end + def hidden_filter(query_hash) if ::Elastic::DataMigrationService.migration_has_finished?(:backfill_hidden_on_merge_requests) query_hash[:query][:bool][:filter] << { term: { hidden: { _name: context.name(:non_hidden), value: false } } } diff --git a/ee/lib/elastic/latest/merge_request_config.rb b/ee/lib/elastic/latest/merge_request_config.rb index 1bb8d2f1f1507703..94eb111cf17c4295 100644 --- a/ee/lib/elastic/latest/merge_request_config.rb +++ b/ee/lib/elastic/latest/merge_request_config.rb @@ -37,6 +37,7 @@ module MergeRequestConfig indexes :updated_at, type: :date indexes :hidden, type: :boolean + indexes :archived, type: :boolean indexes :visibility_level, type: :integer indexes :merge_requests_access_level, type: :integer indexes :upvotes, type: :integer diff --git a/ee/lib/elastic/latest/merge_request_instance_proxy.rb b/ee/lib/elastic/latest/merge_request_instance_proxy.rb index 493ede4fe50d215b..ede389a000bf8635 100644 --- a/ee/lib/elastic/latest/merge_request_instance_proxy.rb +++ b/ee/lib/elastic/latest/merge_request_instance_proxy.rb @@ -39,6 +39,10 @@ def as_indexed_json(options = {}) data['hidden'] = target.author&.banned? end + if ::Elastic::DataMigrationService.migration_has_finished?(:add_archived_to_merge_requests) + data['archived'] = target.project.archived? + end + data.merge(generic_attributes) end diff --git a/ee/lib/gitlab/elastic/search_results.rb b/ee/lib/gitlab/elastic/search_results.rb index 4f60266d599448e7..ebbdbf625450a8d8 100644 --- a/ee/lib/gitlab/elastic/search_results.rb +++ b/ee/lib/gitlab/elastic/search_results.rb @@ -286,7 +286,7 @@ def scope_options(scope) when :projects base_options.merge(filters.slice(:include_archived)) when :merge_requests - base_options.merge(filters.slice(:order_by, :sort, :state)) + base_options.merge(filters.slice(:order_by, :sort, :state, :include_archived)) when :issues base_options.merge(filters.slice(:order_by, :sort, :confidential, :state, :labels, :include_archived)) when :milestones diff --git a/ee/spec/elastic/migrate/20230710142700_add_archived_to_merge_requests_spec.rb b/ee/spec/elastic/migrate/20230710142700_add_archived_to_merge_requests_spec.rb new file mode 100644 index 0000000000000000..8725d4cb5276c069 --- /dev/null +++ b/ee/spec/elastic/migrate/20230710142700_add_archived_to_merge_requests_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative 'migration_shared_examples' +require File.expand_path('ee/elastic/migrate/20230710142700_add_archived_to_merge_requests.rb') + +RSpec.describe AddArchivedToMergeRequests, :elastic, :sidekiq_inline, feature_category: :global_search do + let(:version) { 20230710142700 } + + include_examples 'migration adds mapping' +end diff --git a/ee/spec/elastic/migrate/20230711140500_backfill_archived_on_merge_requests_spec.rb b/ee/spec/elastic/migrate/20230711140500_backfill_archived_on_merge_requests_spec.rb new file mode 100644 index 0000000000000000..e417863ec30b61a2 --- /dev/null +++ b/ee/spec/elastic/migrate/20230711140500_backfill_archived_on_merge_requests_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative 'migration_shared_examples' +require File.expand_path('ee/elastic/migrate/20230711140500_backfill_archived_on_merge_requests.rb') + +RSpec.describe BackfillArchivedOnMergeRequests, :elastic_delete_by_query, :sidekiq_inline, feature_category: :global_search do + let(:version) { 20230711140500 } + + include_examples 'migration backfills fields' do + let_it_be(:project) { create(:project, archived: true) } + let(:objects) { create_list(:merge_request, 3, :unique_branches, target_project: project, source_project: project) } + let(:namespace) { project.namespace } + let(:expected_fields) { { archived: project.archived? } } + + let(:expected_throttle_delay) { 1.minute } + let(:expected_batch_size) { 9000 } + end +end diff --git a/ee/spec/lib/elastic/latest/merge_request_class_proxy_spec.rb b/ee/spec/lib/elastic/latest/merge_request_class_proxy_spec.rb index f411787adc6a1104..d7a9abe0f5343bff 100644 --- a/ee/spec/lib/elastic/latest/merge_request_class_proxy_spec.rb +++ b/ee/spec/lib/elastic/latest/merge_request_class_proxy_spec.rb @@ -52,6 +52,37 @@ it 'does not include merge_requests from the banned authors' do expect(elasticsearch_hit_ids(result)).to match_array([active_user_mr.id]) end + + it 'has the correct named queries' do + result.response + + assert_named_queries( + 'merge_request:match:search_terms', + 'merge_request:archived:non_archived' + ) + end + + context 'when search_issues_hide_archived_projects is disabled' do + before do + stub_feature_flags(search_merge_requests_hide_archived_projects: false) + end + + it 'does not have a filter for archived' do + result.response + + assert_named_queries('merge_request:match:search_terms', without: ['merge_request:archived:non_archived']) + end + end + + context 'when include_archived is set' do + let(:options) { { include_archived: true } } + + it 'does not have a filter for archived' do + result.response + + assert_named_queries('merge_request:match:search_terms', without: ['merge_request:archived:non_archived']) + end + end end context 'when current_user is anonymous' do diff --git a/ee/spec/lib/gitlab/elastic/group_search_results_spec.rb b/ee/spec/lib/gitlab/elastic/group_search_results_spec.rb index 548b315368270236..f75fe296ea2de040 100644 --- a/ee/spec/lib/gitlab/elastic/group_search_results_spec.rb +++ b/ee/spec/lib/gitlab/elastic/group_search_results_spec.rb @@ -38,17 +38,23 @@ context 'merge_requests search', :sidekiq_inline do let!(:project) { create(:project, :public, group: group) } + let_it_be(:unarchived_project) { create(:project, :public, group: group) } + let_it_be(:archived_project) { create(:project, :public, :archived, 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!(:unarchived_result) { create(:merge_request, source_project: unarchived_project, title: 'foo unarchived') } + let!(:archived_result) { create(:merge_request, source_project: archived_project, title: 'foo archived') } let(:query) { 'foo' } let(:scope) { 'merge_requests' } - include_examples 'search results filtered by state' do - before do - ensure_elasticsearch_index! - end + before do + set_elasticsearch_migration_to(:backfill_archived_on_merge_requests, including: true) + ensure_elasticsearch_index! end + + include_examples 'search results filtered by state' + include_examples 'search results filtered by archived', 'search_merge_requests_hide_archived_projects' end context 'blobs' do @@ -58,12 +64,12 @@ end context 'for projects' do - let!(:unarchived_project) { create(:project, :public, group: group) } - let!(:archived_project) { create(:project, :archived, :public, group: group) } + let!(:unarchived_result) { create(:project, :public, group: group) } + let!(:archived_result) { create(:project, :archived, :public, group: group) } let(:scope) { 'projects' } - it_behaves_like 'search results filtered by archived' do + it_behaves_like 'search results filtered by archived', 'search_projects_hide_archived' do before do ensure_elasticsearch_index! end diff --git a/ee/spec/lib/gitlab/elastic/search_results_spec.rb b/ee/spec/lib/gitlab/elastic/search_results_spec.rb index d690085ee51f4395..590ec9aecb1441b0 100644 --- a/ee/spec/lib/gitlab/elastic/search_results_spec.rb +++ b/ee/spec/lib/gitlab/elastic/search_results_spec.rb @@ -401,12 +401,12 @@ context 'for projects' do let_it_be(:group) { create(:group) } - let!(:unarchived_project) { create(:project, :public, group: group) } - let!(:archived_project) { create(:project, :archived, :public, group: group) } + let!(:unarchived_result) { create(:project, :public, group: group) } + let!(:archived_result) { create(:project, :archived, :public, group: group) } + let(:scope) { 'projects' } + let(:results) { described_class.new(user, '*', [unarchived_result.id, archived_result.id], filters: filters) } - let(:results) { described_class.new(user, '*', [unarchived_project.id, archived_project.id], filters: filters) } - - it_behaves_like 'search results filtered by archived' do + it_behaves_like 'search results filtered by archived', 'search_projects_hide_archived' do before do ensure_elasticsearch_index! end @@ -802,16 +802,24 @@ context 'filtering' do let!(:project) { create(:project, :public) } + let_it_be(:unarchived_project) { create(:project, :public) } + let_it_be(:archived_project) { create(:project, :public, :archived) } 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!(:unarchived_result) { create(:merge_request, source_project: unarchived_project, title: 'foo unarchived') } + let!(:archived_result) { create(:merge_request, source_project: archived_project, title: 'foo archived') } + let(:scope) { 'merge_requests' } + let(:project_ids) { [project.id, unarchived_project.id, archived_project.id] } - let(:results) { described_class.new(user, 'foo', [project.id], filters: filters) } + let(:results) { described_class.new(user, 'foo', project_ids, filters: filters) } - include_examples 'search results filtered by state' do - before do - ensure_elasticsearch_index! - end + before do + set_elasticsearch_migration_to(:backfill_archived_on_merge_requests, including: true) + ensure_elasticsearch_index! end + + include_examples 'search results filtered by state' + include_examples 'search results filtered by archived', 'search_merge_requests_hide_archived_projects' end context 'ordering' do diff --git a/ee/spec/models/concerns/elastic/merge_request_spec.rb b/ee/spec/models/concerns/elastic/merge_request_spec.rb index 19c7f745deb0279d..b053fe2638591235 100644 --- a/ee/spec/models/concerns/elastic/merge_request_spec.rb +++ b/ee/spec/models/concerns/elastic/merge_request_spec.rb @@ -90,6 +90,8 @@ 'merge_requests_access_level' => ProjectFeature::ENABLED, 'visibility_level' => Gitlab::VisibilityLevel::INTERNAL, 'project_id' => merge_request.target_project.id, + 'hidden' => merge_request.author.banned?, + 'archived' => merge_request.target_project.archived?, 'hashed_root_namespace_id' => merge_request.target_project.namespace.hashed_root_namespace_id }) end @@ -99,11 +101,15 @@ it 'does not include hidden if add_hidden_to_merge_requests is not finished' do set_elasticsearch_migration_to :add_hidden_to_merge_requests, including: false - expect(merge_request.__elasticsearch__.as_indexed_json).to eq(expected_hash) + expect(merge_request.__elasticsearch__.as_indexed_json).to eq(expected_hash.except('hidden', 'archived')) + end + + it 'does not include archived if add_archived_to_merge_requests is not finished' do + set_elasticsearch_migration_to :add_archived_to_merge_requests, including: false + expect(merge_request.__elasticsearch__.as_indexed_json).to eq(expected_hash.except('archived')) end it 'returns json with all needed elements' do - expected_hash['hidden'] = merge_request.author.banned? expect(merge_request.__elasticsearch__.as_indexed_json).to eq(expected_hash) end end diff --git a/ee/spec/support/elastic_query_name_inspector.rb b/ee/spec/support/elastic_query_name_inspector.rb index 0aa36f315a8ff67b..ef599d9a6a65b3f1 100644 --- a/ee/spec/support/elastic_query_name_inspector.rb +++ b/ee/spec/support/elastic_query_name_inspector.rb @@ -15,4 +15,8 @@ def inspect(query) def has_named_query?(*expected_names) @names.superset?(expected_names.to_set) end + + def excludes_named_query?(*unexpected_names) + unexpected_names.all? { |name| @names.exclude?(name) } + end end diff --git a/ee/spec/support/helpers/elasticsearch_helpers.rb b/ee/spec/support/helpers/elasticsearch_helpers.rb index eab9b857122ff907..14065ecc5517a620 100644 --- a/ee/spec/support/helpers/elasticsearch_helpers.rb +++ b/ee/spec/support/helpers/elasticsearch_helpers.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module ElasticsearchHelpers - def assert_named_queries(*expected_names) + def assert_named_queries(*expected_names, without: []) es_host = Gitlab::CurrentSettings.elasticsearch_url.first search_uri = %r{#{es_host}/[\w-]+/_search} @@ -15,12 +15,14 @@ def assert_named_queries(*expected_names) inspector.inspect(query) inspector.has_named_query?(*expected_names) + inspector.excludes_named_query?(*without) rescue ::JSON::ParserError false end a_named_query = a_request(:post, search_uri).with(&ensure_names_present) - message = "Expected a query with the following names: #{expected_names.inspect}" + message = "Expected a query with the names #{expected_names.inspect}" + message << " and without the names #{without.inspect}" if without.any? expect(a_named_query).to have_been_made.at_least_once, message end diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb index 4fedc450f9bb808f..55ec90472e961159 100644 --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb @@ -203,7 +203,13 @@ def merge_requests merge_requests = MergeRequestsFinder.new(current_user, issuable_params).execute unless default_project_filter - merge_requests = merge_requests.of_projects(project_ids_relation) + project_ids = if Feature.enabled?(:search_merge_requests_hide_archived_projects) && !filters[:include_archived] + project_ids_relation.non_archived + else + project_ids_relation + end + + merge_requests = merge_requests.of_projects(project_ids) end apply_sort(merge_requests, scope: 'merge_requests') diff --git a/spec/lib/gitlab/group_search_results_spec.rb b/spec/lib/gitlab/group_search_results_spec.rb index 1206a1c913194d93..071b303d77764992 100644 --- a/spec/lib/gitlab/group_search_results_spec.rb +++ b/spec/lib/gitlab/group_search_results_spec.rb @@ -32,8 +32,12 @@ end describe 'merge_requests search' do + let_it_be(:unarchived_project) { create(:project, :public, group: group) } + let_it_be(:archived_project) { create(:project, :public, :archived, 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_it_be(:unarchived_result) { create(:merge_request, source_project: unarchived_project, title: 'foo') } + let_it_be(:archived_result) { create(:merge_request, source_project: archived_project, title: 'foo') } let(:query) { 'foo' } let(:scope) { 'merge_requests' } @@ -44,6 +48,7 @@ end include_examples 'search results filtered by state' + include_examples 'search results filtered by archived', 'search_merge_requests_hide_archived_projects' end describe '#projects' do @@ -52,10 +57,10 @@ describe 'filtering' do let_it_be(:group) { create(:group) } - let_it_be(:unarchived_project) { create(:project, :public, group: group, name: 'Test1') } - let_it_be(:archived_project) { create(:project, :archived, :public, group: group, name: 'Test2') } + let_it_be(:unarchived_result) { create(:project, :public, group: group, name: 'Test1') } + let_it_be(:archived_result) { create(:project, :archived, :public, group: group, name: 'Test2') } - it_behaves_like 'search results filtered by archived' + it_behaves_like 'search results filtered by archived', 'search_projects_hide_archived' end end diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb index 662eab11cc0bd727..725b7901e6885f0a 100644 --- a/spec/lib/gitlab/search_results_spec.rb +++ b/spec/lib/gitlab/search_results_spec.rb @@ -187,11 +187,16 @@ end context 'filtering' do + let_it_be(:unarchived_project) { create(:project, :public) } + let_it_be(:archived_project) { create(:project, :public, :archived) } 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(:unarchived_result) { create(:merge_request, source_project: unarchived_project, title: 'foo unarchived') } + let(:archived_result) { create(:merge_request, source_project: archived_project, title: 'foo archived') } let(:query) { 'foo' } include_examples 'search results filtered by state' + include_examples 'search results filtered by archived', 'search_merge_requests_hide_archived_projects' end context 'ordering' do @@ -266,25 +271,10 @@ describe 'filtering' do let_it_be(:group) { create(:group) } - let_it_be(:unarchived_project) { create(:project, :public, group: group, name: 'Test1') } - let_it_be(:archived_project) { create(:project, :archived, :public, group: group, name: 'Test2') } + let_it_be(:unarchived_result) { create(:project, :public, group: group, name: 'Test1') } + let_it_be(:archived_result) { create(:project, :archived, :public, group: group, name: 'Test2') } - it_behaves_like 'search results filtered by archived' - - context 'when the search_projects_hide_archived feature flag is disabled' do - before do - stub_feature_flags(search_projects_hide_archived: false) - end - - context 'when filter not provided' do - let(:filters) { {} } - - it 'returns archived and unarchived results', :aggregate_failures do - expect(results.objects('projects')).to include unarchived_project - expect(results.objects('projects')).to include archived_project - end - end - end + it_behaves_like 'search results filtered by archived', 'search_projects_hide_archived' end end diff --git a/spec/support/shared_examples/lib/gitlab/search_archived_filter_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/search_archived_filter_shared_examples.rb index 7bcefd07fc4a787c..6b296d0e78afb426 100644 --- a/spec/support/shared_examples/lib/gitlab/search_archived_filter_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/search_archived_filter_shared_examples.rb @@ -1,30 +1,43 @@ # frozen_string_literal: true -RSpec.shared_examples 'search results filtered by archived' do +RSpec.shared_examples 'search results filtered by archived' do |feature_flag_name| context 'when filter not provided (all behavior)' do let(:filters) { {} } - it 'returns unarchived results only', :aggregate_failures do - expect(results.objects('projects')).to include unarchived_project - expect(results.objects('projects')).not_to include archived_project + it 'returns unarchived results only' do + expect(results.objects(scope)).to include unarchived_result + expect(results.objects(scope)).not_to include archived_result end end context 'when include_archived is true' do let(:filters) { { include_archived: true } } - it 'returns archived and unarchived results', :aggregate_failures do - expect(results.objects('projects')).to include unarchived_project - expect(results.objects('projects')).to include archived_project + it 'returns archived and unarchived results' do + expect(results.objects(scope)).to include unarchived_result + expect(results.objects(scope)).to include archived_result end end context 'when include_archived filter is false' do let(:filters) { { include_archived: false } } - it 'returns unarchived results only', :aggregate_failures do - expect(results.objects('projects')).to include unarchived_project - expect(results.objects('projects')).not_to include archived_project + it 'returns unarchived results only' do + expect(results.objects(scope)).to include unarchived_result + expect(results.objects(scope)).not_to include archived_result + end + end + + context "when the #{feature_flag_name} feature flag is disabled" do + let(:filters) { {} } + + before do + stub_feature_flags("#{feature_flag_name}": false) + end + + it 'returns archived and unarchived results' do + expect(results.objects(scope)).to include unarchived_result + expect(results.objects(scope)).to include archived_result end end end -- GitLab From a544a7e9ca277ce721f84cf2b8ec80d91515bb68 Mon Sep 17 00:00:00 2001 From: maddievn <mvanniekerk@gitlab.com> Date: Wed, 19 Jul 2023 15:08:22 +0200 Subject: [PATCH 2/3] Adds migration dictionary records for migrations --- .../20230710142700_add_archived_to_merge_requests.yml | 10 ++++++++++ ...30711140500_backfill_archived_on_merge_requests.yml | 10 ++++++++++ 2 files changed, 20 insertions(+) create mode 100644 ee/elastic/docs/20230710142700_add_archived_to_merge_requests.yml create mode 100644 ee/elastic/docs/20230711140500_backfill_archived_on_merge_requests.yml diff --git a/ee/elastic/docs/20230710142700_add_archived_to_merge_requests.yml b/ee/elastic/docs/20230710142700_add_archived_to_merge_requests.yml new file mode 100644 index 0000000000000000..ecfd84595f59c3a5 --- /dev/null +++ b/ee/elastic/docs/20230710142700_add_archived_to_merge_requests.yml @@ -0,0 +1,10 @@ +--- +name: AddArchivedToMergeRequests +version: '20230710142700' +description: Adds archvied (boolean) to the merge request index mapping +group: group::global search +milestone: 16.3 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/126024 +obsolete: false +marked_obsolete_by_url: +marked_obsolete_in_milestone: diff --git a/ee/elastic/docs/20230711140500_backfill_archived_on_merge_requests.yml b/ee/elastic/docs/20230711140500_backfill_archived_on_merge_requests.yml new file mode 100644 index 0000000000000000..ccf74e6a5a19ffdf --- /dev/null +++ b/ee/elastic/docs/20230711140500_backfill_archived_on_merge_requests.yml @@ -0,0 +1,10 @@ +--- +name: BackfillArchivedOnMergeRequests +version: '20230711140500' +description: Backfills archived for merge request documents +group: group::global search +milestone: 16.3 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/126024 +obsolete: false +marked_obsolete_by_url: +marked_obsolete_in_milestone: -- GitLab From 270babcee5d3097c76b3dc7c803363ff25a47fed Mon Sep 17 00:00:00 2001 From: Siddharth Dungarwal <sdungarwal@gitlab.com> Date: Wed, 19 Jul 2023 14:34:25 +0000 Subject: [PATCH 3/3] Apply 1 suggestion(s) to 1 file(s) --- .../search_merge_requests_hide_archived_projects.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/development/search_merge_requests_hide_archived_projects.yml b/config/feature_flags/development/search_merge_requests_hide_archived_projects.yml index a33d6605644d7e3f..565d32b71889dfe6 100644 --- a/config/feature_flags/development/search_merge_requests_hide_archived_projects.yml +++ b/config/feature_flags/development/search_merge_requests_hide_archived_projects.yml @@ -2,7 +2,7 @@ name: search_merge_requests_hide_archived_projects introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/126024 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/417595 -milestone: '16.2' +milestone: '16.3' type: development group: group::global search default_enabled: false -- GitLab