diff --git a/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb b/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb index 18d37251e..dc5a02c29 100644 --- a/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb +++ b/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb @@ -25,9 +25,16 @@ class PieceJustificativeToChampPieceJointeMigrationService populate_champs_pjs!(procedure, types_de_champ_pj, &progress) # Only destroy the old types PJ once everything has been safely migrated to - # champs PJs. Destroying the types PJ will cascade and destroy the PJs, - # and delete the linked objects from remote storage. This means that no other - # cleanup action is required. + # champs PJs. + + # First destroy the individual PJ champs on all dossiers. + # It will cascade and destroy the PJs, and delete the linked objects from remote storage. + procedure.dossiers.unscope(where: :hidden_at).includes(:champs).find_each do |dossier| + destroy_pieces_justificatives(dossier) + end + + # Now we can destroy the type de champ themselves, + # without cascading the timestamp update on all attached dossiers. procedure.types_de_piece_justificative.destroy_all end @@ -40,32 +47,7 @@ class PieceJustificativeToChampPieceJointeMigrationService # Unscope to make sure all dossiers are migrated, even the soft-deleted ones procedure.dossiers.unscope(where: :hidden_at).includes(:champs).find_each do |dossier| - # Add the new pieces justificatives champs to the dossier - champs_pj = types_de_champ_pj.map(&:build_champ) - dossier.champs += champs_pj - - # Copy the dossier old pieces jointes to the new champs - # (even if the champs already existed, so that we ensure a clean state) - champs_pj.each do |champ| - type_pj_id = champ.type_de_champ.old_pj&.fetch('stable_id', nil) - pj = dossier.retrieve_last_piece_justificative_by_type(type_pj_id) - - if pj.present? - convert_pj_to_champ!(pj, champ) - - champ.update( - updated_at: pj.updated_at, - created_at: pj.created_at - ) - else - champ.update( - updated_at: dossier.updated_at, - created_at: dossier.created_at - ) - end - - yield if block_given? - end + migrate_dossier!(dossier, types_de_champ_pj, &progress) end rescue StandardError, SignalException @@ -74,8 +56,11 @@ class PieceJustificativeToChampPieceJointeMigrationService rake_puts "Error received. Rolling back migration of procedure #{procedure.id}…" types_de_champ_pj.each do |type_champ| - type_champ.champ.each { |c| c.piece_justificative_file.purge } - type_champ.destroy + # First destroy all the individual champs on dossiers + type_champ.champ.each { |c| destroy_champ_pj(c.dossier.reload, c) } + # Now we can destroy the type de champ itself, + # without cascading the timestamp update on all attached dossiers. + type_champ.reload.destroy end rake_puts "Migration of procedure #{procedure.id} rolled back." @@ -84,6 +69,39 @@ class PieceJustificativeToChampPieceJointeMigrationService raise end + def migrate_dossier!(dossier, types_de_champ_pj) + # Add the new pieces justificatives champs to the dossier + champs_pj = types_de_champ_pj.map(&:build_champ) + preserving_updated_at(dossier) do + dossier.champs += champs_pj + end + + # Copy the dossier old pieces jointes to the new champs + # (even if the champs already existed, so that we ensure a clean state) + champs_pj.each do |champ| + type_pj_id = champ.type_de_champ.old_pj&.fetch('stable_id', nil) + pj = dossier.retrieve_last_piece_justificative_by_type(type_pj_id) + + if pj.present? + preserving_updated_at(dossier) do + convert_pj_to_champ!(pj, champ) + end + + champ.update_columns( + updated_at: pj.updated_at, + created_at: pj.created_at + ) + else + champ.update_columns( + updated_at: dossier.updated_at, + created_at: dossier.created_at + ) + end + + yield if block_given? + end + end + def convert_pj_to_champ!(pj, champ) actual_file_exists = pj.content.file.send(:file) if actual_file_exists @@ -133,4 +151,26 @@ class PieceJustificativeToChampPieceJointeMigrationService def make_empty_blob(pj) storage_service.make_empty_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename) end + + def preserving_updated_at(model) + original_modification_date = model.updated_at + begin + yield + ensure + model.update_column(:updated_at, original_modification_date) + end + end + + def destroy_pieces_justificatives(dossier) + preserving_updated_at(dossier) do + dossier.pieces_justificatives.destroy_all + end + end + + def destroy_champ_pj(dossier, champ) + preserving_updated_at(dossier) do + champ.piece_justificative_file.purge + champ.destroy + end + end end diff --git a/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb b/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb index 86ea98b7f..b1e96df09 100644 --- a/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb +++ b/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb @@ -26,6 +26,18 @@ describe PieceJustificativeToChampPieceJointeMigrationService do end end + def timestamps(dossier) + # Reload dossier because the resolution of in-database timestamps is + # different from the resolution of in-memory timestamps, causing the + # tests to fail on fractional time differences. + dossier.reload + + { + created_at: dossier.created_at, + updated_at: dossier.updated_at + } + end + def expect_storage_service_to_convert_object expect(storage_service).to receive(:make_blob) expect(storage_service).to receive(:copy_from_carrierwave_to_active_storage!) @@ -64,6 +76,8 @@ describe PieceJustificativeToChampPieceJointeMigrationService do end context 'no notifications are sent to instructeurs' do + let!(:initial_dossier_timestamps) { timestamps(dossier) } + context 'when there is a PJ' do let(:pjs) { make_pjs } @@ -80,24 +94,18 @@ describe PieceJustificativeToChampPieceJointeMigrationService do dossier.reload end - it 'the champ has the same created_at as the PJ' do + it 'the champ has the same timestamps as the PJ' do expect(dossier.champs.last.created_at).to eq(pjs.last.created_at) + expect(dossier.champs.last.updated_at).to eq(pjs.last.updated_at) end - it 'the champ has the same updated_at as the PJ' do - expect(dossier.champs.last.updated_at).to eq(pjs.last.updated_at) + it 'does not change the dossier timestamps' do + expect(dossier.created_at).to eq(initial_dossier_timestamps[:created_at]) + expect(dossier.updated_at).to eq(initial_dossier_timestamps[:updated_at]) end end context 'when there is no PJ' do - let!(:expected_updated_at) do - # Reload dossier because the resolution of in-database timestamps is - # different from the resolution of in-memory timestamps, causing the - # tests to fail on fractional time differences. - dossier.reload - dossier.updated_at - end - before do Timecop.travel(1.hour) { service.convert_procedure_pjs_to_champ_pjs(procedure) } @@ -105,12 +113,14 @@ describe PieceJustificativeToChampPieceJointeMigrationService do dossier.reload end - it 'the champ has the same created_at as the dossier' do - expect(dossier.champs.last.created_at).to eq(dossier.created_at) + it 'the champ has the same timestamps as the dossier' do + expect(dossier.champs.last.created_at).to eq(initial_dossier_timestamps[:created_at]) + expect(dossier.champs.last.updated_at).to eq(initial_dossier_timestamps[:updated_at]) end - it 'the champ has the same updated_at as the dossier' do - expect(dossier.champs.last.updated_at).to eq(expected_updated_at) + it 'does not change the dossier timestamps' do + expect(dossier.created_at).to eq(initial_dossier_timestamps[:created_at]) + expect(dossier.updated_at).to eq(initial_dossier_timestamps[:updated_at]) end end end @@ -192,6 +202,9 @@ describe PieceJustificativeToChampPieceJointeMigrationService do ) end + let!(:initial_dossier_timestamps) { timestamps(dossier) } + let!(:initial_failing_dossier_timestamps) { timestamps(failing_dossier) } + before do allow(storage_service).to receive(:checksum).and_return('cafe') allow(storage_service).to receive(:fix_content_type) @@ -204,8 +217,10 @@ describe PieceJustificativeToChampPieceJointeMigrationService do end def try_convert(procedure) - service.convert_procedure_pjs_to_champ_pjs(procedure) + Timecop.travel(1.hour) { service.convert_procedure_pjs_to_champ_pjs(procedure) } rescue StandardError, SignalException => e + dossier.reload + failing_dossier.reload e end @@ -234,6 +249,12 @@ describe PieceJustificativeToChampPieceJointeMigrationService do .not_to change { procedure.types_de_piece_justificative.count } end + it 'does not change the dossiers timestamps' do + try_convert(procedure) + expect(dossier.updated_at).to eq(initial_dossier_timestamps[:updated_at]) + expect(failing_dossier.updated_at).to eq(initial_failing_dossier_timestamps[:updated_at]) + end + it 'does not leave stale blobs behind' do expect { try_convert(procedure) } .not_to change { ActiveStorage::Blob.count }