From 3817f334c93ebcccc0da3076733bd1d9dbb8c59c Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Thu, 22 Aug 2024 17:29:49 +0200
Subject: [PATCH 01/12] refactor(repetition): use less champs and public_id
 methods

---
 app/models/champ.rb                           |  2 +-
 app/models/champs/repetition_champ.rb         |  2 +-
 spec/models/champ_spec.rb                     | 77 ++++++++++---------
 .../services/procedure_export_service_spec.rb | 10 +--
 4 files changed, 47 insertions(+), 44 deletions(-)

diff --git a/app/models/champ.rb b/app/models/champ.rb
index c3ccf0774..56e62bb2d 100644
--- a/app/models/champ.rb
+++ b/app/models/champ.rb
@@ -94,7 +94,7 @@ class Champ < ApplicationRecord
   end
 
   def child?
-    parent_id.present?
+    row_id.present?
   end
 
   # used for the `required` html attribute
diff --git a/app/models/champs/repetition_champ.rb b/app/models/champs/repetition_champ.rb
index 7ea9d83cd..0731dea3e 100644
--- a/app/models/champs/repetition_champ.rb
+++ b/app/models/champs/repetition_champ.rb
@@ -37,7 +37,7 @@ class Champs::RepetitionChamp < Champ
   end
 
   def blank?
-    champs.empty?
+    row_ids.empty?
   end
 
   def search_terms
diff --git a/spec/models/champ_spec.rb b/spec/models/champ_spec.rb
index 50b033a58..6375ee4dd 100644
--- a/spec/models/champ_spec.rb
+++ b/spec/models/champ_spec.rb
@@ -8,31 +8,50 @@ describe Champ do
     let(:champ) { Champ.new(value: value) }
     let(:value) { '' }
     let(:mandatory) { true }
-    before { allow(champ).to receive(:type_de_champ).and_return(type_de_champ) }
 
-    context 'when mandatory and blank' do
-      it { expect(champ.mandatory_blank?).to be(true) }
-    end
+    context 'with champ' do
+      before { allow(champ).to receive(:type_de_champ).and_return(type_de_champ) }
 
-    context 'when carte mandatory and blank' do
-      let(:type_de_champ) { build(:type_de_champ_carte, mandatory: mandatory) }
-      let(:champ) { Champs::CarteChamp.new(value: value) }
-      let(:value) { nil }
-      it { expect(champ.mandatory_blank?).to be(true) }
-    end
+      context 'when mandatory and blank' do
+        it { expect(champ.mandatory_blank?).to be(true) }
+      end
 
-    context 'when multiple_drop_down_list mandatory and blank' do
-      let(:type_de_champ) { build(:type_de_champ_multiple_drop_down_list, mandatory: mandatory) }
-      let(:champ) { Champs::MultipleDropDownListChamp.new(value: value) }
-      let(:value) { '[]' }
-      it { expect(champ.mandatory_blank?).to be(true) }
-    end
+      context 'when carte mandatory and blank' do
+        let(:type_de_champ) { build(:type_de_champ_carte, mandatory: mandatory) }
+        let(:champ) { Champs::CarteChamp.new(value: value) }
+        let(:value) { nil }
+        it { expect(champ.mandatory_blank?).to be(true) }
+      end
 
-    context 'when repetition blank' do
-      let(:type_de_champ) { build(:type_de_champ_repetition) }
-      let(:champ) { Champs::RepetitionChamp.new }
+      context 'when multiple_drop_down_list mandatory and blank' do
+        let(:type_de_champ) { build(:type_de_champ_multiple_drop_down_list, mandatory: mandatory) }
+        let(:champ) { Champs::MultipleDropDownListChamp.new(value: value) }
+        let(:value) { '[]' }
+        it { expect(champ.mandatory_blank?).to be(true) }
+      end
 
-      it { expect(champ.blank?).to be(true) }
+      context 'when repetition blank' do
+        let(:type_de_champ) { build(:type_de_champ_repetition) }
+        let(:champ) { Champs::RepetitionChamp.new(dossier: Dossier.new(revision: ProcedureRevision.new)) }
+
+        it { expect(champ.blank?).to be(true) }
+      end
+
+      context 'when not blank' do
+        let(:value) { 'yop' }
+        it { expect(champ.mandatory_blank?).to be(false) }
+      end
+
+      context 'when not mandatory' do
+        let(:mandatory) { false }
+        it { expect(champ.mandatory_blank?).to be(false) }
+      end
+
+      context 'when not mandatory or blank' do
+        let(:value) { 'u' }
+        let(:mandatory) { false }
+        it { expect(champ.mandatory_blank?).to be(false) }
+      end
     end
 
     context 'when repetition not blank' do
@@ -42,22 +61,6 @@ describe Champ do
 
       it { expect(champ.blank?).to be(false) }
     end
-
-    context 'when not blank' do
-      let(:value) { 'yop' }
-      it { expect(champ.mandatory_blank?).to be(false) }
-    end
-
-    context 'when not mandatory' do
-      let(:mandatory) { false }
-      it { expect(champ.mandatory_blank?).to be(false) }
-    end
-
-    context 'when not mandatory or blank' do
-      let(:value) { 'u' }
-      let(:mandatory) { false }
-      it { expect(champ.mandatory_blank?).to be(false) }
-    end
   end
 
   describe "associations" do
@@ -125,7 +128,7 @@ describe Champ do
     let(:standalone_champ) { build(:champ, type_de_champ: build(:type_de_champ), dossier: build(:dossier)) }
     let(:public_sections) { dossier.champs_public.filter(&:header_section?) }
     let(:private_sections) { dossier.champs_private.filter(&:header_section?) }
-    let(:sections_in_repetition) { champ_in_repetition.parent.champs.filter(&:header_section?) }
+    let(:sections_in_repetition) { dossier.champs.filter(&:child?).filter(&:header_section?) }
 
     it 'returns the sibling sections of a champ' do
       expect(public_sections).not_to be_empty
diff --git a/spec/services/procedure_export_service_spec.rb b/spec/services/procedure_export_service_spec.rb
index a92466a17..34cecb9f1 100644
--- a/spec/services/procedure_export_service_spec.rb
+++ b/spec/services/procedure_export_service_spec.rb
@@ -390,11 +390,11 @@ describe ProcedureExportService do
 
       context 'with long libelle composed of utf8 characteres' do
         before do
-          procedure.active_revision.types_de_champ_public.each do |c|
-            c.update!(libelle: "#{c.id} - ?/[] ééé ééé ééééééé ééééééé éééééééé. ééé éé éééééééé éé ééé. ééééé éééééééé ééé ééé.")
+          procedure.active_revision.types_de_champ_public.each do |type_de_champ|
+            type_de_champ.update!(libelle: "#{type_de_champ.id} - ?/[] ééé ééé ééééééé ééééééé éééééééé. ééé éé éééééééé éé ééé. ééééé éééééééé ééé ééé.")
           end
-          champ_repetition.champs.each do |c|
-            c.type_de_champ.update!(libelle: "#{c.id} - Quam rem nam maiores numquam dolorem nesciunt. Cum et possimus et aut. Fugit voluptas qui qui.")
+          procedure.active_revision.children_of(champ_repetition.type_de_champ).each do |type_de_champ|
+            type_de_champ.update!(libelle: "#{type_de_champ.id} - Quam rem nam maiores numquam dolorem nesciunt. Cum et possimus et aut. Fugit voluptas qui qui.")
           end
         end
 
@@ -417,7 +417,7 @@ describe ProcedureExportService do
       context 'with empty repetition' do
         before do
           dossiers.flat_map { |dossier| dossier.champs_public.filter(&:repetition?) }.each do |champ|
-            champ.champs.destroy_all
+            Champ.where(row_id: champ.row_ids).destroy_all
           end
         end
 

From ed4fbc887ad492269b8e62ec20a9f6ce898ec9de Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Thu, 22 Aug 2024 17:31:36 +0200
Subject: [PATCH 02/12] refactor(dossier): remove unused scopes

---
 app/models/dossier.rb   | 33 ++++++++-------------------------
 app/models/procedure.rb | 14 --------------
 2 files changed, 8 insertions(+), 39 deletions(-)

diff --git a/app/models/dossier.rb b/app/models/dossier.rb
index eee97cee9..e9304b63e 100644
--- a/app/models/dossier.rb
+++ b/app/models/dossier.rb
@@ -270,33 +270,16 @@ class Dossier < ApplicationRecord
   scope :en_cours,                    -> { not_archived.state_en_construction_ou_instruction }
   scope :without_followers,           -> { where.missing(:follows) }
   scope :with_followers,              -> { left_outer_joins(:follows).where.not(follows: { id: nil }) }
-  scope :with_champs, -> {
-    includes(champs_public: [
-      :geo_areas,
-      piece_justificative_file_attachments: :blob,
-      champs: [piece_justificative_file_attachments: :blob]
-    ])
-  }
-
   scope :brouillons_recently_updated, -> { updated_since(2.days.ago).state_brouillon.order_by_updated_at }
-  scope :with_annotations, -> {
-    includes(champs_private: [
-      :geo_areas,
-      piece_justificative_file_attachments: :blob,
-      champs: [piece_justificative_file_attachments: :blob]
-    ])
-  }
   scope :for_api, -> {
-    with_champs
-      .with_annotations
-      .includes(commentaires: { piece_jointe_attachments: :blob },
-        justificatif_motivation_attachment: :blob,
-        attestation: [],
-        avis: { piece_justificative_file_attachment: :blob },
-        traitement: [],
-        etablissement: [],
-        individual: [],
-        user: [])
+    includes(commentaires: { piece_jointe_attachments: :blob },
+      justificatif_motivation_attachment: :blob,
+      attestation: [],
+      avis: { piece_justificative_file_attachment: :blob },
+      traitement: [],
+      etablissement: [],
+      individual: [],
+      user: [])
   }
 
   scope :with_notifiable_procedure, -> (opts = { notify_on_closed: false }) do
diff --git a/app/models/procedure.rb b/app/models/procedure.rb
index b8e16abdf..403d64f0e 100644
--- a/app/models/procedure.rb
+++ b/app/models/procedure.rb
@@ -209,20 +209,6 @@ class Procedure < ApplicationRecord
     includes(:draft_revision, :published_revision, administrateurs: :user)
   }
 
-  scope :for_download, -> {
-    includes(
-      :groupe_instructeurs,
-      dossiers: {
-        champs_public: [
-          piece_justificative_file_attachments: :blob,
-          champs: [
-            piece_justificative_file_attachments: :blob
-          ]
-        ]
-      }
-    )
-  }
-
   validates :libelle, presence: true, allow_blank: false, allow_nil: false
   validates :description, presence: true, allow_blank: false, allow_nil: false
   validates :administrateurs, presence: true

From df83322f0cffa5c21e5531cb9c2f6b15cfe77705 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Thu, 22 Aug 2024 17:32:01 +0200
Subject: [PATCH 03/12] refactor(apiv1): use dossier preloader on api v1

---
 app/controllers/api/v1/dossiers_controller.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/controllers/api/v1/dossiers_controller.rb b/app/controllers/api/v1/dossiers_controller.rb
index bf2470517..09c7d8fe4 100644
--- a/app/controllers/api/v1/dossiers_controller.rb
+++ b/app/controllers/api/v1/dossiers_controller.rb
@@ -18,6 +18,7 @@ class API::V1::DossiersController < APIController
 
   def show
     dossier = @dossiers.for_api.find(params[:id])
+    DossierPreloader.load_one(dossier)
 
     render json: { dossier: DossierSerializer.new(dossier).as_json }
   rescue ActiveRecord::RecordNotFound

From 988025ba69724c3a5f7d8d68eb7a0062c0f6f046 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Thu, 22 Aug 2024 17:33:32 +0200
Subject: [PATCH 04/12] refactor(champs): validate only champs in current
 revision

---
 app/models/champ.rb                                  |  2 ++
 app/models/concerns/champs_validate_concern.rb       |  8 ++++++--
 spec/models/champs/cnaf_champ_spec.rb                |  5 ++++-
 spec/models/champs/decimal_number_champ_spec.rb      |  1 +
 spec/models/champs/departement_champ_spec.rb         |  5 ++++-
 spec/models/champs/dgfip_champ_spec.rb               |  5 ++++-
 spec/models/champs/dossier_link_champ_spec.rb        |  1 +
 spec/models/champs/drop_down_list_champ_spec.rb      |  5 ++++-
 spec/models/champs/email_champ_spec.rb               | 12 ++++++++++--
 .../{ => champs}/engagement_juridique_champ_spec.rb  |  5 ++++-
 spec/models/champs/epci_champ_spec.rb                |  5 ++++-
 spec/models/champs/iban_champ_spec.rb                |  5 ++++-
 spec/models/champs/integer_number_champ_spec.rb      |  5 ++++-
 .../champs/multiple_drop_down_list_champ_spec.rb     |  5 ++++-
 spec/models/champs/phone_champ_spec.rb               |  5 ++++-
 spec/models/champs/rna_champ_spec.rb                 |  5 ++++-
 16 files changed, 64 insertions(+), 15 deletions(-)
 rename spec/models/{ => champs}/engagement_juridique_champ_spec.rb (83%)

diff --git a/app/models/champ.rb b/app/models/champ.rb
index 56e62bb2d..c8bfbff03 100644
--- a/app/models/champ.rb
+++ b/app/models/champ.rb
@@ -6,6 +6,8 @@ class Champ < ApplicationRecord
 
   self.ignored_columns += [:type_de_champ_id]
 
+  attr_readonly :stable_id
+
   belongs_to :dossier, inverse_of: false, touch: true, optional: false
   belongs_to :parent, class_name: 'Champ', optional: true
   has_many_attached :piece_justificative_file
diff --git a/app/models/concerns/champs_validate_concern.rb b/app/models/concerns/champs_validate_concern.rb
index bef65d73b..ab5b2595b 100644
--- a/app/models/concerns/champs_validate_concern.rb
+++ b/app/models/concerns/champs_validate_concern.rb
@@ -16,9 +16,9 @@ module ChampsValidateConcern
     def validate_champ_value?
       case validation_context
       when :champs_public_value
-        public? && visible?
+        public? && in_dossier_revision? && visible?
       when :champs_private_value
-        private? && visible?
+        private? && in_dossier_revision? && visible?
       else
         false
       end
@@ -27,5 +27,9 @@ module ChampsValidateConcern
     def validate_champ_value_or_prefill?
       validate_champ_value? || validation_context == :prefill
     end
+
+    def in_dossier_revision?
+      dossier.revision.types_de_champ.any? { _1.stable_id == stable_id }
+    end
   end
 end
diff --git a/spec/models/champs/cnaf_champ_spec.rb b/spec/models/champs/cnaf_champ_spec.rb
index f8f03ad08..acc7d0ae0 100644
--- a/spec/models/champs/cnaf_champ_spec.rb
+++ b/spec/models/champs/cnaf_champ_spec.rb
@@ -2,7 +2,10 @@
 
 describe Champs::CnafChamp, type: :model do
   let(:champ) { described_class.new(dossier: build(:dossier)) }
-  before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_cnaf)) }
+  before do
+    allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_cnaf))
+    allow(champ).to receive(:in_dossier_revision?).and_return(true)
+  end
   describe 'numero_allocataire and code_postal' do
     before do
       champ.numero_allocataire = '1234567'
diff --git a/spec/models/champs/decimal_number_champ_spec.rb b/spec/models/champs/decimal_number_champ_spec.rb
index 78958b7a8..8603f2f9a 100644
--- a/spec/models/champs/decimal_number_champ_spec.rb
+++ b/spec/models/champs/decimal_number_champ_spec.rb
@@ -6,6 +6,7 @@ describe Champs::DecimalNumberChamp do
     before do
       allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_decimal_number))
       allow(champ).to receive(:visible?).and_return(true)
+      allow(champ).to receive(:in_dossier_revision?).and_return(true)
       champ.run_callbacks(:validation)
     end
     subject { champ.validate(:champs_public_value) }
diff --git a/spec/models/champs/departement_champ_spec.rb b/spec/models/champs/departement_champ_spec.rb
index 9794e01a7..3bfb5f41c 100644
--- a/spec/models/champs/departement_champ_spec.rb
+++ b/spec/models/champs/departement_champ_spec.rb
@@ -4,7 +4,10 @@ describe Champs::DepartementChamp, type: :model do
   describe 'validations' do
     describe 'external link' do
       let(:champ) { described_class.new(external_id: external_id, dossier: build(:dossier)) }
-      before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_departements)) }
+      before do
+        allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_departements))
+        allow(champ).to receive(:in_dossier_revision?).and_return(true)
+      end
       subject { champ.validate(:champs_public_value) }
 
       context 'when nil' do
diff --git a/spec/models/champs/dgfip_champ_spec.rb b/spec/models/champs/dgfip_champ_spec.rb
index bfe94d895..bafa48adb 100644
--- a/spec/models/champs/dgfip_champ_spec.rb
+++ b/spec/models/champs/dgfip_champ_spec.rb
@@ -40,7 +40,10 @@ describe Champs::DgfipChamp, type: :model do
     let(:numero_fiscal) { '1122299999092' }
     let(:reference_avis) { 'FC22299999092' }
     let(:champ) { described_class.new(dossier: build(:dossier)) }
-    before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_dgfip)) }
+    before do
+      allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_dgfip))
+      allow(champ).to receive(:in_dossier_revision?).and_return(true)
+    end
     let(:validation_context) { :champs_public_value }
 
     subject { champ.valid?(validation_context) }
diff --git a/spec/models/champs/dossier_link_champ_spec.rb b/spec/models/champs/dossier_link_champ_spec.rb
index 411631c39..d5f5e4550 100644
--- a/spec/models/champs/dossier_link_champ_spec.rb
+++ b/spec/models/champs/dossier_link_champ_spec.rb
@@ -42,6 +42,7 @@ describe Champs::DossierLinkChamp, type: :model do
 
     before do
       allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_dossier_link, mandatory:))
+      allow(champ).to receive(:in_dossier_revision?).and_return(true)
       champ.run_callbacks(:validation)
     end
 
diff --git a/spec/models/champs/drop_down_list_champ_spec.rb b/spec/models/champs/drop_down_list_champ_spec.rb
index 842adf8f4..d69487b08 100644
--- a/spec/models/champs/drop_down_list_champ_spec.rb
+++ b/spec/models/champs/drop_down_list_champ_spec.rb
@@ -4,7 +4,10 @@ describe Champs::DropDownListChamp do
   describe 'validations' do
     describe 'inclusion' do
       let(:champ) { described_class.new(other:, value:, dossier: build(:dossier)) }
-      before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_drop_down_list, drop_down_other: other)) }
+      before do
+        allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_drop_down_list, drop_down_other: other))
+        allow(champ).to receive(:in_dossier_revision?).and_return(true)
+      end
       subject { champ.validate(:champs_public_value) }
 
       context 'when the other value is accepted' do
diff --git a/spec/models/champs/email_champ_spec.rb b/spec/models/champs/email_champ_spec.rb
index e24775296..f23705648 100644
--- a/spec/models/champs/email_champ_spec.rb
+++ b/spec/models/champs/email_champ_spec.rb
@@ -2,8 +2,10 @@
 
 describe Champs::EmailChamp do
   describe 'validation' do
-    let(:champ) { described_class.new(value:, dossier: build(:dossier)) }
-    before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_email)) }
+    let(:procedure) { create(:procedure, types_de_champ_public: [{}, { type: :email }, {}]) }
+    let(:dossier) { create(:dossier, procedure:) }
+    let(:champ) { dossier.champs.second }
+    before { champ.value = value }
     subject { champ.validate(:champs_public_value) }
 
     context 'when nil' do
@@ -79,5 +81,11 @@ describe Champs::EmailChamp do
         expect { subject }.to change { champ.value }.from(value).to('username@mailserver.domain')
       end
     end
+
+    context 'when type_de_champ is not in dossier revision anymore' do
+      before { dossier.revision.remove_type_de_champ(champ.stable_id) }
+      let(:value) { 'username' }
+      it { is_expected.to be_truthy }
+    end
   end
 end
diff --git a/spec/models/engagement_juridique_champ_spec.rb b/spec/models/champs/engagement_juridique_champ_spec.rb
similarity index 83%
rename from spec/models/engagement_juridique_champ_spec.rb
rename to spec/models/champs/engagement_juridique_champ_spec.rb
index 55b51df08..795cd7813 100644
--- a/spec/models/engagement_juridique_champ_spec.rb
+++ b/spec/models/champs/engagement_juridique_champ_spec.rb
@@ -7,7 +7,10 @@ describe Champs::EngagementJuridiqueChamp do
         .new(dossier: build(:dossier))
         .tap { _1.value = value }
     end
-    before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_engagement_juridique)) }
+    before {
+      allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_engagement_juridique))
+      allow(champ).to receive(:in_dossier_revision?).and_return(true)
+    }
     subject { champ.validate(:champs_public_value) }
 
     context 'with [A-Z]' do
diff --git a/spec/models/champs/epci_champ_spec.rb b/spec/models/champs/epci_champ_spec.rb
index d79cc7df6..58d7b7868 100644
--- a/spec/models/champs/epci_champ_spec.rb
+++ b/spec/models/champs/epci_champ_spec.rb
@@ -6,7 +6,10 @@ describe Champs::EpciChamp, type: :model do
 
     describe 'code_departement' do
       let(:champ) { Champs::EpciChamp.new(code_departement: code_departement, dossier: build(:dossier)) }
-      before { allow(champ).to receive(:visible?).and_return(true) }
+      before do
+        allow(champ).to receive(:visible?).and_return(true)
+        allow(champ).to receive(:in_dossier_revision?).and_return(true)
+      end
       context 'when nil' do
         let(:code_departement) { nil }
 
diff --git a/spec/models/champs/iban_champ_spec.rb b/spec/models/champs/iban_champ_spec.rb
index 7088f9a34..bf7c63e2f 100644
--- a/spec/models/champs/iban_champ_spec.rb
+++ b/spec/models/champs/iban_champ_spec.rb
@@ -3,7 +3,10 @@
 describe Champs::IbanChamp do
   describe '#valid?' do
     let(:champ) { Champs::IbanChamp.new(dossier: build(:dossier)) }
-    before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_iban)) }
+    before do
+      allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_iban))
+      allow(champ).to receive(:in_dossier_revision?).and_return(true)
+    end
     def with_value(value)
       champ.tap { _1.value = value }
     end
diff --git a/spec/models/champs/integer_number_champ_spec.rb b/spec/models/champs/integer_number_champ_spec.rb
index 4367a2465..8a5d2182f 100644
--- a/spec/models/champs/integer_number_champ_spec.rb
+++ b/spec/models/champs/integer_number_champ_spec.rb
@@ -2,7 +2,10 @@
 
 describe Champs::IntegerNumberChamp do
   let(:champ) { Champs::IntegerNumberChamp.new(value:, dossier: build(:dossier)) }
-  before { allow(champ).to receive(:visible?).and_return(true) }
+  before do
+    allow(champ).to receive(:visible?).and_return(true)
+    allow(champ).to receive(:in_dossier_revision?).and_return(true)
+  end
   subject { champ.validate(:champs_public_value) }
 
   describe '#valid?' do
diff --git a/spec/models/champs/multiple_drop_down_list_champ_spec.rb b/spec/models/champs/multiple_drop_down_list_champ_spec.rb
index 6ee9805c3..704572c47 100644
--- a/spec/models/champs/multiple_drop_down_list_champ_spec.rb
+++ b/spec/models/champs/multiple_drop_down_list_champ_spec.rb
@@ -4,7 +4,10 @@ describe Champs::MultipleDropDownListChamp do
   let(:type_de_champ) { build(:type_de_champ_multiple_drop_down_list, drop_down_list_value: "val1\r\nval2\r\nval3\r\n[brackets] val4") }
   let(:value) { nil }
   let(:champ) { Champs::MultipleDropDownListChamp.new(value:, dossier: build(:dossier)) }
-  before { allow(champ).to receive(:type_de_champ).and_return(type_de_champ) }
+  before do
+    allow(champ).to receive(:type_de_champ).and_return(type_de_champ)
+    allow(champ).to receive(:in_dossier_revision?).and_return(true)
+  end
 
   describe 'validations' do
     subject { champ.validate(:champs_public_value) }
diff --git a/spec/models/champs/phone_champ_spec.rb b/spec/models/champs/phone_champ_spec.rb
index c90951bc7..0653bf39d 100644
--- a/spec/models/champs/phone_champ_spec.rb
+++ b/spec/models/champs/phone_champ_spec.rb
@@ -2,7 +2,10 @@
 
 describe Champs::PhoneChamp do
   let(:champ) { Champs::PhoneChamp.new(dossier: build(:dossier)) }
-  before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_phone)) }
+  before do
+    allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_phone))
+    allow(champ).to receive(:in_dossier_revision?).and_return(true)
+  end
   describe '#validate' do
     it do
       expect(champ_with_value(nil).validate(:champs_public_value)).to be_truthy
diff --git a/spec/models/champs/rna_champ_spec.rb b/spec/models/champs/rna_champ_spec.rb
index 248f27c06..c2968f752 100644
--- a/spec/models/champs/rna_champ_spec.rb
+++ b/spec/models/champs/rna_champ_spec.rb
@@ -2,7 +2,10 @@
 
 describe Champs::RNAChamp do
   let(:champ) { Champs::RNAChamp.new(value: "W182736273", dossier: build(:dossier)) }
-  before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_rna)) }
+  before do
+    allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_rna))
+    allow(champ).to receive(:in_dossier_revision?).and_return(true)
+  end
   def with_value(value)
     champ.tap { _1.value = value }
   end

From 54926db89b592b6ee50b07e00fc9270b90e98bcc Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Thu, 22 Aug 2024 17:35:40 +0200
Subject: [PATCH 05/12] refactor(champ): more concistent champ_for_update usage

---
 app/controllers/champs/champ_controller.rb              | 6 +++++-
 app/models/concerns/dossier_champs_concern.rb           | 2 +-
 spec/models/concerns/dossier_searchable_concern_spec.rb | 6 ++----
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/app/controllers/champs/champ_controller.rb b/app/controllers/champs/champ_controller.rb
index 8d82161bb..5af93d346 100644
--- a/app/controllers/champs/champ_controller.rb
+++ b/app/controllers/champs/champ_controller.rb
@@ -9,7 +9,11 @@ class Champs::ChampController < ApplicationController
   def find_champ
     dossier = policy_scope(Dossier).includes(:champs, revision: [:types_de_champ]).find(params[:dossier_id])
     type_de_champ = dossier.find_type_de_champ_by_stable_id(params[:stable_id])
-    dossier.champ_for_update(type_de_champ, params_row_id, updated_by: current_user.email)
+    if type_de_champ.repetition?
+      dossier.project_champ(type_de_champ, nil)
+    else
+      dossier.champ_for_update(type_de_champ, params_row_id, updated_by: current_user.email)
+    end
   end
 
   def params_row_id
diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb
index 485343e5c..53a6bb96e 100644
--- a/app/models/concerns/dossier_champs_concern.rb
+++ b/app/models/concerns/dossier_champs_concern.rb
@@ -57,7 +57,7 @@ module DossierChampsConcern
       .types_de_champ
       .filter { _1.stable_id.in?(stable_ids) }
       .filter { !revision.child?(_1) }
-      .map { champ_for_update(_1, nil, updated_by: nil) }
+      .map { _1.repetition? ? project_champ(_1, nil) : champ_for_update(_1, nil, updated_by: nil) }
   end
 
   def champ_for_update(type_de_champ, row_id, updated_by:)
diff --git a/spec/models/concerns/dossier_searchable_concern_spec.rb b/spec/models/concerns/dossier_searchable_concern_spec.rb
index 560184c6c..d1b0c1114 100644
--- a/spec/models/concerns/dossier_searchable_concern_spec.rb
+++ b/spec/models/concerns/dossier_searchable_concern_spec.rb
@@ -38,10 +38,8 @@ describe DossierSearchableConcern do
       end
 
       it "update columns en construction" do
-        dossier.update(
-          champs_public_attributes: [{ id: champ_public.id, value: 'nouvelle valeur publique' }],
-          champs_private_attributes: [{ id: champ_private.id, value: 'nouvelle valeur privee' }]
-        )
+        dossier.update_champs_attributes({ champ_public.public_id => { value: 'nouvelle valeur publique' } }, :public, updated_by: 'test')
+        dossier.update_champs_attributes({ champ_private.public_id => { value: 'nouvelle valeur privee' } }, :private, updated_by: 'test')
 
         assert_enqueued_jobs(1, only: DossierIndexSearchTermsJob) do
           dossier.passer_en_construction

From a474cf4590c2e6197db91d7da2ea7178d2b955a3 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Thu, 22 Aug 2024 17:36:31 +0200
Subject: [PATCH 06/12] refactor(attestation): unspecified_champs_for_dossier
 based on types_de_champ

---
 app/models/attestation_template.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/models/attestation_template.rb b/app/models/attestation_template.rb
index 37c8180f9..8d9fdfca4 100644
--- a/app/models/attestation_template.rb
+++ b/app/models/attestation_template.rb
@@ -87,13 +87,13 @@ class AttestationTemplate < ApplicationRecord
   end
 
   def unspecified_champs_for_dossier(dossier)
-    champs_by_stable_id = dossier.champs_for_revision(root: true).index_by { "tdc#{_1.stable_id}" }
+    types_de_champ_by_tag_id = dossier.revision.types_de_champ.index_by { "tdc#{_1.stable_id}" }
 
     used_tags.filter_map do |used_tag|
-      corresponding_champ = champs_by_stable_id[used_tag]
+      corresponding_type_de_champ = types_de_champ_by_tag_id[used_tag]
 
-      if corresponding_champ && corresponding_champ.blank?
-        corresponding_champ
+      if corresponding_type_de_champ && dossier.project_champ(corresponding_type_de_champ, nil).blank?
+        corresponding_type_de_champ
       end
     end
   end

From f02ec3da828a643b76dd5782e1631dcfe436c174 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Thu, 22 Aug 2024 17:36:47 +0200
Subject: [PATCH 07/12] cleanup in spec

---
 .../controllers/champs/repetition_controller_spec.rb |  2 +-
 .../models/concerns/dossier_sections_concern_spec.rb | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/spec/controllers/champs/repetition_controller_spec.rb b/spec/controllers/champs/repetition_controller_spec.rb
index d2c5aca3f..583ec2c88 100644
--- a/spec/controllers/champs/repetition_controller_spec.rb
+++ b/spec/controllers/champs/repetition_controller_spec.rb
@@ -9,7 +9,7 @@ describe Champs::RepetitionController, type: :controller do
     it 'removes repetition' do
       rows, repetitions = dossier.champs.partition(&:child?)
       repetition = repetitions.first
-      expect { delete :remove, params: { dossier_id: repetition.dossier, stable_id: repetition.stable_id, row_id: rows.first.row_id }, format: :turbo_stream }
+      expect { delete :remove, params: { dossier_id: dossier, stable_id: repetition.stable_id, row_id: rows.first.row_id }, format: :turbo_stream }
         .to change { dossier.reload.champs.size }.from(3).to(1)
     end
   end
diff --git a/spec/models/concerns/dossier_sections_concern_spec.rb b/spec/models/concerns/dossier_sections_concern_spec.rb
index 43b0b556f..17f86be88 100644
--- a/spec/models/concerns/dossier_sections_concern_spec.rb
+++ b/spec/models/concerns/dossier_sections_concern_spec.rb
@@ -50,12 +50,12 @@ describe DossierSectionsConcern do
   describe '#index_for_section_header' do
     include Logic
     let(:number_stable_id) { 99 }
-    let(:types_de_champ) {
-  [
-    { type: :header_section, libelle: "Infos" }, { type: :integer_number, stable_id: number_stable_id },
-    { type: :header_section, libelle: "Details", condition: ds_eq(champ_value(99), constant(5)) }, { type: :header_section, libelle: "Conclusion" }
-  ]
-}
+    let(:types_de_champ) do
+      [
+        { type: :header_section, libelle: "Infos" }, { type: :integer_number, stable_id: number_stable_id },
+        { type: :header_section, libelle: "Details", condition: ds_eq(champ_value(99), constant(5)) }, { type: :header_section, libelle: "Conclusion" }
+      ]
+    end
 
     let(:procedure) { create(:procedure, :for_individual, types_de_champ_public: types_de_champ) }
     let(:dossier) { create(:dossier, procedure: procedure) }

From c94d1b7eeaffb1b341b14c80f7a29982f1bcba19 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Thu, 22 Aug 2024 17:38:04 +0200
Subject: [PATCH 08/12] cleanup: more concistent methods usage

---
 app/models/dossier.rb                                         | 4 ++--
 app/models/procedure_revision_type_de_champ.rb                | 4 ++--
 .../dossiers/_instruction_button_motivation.html.haml         | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/models/dossier.rb b/app/models/dossier.rb
index e9304b63e..5b32be941 100644
--- a/app/models/dossier.rb
+++ b/app/models/dossier.rb
@@ -932,7 +932,7 @@ class Dossier < ApplicationRecord
   end
 
   def remove_titres_identite!
-    champs_public.filter(&:titre_identite?).map(&:piece_justificative_file).each(&:purge_later)
+    champs.filter(&:titre_identite?).map(&:piece_justificative_file).each(&:purge_later)
   end
 
   def remove_piece_justificative_file_not_visible!
@@ -1147,7 +1147,7 @@ class Dossier < ApplicationRecord
   end
 
   def has_annotations?
-    revision.revision_types_de_champ_private.present?
+    revision.types_de_champ_private.present?
   end
 
   def hide_info_with_accuse_lecture?
diff --git a/app/models/procedure_revision_type_de_champ.rb b/app/models/procedure_revision_type_de_champ.rb
index 503595143..b8e538c97 100644
--- a/app/models/procedure_revision_type_de_champ.rb
+++ b/app/models/procedure_revision_type_de_champ.rb
@@ -32,8 +32,8 @@ class ProcedureRevisionTypeDeChamp < ApplicationRecord
   end
 
   def siblings
-    if parent_id.present?
-      revision.revision_types_de_champ.where(parent_id: parent_id).ordered
+    if child?
+      revision.revision_types_de_champ.where(parent_id:).ordered
     elsif private?
       revision.revision_types_de_champ_private
     else
diff --git a/app/views/instructeurs/dossiers/_instruction_button_motivation.html.haml b/app/views/instructeurs/dossiers/_instruction_button_motivation.html.haml
index 76b4d576e..56c83c685 100644
--- a/app/views/instructeurs/dossiers/_instruction_button_motivation.html.haml
+++ b/app/views/instructeurs/dossiers/_instruction_button_motivation.html.haml
@@ -12,7 +12,7 @@
       - if unspecified_attestation_champs.present?
         .warning
           Attention, les valeurs suivantes n’ont pas été renseignées mais sont nécessaires pour pouvoir envoyer une attestation valide :
-          - unspecified_annotations_privees, unspecified_champs = unspecified_attestation_champs.partition(&:private)
+          - unspecified_annotations_privees, unspecified_champs = unspecified_attestation_champs.partition(&:private?)
 
           - if unspecified_champs.present?
             %h4 Champs de la demande

From 2ba356bc6863cebbe06956026a60efe994de2db0 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Thu, 22 Aug 2024 17:40:12 +0200
Subject: [PATCH 09/12] cleanup(prefill): remove unused validation

---
 app/models/dossier.rb | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/app/models/dossier.rb b/app/models/dossier.rb
index 5b32be941..e2b64d26f 100644
--- a/app/models/dossier.rb
+++ b/app/models/dossier.rb
@@ -51,7 +51,6 @@ class Dossier < ApplicationRecord
   has_many :champs_to_destroy, -> { order(:parent_id) }, class_name: 'Champ', inverse_of: false, dependent: :destroy
   has_many :champs_public, -> { root.public_only }, class_name: 'Champ', inverse_of: false
   has_many :champs_private, -> { root.private_only }, class_name: 'Champ', inverse_of: false
-  has_many :prefilled_champs_public, -> { root.public_only.prefilled }, class_name: 'Champ', inverse_of: false
 
   has_many :commentaires, inverse_of: :dossier, dependent: :destroy
   has_many :preloaded_commentaires, -> { includes(:dossier_correction, piece_jointe_attachments: :blob) }, class_name: 'Commentaire', inverse_of: :dossier
@@ -424,8 +423,6 @@ class Dossier < ApplicationRecord
   validates :mandataire_last_name, presence: true, if: :for_tiers?
   validates :for_tiers, inclusion: { in: [true, false] }, if: -> { revision&.procedure&.for_individual? }
 
-  validates_associated :prefilled_champs_public, on: :champs_public_value
-
   def types_de_champ_public
     types_de_champ
   end

From 33e502234ed263d9dbc4349ec716d5e0ca1d635c Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Thu, 22 Aug 2024 17:40:36 +0200
Subject: [PATCH 10/12] refactor(dossier loader): simplify all the things

---
 app/models/dossier_preloader.rb | 74 ++++++++++-----------------------
 1 file changed, 23 insertions(+), 51 deletions(-)

diff --git a/app/models/dossier_preloader.rb b/app/models/dossier_preloader.rb
index 7c6dcd4ac..4eea05534 100644
--- a/app/models/dossier_preloader.rb
+++ b/app/models/dossier_preloader.rb
@@ -37,18 +37,12 @@ class DossierPreloader
 
   private
 
-  def revisions
+  def revisions(pj_template: false)
     @revisions ||= ProcedureRevision.where(id: @dossiers.pluck(:revision_id).uniq)
-      .includes(types_de_champ: { piece_justificative_template_attachment: :blob })
+      .includes(types_de_champ: pj_template ? { piece_justificative_template_attachment: :blob } : [])
       .index_by(&:id)
   end
 
-  # returns: { revision_id : { stable_id : position } }
-  def positions
-    @positions ||= revisions
-      .transform_values { |revision| revision.revision_types_de_champ.map { [_1.stable_id, _1.position] }.to_h }
-  end
-
   def load_dossiers(dossiers, pj_template: false)
     to_include = @includes_for_champ.dup
     to_include << [piece_justificative_file_attachments: :blob]
@@ -58,16 +52,10 @@ class DossierPreloader
       .where(dossier_id: dossiers)
       .to_a
 
-    children_champs, root_champs = all_champs.partition(&:child?)
-    champs_by_dossier = root_champs.group_by(&:dossier_id)
-    champs_by_dossier_by_parent = children_champs
-      .group_by(&:dossier_id)
-      .transform_values do |champs|
-        champs.group_by(&:parent_id)
-      end
+    champs_by_dossier = all_champs.group_by(&:dossier_id)
 
     dossiers.each do |dossier|
-      load_dossier(dossier, champs_by_dossier[dossier.id] || [], champs_by_dossier_by_parent[dossier.id] || {})
+      load_dossier(dossier, champs_by_dossier[dossier.id] || [], pj_template:)
     end
 
     load_etablissements(all_champs)
@@ -86,17 +74,30 @@ class DossierPreloader
     end
   end
 
-  def load_dossier(dossier, champs, children_by_parent = {})
-    revision = revisions[dossier.revision_id]
+  def load_dossier(dossier, champs, pj_template: false)
+    revision = revisions(pj_template:)[dossier.revision_id]
     if revision.present?
       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)
 
-    champs_public, champs_private = champs.partition(&:public?)
+    # remove once parent_id is deprecated
+    champs_by_parent_id = champs.group_by(&:parent_id)
 
-    dossier.association(:champs).target = []
-    load_champs(dossier, :champs_public, champs_public, dossier, children_by_parent)
-    load_champs(dossier, :champs_private, champs_private, dossier, children_by_parent)
+    champs.each do |champ|
+      champ.association(:dossier).target = dossier
+
+      # remove once parent_id is deprecated
+      if champ.repetition?
+        children = champs_by_parent_id.fetch(champ.id, [])
+        children.each do |child|
+          child.association(:parent).target = champ
+        end
+        champ.association(:champs).target = children
+      end
+    end
 
     # We need to do this because of the check on `Etablissement#champ` in
     # `Etablissement#libelle_for_export`. By assigning `nil` to `target` we mark association
@@ -105,33 +106,4 @@ class DossierPreloader
       dossier.etablissement.association(:champ).target = nil
     end
   end
-
-  def load_champs(parent, name, champs, dossier, children_by_parent)
-    if champs.empty?
-      parent.association(name).target = [] # tells to Rails association has been loaded
-      return
-    end
-
-    champs.each do |champ|
-      champ.association(:dossier).target = dossier
-
-      if parent.is_a?(Champ)
-        champ.association(:parent).target = parent
-      end
-    end
-
-    dossier.association(:champs).target += champs
-
-    parent.association(name).target = champs
-      .filter { positions[dossier.revision_id][_1.stable_id].present? }
-      .sort_by { [_1.row_id, positions[dossier.revision_id][_1.stable_id]] }
-
-    # Load children champs
-    champs.filter(&:block?).each do |parent_champ|
-      champs = children_by_parent[parent_champ.id] || []
-      parent_champ.association(:dossier).target = dossier
-
-      load_champs(parent_champ, :champs, champs, dossier, children_by_parent)
-    end
-  end
 end

From 54132df77d3497fe68c548788452f52dd00f838c Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Mon, 16 Sep 2024 12:26:37 +0200
Subject: [PATCH 11/12] dossier(champ): validate presence of row_ids

---
 app/models/concerns/dossier_champs_concern.rb | 13 +++++++++
 app/models/procedure_revision.rb              |  2 +-
 .../concerns/dossier_champs_concern_spec.rb   | 27 +++++++++++++------
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb
index 53a6bb96e..7ab2dc65d 100644
--- a/app/models/concerns/dossier_champs_concern.rb
+++ b/app/models/concerns/dossier_champs_concern.rb
@@ -33,6 +33,7 @@ module DossierChampsConcern
   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)]
     if champ.nil?
       type_de_champ.build_champ(dossier: self, row_id:)
@@ -81,6 +82,7 @@ module DossierChampsConcern
   end
 
   def champ_for_export(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)]
     if champ.blank? || !champ.visible?
       nil
@@ -96,6 +98,7 @@ module DossierChampsConcern
   end
 
   def champ_with_attributes_for_update(type_de_champ, row_id, updated_by:)
+    check_valid_row_id?(type_de_champ, row_id)
     attributes = type_de_champ.params_for_champ
     # TODO: Once we have the right index in place, we should change this to use `create_or_find_by` instead of `find_or_create_by`
     champ = champs
@@ -124,4 +127,14 @@ module DossierChampsConcern
 
     [champ, attributes]
   end
+
+  def check_valid_row_id?(type_de_champ, row_id)
+    if revision.child?(type_de_champ)
+      if row_id.blank?
+        raise "type_de_champ #{type_de_champ.stable_id} must have a row_id because it is part of a repetition"
+      end
+    elsif row_id.present?
+      raise "type_de_champ #{type_de_champ.stable_id} can not have a row_id because it is not part of a repetition"
+    end
+  end
 end
diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb
index c1181ad02..d1073e5e5 100644
--- a/app/models/procedure_revision.rb
+++ b/app/models/procedure_revision.rb
@@ -221,7 +221,7 @@ class ProcedureRevision < ApplicationRecord
 
   def child?(tdc)
     revision_types_de_champ
-      .find { _1.type_de_champ_id == tdc.id }.child?
+      .find { _1.type_de_champ_id == tdc.id }&.child?
   end
 
   def remove_children_of(tdc)
diff --git a/spec/models/concerns/dossier_champs_concern_spec.rb b/spec/models/concerns/dossier_champs_concern_spec.rb
index ce6581fbb..5e68d5e99 100644
--- a/spec/models/concerns/dossier_champs_concern_spec.rb
+++ b/spec/models/concerns/dossier_champs_concern_spec.rb
@@ -35,7 +35,6 @@ RSpec.describe DossierChampsConcern do
     let(:type_de_champ_repetition) { dossier.find_type_de_champ_by_stable_id(993) }
     let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(99) }
     let(:type_de_champ_private) { dossier.find_type_de_champ_by_stable_id(995) }
-    let(:row_ids) { dossier.project_champ(type_de_champ_repetition, nil).row_ids }
 
     context "public champ" do
       let(:row_id) { nil }
@@ -45,13 +44,20 @@ RSpec.describe DossierChampsConcern do
 
       context "in repetition" do
         let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(994) }
-        let(:row_id) { row_ids.first }
+        let(:row_id) { dossier.project_champ(type_de_champ_repetition, nil).row_ids.first }
 
         it {
           expect(subject.persisted?).to be_truthy
           expect(subject.row_id).to eq(row_id)
           expect(subject.parent_id).not_to be_nil
         }
+
+        context "invalid row_id" do
+          let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(99) }
+          it {
+            expect { subject }.to raise_error("type_de_champ #{type_de_champ_public.stable_id} can not have a row_id because it is not part of a repetition")
+          }
+        end
       end
 
       context "missing champ" do
@@ -64,13 +70,20 @@ RSpec.describe DossierChampsConcern do
 
         context "in repetition" do
           let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(994) }
-          let(:row_id) { row_ids.first }
+          let(:row_id) { ULID.generate }
 
           it {
             expect(subject.new_record?).to be_truthy
             expect(subject.is_a?(Champs::TextChamp)).to be_truthy
             expect(subject.row_id).to eq(row_id)
           }
+
+          context "invalid row_id" do
+            let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(99) }
+            it {
+              expect { subject }.to raise_error("type_de_champ #{type_de_champ_public.stable_id} can not have a row_id because it is not part of a repetition")
+            }
+          end
         end
       end
     end
@@ -122,7 +135,6 @@ RSpec.describe DossierChampsConcern do
     let(:type_de_champ_repetition) { dossier.find_type_de_champ_by_stable_id(993) }
     let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(99) }
     let(:type_de_champ_private) { dossier.find_type_de_champ_by_stable_id(995) }
-    let(:row_ids) { dossier.project_champ(type_de_champ_repetition, nil).row_ids }
     let(:row_id) { nil }
 
     context "public champ" do
@@ -135,7 +147,7 @@ RSpec.describe DossierChampsConcern do
 
       context "in repetition" do
         let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(994) }
-        let(:row_id) { row_ids.first }
+        let(:row_id) { ULID.generate }
 
         it {
           expect(subject.persisted?).to be_truthy
@@ -154,7 +166,7 @@ RSpec.describe DossierChampsConcern do
 
         context "in repetition" do
           let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(994) }
-          let(:row_id) { row_ids.first }
+          let(:row_id) { ULID.generate }
 
           it {
             expect(subject.persisted?).to be_truthy
@@ -178,8 +190,7 @@ RSpec.describe DossierChampsConcern do
 
   describe "#update_champs_attributes(public)" do
     let(:type_de_champ_repetition) { dossier.find_type_de_champ_by_stable_id(993) }
-    let(:row_ids) { dossier.project_champ(type_de_champ_repetition, nil).row_ids }
-    let(:row_id) { row_ids.first }
+    let(:row_id) { ULID.generate }
 
     let(:attributes) do
       {

From 3fef83152446593ba07e36b8b6aecc17255c1e6d Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Mon, 16 Sep 2024 17:03:40 +0200
Subject: [PATCH 12/12] move child? method for clarity

---
 app/models/concerns/dossier_champs_concern.rb   | 4 ++--
 app/models/concerns/dossier_sections_concern.rb | 2 +-
 app/models/procedure_revision.rb                | 5 -----
 app/models/type_de_champ.rb                     | 4 ++++
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb
index 7ab2dc65d..8e1f3d57a 100644
--- a/app/models/concerns/dossier_champs_concern.rb
+++ b/app/models/concerns/dossier_champs_concern.rb
@@ -57,7 +57,7 @@ module DossierChampsConcern
     revision
       .types_de_champ
       .filter { _1.stable_id.in?(stable_ids) }
-      .filter { !revision.child?(_1) }
+      .filter { !_1.child?(revision) }
       .map { _1.repetition? ? project_champ(_1, nil) : champ_for_update(_1, nil, updated_by: nil) }
   end
 
@@ -129,7 +129,7 @@ module DossierChampsConcern
   end
 
   def check_valid_row_id?(type_de_champ, row_id)
-    if revision.child?(type_de_champ)
+    if type_de_champ.child?(revision)
       if row_id.blank?
         raise "type_de_champ #{type_de_champ.stable_id} must have a row_id because it is part of a repetition"
       end
diff --git a/app/models/concerns/dossier_sections_concern.rb b/app/models/concerns/dossier_sections_concern.rb
index 679e5b896..c4644dba3 100644
--- a/app/models/concerns/dossier_sections_concern.rb
+++ b/app/models/concerns/dossier_sections_concern.rb
@@ -19,7 +19,7 @@ module DossierSectionsConcern
     end
 
     def auto_numbering_section_headers_for?(type_de_champ)
-      return false if revision.child?(type_de_champ)
+      return false if type_de_champ.child?(revision)
 
       sections_for(type_de_champ)&.none? { _1.libelle =~ /^\d/ }
     end
diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb
index d1073e5e5..88c9f7dbf 100644
--- a/app/models/procedure_revision.rb
+++ b/app/models/procedure_revision.rb
@@ -219,11 +219,6 @@ class ProcedureRevision < ApplicationRecord
       .find { _1.type_de_champ_id == tdc.id }.parent&.type_de_champ
   end
 
-  def child?(tdc)
-    revision_types_de_champ
-      .find { _1.type_de_champ_id == tdc.id }&.child?
-  end
-
   def remove_children_of(tdc)
     children_of(tdc).each do |child|
       remove_type_de_champ(child.stable_id)
diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb
index 0fd8333df..8217a53c3 100644
--- a/app/models/type_de_champ.rb
+++ b/app/models/type_de_champ.rb
@@ -456,6 +456,10 @@ class TypeDeChamp < ApplicationRecord
     !private?
   end
 
+  def child?(revision)
+    revision.revision_types_de_champ.find { _1.type_de_champ_id == id }&.child?
+  end
+
   def filename_for_attachement(attachment_sym)
     attachment = send(attachment_sym)
     if attachment.attached?