diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb
index 00fad9886ae88dc5941e3466a1dd88080408b098..4b6a6658bb938c30f144d4f3d07bdf923941d460 100644
--- a/ee/app/models/ee/project.rb
+++ b/ee/app/models/ee/project.rb
@@ -108,7 +108,6 @@ def lock_for_confirmation!(id)
has_one :security_orchestration_policy_configuration, class_name: 'Security::OrchestrationPolicyConfiguration', foreign_key: :project_id, inverse_of: :project
elastic_index_dependant_association :issues, on_change: :visibility_level
- elastic_index_dependant_association :notes, on_change: :visibility_level
scope :with_shared_runners_limit_enabled, -> do
if ::Ci::Runner.has_shared_runners_with_non_zero_public_cost?
diff --git a/ee/app/models/ee/project_feature.rb b/ee/app/models/ee/project_feature.rb
index 29eee25fc64ef46512b32d5c930e4a0aac79286f..ecf506b801e9823acfab469d915ece307f9a5617 100644
--- a/ee/app/models/ee/project_feature.rb
+++ b/ee/app/models/ee/project_feature.rb
@@ -5,7 +5,6 @@ module ProjectFeature
extend ActiveSupport::Concern
EE_FEATURES = %i(requirements security_and_compliance).freeze
- NOTES_PERMISSION_TRACKED_FIELDS = %w(issues_access_level repository_access_level merge_requests_access_level snippets_access_level).freeze
prepended do
set_available_features(EE_FEATURES)
@@ -15,11 +14,7 @@ module ProjectFeature
if project.maintaining_elasticsearch?
project.maintain_elasticsearch_update
- associations_to_update = []
- associations_to_update << 'issues' if elasticsearch_project_issues_need_updating?
- associations_to_update << 'notes' if elasticsearch_project_notes_need_updating?
-
- ElasticAssociationIndexerWorker.perform_async(self.project.class.name, project_id, associations_to_update) if associations_to_update.any?
+ ElasticAssociationIndexerWorker.perform_async(self.project.class.name, project_id, ['issues']) if elasticsearch_project_associations_need_updating?
end
end
@@ -28,11 +23,7 @@ module ProjectFeature
private
- def elasticsearch_project_notes_need_updating?
- self.previous_changes.keys.any? { |key| NOTES_PERMISSION_TRACKED_FIELDS.include?(key) }
- end
-
- def elasticsearch_project_issues_need_updating?
+ def elasticsearch_project_associations_need_updating?
self.previous_changes.key?(:issues_access_level)
end
end
diff --git a/ee/lib/elastic/instance_proxy_util.rb b/ee/lib/elastic/instance_proxy_util.rb
index eaf5be4e9c9da7ce64015a48dd78e3b760ab49f2..3f85185a84dfdb21e823a1dad2a4d22879c73df4 100644
--- a/ee/lib/elastic/instance_proxy_util.rb
+++ b/ee/lib/elastic/instance_proxy_util.rb
@@ -54,22 +54,6 @@ def safely_read_attribute_for_elasticsearch(attr_name)
nil
end
- # protect against missing project_feature and set visibility to PRIVATE
- # if the project_feature is missing on a project
- def safely_read_project_feature_for_elasticsearch(feature)
- if target.project.project_feature
- target.project.project_feature.access_level(feature)
- else
- logger.warn(
- message: 'Project is missing ProjectFeature',
- project_id: target.project_id,
- id: target.id,
- class: target.class
- )
- ProjectFeature::PRIVATE
- end
- end
-
def apply_field_limit(result)
return result unless result.is_a? String
diff --git a/ee/lib/elastic/latest/issue_instance_proxy.rb b/ee/lib/elastic/latest/issue_instance_proxy.rb
index 967483f071656bc687e718e8aaa7c64cabed5988..084cbff4a0398f0cb80934e104b938cee6b84786 100644
--- a/ee/lib/elastic/latest/issue_instance_proxy.rb
+++ b/ee/lib/elastic/latest/issue_instance_proxy.rb
@@ -14,7 +14,15 @@ def as_indexed_json(options = {})
data['assignee_id'] = safely_read_attribute_for_elasticsearch(:assignee_ids)
data['visibility_level'] = target.project.visibility_level
- data['issues_access_level'] = safely_read_project_feature_for_elasticsearch(:issues)
+
+ # protect against missing project_feature and set visibility to PRIVATE
+ # if the project_feature is missing on a project
+ begin
+ data['issues_access_level'] = target.project.project_feature.issues_access_level
+ rescue NoMethodError => e
+ Gitlab::ErrorTracking.track_exception(e, project_id: target.project_id, issue_id: target.id)
+ data['issues_access_level'] = ProjectFeature::PRIVATE
+ end
data.merge(generic_attributes)
end
diff --git a/ee/lib/elastic/latest/note_instance_proxy.rb b/ee/lib/elastic/latest/note_instance_proxy.rb
index 8a3a1371b80dad9d95dab9d6fdc14f9297acfcc3..42bbec9cc5c07c83ab8f34cfe7697107fca9f0f3 100644
--- a/ee/lib/elastic/latest/note_instance_proxy.rb
+++ b/ee/lib/elastic/latest/note_instance_proxy.rb
@@ -22,31 +22,8 @@ def as_indexed_json(options = {})
}
end
- # do not add the permission fields unless the `remove_permissions_data_from_notes_documents`
- # migration has completed otherwise the migration will never finish
- if Elastic::DataMigrationService.migration_has_finished?(:remove_permissions_data_from_notes_documents)
- data['visibility_level'] = target.project.visibility_level
- merge_project_feature_access_level(data, noteable)
- end
-
data.merge(generic_attributes)
end
-
- private
-
- def merge_project_feature_access_level(data, noteable)
- return unless noteable
-
- case noteable
- when Snippet
- data['snippets_access_level'] = safely_read_project_feature_for_elasticsearch(:snippets)
- when Commit
- data['repository_access_level'] = safely_read_project_feature_for_elasticsearch(:repository)
- else
- access_level_attribute = ProjectFeature.access_level_attribute(noteable)
- data[access_level_attribute.to_s] = safely_read_project_feature_for_elasticsearch(noteable)
- end
- end
end
end
end
diff --git a/ee/spec/elastic/migrate/20210127154600_remove_permissions_data_from_notes_documents_spec.rb b/ee/spec/elastic/migrate/20210127154600_remove_permissions_data_from_notes_documents_spec.rb
index 4be8d3e4723aa358d6b04baff1311b5cea0d01cb..07a673aa657ac5f3809fbaa556405a5259a76c06 100644
--- a/ee/spec/elastic/migrate/20210127154600_remove_permissions_data_from_notes_documents_spec.rb
+++ b/ee/spec/elastic/migrate/20210127154600_remove_permissions_data_from_notes_documents_spec.rb
@@ -11,13 +11,6 @@
before do
stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
allow(migration).to receive(:helper).and_return(helper)
-
- allow(Elastic::DataMigrationService).to receive(:migration_has_finished?).and_call_original
- # migrations are completed by default in test environments
- # required to prevent the `as_indexed_json` method from populating the permissions fields
- allow(Elastic::DataMigrationService).to receive(:migration_has_finished?)
- .with(:remove_permissions_data_from_notes_documents)
- .and_return(false)
end
describe 'migration_options' do
diff --git a/ee/spec/models/concerns/elastic/issue_spec.rb b/ee/spec/models/concerns/elastic/issue_spec.rb
index 99e53d6fcae6b6bbc56b8e0f2ab43a33042dbb1c..fc38ae2ba6a334eff5e70a975206ec6f981ae494 100644
--- a/ee/spec/models/concerns/elastic/issue_spec.rb
+++ b/ee/spec/models/concerns/elastic/issue_spec.rb
@@ -132,11 +132,12 @@
expect(issue.__elasticsearch__.as_indexed_json).to eq(expected_hash)
end
- it 'handles a project missing project_feature', :aggregate_failures do
+ it 'handles a project missing project_feature' do
issue = create :issue, project: project
allow(issue.project).to receive(:project_feature).and_return(nil)
- expect { issue.__elasticsearch__.as_indexed_json }.not_to raise_error
+ expect(Gitlab::ErrorTracking).to receive(:track_exception)
+
expect(issue.__elasticsearch__.as_indexed_json['issues_access_level']).to eq(ProjectFeature::PRIVATE)
end
diff --git a/ee/spec/models/concerns/elastic/note_spec.rb b/ee/spec/models/concerns/elastic/note_spec.rb
index 8e5b48bd749eb9458a5781d226b99d248b491e63..1abb3bbcd6d348695d80ae906326962f985e7538 100644
--- a/ee/spec/models/concerns/elastic/note_spec.rb
+++ b/ee/spec/models/concerns/elastic/note_spec.rb
@@ -85,80 +85,34 @@
expect(described_class.elastic_search('term', options: options).total_count).to eq(4)
end
- describe 'json serialization' do
- using RSpec::Parameterized::TableSyntax
-
- it "returns json with all needed elements" do
- assignee = create(:user)
- project = create(:project)
- issue = create(:issue, project: project, assignees: [assignee])
- note = create(:note, noteable: issue, project: project)
-
- expected_hash = note.attributes.extract!(
- 'id',
- 'note',
- 'project_id',
- 'noteable_type',
- 'noteable_id',
- 'created_at',
- 'updated_at',
- 'confidential'
- ).merge({
- 'issue' => {
- 'assignee_id' => issue.assignee_ids,
- 'author_id' => issue.author_id,
- 'confidential' => issue.confidential
- },
- 'type' => note.es_type,
- 'join_field' => {
- 'name' => note.es_type,
- 'parent' => note.es_parent
- },
- 'visibility_level' => project.visibility_level,
- 'issues_access_level' => project.issues_access_level
- })
-
- expect(note.__elasticsearch__.as_indexed_json).to eq(expected_hash)
- end
-
- it 'does not raise error for notes with null noteable references' do
- note = create(:note_on_issue)
- allow(note).to receive(:noteable).and_return(nil)
-
- expect { note.__elasticsearch__.as_indexed_json }.not_to raise_error
- end
-
- where(:note_type, :permission, :access_level) do
- :note_on_issue | ProjectFeature::ENABLED | 'issues_access_level'
- :note_on_project_snippet | ProjectFeature::DISABLED | 'snippets_access_level'
- :note_on_merge_request | ProjectFeature::PUBLIC | 'merge_requests_access_level'
- :note_on_commit | ProjectFeature::PRIVATE | 'repository_access_level'
- end
-
- with_them do
- let_it_be(:project) { create(:project, :repository) }
- let!(:note) { create(note_type, project: project) } # rubocop:disable Rails/SaveBang
- let(:note_json) { note.__elasticsearch__.as_indexed_json }
-
- before do
- project.project_feature.update_attribute(access_level.to_sym, permission)
- end
-
- it 'does not contain permissions if remove_permissions_data_from_notes_documents is not finished' do
- allow(Elastic::DataMigrationService).to receive(:migration_has_finished?)
- .with(:remove_permissions_data_from_notes_documents)
- .and_return(false)
-
- expect(note_json).not_to have_key(access_level)
- expect(note_json).not_to have_key('visibility_level')
- end
-
- it 'contains the correct permissions', :aggregate_failures do
- expect(note_json).to have_key(access_level)
- expect(note_json[access_level]).to eq(permission)
- expect(note_json).to have_key('visibility_level')
- end
- end
+ it "returns json with all needed elements" do
+ assignee = create(:user)
+ issue = create(:issue, assignees: [assignee])
+ note = create(:note, noteable: issue, project: issue.project)
+
+ expected_hash = note.attributes.extract!(
+ 'id',
+ 'note',
+ 'project_id',
+ 'noteable_type',
+ 'noteable_id',
+ 'created_at',
+ 'updated_at',
+ 'confidential'
+ ).merge({
+ 'issue' => {
+ 'assignee_id' => issue.assignee_ids,
+ 'author_id' => issue.author_id,
+ 'confidential' => issue.confidential
+ },
+ 'type' => note.es_type,
+ 'join_field' => {
+ 'name' => note.es_type,
+ 'parent' => note.es_parent
+ }
+ })
+
+ expect(note.__elasticsearch__.as_indexed_json).to eq(expected_hash)
end
it 'does not track system note updates' do
diff --git a/ee/spec/models/project_feature_spec.rb b/ee/spec/models/project_feature_spec.rb
index 0eff55b3746ec793019e8e6d2742d12ecb75524d..476a35b3231c3df07025dddf687c4081f9f79c53 100644
--- a/ee/spec/models/project_feature_spec.rb
+++ b/ee/spec/models/project_feature_spec.rb
@@ -28,14 +28,13 @@
allow(project).to receive(:maintaining_elasticsearch?).and_return(true)
end
- where(:feature, :worker_expected, :associations) do
- 'issues' | true | %w[issues notes]
- 'wiki' | false | nil
- 'builds' | false | nil
- 'merge_requests' | true | %w[notes]
- 'repository' | true | %w[notes]
- 'snippets' | true | %w[notes]
- 'pages' | false | nil
+ where(:feature, :worker_expected) do
+ 'issues' | true
+ 'wiki' | false
+ 'builds' | false
+ 'merge_requests' | false
+ 'repository' | false
+ 'pages' | false
end
with_them do
@@ -43,7 +42,7 @@
expect(project).to receive(:maintain_elasticsearch_update)
if worker_expected
- expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project.id, associations)
+ expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project.id, ['issues'])
else
expect(ElasticAssociationIndexerWorker).not_to receive(:perform_async)
end
diff --git a/ee/spec/models/project_spec.rb b/ee/spec/models/project_spec.rb
index 606116c305f8f3c9e39e027131cb7aa5d76ad80e..db2be09e8b95d9dd6afa4f85571e7a8c785b4665 100644
--- a/ee/spec/models/project_spec.rb
+++ b/ee/spec/models/project_spec.rb
@@ -2665,8 +2665,8 @@ def stub_default_url_options(host)
let!(:issue) { create(:issue, project: project) }
context 'when updating the visibility_level' do
- it 'triggers ElasticAssociationIndexerWorker to update issues and notes' do
- expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project.id, %w[issues notes])
+ it 'triggers ElasticAssociationIndexerWorker to update issues' do
+ expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project.id, ['issues'])
project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
end
diff --git a/ee/spec/services/groups/transfer_service_spec.rb b/ee/spec/services/groups/transfer_service_spec.rb
index f5041558010cb0c0eb2b3190bff426c7666cbbb0..edf88f439a0b270a3787003f8a0479cc36880916 100644
--- a/ee/spec/services/groups/transfer_service_spec.rb
+++ b/ee/spec/services/groups/transfer_service_spec.rb
@@ -67,11 +67,11 @@
expect(::Gitlab::CurrentSettings).not_to receive(:invalidate_elasticsearch_indexes_cache_for_project!)
expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project1)
- expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project1.id, %w[issues notes])
+ expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project1.id, ['issues'])
expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project2)
- expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project2.id, %w[issues notes])
+ expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project2.id, ['issues'])
expect(Elastic::ProcessBookkeepingService).not_to receive(:track!).with(project3)
- expect(ElasticAssociationIndexerWorker).not_to receive(:perform_async).with('Project', project3.id, %w[issues notes])
+ expect(ElasticAssociationIndexerWorker).not_to receive(:perform_async).with('Project', project3.id, ['issues'])
transfer_service.execute(new_group)
diff --git a/ee/spec/services/projects/transfer_service_spec.rb b/ee/spec/services/projects/transfer_service_spec.rb
index 90d7d5c4541d22762e7c08dc975543fb7d68d578..c171d6f825cd7bca9a4ac4617c29ba0a1f16e147 100644
--- a/ee/spec/services/projects/transfer_service_spec.rb
+++ b/ee/spec/services/projects/transfer_service_spec.rb
@@ -72,9 +72,9 @@
context 'when visibility level changes' do
let_it_be(:group) { create(:group, :private) }
- it 'reindexes the project and associated issues and notes' do
+ it 'reindexes the project and associated issues' do
expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project)
- expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project.id, %w[issues notes])
+ expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project.id, ['issues'])
subject.execute(group)
end