diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 2a8f83e94..d4d4d1469 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 6b2163240..2f12ade8e 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 798526e71..b98acd936 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