Merge pull request #2556 from betagouv/frederic/fix_1421_linked_menus/admin_validation

Validation de la liste d’options d’un champ double menu déroulant

app/controllers/admin/types_de_champ_controller.rb

@@ -16,9 +16,12 @@ class Admin::TypesDeChampController < AdminController
   end
 
   def update
-    @procedure.update(TypesDeChampService.create_update_procedure_params params)
+    if @procedure.update(TypesDeChampService.create_update_procedure_params params)
+      flash.now.notice = 'Modifications sauvegardées'
+    else
+      flash.now.alert = @procedure.errors.full_messages.join(', ')
+    end
     create_facade
-    flash.now.notice = 'Modifications sauvegardées'
     render 'show', format: :js
   end
 

app/models/type_de_champ.rb

@@ -59,6 +59,17 @@ class TypeDeChamp < ApplicationRecord
   before_validation :check_mandatory
   before_save :remove_piece_justificative_template, if: -> { type_champ_changed? }
 
+  def valid?(context = nil)
+    super
+    if dynamic_type.present?
+      dynamic_type.valid?
+      errors.merge!(dynamic_type.errors)
+    end
+    errors.empty?
+  end
+
+  alias_method :validate, :valid?
+
   def set_dynamic_type
     @dynamic_type = type_champ.present? ? self.class.type_champ_to_class_name(type_champ).constantize.new(self) : nil
   end

app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb

@@ -3,6 +3,8 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
 
   delegate :drop_down_list, to: :@type_de_champ
 
+  validate :check_presence_of_primary_options
+
   def primary_options
     primary_options = unpack_options.map(&:first)
     if primary_options.present?
@@ -21,13 +23,19 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
 
   private
 
+  def check_presence_of_primary_options
+    if !PRIMARY_PATTERN.match?(drop_down_list.options.second)
+      errors.add(libelle, "doit commencer par une entrée de menu primaire de la forme <code style='white-space: pre-wrap;'>--texte--</code>")
+    end
+  end
+
   def unpack_options
     _, *options = drop_down_list.options
     chunked = options.slice_before(PRIMARY_PATTERN)
     chunked.map do |chunk|
       primary, *secondary = chunk
       secondary.unshift('')
-      [PRIMARY_PATTERN.match(primary)[1], secondary]
+      [PRIMARY_PATTERN.match(primary)&.[](1), secondary]
     end
   end
 end

app/models/types_de_champ/type_de_champ_base.rb

@@ -1,4 +1,8 @@
 class TypesDeChamp::TypeDeChampBase
+  include ActiveModel::Validations
+
+  delegate :libelle, to: :@type_de_champ
+
   def initialize(type_de_champ)
     @type_de_champ = type_de_champ
   end

spec/factories/type_de_champ.rb

@@ -54,7 +54,7 @@ FactoryBot.define do
     end
     factory :type_de_champ_linked_drop_down_list do
       type_champ { TypeDeChamp.type_champs.fetch(:linked_drop_down_list) }
-      drop_down_list { create(:drop_down_list) }
+      drop_down_list { create(:drop_down_list, value: "--primary--\nsecondary\n") }
     end
     factory :type_de_champ_pays do
       type_champ { TypeDeChamp.type_champs.fetch(:pays) }

spec/models/type_de_champ_shared_example.rb

@@ -79,5 +79,26 @@ shared_examples 'type_de_champ_spec' do
         end
       end
     end
+
+    context 'delegate validation to dynamic type' do
+      subject { build(:type_de_champ_text) }
+      let(:dynamic_type) do
+        Class.new(TypesDeChamp::TypeDeChampBase) do
+          validate :never_valid
+
+          def never_valid
+            errors.add(:troll, 'always invalid')
+          end
+        end.new(subject)
+      end
+
+      before { subject.instance_variable_set(:@dynamic_type, dynamic_type) }
+
+      it { is_expected.to be_invalid }
+      it do
+        subject.validate
+        expect(subject.errors.full_messages.to_sentence).to eq('Troll always invalid')
+      end
+    end
   end
 end

spec/models/types_de_champ/linked_drop_down_list_type_de_champ_spec.rb

@@ -1,12 +1,78 @@
 require 'spec_helper'
 
 describe TypesDeChamp::LinkedDropDownListTypeDeChamp do
+  let(:drop_down_list) { build(:drop_down_list, value: menu_options) }
+  let(:type_de_champ) { build(:type_de_champ_linked_drop_down_list, drop_down_list: drop_down_list) }
+
+  subject { type_de_champ.dynamic_type }
+
+  describe 'validation' do
+    context 'It must start with one primary option' do
+      context 'valid menu' do
+        let(:menu_options) do
+          <<~END_OPTIONS
+            --Primary 1--
+            secondary 1.1
+            secondary 1.2
+            --Primary 2--
+            secondary 2.1
+            secondary 2.2
+            secondary 2.3
+          END_OPTIONS
+        end
+
+        it { is_expected.to be_valid }
+      end
+
+      context 'degenerate but valid menu' do
+        let(:menu_options) do
+          <<~END_OPTIONS
+            --Primary 1--
+          END_OPTIONS
+        end
+
+        it { is_expected.to be_valid }
+      end
+
+      context 'invalid menus' do
+        shared_examples 'missing primary option' do
+          it { is_expected.to be_invalid }
+          it do
+            subject.validate
+            expect(subject.errors.full_messages).to eq ["#{subject.libelle} doit commencer par une entrée de menu primaire de la forme <code style='white-space: pre-wrap;'>--texte--</code>"]
+          end
+        end
+
+        context 'no primary option' do
+          let(:menu_options) do
+            <<~END_OPTIONS
+              secondary 1.1
+              secondary 1.2
+            END_OPTIONS
+          end
+
+          it_should_behave_like 'missing primary option'
+        end
+
+        context 'starting with secondary options' do
+          let(:menu_options) do
+            <<~END_OPTIONS
+              secondary 1.1
+              secondary 1.2
+              --Primary 2--
+              secondary 2.1
+              secondary 2.2
+              secondary 2.3
+            END_OPTIONS
+          end
+
+          it_should_behave_like 'missing primary option'
+        end
+      end
+    end
+  end
+
   describe '#unpack_options' do
-    let(:drop_down_list) { build(:drop_down_list, value: menu_options) }
-    let(:type_de_champ) { build(:type_de_champ_linked_drop_down_list, drop_down_list: drop_down_list) }
-
-    subject { type_de_champ.dynamic_type }
-
     context 'with no options' do
       let(:menu_options) { '' }
       it { expect(subject.secondary_options).to eq({}) }