From e5f544066303b360a018d2159a7db5f39e89343b Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Tue, 23 Nov 2021 13:27:28 +0100
Subject: [PATCH] models: explicitely save procedure's new revision
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Deep-cloned objects have all their relationships stale. Thus, for a
newly deep-cloned revision, `revision.types_de_champs` returns `[]`,
even when it actually has associated types de champ.

This causes consecutive champs creations and re-ordering to fail in
subtle ways, like:

```
procedure.draft_revision.add_type_de_champ(…)
procedure.publish_revision!
procedure.draft_revision.add_type_de_champ(…)
procedure.draft_revision.move_type_de_champ(…) # this will fail
```

As `publish_revision!` created a new stale revision, moving the type
de champ fails because not all existing champs are found until the
object is refreshed.

We don't hit this path in production, because usually only a single
operation is made in a request.

To fix this, save the new revision before associating it as the draft
procedure.

(Another option would be to `reload` the revision after creation, but
this seems better contained and matches the name of the method.)
---
 app/models/procedure.rb       |  4 +++-
 spec/models/procedure_spec.rb | 44 +++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/app/models/procedure.rb b/app/models/procedure.rb
index cb2bfad02..0179b6219 100644
--- a/app/models/procedure.rb
+++ b/app/models/procedure.rb
@@ -678,7 +678,9 @@ class Procedure < ApplicationRecord
   end
 
   def create_new_revision
-    draft_revision.deep_clone(include: [:revision_types_de_champ, :revision_types_de_champ_private])
+    draft_revision
+      .deep_clone(include: [:revision_types_de_champ, :revision_types_de_champ_private])
+      .tap(&:save!)
   end
 
   def average_dossier_weight
diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb
index 8db6d4716..dc659c6e2 100644
--- a/spec/models/procedure_spec.rb
+++ b/spec/models/procedure_spec.rb
@@ -695,6 +695,50 @@ describe Procedure do
     end
   end
 
+  describe "#publish_revision!" do
+    let(:procedure) { create(:procedure, :published) }
+    let(:tdc_attributes) { { type_champ: :number, libelle: 'libelle 1' } }
+    let(:publication_date) { Time.zone.local(2021, 1, 1, 12, 00, 00) }
+
+    before do
+      procedure.draft_revision.add_type_de_champ(tdc_attributes)
+    end
+
+    subject do
+      Timecop.freeze(publication_date) do
+        procedure.publish_revision!
+      end
+    end
+
+    it 'publishes the new revision' do
+      subject
+      expect(procedure.published_revision).to be_present
+      expect(procedure.published_revision.published_at).to eq(publication_date)
+      expect(procedure.published_revision.types_de_champ.first.libelle).to eq('libelle 1')
+    end
+
+    it 'creates a new draft revision' do
+      expect { subject }.to change(ProcedureRevision, :count).by(1)
+      expect(procedure.draft_revision).to be_present
+      expect(procedure.draft_revision.revision_types_de_champ).to be_present
+      expect(procedure.draft_revision.types_de_champ).to be_present
+      expect(procedure.draft_revision.types_de_champ.first.libelle).to eq('libelle 1')
+    end
+
+    context 'when the procedure has dossiers' do
+      let(:dossier_draft) { create(:dossier, :brouillon, procedure: procedure) }
+      let(:dossier_submitted) { create(:dossier, :en_construction, procedure: procedure) }
+
+      before { [dossier_draft, dossier_submitted] }
+
+      it 'enqueues rebase jobs for draft dossiers' do
+        subject
+        expect(DossierRebaseJob).to have_been_enqueued.with(dossier_draft)
+        expect(DossierRebaseJob).not_to have_been_enqueued.with(dossier_submitted)
+      end
+    end
+  end
+
   describe "#unpublish!" do
     let(:procedure) { create(:procedure, :published) }
     let(:now) { Time.zone.now.beginning_of_minute }