Merge pull request #10169 from tchak/refactor-champ-public_id

fix(champ): use public_id in views

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

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

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 }

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?

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

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

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 = ['']

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

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

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'))