Merge pull request #8101 from tchak/refactor-nested-champs-attributes

refactor(dossier): flatten champ attributes

app/components/editable_champ/editable_champ_component.rb

@@ -23,7 +23,7 @@ class EditableChamp::EditableChampComponent < ApplicationComponent
         "hidden": !@champ.visible?
       ),
       id: @champ.input_group_id,
-      data: { controller: stimulus_controller, block: @champ.block? }
+      data: { controller: stimulus_controller }
     }
   end
 

app/components/editable_champ/editable_champ_component/editable_champ_component.html.haml

@@ -9,5 +9,5 @@
   - if @champ.type_champ == "titre_identite"
     %p.notice  Carte nationale d’identité (uniquement le recto), passeport, titre de séjour ou autre justificatif d’identité.
 
-  = @form.hidden_field :id, value: @champ.id, data: @champ.block? ? { id: true } : {}
+  = @form.hidden_field :id, value: @champ.id
   = render component_class.new(form: @form, champ: @champ, seen_at: @seen_at)

app/controllers/instructeurs/dossiers_controller.rb

@@ -212,7 +212,7 @@ module Instructeurs
 
     def update_annotations
       dossier_with_champs.assign_attributes(champs_private_params)
-      if dossier.champs_private.any?(&:changed?)
+      if dossier.champs_private_all.any?(&:changed?)
         dossier.last_champ_private_updated_at = Time.zone.now
       end
       dossier.save
@@ -286,10 +286,12 @@ module Instructeurs
     end
 
     def champs_private_params
-      params.require(:dossier).permit(champs_private_attributes: [
+      champs_params = params.require(:dossier).permit(champs_private_attributes: [
         :id, :primary_value, :secondary_value, :piece_justificative_file, :value_other, :external_id, :numero_allocataire, :code_postal, :departement, :code_departement, :value, value: [],
         champs_attributes: [:id, :_destroy, :value, :primary_value, :secondary_value, :piece_justificative_file, :value_other, :external_id, :numero_allocataire, :code_postal, :departement, :code_departement, value: []]
       ])
+      champs_params[:champs_private_all_attributes] = champs_params.delete(:champs_private_attributes) || {}
+      champs_params
     end
 
     def mark_demande_as_read

app/controllers/users/dossiers_controller.rb

@@ -200,7 +200,7 @@ module Users
       respond_to do |format|
         format.html { render :brouillon }
         format.turbo_stream do
-          @to_shows, @to_hides = @dossier.champs_public
+          @to_shows, @to_hides = @dossier.champs_public_all
             .filter(&:conditional?)
             .partition(&:visible?)
             .map { |champs| champs_to_one_selector(champs) }
@@ -221,7 +221,7 @@ module Users
       respond_to do |format|
         format.html { render :modifier }
         format.turbo_stream do
-          @to_shows, @to_hides = @dossier.champs_public
+          @to_shows, @to_hides = @dossier.champs_public_all
             .filter(&:conditional?)
             .partition(&:visible?)
             .map { |champs| champs_to_one_selector(champs) }
@@ -391,14 +391,13 @@ module Users
       [params[:page].to_i, 1].max
     end
 
-    # FIXME: require(:dossier) when all the champs are united
+    def champs_public_params
-    def champs_params
+      champs_params = params.require(:dossier).permit(champs_public_attributes: [
-      params.permit(dossier: {
+        :id, :value, :value_other, :external_id, :primary_value, :secondary_value, :numero_allocataire, :code_postal, :identifiant, :numero_fiscal, :reference_avis, :ine, :piece_justificative_file, :departement, :code_departement, value: [],
-        champs_public_attributes: [
+        champs_attributes: [:id, :_destroy, :value, :value_other, :external_id, :primary_value, :secondary_value, :numero_allocataire, :code_postal, :identifiant, :numero_fiscal, :reference_avis, :ine, :piece_justificative_file, :departement, :code_departement, value: []]
-          :id, :value, :value_other, :external_id, :primary_value, :secondary_value, :numero_allocataire, :code_postal, :identifiant, :numero_fiscal, :reference_avis, :ine, :piece_justificative_file, :departement, :code_departement, value: [],
+      ])
-          champs_attributes: [:id, :_destroy, :value, :value_other, :external_id, :primary_value, :secondary_value, :numero_allocataire, :code_postal, :identifiant, :numero_fiscal, :reference_avis, :ine, :piece_justificative_file, :departement, :code_departement, value: []]
+      champs_params[:champs_public_all_attributes] = champs_params.delete(:champs_public_attributes) || {}
-        ]
+      champs_params
-      })
     end
 
     def dossier_scope
@@ -454,23 +453,16 @@ module Users
     def update_dossier_and_compute_errors
       errors = []
 
-      if champs_params[:dossier]
+      @dossier.assign_attributes(champs_public_params)
-        @dossier.assign_attributes(champs_params[:dossier])
+      if @dossier.champs_public_all.any?(&:changed_for_autosave?)
-        # FIXME: in some cases a removed repetition bloc row is submitted.
+        @dossier.last_champ_updated_at = Time.zone.now
-        # In this case it will be treated as a new record, and the action will fail.
+      end
-        @dossier.champs_public.filter(&:block?).each do |champ|
+      if !@dossier.save(**validation_options)
-          champ.champs = champ.champs.filter(&:persisted?)
+        errors += @dossier.errors.full_messages
-        end
+      end
-        if @dossier.champs_public.any?(&:changed_for_autosave?)
-          @dossier.last_champ_updated_at = Time.zone.now
-        end
-        if !@dossier.save(**validation_options)
-          errors += @dossier.errors.full_messages
-        end
 
-        if should_change_groupe_instructeur?
+      if should_change_groupe_instructeur?
-          @dossier.assign_to_groupe_instructeur(groupe_instructeur_from_params)
+        @dossier.assign_to_groupe_instructeur(groupe_instructeur_from_params)
-        end
       end
 
       if dossier.en_construction?

app/javascript/controllers/autosave_controller.ts

@@ -201,19 +201,10 @@ export class AutosaveController extends ApplicationController {
   private get inputs() {
     const element = this.element as HTMLElement;
 
-    const inputs = [
+    return [
       ...element.querySelectorAll<HTMLInputElement>(
         'input:not([type=file]), textarea, select'
       )
     ].filter((element) => !element.disabled);
-
-    const parent = this.element.closest('[data-block]');
-    if (parent) {
-      return [
-        ...inputs,
-        ...parent.querySelectorAll<HTMLInputElement>('input[data-id]')
-      ];
-    }
-    return inputs;
   }
 }

app/models/champ.rb

@@ -178,10 +178,10 @@ class Champ < ApplicationRecord
   # attributes. So instead of the field index, this method uses the champ id; which gives us an independent and
   # predictable input name.
   def input_name
-    if parent_id
+    if private?
-      "#{parent.input_name}[champs_attributes][#{id}]"
+      "dossier[champs_private_attributes][#{id}]"
     else
-      "dossier[#{champs_attributes_accessor}][#{id}]"
+      "dossier[champs_public_attributes][#{id}]"
     end
   end
 
@@ -246,14 +246,6 @@ class Champ < ApplicationRecord
     "#{stable_id}-#{id}"
   end
 
-  def champs_attributes_accessor
-    if private?
-      "champs_private_attributes"
-    else
-      "champs_public_attributes"
-    end
-  end
-
   def needs_dossier_id?
     !dossier_id && parent_id
   end

app/models/dossier.rb

@@ -79,9 +79,13 @@ class Dossier < ApplicationRecord
 
   has_one_attached :justificatif_motivation
 
+  has_many :champs
   has_many :champs_public, -> { root.public_ordered }, class_name: 'Champ', inverse_of: false, dependent: :destroy
   has_many :champs_private, -> { root.private_ordered }, class_name: 'Champ', inverse_of: false, dependent: :destroy
-  has_many :prefilled_champs_public, -> { root.public_only.prefilled }, class_name: 'Champ', inverse_of: false, dependent: :destroy
+  has_many :champs_public_all, -> { public_only }, class_name: 'Champ', inverse_of: false
+  has_many :champs_private_all, -> { private_only }, class_name: 'Champ', inverse_of: false
+  has_many :prefilled_champs_public, -> { root.public_only.prefilled }, class_name: 'Champ', inverse_of: false
+
   has_many :commentaires, inverse_of: :dossier, dependent: :destroy
   has_many :invites, dependent: :destroy
   has_many :follows, -> { active }, inverse_of: :dossier
@@ -151,6 +155,8 @@ class Dossier < ApplicationRecord
 
   accepts_nested_attributes_for :champs_public
   accepts_nested_attributes_for :champs_private
+  accepts_nested_attributes_for :champs_public_all
+  accepts_nested_attributes_for :champs_private_all
 
   include AASM
 

app/models/dossier_preloader.rb

@@ -78,6 +78,9 @@ class DossierPreloader
   def load_dossier(dossier, champs, children_by_parent = {})
     champs_public, champs_private = champs.partition(&:public?)
 
+    dossier.association(:champs).target = []
+    dossier.association(:champs_public_all).target = []
+    dossier.association(:champs_private_all).target = []
     load_champs(dossier, :champs_public, champs_public, dossier, children_by_parent)
     load_champs(dossier, :champs_private, champs_private, dossier, children_by_parent)
 
@@ -90,6 +93,8 @@ class DossierPreloader
   end
 
   def load_champs(parent, name, champs, dossier, children_by_parent)
+    return if champs.empty?
+
     champs.each do |champ|
       champ.association(:dossier).target = dossier
 
@@ -98,6 +103,14 @@ class DossierPreloader
       end
     end
 
+    dossier.association(:champs).target += champs
+
+    if champs.first.public?
+      dossier.association(:champs_public_all).target += champs
+    else
+      dossier.association(:champs_private_all).target += champs
+    end
+
     parent.association(name).target = champs.sort_by do |champ|
       [champ.row, positions[dossier.revision_id][champ.type_de_champ_id]]
     end

spec/controllers/instructeurs/dossiers_controller_spec.rb

@@ -793,11 +793,8 @@ describe Instructeurs::DossiersController, type: :controller do
                 secondary_value: 'secondary'
               },
               '3': {
-                id: champ_repetition.id,
+                id: champ_repetition.champs.first.id,
-                champs_attributes: {
+                value: 'text'
-                  id: champ_repetition.champs.first.id,
-                  value: 'text'
-                }
               }
             }
           }

spec/controllers/users/dossiers_controller_spec.rb

@@ -494,7 +494,7 @@ describe Users::DossiersController, type: :controller do
           {
             id: dossier.id,
             dossier: {
-              champs_public_attributes: {}
+              champs_public_attributes: [{ value: '' }]
             }
           }
         end
@@ -506,16 +506,6 @@ describe Users::DossiersController, type: :controller do
       end
     end
 
-    context 'when dossier has no champ' do
-      let(:submit_payload) { { id: dossier.id } }
-
-      it 'does not raise any errors' do
-        subject
-
-        expect(response).to have_http_status(:ok)
-      end
-    end
-
     context 'when the user has an invitation but is not the owner' do
       let(:dossier) { create(:dossier) }
       let!(:invite) { create(:invite, dossier: dossier, user: user) }
@@ -664,16 +654,6 @@ describe Users::DossiersController, type: :controller do
       it { expect(flash.alert).to eq(['Le champ l doit être rempli.']) }
     end
 
-    context 'when dossier has no champ' do
-      let(:submit_payload) { { id: dossier.id } }
-
-      it 'does not raise any errors' do
-        subject
-
-        expect(response).to have_http_status(:ok)
-      end
-    end
-
     context 'when the user has an invitation but is not the owner' do
       let(:dossier) { create(:dossier, :en_construction) }
       let!(:invite) { create(:invite, dossier: dossier, user: user) }

spec/models/champ_spec.rb

@@ -599,12 +599,12 @@ describe Champ do
 
       context "when has parent" do
         let(:champ) { create(:champ_text, parent: create(:champ_text)) }
-        it { expect(champ.input_name).to eq "dossier[champs_public_attributes][#{champ.parent_id}][champs_attributes][#{champ.id}]" }
+        it { expect(champ.input_name).to eq "dossier[champs_public_attributes][#{champ.id}]" }
       end
 
       context "when has private parent" do
         let(:champ) { create(:champ_text, private: true, parent: create(:champ_text, private: true)) }
-        it { expect(champ.input_name).to eq "dossier[champs_private_attributes][#{champ.parent_id}][champs_attributes][#{champ.id}]" }
+        it { expect(champ.input_name).to eq "dossier[champs_private_attributes][#{champ.id}]" }
       end
     end
   end

spec/models/dossier_preloader_spec.rb

@@ -26,8 +26,16 @@ describe DossierPreloader do
 
         expect(first_child.type).to eq('Champs::TextChamp')
         expect(repetition.id).not_to eq(first_child.id)
+        expect(subject.champs.first.dossier).to eq(subject)
+        expect(subject.champs_public_all.first.dossier).to eq(subject)
         expect(subject.champs_public.first.dossier).to eq(subject)
+
         expect(subject.champs_public.first.type_de_champ.piece_justificative_template.attached?).to eq(false)
+
+        expect(subject.champs.first.conditional?).to eq(false)
+        expect(subject.champs_public_all.first.conditional?).to eq(false)
+        expect(subject.champs_public.first.conditional?).to eq(false)
+
         expect(first_child.parent).to eq(repetition)
       end