From 174ed50ce651fc8dc7b12efcc71328d82d5c003e Mon Sep 17 00:00:00 2001 From: CatalinVoineag <11318084+CatalinVoineag@users.noreply.github.com> Date: Fri, 20 Sep 2024 11:52:05 +0100 Subject: [PATCH] Change work_histories migration to insert via SQL Previously, the data migration BackfillApplicationChoicesWithWorkExperiences was inserting records via ActiveRecord. This caused an incident, known as 'The Big Touch'. A lot of application choices, forms and candidates had their updated_at updated and this caused some of our APIs to be slow. We still need to run this migration in sandbox so we thought it's a good idea to change this migration to insert records via SQL avoiding any callbacks. --- .../migrate_application_choices_worker.rb | 63 ++++++++++++------- ...migrate_application_choices_worker_spec.rb | 34 ++++++---- 2 files changed, 62 insertions(+), 35 deletions(-) diff --git a/app/workers/migrate_application_choices_worker.rb b/app/workers/migrate_application_choices_worker.rb index a22303487c5..84004021346 100644 --- a/app/workers/migrate_application_choices_worker.rb +++ b/app/workers/migrate_application_choices_worker.rb @@ -4,36 +4,55 @@ class MigrateApplicationChoicesWorker sidekiq_options queue: :low_priority def perform(choice_ids) - errors = [] + return if HostingEnvironment.production? - ApplicationChoice.where(id: choice_ids).find_each(batch_size: 100) do |choice| - ActiveRecord::Base.transaction do - application_form = choice.application_form + application_experiences = [] + work_history_breaks = [] - if choice.work_experiences.blank? && application_form.application_work_experiences.any? - choice.work_experiences = application_form.application_work_experiences.map(&:dup) - end + ApplicationChoice.where(id: choice_ids).find_each(batch_size: 100) do |choice| + application_form = choice.application_form + + if choice.work_experiences.blank? && application_form.application_work_experiences.any? + application_experiences += valid_attributes( + application_form.application_work_experiences.map(&:dup), + choice, + 'experienceable', + ) + end - if choice.volunteering_experiences.blank? && application_form.application_volunteering_experiences.any? - choice.volunteering_experiences = application_form.application_volunteering_experiences.map(&:dup) - end + if choice.volunteering_experiences.blank? && application_form.application_volunteering_experiences.any? + application_experiences += valid_attributes( + application_form.application_volunteering_experiences.map(&:dup), + choice, + 'experienceable', + ) + end - if choice.work_history_breaks.blank? && application_form.application_work_history_breaks.any? - choice.work_history_breaks = application_form.application_work_history_breaks.map(&:dup) - end + if choice.work_history_breaks.blank? && application_form.application_work_history_breaks.any? + work_history_breaks += valid_attributes( + application_form.application_work_history_breaks.map(&:dup), + choice, + 'breakable', + ) end - rescue ActiveRecord::RecordInvalid => e - errors << "Error choice id #{choice.id}: #{e.message}" end - if errors - errors.each do |error| - Rails.logger.info error - end + ApplicationExperience.insert_all(application_experiences) if application_experiences.any? + ApplicationWorkHistoryBreak.insert_all(work_history_breaks) if work_history_breaks.any? + end - Rails.logger.info "#{errors.count} errors" - end +private - Rails.logger.info 'No errors' if errors.blank? + def valid_attributes(records, choice, polymorphic_column) + records.map do |record| + attributes = record.attributes + attributes["#{polymorphic_column}_type"] = 'ApplicationChoice' + attributes["#{polymorphic_column}_id"] = choice.id + attributes['created_at'] = Time.zone.now + attributes['updated_at'] = Time.zone.now + attributes.delete('id') + + attributes + end end end diff --git a/spec/workers/migrate_application_choices_worker_spec.rb b/spec/workers/migrate_application_choices_worker_spec.rb index c8a09c35021..17b1ed8e25c 100644 --- a/spec/workers/migrate_application_choices_worker_spec.rb +++ b/spec/workers/migrate_application_choices_worker_spec.rb @@ -2,7 +2,11 @@ RSpec.describe MigrateApplicationChoicesWorker do describe '#perform' do - it 'dups the working experiences and histories from application_form to choice' do + before do + TestSuiteTimeMachine.unfreeze! + end + + it 'dups the working experiences and histories from application_form to choice', :with_audited do application_form = create( :completed_application_form, volunteering_experiences_count: 1, @@ -31,7 +35,6 @@ breakable: choice_with_data_migrated, ) choice_ids = [choice.id, choice_with_data_migrated.id] - allow(Rails.logger).to receive(:info) expect { described_class.new.perform(choice_ids) @@ -41,27 +44,32 @@ .and not_change(choice_with_data_migrated.work_experiences, :count) .and not_change(choice_with_data_migrated.volunteering_experiences, :count) .and not_change(choice_with_data_migrated.work_history_breaks, :count) - expect(Rails.logger).to have_received(:info).with('No errors') + .and not_change(choice.own_and_associated_audits, :count) + .and not_change(choice.reload, :updated_at) + .and not_change(application_form.reload, :updated_at) + .and not_change(application_form.candidate, :updated_at) end - context 'with errors' do - it 'rescues ActiveRecord::RecordInvalid exceptions and logs the errors' do + context 'when env is production' do + it 'does not create work histories' do application_form = create( :completed_application_form, volunteering_experiences_count: 1, full_work_history: true, ) choice = create(:application_choice, application_form:) - choice_ids = [choice.id] - # rubocop:disable RSpec/AnyInstance - allow_any_instance_of(ApplicationChoice).to receive(:work_experiences=) - .and_raise(ActiveRecord::RecordInvalid) - # rubocop:enable RSpec/AnyInstance - allow(Rails.logger).to receive(:info) + allow(HostingEnvironment).to receive(:production?).and_return(true) - described_class.new.perform(choice_ids) - expect(Rails.logger).to have_received(:info).with("Error choice id #{choice.id}: Record invalid") + expect { + described_class.new.perform([choice.id]) + }.to not_change(choice.work_experiences, :count) + .and not_change(choice.volunteering_experiences, :count) + .and not_change(choice.work_history_breaks, :count) + .and not_change(choice.own_and_associated_audits, :count) + .and not_change(choice.reload, :updated_at) + .and not_change(application_form.reload, :updated_at) + .and not_change(application_form.candidate, :updated_at) end end end