Merge pull request #8987 from mfo/US/link-to-errored-champs

amelioration(dossier#submit_brouillon): ETQ usager, je souhaite pouvoir acceder aux champs en erreur facilement

app/controllers/users/dossiers_controller.rb

@@ -469,13 +469,12 @@ module Users
 
     def update_dossier_and_compute_errors
       errors = []
-
       @dossier.assign_attributes(champs_public_params)
       if @dossier.champs_public_all.any?(&:changed_for_autosave?)
         @dossier.last_champ_updated_at = Time.zone.now
       end
       if !@dossier.save(**validation_options)
-        errors += @dossier.errors.full_messages
+        errors += format_errors(errors: @dossier.errors)
       end
 
       if should_change_groupe_instructeur?
@@ -487,7 +486,7 @@ module Users
       end
 
       if dossier.en_construction?
-        errors += @dossier.check_mandatory_and_visible_champs
+        errors += format_errors(errors: @dossier.check_mandatory_and_visible_champs)
       end
 
       errors
@@ -497,20 +496,43 @@ module Users
       errors = []
 
       @dossier.valid?(**submit_validation_options)
-      errors += @dossier.errors.full_messages
+      errors += format_errors(errors: @dossier.errors)
-      errors += @dossier.check_mandatory_and_visible_champs
+      errors += format_errors(errors: @dossier.check_mandatory_and_visible_champs)
 
       if should_fill_groupe_instructeur?
         @dossier.assign_to_groupe_instructeur(defaut_groupe_instructeur)
       end
 
       if @dossier.groupe_instructeur.nil?
-        errors << "Le champ « #{@dossier.procedure.routing_criteria_name} » doit être rempli"
+        errors += format_errors(errors: ["Le champ « #{@dossier.procedure.routing_criteria_name} » doit être rempli"])
       end
 
       errors
     end
 
+    def format_errors(errors:)
+      errors.map do |active_model_error|
+        case active_model_error.class.name
+        when "ActiveModel::NestedError"
+          append_anchor_link(active_model_error.full_message, active_model_error.inner_error.base)
+        when "ActiveModel::Error"
+          append_anchor_link(active_model_error.full_message, active_model_error.base)
+        else # "String"
+          active_model_error
+        end
+      end
+    end
+
+    def append_anchor_link(str_error, model)
+      return str_error.full_message if !model.is_a?(Champ)
+      [
+        "Le champ « #{model.libelle.truncate(200)} » #{str_error}",
+        helpers.link_to(t('views.users.dossiers.fix_champ'), brouillon_dossier_path(anchor: model.input_id))
+      ].join(", ")
+    rescue # case of invalid type de champ on champ
+      str_error
+    end
+
     def ensure_ownership!
       if !current_user.owns?(dossier)
         forbidden!

app/models/dossier.rb

@@ -1068,7 +1068,7 @@ class Dossier < ApplicationRecord
       .filter(&:visible?)
       .filter(&:mandatory_blank?)
       .map do |champ|
-        "Le champ #{champ.libelle.truncate(200)} doit être rempli."
+        champ.errors.add(:value, message: champ.errors.generate_message(:value, :missing))
       end
   end
 

app/validators/iban_validator.rb

@@ -4,7 +4,7 @@ class IbanValidator < ActiveModel::Validator
   def validate(record)
     if record.value.present?
       unless IBANTools::IBAN.valid?(record.value)
-        record.errors.add :iban, record.errors.generate_message(:value, :invalid)
+        record.errors.add :value, message: record.errors.generate_message(:value, :invalid_iban)
       end
     end
   end

config/locales/en.yml

@@ -389,6 +389,7 @@ en:
           empty: "No file"
           detail_one: "To complete a procedure, contact your administration and ask for the link to the procedure."
           detail_two: "This one should look like"
+        fix_champ: "fill in this field"
         archived_dossier: "Your file will be kept %{duree_conservation_dossiers_dans_ds} more months"
         identite:
           identity_data: Identity data
@@ -595,6 +596,11 @@ en:
               taken: is already used for procedure. You cannot use it because it belongs to another administrator.
               taken_can_be_claimed: Is the same as another of your procedure. If you publish this procedure, the other one will be unpublished
               invalid: is not valid. It must countain between 3 and 50 characters among a-z, 0-9, '_' and '-'.
+        "dossier/champs":
+          format: "%{message}"
+          attributes:
+            value:
+              format: "%{message}"
         "champs/cnaf_champ":
           attributes:
             numero_allocataire:

config/locales/fr.yml

@@ -390,6 +390,7 @@ fr:
           empty: "Aucun dossier"
           detail_one: "Pour remplir une démarche, contactez votre administration en lui demandant le lien de la démarche."
           detail_two: "Celui ci doit ressembler à"
+        fix_champ: "corriger l'erreur"
         archived_dossier: "Votre dossier sera conservé %{duree_conservation_dossiers_dans_ds} mois supplémentaire"
         identite:
           identity_data: Données d’identité
@@ -595,6 +596,12 @@ fr:
               taken: est déjà utilisé par une démarche. Vous ne pouvez pas l’utiliser car il appartient à un autre administrateur.
               taken_can_be_claimed: est identique à celui d’une autre de vos démarches publiées. Si vous publiez cette démarche, l’ancienne sera dépubliée et ne sera plus accessible au public. Les utilisateurs qui ont commencé un brouillon vont pouvoir le déposer.
               invalid: n’est pas valide. Il doit comporter au moins 3 caractères, au plus 50 caractères et seuls les caractères a-z, 0-9, '_' et '-' sont autorisés.
+        "dossier/champs":
+          format: "%{message}"
+          attributes:
+            value:
+              format: "%{message}"
+
         "champs/cnaf_champ":
           attributes:
             numero_allocataire:

config/locales/models/champs/champs.en.yml

@@ -0,0 +1,10 @@
+fr:
+  activerecord:
+    errors:
+      models:
+        champ:
+          attributes:
+            value:
+              format: '%{message}'
+              missing: must be filled
+

config/locales/models/champs/champs.fr.yml

@@ -0,0 +1,10 @@
+fr:
+  activerecord:
+    errors:
+      models:
+        champ:
+          attributes:
+            value:
+              format: '%{message}'
+              missing: doit être rempli
+

config/locales/models/champs/iban_champ/en.yml

@@ -0,0 +1,9 @@
+en:
+  activerecord:
+    errors:
+      models:
+        champs/iban_champ:
+          attributes:
+            value:
+              invalid_iban: is not a valid IBAN
+

config/locales/models/champs/iban_champ/fr.yml

@@ -0,0 +1,9 @@
+fr:
+  activerecord:
+    errors:
+      models:
+        champs/iban_champ:
+          attributes:
+            value:
+              invalid_iban: n'est pas au format IBAN
+

spec/controllers/users/dossiers_controller_spec.rb

@@ -345,6 +345,7 @@ describe Users::DossiersController, type: :controller do
 
     let!(:dossier) { create(:dossier, user: user) }
     let(:first_champ) { dossier.champs_public.first }
+    let(:anchor_to_first_champ) { controller.helpers.link_to I18n.t('views.users.dossiers.fix_champ'), brouillon_dossier_path(anchor: first_champ.input_id) }
     let(:value) { 'beautiful value' }
     let(:now) { Time.zone.parse('01/01/2100') }
     let(:payload) { { id: dossier.id } }
@@ -383,14 +384,14 @@ describe Users::DossiersController, type: :controller do
     context 'when the update fails' do
       before do
         expect_any_instance_of(Dossier).to receive(:valid?).and_return(false)
-        expect_any_instance_of(Dossier).to receive(:errors)
+        expect_any_instance_of(Dossier).to receive(:errors).and_return(
-          .and_return(double('errors', full_messages: ['nop']))
+          [double(class: ActiveModel::Error, full_message: 'nop', base: first_champ)]
-
+        )
         subject
       end
 
       it { expect(response).to render_template(:brouillon) }
-      it { expect(flash.alert).to eq(['nop']) }
+      it { expect(flash.alert).to eq(["Le champ « #{first_champ.libelle} » nop, #{anchor_to_first_champ}"]) }
 
       it 'does not send an email' do
         expect(NotificationMailer).not_to receive(:send_en_construction_notification)
@@ -408,7 +409,17 @@ describe Users::DossiersController, type: :controller do
       end
 
       it { expect(response).to render_template(:brouillon) }
-      it { expect(flash.alert).to eq(['Le champ l doit être rempli.']) }
+      it { expect(flash.alert).to eq(["Le champ « l » doit être rempli, #{anchor_to_first_champ}"]) }
+    end
+
+    context 'when a dossier is invalid' do
+      before do
+        allow_any_instance_of(Dossier).to receive(:groupe_instructeur).and_return(double(nil?: true))
+        subject
+      end
+
+      it { expect(response).to render_template(:brouillon) }
+      it { expect(flash.alert).to eq(["Le champ « Votre ville » doit être rempli"]) }
     end
 
     context 'when dossier has no champ' do
@@ -525,6 +536,7 @@ describe Users::DossiersController, type: :controller do
     let(:procedure) { create(:procedure, :published, :with_type_de_champ, :with_piece_justificative) }
     let!(:dossier) { create(:dossier, :en_construction, user: user, procedure: procedure) }
     let(:first_champ) { dossier.champs_public.first }
+    let(:anchor_to_first_champ) { controller.helpers.link_to I18n.t('views.users.dossiers.fix_champ'), brouillon_dossier_path(anchor: first_champ.input_id) }
     let(:piece_justificative_champ) { dossier.champs_public.last }
     let(:value) { 'beautiful value' }
     let(:file) { fixture_file_upload('spec/fixtures/files/piece_justificative_0.pdf', 'application/pdf') }
@@ -622,14 +634,14 @@ describe Users::DossiersController, type: :controller do
     context 'when the update fails' do
       before do
         expect_any_instance_of(Dossier).to receive(:save).and_return(false)
-        expect_any_instance_of(Dossier).to receive(:errors)
+        expect_any_instance_of(Dossier).to receive(:errors).and_return(
-          .and_return(double(full_messages: ['nop']))
+          [double(class: ActiveModel::Error, full_message: 'nop', base: first_champ)]
-
+        )
         subject
       end
 
       it { expect(response).to render_template(:modifier) }
-      it { expect(flash.alert).to eq(['nop']) }
+      it { expect(flash.alert).to eq(["Le champ « #{first_champ.libelle} » nop, #{anchor_to_first_champ}"]) }
 
       it 'does not update the dossier timestamps' do
         dossier.reload
@@ -653,7 +665,20 @@ describe Users::DossiersController, type: :controller do
       end
 
       it { expect(response).to render_template(:modifier) }
-      it { expect(flash.alert).to eq(['Le champ l doit être rempli.']) }
+      it { expect(flash.alert).to eq(["Le champ « l » doit être rempli, #{anchor_to_first_champ}"]) }
+    end
+
+    context 'when a champ validation fails' do
+      let(:value) { 'abc' }
+
+      before do
+        first_champ.type_de_champ.update!(type_champ: :iban, mandatory: true, libelle: 'l')
+        dossier.champs_public.first.becomes!(Champs::IbanChamp).save!
+
+        subject
+      end
+
+      it { expect(flash.alert).to include("Le champ « l » n'est pas au format IBAN, #{anchor_to_first_champ}") }
     end
 
     context 'when the user has an invitation but is not the owner' do

spec/models/dossier_spec.rb

@@ -1240,7 +1240,7 @@ describe Dossier do
 
       it 'should have errors' do
         expect(errors).not_to be_empty
-        expect(errors.first).to eq("Le champ #{champ_with_error.libelle} doit être rempli.")
+        expect(errors.first.full_message).to eq("doit être rempli")
       end
 
       context "conditionaly visible" do
@@ -1273,7 +1273,7 @@ describe Dossier do
 
         it 'should have errors' do
           expect(errors).not_to be_empty
-          expect(errors.first).to eq("Le champ #{champ_siret.libelle} doit être rempli.")
+          expect(errors.first.full_message).to eq("doit être rempli")
         end
       end
     end
@@ -1290,7 +1290,7 @@ describe Dossier do
           dossier.champs_public.first.champs.destroy_all
           expect(dossier.champs_public.first.rows).to be_empty
           expect(errors).not_to be_empty
-          expect(errors.first).to eq("Le champ #{champ_with_error.libelle} doit être rempli.")
+          expect(errors.first.full_message).to eq("doit être rempli")
         end
       end
 
@@ -1299,8 +1299,7 @@ describe Dossier do
 
         it 'should have errors' do
           expect(dossier.champs_public.first.rows).not_to be_empty
-          expect(errors).not_to be_empty
+          expect(errors.first.full_message).to eq("doit être rempli")
-          expect(errors.first).to eq("Le champ #{champ_with_error.libelle} doit être rempli.")
         end
 
         context "conditionaly visible" do
@@ -1317,7 +1316,7 @@ describe Dossier do
             dossier.champs_public.first.update(value: 'true')
             expect(dossier.champs_public.second.rows).not_to be_empty
             expect(errors).not_to be_empty
-            expect(errors.first).to eq("Le champ #{champ_with_error.libelle} doit être rempli.")
+            expect(errors.first.full_message).to eq("doit être rempli")
           end
         end
       end