Merge branch 'dev'

- Usager : correction d'une erreur lors de l'enregistrement simultané d'un champ invalide et d'une pièce jointe
- Admin : correction d'un problème lors de la création d'une démarche sans cadre juridique

- Migration des PJ : gestion des erreurs qui peuvent se produire pendant le rollback
- Migration des PJ : corrige le rollback dans le cas où les dossiers sont cachés
- Correction du mailer_preview pour prendre en compte les administrateurs multiples

app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb

@@ -54,15 +54,7 @@ class PieceJustificativeToChampPieceJointeMigrationService
     # 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}…"
-
+    rollback_migration!(types_de_champ_pj)
-    types_de_champ_pj.each do |type_champ|
-      # 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."
 
     # Reraise the exception to abort the migration.
@@ -144,6 +136,23 @@ class PieceJustificativeToChampPieceJointeMigrationService
     raise
   end
 
+  def rollback_migration!(types_de_champ_pj)
+    types_de_champ_pj.each do |type_champ|
+      # First destroy all the individual champs on dossiers
+      type_champ.champ.each do |champ|
+        begin
+          destroy_champ_pj(Dossier.unscope(where: :hidden_at).find(champ.dossier_id), champ)
+        rescue => e
+          rake_puts e
+          rake_puts "Rolling back of champ #{champ.id} failed. Continuing to roll back…"
+        end
+      end
+      # Now we can destroy the type de champ itself,
+      # without cascading the timestamp update on all attached dossiers.
+      type_champ.reload.destroy
+    end
+  end
+
   def make_blob(pj)
     storage_service.make_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename)
   end

app/views/admin/procedures/new.html.haml

@@ -6,7 +6,4 @@
     = form_for @procedure, url: { controller: 'admin/procedures', action: :create }, multipart: true do |f|
       = render partial: 'informations', locals: { f: f }
       .text-right
-        - if @availability.in?(Procedure::PATH_CAN_PUBLISH)
         = f.button 'Valider', class: 'btn btn-info', id: 'save-procedure'
-        - else
-          = f.button 'Valider', class: 'btn btn-info', id: 'save-procedure', disabled: true

app/views/shared/attachment/_update.html.haml

@@ -5,8 +5,10 @@
       = link_to('le modèle suivant', url_for(template), target: '_blank', rel: 'noopener')
 
   - attachment_id = attachment ? attachment.id : SecureRandom.uuid
+  - persisted = attachment && attachment.persisted?
   - user_can_destroy = defined?(user_can_destroy) ? user_can_destroy : false
-  - if attachment
+
+  - if persisted
     .piece-justificative-actions{ id: "piece_justificative_#{attachment_id}" }
       .piece-justificative-action
         = render partial: "shared/attachment/show", locals: { attachment: attachment, user_can_upload: true }
@@ -18,5 +20,5 @@
 
   = form.file_field :piece_justificative_file,
     id: "piece_justificative_file_#{attachment_id}",
-    class: "piece-justificative-input #{'hidden' if attachment}",
+    class: "piece-justificative-input #{'hidden' if persisted}",
     direct_upload: true

spec/factories/champ.rb

@@ -147,7 +147,7 @@ FactoryBot.define do
   factory :champ_piece_justificative, class: 'Champs::PieceJustificativeChamp' do
     type_de_champ { create(:type_de_champ_piece_justificative) }
 
-    after(:create) do |champ, _evaluator|
+    after(:build) do |champ, _evaluator|
       champ.piece_justificative_file.attach(io: StringIO.new("toto"), filename: "toto.txt", content_type: "text/plain")
     end
   end

spec/mailers/previews/administration_mailer_preview.rb

@@ -29,11 +29,11 @@ class AdministrationMailerPreview < ActionMailer::Preview
   private
 
   def procedure_1
-    Procedure.new(id: 10, libelle: "Démarche des marches", administrateur: administrateur)
+    Procedure.new(id: 10, libelle: "Démarche des marches", administrateurs: [administrateur])
   end
 
   def procedure_2
-    Procedure.new(id: 20, libelle: "Démarche pieds", administrateur: administrateur)
+    Procedure.new(id: 20, libelle: "Démarche pieds", administrateurs: [administrateur])
   end
 
   def administrateur

spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb

@@ -265,6 +265,22 @@ describe PieceJustificativeToChampPieceJointeMigrationService do
         .not_to change { ActiveStorage::Attachment.count }
     end
 
+    context 'when some dossiers to roll back are hidden' do
+      before do
+        dossier.update_column(:hidden_at, Time.zone.now)
+      end
+
+      it 'does not create champs' do
+        expect { try_convert(procedure) }
+          .not_to change { dossier.champs.count }
+      end
+
+      it 'does not change the hidden dossier timestamps' do
+        try_convert(procedure)
+        expect(dossier.updated_at).to eq(initial_dossier_timestamps[:updated_at])
+      end
+    end
+
     context 'when receiving a Signal interruption (like Ctrl+C)' do
       let(:exception) { Interrupt }
 
@@ -276,5 +292,26 @@ describe PieceJustificativeToChampPieceJointeMigrationService do
         expect { try_convert(procedure) }.not_to change { dossier.champs.count }
       end
     end
+
+    context 'when rolling back a dossier fails' do
+      before do
+        allow(service).to receive(:destroy_champ_pj)
+          .with(having_attributes(id: dossier.id), anything)
+          .and_raise(StandardError)
+        allow(service).to receive(:destroy_champ_pj)
+          .with(any_args)
+          .and_call_original
+      end
+
+      it 'continues to roll back the other dossiers' do
+        expect { try_convert(procedure) }
+          .not_to change { failing_dossier.champs.count }
+      end
+
+      it 'does not creates types de champ on the procedure' do
+        expect { try_convert(procedure) }
+          .not_to change { procedure.types_de_champ.count }
+      end
+    end
   end
 end

spec/views/shared/attachment/_update.html.haml_spec.rb

@@ -0,0 +1,65 @@
+require 'rails_helper'
+
+describe 'shared/attachment/_update.html.haml', type: :view do
+  let(:champ) { build(:champ_piece_justificative, dossier: create(:dossier)) }
+  let(:attachment) { nil }
+  let(:virus_scan_result) { nil }
+  let(:user_can_destroy) { false }
+
+  subject do
+    form_for(champ.dossier) do |form|
+      render 'shared/attachment/update', {
+        attachment: attachment,
+        user_can_destroy: user_can_destroy,
+        form: form
+      }
+    end
+  end
+
+  it 'renders a form field for uploading a file' do
+    expect(subject).to have_selector('input[type=file]:not(.hidden)')
+  end
+
+  context 'when there is a attached file' do
+    let(:attachment) { champ.piece_justificative_file.attachment }
+
+    it 'renders a form field for uploading a file' do
+      expect(subject).to have_selector('input[type=file]:not(.hidden)')
+    end
+
+    it 'does not renders a link to the unsaved file' do
+      expect(subject).not_to have_content(attachment.filename.to_s)
+    end
+
+    it 'doesn’t render action buttons' do
+      expect(subject).not_to have_link('Remplacer')
+      expect(subject).not_to have_link('Supprimer')
+    end
+
+    context 'and the attachment has been saved' do
+      before { champ.save! }
+
+      it 'renders a link to the file' do
+        expect(subject).to have_content(attachment.filename.to_s)
+      end
+
+      it 'renders action buttons' do
+        expect(subject).to have_button('Remplacer')
+      end
+
+      it 'hides the form field by default' do
+        expect(subject).to have_selector('input[type=file].hidden')
+      end
+
+      it 'hides the Delete button by default' do
+        is_expected.not_to have_link('Supprimer')
+      end
+
+      context 'and the user can delete the attachment' do
+        let(:user_can_destroy) { true }
+
+        it { is_expected.to have_link('Supprimer') }
+      end
+    end
+  end
+end