Merge pull request #7550 from mfo/US/fix-additional-empty-option

bug(linked_dropdown): when mandatory, add an extra blank option

app/javascript/new_design/champs/linked-drop-down-list.js

@@ -13,15 +13,19 @@ delegate('change', PRIMARY_SELECTOR, (evt) => {
 
   selectOptions(secondary, options[primary.value]);
 });
-
+function makeOption(option) {
+  let element = document.createElement('option');
+  element.textContent = option;
+  element.value = option;
+  return element;
+}
 function selectOptions(selectElement, options) {
   selectElement.innerHTML = '';
-
+  if (selectElement.required) {
+    selectElement.appendChild(makeOption(''));
+  }
   for (let option of options) {
-    let element = document.createElement('option');
-    element.textContent = option;
-    element.value = option;
-    selectElement.appendChild(element);
+    selectElement.appendChild(makeOption(option));
   }
 
   selectElement.selectedIndex = 0;

app/models/champs/drop_down_list_champ.rb

@@ -22,6 +22,7 @@
 class Champs::DropDownListChamp < Champ
   THRESHOLD_NB_OPTIONS_AS_RADIO = 5
   OTHER = '__other__'
+  delegate :options_without_empty_value_when_mandatory, to: :type_de_champ
 
   def render_as_radios?
     enabled_non_empty_options.size <= THRESHOLD_NB_OPTIONS_AS_RADIO

app/models/type_de_champ.rb

@@ -297,6 +297,14 @@ class TypeDeChamp < ApplicationRecord
     self.drop_down_options = parse_drop_down_list_value(value)
   end
 
+  # historicaly we added a blank ("") option by default to avoid wrong selection
+  #   see self.parse_drop_down_list_value
+  #   then rails decided to add this blank ("") option when the select is required
+  #   so we revert this change
+  def options_without_empty_value_when_mandatory(options)
+    mandatory? ? options.reject(&:blank?) : options
+  end
+
   def drop_down_list_options?
     drop_down_list_options.any?
   end
@@ -354,10 +362,11 @@ class TypeDeChamp < ApplicationRecord
 
   private
 
+  DEFAULT_EMPTY = ['']
   def parse_drop_down_list_value(value)
     value = value ? value.split("\r\n").map(&:strip).join("\r\n") : ''
     result = value.split(/[\r\n]|[\r]|[\n]|[\n\r]/).reject(&:empty?)
-    result.blank? ? [] : [''] + result
+    result.blank? ? [] : DEFAULT_EMPTY + result
   end
 
   def populate_stable_id

app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb

@@ -30,10 +30,15 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
     tags
   end
 
+  def add_blank_option_when_not_mandatory(options)
+    return options if mandatory
+    options.unshift('')
+  end
+
   def primary_options
     primary_options = unpack_options.map(&:first)
     if primary_options.present?
-      primary_options.unshift('')
+      primary_options = add_blank_option_when_not_mandatory(primary_options)
     end
     primary_options
   end
@@ -53,7 +58,7 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
     chunked = options.slice_before(PRIMARY_PATTERN)
     chunked.map do |chunk|
       primary, *secondary = chunk
-      secondary.unshift('')
+      secondary = add_blank_option_when_not_mandatory(secondary)
       [PRIMARY_PATTERN.match(primary)&.[](1), secondary]
     end
   end

app/views/shared/dossiers/editable_champs/_drop_down_list.html.haml

@@ -16,7 +16,7 @@
           = form.radio_button :value, Champs::DropDownListChamp::OTHER, checked: champ.other_value_present?
           Autre
   - else
-    = form.select :value, champ.options, { selected: champ.selected}, required: champ.mandatory?, id: champ.input_id, aria: { describedby: champ.describedby_id }
+    = form.select :value, champ.options_without_empty_value_when_mandatory(champ.options), { selected: champ.selected }, required: champ.mandatory?, id: champ.input_id, aria: { describedby: champ.describedby_id }
 
   - if champ.drop_down_other?
     = render partial: "shared/dossiers/editable_champs/drop_down_other_input", locals: { form: form, champ: champ }

spec/system/users/linked_dropdown_spec.rb

@@ -13,30 +13,56 @@ describe 'linked dropdown lists' do
       Secondary 2.3
     END_OF_LIST
   end
-  let(:type_de_champ) { build(:type_de_champ_linked_drop_down_list, libelle: 'linked dropdown', drop_down_list_value: list_items) }
 
   let!(:procedure) do
     create(:procedure, :published, :for_individual, types_de_champ: [type_de_champ])
   end
+  let(:type_de_champ) { build(:type_de_champ_linked_drop_down_list, libelle: 'linked dropdown', drop_down_list_value: list_items, mandatory: mandatory) }
 
   let(:user_dossier) { user.dossiers.first }
+  context 'not mandatory' do
+    let(:mandatory) { false }
+    scenario 'change primary value, secondary options are updated', js: true do
+      log_in(user.email, password, procedure)
 
-  scenario 'change primary value, secondary options are updated', js: true do
-    log_in(user.email, password, procedure)
+      fill_individual
+      expect(page).to have_select("linked dropdown", options: ['', 'Primary 1', 'Primary 2'])
 
-    fill_individual
+      # Select a primary value
+      select('Primary 2', from: 'linked dropdown')
 
-    # Select a primary value
-    select('Primary 2', from: 'linked dropdown')
+      # Secondary menu reflects chosen primary value
+      expect(page).to have_select("Valeur secondaire dépendant de la première", options: ['', 'Secondary 2.1', 'Secondary 2.2', 'Secondary 2.3'])
 
-    # Secondary menu reflects chosen primary value
-    expect(page).to have_select("Valeur secondaire dépendant de la première", options: ['', 'Secondary 2.1', 'Secondary 2.2', 'Secondary 2.3'])
+      # Select another primary value
+      select('Primary 1', from: 'linked dropdown')
 
-    # Select another primary value
-    select('Primary 1', from: 'linked dropdown')
+      # Secondary menu gets updated
+      expect(page).to have_select("Valeur secondaire dépendant de la première", options: ['', 'Secondary 1.1', 'Secondary 1.2'])
+    end
+  end
 
-    # Secondary menu gets updated
-    expect(page).to have_select("Valeur secondaire dépendant de la première", options: ['', 'Secondary 1.1', 'Secondary 1.2'])
+  context 'mandatory' do
+    let(:mandatory) { true }
+
+    scenario 'change primary value, secondary options are updated', js: true do
+      log_in(user.email, password, procedure)
+
+      fill_individual
+      expect(page).to have_select("linked dropdown", options: ['', 'Primary 1', 'Primary 2'])
+
+      # Select a primary value
+      select('Primary 2', from: 'linked dropdown')
+
+      # Secondary menu reflects chosen primary value
+      expect(page).to have_select("Valeur secondaire dépendant de la première", options: ['', 'Secondary 2.1', 'Secondary 2.2', 'Secondary 2.3'])
+
+      # Select another primary value
+      select('Primary 1', from: 'linked dropdown')
+
+      # Secondary menu gets updated
+      expect(page).to have_select("Valeur secondaire dépendant de la première", options: ['', 'Secondary 1.1', 'Secondary 1.2'])
+    end
   end
 
   private