refactor(champ): stop using parent_id on repetition champs

app/controllers/champs/repetition_controller.rb

@@ -2,14 +2,13 @@
 
 class Champs::RepetitionController < Champs::ChampController
   def add
-    row = @champ.add_row(@champ.dossier.revision)
-    @first_champ_id = row.map(&:focusable_input_id).compact.first
-    @row_id = row.first&.row_id
+    @row_id = @champ.add_row(updated_by: current_user.email)
+    @first_champ_id = @champ.focusable_input_id
     @row_number = @row_id.nil? ? 0 : @champ.row_ids.find_index(@row_id) + 1
   end
 
   def remove
-    @champ.remove_row(params[:row_id])
+    @champ.remove_row(params[:row_id], updated_by: current_user.email)
     @to_remove = "safe-row-selector-#{params[:row_id]}"
     @to_focus = @champ.focusable_input_id || helpers.dom_id(@champ, :create_repetition)
   end

app/graphql/mutations/dossier_modifier_annotation_ajouter_ligne.rb

@@ -16,7 +16,7 @@ def resolve(dossier:, annotation_id:, instructeur:)
         return { errors: ["L’annotation \"#{annotation_id}\" n’existe pas"] }
       end
 
-      annotation.add_row(dossier.revision)
+      annotation.add_row(updated_by: instructeur.email)
 
       { annotation:, errors: nil }
     end

app/graphql/types/dossier_type.rb

@@ -170,7 +170,7 @@ def champs(id: nil)
           .for(object, private: false)
           .load(ApplicationRecord.id_from_typed_id(id))
       else
-        object.champs_for_revision(scope: :public, root: true).filter(&:visible?)
+        object.project_champs_public.filter(&:visible?)
       end
     end
 
@@ -180,7 +180,7 @@ def annotations(id: nil)
           .for(object, private: true)
           .load(ApplicationRecord.id_from_typed_id(id))
       else
-        object.champs_for_revision(scope: :private, root: true).filter(&:visible?)
+        object.project_champs_private.filter(&:visible?)
       end
     end
 

app/models/champs/repetition_champ.rb

@@ -1,35 +1,26 @@
 # frozen_string_literal: true
 
 class Champs::RepetitionChamp < Champ
-  accepts_nested_attributes_for :champs
+  delegate :libelle_for_export, to: :type_de_champ
 
   def rows
-    dossier
-      .champs_for_revision(scope: type_de_champ)
-      .group_by(&:row_id)
-      .sort
-      .map(&:second)
+    dossier.project_rows_for(type_de_champ)
   end
 
   def row_ids
-    rows.map { _1.first.row_id }
+    dossier.repetition_row_ids(type_de_champ)
   end
 
-  def add_row(revision)
-    added_champs = []
-    transaction do
-      row_id = ULID.generate
-      revision.children_of(type_de_champ).each do |type_de_champ|
-        added_champs << type_de_champ.build_champ(row_id:)
-      end
-      self.champs << added_champs
-    end
-    added_champs
+  def add_row(updated_by:)
+    # TODO: clean this up when parent_id is deprecated
+    row_id, added_champs = dossier.repetition_add_row(type_de_champ, updated_by:)
+    self.champs << added_champs
+    dossier.champs.reload if dossier.persisted?
+    row_id
   end
 
-  def remove_row(row_id)
-    dossier.champs.where(row_id:).destroy_all
-    dossier.champs.reload
+  def remove_row(row_id, updated_by:)
+    dossier.repetition_remove_row(type_de_champ, row_id, updated_by:)
   end
 
   def focusable_input_id

app/models/concerns/dossier_champs_concern.rb

@@ -3,21 +3,10 @@
 module DossierChampsConcern
   extend ActiveSupport::Concern
 
-  def champs_for_revision(scope: nil, root: false)
+  def champs_for_revision(scope: nil)
     champs_index = champs.group_by(&:stable_id)
-      # Due to some bad data we can have multiple copies of the same champ. Ignore extra copy.
-      .transform_values { _1.sort_by(&:id).uniq(&:row_id) }
-
-    if scope.is_a?(TypeDeChamp)
-      revision
-        .children_of(scope)
-        .flat_map { champs_index[_1.stable_id] || [] }
-        .filter(&:child?) # TODO: remove once bad data (child champ without a row id) is cleaned
-    else
-      revision
-        .types_de_champ_for(scope:, root:)
-        .flat_map { champs_index[_1.stable_id] || [] }
-    end
+    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.
@@ -74,6 +63,50 @@ def update_champs_attributes(attributes, scope, updated_by:)
     assign_attributes(champs_attributes:)
   end
 
+  def project_champs_public
+    revision.types_de_champ_public.map { project_champ(_1, nil) }
+  end
+
+  def project_champs_private
+    revision.types_de_champ_private.map { project_champ(_1, nil) }
+  end
+
+  def repetition_row_ids(type_de_champ)
+    stable_ids = revision.children_of(type_de_champ).map(&:stable_id)
+    champs.filter { _1.stable_id.in?(stable_ids) && _1.row_id.present? }
+      .map(&:row_id)
+      .uniq
+      .sort
+  end
+
+  def repetition_add_row(type_de_champ, updated_by:)
+    row_id = ULID.generate
+    types_de_champ = revision.children_of(type_de_champ)
+    # TODO: clean this up when parent_id is deprecated
+    added_champs = types_de_champ.map { _1.build_champ(row_id:, updated_by:) }
+    @champs_by_public_id = nil
+    [row_id, added_champs]
+  end
+
+  def repetition_remove_row(type_de_champ, row_id, updated_by:)
+    champs.where(row_id:).destroy_all
+    champs.reload if persisted?
+    @champs_by_public_id = nil
+  end
+
+  def project_rows_for(type_de_champ)
+    types_de_champ = revision.children_of(type_de_champ)
+    repetition_row_ids(type_de_champ).map do |row_id|
+      types_de_champ.map { project_champ(_1, row_id) }
+    end
+  end
+
+  def reload
+    super.tap do
+      @champs_by_public_id = nil
+    end
+  end
+
   private
 
   def champs_by_public_id

app/models/concerns/dossier_searchable_concern.rb

@@ -15,15 +15,15 @@ def index_search_terms
 
       search_terms = [
         user&.email,
-        *champs_public.flat_map(&:search_terms),
+        *project_champs_public.flat_map(&:search_terms),
         *etablissement&.search_terms,
         individual&.nom,
         individual&.prenom,
         mandataire_first_name,
         mandataire_last_name
       ].compact_blank.join(' ')
 
-      private_search_terms = champs_private.flat_map(&:search_terms).compact_blank.join(' ')
+      private_search_terms = project_champs_private.flat_map(&:search_terms).compact_blank.join(' ')
 
       sql = "UPDATE dossiers SET search_terms = :search_terms, private_search_terms = :private_search_terms WHERE id = :id"
       sanitized_sql = self.class.sanitize_sql_array([sql, search_terms:, private_search_terms:, id:])

app/models/dossier.rb

@@ -479,10 +479,10 @@ def build_default_champs_for_new_dossier
       champs_private << champ
     end
     champs_public.filter { _1.repetition? && _1.mandatory? }.each do |champ|
-      champ.add_row(revision)
+      champ.add_row(updated_by: user.email)
     end
     champs_private.filter(&:repetition?).each do |champ|
-      champ.add_row(revision)
+      champ.add_row(updated_by: user.email)
     end
   end
 
@@ -942,14 +942,21 @@ def remove_piece_justificative_file_not_visible!
   end
 
   def check_mandatory_and_visible_champs
-    champs_for_revision(scope: :public)
-      .filter { _1.child? ? _1.parent.visible? : true }
-      .filter(&:visible?)
-      .filter(&:mandatory_blank?)
-      .map do |champ|
-        champ.errors.add(:value, :missing)
+    project_champs_public.filter(&:visible?).each do |champ|
+      if champ.mandatory_blank?
+        error = champ.errors.add(:value, :missing)
+        errors.import(error)
       end
-      .each { errors.import(_1) }
+      if champ.repetition?
+        champ.rows.each do |champs|
+          champs.filter(&:visible?).filter(&:mandatory_blank?).each do |champ|
+            error = champ.errors.add(:value, :missing)
+            errors.import(error)
+          end
+        end
+      end
+    end
+    errors
   end
 
   def demander_un_avis!(avis)

app/models/dossier_preloader.rb

@@ -80,8 +80,8 @@ def load_dossier(dossier, champs, pj_template: false)
       dossier.association(:revision).target = revision
     end
     dossier.association(:champs).target = champs
-    dossier.association(:champs_public).target = dossier.champs_for_revision(scope: :public, root: true)
-    dossier.association(:champs_private).target = dossier.champs_for_revision(scope: :private, root: true)
+    dossier.association(:champs_public).target = dossier.project_champs_public
+    dossier.association(:champs_private).target = dossier.project_champs_private
 
     # remove once parent_id is deprecated
     champs_by_parent_id = champs.group_by(&:parent_id)

app/models/procedure_revision.rb

@@ -179,19 +179,14 @@ def dossier_for_preview(user)
     dossier
   end
 
-  def types_de_champ_for(scope: nil, root: false)
-    # We return an unordered collection
-    return types_de_champ if !root && scope.nil?
-    return types_de_champ.filter { scope == :public ? _1.public? : _1.private? } if !root
-
-    # We return an ordered collection
+  def types_de_champ_for(scope: nil)
     case scope
     when :public
-      types_de_champ_public
+      types_de_champ.filter(&:public?)
     when :private
-      types_de_champ_private
+      types_de_champ.filter(&:private?)
     else
-      types_de_champ_public + types_de_champ_private
+      types_de_champ
     end
   end
 

app/models/types_de_champ/prefill_repetition_type_de_champ.rb

@@ -56,8 +56,7 @@ def initialize(champ, repetition, index, revision)
     def to_assignable_attributes
       return unless repetition.is_a?(Hash)
 
-      row = champ.rows[index] || champ.add_row(revision)
-      row_id = row.first.row_id
+      row_id = champ.row_ids[index] || champ.add_row(updated_by: nil)
 
       repetition.map do |key, value|
         next unless key.is_a?(String) && key.starts_with?("champ_")

app/serializers/champ_serializer.rb

@@ -48,11 +48,7 @@ def read_attribute_for_serialization(attribute)
   end
 
   def rows
-    object.dossier
-      .champs_for_revision(scope: object.type_de_champ)
-      .group_by(&:row_id)
-      .values
-      .map.with_index(1) { |champs, index| Row.new(index:, champs:) }
+    object.rows.map.with_index(1) { |champs, index| Row.new(index:, champs:) }
   end
 
   def include_etablissement?

app/serializers/dossier_serializer.rb

@@ -32,7 +32,7 @@ class DossierSerializer < ActiveModel::Serializer
   has_many :champs, serializer: ChampSerializer
 
   def champs
-    champs = object.champs_public.reject { |c| c.type_de_champ.old_pj.present? }
+    champs = object.project_champs_public.reject { |c| c.type_de_champ.old_pj.present? }
 
     if object.expose_legacy_carto_api?
       champ_carte = champs.find do |champ|
@@ -52,12 +52,16 @@ def champs
     champs
   end
 
+  def champs_private
+    object.project_champs_private
+  end
+
   def cerfa
     []
   end
 
   def pieces_justificatives
-    object.champs_public.filter { |champ| champ.type_de_champ.old_pj }.map do |champ|
+    object.project_champs_public.filter { |champ| champ.type_de_champ.old_pj }.map do |champ|
       {
         created_at: champ.created_at&.in_time_zone('UTC'),
         type_de_piece_justificative_id: champ.type_de_champ.old_pj[:stable_id],

spec/graphql/annotation_spec.rb

@@ -5,7 +5,7 @@
   let(:procedure) { create(:procedure, :published, :for_individual, types_de_champ_private: [{ type: :repetition, children: [{ libelle: 'Nom' }, { type: :integer_number, libelle: 'Age' }] }, {}], administrateurs: [admin]) }
   let(:dossiers) { [] }
   let(:instructeur) { create(:instructeur, followed_dossiers: dossiers) }
-  let(:champs_private) { dossier.champs_for_revision(scope: :private, root: true) }
+  let(:champs_private) { dossier.project_champs_private }
 
   let(:query) { '' }
   let(:context) { { administrateur_id: admin.id, procedure_ids: admin.procedure_ids, write_access: true } }

spec/lib/recovery/dossier_life_cycle_spec.rb

@@ -17,7 +17,7 @@
     let(:dossier) do
       d = create(:dossier, procedure:)
 
-      repetition(d).add_row(d.revision)
+      repetition(d).add_row(updated_by: 'test')
       pj_champ(d).piece_justificative_file.attach(some_file)
       carte(d).update(geo_areas: [geo_area])
       d.etablissement = create(:etablissement, :with_exercices)

spec/models/concerns/dossier_rebase_concern_spec.rb

@@ -346,8 +346,8 @@
         datetime_champ.update(value: Time.zone.now.to_s)
         text_champ.update(value: 'bonjour')
         # Add two rows then remove previous to last row in order to create a "hole" in the sequence
-        repetition_champ.add_row(repetition_champ.dossier.revision)
-        repetition_champ.add_row(repetition_champ.dossier.revision)
+        repetition_champ.add_row(updated_by: 'test')
+        repetition_champ.add_row(updated_by: 'test')
         repetition_champ.champs.where(row_id: repetition_champ.rows[-2].first.row_id).destroy_all
         repetition_champ.reload
       end

spec/models/concerns/tags_substitution_concern_spec.rb

@@ -233,9 +233,8 @@ def procedure
       let(:dossier) { create(:dossier, procedure:) }
 
       before do
-        repetition = dossier.champs_public
-          .find { |champ| champ.libelle == 'Répétition' }
-        repetition.add_row(dossier.revision)
+        repetition = dossier.project_champs_public.find(&:repetition?)
+        repetition.add_row(updated_by: 'test')
         paul_champs, pierre_champs = repetition.rows
 
         paul_champs.first.update(value: 'Paul')