Commit 8d5a9a37 authored by Dylan Griffith's avatar Dylan Griffith

Correctly handle indexing issues when transferring group

Previously transferring groups used custom code to trigger indexing
after the projects were updated. But this did not account for checking
visibility_level changes. As such we need to pass through the changed
attributes to maintain_elasticsearch_update when calling it directly so
we can determine if we need to also index the associated issues again.
parent b78c8dda
...@@ -48,10 +48,11 @@ module Elastic ...@@ -48,10 +48,11 @@ module Elastic
::Elastic::ProcessBookkeepingService.track!(self) ::Elastic::ProcessBookkeepingService.track!(self)
end end
def maintain_elasticsearch_update def maintain_elasticsearch_update(updated_attributes: previous_changes.keys)
updated_attributes = updated_attributes.map(&:to_s) # Allow caller to provide symbols but keep consistent to using strings
::Elastic::ProcessBookkeepingService.track!(self) ::Elastic::ProcessBookkeepingService.track!(self)
associations_to_update = associations_needing_elasticsearch_update associations_to_update = associations_needing_elasticsearch_update(updated_attributes)
if associations_to_update.present? if associations_to_update.present?
ElasticAssociationIndexerWorker.perform_async(self.class.name, id, associations_to_update) ElasticAssociationIndexerWorker.perform_async(self.class.name, id, associations_to_update)
end end
...@@ -63,12 +64,12 @@ module Elastic ...@@ -63,12 +64,12 @@ module Elastic
# Override in child object if there are associations that need to be # Override in child object if there are associations that need to be
# updated when specific fields are updated # updated when specific fields are updated
def associations_needing_elasticsearch_update def associations_needing_elasticsearch_update(updated_attributes)
self.class.elastic_index_dependants.map do |dependant| self.class.elastic_index_dependants.map do |dependant|
association_name = dependant[:association_name] association_name = dependant[:association_name]
on_change = dependant[:on_change] on_change = dependant[:on_change]
next nil unless previous_changes.include?(on_change) next nil unless updated_attributes.include?(on_change.to_s)
association_name.to_s association_name.to_s
end.compact.uniq end.compact.uniq
......
...@@ -16,7 +16,7 @@ module EE ...@@ -16,7 +16,7 @@ module EE
override :post_update_hooks override :post_update_hooks
def post_update_hooks(updated_project_ids) def post_update_hooks(updated_project_ids)
::Project.id_in(updated_project_ids).find_each do |project| ::Project.id_in(updated_project_ids).find_each do |project|
project.maintain_elasticsearch_update if project.maintaining_elasticsearch? project.maintain_elasticsearch_update(updated_attributes: [:visibility_level]) if project.maintaining_elasticsearch?
end end
super super
end end
......
...@@ -21,14 +21,17 @@ RSpec.describe Groups::TransferService, '#execute' do ...@@ -21,14 +21,17 @@ RSpec.describe Groups::TransferService, '#execute' do
stub_ee_application_setting(elasticsearch_indexing: true) stub_ee_application_setting(elasticsearch_indexing: true)
end end
it 'reindexes projects', :elastic do it 'reindexes projects and associated issues', :elastic do
project1 = create(:project, :repository, :public, namespace: group) project1 = create(:project, :repository, :public, namespace: group)
project2 = create(:project, :repository, :public, namespace: group) project2 = create(:project, :repository, :public, namespace: group)
project3 = create(:project, :repository, :private, namespace: group) project3 = create(:project, :repository, :private, namespace: group)
expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project1) expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project1)
expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project1.id, ['issues'])
expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project2) expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project2)
expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project2.id, ['issues'])
expect(Elastic::ProcessBookkeepingService).not_to receive(:track!).with(project3) expect(Elastic::ProcessBookkeepingService).not_to receive(:track!).with(project3)
expect(ElasticAssociationIndexerWorker).not_to receive(:perform_async).with('Project', project3.id, ['issues'])
transfer_service.execute(new_group) transfer_service.execute(new_group)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment