Merge pull request #10897 from tchak/refactor-dossier-diff-merge

ETQ dev, je ne veux plus de méthode `champs_for_revision`

app/models/concerns/dossier_champs_concern.rb

@@ -3,24 +3,6 @@
 module DossierChampsConcern
   extend ActiveSupport::Concern
 
-  def champs_for_revision(scope: nil)
-    champs_index = champs.group_by(&:stable_id)
-    revision.types_de_champ_for(scope:)
-      .flat_map { champs_index[_1.stable_id] || [] }
-  end
-
-  # Get all the champs values for the types de champ in the final list.
-  # Dossier might not have corresponding champ – display nil.
-  # To do so, we build a virtual champ when there is no value so we can call for_export with all indexes
-  def champs_for_export(types_de_champ, row_id = nil)
-    types_de_champ.flat_map do |type_de_champ|
-      champ = champ_for_export(type_de_champ, row_id)
-      type_de_champ.libelles_for_export.map do |(libelle, path)|
-        [libelle, TypeDeChamp.champ_value_for_export(type_de_champ.type_champ, champ, path)]
-      end
-    end
-  end
-
   def project_champ(type_de_champ, row_id)
     check_valid_row_id?(type_de_champ, row_id)
     champ = champs_by_public_id[type_de_champ.public_id(row_id)]
@@ -39,6 +21,34 @@ module DossierChampsConcern
     revision.types_de_champ_private.map { project_champ(_1, nil) }
   end
 
+  def filled_champs_public
+    project_champs_public.flat_map do |champ|
+      if champ.repetition?
+        champ.rows.flatten.filter { _1.persisted? && _1.fillable? }
+      elsif champ.persisted? && champ.fillable?
+        champ
+      else
+        []
+      end
+    end
+  end
+
+  def filled_champs_private
+    project_champs_private.flat_map do |champ|
+      if champ.repetition?
+        champ.rows.flatten.filter { _1.persisted? && _1.fillable? }
+      elsif champ.persisted? && champ.fillable?
+        champ
+      else
+        []
+      end
+    end
+  end
+
+  def filled_champs
+    filled_champs_public + filled_champs_private
+  end
+
   def project_rows_for(type_de_champ)
     return [] if !type_de_champ.repetition?
 
@@ -69,6 +79,15 @@ module DossierChampsConcern
       .map { _1.repetition? ? project_champ(_1, nil) : champ_for_update(_1, nil, updated_by: nil) }
   end
 
+  def champs_for_export(types_de_champ, row_id = nil)
+    types_de_champ.flat_map do |type_de_champ|
+      champ = champ_for_export(type_de_champ, row_id)
+      type_de_champ.libelles_for_export.map do |(libelle, path)|
+        [libelle, TypeDeChamp.champ_value_for_export(type_de_champ.type_champ, champ, path)]
+      end
+    end
+  end
+
   def champ_for_update(type_de_champ, row_id, updated_by:)
     champ, attributes = champ_with_attributes_for_update(type_de_champ, row_id, updated_by:)
     champ.assign_attributes(attributes)

app/models/concerns/dossier_clone_concern.rb

@@ -44,10 +44,10 @@ module DossierCloneConcern
   end
 
   def make_diff(editing_fork)
-    origin_champs_index = champs_for_revision(scope: :public).index_by(&:public_id)
-    forked_champs_index = editing_fork.champs_for_revision(scope: :public).index_by(&:public_id)
+    origin_champs_index = project_champs_public_all.index_by(&:public_id)
+    forked_champs_index = editing_fork.project_champs_public_all.index_by(&:public_id)
     updated_champs_index = editing_fork
-      .champs_for_revision(scope: :public)
+      .project_champs_public_all
       .filter { _1.updated_at > editing_fork.created_at }
       .index_by(&:public_id)
 
@@ -83,7 +83,7 @@ module DossierCloneConcern
     dossier_attributes += [:groupe_instructeur_id] if fork
     relationships = [:individual, :etablissement]
 
-    cloned_champs = champs_for_revision
+    cloned_champs = champs
       .index_by(&:id)
       .transform_values { [_1, _1.clone(fork)] }
 
@@ -149,15 +149,34 @@ module DossierCloneConcern
   end
 
   def apply_diff(diff)
-    champs_index = (champs_for_revision(scope: :public) + diff[:added]).index_by(&:public_id)
+    champs_added = diff[:added].filter(&:persisted?)
+    champs_updated = diff[:updated].filter(&:persisted?)
+    champs_removed = diff[:removed].filter(&:persisted?)
 
-    diff[:added].each { _1.update_column(:dossier_id, id) }
+    champs_added.each { _1.update_column(:dossier_id, id) }
 
-    diff[:updated].each do |champ|
-      champs_index.fetch(champ.public_id)&.destroy!
+    if champs_updated.present?
+      champs_index = filled_champs_public.index_by(&:public_id)
+      champs_updated.each do |champ|
+        champs_index[champ.public_id]&.destroy!
         champ.update_column(:dossier_id, id)
       end
+    end
 
-    diff[:removed].each(&:destroy!)
+    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
+      end
+    end
   end
 end

app/models/dossier.rb

@@ -517,7 +517,7 @@ class Dossier < ApplicationRecord
   def can_passer_en_construction?
     return true if !revision.ineligibilite_enabled || !revision.ineligibilite_rules
 
-    !revision.ineligibilite_rules.compute(champs_for_revision(scope: :public))
+    !revision.ineligibilite_rules.compute(filled_champs_public)
   end
 
   def can_passer_en_instruction?
@@ -567,7 +567,7 @@ class Dossier < ApplicationRecord
 
   def any_etablissement_as_degraded_mode?
     return true if etablissement&.as_degraded_mode?
-    return true if champs_for_revision(scope: :public).any? { _1.etablissement&.as_degraded_mode? }
+    return true if filled_champs_public.any? { _1.etablissement&.as_degraded_mode? }
 
     false
   end
@@ -1031,7 +1031,7 @@ class Dossier < ApplicationRecord
   end
 
   def linked_dossiers_for(instructeur_or_expert)
-    dossier_ids = champs_for_revision.filter(&:dossier_link?).filter_map(&:value)
+    dossier_ids = filled_champs.filter(&:dossier_link?).filter_map(&:value)
     instructeur_or_expert.dossiers.where(id: dossier_ids)
   end
 
@@ -1040,7 +1040,7 @@ class Dossier < ApplicationRecord
   end
 
   def geo_data?
-    GeoArea.exists?(champ_id: champs_for_revision)
+    GeoArea.exists?(champ_id: filled_champs)
   end
 
   def to_feature_collection
@@ -1195,7 +1195,7 @@ class Dossier < ApplicationRecord
   end
 
   def geo_areas
-    champs_for_revision.flat_map(&:geo_areas)
+    filled_champs.flat_map(&:geo_areas)
   end
 
   def bounding_box

spec/models/concerns/dossier_champs_concern_spec.rb

@@ -110,6 +110,7 @@ RSpec.describe DossierChampsConcern do
     subject { dossier.project_champs_public }
 
     it { expect(subject.size).to eq(4) }
+    it { expect(subject.find { _1.libelle == 'Nom' }).to be_falsey }
   end
 
   describe '#project_champs_private' do
@@ -118,6 +119,36 @@ RSpec.describe DossierChampsConcern do
     it { expect(subject.size).to eq(1) }
   end
 
+  describe '#filled_champs_public' do
+    let(:types_de_champ_public) do
+      [
+        { type: :header_section },
+        { type: :text, libelle: "Un champ text" },
+        { type: :text, libelle: "Un autre champ text" },
+        { type: :yes_no, libelle: "Un champ yes no" },
+        { type: :repetition, libelle: "Un champ répétable", mandatory: true, children: [{ type: :text, libelle: 'Nom' }] },
+        { type: :explication }
+      ]
+    end
+    subject { dossier.filled_champs_public }
+
+    it { expect(subject.size).to eq(4) }
+    it { expect(subject.find { _1.libelle == 'Nom' }).to be_truthy }
+  end
+
+  describe '#filled_champs_private' do
+    let(:types_de_champ_private) do
+      [
+        { type: :header_section },
+        { type: :text, libelle: "Une annotation" },
+        { type: :explication }
+      ]
+    end
+    subject { dossier.filled_champs_private }
+
+    it { expect(subject.size).to eq(1) }
+  end
+
   describe '#repetition_row_ids' do
     let(:type_de_champ_repetition) { dossier.find_type_de_champ_by_stable_id(993) }
     subject { dossier.repetition_row_ids(type_de_champ_repetition) }

spec/models/dossier_spec.rb

@@ -1852,7 +1852,7 @@ describe Dossier, type: :model do
       let(:types_de_champ_public) { [{ type: :carte }, { type: :carte }, { type: :carte }] }
 
       it do
-        dossier.champs_for_revision
+        dossier.filled_champs
 
         count = 0
 

spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb

@@ -13,7 +13,7 @@ module Maintenance
       before { expect(champ).to receive(:visible?).and_return(visible) }
 
       context 'when piece_justificative' do
-        let(:champ) { dossier.champs_for_revision(scope: :public).find(&:piece_justificative?) }
+        let(:champ) { dossier.filled_champs_public.find(&:piece_justificative?) }
 
         context 'when not visible' do
           let(:visible) { false }