From 6ad0b901b6c09f5f069439c07d08e69e36d46fa6 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 21 Mar 2024 12:01:55 +0100 Subject: [PATCH] fix(champ): use public_id in views --- app/components/attachment/edit_component.rb | 2 +- .../attachment/multiple_component.rb | 2 +- .../drop_down_list_component.html.haml | 12 +++++----- app/models/champ.rb | 14 +++++++----- app/models/concerns/dossier_clone_concern.rb | 14 ++++++------ app/models/dossier.rb | 10 ++++----- app/models/type_de_champ.rb | 8 +++++++ .../action_view_record_identifier.rb | 22 +++++++++++++++++++ spec/models/champ_spec.rb | 9 ++++++++ spec/system/users/en_construction_spec.rb | 2 +- 10 files changed, 68 insertions(+), 27 deletions(-) create mode 100644 config/initializers/action_view_record_identifier.rb diff --git a/app/components/attachment/edit_component.rb b/app/components/attachment/edit_component.rb index 81803c113..05ddb7b18 100644 --- a/app/components/attachment/edit_component.rb +++ b/app/components/attachment/edit_component.rb @@ -54,7 +54,7 @@ class Attachment::EditComponent < ApplicationComponent end def destroy_attachment_path - attachment_path(champ_id: champ&.id) + attachment_path(champ_id: champ&.public_id) end def attachment_input_class diff --git a/app/components/attachment/multiple_component.rb b/app/components/attachment/multiple_component.rb index c0229bf90..b5900cd13 100644 --- a/app/components/attachment/multiple_component.rb +++ b/app/components/attachment/multiple_component.rb @@ -35,7 +35,7 @@ class Attachment::MultipleComponent < ApplicationComponent end def empty_component_id - "attachment-multiple-empty-#{champ.id}" + "attachment-multiple-empty-#{champ.public_id}" end def auto_attach_url diff --git a/app/components/editable_champ/drop_down_list_component/drop_down_list_component.html.haml b/app/components/editable_champ/drop_down_list_component/drop_down_list_component.html.haml index 0722af4f3..c2268eba0 100644 --- a/app/components/editable_champ/drop_down_list_component/drop_down_list_component.html.haml +++ b/app/components/editable_champ/drop_down_list_component/drop_down_list_component.html.haml @@ -2,20 +2,20 @@ .fr-fieldset__content - @champ.enabled_non_empty_options.each_with_index do |option, index| .fr-radio-group - = @form.radio_button :value, option, id: "#{@champ.id}_radio_option_#{index}" - %label.fr-label{ for: "#{@champ.id}_radio_option_#{index}" } + = @form.radio_button :value, option, id: dom_id(@champ, "radio_option_#{index}") + %label.fr-label{ for: dom_id(@champ, "radio_option_#{index}") } = option - if !@champ.mandatory? .fr-radio-group - = @form.radio_button :value, '', checked: @champ.value.blank? && !@champ.other?, id: "#{@champ.id}_radio_option_blank" - %label.fr-label{ for: "#{@champ.id}_radio_option_blank" } + = @form.radio_button :value, '', checked: @champ.value.blank? && !@champ.other?, id: dom_id(@champ, "radio_option_blank") + %label.fr-label{ for: dom_id(@champ, "radio_option_blank") } Non renseigné - if @champ.drop_down_other? .fr-radio-group - = @form.radio_button :value, Champs::DropDownListChamp::OTHER, checked: @champ.other?, id: "#{@champ.id}_radio_option_other" - %label.fr-label{ for: "#{@champ.id}_radio_option_other" } + = @form.radio_button :value, Champs::DropDownListChamp::OTHER, checked: @champ.other?, id: dom_id(@champ, "radio_option_other") + %label.fr-label{ for: dom_id(@champ, "radio_option_other") } = t('shared.champs.drop_down_list.other') - elsif @champ.render_as_combobox? = render Dsfr::ComboboxComponent.new form: @form, options: @champ.enabled_non_empty_options(other: true), selected: @champ.selected, input_html_options: { name: :value, id: @champ.input_id, class: select_class_names, describedby: @champ.describedby_id } diff --git a/app/models/champ.rb b/app/models/champ.rb index 96ccf834b..dee09b37d 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -87,10 +87,6 @@ class Champ < ApplicationRecord parent_id.present? end - def stable_id_with_row - [row_id, stable_id].compact - end - # used for the `required` html attribute # check visibility to avoid hidden required input # which prevent the form from being sent. @@ -246,6 +242,14 @@ class Champ < ApplicationRecord public? && dossier.champ_forked_with_changes?(self) end + def public_id + if row_id.blank? + stable_id.to_s + else + "#{stable_id}-#{row_id}" + end + end + protected def valid_champ_value? @@ -266,7 +270,7 @@ class Champ < ApplicationRecord end def html_id - row_id.present? ? "champ-#{stable_id}-#{row_id}" : "champ-#{stable_id}" + "champ-#{public_id}" end def needs_dossier_id? diff --git a/app/models/concerns/dossier_clone_concern.rb b/app/models/concerns/dossier_clone_concern.rb index ae56e9bdb..41f02e59a 100644 --- a/app/models/concerns/dossier_clone_concern.rb +++ b/app/models/concerns/dossier_clone_concern.rb @@ -42,12 +42,12 @@ module DossierCloneConcern end def make_diff(editing_fork) - origin_champs_index = champs_for_revision(scope: :public).index_by(&:stable_id_with_row) - forked_champs_index = editing_fork.champs_for_revision(scope: :public).index_by(&:stable_id_with_row) + origin_champs_index = champs_for_revision(scope: :public).index_by(&:public_id) + forked_champs_index = editing_fork.champs_for_revision(scope: :public).index_by(&:public_id) updated_champs_index = editing_fork .champs_for_revision(scope: :public) .filter { _1.updated_at > editing_fork.created_at } - .index_by(&:stable_id_with_row) + .index_by(&:public_id) added = forked_champs_index.keys - origin_champs_index.keys removed = origin_champs_index.keys - forked_champs_index.keys @@ -142,11 +142,11 @@ module DossierCloneConcern end def apply_diff(diff) - champs_index = (champs_for_revision(scope: :public) + diff[:added]).index_by(&:stable_id_with_row) + champs_index = (champs_for_revision(scope: :public) + diff[:added]).index_by(&:public_id) diff[:added].each do |champ| if champ.child? - champ.update_columns(dossier_id: id, parent_id: champs_index.fetch(champ.parent.stable_id_with_row).id) + champ.update_columns(dossier_id: id, parent_id: champs_index.fetch(champ.parent.public_id).id) else champ.update_column(:dossier_id, id) end @@ -154,13 +154,13 @@ module DossierCloneConcern champs_to_remove = [] diff[:updated].each do |champ| - old_champ = champs_index.fetch(champ.stable_id_with_row) + old_champ = champs_index.fetch(champ.public_id) champs_to_remove << old_champ if champ.child? # we need to do that in order to avoid a foreign key constraint old_champ.update(row_id: nil) - champ.update_columns(dossier_id: id, parent_id: champs_index.fetch(champ.parent.stable_id_with_row).id) + champ.update_columns(dossier_id: id, parent_id: champs_index.fetch(champ.parent.public_id).id) else champ.update_column(:dossier_id, id) end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 2937b6490..7b1021708 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -1399,8 +1399,7 @@ class Dossier < ApplicationRecord end def project_champ(type_de_champ, row_id) - stable_id_with_row = [row_id, type_de_champ.stable_id].compact - champ = champs_by_stable_id_with_row[stable_id_with_row] + champ = champs_by_public_id[type_de_champ.public_id(row_id)] if champ.nil? type_de_champ.build_champ(dossier: self, row_id:) else @@ -1409,8 +1408,7 @@ class Dossier < ApplicationRecord end def champ_for_export(type_de_champ, row_id) - stable_id_with_row = [row_id, type_de_champ.stable_id].compact - champ = champs_by_stable_id_with_row[stable_id_with_row] + champ = champs_by_public_id[type_de_champ.public_id(row_id)] if champ.nil? || !champ.visible? # some champs export multiple columns # ex: commune.for_export => [commune, insee, departement] @@ -1423,8 +1421,8 @@ class Dossier < ApplicationRecord private - def champs_by_stable_id_with_row - @champs_by_stable_id_with_row ||= champs.sort_by(&:id).index_by(&:stable_id_with_row) + def champs_by_public_id + @champs_by_public_id ||= champs.sort_by(&:id).index_by(&:public_id) end def create_missing_traitemets diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index e97357272..1c76bbaae 100644 --- a/app/models/type_de_champ.rb +++ b/app/models/type_de_champ.rb @@ -679,6 +679,14 @@ class TypeDeChamp < ApplicationRecord true end + def public_id(row_id) + if row_id.blank? + stable_id.to_s + else + "#{stable_id}-#{row_id}" + end + end + private DEFAULT_EMPTY = [''] diff --git a/config/initializers/action_view_record_identifier.rb b/config/initializers/action_view_record_identifier.rb new file mode 100644 index 000000000..43105f861 --- /dev/null +++ b/config/initializers/action_view_record_identifier.rb @@ -0,0 +1,22 @@ +module ActionView::RecordIdentifier + alias original_dom_class dom_class + alias original_record_key_for_dom_id record_key_for_dom_id + + def dom_class(record_or_class, prefix = nil) + if record_or_class.is_a?(Champ) + prefix ? "#{prefix}#{JOIN}champ" : "champ" + else + original_dom_class(record_or_class, prefix) + end + end + + private + + def record_key_for_dom_id(record) + if record.is_a?(Champ) + record.public_id + else + original_record_key_for_dom_id(record) + end + end +end diff --git a/spec/models/champ_spec.rb b/spec/models/champ_spec.rb index ca7e32bc2..355d75761 100644 --- a/spec/models/champ_spec.rb +++ b/spec/models/champ_spec.rb @@ -597,4 +597,13 @@ describe Champ do it { expect { subject }.to change { champ.reload.data }.to(data) } end + + describe 'dom_id' do + let(:champ) { build(:champ_text, row_id: '1234') } + + it { expect(champ.public_id).to eq("#{champ.stable_id}-#{champ.row_id}") } + it { expect(ActionView::RecordIdentifier.dom_id(champ)).to eq("champ_#{champ.public_id}") } + it { expect(ActionView::RecordIdentifier.dom_id(champ.type_de_champ)).to eq("type_de_champ_#{champ.type_de_champ.id}") } + it { expect(ActionView::RecordIdentifier.dom_class(champ)).to eq("champ") } + end end diff --git a/spec/system/users/en_construction_spec.rb b/spec/system/users/en_construction_spec.rb index 719f69be4..0444caba9 100644 --- a/spec/system/users/en_construction_spec.rb +++ b/spec/system/users/en_construction_spec.rb @@ -31,7 +31,7 @@ describe "Dossier en_construction" do click_on "Supprimer le fichier toto.txt" - input_selector = "#attachment-multiple-empty-#{champ.id}" + input_selector = "#attachment-multiple-empty-#{champ.public_id} " expect(page).to have_selector(input_selector) find(input_selector).attach_file(Rails.root.join('spec/fixtures/files/file.pdf'))