refactor(dossier): update clone and apply diff to work with rows

app/models/concerns/dossier_clone_concern.rb

@@ -89,7 +89,11 @@ module DossierCloneConcern
     dossier_attributes += [:groupe_instructeur_id] if fork
     relationships = [:individual, :etablissement]
 
-    cloned_champs = champs
+    discarded_row_ids = champs_in_revision
+      .filter { _1.row? && _1.discarded? }
+      .to_set(&:row_id)
+    cloned_champs = champs_in_revision
+      .reject { discarded_row_ids.member?(_1.row_id) }
       .index_by(&:id)
       .transform_values { [_1, _1.clone(fork)] }
 
@@ -143,34 +147,38 @@ module DossierCloneConcern
   end
 
   def apply_diff(diff)
-    champs_added = diff[:added].filter(&:persisted?)
-    champs_updated = diff[:updated].filter(&:persisted?)
-    champs_removed = diff[:removed].filter(&:persisted?)
+    added_row_ids = {}
+    diff[:added].each do |champ|
+      next if !champ.child?
+      next if added_row_ids.key?(champ.row_id)
+      added_row_ids[champ.row_id] = revision.parent_of(champ.type_de_champ)
+    end
 
-    champs_added.each { _1.update_column(:dossier_id, id) }
+    removed_row_ids = {}
+    diff[:removed].each do |champ|
+      next if !champ.child?
+      next if removed_row_ids.key?(champ.row_id)
+      removed_row_ids[champ.row_id] = revision.parent_of(champ.type_de_champ)
+    end
 
-    if champs_updated.present?
+    added_champs = diff[:added].filter { _1.persisted? && _1.fillable? }
+    updated_champs = diff[:updated].filter { _1.persisted? && _1.fillable? }
+
+    added_champs.each { _1.update_column(:dossier_id, id) }
+
+    if updated_champs.present?
       champs_index = filled_champs_public.index_by(&:public_id)
-      champs_updated.each do |champ|
+      updated_champs.each do |champ|
         champs_index[champ.public_id]&.destroy!
         champ.update_column(:dossier_id, id)
       end
     end
 
-    champs_removed.each(&:destroy!)
-  end
-
-  protected
-
-  # This is a temporary method that is only used by diff/merge algorithm. Once it's gone, this method should be removed.
-  def project_champs_public_all
-    revision.types_de_champ_public.flat_map do |type_de_champ|
-      champ = project_champ(type_de_champ, nil)
-      if type_de_champ.repetition?
-        [champ] + project_rows_for(type_de_champ).flatten
-      else
-        champ
+    added_row_ids.each do |row_id, repetition_type_de_champ|
+      champ_for_update(repetition_type_de_champ, row_id, updated_by: user.email).save!
     end
+    removed_row_ids.each do |row_id, repetition_type_de_champ|
+      champ_for_update(repetition_type_de_champ, row_id, updated_by: user.email).discard!
     end
   end
 end

spec/models/concerns/dossier_clone_concern_spec.rb

@@ -135,8 +135,8 @@ RSpec.describe DossierCloneConcern do
 
         context 'for Champs::Repetition with rows, original_champ.repetition and rows are duped' do
           let(:types_de_champ_public) { [{ type: :repetition, children: [{}, {}] }] }
-          let(:champ_repetition) { dossier.champs.find(&:repetition?) }
-          let(:cloned_champ_repetition) { new_dossier.champs.find(&:repetition?) }
+          let(:champ_repetition) { dossier.project_champs_public.find(&:repetition?) }
+          let(:cloned_champ_repetition) { new_dossier.project_champs_public.find(&:repetition?) }
 
           it do
             expect(cloned_champ_repetition.rows.flatten.count).to eq(4)
@@ -308,7 +308,7 @@ RSpec.describe DossierCloneConcern do
     end
 
     context 'with new revision' do
-      let(:added_champ) { forked_dossier.champs.find { _1.libelle == "Un nouveau champ text" } }
+      let(:added_champ) { forked_dossier.project_champs_public.find { _1.libelle == "Un nouveau champ text" } }
       let(:removed_champ) { dossier.champs.find { _1.stable_id == 99 } }
       let(:new_dossier) { dossier.clone }
 
@@ -325,12 +325,16 @@ RSpec.describe DossierCloneConcern do
         expect(dossier.revision_id).to eq(procedure.revisions.first.id)
         expect(new_dossier.revision_id).to eq(procedure.published_revision.id)
         expect(forked_dossier.revision_id).to eq(procedure.published_revision_id)
-        is_expected.to eq(added: [added_champ], updated: [], removed: [removed_champ])
+        expect(subject[:added].map(&:stable_id)).to eq([added_champ.stable_id])
+        expect(subject[:added].first.new_record?).to be_truthy
+        expect(subject[:updated]).to be_empty
+        expect(subject[:removed]).to eq([removed_champ])
       }
     end
   end
 
   describe '#merge_fork' do
+    let(:dossier) { create(:dossier, :en_construction, :with_populated_champs, procedure:) }
     subject { dossier.merge_fork(forked_dossier) }
 
     context 'with updated champ' do
@@ -348,11 +352,11 @@ RSpec.describe DossierCloneConcern do
         dossier.debounce_index_search_terms_flag.remove
       end
 
-      it { expect { subject }.to change { dossier.champs.size }.by(0) }
+      it { expect { subject }.to change { dossier.champs.reload.size }.by(0) }
       it { expect { subject }.not_to change { dossier.champs.order(:created_at).reject { _1.stable_id.in?([99, 994]) }.map(&:value) } }
       it { expect { subject }.to have_enqueued_job(DossierIndexSearchTermsJob).with(dossier) }
       it { expect { subject }.to change { dossier.champs.find { _1.stable_id == 99 }.value }.from('old value').to('new value') }
-      it { expect { subject }.to change { dossier.champs.find { _1.stable_id == 994 }.value }.from('old value').to('new value in repetition') }
+      it { expect { subject }.to change { dossier.reload.champs.find { _1.stable_id == 994 }.value }.from('old value').to('new value in repetition') }
 
       it 'fork is hidden after merge' do
         subject
@@ -362,8 +366,16 @@ RSpec.describe DossierCloneConcern do
     end
 
     context 'with new revision' do
-      let(:added_champ) { forked_dossier.champs.find { _1.libelle == "Un nouveau champ text" } }
-      let(:added_repetition_champ) { forked_dossier.champs.find { _1.libelle == "Texte en répétition" } }
+      let(:added_champ) {
+        tdc = forked_dossier.revision.types_de_champ.find { _1.libelle == "Un nouveau champ text" }
+        forked_dossier.champ_for_update(tdc, nil, updated_by: 'test')
+      }
+      let(:added_repetition_champ) {
+        tdc_repetition = forked_dossier.revision.types_de_champ.find { _1.stable_id == 993 }
+        tdc = forked_dossier.revision.types_de_champ.find { _1.libelle == "Texte en répétition" }
+        row_id = forked_dossier.repetition_row_ids(tdc_repetition).first
+        forked_dossier.champ_for_update(tdc, row_id, updated_by: 'test')
+      }
       let(:removed_champ) { dossier.champs.find { _1.stable_id == 99 } }
       let(:updated_champ) { dossier.champs.find { _1.stable_id == 991 } }
 
@@ -393,8 +405,8 @@ RSpec.describe DossierCloneConcern do
         super()
       }
 
-      it { expect { subject }.to change { dossier.champs.size }.by(1) }
-      it { expect { subject }.to change { dossier.champs.order(:created_at).map(&:to_s) }.from(['old value', 'old value', 'Non', 'old value', 'old value']).to(['new value for updated champ', 'Non', 'old value', 'old value', 'new value for added champ', 'new value in repetition champ']) }
+      it { expect { subject }.to change { dossier.filled_champs.size }.by(1) }
+      it { expect { subject }.to change { dossier.filled_champs.sort_by(&:created_at).map(&:to_s) }.from(['old value', 'old value', 'Non', 'old value', 'old value']).to(['new value for updated champ', 'Non', 'old value', 'old value', 'new value for added champ', 'new value in repetition champ']) }
 
       it "dossier after merge should be on last published revision" do
         expect(dossier.revision_id).to eq(procedure.revisions.first.id)
@@ -404,13 +416,13 @@ RSpec.describe DossierCloneConcern do
         perform_enqueued_jobs only: DestroyRecordLaterJob
 
         expect(dossier.revision_id).to eq(procedure.published_revision_id)
-        expect(dossier.champs.all? { dossier.revision.in?(_1.type_de_champ.revisions) }).to be_truthy
+        expect(dossier.filled_champs.all? { dossier.revision.in?(_1.type_de_champ.revisions) }).to be_truthy
         expect(Dossier.exists?(forked_dossier.id)).to be_falsey
       end
     end
 
     context 'with old revision having repetition' do
-      let(:removed_champ) { dossier.champs.find(&:repetition?) }
+      let(:removed_champ) { dossier.project_champs_public.find(&:repetition?) }
 
       before do
         dossier.champs.each do |champ|
@@ -423,5 +435,42 @@ RSpec.describe DossierCloneConcern do
         expect { subject }.not_to raise_error
       end
     end
+
+    context 'with added row' do
+      let(:repetition_champ) { forked_dossier.project_champs_public.find(&:repetition?) }
+
+      def dossier_rows(dossier) = dossier.champs.filter(&:row?)
+
+      before do
+        repetition_champ.add_row(updated_by: 'test')
+      end
+
+      it {
+        expect(dossier_rows(dossier).size).to eq(2)
+        expect { subject }.to change { dossier_rows(dossier).size }.by(1)
+      }
+    end
+
+    context 'with removed row' do
+      let(:repetition_champ) { forked_dossier.project_champs_public.find(&:repetition?) }
+      let(:row_id) { repetition_champ.row_ids.first }
+
+      def dossier_rows(dossier) = dossier.champs.filter(&:row?)
+      def dossier_discarded_rows(dossier) = dossier_rows(dossier).filter(&:discarded?)
+
+      before do
+        repetition_champ.remove_row(row_id, updated_by: 'test')
+      end
+
+      it {
+        expect(dossier_rows(dossier).size).to eq(2)
+        expect { subject }.to change { dossier_rows(dossier).size }.by(0)
+      }
+
+      it {
+        expect(dossier_discarded_rows(dossier).size).to eq(0)
+        expect { subject }.to change { dossier_discarded_rows(dossier).size }.by(1)
+      }
+    end
   end
 end