From c7f2113972aea0c1d852dbb8e5dbbff4fa28fe44 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Tue, 28 May 2019 15:29:59 +0200
Subject: [PATCH 01/10] tasks: add a description to make :pieces_justificatives
 tasks valid

---
 lib/tasks/pieces_justificatives.rake | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/tasks/pieces_justificatives.rake b/lib/tasks/pieces_justificatives.rake
index 49fe00193..5f11f5fa8 100644
--- a/lib/tasks/pieces_justificatives.rake
+++ b/lib/tasks/pieces_justificatives.rake
@@ -1,6 +1,9 @@
 require Rails.root.join("lib", "tasks", "task_helper")
 
 namespace :pieces_justificatives do
+  desc <<~EOD
+    Migrate the PJ to champs for a single PROCEDURE_ID.
+  EOD
   task migrate_procedure_to_champs: :environment do
     procedure_id = ENV['PROCEDURE_ID']
     procedure = Procedure.find(procedure_id)
@@ -17,6 +20,9 @@ namespace :pieces_justificatives do
     progress.finish
   end
 
+  desc <<~EOD
+    Migrate the PJ to champs for several procedures ids, from RANGE_START to RANGE_END.
+  EOD
   task migrate_procedures_range_to_champs: :environment do
     if ENV['RANGE_START'].nil? || ENV['RANGE_END'].nil?
       fail "RANGE_START and RANGE_END must be specified"

From 44c410d40dce2bda654a4573f8db40a791223a32 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Tue, 28 May 2019 17:20:07 +0200
Subject: [PATCH 02/10] piece_justificative_service: fix for missing
 order_place

In production some pieces justificatives don't have an order place.

In this case, insert the champs after the ones that have an order place.
---
 app/services/pieces_justificatives_service.rb    |  8 ++------
 .../pieces_justificatives_service_spec.rb        | 16 +++++++++++++++-
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb
index 7ca08d233..411e11124 100644
--- a/app/services/pieces_justificatives_service.rb
+++ b/app/services/pieces_justificatives_service.rb
@@ -33,12 +33,8 @@ class PiecesJustificativesService
   end
 
   def self.types_pj_as_types_de_champ(procedure)
-    last_champ = procedure.types_de_champ.last
-    if last_champ.present?
-      order_place = last_champ.order_place + 1
-    else
-      order_place = 0
-    end
+    max_order_place = procedure.types_de_champ.pluck(:order_place).compact.max || -1
+    order_place = max_order_place + 1
 
     types_de_champ = [
       TypeDeChamp.new(
diff --git a/spec/services/pieces_justificatives_service_spec.rb b/spec/services/pieces_justificatives_service_spec.rb
index 63754ecdd..3d8eb6d09 100644
--- a/spec/services/pieces_justificatives_service_spec.rb
+++ b/spec/services/pieces_justificatives_service_spec.rb
@@ -112,11 +112,25 @@ describe PiecesJustificativesService do
         create(
           :procedure,
           types_de_piece_justificative: tpjs,
-          types_de_champ: [build(:type_de_champ, order_place: 0)]
+          types_de_champ: [build(:type_de_champ, order_place: 0), build(:type_de_champ, order_place: 1)]
         )
       end
 
       it 'generates a sequence of incrementing order_places that continues where the last type de champ left off' do
+        expect(subject.pluck(:order_place)).to contain_exactly(2, 3)
+      end
+    end
+
+    context 'with pre-existing champs without an order place' do
+      let(:procedure) do
+        create(
+          :procedure,
+          types_de_piece_justificative: tpjs,
+          types_de_champ: [build(:type_de_champ, order_place: 0), build(:type_de_champ, order_place: nil)]
+        )
+      end
+
+      it 'ignores champs without an order place' do
         expect(subject.pluck(:order_place)).to contain_exactly(1, 2)
       end
     end

From 10df7b70eef2c7198f322d5e308c9aa5934723b5 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Tue, 28 May 2019 18:19:33 +0200
Subject: [PATCH 03/10] carrierwave: when migrating, create an empty blob if
 file is missing

---
 ...erwave_active_storage_migration_service.rb | 16 +++++++-
 ...to_champ_piece_jointe_migration_service.rb | 20 +++++++---
 ...e_active_storage_migration_service_spec.rb | 37 ++++++++++++++++++-
 3 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/app/services/carrierwave_active_storage_migration_service.rb b/app/services/carrierwave_active_storage_migration_service.rb
index 9d80f3be3..1183cf8f9 100644
--- a/app/services/carrierwave_active_storage_migration_service.rb
+++ b/app/services/carrierwave_active_storage_migration_service.rb
@@ -79,7 +79,7 @@ class CarrierwaveActiveStorageMigrationService
 
     ActiveStorage::Blob.create(
       filename: filename || uploader.filename,
-      content_type: uploader.content_type,
+      content_type: content_type,
       byte_size: uploader.size,
       checksum: checksum(uploader),
       created_at: created_at,
@@ -87,6 +87,20 @@ class CarrierwaveActiveStorageMigrationService
     )
   end
 
+  def make_empty_blob(uploader, created_at, filename: nil)
+    content_type = uploader.content_type || 'text/plain'
+
+    blob = ActiveStorage::Blob.build_after_upload(
+      io: StringIO.new('File not found when migrating from CarrierWave.'),
+      filename: filename || uploader.filename,
+      content_type: content_type || 'text/plain',
+      metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE }
+    )
+    blob.created_at = created_at
+    blob.save!
+    blob
+  end
+
   def checksum(uploader)
     hex_to_base64(uploader.file.send(:file).etag)
   end
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 30b025201..0ca726b19 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
@@ -75,12 +75,18 @@ class PieceJustificativeToChampPieceJointeMigrationService
   end
 
   def convert_pj_to_champ!(pj, champ)
-    blob = make_blob(pj)
+    actual_file_exists = pj.content.file.send(:file)
+    if actual_file_exists
+      blob = make_blob(pj)
 
-    # Upload the file before creating the attachment to make sure MIME type
-    # identification doesn’t fail.
-    storage_service.copy_from_carrierwave_to_active_storage!(pj.content.path, blob)
-    attachment = storage_service.make_attachment(champ, 'piece_justificative_file', blob)
+      # Upload the file before creating the attachment to make sure MIME type
+      # identification doesn’t fail.
+      storage_service.copy_from_carrierwave_to_active_storage!(pj.content.path, blob)
+      attachment = storage_service.make_attachment(champ, 'piece_justificative_file', blob)
+
+    else
+      make_empty_blob(pj)
+    end
 
     # By reloading, we force ActiveStorage to look at the attachment again, and see
     # that one exists now. We do this so that, if we need to roll back and destroy the champ,
@@ -112,4 +118,8 @@ class PieceJustificativeToChampPieceJointeMigrationService
   def make_blob(pj)
     storage_service.make_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename)
   end
+
+  def make_empty_blob(pj)
+    storage_service.make_empty_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename)
+  end
 end
diff --git a/spec/services/carrierwave_active_storage_migration_service_spec.rb b/spec/services/carrierwave_active_storage_migration_service_spec.rb
index a3bb2b042..fb70b0d6f 100644
--- a/spec/services/carrierwave_active_storage_migration_service_spec.rb
+++ b/spec/services/carrierwave_active_storage_migration_service_spec.rb
@@ -8,7 +8,7 @@ describe CarrierwaveActiveStorageMigrationService do
   end
 
   describe '.make_blob' do
-    let(:pj) { create(:piece_justificative, :rib) }
+    let(:pj) { create(:piece_justificative, :rib, updated_at: Time.zone.local(2019, 01, 01, 12, 00)) }
     let(:identify) { false }
 
     before do
@@ -17,6 +17,8 @@ describe CarrierwaveActiveStorageMigrationService do
 
     subject(:blob) { service.make_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename, identify: identify) }
 
+    it { expect(blob.created_at).to eq pj.updated_at }
+
     it 'marks the blob as already scanned by the antivirus' do
       expect(blob.metadata[:virus_scan_result]).to eq(ActiveStorage::VirusScanner::SAFE)
     end
@@ -34,4 +36,37 @@ describe CarrierwaveActiveStorageMigrationService do
       end
     end
   end
+
+  describe '.make_empty_blob' do
+    let(:pj) { create(:piece_justificative, :rib, updated_at: Time.zone.local(2019, 01, 01, 12, 00)) }
+
+    before 'set the underlying stored file as missing' do
+      allow(pj.content.file).to receive(:file).and_return(nil)
+    end
+
+    subject(:blob) { service.make_empty_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename) }
+
+    it { expect(blob.created_at).to eq pj.updated_at }
+
+    it 'marks the blob as already scanned by the antivirus' do
+      expect(blob.metadata[:virus_scan_result]).to eq(ActiveStorage::VirusScanner::SAFE)
+    end
+
+    it 'sets the blob MIME type from the file' do
+      expect(blob.identified).to be true
+      expect(blob.content_type).to eq 'application/pdf'
+    end
+
+    context 'when the file metadata are also missing' do
+      before do
+        allow(pj).to receive(:original_filename).and_return(nil)
+        allow(pj.content).to receive(:content_type).and_return(nil)
+      end
+
+      it 'fallbacks on default values' do
+        expect(blob.filename).to eq pj.content.filename
+        expect(blob.content_type).to eq 'text/plain'
+      end
+    end
+  end
 end

From 4d9cb0bb7aea821c7a0258f3c5ba6a18a7c33267 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 29 May 2019 11:48:21 +0200
Subject: [PATCH 04/10] task_helper: fix duration formatting

If we don't convert the duration to utc, it starts at 01:00:00
---
 lib/tasks/task_helper.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/tasks/task_helper.rb b/lib/tasks/task_helper.rb
index 8be2ab6dc..fa60c58c7 100644
--- a/lib/tasks/task_helper.rb
+++ b/lib/tasks/task_helper.rb
@@ -60,7 +60,7 @@ class ProgressReport
 
   def format_duration(seconds)
     if seconds.finite?
-      Time.zone.at(seconds).strftime('%H:%M:%S')
+      Time.zone.at(seconds).utc.strftime('%H:%M:%S')
     else
       '--:--:--'
     end

From 2bb103b8bd51173510f772e76b1bef05231d70cd Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 29 May 2019 11:48:41 +0200
Subject: [PATCH 05/10] task_helper: mark private methods

---
 lib/tasks/task_helper.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/tasks/task_helper.rb b/lib/tasks/task_helper.rb
index fa60c58c7..ef6a0bd95 100644
--- a/lib/tasks/task_helper.rb
+++ b/lib/tasks/task_helper.rb
@@ -37,6 +37,8 @@ class ProgressReport
     rake_puts
   end
 
+  private
+
   def set_progress(total: nil, count: nil)
     if total.present?
       @total = total

From 60121a1be6d6bfff3df0ec87a7b2d4fed4341675 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 29 May 2019 11:49:11 +0200
Subject: [PATCH 06/10] =?UTF-8?q?task=5Fhelper:=20ensure=20that=20incremen?=
 =?UTF-8?q?ting=20above=20100%=20doesn=E2=80=99t=20raise=20an=20error?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 spec/lib/tasks/task_helper_spec.rb | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 spec/lib/tasks/task_helper_spec.rb

diff --git a/spec/lib/tasks/task_helper_spec.rb b/spec/lib/tasks/task_helper_spec.rb
new file mode 100644
index 000000000..4d02d159c
--- /dev/null
+++ b/spec/lib/tasks/task_helper_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe ProgressReport, lib: true do
+  context 'when the count pass above 100%' do
+    let(:total) { 2 }
+
+    subject(:progress) { ProgressReport.new(total) }
+
+    it 'doesn’t raise errors' do
+      expect do
+        (total + 2).times { progress.inc }
+        progress.finish
+      end.not_to raise_error
+    end
+  end
+end

From a168ae3a729df0c9c4a2c3e94a8efee27d6454c8 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 29 May 2019 12:02:33 +0200
Subject: [PATCH 07/10] pj migration: reduce the number of queries by
 preloading champs

---
 ...ece_justificative_to_champ_piece_jointe_migration_service.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 0ca726b19..a157a1983 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
@@ -37,7 +37,7 @@ class PieceJustificativeToChampPieceJointeMigrationService
     procedure.types_de_champ += types_de_champ_pj
 
     # Unscope to make sure all dossiers are migrated, even the soft-deleted ones
-    procedure.dossiers.unscope(where: :hidden_at).find_each do |dossier|
+    procedure.dossiers.unscope(where: :hidden_at).includes(:champs).find_each do |dossier|
       champs_pj = types_de_champ_pj.map(&:build_champ)
       dossier.champs += champs_pj
 

From 1a256b37f89681e847681dd49914603e95c04ded Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 29 May 2019 12:03:16 +0200
Subject: [PATCH 08/10] pj migration: print a notice when migrating missing
 files

---
 ...ce_justificative_to_champ_piece_jointe_migration_service.rb | 3 +++
 1 file changed, 3 insertions(+)

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 a157a1983..8b2f4946e 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
@@ -1,3 +1,5 @@
+require Rails.root.join("lib", "tasks", "task_helper")
+
 class PieceJustificativeToChampPieceJointeMigrationService
   def initialize(**params)
     params.each do |key, value|
@@ -86,6 +88,7 @@ class PieceJustificativeToChampPieceJointeMigrationService
 
     else
       make_empty_blob(pj)
+      rake_puts "Notice: attached file for champ #{champ.id} not found. An empty blob has been attached instead."
     end
 
     # By reloading, we force ActiveStorage to look at the attachment again, and see

From 6cb02f292793d08dba7edbf5468daad77fd15082 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 29 May 2019 13:06:36 +0200
Subject: [PATCH 09/10] pj migration: handle signal interrupts

---
 ...e_to_champ_piece_jointe_migration_service.rb |  5 +++--
 ...champ_piece_jointe_migration_service_spec.rb | 17 +++++++++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

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 8b2f4946e..ed6597e94 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
@@ -64,7 +64,8 @@ class PieceJustificativeToChampPieceJointeMigrationService
         yield if block_given?
       end
     end
-  rescue
+
+  rescue StandardError, SignalException
     # If anything goes wrong, we roll back the migration by destroying the newly created
     # types de champ, champs blobs and attachments.
     types_de_champ_pj.each do |type_champ|
@@ -95,7 +96,7 @@ class PieceJustificativeToChampPieceJointeMigrationService
     # that one exists now. We do this so that, if we need to roll back and destroy the champ,
     # the blob, the attachment and the actual file on OpenStack also get deleted.
     champ.reload
-  rescue
+  rescue StandardError, SignalException
     # Destroy partially attached object that the more general rescue in `populate_champs_pjs!`
     # might not be able to handle.
 
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 bbd94d8ef..86ea98b7f 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
@@ -182,6 +182,7 @@ describe PieceJustificativeToChampPieceJointeMigrationService do
 
   context 'cleanup when conversion fails' do
     let(:pjs) { make_pjs }
+    let(:exception) { 'LOL no!' }
 
     let!(:failing_dossier) do
       create(
@@ -197,14 +198,14 @@ describe PieceJustificativeToChampPieceJointeMigrationService do
 
       expect(storage_service).to receive(:copy_from_carrierwave_to_active_storage!)
       expect(storage_service).to receive(:copy_from_carrierwave_to_active_storage!)
-        .and_raise('LOL no!')
+        .and_raise(exception)
 
       expect(storage_service).to receive(:delete_from_active_storage!)
     end
 
     def try_convert(procedure)
       service.convert_procedure_pjs_to_champ_pjs(procedure)
-    rescue => e
+    rescue StandardError, SignalException => e
       e
     end
 
@@ -242,5 +243,17 @@ describe PieceJustificativeToChampPieceJointeMigrationService do
       expect { try_convert(procedure) }
         .not_to change { ActiveStorage::Attachment.count }
     end
+
+    context 'when receiving a Signal interruption (like Ctrl+C)' do
+      let(:exception) { Interrupt }
+
+      it 'handles the exception as well' do
+        expect { service.convert_procedure_pjs_to_champ_pjs(procedure) }.to raise_error { Interrupt }
+      end
+
+      it 'does not create champs' do
+        expect { try_convert(procedure) }.not_to change { dossier.champs.count }
+      end
+    end
   end
 end

From a8847e40ea3585cbc5af3e4aecde7b10aa856a38 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 29 May 2019 13:06:57 +0200
Subject: [PATCH 10/10] pj migration: add comments and notices

---
 ...ustificative_to_champ_piece_jointe_migration_service.rb | 7 +++++++
 1 file changed, 7 insertions(+)

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 ed6597e94..18d37251e 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
@@ -40,9 +40,12 @@ 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)
@@ -68,11 +71,15 @@ class PieceJustificativeToChampPieceJointeMigrationService
   rescue StandardError, SignalException
     # If anything goes wrong, we roll back the migration by destroying the newly created
     # types de champ, champs blobs and attachments.
+    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
     end
 
+    rake_puts "Migration of procedure #{procedure.id} rolled back."
+
     # Reraise the exception to abort the migration.
     raise
   end