From 3da8c4b672691fb2ac37c3dc82f91446bc242a6c Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Thu, 10 Oct 2024 11:01:20 +0200
Subject: [PATCH 01/10] feat(repetition): make champ repetition discardable

---
 app/models/champs/repetition_champ.rb                   | 8 ++++++++
 db/migrate/20241007174301_add_discarded_at_to_champs.rb | 7 +++++++
 db/schema.rb                                            | 1 +
 3 files changed, 16 insertions(+)
 create mode 100644 db/migrate/20241007174301_add_discarded_at_to_champs.rb

diff --git a/app/models/champs/repetition_champ.rb b/app/models/champs/repetition_champ.rb
index 0e9e8ef64..185aaa258 100644
--- a/app/models/champs/repetition_champ.rb
+++ b/app/models/champs/repetition_champ.rb
@@ -23,6 +23,14 @@ class Champs::RepetitionChamp < Champ
     rows.last&.first&.focusable_input_id
   end
 
+  def discarded?
+    discarded_at.present?
+  end
+
+  def discard!
+    touch(:discarded_at)
+  end
+
   def search_terms
     # The user cannot enter any information here so it doesn’t make much sense to search
   end
diff --git a/db/migrate/20241007174301_add_discarded_at_to_champs.rb b/db/migrate/20241007174301_add_discarded_at_to_champs.rb
new file mode 100644
index 000000000..c25e4c585
--- /dev/null
+++ b/db/migrate/20241007174301_add_discarded_at_to_champs.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddDiscardedAtToChamps < ActiveRecord::Migration[7.0]
+  def change
+    add_column :champs, :discarded_at, :datetime
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 756cedcaf..e0bd12e54 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -250,6 +250,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_11_26_145420) do
   create_table "champs", id: :serial, force: :cascade do |t|
     t.datetime "created_at", precision: nil
     t.jsonb "data"
+    t.datetime "discarded_at"
     t.integer "dossier_id"
     t.integer "etablissement_id"
     t.string "external_id"

From 7ee90b6d85f8afbaec81bb544e1c2a5c1644cb84 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Tue, 26 Nov 2024 22:05:26 +0100
Subject: [PATCH 02/10] refactor(champ): validate only champs in revision

---
 app/models/champ.rb                           |  9 +++-
 app/models/concerns/champ_revision_concern.rb | 23 +++++++++
 ...e_concern.rb => champ_validate_concern.rb} | 14 +++---
 app/models/concerns/dossier_champs_concern.rb | 14 +++++-
 app/models/procedure_revision.rb              |  4 --
 app/models/type_de_champ.rb                   |  4 --
 spec/models/champs/epci_champ_spec.rb         |  2 +-
 .../champs/integer_number_champ_spec.rb       |  2 +-
 ...spec.rb => champ_validate_concern_spec.rb} | 47 +++++++++++++++++--
 9 files changed, 96 insertions(+), 23 deletions(-)
 create mode 100644 app/models/concerns/champ_revision_concern.rb
 rename app/models/concerns/{champs_validate_concern.rb => champ_validate_concern.rb} (70%)
 rename spec/models/concerns/{champs_validate_concern_spec.rb => champ_validate_concern_spec.rb} (58%)

diff --git a/app/models/champ.rb b/app/models/champ.rb
index e58dd4fb9..5a9e79520 100644
--- a/app/models/champ.rb
+++ b/app/models/champ.rb
@@ -2,7 +2,8 @@
 
 class Champ < ApplicationRecord
   include ChampConditionalConcern
-  include ChampsValidateConcern
+  include ChampValidateConcern
+  include ChampRevisionConcern
 
   self.ignored_columns += [:type_de_champ_id, :parent_id]
 
@@ -69,7 +70,11 @@ class Champ < ApplicationRecord
   end
 
   def child?
-    row_id.present?
+    row_id.present? && !is_type?(TypeDeChamp.type_champs.fetch(:repetition))
+  end
+
+  def row?
+    row_id.present? && is_type?(TypeDeChamp.type_champs.fetch(:repetition))
   end
 
   # used for the `required` html attribute
diff --git a/app/models/concerns/champ_revision_concern.rb b/app/models/concerns/champ_revision_concern.rb
new file mode 100644
index 000000000..04a346754
--- /dev/null
+++ b/app/models/concerns/champ_revision_concern.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module ChampRevisionConcern
+  extend ActiveSupport::Concern
+
+  protected
+
+  def is_same_type_as_revision?
+    is_type?(type_de_champ.type_champ)
+  end
+
+  def in_dossier_revision?
+    dossier.stable_id_in_revision?(stable_id)
+  end
+
+  def in_discarded_row?
+    if child?
+      repetition_type_de_champ = dossier.revision.parent_of(type_de_champ)
+      row_ids = dossier.repetition_row_ids(repetition_type_de_champ)
+      !row_id.in?(row_ids)
+    end
+  end
+end
diff --git a/app/models/concerns/champs_validate_concern.rb b/app/models/concerns/champ_validate_concern.rb
similarity index 70%
rename from app/models/concerns/champs_validate_concern.rb
rename to app/models/concerns/champ_validate_concern.rb
index 97c0c791e..a31cfe8b9 100644
--- a/app/models/concerns/champs_validate_concern.rb
+++ b/app/models/concerns/champ_validate_concern.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module ChampsValidateConcern
+module ChampValidateConcern
   extend ActiveSupport::Concern
 
   included do
@@ -16,20 +16,20 @@ module ChampsValidateConcern
     def validate_champ_value?
       case validation_context
       when :champs_public_value
-        public? && in_dossier_revision? && visible?
+        public? && can_validate? && visible?
       when :champs_private_value
-        private? && in_dossier_revision? && visible?
+        private? && can_validate? && visible?
       else
         false
       end
     end
 
+    def can_validate?
+      in_dossier_revision? && is_same_type_as_revision? && !row? && !in_discarded_row?
+    end
+
     def validate_champ_value_or_prefill?
       validate_champ_value? || validation_context == :prefill
     end
-
-    def in_dossier_revision?
-      dossier.revision.in_revision?(stable_id) && is_type?(type_de_champ.type_champ)
-    end
   end
 end
diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb
index 6392393a1..406521638 100644
--- a/app/models/concerns/dossier_champs_concern.rb
+++ b/app/models/concerns/dossier_champs_concern.rb
@@ -135,6 +135,10 @@ module DossierChampsConcern
     reload_champs_cache
   end
 
+  def stable_id_in_revision?(stable_id)
+    revision_stable_ids.member?(stable_id.to_i)
+  end
+
   def reload
     super.tap { reset_champs_cache }
   end
@@ -142,7 +146,15 @@ module DossierChampsConcern
   private
 
   def champs_by_public_id
-    @champs_by_public_id ||= champs.sort_by(&:id).index_by(&:public_id)
+    @champs_by_public_id ||= champs_in_revision.index_by(&:public_id)
+  end
+
+  def revision_stable_ids
+    @revision_stable_ids ||= revision.types_de_champ.map(&:stable_id).to_set
+  end
+
+  def champs_in_revision
+    champs.filter { stable_id_in_revision?(_1.stable_id) }
   end
 
   def filled_champ(type_de_champ, row_id)
diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb
index cffc1c96b..94f896d62 100644
--- a/app/models/procedure_revision.rb
+++ b/app/models/procedure_revision.rb
@@ -203,10 +203,6 @@ class ProcedureRevision < ApplicationRecord
     coordinate_for(tdc).parent_type_de_champ
   end
 
-  def in_revision?(stable_id)
-    types_de_champ.any? { _1.stable_id == stable_id }
-  end
-
   def dependent_conditions(tdc)
     stable_id = tdc.stable_id
 
diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb
index 707848b12..bf4a0046d 100644
--- a/app/models/type_de_champ.rb
+++ b/app/models/type_de_champ.rb
@@ -331,10 +331,6 @@ class TypeDeChamp < ApplicationRecord
     !private?
   end
 
-  def in_revision?(revision)
-    revision.types_de_champ.any? { _1.stable_id == stable_id }
-  end
-
   def child?(revision)
     revision.coordinate_for(self)&.child?
   end
diff --git a/spec/models/champs/epci_champ_spec.rb b/spec/models/champs/epci_champ_spec.rb
index 58d7b7868..4c07d7bbd 100644
--- a/spec/models/champs/epci_champ_spec.rb
+++ b/spec/models/champs/epci_champ_spec.rb
@@ -8,7 +8,7 @@ describe Champs::EpciChamp, type: :model do
       let(:champ) { Champs::EpciChamp.new(code_departement: code_departement, dossier: build(:dossier)) }
       before do
         allow(champ).to receive(:visible?).and_return(true)
-        allow(champ).to receive(:in_dossier_revision?).and_return(true)
+        allow(champ).to receive(:can_validate?).and_return(true)
       end
       context 'when nil' do
         let(:code_departement) { nil }
diff --git a/spec/models/champs/integer_number_champ_spec.rb b/spec/models/champs/integer_number_champ_spec.rb
index 8a5d2182f..e46c0cdb4 100644
--- a/spec/models/champs/integer_number_champ_spec.rb
+++ b/spec/models/champs/integer_number_champ_spec.rb
@@ -4,7 +4,7 @@ describe Champs::IntegerNumberChamp do
   let(:champ) { Champs::IntegerNumberChamp.new(value:, dossier: build(:dossier)) }
   before do
     allow(champ).to receive(:visible?).and_return(true)
-    allow(champ).to receive(:in_dossier_revision?).and_return(true)
+    allow(champ).to receive(:can_validate?).and_return(true)
   end
   subject { champ.validate(:champs_public_value) }
 
diff --git a/spec/models/concerns/champs_validate_concern_spec.rb b/spec/models/concerns/champ_validate_concern_spec.rb
similarity index 58%
rename from spec/models/concerns/champs_validate_concern_spec.rb
rename to spec/models/concerns/champ_validate_concern_spec.rb
index edf0e5224..83f5401b4 100644
--- a/spec/models/concerns/champs_validate_concern_spec.rb
+++ b/spec/models/concerns/champ_validate_concern_spec.rb
@@ -1,15 +1,15 @@
 # frozen_string_literal: true
 
-RSpec.describe ChampsValidateConcern do
+RSpec.describe ChampValidateConcern do
   let(:procedure) { create(:procedure, :published, types_de_champ_public:) }
   let(:dossier) { create(:dossier, :with_populated_champs, procedure:) }
   let(:type_de_champ) { dossier.revision.types_de_champ_public.first }
-
+  let(:public_id) { type_de_champ.public_id(nil) }
   let(:types_de_champ_public) { [{ type: :email }] }
 
   def update_champ(value)
     dossier.update_champs_attributes({
-      type_de_champ.stable_id.to_s => { value: }
+      public_id => { value: }
     }, :public, updated_by: 'test')
     dossier.save
   end
@@ -68,4 +68,45 @@ RSpec.describe ChampsValidateConcern do
       }
     end
   end
+
+  context 'when in a row' do
+    let(:types_de_champ_public) { [{ type: :repetition, children: [{ type: :email }], mandatory: true }] }
+    let(:type_de_champ_in_repetition) { dossier.revision.children_of(type_de_champ).first }
+    let(:row_id) { dossier.repetition_row_ids(type_de_champ).first }
+    let(:public_id) { type_de_champ_in_repetition.public_id(row_id) }
+
+    context 'valid' do
+      before {
+        update_champ('test@test.com')
+        dossier.validate(:champs_public_value)
+      }
+      it {
+        expect(dossier.champs).not_to be_empty
+        expect(dossier.errors).to be_empty
+      }
+    end
+
+    context 'invalid' do
+      before {
+        update_champ('test')
+        dossier.validate(:champs_public_value)
+      }
+      it {
+        expect(dossier.champs).not_to be_empty
+        expect(dossier.errors).not_to be_empty
+      }
+    end
+
+    context 'do not validate when in discarded row' do
+      before {
+        update_champ('test')
+        dossier.repetition_remove_row(type_de_champ, row_id, updated_by: 'test')
+        dossier.validate(:champs_public_value)
+      }
+      it {
+        expect(dossier.champs).not_to be_empty
+        expect(dossier.errors).to be_empty
+      }
+    end
+  end
 end

From 5f10486a519940f3bfa9d6059951d412541426a2 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Tue, 26 Nov 2024 22:07:38 +0100
Subject: [PATCH 03/10] refactor(dossier): create only fillable champs on new
 dossiers

---
 app/models/dossier.rb     | 12 +++++------
 spec/factories/champ.rb   | 28 -------------------------
 spec/factories/dossier.rb | 43 ++++++++++++++++++++++++++-------------
 3 files changed, 35 insertions(+), 48 deletions(-)

diff --git a/app/models/dossier.rb b/app/models/dossier.rb
index 2cc8a28e3..44f3b973d 100644
--- a/app/models/dossier.rb
+++ b/app/models/dossier.rb
@@ -1058,13 +1058,13 @@ class Dossier < ApplicationRecord
   end
 
   def build_default_champs_for(types_de_champ)
-    self.champs << types_de_champ.flat_map do |type_de_champ|
-      champ = type_de_champ.build_champ(dossier: self)
-      if type_de_champ.repetition? && (type_de_champ.private? || type_de_champ.mandatory?)
-        row_id = ULID.generate
-        [champ] + revision.children_of(type_de_champ).map { _1.build_champ(dossier: self, row_id:) }
+    self.champs << types_de_champ.filter(&:fillable?).filter_map do |type_de_champ|
+      if type_de_champ.repetition?
+        if type_de_champ.private? || type_de_champ.mandatory?
+          type_de_champ.build_champ(dossier: self, row_id: ULID.generate)
+        end
       else
-        champ
+        type_de_champ.build_champ(dossier: self)
       end
     end
   end
diff --git a/spec/factories/champ.rb b/spec/factories/champ.rb
index ed0fad2a2..11bcf886c 100644
--- a/spec/factories/champ.rb
+++ b/spec/factories/champ.rb
@@ -94,14 +94,6 @@ FactoryBot.define do
       external_id { '200071991' }
     end
 
-    factory :champ_do_not_use_header_section, class: 'Champs::HeaderSectionChamp' do
-      value { 'une section' }
-    end
-
-    factory :champ_do_not_use_explication, class: 'Champs::ExplicationChamp' do
-      value { '' }
-    end
-
     factory :champ_do_not_use_dossier_link, class: 'Champs::DossierLinkChamp' do
       value { create(:dossier, :en_construction).id }
     end
@@ -188,25 +180,5 @@ FactoryBot.define do
 
     factory :champ_do_not_use_expression_reguliere, class: 'Champs::ExpressionReguliereChamp' do
     end
-
-    factory :champ_do_not_use_repetition, class: 'Champs::RepetitionChamp' do
-      transient do
-        rows { 2 }
-      end
-
-      after(:build) do |champ_repetition, evaluator|
-        revision = champ_repetition.procedure.active_revision
-        parent = revision.revision_types_de_champ.find { _1.type_de_champ == champ_repetition.type_de_champ }
-        types_de_champ = revision.revision_types_de_champ.filter { _1.parent == parent }.map(&:type_de_champ)
-
-        evaluator.rows.times do
-          row_id = ULID.generate
-          champ_repetition.dossier.champs << types_de_champ.map do |type_de_champ|
-            attrs = { dossier: champ_repetition.dossier, private: champ_repetition.private?, stable_id: type_de_champ.stable_id, row_id: }
-            build(:"champ_do_not_use_#{type_de_champ.type_champ}", **attrs)
-          end
-        end
-      end
-    end
   end
 end
diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb
index f2a6b799f..47a443dfb 100644
--- a/spec/factories/dossier.rb
+++ b/spec/factories/dossier.rb
@@ -24,25 +24,13 @@ FactoryBot.define do
     after(:create) do |dossier, evaluator|
       if evaluator.populate_champs
         dossier.revision.types_de_champ_public.each do |type_de_champ|
-          value = if type_de_champ.drop_down_list?
-            type_de_champ.drop_down_options.first
-          elsif type_de_champ.multiple_drop_down_list?
-            type_de_champ.drop_down_options.first(2).to_json
-          end
-          attrs = { stable_id: type_de_champ.stable_id, dossier:, value: }.compact
-          create(:"champ_do_not_use_#{type_de_champ.type_champ}", **attrs)
+          dossier_factory_create_champ_or_repetition(type_de_champ, dossier)
         end
       end
 
       if evaluator.populate_annotations
         dossier.revision.types_de_champ_private.each do |type_de_champ|
-          value = if type_de_champ.drop_down_list?
-            type_de_champ.drop_down_options.first
-          elsif type_de_champ.multiple_drop_down_list?
-            type_de_champ.drop_down_options.first(2).to_json
-          end
-          attrs = { stable_id: type_de_champ.stable_id, dossier:, private: true, value: }.compact
-          create(:"champ_do_not_use_#{type_de_champ.type_champ}", **attrs)
+          dossier_factory_create_champ_or_repetition(type_de_champ, dossier)
         end
       end
 
@@ -301,3 +289,30 @@ FactoryBot.define do
     end
   end
 end
+
+def dossier_factory_create_champ_or_repetition(type_de_champ, dossier)
+  if type_de_champ.repetition?
+    types_de_champ = dossier.revision.children_of(type_de_champ)
+    2.times do
+      row_id = ULID.generate
+      type_de_champ.build_champ(dossier:, row_id:).save!
+      types_de_champ.each do |type_de_champ|
+        dossier_factory_create_champ(type_de_champ, dossier, row_id:)
+      end
+    end
+  else
+    dossier_factory_create_champ(type_de_champ, dossier)
+  end
+end
+
+def dossier_factory_create_champ(type_de_champ, dossier, row_id: nil)
+  return unless type_de_champ.fillable?
+
+  value = if type_de_champ.drop_down_list?
+    type_de_champ.drop_down_options.first
+  elsif type_de_champ.multiple_drop_down_list?
+    type_de_champ.drop_down_options.first(2).to_json
+  end
+  attrs = { stable_id: type_de_champ.stable_id, private: type_de_champ.private?, row_id:, dossier:, value: }.compact
+  create(:"champ_do_not_use_#{type_de_champ.type_champ}", **attrs)
+end

From 8e582e0be7fdeb225a44aa93a93f3c91e2686b06 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Tue, 26 Nov 2024 22:08:58 +0100
Subject: [PATCH 04/10] refactor(repetition): materialize repetition rows in db

---
 .../instructeurs/dossiers_controller.rb       |  2 +-
 app/controllers/users/dossiers_controller.rb  |  2 +-
 app/models/concerns/dossier_champs_concern.rb | 83 ++++++++++++++-----
 .../concerns/dossier_champs_concern_spec.rb   | 19 ++---
 4 files changed, 70 insertions(+), 36 deletions(-)

diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb
index f862f3d7a..62a2fc575 100644
--- a/app/controllers/instructeurs/dossiers_controller.rb
+++ b/app/controllers/instructeurs/dossiers_controller.rb
@@ -308,7 +308,7 @@ module Instructeurs
 
       respond_to do |format|
         format.turbo_stream do
-          @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_private_attributes_params, dossier.champs.filter(&:private?))
+          @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_private_attributes_params, dossier.project_champs_private_all)
         end
       end
     end
diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb
index b07169338..1fff9841a 100644
--- a/app/controllers/users/dossiers_controller.rb
+++ b/app/controllers/users/dossiers_controller.rb
@@ -286,7 +286,7 @@ module Users
 
       respond_to do |format|
         format.turbo_stream do
-          @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_public_attributes_params, dossier.champs.filter(&:public?))
+          @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_public_attributes_params, dossier.project_champs_public_all)
           render :update, layout: false
         end
       end
diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb
index 406521638..75871fd5f 100644
--- a/app/models/concerns/dossier_champs_concern.rb
+++ b/app/models/concerns/dossier_champs_concern.rb
@@ -4,7 +4,7 @@ module DossierChampsConcern
   extend ActiveSupport::Concern
 
   def project_champ(type_de_champ, row_id)
-    check_valid_row_id?(type_de_champ, row_id)
+    check_valid_row_id_on_read?(type_de_champ, row_id)
     champ = champs_by_public_id[type_de_champ.public_id(row_id)]
     if champ.nil? || !champ.is_type?(type_de_champ.type_champ)
       value = type_de_champ.champ_blank?(champ) ? nil : champ.value
@@ -52,6 +52,28 @@ module DossierChampsConcern
     filled_champs_public + filled_champs_private
   end
 
+  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
+
+  def project_champs_private_all
+    revision.types_de_champ_private.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
+
   def project_rows_for(type_de_champ)
     return [] if !type_de_champ.repetition?
 
@@ -83,7 +105,11 @@ module DossierChampsConcern
   end
 
   def champ_value_for_tag(type_de_champ, path = :value)
-    champ = filled_champ(type_de_champ, nil)
+    champ = if type_de_champ.repetition?
+      project_champ(type_de_champ, nil)
+    else
+      filled_champ(type_de_champ, nil)
+    end
     type_de_champ.champ_value_for_tag(champ, path)
   end
 
@@ -109,30 +135,38 @@ module DossierChampsConcern
 
   def repetition_row_ids(type_de_champ)
     return [] if !type_de_champ.repetition?
-    return [] if !revision.in_revision?(type_de_champ.stable_id)
+    return [] unless stable_id_in_revision?(type_de_champ.stable_id)
+    @repetition_row_ids ||= {}
+    @repetition_row_ids[type_de_champ.stable_id] ||= begin
+      rows = champs_in_revision.filter { _1.row? && _1.stable_id == type_de_champ.stable_id }
+      row_ids = rows.reject(&:discarded?).map(&:row_id)
 
-    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
+      # Legacy rows are rows that have been created before the introduction of the discarded_at column
+      # TODO migrate and clean
+      children_stable_ids = revision.children_of(type_de_champ).map(&:stable_id)
+      discarded_row_ids = rows.filter(&:discarded?).map(&:row_id)
+      legacy_row_ids = champs_in_revision.filter { _1.stable_id.in?(children_stable_ids) && _1.row_id.present? }.map(&:row_id).uniq
+      row_ids += (legacy_row_ids - discarded_row_ids)
+      row_ids.uniq.sort
+    end
   end
 
   def repetition_add_row(type_de_champ, updated_by:)
     raise "Can't add row to non-repetition type de champ" if !type_de_champ.repetition?
 
     row_id = ULID.generate
-    types_de_champ = revision.children_of(type_de_champ)
-    self.champs += types_de_champ.map { _1.build_champ(row_id:, updated_by:) }
-    reload_champs_cache
+    champ = champ_for_update(type_de_champ, row_id, updated_by:)
+    champ.save!
+    reset_champ_cache(champ)
     row_id
   end
 
   def repetition_remove_row(type_de_champ, row_id, updated_by:)
     raise "Can't remove row from non-repetition type de champ" if !type_de_champ.repetition?
 
-    champs.where(row_id:).destroy_all
-    reload_champs_cache
+    champ = champ_for_update(type_de_champ, row_id, updated_by:)
+    champ.discard!
+    reset_champ_cache(champ)
   end
 
   def stable_id_in_revision?(stable_id)
@@ -173,7 +207,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)
+    check_valid_row_id_on_write?(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
@@ -198,12 +232,22 @@ module DossierChampsConcern
     [champ, attributes]
   end
 
-  def check_valid_row_id?(type_de_champ, row_id)
+  def check_valid_row_id_on_write?(type_de_champ, row_id)
+    if type_de_champ.repetition?
+      if row_id.blank?
+        raise "type_de_champ #{type_de_champ.stable_id} in revision #{revision_id} must have a row_id because it represents a row in a repetition"
+      end
+    else
+      check_valid_row_id_on_read?(type_de_champ, row_id)
+    end
+  end
+
+  def check_valid_row_id_on_read?(type_de_champ, row_id)
     if type_de_champ.child?(revision)
       if row_id.blank?
         raise "type_de_champ #{type_de_champ.stable_id} in revision #{revision_id} must have a row_id because it is part of a repetition"
       end
-    elsif row_id.present? && type_de_champ.in_revision?(revision)
+    elsif row_id.present? && stable_id_in_revision?(type_de_champ.stable_id)
       raise "type_de_champ #{type_de_champ.stable_id} in revision #{revision_id} can not have a row_id because it is not part of a repetition"
     end
   end
@@ -214,15 +258,12 @@ module DossierChampsConcern
     @filled_champs_private = nil
     @project_champs_public = nil
     @project_champs_private = nil
+    @repetition_row_ids = nil
+    @revision_stable_ids = nil
   end
 
   def reset_champ_cache(champ)
     champs_by_public_id[champ.public_id]&.reload
     reset_champs_cache
   end
-
-  def reload_champs_cache
-    champs.reload if persisted?
-    reset_champs_cache
-  end
 end
diff --git a/spec/models/concerns/dossier_champs_concern_spec.rb b/spec/models/concerns/dossier_champs_concern_spec.rb
index 4fe86b3a7..7d5df328e 100644
--- a/spec/models/concerns/dossier_champs_concern_spec.rb
+++ b/spec/models/concerns/dossier_champs_concern_spec.rb
@@ -1,9 +1,7 @@
 # frozen_string_literal: true
 
 RSpec.describe DossierChampsConcern do
-  let(:procedure) do
-    create(:procedure, types_de_champ_public:, types_de_champ_private:)
-  end
+  let(:procedure) { create(:procedure, types_de_champ_public:, types_de_champ_private:) }
   let(:types_de_champ_public) do
     [
       { type: :text, libelle: "Un champ text", stable_id: 99 },
@@ -47,7 +45,7 @@ RSpec.describe DossierChampsConcern do
         let(:row_id) { dossier.project_champ(type_de_champ_repetition, nil).row_ids.first }
 
         it {
-          expect(subject.persisted?).to be_truthy
+          expect(subject.new_record?).to be_truthy
           expect(subject.row_id).to eq(row_id)
         }
 
@@ -130,10 +128,11 @@ RSpec.describe DossierChampsConcern do
         { type: :explication }
       ]
     end
+    let(:dossier) { create(:dossier, :with_populated_champs, procedure:) }
     subject { dossier.filled_champs_public }
 
-    it { expect(subject.size).to eq(4) }
-    it { expect(subject.find { _1.libelle == 'Nom' }).to be_truthy }
+    it { expect(subject.size).to eq(5) }
+    it { expect(subject.filter { _1.libelle == 'Nom' }.size).to eq(2) }
   end
 
   describe '#filled_champs_private' do
@@ -156,14 +155,8 @@ RSpec.describe DossierChampsConcern do
     it { expect(subject.size).to eq(1) }
 
     context 'given a type de champ repetition in another revision' do
-      let(:procedure) { create(:procedure, :published, types_de_champ_public:, types_de_champ_private:) }
-      let(:draft) { procedure.draft_revision }
-      let(:errored_stable_id) { 666 }
-      let(:type_de_champ_repetition) { procedure.active_revision.types_de_champ.find { _1.stable_id == errored_stable_id } }
       before do
-        dossier
-        tdc_repetition = draft.add_type_de_champ(type_champ: :repetition, libelle: "repetition", stable_id: errored_stable_id)
-        draft.add_type_de_champ(type_champ: :text, libelle: "t1", parent_stable_id: tdc_repetition.stable_id)
+        procedure.draft_revision.remove_type_de_champ(type_de_champ_repetition.stable_id)
         procedure.publish_revision!
       end
 

From 059488740183489469c8461188682ee51744090c Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Tue, 26 Nov 2024 22:12:07 +0100
Subject: [PATCH 05/10] refactor(dossier): update clone and apply diff to work
 with rows

---
 app/models/concerns/dossier_clone_concern.rb  | 50 +++++++------
 .../concerns/dossier_clone_concern_spec.rb    | 73 ++++++++++++++++---
 2 files changed, 90 insertions(+), 33 deletions(-)

diff --git a/app/models/concerns/dossier_clone_concern.rb b/app/models/concerns/dossier_clone_concern.rb
index 6f5aab6f2..582dec0d5 100644
--- a/app/models/concerns/dossier_clone_concern.rb
+++ b/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
-      end
+    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
diff --git a/spec/models/concerns/dossier_clone_concern_spec.rb b/spec/models/concerns/dossier_clone_concern_spec.rb
index f5339d842..853454cdb 100644
--- a/spec/models/concerns/dossier_clone_concern_spec.rb
+++ b/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

From 6cbf66dfb8412c168c406f95bdb640805a139207 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Tue, 26 Nov 2024 22:13:03 +0100
Subject: [PATCH 06/10] refactor(dossier): stop destroying champs in rebase

---
 app/models/concerns/dossier_rebase_concern.rb | 62 +++++--------------
 app/models/type_de_champ.rb                   |  1 +
 .../concerns/dossier_rebase_concern_spec.rb   |  8 +--
 3 files changed, 19 insertions(+), 52 deletions(-)

diff --git a/app/models/concerns/dossier_rebase_concern.rb b/app/models/concerns/dossier_rebase_concern.rb
index 675bd6117..54d79f10f 100644
--- a/app/models/concerns/dossier_rebase_concern.rb
+++ b/app/models/concerns/dossier_rebase_concern.rb
@@ -49,59 +49,27 @@ module DossierRebaseConcern
     # revision we are rebasing to
     target_revision = procedure.published_revision
 
-    # index published types de champ coordinates by stable_id
-    target_coordinates_by_stable_id = target_revision
-      .revision_types_de_champ
-      .includes(:parent)
-      .index_by(&:stable_id)
-
-    changes_by_op = pending_changes
+    changed_stable_ids_by_op = pending_changes
       .group_by(&:op)
-      .tap { _1.default = [] }
-
-    champs_by_stable_id = champs
-      .group_by(&:stable_id)
-      .transform_values { Champ.where(id: _1) }
-      .tap { _1.default = Champ.none }
-
-    # remove champ
-    changes_by_op[:remove].each { champs_by_stable_id[_1.stable_id].destroy_all }
-
-    # update champ
-    changes_by_op[:update].each { champs_by_stable_id[_1.stable_id].update_all(rebased_at: Time.zone.now) }
+      .transform_values { _1.map(&:stable_id) }
+    updated_stable_ids = changed_stable_ids_by_op.fetch(:update, [])
+    added_stable_ids = changed_stable_ids_by_op.fetch(:add, [])
 
     # update dossier revision
     update_column(:revision_id, target_revision.id)
 
-    # add champ (after changing dossier revision to avoid errors)
-    changes_by_op[:add]
-      .map { target_coordinates_by_stable_id[_1.stable_id] }
-      .each { add_new_champs_for_revision(_1) }
-  end
+    # mark updated champs as rebased
+    champs.where(stable_id: updated_stable_ids).update_all(rebased_at: Time.zone.now)
 
-  def add_new_champs_for_revision(target_coordinate)
-    if target_coordinate.child?
-      row_ids = repetition_row_ids(target_coordinate.parent.type_de_champ)
-
-      if row_ids.present?
-        row_ids.each do |row_id|
-          create_champ(target_coordinate, row_id:)
-        end
-      elsif target_coordinate.parent.mandatory?
-        create_champ(target_coordinate, row_id: ULID.generate)
+    # add rows for new repetitions
+    repetition_types_de_champ = target_revision
+      .types_de_champ
+      .repetition
+      .where(stable_id: added_stable_ids)
+    repetition_types_de_champ.mandatory
+      .or(repetition_types_de_champ.private_only)
+      .find_each do |type_de_champ|
+        self.champs << type_de_champ.build_champ(row_id: ULID.generate, rebased_at: Time.zone.now)
       end
-    else
-      create_champ(target_coordinate)
-    end
-  end
-
-  def create_champ(target_coordinate, row_id: nil)
-    self.champs << target_coordinate
-      .type_de_champ
-      .build_champ(rebased_at: Time.zone.now, row_id:)
-  end
-
-  def purge_piece_justificative_file(champ)
-    ActiveStorage::Attachment.where(id: champ.piece_justificative_file.ids).delete_all
   end
 end
diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb
index bf4a0046d..6d98fe99f 100644
--- a/app/models/type_de_champ.rb
+++ b/app/models/type_de_champ.rb
@@ -171,6 +171,7 @@ class TypeDeChamp < ApplicationRecord
   scope :not_condition, -> { where(condition: nil) }
   scope :fillable, -> { where.not(type_champ: [type_champs.fetch(:header_section), type_champs.fetch(:explication)]) }
   scope :with_header_section, -> { where.not(type_champ: TypeDeChamp.type_champs[:explication]) }
+  scope :mandatory, -> { where(mandatory: true) }
 
   scope :dubious, -> {
     where("unaccent(types_de_champ.libelle) ~* unaccent(?)", DubiousProcedure.forbidden_regexp)
diff --git a/spec/models/concerns/dossier_rebase_concern_spec.rb b/spec/models/concerns/dossier_rebase_concern_spec.rb
index 6564c8b22..851080b30 100644
--- a/spec/models/concerns/dossier_rebase_concern_spec.rb
+++ b/spec/models/concerns/dossier_rebase_concern_spec.rb
@@ -356,7 +356,7 @@ describe DossierRebaseConcern do
       it "updates the brouillon champs with the latest revision changes" do
         expect(dossier.revision).to eq(procedure.published_revision)
         expect(dossier.project_champs_public.size).to eq(5)
-        expect(dossier.champs.count(&:public?)).to eq(7)
+        expect(dossier.champs.count(&:public?)).to eq(6)
         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)
@@ -369,7 +369,7 @@ describe DossierRebaseConcern do
         expect(procedure.revisions.size).to eq(3)
         expect(dossier.revision).to eq(procedure.published_revision)
         expect(dossier.project_champs_public.size).to eq(7)
-        expect(dossier.champs.count(&:public?)).to eq(13)
+        expect(dossier.champs.count(&:public?)).to eq(7)
         expect(rebased_text_champ.value).to eq(text_champ.value)
         expect(rebased_text_champ.type_de_champ).not_to eq(text_champ.type_de_champ)
         expect(rebased_datetime_champ.type_champ).to eq(TypeDeChamp.type_champs.fetch(:date))
@@ -381,7 +381,6 @@ describe DossierRebaseConcern do
         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)
 
@@ -728,9 +727,8 @@ describe DossierRebaseConcern do
           parent.update(type_champ: :integer_number)
         end
 
-        it { expect { subject }.to change { dossier.champs.filter(&:child?).count }.from(2).to(0) }
-        it { expect { subject }.to change { Champ.count }.from(3).to(1) }
         it { expect { subject }.to change { dossier.project_champs_public.find(&:repetition?)&.libelle }.from('p1').to(nil) }
+        it { expect { subject }.not_to change { Champ.count } }
       end
     end
   end

From 474d206ee119c32b7bc756df835f14525d73cb34 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Tue, 26 Nov 2024 22:14:47 +0100
Subject: [PATCH 07/10] refactor(champs): remove orphaned and invisible champs
 after submit

---
 app/models/concerns/dossier_state_concern.rb  |  71 +++++++--
 app/models/dossier.rb                         |  13 --
 ...ece_justificative_file_not_visible_task.rb |  17 --
 .../concerns/dossier_state_concern_spec.rb    | 149 ++++++++++++++++++
 spec/models/dossier_spec.rb                   |  10 +-
 spec/system/users/brouillon_spec.rb           |  17 +-
 ...ustificative_file_not_visible_task_spec.rb |  30 ----
 7 files changed, 225 insertions(+), 82 deletions(-)
 delete mode 100644 app/tasks/maintenance/remove_piece_justificative_file_not_visible_task.rb
 create mode 100644 spec/models/concerns/dossier_state_concern_spec.rb
 delete mode 100644 spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb

diff --git a/app/models/concerns/dossier_state_concern.rb b/app/models/concerns/dossier_state_concern.rb
index a9ea4c741..8401d0bb9 100644
--- a/app/models/concerns/dossier_state_concern.rb
+++ b/app/models/concerns/dossier_state_concern.rb
@@ -11,9 +11,7 @@ module DossierStateConcern
 
     resolve_pending_correction!
     process_sva_svr!
-    remove_piece_justificative_file_not_visible!
-
-    editing_forks.each(&:destroy_editing_fork!)
+    clean_champs_after_submit!
   end
 
   def after_passer_en_construction
@@ -41,7 +39,8 @@ module DossierStateConcern
     groupe_instructeur.instructeurs.with_instant_email_dossier_notifications.each do |instructeur|
       DossierMailer.notify_new_dossier_depose_to_instructeur(self, instructeur.email).deliver_later
     end
-    remove_piece_justificative_file_not_visible!
+
+    clean_champs_after_submit!
   end
 
   def after_passer_en_instruction(h)
@@ -154,7 +153,7 @@ module DossierStateConcern
     end
 
     send_dossier_decision_to_experts(self)
-    remove_titres_identite!
+    clean_champs_after_instruction!
   end
 
   def after_accepter_automatiquement
@@ -189,7 +188,7 @@ module DossierStateConcern
     NotificationMailer.send_notification_for_tiers(self).deliver_later if self.for_tiers?
 
     send_dossier_decision_to_experts(self)
-    remove_titres_identite!
+    clean_champs_after_instruction!
   end
 
   def after_refuser(h)
@@ -226,7 +225,7 @@ module DossierStateConcern
     end
 
     send_dossier_decision_to_experts(self)
-    remove_titres_identite!
+    clean_champs_after_instruction!
   end
 
   def after_refuser_automatiquement
@@ -254,7 +253,7 @@ module DossierStateConcern
     NotificationMailer.send_notification_for_tiers(self).deliver_later if self.for_tiers?
 
     send_dossier_decision_to_experts(self)
-    remove_titres_identite!
+    clean_champs_after_instruction!
   end
 
   def after_classer_sans_suite(h)
@@ -291,7 +290,7 @@ module DossierStateConcern
     end
 
     send_dossier_decision_to_experts(self)
-    remove_titres_identite!
+    clean_champs_after_instruction!
   end
 
   def after_repasser_en_instruction(h)
@@ -329,4 +328,58 @@ module DossierStateConcern
 
     rebase_later
   end
+
+  def clean_champs_after_submit!
+    remove_discarded_rows!
+    remove_not_visible_rows!
+    remove_not_visible_or_empty_champs!
+    editing_forks.each(&:destroy_editing_fork!)
+  end
+
+  def clean_champs_after_instruction!
+    remove_discarded_rows!
+    remove_titres_identite!
+  end
+
+  private
+
+  def remove_discarded_rows!
+    row_to_remove_ids = champs.filter { _1.row? && _1.discarded? }.map(&:row_id)
+
+    return if row_to_remove_ids.empty?
+    champs.where(row_id: row_to_remove_ids).destroy_all
+  end
+
+  def remove_not_visible_or_empty_champs!
+    repetition_to_keep_stable_ids, champ_to_keep_public_ids = project_champs_public_all
+      .reject { _1.blank? || !_1.visible? }
+      .partition(&:repetition?)
+      .then { |(repetitions, champs)| [repetitions.to_set(&:stable_id), champs.to_set(&:public_id)] }
+
+    rows_public, champs_public = champs
+      .filter(&:public?)
+      .partition(&:row?)
+
+    champs_to_remove = champs_public.reject { champ_to_keep_public_ids.member?(_1.public_id) }
+    champs_to_remove += rows_public.reject { repetition_to_keep_stable_ids.member?(_1.stable_id) }
+
+    return if champs_to_remove.empty?
+    champs.where(id: champs_to_remove).destroy_all
+  end
+
+  def remove_not_visible_rows!
+    row_to_remove_ids = project_champs_public
+      .filter { _1.repetition? && !_1.visible? }
+      .flat_map(&:row_ids)
+
+    return if row_to_remove_ids.empty?
+    champs.where(row_id: row_to_remove_ids).destroy_all
+  end
+
+  def remove_titres_identite!
+    champ_to_remove_ids = filled_champs.filter(&:titre_identite?).map(&:id)
+
+    return if champ_to_remove_ids.empty?
+    champs.where(id: champ_to_remove_ids).destroy_all
+  end
 end
diff --git a/app/models/dossier.rb b/app/models/dossier.rb
index 44f3b973d..197702963 100644
--- a/app/models/dossier.rb
+++ b/app/models/dossier.rb
@@ -905,19 +905,6 @@ class Dossier < ApplicationRecord
     traitements.any?(&:termine?)
   end
 
-  def remove_titres_identite!
-    champs.filter(&:titre_identite?).map(&:piece_justificative_file).each(&:purge_later)
-  end
-
-  def remove_piece_justificative_file_not_visible!
-    champs.each do |champ|
-      next unless champ.piece_justificative_file.attached?
-      next if champ.visible?
-
-      champ.piece_justificative_file.purge_later
-    end
-  end
-
   def check_mandatory_and_visible_champs
     project_champs_public.filter(&:visible?).each do |champ|
       if champ.mandatory_blank?
diff --git a/app/tasks/maintenance/remove_piece_justificative_file_not_visible_task.rb b/app/tasks/maintenance/remove_piece_justificative_file_not_visible_task.rb
deleted file mode 100644
index 473b5698c..000000000
--- a/app/tasks/maintenance/remove_piece_justificative_file_not_visible_task.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-# frozen_string_literal: true
-
-module Maintenance
-  class RemovePieceJustificativeFileNotVisibleTask < MaintenanceTasks::Task
-    attribute :procedure_id, :string
-    validates :procedure_id, presence: true
-
-    def collection
-      procedure = Procedure.with_discarded.find(procedure_id.strip.to_i)
-      procedure.dossiers.state_not_brouillon
-    end
-
-    def process(dossier)
-      dossier.remove_piece_justificative_file_not_visible!
-    end
-  end
-end
diff --git a/spec/models/concerns/dossier_state_concern_spec.rb b/spec/models/concerns/dossier_state_concern_spec.rb
new file mode 100644
index 000000000..88a39157c
--- /dev/null
+++ b/spec/models/concerns/dossier_state_concern_spec.rb
@@ -0,0 +1,149 @@
+# frozen_string_literal: true
+
+RSpec.describe DossierStateConcern do
+  include Logic
+
+  let(:procedure) { create(:procedure, :published, :for_individual, types_de_champ_public:, declarative_with_state:) }
+  let(:types_de_champ_public) do
+    [
+      { type: :text, stable_id: 90 },
+      { type: :text, stable_id: 91 },
+      { type: :piece_justificative, stable_id: 92, condition: ds_eq(constant(true), constant(false)) },
+      { type: :titre_identite, stable_id: 93, condition: ds_eq(constant(true), constant(false)) },
+      { type: :repetition, stable_id: 94, children: [{ type: :text, stable_id: 941 }, { type: :text, stable_id: 942 }] },
+      { type: :repetition, stable_id: 95, children: [{ type: :text, stable_id: 951 }] },
+      { type: :repetition, stable_id: 96, children: [{ type: :text, stable_id: 961 }], condition: ds_eq(constant(true), constant(false)) },
+      { type: :text, stable_id: 97, condition: ds_eq(constant(true), constant(false)) },
+      { type: :titre_identite, stable_id: 98 }
+    ]
+  end
+  let(:declarative_with_state) { nil }
+  let(:dossier_state) { :brouillon }
+  let(:dossier) do
+    create(:dossier, dossier_state, :with_individual, :with_populated_champs, procedure:).tap do |dossier|
+      procedure.draft_revision.remove_type_de_champ(91)
+      procedure.draft_revision.remove_type_de_champ(95)
+      procedure.draft_revision.remove_type_de_champ(942)
+      procedure.publish_revision!
+      perform_enqueued_jobs
+      dossier.reload
+      champ_repetition = dossier.project_champs_public.find { _1.stable_id == 94 }
+      row_id = champ_repetition.row_ids.first
+      champ_repetition.remove_row(row_id, updated_by: 'test')
+    end
+  end
+
+  describe 'submit brouillon' do
+    it do
+      expect(dossier.champs.size).to eq(20)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2)
+      expect(dossier.champs.filter { _1.row? && _1.discarded? }.size).to eq(1)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id.in?([95, 96]) }.size).to eq(4)
+      expect(dossier.champs.filter { _1.stable_id.in?([90, 92, 93, 97, 961, 951]) }.size).to eq(8)
+
+      champ_text = dossier.project_champs_public.find { _1.stable_id == 90 }
+      champ_text.update(value: '')
+
+      dossier.passer_en_construction!
+      dossier.reload
+
+      expect(dossier.champs.size).to eq(3)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1)
+      expect(dossier.champs.filter { _1.row? && _1.discarded? }.size).to eq(0)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id.in?([95, 96]) }.size).to eq(0)
+      expect(dossier.champs.filter { _1.stable_id.in?([90, 92, 93, 97, 961, 951]) }.size).to eq(0)
+    end
+  end
+
+  describe 'submit en construction' do
+    let(:dossier_state) { :en_construction }
+
+    it do
+      expect(dossier.champs.size).to eq(20)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2)
+      expect(dossier.champs.filter { _1.row? && _1.discarded? }.size).to eq(1)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id.in?([95, 96]) }.size).to eq(4)
+      expect(dossier.champs.filter { _1.stable_id.in?([92, 93, 97, 961, 951]) }.size).to eq(7)
+
+      dossier.submit_en_construction!
+      dossier.reload
+
+      expect(dossier.champs.size).to eq(4)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1)
+      expect(dossier.champs.filter { _1.row? && _1.discarded? }.size).to eq(0)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id.in?([95, 96]) }.size).to eq(0)
+      expect(dossier.champs.filter { _1.stable_id.in?([92, 93, 97, 961, 951]) }.size).to eq(0)
+    end
+  end
+
+  describe 'accepter' do
+    let(:dossier_state) { :en_instruction }
+
+    it do
+      expect(dossier.champs.size).to eq(20)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2)
+      expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(2)
+
+      dossier.accepter!(motivation: 'test')
+      dossier.reload
+
+      expect(dossier.champs.size).to eq(15)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1)
+      expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(0)
+    end
+  end
+
+  describe 'refuser' do
+    let(:dossier_state) { :en_instruction }
+
+    it do
+      expect(dossier.champs.size).to eq(20)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2)
+      expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(2)
+
+      dossier.refuser!(motivation: 'test')
+      dossier.reload
+
+      expect(dossier.champs.size).to eq(15)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1)
+      expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(0)
+    end
+  end
+
+  describe 'classer_sans_suite' do
+    let(:dossier_state) { :en_instruction }
+
+    it do
+      expect(dossier.champs.size).to eq(20)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2)
+      expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(2)
+
+      dossier.classer_sans_suite!(motivation: 'test')
+      dossier.reload
+
+      expect(dossier.champs.size).to eq(15)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1)
+      expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(0)
+    end
+  end
+
+  describe 'automatiquement' do
+    let(:dossier_state) { :en_construction }
+    let(:declarative_with_state) { Dossier.states.fetch(:accepte) }
+
+    describe 'accepter' do
+      it do
+        expect(dossier.champs.size).to eq(20)
+        expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2)
+        expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(2)
+
+        dossier.accepter_automatiquement!
+        dossier.reload
+
+        expect(dossier.champs.size).to eq(15)
+        expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1)
+        expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(0)
+      end
+    end
+  end
+end
diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb
index cba6e5188..dcc9cd685 100644
--- a/spec/models/dossier_spec.rb
+++ b/spec/models/dossier_spec.rb
@@ -501,15 +501,13 @@ describe Dossier, type: :model do
           let(:procedure) { create(:procedure, types_de_champ_public:) }
           let(:dossier) { create(:dossier, :brouillon, :with_populated_champs, procedure:) }
 
-          before { expect(champ).to receive(:visible?).and_return(visible) }
-
           context 'when piece_justificative' do
-            let(:types_de_champ_public) { [{ type: :piece_justificative }] }
+            let(:types_de_champ_public) { [{ type: :piece_justificative, condition: ds_eq(constant(true), constant(visible)) }] }
             let(:champ) { dossier.project_champs_public.find(&:piece_justificative?) }
 
             context 'when not visible' do
               let(:visible) { false }
-              it { expect { subject }.to change { champ.reload.piece_justificative_file.attached? } }
+              it { expect { subject }.to change { Champ.exists?(champ.id) } }
             end
 
             context 'when visible' do
@@ -519,12 +517,12 @@ describe Dossier, type: :model do
           end
 
           context 'when titre identite' do
-            let(:types_de_champ_public) { [{ type: :titre_identite }] }
+            let(:types_de_champ_public) { [{ type: :titre_identite, condition: ds_eq(constant(true), constant(visible)) }] }
             let(:champ) { dossier.project_champs_public.find(&:titre_identite?) }
 
             context 'when not visible' do
               let(:visible) { false }
-              it { expect { subject }.to change { champ.reload.piece_justificative_file.attached? } }
+              it { expect { subject }.to change { Champ.exists?(champ.id) } }
             end
 
             context 'when visible' do
diff --git a/spec/system/users/brouillon_spec.rb b/spec/system/users/brouillon_spec.rb
index 98c414e4b..a53bc09a3 100644
--- a/spec/system/users/brouillon_spec.rb
+++ b/spec/system/users/brouillon_spec.rb
@@ -569,6 +569,16 @@ describe 'The user', js: true do
         expect(page).to have_no_css('legend', text: 'permis de conduire', visible: true)
         expect(page).to have_no_css('label', text: 'tonnage', visible: true)
 
+        fill_in('age du candidat', with: '18')
+        wait_for_autosave
+
+        # the champ keeps their previous value so they are all displayed
+        expect(page).to have_css('legend', text: 'permis de conduire', visible: true)
+        expect(page).to have_css('label', text: 'tonnage', visible: true)
+
+        fill_in('age du candidat', with: '2')
+        wait_for_autosave
+
         click_on 'Déposer le dossier'
         click_on 'Accéder à votre dossier'
         click_on 'Modifier mon dossier'
@@ -576,13 +586,6 @@ describe 'The user', js: true do
         expect(page).to have_css('label', text: 'age du candidat', visible: true)
         expect(page).to have_no_css('legend', text: 'permis de conduire', visible: true)
         expect(page).to have_no_css('label', text: 'tonnage', visible: true)
-
-        fill_in('age du candidat', with: '18')
-        wait_for_autosave
-
-        # the champ keeps their previous value so they are all displayed
-        expect(page).to have_css('legend', text: 'permis de conduire', visible: true)
-        expect(page).to have_css('label', text: 'tonnage', visible: true)
       end
     end
   end
diff --git a/spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb b/spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb
deleted file mode 100644
index e7e68ff2b..000000000
--- a/spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb
+++ /dev/null
@@ -1,30 +0,0 @@
-# frozen_string_literal: true
-
-require "rails_helper"
-
-module Maintenance
-  RSpec.describe RemovePieceJustificativeFileNotVisibleTask do
-    describe "#process" do
-      subject(:process) { described_class.process(dossier) }
-
-      let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :piece_justificative }]) }
-      let(:dossier) { create(:dossier, :en_construction, :with_populated_champs, procedure:) }
-
-      before { expect(champ).to receive(:visible?).and_return(visible) }
-
-      context 'when piece_justificative' do
-        let(:champ) { dossier.filled_champs_public.find(&:piece_justificative?) }
-
-        context 'when not visible' do
-          let(:visible) { false }
-          it { expect { subject }.to change { champ.reload.piece_justificative_file.attached? } }
-        end
-
-        context 'when visible' do
-          let(:visible) { true }
-          it { expect { subject }.not_to change { champ.reload.piece_justificative_file.attached? } }
-        end
-      end
-    end
-  end
-end

From 56b0b156bf4a9a6c39dd319e718911b3c080cbc5 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Tue, 26 Nov 2024 22:15:26 +0100
Subject: [PATCH 08/10] refactor(champs): update specs

---
 .../explication_component_spec.rb             |  4 +--
 .../piece_justificative_controller_spec.rb    | 17 ++--------
 .../champs/repetition_controller_spec.rb      | 16 ++++++----
 .../instructeurs/dossiers_controller_spec.rb  | 11 ++++---
 .../users/commencer_controller_spec.rb        |  6 ++--
 .../users/dossiers_controller_spec.rb         |  6 ++--
 spec/graphql/annotation_spec.rb               |  3 +-
 spec/lib/recovery/dossier_life_cycle_spec.rb  |  6 ++--
 .../repetition_presentation_spec.rb           | 12 +++----
 spec/models/champ_spec.rb                     |  5 ++-
 spec/models/champs/repetition_champ_spec.rb   |  4 +--
 .../tags_substitution_concern_spec.rb         | 14 ++++----
 spec/models/dossier_preloader_spec.rb         |  4 +--
 spec/models/dossier_spec.rb                   |  5 ++-
 .../prefill_repetition_type_de_champ_spec.rb  |  6 ++--
 spec/rails_helper.rb                          |  7 ++++
 .../pieces_justificatives_service_spec.rb     | 32 +++++++++----------
 .../procedure_export_service_zip_spec.rb      |  6 ++--
 spec/system/users/brouillon_spec.rb           |  2 +-
 19 files changed, 83 insertions(+), 83 deletions(-)

diff --git a/spec/components/editable_champ/explication_component/explication_component_spec.rb b/spec/components/editable_champ/explication_component/explication_component_spec.rb
index ae341b64a..d87392e66 100644
--- a/spec/components/editable_champ/explication_component/explication_component_spec.rb
+++ b/spec/components/editable_champ/explication_component/explication_component_spec.rb
@@ -2,8 +2,8 @@
 
 describe EditableChamp::ExplicationComponent, type: :component do
   let(:procedure) { create(:procedure, :published, types_de_champ_public:) }
-  let(:dossier) { create(:dossier, :with_populated_champs, procedure:) }
-  let(:champ) { dossier.champs.first }
+  let(:dossier) { create(:dossier, procedure:) }
+  let(:champ) { dossier.project_champs_public.first }
 
   let(:component) {
     described_class.new(form: instance_double(ActionView::Helpers::FormBuilder, object_name: "dossier[champs_public_attributes]"), champ:)
diff --git a/spec/controllers/champs/piece_justificative_controller_spec.rb b/spec/controllers/champs/piece_justificative_controller_spec.rb
index 90a43aba6..25a908d57 100644
--- a/spec/controllers/champs/piece_justificative_controller_spec.rb
+++ b/spec/controllers/champs/piece_justificative_controller_spec.rb
@@ -40,20 +40,7 @@ describe Champs::PieceJustificativeController, type: :controller do
     end
 
     context 'when the file is invalid' do
-      let(:file) { fixture_file_upload('spec/fixtures/files/invalid_file_format.json', 'application/json') }
-
-      # TODO: for now there are no validators on the champ piece_justificative_file,
-      # so we have to mock a failing validation.
-      # Once the validators will be enabled, remove those mocks, and let the usual
-      # validation fail naturally.
-      #
-      # See https://github.com/betagouv/demarches-simplifiees.fr/issues/4926
-      before do
-        champ
-        expect_any_instance_of(Champs::PieceJustificativeChamp).to receive(:save).and_return(false)
-        expect_any_instance_of(Champs::PieceJustificativeChamp).to receive(:errors)
-          .and_return(double(full_messages: ['La pièce justificative n’est pas d’un type accepté']))
-      end
+      let(:file) { fixture_file_upload('spec/fixtures/files/invalid_file_format.json', 'bad/bad') }
 
       it 'doesn’t attach the file' do
         subject
@@ -64,7 +51,7 @@ describe Champs::PieceJustificativeController, type: :controller do
         subject
         expect(response.status).to eq(422)
         expect(response.header['Content-Type']).to include('application/json')
-        expect(response.parsed_body).to eq({ 'errors' => ['La pièce justificative n’est pas d’un type accepté'] })
+        expect(response.parsed_body).to eq({ 'errors' => ['Le champ « Piece justificative file » n’est pas d’un type accepté'] })
       end
     end
   end
diff --git a/spec/controllers/champs/repetition_controller_spec.rb b/spec/controllers/champs/repetition_controller_spec.rb
index 583ec2c88..7a8c41bf5 100644
--- a/spec/controllers/champs/repetition_controller_spec.rb
+++ b/spec/controllers/champs/repetition_controller_spec.rb
@@ -3,14 +3,18 @@
 describe Champs::RepetitionController, type: :controller do
   describe '#remove' do
     let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :repetition, mandatory: true, children: [{ libelle: 'Nom' }, { type: :integer_number, libelle: 'Age' }] }]) }
-    let(:dossier) { create(:dossier, procedure: procedure) }
+    let(:dossier) { create(:dossier, procedure:) }
+    let(:repetition) { dossier.project_champs_public.find(&:repetition?) }
+    let(:row) { dossier.champs.find(&:row?) }
 
     before { sign_in dossier.user }
-    it 'removes repetition' do
-      rows, repetitions = dossier.champs.partition(&:child?)
-      repetition = repetitions.first
-      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)
+
+    subject { delete :remove, params: { dossier_id: dossier, stable_id: repetition.stable_id, row_id: row.row_id }, format: :turbo_stream }
+
+    context 'removes repetition' do
+      it { expect { subject }.not_to change { dossier.reload.champs.size } }
+      it { expect { subject }.to change { dossier.reload; dossier.project_champs_public.find(&:repetition?).row_ids.size }.from(1).to(0) }
+      it { expect { subject }.to change { row.reload.discarded_at }.from(nil).to(Time) }
     end
   end
 end
diff --git a/spec/controllers/instructeurs/dossiers_controller_spec.rb b/spec/controllers/instructeurs/dossiers_controller_spec.rb
index a87e98c48..aa0f53c4a 100644
--- a/spec/controllers/instructeurs/dossiers_controller_spec.rb
+++ b/spec/controllers/instructeurs/dossiers_controller_spec.rb
@@ -1002,10 +1002,11 @@ describe Instructeurs::DossiersController, type: :controller do
     let(:another_instructeur) { create(:instructeur) }
     let(:now) { Time.zone.parse('01/01/2100') }
 
+    let(:champ_repetition) { dossier.project_champs_private.fourth }
+    let(:champ_text) { champ_repetition.rows.first.first }
     let(:champ_multiple_drop_down_list) { dossier.project_champs_private.first }
     let(:champ_linked_drop_down_list) { dossier.project_champs_private.second }
     let(:champ_datetime) { dossier.project_champs_private.third }
-    let(:champ_repetition) { dossier.project_champs_private.fourth }
     let(:champ_drop_down_list) { dossier.project_champs_private.fifth }
 
     context 'when no invalid champs_public' do
@@ -1015,12 +1016,12 @@ describe Instructeurs::DossiersController, type: :controller do
           another_instructeur.follow(dossier)
           Timecop.freeze(now)
           patch :update_annotations, params: params, format: :turbo_stream
-
+          dossier.reload
           champ_multiple_drop_down_list.reload
           champ_linked_drop_down_list.reload
           champ_datetime.reload
-          champ_repetition.reload
           champ_drop_down_list.reload
+          champ_text.reload
         end
 
         after do
@@ -1059,9 +1060,9 @@ describe Instructeurs::DossiersController, type: :controller do
           expect(champ_linked_drop_down_list.primary_value).to eq('primary')
           expect(champ_linked_drop_down_list.secondary_value).to eq('secondary')
           expect(champ_datetime.value).to eq(Time.zone.parse('2019-12-21T13:17:00').iso8601)
-          expect(champ_repetition.rows.first.first.value).to eq('text')
+          expect(champ_text.value).to eq('text')
           expect(champ_drop_down_list.value).to eq('other value')
-          expect(dossier.reload.last_champ_private_updated_at).to eq(now)
+          expect(dossier.last_champ_private_updated_at).to eq(now)
           expect(response).to have_http_status(200)
           assert_enqueued_jobs(1, only: DossierIndexSearchTermsJob)
         }
diff --git a/spec/controllers/users/commencer_controller_spec.rb b/spec/controllers/users/commencer_controller_spec.rb
index 50b86994a..e984d7c50 100644
--- a/spec/controllers/users/commencer_controller_spec.rb
+++ b/spec/controllers/users/commencer_controller_spec.rb
@@ -2,7 +2,8 @@
 
 describe Users::CommencerController, type: :controller do
   let(:user) { create(:user) }
-  let(:published_procedure) { create(:procedure, :for_individual, :published) }
+  let(:published_procedure) { create(:procedure, :for_individual, :published, types_de_champ_public:) }
+  let(:types_de_champ_public) { [] }
   let(:draft_procedure) { create(:procedure, :with_path) }
 
   describe '#commencer' do
@@ -160,7 +161,8 @@ describe Users::CommencerController, type: :controller do
     end
 
     context 'when a dossier is being prefilled by GET' do
-      let(:type_de_champ_text) { create(:type_de_champ_text, procedure: published_procedure) }
+      let(:types_de_champ_public) { [{}] }
+      let(:type_de_champ_text) { published_procedure.published_revision.types_de_champ.first }
       let(:path) { published_procedure.path }
       let(:user) { create(:user) }
 
diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb
index 317c11365..11ce0e20f 100644
--- a/spec/controllers/users/dossiers_controller_spec.rb
+++ b/spec/controllers/users/dossiers_controller_spec.rb
@@ -594,11 +594,11 @@ describe Users::DossiersController, type: :controller do
       let(:dossier) { create(:dossier, :en_construction, :with_populated_champs, procedure:, user:) }
       let(:types_de_champ_public) { [{ type: :repetition, libelle: 'repetition', children: [{ type: :text, libelle: 'child' }] }] }
       let(:editing_fork) { dossier.owner_editing_fork }
-      let(:champ_repetition) { editing_fork.champs.find(&:repetition?) }
+      let(:champ_repetition) { editing_fork.project_champs_public.find(&:repetition?) }
       before do
         editing_fork
 
-        procedure.draft_revision.remove_type_de_champ(editing_fork.champs.find(&:repetition?).stable_id)
+        procedure.draft_revision.remove_type_de_champ(champ_repetition.stable_id)
         procedure.publish_revision!
 
         editing_fork.reload
@@ -611,7 +611,7 @@ describe Users::DossiersController, type: :controller do
 
     context 'when dossier was already submitted' do
       before do
-        expect_any_instance_of(Dossier).to receive(:remove_piece_justificative_file_not_visible!)
+        expect_any_instance_of(Dossier).to receive(:remove_not_visible_or_empty_champs!)
         post :submit_en_construction, params: payload
       end
 
diff --git a/spec/graphql/annotation_spec.rb b/spec/graphql/annotation_spec.rb
index 480b0a96b..677710db4 100644
--- a/spec/graphql/annotation_spec.rb
+++ b/spec/graphql/annotation_spec.rb
@@ -55,7 +55,8 @@ RSpec.describe Mutations::DossierModifierAnnotation, type: :graphql do
         },
         errors: nil
       })
-      expect(annotation.reload.row_ids.size).to eq(3)
+      dossier.reload
+      expect(annotation.row_ids.size).to eq(3)
     end
   end
 
diff --git a/spec/lib/recovery/dossier_life_cycle_spec.rb b/spec/lib/recovery/dossier_life_cycle_spec.rb
index 3a6a05d4b..13dc53b66 100644
--- a/spec/lib/recovery/dossier_life_cycle_spec.rb
+++ b/spec/lib/recovery/dossier_life_cycle_spec.rb
@@ -15,7 +15,7 @@ describe 'Dossier::Recovery::LifeCycle' do
     let(:geo_area) { build(:geo_area, :selection_utilisateur, :polygon) }
     let(:fp) { Rails.root.join('spec', 'fixtures', 'export.dump') }
     let(:dossier) do
-      d = create(:dossier, procedure:)
+      d = create(:dossier, :with_populated_champs, procedure:)
 
       repetition(d).add_row(updated_by: 'test')
       pj_champ(d).piece_justificative_file.attach(some_file)
@@ -49,7 +49,7 @@ describe 'Dossier::Recovery::LifeCycle' do
       d
     end
 
-    def repetition(d) = d.champs.find_by(type: "Champs::RepetitionChamp")
+    def repetition(d) = d.project_champs_public.find(&:repetition?)
     def pj_champ(d) = d.champs.find_by(type: "Champs::PieceJustificativeChamp")
     def carte(d) = d.champs.find_by(type: "Champs::CarteChamp")
     def siret(d) = d.champs.find_by(type: "Champs::SiretChamp")
@@ -83,7 +83,7 @@ describe 'Dossier::Recovery::LifeCycle' do
 
       expect(reloaded_dossier.champs.count).not_to be(0)
 
-      expect(repetition(reloaded_dossier).rows.flatten.map(&:type)).to match_array(["Champs::PieceJustificativeChamp"])
+      expect(repetition(reloaded_dossier).rows.flatten.map(&:type)).to match_array(["Champs::PieceJustificativeChamp", "Champs::PieceJustificativeChamp", "Champs::PieceJustificativeChamp"])
       expect(pj_champ(reloaded_dossier).piece_justificative_file).to be_attached
       expect(carte(reloaded_dossier).geo_areas).to be_present
 
diff --git a/spec/models/champ_presentations/repetition_presentation_spec.rb b/spec/models/champ_presentations/repetition_presentation_spec.rb
index 66c498dd7..13c8815e6 100644
--- a/spec/models/champ_presentations/repetition_presentation_spec.rb
+++ b/spec/models/champ_presentations/repetition_presentation_spec.rb
@@ -15,7 +15,7 @@ describe ChampPresentations::RepetitionPresentation do
   }
 
   let(:dossier) { create(:dossier, procedure:) }
-  let(:champ_repetition) { dossier.champs.find(&:repetition?) }
+  let(:champ_repetition) { dossier.project_champs_public.first }
 
   before do
     champ_repetition.add_row(updated_by: 'test')
@@ -23,15 +23,15 @@ describe ChampPresentations::RepetitionPresentation do
     row1, row2, row3 = champ_repetition.rows
 
     nom, stars = row1
-    nom.update(value: "ruby")
-    stars.update(value: 5)
+    champ_for_update(nom).update(value: "ruby")
+    champ_for_update(stars).update(value: 5)
 
     nom = row2.first
-    nom.update(value: "js")
+    champ_for_update(nom).update(value: "js")
 
     nom, stars = row3
-    nom.update(value: "rust")
-    stars.update(value: 4)
+    champ_for_update(nom).update(value: "rust")
+    champ_for_update(stars).update(value: 4)
   end
 
   let(:representation) { described_class.new(libelle, champ_repetition.rows) }
diff --git a/spec/models/champ_spec.rb b/spec/models/champ_spec.rb
index 61b554211..90fbf30e4 100644
--- a/spec/models/champ_spec.rb
+++ b/spec/models/champ_spec.rb
@@ -58,7 +58,7 @@ describe Champ do
     context 'when repetition not blank' do
       let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :repetition, children: [{ type: :text }] }]) }
       let(:dossier) { create(:dossier, :with_populated_champs, procedure:) }
-      let(:champ) { dossier.champs.find(&:repetition?) }
+      let(:champ) { dossier.project_champs_public.find(&:repetition?) }
 
       it { expect(champ.blank?).to be(false) }
     end
@@ -108,11 +108,10 @@ describe Champ do
     let(:dossier) { create(:dossier, procedure: procedure) }
     let(:public_champ) { dossier.project_champs_public.first }
     let(:private_champ) { dossier.project_champs_private.first }
-    let(:champ_in_repetition) { dossier.project_champs_public.find(&:repetition?).champs.first }
     let(:standalone_champ) { build(:champ, type_de_champ: build(:type_de_champ), dossier: build(:dossier)) }
     let(:public_sections) { dossier.project_champs_public.filter(&:header_section?) }
     let(:private_sections) { dossier.project_champs_private.filter(&:header_section?) }
-    let(:sections_in_repetition) { dossier.champs.filter(&:child?).filter(&:header_section?) }
+    let(:sections_in_repetition) { dossier.project_champs_public.find(&:repetition?).rows.flatten.filter(&:header_section?) }
 
     it 'returns the sibling sections of a champ' do
       expect(public_sections).not_to be_empty
diff --git a/spec/models/champs/repetition_champ_spec.rb b/spec/models/champs/repetition_champ_spec.rb
index 38bb197f7..000c5232f 100644
--- a/spec/models/champs/repetition_champ_spec.rb
+++ b/spec/models/champs/repetition_champ_spec.rb
@@ -11,11 +11,11 @@ describe Champs::RepetitionChamp do
       ])
   }
   let(:dossier) { create(:dossier, procedure:) }
-  let(:champ) { dossier.champs.find(&:repetition?) }
+  let(:champ) { dossier.project_champs_public.find(&:repetition?) }
 
   describe "#for_tag" do
     before do
-      champ.rows[0][0].update(value: "rb")
+      champ_for_update(champ.rows.first.first).update(value: "rb")
     end
 
     it "can render as string" do
diff --git a/spec/models/concerns/tags_substitution_concern_spec.rb b/spec/models/concerns/tags_substitution_concern_spec.rb
index 89f5800fd..d70dce98c 100644
--- a/spec/models/concerns/tags_substitution_concern_spec.rb
+++ b/spec/models/concerns/tags_substitution_concern_spec.rb
@@ -237,11 +237,11 @@ describe TagsSubstitutionConcern, type: :model do
         repetition.add_row(updated_by: 'test')
         paul_champs, pierre_champs = repetition.rows
 
-        paul_champs.first.update(value: 'Paul')
-        paul_champs.last.update(value: 'Chavard')
+        champ_for_update(paul_champs.first).update(value: 'Paul')
+        champ_for_update(paul_champs.last).update(value: 'Chavard')
 
-        pierre_champs.first.update(value: 'Pierre')
-        pierre_champs.last.update(value: 'de La Morinerie')
+        champ_for_update(pierre_champs.first).update(value: 'Pierre')
+        champ_for_update(pierre_champs.last).update(value: 'de La Morinerie')
       end
 
       it { is_expected.to eq("Répétition\n\nNom : Paul\nPrénom : Chavard\n\nNom : Pierre\nPrénom : de La Morinerie") }
@@ -258,7 +258,7 @@ describe TagsSubstitutionConcern, type: :model do
 
       context 'and the champ has a primary value' do
         before do
-          dossier.champs.find_by(stable_id: type_de_champ.stable_id).update(primary_value: 'primo')
+          dossier.champ_for_update(type_de_champ, nil, updated_by: 'test').update(primary_value: 'primo')
           dossier.reload
         end
 
@@ -266,7 +266,7 @@ describe TagsSubstitutionConcern, type: :model do
 
         context 'and the champ has a secondary value' do
           before do
-            dossier.champs.find_by(stable_id: type_de_champ.stable_id).update(secondary_value: 'secundo')
+            dossier.champ_for_update(type_de_champ, nil, updated_by: 'test').update(secondary_value: 'secundo')
             dossier.reload
           end
 
@@ -481,7 +481,7 @@ describe TagsSubstitutionConcern, type: :model do
       before do
         draft_type_de_champ.update(libelle: 'mon nouveau libellé')
         dossier.project_champs_public.first.update(value: 'valeur')
-        procedure.update!(draft_revision: procedure.create_new_revision, published_revision: procedure.draft_revision)
+        procedure.publish_revision!
       end
 
       context "when using the champ's original label" do
diff --git a/spec/models/dossier_preloader_spec.rb b/spec/models/dossier_preloader_spec.rb
index 692aa7881..17bf2f093 100644
--- a/spec/models/dossier_preloader_spec.rb
+++ b/spec/models/dossier_preloader_spec.rb
@@ -29,7 +29,7 @@ describe DossierPreloader do
         expect(subject.changed?).to be false
 
         expect(first_child.type).to eq('Champs::TextChamp')
-        expect(repetition.id).not_to eq(first_child.id)
+        expect(repetition).not_to eq(first_child)
         expect(subject.champs.first.dossier).to eq(subject)
         expect(subject.champs.find(&:public?).dossier).to eq(subject)
         expect(subject.project_champs_public.first.dossier).to eq(subject)
@@ -40,7 +40,7 @@ describe DossierPreloader do
         expect(subject.champs.find(&:public?).conditional?).to eq(false)
         expect(subject.project_champs_public.first.conditional?).to eq(false)
 
-        expect(repetition.rows.first.first).to eq(first_child)
+        expect(repetition.rows.first.first.public_id).to eq(first_child.public_id)
         expect(repetition_optional.row_ids).to be_empty
       end
 
diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb
index dcc9cd685..3c919cfd6 100644
--- a/spec/models/dossier_spec.rb
+++ b/spec/models/dossier_spec.rb
@@ -1984,13 +1984,12 @@ describe Dossier, type: :model do
     context 'with integer_number' do
       let(:procedure) { create(:procedure, :published, types_de_champ_public: [{ type: :integer_number, libelle: 'c1' }]) }
       let(:dossier) { create(:dossier, :with_populated_champs, procedure:) }
-      let(:integer_number_type_de_champ) { procedure.active_revision.types_de_champ_public.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:integer_number) } }
+      let(:integer_number_type_de_champ) { procedure.active_revision.types_de_champ_public.find(&:integer_number?) }
 
       it 'give me back my decimal number' do
         dossier
         expect {
           integer_number_type_de_champ.update(type_champ: :decimal_number)
-          procedure.update(published_revision: procedure.draft_revision, draft_revision: procedure.create_new_revision)
         }.to change { dossier.reload.champ_values_for_export(procedure.all_revisions_types_de_champ.not_repetition.to_a, format: :xlsx) }
           .from([["c1", 42]]).to([["c1", 42.0]])
       end
@@ -2030,7 +2029,7 @@ describe Dossier, type: :model do
           procedure.draft_revision.find_and_ensure_exclusive_use(yes_no_type_de_champ.stable_id).update(libelle: 'Updated yes/no')
           procedure.draft_revision.find_and_ensure_exclusive_use(commune_type_de_champ.stable_id).update(libelle: 'Commune de naissance')
           procedure.draft_revision.find_and_ensure_exclusive_use(repetition_type_de_champ.stable_id).update(libelle: 'Repetition')
-          procedure.update(published_revision: procedure.draft_revision, draft_revision: procedure.create_new_revision)
+          procedure.publish_revision!
           dossier.reload
           procedure.reload
         end
diff --git a/spec/models/types_de_champ/prefill_repetition_type_de_champ_spec.rb b/spec/models/types_de_champ/prefill_repetition_type_de_champ_spec.rb
index a5f121fe5..7dbb9c7b0 100644
--- a/spec/models/types_de_champ/prefill_repetition_type_de_champ_spec.rb
+++ b/spec/models/types_de_champ/prefill_repetition_type_de_champ_spec.rb
@@ -3,14 +3,14 @@
 RSpec.describe TypesDeChamp::PrefillRepetitionTypeDeChamp, type: :model do
   let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :repetition, children: [{}, { type: :integer_number }, { type: :regions }] }]) }
   let(:dossier) { create(:dossier, procedure: procedure) }
+  let(:champ) { dossier.project_champs_public.first }
   let(:type_de_champ) { champ.type_de_champ }
-  let(:champ) { dossier.champs.first }
   let(:prefillable_subchamps) { TypesDeChamp::PrefillRepetitionTypeDeChamp.new(type_de_champ, procedure.active_revision).send(:prefillable_subchamps) }
   let(:text_repetition) { prefillable_subchamps.first }
   let(:integer_repetition) { prefillable_subchamps.second }
   let(:region_repetition) { prefillable_subchamps.third }
-  let(:text_repetition_champs) { dossier.champs.where(stable_id: text_repetition.stable_id) }
-  let(:integer_repetition_champs) { dossier.champs.where(stable_id: integer_repetition.stable_id) }
+  let(:text_repetition_champs) { champ.rows.flat_map(&:first) }
+  let(:integer_repetition_champs) { champ.rows.flat_map(&:second) }
 
   describe 'ancestors' do
     subject { described_class.build(type_de_champ, procedure.active_revision) }
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 9235ecdc2..170837f9c 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -125,6 +125,13 @@ RSpec.configure do |config|
     end
   end
 
+  module SpecHelpers
+    def champ_for_update(champ)
+      champ.dossier.champ_for_update(champ.type_de_champ, champ.row_id, updated_by: 'test')
+    end
+  end
+
+  config.include SpecHelpers
   config.include ActiveSupport::Testing::TimeHelpers
   config.include Shoulda::Matchers::ActiveRecord, type: :model
   config.include Shoulda::Matchers::ActiveModel, type: :model
diff --git a/spec/services/pieces_justificatives_service_spec.rb b/spec/services/pieces_justificatives_service_spec.rb
index a9714f20f..d7c940dcc 100644
--- a/spec/services/pieces_justificatives_service_spec.rb
+++ b/spec/services/pieces_justificatives_service_spec.rb
@@ -10,8 +10,8 @@ describe PiecesJustificativesService do
     let(:pj_service) { PiecesJustificativesService.new(user_profile:, export_template:) }
     let(:user_profile) { build(:administrateur) }
 
-    def pj_champ(d) = d.project_champs_public.find { _1.type == 'Champs::PieceJustificativeChamp' }
-    def repetition(d) = d.champs.find_by(type: "Champs::RepetitionChamp")
+    def pj_champ(d) = d.project_champs_public.find(&:piece_justificative?)
+    def repetition(d) = d.project_champs_public.find(&:repetition?)
     def attachments(champ) = champ.piece_justificative_file.attachments
 
     before { attach_file_to_champ(pj_champ(witness)) }
@@ -52,21 +52,20 @@ describe PiecesJustificativesService do
     end
 
     context 'with a repetition' do
-      let(:first_champ) { repetition(dossier).rows.first.first }
-      let(:second_champ) { repetition(dossier).rows.second.first }
+      let(:first_champ) { champ_for_update(repetition(dossier).rows.first.first) }
+      let(:second_champ) { champ_for_update(repetition(dossier).rows.second.first) }
 
       before do
         repetition(dossier).add_row(updated_by: 'test')
-        attach_file_to_champ(first_champ)
-        attach_file_to_champ(first_champ)
-
         repetition(dossier).add_row(updated_by: 'test')
+        attach_file_to_champ(first_champ)
+        attach_file_to_champ(first_champ)
         attach_file_to_champ(second_champ)
       end
 
       it do
-        first_child_attachments = attachments(repetition(dossier).rows.first.first)
-        second_child_attachments = attachments(repetition(dossier).rows.second.first)
+        first_child_attachments = attachments(first_champ)
+        second_child_attachments = attachments(second_champ)
 
         expect(export_template).to receive(:attachment_path)
           .with(dossier, first_child_attachments.first, index: 0, row_index: 0, champ: first_champ)
@@ -103,7 +102,7 @@ describe PiecesJustificativesService do
       let(:user_profile) { build(:administrateur) }
       let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :piece_justificative }]) }
       let(:witness) { create(:dossier, procedure: procedure) }
-      def pj_champ(d) = d.project_champs_public.find { |c| c.type == 'Champs::PieceJustificativeChamp' }
+      def pj_champ(d) = d.project_champs_public.find(&:piece_justificative?)
 
       context 'with a single attachment' do
         before do
@@ -144,7 +143,7 @@ describe PiecesJustificativesService do
         let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :titre_identite }]) }
         let(:dossier) { create(:dossier, procedure: procedure) }
 
-        let(:champ_identite) { dossier.project_champs_public.find { |c| c.type == 'Champs::TitreIdentiteChamp' } }
+        let(:champ_identite) { dossier.project_champs_public.find(&:titre_identite?) }
 
         before { attach_file_to_champ(champ_identite) }
 
@@ -261,7 +260,7 @@ describe PiecesJustificativesService do
       let(:witness) { create(:dossier, procedure: procedure) }
 
       let!(:private_pj) { create(:type_de_champ_piece_justificative, procedure: procedure, private: true) }
-      def private_pj_champ(d) = d.project_champs_private.find { |c| c.type == 'Champs::PieceJustificativeChamp' }
+      def private_pj_champ(d) = d.project_champs_private.find(&:piece_justificative?)
 
       before do
         attach_file_to_champ(private_pj_champ(dossier))
@@ -501,17 +500,14 @@ describe PiecesJustificativesService do
     end
 
     let(:procedure) { create(:procedure, types_de_champ_public:) }
-    let(:dossier_1) { create(:dossier, procedure:) }
-    let(:champs) { dossier_1.champs }
+    let(:dossier_1) { create(:dossier, :with_populated_champs, procedure:) }
+    let(:champs) { dossier_1.filled_champs }
 
-    def pj_champ(d) = d.project_champs_public.find { _1.type == 'Champs::PieceJustificativeChamp' }
     def repetition(d, index:) = d.project_champs_public.filter(&:repetition?)[index]
 
     subject { PiecesJustificativesService.new(user_profile:, export_template: nil).send(:compute_champ_id_row_index, champs) }
 
     before do
-      pj_champ(dossier_1)
-
       # repet_0 (stable_id: r0)
       # # row_0
       # # # pj_champ_0 (stable_id: 0)
@@ -553,7 +549,9 @@ describe PiecesJustificativesService do
   end
 
   def attach_file_to_champ(champ, safe = true)
+    champ = champ_for_update(champ)
     attach_file(champ.piece_justificative_file, safe)
+    champ.save!
   end
 
   def attach_file(attachable, safe = true)
diff --git a/spec/services/procedure_export_service_zip_spec.rb b/spec/services/procedure_export_service_zip_spec.rb
index 13050d011..c0ff7125d 100644
--- a/spec/services/procedure_export_service_zip_spec.rb
+++ b/spec/services/procedure_export_service_zip_spec.rb
@@ -7,8 +7,8 @@ describe ProcedureExportService do
   let(:export_template) { create(:export_template, :enabled_pjs, groupe_instructeur: procedure.defaut_groupe_instructeur) }
   let(:service) { ProcedureExportService.new(procedure, procedure.dossiers, instructeur, export_template) }
 
-  def pj_champ(d) = d.project_champs_public.find { _1.type == 'Champs::PieceJustificativeChamp' }
-  def repetition(d) = d.champs.find_by(type: "Champs::RepetitionChamp")
+  def pj_champ(d) = d.project_champs_public.find(&:piece_justificative?)
+  def repetition(d) = d.project_champs_public.find(&:repetition?)
   def attachments(champ) = champ.piece_justificative_file.attachments
 
   before do
@@ -69,7 +69,9 @@ describe ProcedureExportService do
   end
 
   def attach_file_to_champ(champ, safe = true)
+    champ = champ_for_update(champ)
     attach_file(champ.piece_justificative_file, safe)
+    champ.save!
   end
 
   def attach_file(attachable, safe = true)
diff --git a/spec/system/users/brouillon_spec.rb b/spec/system/users/brouillon_spec.rb
index a53bc09a3..5199d74be 100644
--- a/spec/system/users/brouillon_spec.rb
+++ b/spec/system/users/brouillon_spec.rb
@@ -171,7 +171,7 @@ describe 'The user', js: true do
       wait_until { page.all(".row").size == 1 }
       # removing a repetition means one child only, thus its button destroy is not visible
       expect(page).to have_selector(".repetition .row:first-child .utils-repetition-required-destroy-button", count: 1, visible: false)
-    end.to change { Champ.count }
+    end.to change { Champ.where.not(discarded_at: nil).count }
   end
 
   let(:simple_procedure) {

From 3556fcc11e4bd63d72eafcfd69f69054faf72cba Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Thu, 5 Dec 2024 15:38:46 +0100
Subject: [PATCH 09/10] refactor(champs): cleanup champ_for_update

---
 app/models/concerns/dossier_champs_concern.rb | 34 +++++++++----------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb
index 75871fd5f..40eea293d 100644
--- a/app/models/concerns/dossier_champs_concern.rb
+++ b/app/models/concerns/dossier_champs_concern.rb
@@ -114,8 +114,8 @@ module DossierChampsConcern
   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)
+    champ = champ_upsert_by!(type_de_champ, row_id)
+    champ.updated_by = updated_by
     champ
   end
 
@@ -203,33 +203,33 @@ module DossierChampsConcern
   def champ_attributes_by_public_id(public_id, attributes, scope, updated_by:)
     stable_id, row_id = public_id.split('-')
     type_de_champ = find_type_de_champ_by_stable_id(stable_id, scope)
-    champ_with_attributes_for_update(type_de_champ, row_id, updated_by:).last.merge(attributes)
+    champ = champ_upsert_by!(type_de_champ, row_id)
+    attributes.merge(id: champ.id, updated_by:)
   end
 
-  def champ_with_attributes_for_update(type_de_champ, row_id, updated_by:)
+  def champ_upsert_by!(type_de_champ, row_id)
     check_valid_row_id_on_write?(type_de_champ, row_id)
-    attributes = type_de_champ.params_for_champ
+    champ_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
-      .create_with(**attributes)
+      .create_with(**champ_attributes)
       .find_or_create_by!(stable_id: type_de_champ.stable_id, row_id:)
 
-    attributes[:id] = champ.id
-    attributes[:updated_by] = updated_by
-
     # Needed when a revision change the champ type in this case, we reset the champ data
-    if champ.type != attributes[:type]
-      attributes[:value] = nil
-      attributes[:value_json] = nil
-      attributes[:external_id] = nil
-      attributes[:data] = nil
-      champ = champ.becomes!(attributes[:type].constantize)
-      champ.save!
+    if champ.type != champ_attributes[:type]
+      champ_attributes[:value] = nil
+      champ_attributes[:value_json] = nil
+      champ_attributes[:external_id] = nil
+      champ_attributes[:data] = nil
+      champ = champ.becomes!(champ_attributes[:type].constantize)
     end
 
+    champ.assign_attributes(champ_attributes)
+    champ.save!
+
     reset_champ_cache(champ)
 
-    [champ, attributes]
+    champ
   end
 
   def check_valid_row_id_on_write?(type_de_champ, row_id)

From 10333908c3aac15c052da9593bae79488bff1dd7 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Mon, 9 Dec 2024 09:36:27 +0100
Subject: [PATCH 10/10] refactor(champ): make row_id a named param

---
 .../editable_champ/section_component.rb       |  4 +--
 .../viewable_champ/section_component.rb       |  4 +--
 app/controllers/champs/champ_controller.rb    |  4 +--
 .../instructeurs/dossiers_controller.rb       |  2 +-
 app/controllers/users/dossiers_controller.rb  |  2 +-
 .../mutations/dossier_modifier_annotation.rb  |  2 +-
 ...ssier_modifier_annotation_ajouter_ligne.rb |  2 +-
 app/models/attestation_template.rb            |  2 +-
 app/models/concerns/dossier_champs_concern.rb | 26 +++++++++----------
 app/models/concerns/dossier_clone_concern.rb  |  4 +--
 app/models/concerns/dossier_export_concern.rb |  2 +-
 .../concerns/dossier_sections_concern.rb      |  2 +-
 .../prefill_repetition_type_de_champ.rb       |  2 +-
 spec/models/columns/champ_column_spec.rb      |  4 +--
 .../concerns/dossier_champs_concern_spec.rb   | 18 ++++++-------
 .../concerns/dossier_clone_concern_spec.rb    |  4 +--
 .../concerns/dossier_sections_concern_spec.rb |  4 +--
 .../tags_substitution_concern_spec.rb         |  4 +--
 spec/models/type_de_champ_spec.rb             |  2 +-
 spec/rails_helper.rb                          |  2 +-
 .../shared/dossiers/_edit.html.haml_spec.rb   |  6 ++---
 21 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/app/components/editable_champ/section_component.rb b/app/components/editable_champ/section_component.rb
index 9798b9e70..8f3870f55 100644
--- a/app/components/editable_champ/section_component.rb
+++ b/app/components/editable_champ/section_component.rb
@@ -17,7 +17,7 @@ class EditableChamp::SectionComponent < ApplicationComponent
 
   def header_section
     node = @nodes.first
-    @dossier.project_champ(node, @row_id) if node.is_a?(TypeDeChamp) && node.header_section?
+    @dossier.project_champ(node, row_id: @row_id) if node.is_a?(TypeDeChamp) && node.header_section?
   end
 
   def splitted_tail
@@ -40,7 +40,7 @@ class EditableChamp::SectionComponent < ApplicationComponent
     when EditableChamp::SectionComponent
       [node, nil]
     else
-      [nil, @dossier.project_champ(node, @row_id)]
+      [nil, @dossier.project_champ(node, row_id: @row_id)]
     end
   end
 
diff --git a/app/components/viewable_champ/section_component.rb b/app/components/viewable_champ/section_component.rb
index 9120ca96c..27b3ccf7b 100644
--- a/app/components/viewable_champ/section_component.rb
+++ b/app/components/viewable_champ/section_component.rb
@@ -18,11 +18,11 @@ class ViewableChamp::SectionComponent < ApplicationComponent
 
   def header_section
     node = @nodes.first
-    @dossier.project_champ(node, @row_id) if node.is_a?(TypeDeChamp) && node.header_section?
+    @dossier.project_champ(node, row_id: @row_id) if node.is_a?(TypeDeChamp) && node.header_section?
   end
 
   def champs
-    tail.filter_map { _1.is_a?(TypeDeChamp) ? @dossier.project_champ(_1, @row_id) : nil }
+    tail.filter_map { _1.is_a?(TypeDeChamp) ? @dossier.project_champ(_1, row_id: @row_id) : nil }
   end
 
   def sections
diff --git a/app/controllers/champs/champ_controller.rb b/app/controllers/champs/champ_controller.rb
index 5af93d346..b551009f0 100644
--- a/app/controllers/champs/champ_controller.rb
+++ b/app/controllers/champs/champ_controller.rb
@@ -10,9 +10,9 @@ class Champs::ChampController < ApplicationController
     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])
     if type_de_champ.repetition?
-      dossier.project_champ(type_de_champ, nil)
+      dossier.project_champ(type_de_champ)
     else
-      dossier.champ_for_update(type_de_champ, params_row_id, updated_by: current_user.email)
+      dossier.champ_for_update(type_de_champ, row_id: params_row_id, updated_by: current_user.email)
     end
   end
 
diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb
index 62a2fc575..0d1bf2600 100644
--- a/app/controllers/instructeurs/dossiers_controller.rb
+++ b/app/controllers/instructeurs/dossiers_controller.rb
@@ -321,7 +321,7 @@ module Instructeurs
     def annotation
       @dossier = dossier_with_champs(pj_template: false)
       type_de_champ = @dossier.find_type_de_champ_by_stable_id(params[:stable_id], :private)
-      annotation = @dossier.project_champ(type_de_champ, params[:row_id])
+      annotation = @dossier.project_champ(type_de_champ, row_id: params[:row_id])
 
       respond_to do |format|
         format.turbo_stream do
diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb
index 1fff9841a..626cb0692 100644
--- a/app/controllers/users/dossiers_controller.rb
+++ b/app/controllers/users/dossiers_controller.rb
@@ -299,7 +299,7 @@ module Users
     def champ
       @dossier = dossier_with_champs(pj_template: false)
       type_de_champ = dossier.find_type_de_champ_by_stable_id(params[:stable_id], :public)
-      champ = dossier.project_champ(type_de_champ, params[:row_id])
+      champ = dossier.project_champ(type_de_champ, row_id: params[:row_id])
 
       respond_to do |format|
         format.turbo_stream do
diff --git a/app/graphql/mutations/dossier_modifier_annotation.rb b/app/graphql/mutations/dossier_modifier_annotation.rb
index ad3728236..01a25c077 100644
--- a/app/graphql/mutations/dossier_modifier_annotation.rb
+++ b/app/graphql/mutations/dossier_modifier_annotation.rb
@@ -46,7 +46,7 @@ module Mutations
         .find_by(type_champ: annotation_type_champ, stable_id:)
 
       return nil if type_de_champ.nil?
-      dossier.champ_for_update(type_de_champ, row_id, updated_by: current_administrateur.email)
+      dossier.champ_for_update(type_de_champ, row_id:, updated_by: current_administrateur.email)
     end
 
     def annotation_type_champ
diff --git a/app/graphql/mutations/dossier_modifier_annotation_ajouter_ligne.rb b/app/graphql/mutations/dossier_modifier_annotation_ajouter_ligne.rb
index cf65b97b1..61ee2069a 100644
--- a/app/graphql/mutations/dossier_modifier_annotation_ajouter_ligne.rb
+++ b/app/graphql/mutations/dossier_modifier_annotation_ajouter_ligne.rb
@@ -34,7 +34,7 @@ module Mutations
         .find_by(type_champ: TypeDeChamp.type_champs.fetch(:repetition), stable_id:)
 
       return nil if type_de_champ.nil?
-      dossier.project_champ(type_de_champ, nil)
+      dossier.project_champ(type_de_champ)
     end
   end
 end
diff --git a/app/models/attestation_template.rb b/app/models/attestation_template.rb
index 8aba5d352..10a3693d4 100644
--- a/app/models/attestation_template.rb
+++ b/app/models/attestation_template.rb
@@ -92,7 +92,7 @@ class AttestationTemplate < ApplicationRecord
     used_tags.filter_map do |used_tag|
       corresponding_type_de_champ = types_de_champ_by_tag_id[used_tag]
 
-      if corresponding_type_de_champ && dossier.project_champ(corresponding_type_de_champ, nil).blank?
+      if corresponding_type_de_champ && dossier.project_champ(corresponding_type_de_champ).blank?
         corresponding_type_de_champ
       end
     end
diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb
index 40eea293d..176bc72b3 100644
--- a/app/models/concerns/dossier_champs_concern.rb
+++ b/app/models/concerns/dossier_champs_concern.rb
@@ -3,7 +3,7 @@
 module DossierChampsConcern
   extend ActiveSupport::Concern
 
-  def project_champ(type_de_champ, row_id)
+  def project_champ(type_de_champ, row_id: nil)
     check_valid_row_id_on_read?(type_de_champ, row_id)
     champ = champs_by_public_id[type_de_champ.public_id(row_id)]
     if champ.nil? || !champ.is_type?(type_de_champ.type_champ)
@@ -17,11 +17,11 @@ module DossierChampsConcern
   end
 
   def project_champs_public
-    @project_champs_public ||= revision.types_de_champ_public.map { project_champ(_1, nil) }
+    @project_champs_public ||= revision.types_de_champ_public.map { project_champ(_1) }
   end
 
   def project_champs_private
-    @project_champs_private ||= revision.types_de_champ_private.map { project_champ(_1, nil) }
+    @project_champs_private ||= revision.types_de_champ_private.map { project_champ(_1) }
   end
 
   def filled_champs_public
@@ -54,7 +54,7 @@ module DossierChampsConcern
 
   def project_champs_public_all
     revision.types_de_champ_public.flat_map do |type_de_champ|
-      champ = project_champ(type_de_champ, nil)
+      champ = project_champ(type_de_champ)
       if type_de_champ.repetition?
         [champ] + project_rows_for(type_de_champ).flatten
       else
@@ -65,7 +65,7 @@ module DossierChampsConcern
 
   def project_champs_private_all
     revision.types_de_champ_private.flat_map do |type_de_champ|
-      champ = project_champ(type_de_champ, nil)
+      champ = project_champ(type_de_champ)
       if type_de_champ.repetition?
         [champ] + project_rows_for(type_de_champ).flatten
       else
@@ -81,7 +81,7 @@ module DossierChampsConcern
     row_ids = repetition_row_ids(type_de_champ)
 
     row_ids.map do |row_id|
-      children.map { project_champ(_1, row_id) }
+      children.map { project_champ(_1, row_id:) }
     end
   end
 
@@ -101,19 +101,19 @@ module DossierChampsConcern
       .types_de_champ
       .filter { _1.stable_id.in?(stable_ids) }
       .filter { !_1.child?(revision) }
-      .map { _1.repetition? ? project_champ(_1, nil) : champ_for_update(_1, nil, updated_by: nil) }
+      .map { _1.repetition? ? project_champ(_1) : champ_for_update(_1, updated_by: nil) }
   end
 
   def champ_value_for_tag(type_de_champ, path = :value)
     champ = if type_de_champ.repetition?
-      project_champ(type_de_champ, nil)
+      project_champ(type_de_champ)
     else
-      filled_champ(type_de_champ, nil)
+      filled_champ(type_de_champ)
     end
     type_de_champ.champ_value_for_tag(champ, path)
   end
 
-  def champ_for_update(type_de_champ, row_id, updated_by:)
+  def champ_for_update(type_de_champ, row_id: nil, updated_by:)
     champ = champ_upsert_by!(type_de_champ, row_id)
     champ.updated_by = updated_by
     champ
@@ -155,7 +155,7 @@ module DossierChampsConcern
     raise "Can't add row to non-repetition type de champ" if !type_de_champ.repetition?
 
     row_id = ULID.generate
-    champ = champ_for_update(type_de_champ, row_id, updated_by:)
+    champ = champ_for_update(type_de_champ, row_id:, updated_by:)
     champ.save!
     reset_champ_cache(champ)
     row_id
@@ -164,7 +164,7 @@ module DossierChampsConcern
   def repetition_remove_row(type_de_champ, row_id, updated_by:)
     raise "Can't remove row from non-repetition type de champ" if !type_de_champ.repetition?
 
-    champ = champ_for_update(type_de_champ, row_id, updated_by:)
+    champ = champ_for_update(type_de_champ, row_id:, updated_by:)
     champ.discard!
     reset_champ_cache(champ)
   end
@@ -191,7 +191,7 @@ module DossierChampsConcern
     champs.filter { stable_id_in_revision?(_1.stable_id) }
   end
 
-  def filled_champ(type_de_champ, row_id)
+  def filled_champ(type_de_champ, row_id: nil)
     champ = champs_by_public_id[type_de_champ.public_id(row_id)]
     if type_de_champ.champ_blank?(champ) || !champ.visible?
       nil
diff --git a/app/models/concerns/dossier_clone_concern.rb b/app/models/concerns/dossier_clone_concern.rb
index 582dec0d5..e147c9a20 100644
--- a/app/models/concerns/dossier_clone_concern.rb
+++ b/app/models/concerns/dossier_clone_concern.rb
@@ -175,10 +175,10 @@ module DossierCloneConcern
     end
 
     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!
+      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!
+      champ_for_update(repetition_type_de_champ, row_id:, updated_by: user.email).discard!
     end
   end
 end
diff --git a/app/models/concerns/dossier_export_concern.rb b/app/models/concerns/dossier_export_concern.rb
index 887b48207..ad717c299 100644
--- a/app/models/concerns/dossier_export_concern.rb
+++ b/app/models/concerns/dossier_export_concern.rb
@@ -17,7 +17,7 @@ module DossierExportConcern
 
   def champ_values_for_export(types_de_champ, row_id: nil, export_template: nil, format:)
     types_de_champ.flat_map do |type_de_champ|
-      champ = filled_champ(type_de_champ, row_id)
+      champ = filled_champ(type_de_champ, row_id:)
       if export_template.present?
         export_template
           .columns_for_stable_id(type_de_champ.stable_id)
diff --git a/app/models/concerns/dossier_sections_concern.rb b/app/models/concerns/dossier_sections_concern.rb
index c4644dba3..fcfb4c1d7 100644
--- a/app/models/concerns/dossier_sections_concern.rb
+++ b/app/models/concerns/dossier_sections_concern.rb
@@ -33,7 +33,7 @@ module DossierSectionsConcern
           return "#{index}.#{index_in_repetition + 1}" if index_in_repetition
         else
           return index if tdc.stable_id == type_de_champ.stable_id
-          next unless project_champ(tdc, nil).visible?
+          next unless project_champ(tdc).visible?
 
           index += 1 if tdc.header_section?
         end
diff --git a/app/models/types_de_champ/prefill_repetition_type_de_champ.rb b/app/models/types_de_champ/prefill_repetition_type_de_champ.rb
index 5af53c178..36843d748 100644
--- a/app/models/types_de_champ/prefill_repetition_type_de_champ.rb
+++ b/app/models/types_de_champ/prefill_repetition_type_de_champ.rb
@@ -65,7 +65,7 @@ class TypesDeChamp::PrefillRepetitionTypeDeChamp < TypesDeChamp::PrefillTypeDeCh
         type_de_champ = revision.types_de_champ.find { _1.stable_id == stable_id }
         next unless type_de_champ
 
-        subchamp = champ.dossier.champ_for_update(type_de_champ, row_id, updated_by: nil)
+        subchamp = champ.dossier.champ_for_update(type_de_champ, row_id:, updated_by: nil)
         TypesDeChamp::PrefillTypeDeChamp.build(subchamp.type_de_champ, revision).to_assignable_attributes(subchamp, value)
       end.compact
     end
diff --git a/spec/models/columns/champ_column_spec.rb b/spec/models/columns/champ_column_spec.rb
index a1bf8b8b9..af5cec1fa 100644
--- a/spec/models/columns/champ_column_spec.rb
+++ b/spec/models/columns/champ_column_spec.rb
@@ -155,13 +155,13 @@ describe Columns::ChampColumn do
 
   def expect_type_de_champ_values(type, assertion)
     type_de_champ = types_de_champ.find { _1.type_champ == type }
-    champ = dossier.send(:filled_champ, type_de_champ, nil)
+    champ = dossier.send(:filled_champ, type_de_champ)
     columns = type_de_champ.columns(procedure:)
     expect(columns.map { _1.value(champ) }).to assertion
   end
 
   def retrieve_champ(type)
     type_de_champ = types_de_champ.find { _1.type_champ == type }
-    dossier.send(:filled_champ, type_de_champ, nil)
+    dossier.send(:filled_champ, type_de_champ)
   end
 end
diff --git a/spec/models/concerns/dossier_champs_concern_spec.rb b/spec/models/concerns/dossier_champs_concern_spec.rb
index 7d5df328e..96b43cfdb 100644
--- a/spec/models/concerns/dossier_champs_concern_spec.rb
+++ b/spec/models/concerns/dossier_champs_concern_spec.rb
@@ -36,13 +36,13 @@ RSpec.describe DossierChampsConcern do
 
     context "public champ" do
       let(:row_id) { nil }
-      subject { dossier.project_champ(type_de_champ_public, row_id) }
+      subject { dossier.project_champ(type_de_champ_public, row_id:) }
 
       it { expect(subject.persisted?).to be_truthy }
 
       context "in repetition" do
         let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(994) }
-        let(:row_id) { dossier.project_champ(type_de_champ_repetition, nil).row_ids.first }
+        let(:row_id) { dossier.project_champ(type_de_champ_repetition).row_ids.first }
 
         it {
           expect(subject.new_record?).to be_truthy
@@ -88,7 +88,7 @@ RSpec.describe DossierChampsConcern do
     end
 
     context "private champ" do
-      subject { dossier.project_champ(type_de_champ_private, nil) }
+      subject { dossier.project_champ(type_de_champ_private) }
 
       it { expect(subject.persisted?).to be_truthy }
 
@@ -225,7 +225,7 @@ RSpec.describe DossierChampsConcern do
     let(:row_id) { nil }
 
     context "public champ" do
-      subject { dossier.champ_for_update(type_de_champ_public, row_id, updated_by: dossier.user.email) }
+      subject { dossier.champ_for_update(type_de_champ_public, row_id:, updated_by: dossier.user.email) }
 
       it {
         expect(subject.persisted?).to be_truthy
@@ -283,7 +283,7 @@ RSpec.describe DossierChampsConcern do
     end
 
     context "private champ" do
-      subject { dossier.champ_for_update(type_de_champ_private, row_id, updated_by: dossier.user.email) }
+      subject { dossier.champ_for_update(type_de_champ_private, row_id:, updated_by: dossier.user.email) }
 
       it {
         expect(subject.persisted?).to be_truthy
@@ -304,9 +304,9 @@ RSpec.describe DossierChampsConcern do
       }
     end
 
-    let(:champ_99) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(99), nil) }
-    let(:champ_991) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(991), nil) }
-    let(:champ_994) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(994), row_id) }
+    let(:champ_99) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(99)) }
+    let(:champ_991) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(991)) }
+    let(:champ_994) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(994), row_id:) }
 
     subject { dossier.update_champs_attributes(attributes, :public, updated_by: dossier.user.email) }
 
@@ -365,7 +365,7 @@ RSpec.describe DossierChampsConcern do
       }
     end
 
-    let(:annotation_995) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(995), nil) }
+    let(:annotation_995) { dossier.project_champ(dossier.find_type_de_champ_by_stable_id(995)) }
 
     subject { dossier.update_champs_attributes(attributes, :private, updated_by: dossier.user.email) }
 
diff --git a/spec/models/concerns/dossier_clone_concern_spec.rb b/spec/models/concerns/dossier_clone_concern_spec.rb
index 853454cdb..7854b7438 100644
--- a/spec/models/concerns/dossier_clone_concern_spec.rb
+++ b/spec/models/concerns/dossier_clone_concern_spec.rb
@@ -368,13 +368,13 @@ RSpec.describe DossierCloneConcern do
     context 'with new revision' do
       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')
+        forked_dossier.champ_for_update(tdc, 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')
+        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 } }
diff --git a/spec/models/concerns/dossier_sections_concern_spec.rb b/spec/models/concerns/dossier_sections_concern_spec.rb
index 17f86be88..33b77a4b3 100644
--- a/spec/models/concerns/dossier_sections_concern_spec.rb
+++ b/spec/models/concerns/dossier_sections_concern_spec.rb
@@ -72,7 +72,7 @@ describe DossierSectionsConcern do
     context "when there are invisible sections" do
       it "index accordingly header sections" do
          expect(dossier.index_for_section_header(headers[0])).to eq(1)
-         expect(dossier.project_champ(headers[1], nil)).not_to be_visible
+         expect(dossier.project_champ(headers[1])).not_to be_visible
          expect(dossier.index_for_section_header(headers[2])).to eq(2)
        end
     end
@@ -81,7 +81,7 @@ describe DossierSectionsConcern do
       let(:number_value) { 5 }
       it "index accordingly header sections" do
         expect(dossier.index_for_section_header(headers[0])).to eq(1)
-        expect(dossier.project_champ(headers[1], nil)).to be_visible
+        expect(dossier.project_champ(headers[1])).to be_visible
         expect(dossier.index_for_section_header(headers[1])).to eq(2)
         expect(dossier.index_for_section_header(headers[2])).to eq(3)
       end
diff --git a/spec/models/concerns/tags_substitution_concern_spec.rb b/spec/models/concerns/tags_substitution_concern_spec.rb
index d70dce98c..a9d10e3bc 100644
--- a/spec/models/concerns/tags_substitution_concern_spec.rb
+++ b/spec/models/concerns/tags_substitution_concern_spec.rb
@@ -258,7 +258,7 @@ describe TagsSubstitutionConcern, type: :model do
 
       context 'and the champ has a primary value' do
         before do
-          dossier.champ_for_update(type_de_champ, nil, updated_by: 'test').update(primary_value: 'primo')
+          dossier.champ_for_update(type_de_champ, updated_by: 'test').update(primary_value: 'primo')
           dossier.reload
         end
 
@@ -266,7 +266,7 @@ describe TagsSubstitutionConcern, type: :model do
 
         context 'and the champ has a secondary value' do
           before do
-            dossier.champ_for_update(type_de_champ, nil, updated_by: 'test').update(secondary_value: 'secundo')
+            dossier.champ_for_update(type_de_champ, updated_by: 'test').update(secondary_value: 'secundo')
             dossier.reload
           end
 
diff --git a/spec/models/type_de_champ_spec.rb b/spec/models/type_de_champ_spec.rb
index b8d4e44f3..1ae4c52f9 100644
--- a/spec/models/type_de_champ_spec.rb
+++ b/spec/models/type_de_champ_spec.rb
@@ -17,7 +17,7 @@ describe TypeDeChamp do
 
       it do
         dossier.revision.types_de_champ_public.each do |type_de_champ|
-          champ = dossier.project_champ(type_de_champ, nil)
+          champ = dossier.project_champ(type_de_champ)
           expect(type_de_champ.dynamic_type.class.name).to match(/^TypesDeChamp::/)
           expect(champ.class.name).to match(/^Champs::/)
         end
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 170837f9c..8298dfceb 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -127,7 +127,7 @@ RSpec.configure do |config|
 
   module SpecHelpers
     def champ_for_update(champ)
-      champ.dossier.champ_for_update(champ.type_de_champ, champ.row_id, updated_by: 'test')
+      champ.dossier.champ_for_update(champ.type_de_champ, row_id: champ.row_id, updated_by: 'test')
     end
   end
 
diff --git a/spec/views/shared/dossiers/_edit.html.haml_spec.rb b/spec/views/shared/dossiers/_edit.html.haml_spec.rb
index d2222c027..4271d129b 100644
--- a/spec/views/shared/dossiers/_edit.html.haml_spec.rb
+++ b/spec/views/shared/dossiers/_edit.html.haml_spec.rb
@@ -19,9 +19,9 @@ describe 'shared/dossiers/edit', type: :view do
     let(:type_de_champ_checkbox) { procedure.draft_types_de_champ_public.find(&:checkbox?) }
     let(:type_de_champ_textarea) { procedure.draft_types_de_champ_public.find(&:textarea?) }
 
-    let(:champ_checkbox) { dossier.project_champ(type_de_champ_checkbox, nil) }
-    let(:champ_dossier_link) { dossier.project_champ(type_de_champ_dossier_link, nil) }
-    let(:champ_textarea) { dossier.project_champ(type_de_champ_textarea, nil) }
+    let(:champ_checkbox) { dossier.project_champ(type_de_champ_checkbox) }
+    let(:champ_dossier_link) { dossier.project_champ(type_de_champ_dossier_link) }
+    let(:champ_textarea) { dossier.project_champ(type_de_champ_textarea) }
 
     let(:types_de_champ_public) { [{ type: :checkbox }, { type: :header_section }, { type: :explication }, { type: :dossier_link }, { type: :textarea }] }