Merge pull request #8535 from tchak/fix-rebase-repetitions

fix(dossier): fix adding repetition in a new revision

app/jobs/migrations/backfill_dossier_repetition_job.rb

@@ -0,0 +1,14 @@
+class Migrations::BackfillDossierRepetitionJob < ApplicationJob
+  def perform(dossier_ids)
+    Dossier.where(id: dossier_ids)
+      .includes(:champs, revision: :types_de_champ)
+      .find_each do |dossier|
+        dossier.revision
+          .types_de_champ
+          .filter { _1.type_champ == 'repetition' }
+          .each do |type_de_champ|
+            dossier.champs << type_de_champ.champ.build
+          end
+      end
+  end
+end

app/models/concerns/dossier_rebase_concern.rb

@@ -61,7 +61,10 @@ module DossierRebaseConcern
 
     # add champ
     changes_by_op[:add]
-      .each { add_new_champs_for_revision(target_coordinates_by_stable_id[_1.stable_id]) }
+      .map { target_coordinates_by_stable_id[_1.stable_id] }
+      # add parent champs first so we can then add children
+      .sort_by { _1.child? ? 1 : 0 }
+      .each { add_new_champs_for_revision(_1) }
 
     # remove champ
     changes_by_op[:remove]
@@ -120,13 +123,14 @@ module DossierRebaseConcern
     if target_coordinate.child?
       # If this type de champ is a child, we create a new champ for each row of the parent
       parent_stable_id = target_coordinate.parent.stable_id
-      champs_repetition = champs
-        .includes(:champs, :type_de_champ)
-        .where(type_de_champ: { stable_id: parent_stable_id })
 
-      champs_repetition.each do |champ_repetition|
-        champ_repetition.champs.map(&:row_id).uniq.each do |row_id|
-          create_champ(target_coordinate, champ_repetition, row_id:)
+      champs.filter { _1.stable_id == parent_stable_id }.each do |champ_repetition|
+        if champ_repetition.champs.empty?
+          create_champ(target_coordinate, champ_repetition, row_id: ULID.generate)
+        else
+          champ_repetition.champs.map(&:row_id).uniq.each do |row_id|
+            create_champ(target_coordinate, champ_repetition, row_id:)
+          end
         end
       end
     else
@@ -135,10 +139,10 @@ module DossierRebaseConcern
   end
 
   def create_champ(target_coordinate, parent, row_id: nil)
-    params = { revision: target_coordinate.revision, rebased_at: Time.zone.now, row_id: }.compact
     champ = target_coordinate
       .type_de_champ
-      .build_champ(params)
+      .champ
+      .build(rebased_at: Time.zone.now, row_id:)
     parent.champs << champ
   end
 

lib/tasks/deployment/20230201090534_backfill_dossiers_repetitions.rake

@@ -0,0 +1,29 @@
+namespace :after_party do
+  desc 'Deployment task: backfill_dossiers_repetitions'
+  task backfill_dossiers_repetitions: :environment do
+    puts "Running deploy task 'backfill_dossiers_repetitions'"
+
+    revision_ids = ProcedureRevision.joins(:types_de_champ).where(types_de_champ: { type_champ: :repetition }).distinct.pluck(:id)
+    dossier_ids = Dossier.where(revision_id: revision_ids).pluck(:id)
+
+    progress = ProgressReport.new(dossier_ids.size)
+    dossier_ids_to_fix = []
+    dossier_ids.in_groups_of(10000, false) do |dossier_ids|
+      dossier_ids_with_repetition = Champ.where(dossier_id: dossier_ids, type: 'Champs::RepetitionChamp').pluck(:dossier_id).uniq
+      dossier_ids_to_fix += dossier_ids - dossier_ids_with_repetition
+      progress.inc(10000)
+    end
+    progress.finish
+
+    pp "fixing #{dossier_ids_to_fix.size} dossiers"
+
+    dossier_ids_to_fix.in_groups_of(100, false) do |dossier_ids|
+      Migrations::BackfillDossierRepetitionJob.perform_later(dossier_ids)
+    end
+
+    # Update task as completed.  If you remove the line below, the task will
+    # run with every deploy (or every time you call after_party:run).
+    AfterParty::TaskRecord
+      .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp
+  end
+end

spec/models/dossier_rebase_concern_spec.rb

@@ -239,25 +239,38 @@ describe Dossier do
   end
 
   describe "#rebase" do
-    let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :text, mandatory: true }, { type: :repetition, children: [{ type: :text }] }, { type: :datetime }, { type: :yes_no }, { type: :integer_number }]) }
+    let(:procedure) do
+      create(:procedure, types_de_champ_public: [
+        { type: :text, mandatory: true, stable_id: 100 },
+        {
+          type: :repetition, stable_id: 101, children: [
+            { type: :text, stable_id: 102 }
+          ]
+        },
+        { type: :datetime, stable_id: 103 },
+        { type: :yes_no, stable_id: 104 },
+        { type: :integer_number, stable_id: 105 }
+      ])
+    end
     let(:dossier) { create(:dossier, procedure: procedure) }
+    let(:types_de_champ) { procedure.active_revision.types_de_champ }
 
-    let(:yes_no_type_de_champ) { procedure.active_revision.types_de_champ_public.find { |tdc| tdc.type_champ == TypeDeChamp.type_champs.fetch(:yes_no) } }
+    let(:text_type_de_champ) { types_de_champ.find { _1.stable_id == 100 } }
+    let(:repetition_type_de_champ) { types_de_champ.find { _1.stable_id == 101 } }
+    let(:repetition_text_type_de_champ) { types_de_champ.find { _1.stable_id == 102 } }
+    let(:datetime_type_de_champ) { types_de_champ.find { _1.stable_id == 103 } }
+    let(:yes_no_type_de_champ) { types_de_champ.find { _1.stable_id == 104 } }
 
-    let(:text_type_de_champ) { procedure.active_revision.types_de_champ_public.find(&:mandatory?) }
-    let(:text_champ) { dossier.champs_public.find(&:mandatory?) }
-    let(:rebased_text_champ) { dossier.champs_public.find { |c| c.type_champ == TypeDeChamp.type_champs.fetch(:text) } }
+    let(:text_champ) { dossier.champs_public.find { _1.stable_id == 100 } }
+    let(:repetition_champ) { dossier.champs_public.find { _1.stable_id == 101 } }
+    let(:datetime_champ) { dossier.champs_public.find { _1.stable_id == 103 } }
 
-    let(:rebased_number_champ) { dossier.champs_public.find { |c| c.type_champ == TypeDeChamp.type_champs.fetch(:integer_number) } }
+    let(:rebased_text_champ) { dossier.champs_public.find { _1.stable_id == 100 } }
+    let(:rebased_repetition_champ) { dossier.champs_public.find { _1.stable_id == 101 } }
+    let(:rebased_datetime_champ) { dossier.champs_public.find { _1.stable_id == 103 } }
+    let(:rebased_number_champ) { dossier.champs_public.find { _1.stable_id == 105 } }
 
-    let(:datetime_type_de_champ) { procedure.active_revision.types_de_champ_public.find { |tdc| tdc.type_champ == TypeDeChamp.type_champs.fetch(:datetime) } }
-    let(:datetime_champ) { dossier.champs_public.find { |c| c.type_champ == TypeDeChamp.type_champs.fetch(:datetime) } }
-    let(:rebased_datetime_champ) { dossier.champs_public.find { |c| c.type_champ == TypeDeChamp.type_champs.fetch(:date) } }
-
-    let(:repetition_type_de_champ) { procedure.active_revision.types_de_champ_public.find { |tdc| tdc.type_champ == TypeDeChamp.type_champs.fetch(:repetition) } }
-    let(:repetition_text_type_de_champ) { procedure.active_revision.children_of(repetition_type_de_champ).find { |tdc| tdc.type_champ == TypeDeChamp.type_champs.fetch(:text) } }
-    let(:repetition_champ) { dossier.champs_public.find { |c| c.type_champ == TypeDeChamp.type_champs.fetch(:repetition) } }
-    let(:rebased_repetition_champ) { dossier.champs_public.find { |c| c.type_champ == TypeDeChamp.type_champs.fetch(:repetition) } }
+    let(:rebased_new_repetition_champ) { dossier.champs_public.find { _1.libelle == "une autre repetition" } }
 
     before do
       procedure.publish!
@@ -265,15 +278,29 @@ describe Dossier do
         type_champ: TypeDeChamp.type_champs.fetch(:text),
         libelle: "Un champ text"
       })
-      procedure.draft_revision.find_and_ensure_exclusive_use(text_type_de_champ).update(mandatory: false, libelle: "nouveau libelle")
-      procedure.draft_revision.find_and_ensure_exclusive_use(datetime_type_de_champ).update(type_champ: TypeDeChamp.type_champs.fetch(:date))
-      procedure.draft_revision.find_and_ensure_exclusive_use(repetition_text_type_de_champ).update(libelle: "nouveau libelle dans une repetition")
+      procedure.draft_revision.find_and_ensure_exclusive_use(text_type_de_champ.stable_id).update(mandatory: false, libelle: "nouveau libelle")
+      procedure.draft_revision.find_and_ensure_exclusive_use(datetime_type_de_champ.stable_id).update(type_champ: TypeDeChamp.type_champs.fetch(:date))
+      procedure.draft_revision.find_and_ensure_exclusive_use(repetition_text_type_de_champ.stable_id).update(libelle: "nouveau libelle dans une repetition")
       procedure.draft_revision.add_type_de_champ({
         type_champ: TypeDeChamp.type_champs.fetch(:checkbox),
         libelle: "oui ou non",
         parent_stable_id: repetition_type_de_champ.stable_id
       })
       procedure.draft_revision.remove_type_de_champ(yes_no_type_de_champ.stable_id)
+      new_repetition_type_de_champ = procedure.draft_revision.add_type_de_champ({
+        type_champ: TypeDeChamp.type_champs.fetch(:repetition),
+        libelle: "une autre repetition"
+      })
+      procedure.draft_revision.add_type_de_champ({
+        type_champ: TypeDeChamp.type_champs.fetch(:text),
+        libelle: "un champ text dans une autre repetition",
+        parent_stable_id: new_repetition_type_de_champ.stable_id
+      })
+      procedure.draft_revision.add_type_de_champ({
+        type_champ: TypeDeChamp.type_champs.fetch(:date),
+        libelle: "un champ date dans une autre repetition",
+        parent_stable_id: new_repetition_type_de_champ.stable_id
+      })
 
       datetime_champ.update(value: Time.zone.now.to_s)
       text_champ.update(value: 'bonjour')
@@ -285,11 +312,9 @@ describe Dossier do
     end
 
     it "updates the brouillon champs with the latest revision changes" do
-      revision_id = dossier.revision_id
-      libelle = text_type_de_champ.libelle
-
       expect(dossier.revision).to eq(procedure.published_revision)
       expect(dossier.champs_public.size).to eq(5)
+      expect(dossier.champs_public_all.size).to eq(7)
       expect(repetition_champ.rows.size).to eq(2)
       expect(repetition_champ.rows[0].size).to eq(1)
       expect(repetition_champ.rows[1].size).to eq(1)
@@ -301,7 +326,8 @@ describe Dossier do
 
       expect(procedure.revisions.size).to eq(3)
       expect(dossier.revision).to eq(procedure.published_revision)
-      expect(dossier.champs_public.size).to eq(5)
+      expect(dossier.champs_public.size).to eq(6)
+      expect(dossier.champs_public_all.size).to eq(12)
       expect(rebased_text_champ.value).to eq(text_champ.value)
       expect(rebased_text_champ.type_de_champ_id).not_to eq(text_champ.type_de_champ_id)
       expect(rebased_datetime_champ.type_champ).to eq(TypeDeChamp.type_champs.fetch(:date))
@@ -312,6 +338,10 @@ describe Dossier do
       expect(rebased_text_champ.rebased_at).not_to be_nil
       expect(rebased_datetime_champ.rebased_at).not_to be_nil
       expect(rebased_number_champ.rebased_at).to be_nil
+      expect(rebased_new_repetition_champ).not_to be_nil
+      expect(rebased_new_repetition_champ.rebased_at).not_to be_nil
+      expect(rebased_new_repetition_champ.rows.size).to eq(1)
+      expect(rebased_new_repetition_champ.rows[0].size).to eq(2)
     end
   end