From cc6c34aa8800a8c7d66cb41a63db62bbe2db1b03 Mon Sep 17 00:00:00 2001 From: Terri Chu <tchu@gitlab.com> Date: Wed, 5 Jun 2024 21:48:43 +0000 Subject: [PATCH] Mark 20230426195404 as obsolete This migration marks the 20230426195404 AddHiddenToMergeRequests Advanced search migration as obsolete. This MR will still need changes to remove references to the migration in the code. At the moment the `gitlab-housekeeper` is not always capable of removing all references so you must check the diff and pipeline failures to confirm if there are any issues. It is the responsibility of the assignee (picked from ~"group::global search") to push those changes to this branch. You can read more about the process for marking Advanced search migrations as obsolete in https://docs.gitlab.com/ee/development/search/advanced_search_migration_styleguide.html#deleting-advanced-search-migrations-in-a-major-version-upgrade. As part of our process we want to ensure all Advanced search migrations have had at least one [required stop](https://docs.gitlab.com/ee/development/database/required_stops.html) to process the migration. Therefore we can mark any Advanced search migrations added before the last required stop as obsolete. This change was generated by [gitlab-housekeeper](https://gitlab.com/gitlab-org/gitlab/-/tree/master/gems/gitlab-housekeeper) using the Keeps::MarkOldAdvancedSearchMigrationsAsObsolete keep. To provide feedback on your experience with `gitlab-housekeeper` please comment in <https://gitlab.com/gitlab-org/gitlab/-/issues/442003>. Changelog: other EE: true --- ee/app/models/ee/users/banned_user.rb | 8 +-- ...426195404_add_hidden_to_merge_requests.yml | 6 +-- ...0426195404_add_hidden_to_merge_requests.rb | 2 + .../latest/merge_request_instance_proxy.rb | 8 ++- ...95404_add_hidden_to_merge_requests_spec.rb | 6 +-- .../concerns/elastic/merge_request_spec.rb | 5 -- ee/spec/models/ee/users/banned_user_spec.rb | 51 +++++++------------ 7 files changed, 30 insertions(+), 56 deletions(-) diff --git a/ee/app/models/ee/users/banned_user.rb b/ee/app/models/ee/users/banned_user.rb index e1c489ce6457..838de1bfef10 100644 --- a/ee/app/models/ee/users/banned_user.rb +++ b/ee/app/models/ee/users/banned_user.rb @@ -12,13 +12,7 @@ module BannedUser private def reindex_issues_and_merge_requests - associations = [:issues] - - if ::Elastic::DataMigrationService.migration_has_finished?(:add_hidden_to_merge_requests) - associations << :merge_requests - end - - ElasticAssociationIndexerWorker.perform_async(user.class.name, user.id, associations) + ElasticAssociationIndexerWorker.perform_async(user.class.name, user.id, %i[issues merge_requests]) end end end diff --git a/ee/elastic/docs/20230426195404_add_hidden_to_merge_requests.yml b/ee/elastic/docs/20230426195404_add_hidden_to_merge_requests.yml index 4dddf933f924..a8b11c066991 100644 --- a/ee/elastic/docs/20230426195404_add_hidden_to_merge_requests.yml +++ b/ee/elastic/docs/20230426195404_add_hidden_to_merge_requests.yml @@ -5,6 +5,6 @@ description: AddHiddenToMergeRequests group: group::global search milestone: '16.0' introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/118842 -obsolete: false -marked_obsolete_by_url: -marked_obsolete_in_milestone: +obsolete: true +marked_obsolete_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/152977 +marked_obsolete_in_milestone: '17.1' diff --git a/ee/elastic/migrate/20230426195404_add_hidden_to_merge_requests.rb b/ee/elastic/migrate/20230426195404_add_hidden_to_merge_requests.rb index 729c11c3b008..0a86499e3ba0 100644 --- a/ee/elastic/migrate/20230426195404_add_hidden_to_merge_requests.rb +++ b/ee/elastic/migrate/20230426195404_add_hidden_to_merge_requests.rb @@ -15,3 +15,5 @@ def new_mappings } end end + +AddHiddenToMergeRequests.prepend ::Elastic::MigrationObsolete diff --git a/ee/lib/elastic/latest/merge_request_instance_proxy.rb b/ee/lib/elastic/latest/merge_request_instance_proxy.rb index 7f9a031ce8ca..d7490902e4a2 100644 --- a/ee/lib/elastic/latest/merge_request_instance_proxy.rb +++ b/ee/lib/elastic/latest/merge_request_instance_proxy.rb @@ -32,11 +32,9 @@ def as_indexed_json(options = {}) data['merge_requests_access_level'] = safely_read_project_feature_for_elasticsearch(:merge_requests) data['hashed_root_namespace_id'] = target_project.namespace.hashed_root_namespace_id - if ::Elastic::DataMigrationService.migration_has_finished?(:add_hidden_to_merge_requests) - # Use target.hidden? once the FF hide_merge_requests_from_banned_users is fully rolled out - # https://gitlab.com/gitlab-org/gitlab/-/issues/410671 - data['hidden'] = target.author&.banned? - end + # Use target.hidden? once the FF hide_merge_requests_from_banned_users is fully rolled out + # https://gitlab.com/gitlab-org/gitlab/-/issues/410671 + data['hidden'] = target.author&.banned? if ::Elastic::DataMigrationService.migration_has_finished?(:add_archived_to_merge_requests) data['archived'] = target.project.archived? diff --git a/ee/spec/elastic/migrate/20230426195404_add_hidden_to_merge_requests_spec.rb b/ee/spec/elastic/migrate/20230426195404_add_hidden_to_merge_requests_spec.rb index e83b48d4fc57..91b207b99cc8 100644 --- a/ee/spec/elastic/migrate/20230426195404_add_hidden_to_merge_requests_spec.rb +++ b/ee/spec/elastic/migrate/20230426195404_add_hidden_to_merge_requests_spec.rb @@ -3,8 +3,6 @@ require 'spec_helper' require File.expand_path('ee/elastic/migrate/20230426195404_add_hidden_to_merge_requests.rb') -RSpec.describe AddHiddenToMergeRequests, :elastic, :sidekiq_inline, feature_category: :global_search do - let(:version) { 20230426195404 } - - include_examples 'migration adds mapping' +RSpec.describe AddHiddenToMergeRequests, feature_category: :global_search do + it_behaves_like 'a deprecated Advanced Search migration', 20230426195404 end diff --git a/ee/spec/models/concerns/elastic/merge_request_spec.rb b/ee/spec/models/concerns/elastic/merge_request_spec.rb index 7cf4d937f42a..f667255a7197 100644 --- a/ee/spec/models/concerns/elastic/merge_request_spec.rb +++ b/ee/spec/models/concerns/elastic/merge_request_spec.rb @@ -91,11 +91,6 @@ merge_request.project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) end - 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.except('hidden', 'archived', 'schema_version')) - 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', 'schema_version')) diff --git a/ee/spec/models/ee/users/banned_user_spec.rb b/ee/spec/models/ee/users/banned_user_spec.rb index b39ac7d76b0f..6adf1e6f0160 100644 --- a/ee/spec/models/ee/users/banned_user_spec.rb +++ b/ee/spec/models/ee/users/banned_user_spec.rb @@ -16,49 +16,36 @@ end it 'calls reindex_issues on create' do - expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with(user.class.name, user.id, [:issues]) + expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with(user.class.name, user.id, + %i[issues merge_requests]) banned_user end it 'calls reindex_issues on destroy' do banned_user - expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with(user.class.name, user.id, [:issues]) + expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with(user.class.name, user.id, + [:issues, :merge_requests]) banned_user.destroy! end - context 'when add_hidden_to_merge_requests migration is not finished' do - it 'does not call reindex on merge_requests association' do - set_elasticsearch_migration_to :add_hidden_to_merge_requests, including: false - expect(ElasticAssociationIndexerWorker).not_to receive(:perform_async).with(user.class.name, user.id, - array_including(:merge_requests)) - banned_user - end + it 'does not call reindex on merge_requests association for update' do + banned_user + expect(ElasticAssociationIndexerWorker).not_to receive(:perform_async).with(user.class.name, user.id, + array_including(:merge_requests)) + banned_user.touch end - context 'when add_hidden_to_merge_requests migration is finished' do - before do - set_elasticsearch_migration_to :add_hidden_to_merge_requests, including: true - end - - it 'does not call reindex on merge_requests association for update' do - banned_user - expect(ElasticAssociationIndexerWorker).not_to receive(:perform_async).with(user.class.name, user.id, - array_including(:merge_requests)) - banned_user.touch - end - - it 'calls reindex on merge_requests association for create' do - expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with(user.class.name, user.id, - array_including(:merge_requests)) - banned_user - end + it 'calls reindex on merge_requests association for create' do + expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with(user.class.name, user.id, + array_including(:merge_requests)) + banned_user + end - it 'calls reindex on merge_requests association for destroy' do - banned_user - expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with(user.class.name, user.id, - array_including(:merge_requests)) - banned_user.destroy! - end + it 'calls reindex on merge_requests association for destroy' do + banned_user + expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with(user.class.name, user.id, + array_including(:merge_requests)) + banned_user.destroy! end end end -- GitLab