From 1b95809f14a2604f6d5d0ba57d099a3e5ac3ec70 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 29 Nov 2022 11:30:06 +0100 Subject: [PATCH] refactor(dossier): flatten champ attributes --- .../editable_champ_component.rb | 2 +- .../editable_champ_component.html.haml | 2 +- .../instructeurs/dossiers_controller.rb | 6 ++- app/controllers/users/dossiers_controller.rb | 44 ++++++++----------- .../controllers/autosave_controller.ts | 11 +---- app/models/champ.rb | 14 ++---- app/models/dossier.rb | 8 +++- app/models/dossier_preloader.rb | 13 ++++++ .../instructeurs/dossiers_controller_spec.rb | 7 +-- .../users/dossiers_controller_spec.rb | 22 +--------- spec/models/champ_spec.rb | 4 +- spec/models/dossier_preloader_spec.rb | 8 ++++ 12 files changed, 61 insertions(+), 80 deletions(-) diff --git a/app/components/editable_champ/editable_champ_component.rb b/app/components/editable_champ/editable_champ_component.rb index a705c1f6f..d3a314511 100644 --- a/app/components/editable_champ/editable_champ_component.rb +++ b/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 diff --git a/app/components/editable_champ/editable_champ_component/editable_champ_component.html.haml b/app/components/editable_champ/editable_champ_component/editable_champ_component.html.haml index d0f4362d9..b98f43654 100644 --- a/app/components/editable_champ/editable_champ_component/editable_champ_component.html.haml +++ b/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) diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb index 6bf5fd869..9e2f110d6 100644 --- a/app/controllers/instructeurs/dossiers_controller.rb +++ b/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 diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index f6f50cc56..6aa397d38 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/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_params - params.permit(dossier: { - champs_public_attributes: [ - :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: []] - ] - }) + def champs_public_params + champs_params = params.require(:dossier).permit(champs_public_attributes: [ + :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_params[:dossier]) - # FIXME: in some cases a removed repetition bloc row is submitted. - # In this case it will be treated as a new record, and the action will fail. - @dossier.champs_public.filter(&:block?).each do |champ| - champ.champs = champ.champs.filter(&:persisted?) - 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 + @dossier.assign_attributes(champs_public_params) + if @dossier.champs_public_all.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? - @dossier.assign_to_groupe_instructeur(groupe_instructeur_from_params) - end + if should_change_groupe_instructeur? + @dossier.assign_to_groupe_instructeur(groupe_instructeur_from_params) end if dossier.en_construction? diff --git a/app/javascript/controllers/autosave_controller.ts b/app/javascript/controllers/autosave_controller.ts index 93fd4f190..6c7746b33 100644 --- a/app/javascript/controllers/autosave_controller.ts +++ b/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( 'input:not([type=file]), textarea, select' ) ].filter((element) => !element.disabled); - - const parent = this.element.closest('[data-block]'); - if (parent) { - return [ - ...inputs, - ...parent.querySelectorAll('input[data-id]') - ]; - } - return inputs; } } diff --git a/app/models/champ.rb b/app/models/champ.rb index f06f922fa..7d95b352d 100644 --- a/app/models/champ.rb +++ b/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 - "#{parent.input_name}[champs_attributes][#{id}]" + if private? + "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 diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 1563dae31..510a34659 100644 --- a/app/models/dossier.rb +++ b/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 diff --git a/app/models/dossier_preloader.rb b/app/models/dossier_preloader.rb index b8c502075..417304202 100644 --- a/app/models/dossier_preloader.rb +++ b/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 diff --git a/spec/controllers/instructeurs/dossiers_controller_spec.rb b/spec/controllers/instructeurs/dossiers_controller_spec.rb index 287fdd2c5..75d61e993 100644 --- a/spec/controllers/instructeurs/dossiers_controller_spec.rb +++ b/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, - champs_attributes: { - id: champ_repetition.champs.first.id, - value: 'text' - } + id: champ_repetition.champs.first.id, + value: 'text' } } } diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index da4bb9f9e..478792f2d 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/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) } diff --git a/spec/models/champ_spec.rb b/spec/models/champ_spec.rb index 6ef81bde5..75e2d61a4 100644 --- a/spec/models/champ_spec.rb +++ b/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 diff --git a/spec/models/dossier_preloader_spec.rb b/spec/models/dossier_preloader_spec.rb index 8287b0606..cf8ca625e 100644 --- a/spec/models/dossier_preloader_spec.rb +++ b/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