From 2505a4efc46d3c6d3dddd4903fe07f538fffd882 Mon Sep 17 00:00:00 2001 From: Siddharth Dungarwal <sdungarwal@gitlab.com> Date: Mon, 11 Nov 2024 16:42:28 +0000 Subject: [PATCH] Merge branch 'tchu-fix-work-items-visibility-level-updates' into 'master' Index work items when project visibility level changes See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/171978 Merged-by: Siddharth Dungarwal <sdungarwal@gitlab.com> Approved-by: Siddharth Dungarwal <sdungarwal@gitlab.com> Co-authored-by: Terri Chu <tchu@gitlab.com> --- ee/app/models/ee/project.rb | 1 + ee/lib/search/elastic/references/work_item.rb | 6 ++++-- .../lib/search/elastic/references/work_item_spec.rb | 6 ------ ee/spec/models/ee/project_spec.rb | 9 +++++++-- ee/spec/services/search/global_service_spec.rb | 13 ++++++++----- ee/spec/services/search/group_service_spec.rb | 2 +- ee/spec/services/search/project_service_spec.rb | 13 ++++++++----- 7 files changed, 29 insertions(+), 21 deletions(-) diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 8a455f0e8f99e852..7c508ddd068e040e 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -208,6 +208,7 @@ def lock_for_confirmation!(id) elastic_index_dependant_association :issues, on_change: :visibility_level elastic_index_dependant_association :issues, on_change: :archived + elastic_index_dependant_association :work_items, on_change: :visibility_level elastic_index_dependant_association :work_items, on_change: :archived elastic_index_dependant_association :merge_requests, on_change: :visibility_level elastic_index_dependant_association :merge_requests, on_change: :archived diff --git a/ee/lib/search/elastic/references/work_item.rb b/ee/lib/search/elastic/references/work_item.rb index 26ed10c2053c4230..5cfb33ec0fc1ee2a 100644 --- a/ee/lib/search/elastic/references/work_item.rb +++ b/ee/lib/search/elastic/references/work_item.rb @@ -102,9 +102,11 @@ def build_indexed_json(target) data['hashed_root_namespace_id'] = target.namespace.hashed_root_namespace_id data['work_item_type_id'] = target.work_item_type_id data['upvotes'] = target.upvotes_count - data['namespace_visibility_level'] = target.namespace.visibility_level - data['namespace_id'] = target.namespace_id if target.namespace.group_namespace? + if target.namespace.group_namespace? + data['namespace_visibility_level'] = target.namespace.visibility_level + data['namespace_id'] = target.namespace_id + end if target.project.present? data['archived'] = target.project.archived? diff --git a/ee/spec/lib/search/elastic/references/work_item_spec.rb b/ee/spec/lib/search/elastic/references/work_item_spec.rb index 84fee47d10a33f6e..6a598042ba1b7d5b 100644 --- a/ee/spec/lib/search/elastic/references/work_item_spec.rb +++ b/ee/spec/lib/search/elastic/references/work_item_spec.rb @@ -43,7 +43,6 @@ due_date: project_work_item.due_date, traversal_ids: project_work_item.namespace.elastic_namespace_ancestry, hashed_root_namespace_id: project_work_item.namespace.hashed_root_namespace_id, - namespace_visibility_level: project_work_item.namespace.visibility_level, schema_version: described_class::SCHEMA_VERSION, archived: project.archived?, project_visibility_level: project.visibility_level, @@ -80,7 +79,6 @@ due_date: project_work_item.due_date, traversal_ids: project_work_item.namespace.elastic_namespace_ancestry, hashed_root_namespace_id: project_work_item.namespace.hashed_root_namespace_id, - namespace_visibility_level: project_work_item.namespace.visibility_level, schema_version: described_class::SCHEMA_VERSION, archived: project.archived?, project_visibility_level: project.visibility_level, @@ -118,7 +116,6 @@ due_date: project_work_item.due_date, traversal_ids: project_work_item.namespace.elastic_namespace_ancestry, hashed_root_namespace_id: project_work_item.namespace.hashed_root_namespace_id, - namespace_visibility_level: project_work_item.namespace.visibility_level, schema_version: described_class::SCHEMA_VERSION, archived: project.archived?, project_visibility_level: project.visibility_level, @@ -148,7 +145,6 @@ due_date: user_work_item.due_date, traversal_ids: "#{user_work_item.namespace.id}-", hashed_root_namespace_id: user_work_item.namespace.hashed_root_namespace_id, - namespace_visibility_level: user_work_item.namespace.visibility_level, schema_version: described_class::SCHEMA_VERSION, type: 'work_item' ) @@ -210,7 +206,6 @@ due_date: project_work_item.due_date, traversal_ids: project_work_item.namespace.elastic_namespace_ancestry, hashed_root_namespace_id: project_work_item.namespace.hashed_root_namespace_id, - namespace_visibility_level: project_work_item.namespace.visibility_level, schema_version: described_class::SCHEMA_VERSION, archived: project.archived?, project_visibility_level: project.visibility_level, @@ -269,7 +264,6 @@ due_date: user_work_item.due_date, traversal_ids: "#{user_work_item.namespace.id}-", hashed_root_namespace_id: user_work_item.namespace.hashed_root_namespace_id, - namespace_visibility_level: user_work_item.namespace.visibility_level, schema_version: described_class::SCHEMA_VERSION, type: 'work_item' ) diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index 7d33d0032bdfcd36..e6432f857198d21e 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -824,6 +824,10 @@ association_name: :work_items, on_change: :archived }, + { + association_name: :work_items, + on_change: :visibility_level + }, { association_name: :merge_requests, on_change: :visibility_level @@ -3689,8 +3693,9 @@ def stub_default_url_options(host) let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } context 'when updating the visibility_level' do - it 'triggers ElasticAssociationIndexerWorker to update issues, merge_requests and notes' do - expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project.id, %w[issues merge_requests notes milestones]) + it 'triggers ElasticAssociationIndexerWorker to update associations' do + expect(ElasticAssociationIndexerWorker).to receive(:perform_async) + .with('Project', project.id, %w[issues work_items merge_requests notes milestones]) project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) end diff --git a/ee/spec/services/search/global_service_spec.rb b/ee/spec/services/search/global_service_spec.rb index 946659053ecd5de7..af8e5d33d1b6d91e 100644 --- a/ee/spec/services/search/global_service_spec.rb +++ b/ee/spec/services/search/global_service_spec.rb @@ -174,7 +174,7 @@ end end - context 'on issue' do + context 'on issue', :sidekiq_inline do # sidekiq needed for ElasticAssociationIndexerWorker let(:scope) { 'issues' } [:work_item, :issue].each do |document_type| @@ -182,15 +182,18 @@ let!(:work_item) { create document_type, project: project } let(:search) { work_item.title } - before do - stub_feature_flags(search_issues_uses_work_items_index: (document_type == :work_item)) - end - where(:project_level, :feature_access_level, :membership, :admin_mode, :expected_count) do permission_table_for_guest_feature_access end with_them do + before do + stub_feature_flags(search_issues_uses_work_items_index: (document_type == :work_item)) + + Elastic::ProcessInitialBookkeepingService.track!(work_item) + ensure_elasticsearch_index! + end + it_behaves_like 'search respects visibility' end end diff --git a/ee/spec/services/search/group_service_spec.rb b/ee/spec/services/search/group_service_spec.rb index 1f4150bc959dcec2..a1c1204ccbe6e5b5 100644 --- a/ee/spec/services/search/group_service_spec.rb +++ b/ee/spec/services/search/group_service_spec.rb @@ -439,7 +439,7 @@ end end - context 'for issues' do + context 'for issues', :sidekiq_inline do # sidekiq needed for ElasticAssociationIndexerWorker let(:scope) { 'issues' } [:work_item, :issue].each do |document_type| diff --git a/ee/spec/services/search/project_service_spec.rb b/ee/spec/services/search/project_service_spec.rb index a4a1419e58bbe0ac..79f56591eab6c999 100644 --- a/ee/spec/services/search/project_service_spec.rb +++ b/ee/spec/services/search/project_service_spec.rb @@ -423,7 +423,7 @@ end end - context 'for issue' do + context 'for issues', :sidekiq_inline do # sidekiq needed for ElasticAssociationIndexerWorker let(:scope) { 'issues' } [:work_item, :issue].each do |document_type| @@ -432,15 +432,18 @@ let!(:issue2) { create document_type, project: project2, title: issue.title } let(:search) { issue.title } - before do - stub_feature_flags(search_issues_uses_work_items_index: (document_type == :work_item)) - end - where(:project_level, :feature_access_level, :membership, :admin_mode, :expected_count) do permission_table_for_guest_feature_access end with_them do + before do + stub_feature_flags(search_issues_uses_work_items_index: (document_type == :work_item)) + + Elastic::ProcessInitialBookkeepingService.track!(issue, issue2) + ensure_elasticsearch_index! + end + it_behaves_like 'search respects visibility' end end -- GitLab