From c8ab1e8e03d8371ccb032ba2573dfcbc1cd49229 Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 28 Apr 2023 15:32:34 +0200 Subject: [PATCH] amelioration(dossier#submit_brouillon): ETQ usager, je souhaite pouvoir acceder aux champs en erreur facilement --- app/controllers/users/dossiers_controller.rb | 34 +++++++++++--- app/models/dossier.rb | 2 +- app/validators/iban_validator.rb | 2 +- config/locales/en.yml | 6 +++ config/locales/fr.yml | 7 +++ config/locales/models/champs/champs.en.yml | 10 +++++ config/locales/models/champs/champs.fr.yml | 10 +++++ .../locales/models/champs/iban_champ/en.yml | 9 ++++ .../locales/models/champs/iban_champ/fr.yml | 9 ++++ .../users/dossiers_controller_spec.rb | 45 ++++++++++++++----- spec/models/dossier_spec.rb | 11 +++-- 11 files changed, 121 insertions(+), 24 deletions(-) create mode 100644 config/locales/models/champs/champs.en.yml create mode 100644 config/locales/models/champs/champs.fr.yml create mode 100644 config/locales/models/champs/iban_champ/en.yml create mode 100644 config/locales/models/champs/iban_champ/fr.yml diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 022f79bee..fc2a3a7eb 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/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 += @dossier.check_mandatory_and_visible_champs + errors += format_errors(errors: @dossier.errors) + 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! diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 8eb6e95b5..8509ed553 100644 --- a/app/models/dossier.rb +++ b/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 diff --git a/app/validators/iban_validator.rb b/app/validators/iban_validator.rb index d31c30a19..7c27a6daf 100644 --- a/app/validators/iban_validator.rb +++ b/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 diff --git a/config/locales/en.yml b/config/locales/en.yml index ccc09d6c1..d043d4674 100644 --- a/config/locales/en.yml +++ b/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: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 8fcd3b281..f59b3d6bb 100644 --- a/config/locales/fr.yml +++ b/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: diff --git a/config/locales/models/champs/champs.en.yml b/config/locales/models/champs/champs.en.yml new file mode 100644 index 000000000..ead703062 --- /dev/null +++ b/config/locales/models/champs/champs.en.yml @@ -0,0 +1,10 @@ +fr: + activerecord: + errors: + models: + champ: + attributes: + value: + format: '%{message}' + missing: must be filled + diff --git a/config/locales/models/champs/champs.fr.yml b/config/locales/models/champs/champs.fr.yml new file mode 100644 index 000000000..090aabf55 --- /dev/null +++ b/config/locales/models/champs/champs.fr.yml @@ -0,0 +1,10 @@ +fr: + activerecord: + errors: + models: + champ: + attributes: + value: + format: '%{message}' + missing: doit être rempli + diff --git a/config/locales/models/champs/iban_champ/en.yml b/config/locales/models/champs/iban_champ/en.yml new file mode 100644 index 000000000..9623eea19 --- /dev/null +++ b/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 + diff --git a/config/locales/models/champs/iban_champ/fr.yml b/config/locales/models/champs/iban_champ/fr.yml new file mode 100644 index 000000000..f00ab4d0c --- /dev/null +++ b/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 + diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 2cd90fb78..864b532db 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/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) - .and_return(double('errors', full_messages: ['nop'])) - + expect_any_instance_of(Dossier).to receive(:errors).and_return( + [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) - .and_return(double(full_messages: ['nop'])) - + expect_any_instance_of(Dossier).to receive(:errors).and_return( + [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 diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 9c15e2b53..49c01777f 100644 --- a/spec/models/dossier_spec.rb +++ b/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).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 @@ -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