tech(refactor): procedure::error_summary and dossier::ErrorsFullMessa…

…gesComponent use same behaviour to compact/expand errors

app/components/dossiers/errors_full_messages_component.rb

@@ -3,17 +3,15 @@
 class Dossiers::ErrorsFullMessagesComponent < ApplicationComponent
   ErrorDescriptor = Data.define(:anchor, :label, :error_message)
 
-  def initialize(dossier:, errors:)
+  def initialize(dossier:)
     @dossier = dossier
-    @errors = errors
   end
 
   def dedup_and_partitioned_errors
-    formated_errors = @errors.to_enum # ActiveModel::Errors.to_a is an alias to full_messages, we don't want that
+    @dossier.errors.to_enum # ActiveModel::Errors.to_a is an alias to full_messages, we don't want that
       .to_a # but enum.to_a gives back an array
       .uniq { |error| [error.inner_error.base] } # dedup cumulated errors from dossier.champs, dossier.champs_public, dossier.champs_private which run the validator one time per association
       .map { |error| to_error_descriptor(error) }
-    yield(Array(formated_errors[0..2]), Array(formated_errors[3..]))
   end
 
   def to_error_descriptor(error)
@@ -27,6 +25,6 @@ def to_error_descriptor(error)
   end
 
   def render?
-    !@errors.empty?
+    !@dossier.errors.empty?
   end
 end

app/components/dossiers/errors_full_messages_component/errors_full_messages_component.en.yml

@@ -5,4 +5,3 @@ en:
       Your file has 1 error. <a href="%{url}">Fix-it</a> to continue :
     other: |
       Your file has %{count} errors. <a href="%{url}">Fix-them</a> to continue :
-  see_more: Show all errors

app/components/dossiers/errors_full_messages_component/errors_full_messages_component.fr.yml

@@ -5,4 +5,3 @@ fr:
       Votre dossier contient 1 champ en erreur. <a href="%{url}">Corrigez-la</a> pour poursuivre :
     other: |
       Votre dossier contient  %{count} champs en erreurs. <a href="%{url}">Corrigez-les</a> pour poursuivre :
-  see_more: Afficher toutes les erreurs

app/components/dossiers/errors_full_messages_component/errors_full_messages_component.html.haml

@@ -1,15 +1,4 @@
 .fr-alert.fr-alert--error.fr-mb-3w{ role: "alertdialog" }
-  - dedup_and_partitioned_errors do |head, tail|
-    %p#sumup-errors= t('.sumup_html', count: head.size + tail.size, url: head.first.anchor)
-    %ul.fr-mb-0#head-errors
-      - head.each do |error_descriptor|
-        %li
-          = link_to error_descriptor.label, error_descriptor.anchor, class: 'error-anchor'
-          = error_descriptor.error_message
-    - if tail.size > 0
-      %button{ type: "button", "aria-controls": 'tail-errors', "aria-expanded": "false", class: "fr-btn fr-btn--sm fr-btn--tertiary-no-outline" }= t('.see_more')
-      %ul#tail-errors.fr-collapse.fr-mt-0
-        - tail.each do |error_descriptor|
-          %li
-            = link_to error_descriptor.label, error_descriptor.anchor, class: 'error-anchor'
-            = "(#{error_descriptor.error_message})"
+  - if dedup_and_partitioned_errors.size > 0
+    %p#sumup-errors= t('.sumup_html', count: dedup_and_partitioned_errors.size, url: dedup_and_partitioned_errors.first.anchor)
+    = render ExpandableErrorList.new(errors: dedup_and_partitioned_errors)

app/components/expandable_error_list.rb

@@ -0,0 +1,9 @@
+class ExpandableErrorList < ApplicationComponent
+  def initialize(errors:)
+    @errors = errors
+  end
+
+  def splitted_errors
+    yield(Array(@errors[0..2]), Array(@errors[3..]))
+  end
+end

app/components/expandable_error_list/expandable_error_list.html.en.yml

@@ -0,0 +1,3 @@
+---
+en:
+  see_more: Show all errors

app/components/expandable_error_list/expandable_error_list.html.fr.yml

@@ -0,0 +1,3 @@
+---
+fr:
+  see_more: Afficher toutes les erreurs

app/components/expandable_error_list/expandable_error_list.html.haml

@@ -0,0 +1,14 @@
+- splitted_errors do |head, tail|
+  %ul#head-errors.fr-mb-0
+    - head.each do |error_descriptor|
+      %li
+        = link_to error_descriptor.label, error_descriptor.anchor, class: 'error-anchor'
+        = error_descriptor.error_message
+
+  - if tail.size > 0
+    %button.fr-mt-0.fr-btn.fr-btn--sm.fr-btn--tertiary-no-outline{ type: "button", "aria-controls": 'tail-errors', "aria-expanded": "false", class: "" }= t('see_more')
+    %ul#tail-errors.fr-collapse.fr-mt-0
+      - tail.each do |error_descriptor|
+        %li
+          = link_to error_descriptor.label, error_descriptor.anchor, class: 'error-anchor'
+          = error_descriptor.error_message

app/components/procedure/errors_summary.rb

@@ -1,4 +1,6 @@
 class Procedure::ErrorsSummary < ApplicationComponent
+  ErrorDescriptor = Data.define(:anchor, :label, :error_message)
+
   def initialize(procedure:, validation_context:)
     @procedure = procedure
     @validation_context = validation_context
@@ -24,10 +26,8 @@ def invalid?
     @procedure.errors.present?
   end
 
-  def error_messages
-    @procedure.errors.map do |error|
-      [error, error_correction_page(error)]
-    end
+  def errors
+    @procedure.errors.map { to_error_descriptor(_1) }
   end
 
   def error_correction_page(error)
@@ -45,4 +45,14 @@ def error_correction_page(error)
       edit_admin_procedure_mail_template_path(@procedure, klass.const_get(:SLUG))
     end
   end
+
+  def to_error_descriptor(error)
+    libelle = case error.attribute
+    when :draft_types_de_champ_public, :draft_types_de_champ_private
+      error.options[:type_de_champ].libelle.truncate(200)
+    else
+      error.base.class.human_attribute_name(error.attribute)
+    end
+    ErrorDescriptor.new(error_correction_page(error), libelle, error.message)
+  end
 end

app/components/procedure/errors_summary/errors_summary.html.haml

@@ -2,8 +2,4 @@
   - if invalid?
     = render Dsfr::AlertComponent.new(state: :error, title: , extra_class_names: 'fr-mb-2w') do |c|
       - c.with_body do
-        - error_messages.each do |(error, path)|
-          %p.mt-2
-            = error.full_message
-            - if path.present?
-              = "(#{link_to 'corriger', path, class: 'fr-link'})"
+        = render ExpandableErrorList.new(errors:)

app/views/shared/dossiers/_edit.html.haml

@@ -10,7 +10,7 @@
   = render NestedForms::FormOwnerComponent.new
   = form_for dossier_for_editing, url: brouillon_dossier_url(dossier), method: :patch, html: { id: 'dossier-edit-form', class: 'form', multipart: true, novalidate: 'novalidate' } do |f|
 
-    = render Dossiers::ErrorsFullMessagesComponent.new(dossier: @dossier, errors: @errors || [])
+    = render Dossiers::ErrorsFullMessagesComponent.new(dossier: dossier)
     %header.mb-6
       .fr-highlight
         %p.fr-text--sm

config/locales/models/procedure/en.yml

@@ -72,16 +72,16 @@ en:
               invalid: 'invalid format'
             draft_types_de_champ_public:
               format: 'Public field %{message}'
-              invalid_condition: "« %{value} » have an invalid logic"
-              empty_repetition: '« %{value} » requires at least one field'
-              empty_drop_down: '« %{value} » requires at least one option'
-              inconsistent_header_section: "« %{value} » %{custom_message}"
+              invalid_condition: "have an invalid logic"
+              empty_repetition: 'requires at least one field'
+              empty_drop_down: 'requires at least one option'
+              inconsistent_header_section: "%{custom_message}"
             draft_types_de_champ_private:
               format: 'Private field %{message}'
-              invalid_condition: "« %{value} » have an invalid logic"
-              empty_repetition: '« %{value} » requires at least one field'
-              empty_drop_down: '« %{value} » requires at least one option'
-              inconsistent_header_section: "« %{value} » %{custom_message}"
+              invalid_condition: "have an invalid logic"
+              empty_repetition: 'requires at least one field'
+              empty_drop_down: 'requires at least one option'
+              inconsistent_header_section: "%{custom_message}"
             attestation_template:
               format: "%{attribute} %{message}"
             initiated_mail:

config/locales/models/procedure/fr.yml

@@ -78,16 +78,16 @@ fr:
               invalid: 'n’a pas le bon format'
             draft_types_de_champ_public:
               format: 'Le champ %{message}'
-              invalid_condition: "« %{value} » a une logique conditionnelle invalide"
-              empty_repetition: '« %{value} » doit comporter au moins un champ répétable'
-              empty_drop_down: '« %{value} » doit comporter au moins un choix sélectionnable'
-              inconsistent_header_section: "« %{value} » %{custom_message}"
+              invalid_condition: "a une logique conditionnelle invalide"
+              empty_repetition: 'doit comporter au moins un champ répétable'
+              empty_drop_down: 'doit comporter au moins un choix sélectionnable'
+              inconsistent_header_section: "%{custom_message}"
             draft_types_de_champ_private:
               format: 'L’annotation privée %{message}'
-              invalid_condition: "« %{value} » a une logique conditionnelle invalide"
-              empty_repetition: '« %{value} » doit comporter au moins un champ répétable'
-              empty_drop_down: '« %{value} » doit comporter au moins un choix sélectionnable'
-              inconsistent_header_section: "« %{value} » %{custom_message}"
+              invalid_condition: "a une logique conditionnelle invalide"
+              empty_repetition: 'doit comporter au moins un champ répétable'
+              empty_drop_down: 'doit comporter au moins un choix sélectionnable'
+              inconsistent_header_section: "%{custom_message}"
             attestation_template:
               format: "%{attribute} %{message}"
             initiated_mail:

spec/components/procedures/errors_summary_spec.rb

@@ -11,27 +11,33 @@
     context 'when :publication' do
       let(:validation_context) { :publication }
 
-      it 'shows errors for public and private tdc' do
-        expect(page).to have_text("Le champ « public » doit comporter au moins un choix sélectionnable")
-        expect(page).to have_text("L’annotation privée « private » doit comporter au moins un choix sélectionnable")
+      it 'shows errors and links for public and private tdc' do
+        expect(page).to have_content("Erreur : Des problèmes empêchent la publication de la démarche")
+        expect(page).to have_selector("a", text: "public")
+        expect(page).to have_selector("a", text: "private")
+        expect(page).to have_text("doit comporter au moins un choix sélectionnable", count: 2)
       end
     end
 
     context 'when :types_de_champ_public_editor' do
       let(:validation_context) { :types_de_champ_public_editor }
 
-      it 'shows errors for public only tdc' do
-        expect(page).to have_text("Le champ « public » doit comporter au moins un choix sélectionnable")
-        expect(page).not_to have_text("L’annotation privée « private » doit comporter au moins un choix sélectionnable")
+      it 'shows errors and links for public only tdc' do
+        expect(page).to have_text("Erreur : Les champs formulaire contiennent des erreurs")
+        expect(page).to have_selector("a", text: "public")
+        expect(page).to have_text("doit comporter au moins un choix sélectionnable", count: 1)
+        expect(page).not_to have_selector("a", text: "private")
       end
     end
 
     context 'when :types_de_champ_private_editor' do
       let(:validation_context) { :types_de_champ_private_editor }
 
-      it 'shows errors for private only tdc' do
-        expect(page).not_to have_text("Le champ « public » doit comporter au moins un choix sélectionnable")
-        expect(page).to have_text("L’annotation privée « private » doit comporter au moins un choix sélectionnable")
+      it 'shows errors and links for private only tdc' do
+        expect(page).to have_text("Erreur : Les annotations privées contiennent des erreurs")
+        expect(page).to have_selector("a", text: "private")
+        expect(page).to have_text("doit comporter au moins un choix sélectionnable")
+        expect(page).not_to have_selector("a", text: "public")
       end
     end
   end
@@ -52,12 +58,18 @@
 
     before { subject }
 
-    it 'renders all errors on champ' do
-      expect(page).to have_text("Le champ « drop down list requires options » doit comporter au moins un choix sélectionnable")
-      expect(page).to have_text("Le champ « repetition requires children » doit comporter au moins un champ répétable")
-      expect(page).to have_text("Le champ « invalid condition » a une logique conditionnelle invalide")
-      expect(page).to have_text("Le champ « header sections must have consistent order » devrait être précédé d'un titre de niveau 1")
-      # TODO, test attestation_template, initiated_mail, :received_mail, :closed_mail, :refused_mail, :without_continuation_mail, :re_instructed_mail
+    it 'renders all errors  and links on champ' do
+      expect(page).to have_selector("a", text: "drop down list requires options")
+      expect(page).to have_content("doit comporter au moins un choix sélectionnable")
+
+      expect(page).to have_selector("a", text: "repetition requires children")
+      expect(page).to have_content("doit comporter au moins un champ répétable")
+
+      expect(page).to have_selector("a", text: "invalid condition")
+      expect(page).to have_content("a une logique conditionnelle invalide")
+
+      expect(page).to have_selector("a", text: "header sections must have consistent order")
+      expect(page).to have_content("devrait être précédé d'un titre de niveau 1")
     end
   end
 
@@ -73,8 +85,9 @@
     end
 
     it 'render error nicely' do
-      expect(page).to have_text("Le modèle d’attestation n'est pas valide")
-      expect(page).to have_text("L’email de notification de passage de dossier en instruction n'est pas valide")
+      expect(page).to have_selector("a", text: "Le modèle d’attestation")
+      expect(page).to have_selector("a", text: "L’email de notification de passage de dossier en instruction")
+      expect(page).to have_text("n'est pas valide", count: 2)
     end
   end
 end

spec/models/procedure_spec.rb

@@ -372,30 +372,30 @@
           ]
         end
         let(:types_de_champ_private) { [] }
-        let(:invalid_repetition_error_message) { 'Le champ « Enfants » doit comporter au moins un champ répétable' }
-        let(:invalid_drop_down_error_message) { 'Le champ « Civilité » doit comporter au moins un choix sélectionnable' }
+        let(:invalid_repetition_error_message) { "doit comporter au moins un champ répétable" }
+        let(:invalid_drop_down_error_message) { "doit comporter au moins un choix sélectionnable" }
 
         it 'validates that no repetition type de champ is empty' do
           procedure.validate(:publication)
-          expect(procedure.errors.full_messages_for(:draft_types_de_champ_public)).to include(invalid_repetition_error_message)
+          expect(procedure.errors.messages_for(:draft_types_de_champ_public)).to include(invalid_repetition_error_message)
 
           new_draft = procedure.draft_revision
           repetition = procedure.draft_revision.types_de_champ_public.find(&:repetition?)
           parent_coordinate = new_draft.revision_types_de_champ.find_by(type_de_champ: repetition)
           new_draft.revision_types_de_champ.create(type_de_champ: create(:type_de_champ), position: 0, parent: parent_coordinate)
 
           procedure.validate(:publication)
-          expect(procedure.errors.full_messages_for(:draft_types_de_champ_public)).not_to include(invalid_repetition_error_message)
+          expect(procedure.errors.messages_for(:draft_types_de_champ_public)).not_to include(invalid_repetition_error_message)
         end
 
         it 'validates that no drop-down type de champ is empty' do
           procedure.validate(:publication)
-          expect(procedure.errors.full_messages_for(:draft_types_de_champ_public)).to include(invalid_drop_down_error_message)
+          expect(procedure.errors.messages_for(:draft_types_de_champ_public)).to include(invalid_drop_down_error_message)
 
           drop_down = procedure.draft_revision.types_de_champ_public.find(&:drop_down_list?)
           drop_down.update!(drop_down_list_value: "--title--\r\nsome value")
           procedure.reload.validate(:publication)
-          expect(procedure.errors.full_messages_for(:draft_types_de_champ_public)).not_to include(invalid_drop_down_error_message)
+          expect(procedure.errors.messages_for(:draft_types_de_champ_public)).not_to include(invalid_drop_down_error_message)
         end
       end
 
@@ -408,17 +408,21 @@
         end
         let(:types_de_champ_public) { [] }
 
-        let(:invalid_repetition_error_message) { 'L’annotation privée « Enfants » doit comporter au moins un champ répétable' }
-        let(:invalid_drop_down_error_message) { 'L’annotation privée « Civilité » doit comporter au moins un choix sélectionnable' }
+        let(:invalid_repetition_error_message) { "doit comporter au moins un champ répétable" }
+        let(:invalid_drop_down_error_message) { "doit comporter au moins un choix sélectionnable" }
 
         it 'validates that no repetition type de champ is empty' do
           procedure.validate(:publication)
-          expect(procedure.errors.full_messages_for(:draft_types_de_champ_private)).to include(invalid_repetition_error_message)
+          expect(procedure.errors.messages_for(:draft_types_de_champ_private)).to include(invalid_repetition_error_message)
+          repetition = procedure.draft_revision.types_de_champ_private.find(&:repetition?)
+          expect(procedure.errors.to_enum.to_a.map { _1.options[:type_de_champ] }).to include(repetition)
         end
 
         it 'validates that no drop-down type de champ is empty' do
           procedure.validate(:publication)
-          expect(procedure.errors.full_messages_for(:draft_types_de_champ_private)).to include(invalid_drop_down_error_message)
+          expect(procedure.errors.messages_for(:draft_types_de_champ_private)).to include(invalid_drop_down_error_message)
+          drop_down = procedure.draft_revision.types_de_champ_private.find(&:drop_down_list?)
+          expect(procedure.errors.to_enum.to_a.map { _1.options[:type_de_champ] }).to include(drop_down)
         end
       end
 
@@ -441,7 +445,7 @@
         include Logic
         let(:types_de_champ_public) { [{ type: :text, libelle: 'condition', condition: ds_eq(champ_value(1), constant(2)), stable_id: 2 }] }
         let(:types_de_champ_private) { [{ type: :decimal_number, stable_id: 1 }] }
-        let(:error_on_condition) { "Le champ « condition » a une logique conditionnelle invalide" }
+        let(:error_on_condition) { "Le champ a une logique conditionnelle invalide" }
 
         it 'validate without context' do
           procedure.validate

spec/system/administrateurs/procedure_publish_spec.rb

@@ -72,8 +72,8 @@
         visit admin_procedure_path(procedure)
 
         expect(page).to have_content('Des problèmes empêchent la publication de la démarche')
-        expect(page).to have_content("« Enfants » doit comporter au moins un champ répétable")
-        expect(page).to have_content("« Civilité » doit comporter au moins un choix sélectionnable")
+        expect(page).to have_content("Enfants doit comporter au moins un champ répétable")
+        expect(page).to have_content("Civilité doit comporter au moins un choix sélectionnable")
 
         visit admin_procedure_publication_path(procedure)
         expect(find_field('procedure_path').value).to eq procedure.path
@@ -195,7 +195,7 @@
     scenario 'an error message prevents the publication' do
       visit admin_procedure_path(procedure)
       expect(page).to have_content('Des problèmes empêchent la publication des modifications')
-      expect(page).to have_link('corriger', href: edit_admin_procedure_mail_template_path(procedure, Mails::InitiatedMail::SLUG))
+      expect(page).to have_link(href: edit_admin_procedure_mail_template_path(procedure, Mails::InitiatedMail::SLUG))
       expect(page).to have_button('Publier les modifications', disabled: true)
     end
   end