Merge pull request #10365 from tchak/refactor-dossier-champ-by-stable-id-attribute-keys

refactor(champs): do not depend on attributes hash key in old code

app/controllers/champs/options_controller.rb

@@ -4,7 +4,7 @@ class Champs::OptionsController < Champs::ChampController
   def remove
     @champ.remove_option([params[:option]].compact, true)
     @dossier = @champ.private? ? nil : @champ.dossier
-    champs_attributes = params[:champ_id].present? ? { @champ.id => true } : { @champ.public_id => { with_public_id: true } }
+    champs_attributes = { @champ.public_id => params[:champ_id].present? ? { id: @champ.id } : { with_public_id: true } }
     @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_attributes, @champ.dossier.champs)
   end
 end

app/controllers/concerns/turbo_champs_concern.rb

@@ -5,7 +5,7 @@ module TurboChampsConcern
 
   def champs_to_turbo_update(params, champs)
     to_update = if params.values.filter { _1.key?(:with_public_id) }.empty?
-                  champ_ids = params.keys.map(&:to_i)
+                  champ_ids = params.values.map { _1[:id] }.compact.map(&:to_i)
                   champs.filter { _1.id.in?(champ_ids) }
                 else
                   champ_public_ids = params.keys

app/models/champ.rb

@@ -177,13 +177,13 @@ class Champ < ApplicationRecord
   # However the field index makes it difficult to render a single field, independent from the ordering of the others.
   #
   # Luckily, this is only used to make the name unique, but the actual value is ignored when Rails parses nested
-  # attributes. So instead of the field index, this method uses the champ id; which gives us an independent and
+  # attributes. So instead of the field index, this method uses the champ public_id; which gives us an independent and
   # predictable input name.
   def input_name
     if private?
-      "dossier[champs_private_attributes][#{id}]"
+      "dossier[champs_private_attributes][#{public_id}]"
     else
-      "dossier[champs_public_attributes][#{id}]"
+      "dossier[champs_public_attributes][#{public_id}]"
     end
   end
 

spec/controllers/champs/options_controller_spec.rb

@@ -0,0 +1,30 @@
+describe Champs::OptionsController, type: :controller do
+  let(:user) { create(:user) }
+  let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :multiple_drop_down_list }]) }
+
+  describe '#remove' do
+    let(:dossier) { create(:dossier, user:, procedure:) }
+    let(:champ) { dossier.champs.first }
+
+    before {
+      sign_in user
+      champ.update(value: ['toto', 'tata'].to_json)
+    }
+
+    context 'with stable_id' do
+      subject { delete :remove, params: { dossier_id: dossier, stable_id: champ.stable_id, option: 'tata' }, format: :turbo_stream }
+
+      it 'remove option' do
+        expect { subject }.to change { champ.reload.selected_options.size }.from(2).to(1)
+      end
+    end
+
+    context 'with champ_id' do
+      subject { delete :remove, params: { champ_id: champ.id, option: 'tata' }, format: :turbo_stream }
+
+      it 'remove option' do
+        expect { subject }.to change { champ.reload.selected_options.size }.from(2).to(1)
+      end
+    end
+  end
+end

spec/models/champ_spec.rb

@@ -537,21 +537,21 @@ describe Champ do
 
   describe "#input_name" do
     let(:champ) { create(:champ_text) }
-    it { expect(champ.input_name).to eq "dossier[champs_public_attributes][#{champ.id}]" }
+    it { expect(champ.input_name).to eq "dossier[champs_public_attributes][#{champ.public_id}]" }
 
     context "when private" do
       let(:champ) { create(:champ_text, private: true) }
-      it { expect(champ.input_name).to eq "dossier[champs_private_attributes][#{champ.id}]" }
+      it { expect(champ.input_name).to eq "dossier[champs_private_attributes][#{champ.public_id}]" }
     end
 
     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.id}]" }
+      it { expect(champ.input_name).to eq "dossier[champs_public_attributes][#{champ.public_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.id}]" }
+      it { expect(champ.input_name).to eq "dossier[champs_private_attributes][#{champ.public_id}]" }
     end
   end