diff --git a/app/components/dossiers/errors_full_messages_component.rb b/app/components/dossiers/errors_full_messages_component.rb new file mode 100644 index 000000000..760cd3aa6 --- /dev/null +++ b/app/components/dossiers/errors_full_messages_component.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class Dossiers::ErrorsFullMessagesComponent < ApplicationComponent + ErrorDescriptor = Data.define(:anchor, :label, :error_message) + + def initialize(dossier:, errors:) + @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 + .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.message, error.inner_error.base) } + yield(Array(formated_errors[0..2]), Array(formated_errors[3..])) + end + + def to_error_descriptor(str_error, model) + ErrorDescriptor.new("##{model.labelledby_id}", model.libelle.truncate(200), str_error) + end + + def render? + !@errors.empty? + end +end diff --git a/app/components/dossiers/errors_full_messages_component/errors_full_messages_component.en.yml b/app/components/dossiers/errors_full_messages_component/errors_full_messages_component.en.yml new file mode 100644 index 000000000..3fab8164d --- /dev/null +++ b/app/components/dossiers/errors_full_messages_component/errors_full_messages_component.en.yml @@ -0,0 +1,8 @@ +--- +en: + sumup_html: + one: | + Your file has 1 error. Fix-it to continue : + other: | + Your file has %{count} errors. Fix-them to continue : + see_more: Show all errors diff --git a/app/components/dossiers/errors_full_messages_component/errors_full_messages_component.fr.yml b/app/components/dossiers/errors_full_messages_component/errors_full_messages_component.fr.yml new file mode 100644 index 000000000..1fd0e7f8c --- /dev/null +++ b/app/components/dossiers/errors_full_messages_component/errors_full_messages_component.fr.yml @@ -0,0 +1,8 @@ +--- +fr: + sumup_html: + one: | + Votre dossier contient 1 champ en erreur. Corrigez-la pour poursuivre : + other: | + Votre dossier contient %{count} champs en erreurs. Corrigez-les pour poursuivre : + see_more: Afficher toutes les erreurs diff --git a/app/components/dossiers/errors_full_messages_component/errors_full_messages_component.html.haml b/app/components/dossiers/errors_full_messages_component/errors_full_messages_component.html.haml new file mode 100644 index 000000000..269286402 --- /dev/null +++ b/app/components/dossiers/errors_full_messages_component/errors_full_messages_component.html.haml @@ -0,0 +1,16 @@ +.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})" + diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 9debbec4e..5a77cd75e 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -207,9 +207,9 @@ module Users def submit_brouillon @dossier = dossier_with_champs(pj_template: false) - errors = submit_dossier_and_compute_errors + @errors = submit_dossier_and_compute_errors - if errors.blank? + if @errors.blank? @dossier.passer_en_construction! @dossier.process_declarative! @dossier.process_sva_svr! @@ -218,8 +218,6 @@ module Users end redirect_to merci_dossier_path(@dossier) else - flash.now.alert = errors - respond_to do |format| format.html { render :brouillon } format.turbo_stream @@ -250,9 +248,9 @@ module Users def submit_en_construction @dossier = dossier_with_champs(pj_template: false) - errors = submit_dossier_and_compute_errors + @errors = submit_dossier_and_compute_errors - if errors.blank? + if @errors.blank? pending_correction_confirm = cast_bool(params.dig(:dossier, :pending_correction_confirm)) editing_fork_origin = @dossier.editing_fork_origin editing_fork_origin.merge_fork(@dossier) @@ -260,8 +258,6 @@ module Users redirect_to dossier_path(editing_fork_origin) else - flash.now.alert = errors - respond_to do |format| format.html do @dossier = @dossier.editing_fork_origin @@ -279,11 +275,7 @@ module Users def update @dossier = dossier.en_construction? ? dossier.find_editing_fork(dossier.user) : dossier @dossier = dossier_with_champs(pj_template: false) - errors = update_dossier_and_compute_errors - - if errors.present? - flash.now.alert = errors - end + @errors = update_dossier_and_compute_errors respond_to do |format| format.turbo_stream do @@ -531,46 +523,22 @@ module Users if @dossier.champs_public_all.any?(&:changed_for_autosave?) @dossier.last_champ_updated_at = Time.zone.now end + if !@dossier.save(**validation_options) - errors += format_errors(errors: @dossier.errors) + errors = @dossier.errors end errors end def submit_dossier_and_compute_errors - errors = [] - @dossier.valid?(**submit_validation_options) - errors += format_errors(errors: @dossier.errors) - errors += format_errors(errors: @dossier.check_mandatory_and_visible_champs) - 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 + errors = @dossier.errors + @dossier.check_mandatory_and_visible_champs.map do |error_on_champ| + errors.import(error_on_champ) end - end - - def append_anchor_link(str_error, model) - return str_error.full_message if !model.is_a?(Champ) - - route_helper = @dossier.editing_fork? ? :modifier_dossier_path : :brouillon_dossier_path - - [ - "Le champ « #{model.libelle.truncate(200)} » #{str_error}", - helpers.link_to(t('views.users.dossiers.fix_champ'), public_send(route_helper, anchor: model.labelledby_id), class: 'error-anchor') - ].join(", ") - rescue # case of invalid type de champ on champ - str_error + errors end def ensure_ownership! diff --git a/app/models/champs/date_champ.rb b/app/models/champs/date_champ.rb index 10fa8602c..d372eecea 100644 --- a/app/models/champs/date_champ.rb +++ b/app/models/champs/date_champ.rb @@ -29,7 +29,7 @@ class Champs::DateChamp < Champ def iso_8601 return if parsable_iso8601? || value.blank? # i18n-tasks-use t('errors.messages.not_a_date') - errors.add :date, errors.generate_message(:value, :not_a_date) + errors.add :date, :not_a_date end def likely_iso8601_format? diff --git a/app/models/champs/datetime_champ.rb b/app/models/champs/datetime_champ.rb index 08a4c4719..fe1c2ecd7 100644 --- a/app/models/champs/datetime_champ.rb +++ b/app/models/champs/datetime_champ.rb @@ -40,7 +40,7 @@ class Champs::DatetimeChamp < Champ def iso_8601 return if valid_iso8601? || value.blank? # i18n-tasks-use t('errors.messages.not_a_datetime') - errors.add :datetime, errors.generate_message(:value, :not_a_datetime) + errors.add :datetime, :not_a_datetime end def valid_iso8601? diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 4c68a5fa6..17931f171 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -1148,7 +1148,7 @@ class Dossier < ApplicationRecord .filter(&:visible?) .filter(&:mandatory_blank?) .map do |champ| - champ.errors.add(:value, message: champ.errors.generate_message(:value, :missing)) + champ.errors.add(:value, :missing) end end diff --git a/app/validators/iban_validator.rb b/app/validators/iban_validator.rb index 7c27a6daf..5c343005e 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 :value, message: record.errors.generate_message(:value, :invalid_iban) + record.errors.add :value, :invalid_iban end end end diff --git a/app/views/shared/dossiers/_edit.html.haml b/app/views/shared/dossiers/_edit.html.haml index 0df412dfe..cac9e73d4 100644 --- a/app/views/shared/dossiers/_edit.html.haml +++ b/app/views/shared/dossiers/_edit.html.haml @@ -1,13 +1,17 @@ - dossier_for_editing = dossier.en_construction? ? dossier.owner_editing_fork : dossier + - if dossier.france_connect_information.present? - content_for(:notice_info) do = render partial: "shared/dossiers/france_connect_informations_notice", locals: { user_information: dossier.france_connect_information } + .dossier-edit.container.counter-start-header-section = render partial: "shared/dossiers/submit_is_over", locals: { dossier: dossier } = 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 || []) %header.mb-6 .fr-highlight %p.fr-text--sm diff --git a/config/locales/en.yml b/config/locales/en.yml index cbbd64245..a199998ea 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -408,7 +408,6 @@ en: dossier_not_in_instructor_group: "File no. %{dossier_id} of the “%{procedure_libelle}” procedure corresponds to your search, but it is attached to the “%{groupe_instructeur_label}” instructor group." users: dossiers: - 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 diff --git a/config/locales/fr.yml b/config/locales/fr.yml index a8bb3d8cd..f0ec45337 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -410,7 +410,6 @@ fr: dossier_not_in_instructor_group: "Le dossier n° %{dossier_id} de la procédure « %{procedure_libelle} » correspond à votre recherche mais il est rattaché au groupe d’instructeurs « %{groupe_instructeur_label} »." users: dossiers: - 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é diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index c85c9b477..754941ecc 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -370,10 +370,9 @@ describe Users::DossiersController, type: :controller do describe '#submit_brouillon' do before { sign_in(user) } - 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.labelledby_id), class: 'error-anchor' } + let(:anchor_to_first_champ) { controller.helpers.link_to first_champ.libelle, brouillon_dossier_path(anchor: first_champ.labelledby_id), class: 'error-anchor' } let(:value) { 'beautiful value' } let(:now) { Time.zone.parse('01/01/2100') } let(:payload) { { id: dossier.id } } @@ -409,16 +408,19 @@ describe Users::DossiersController, type: :controller do end context 'when the update fails' do + render_views + let(:error_message) { 'nop' } before do expect_any_instance_of(Dossier).to receive(:valid?).and_return(false) expect_any_instance_of(Dossier).to receive(:errors).and_return( - [double(class: ActiveModel::Error, full_message: 'nop', base: first_champ)] + [double(inner_error: double(base: first_champ), message: 'nop')] ) subject end it { expect(response).to render_template(:brouillon) } - it { expect(flash.alert).to eq(["Le champ « #{first_champ.libelle} » nop, #{anchor_to_first_champ}"]) } + it { expect(response.body).to have_link(first_champ.libelle, href: "##{first_champ.labelledby_id}") } + it { expect(response.body).to have_content(error_message) } it 'does not send an email' do expect(NotificationMailer).not_to receive(:send_en_construction_notification) @@ -428,6 +430,8 @@ describe Users::DossiersController, type: :controller do end context 'when a mandatory champ is missing' do + render_views + let(:value) { nil } before do @@ -436,7 +440,8 @@ 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, #{anchor_to_first_champ}"]) } + it { expect(response.body).to have_link(first_champ.libelle, href: "##{first_champ.labelledby_id}") } + it { expect(response.body).to have_content("doit être rempli") } end context 'when dossier has no champ' do @@ -515,27 +520,26 @@ describe Users::DossiersController, type: :controller 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(class: ActiveModel::Error, full_message: 'nop', base: first_champ)] + [double(inner_error: double(base: first_champ), message: 'nop')] ) subject end it { expect(response).to render_template(:modifier) } - it { expect(flash.alert).to eq(["Le champ « #{first_champ.libelle} » nop, #{anchor_to_first_champ}"]) } - it { expect(response.body).to include("Dossier nº #{dossier.id}") } end context 'when a mandatory champ is missing' do let(:value) { nil } - + render_views before do first_champ.type_de_champ.update(mandatory: true, libelle: 'l') subject end it { expect(response).to render_template(:modifier) } - it { expect(flash.alert).to eq(["Le champ « l » doit être rempli, #{anchor_to_first_champ}"]) } + it { expect(response.body).to have_content("doit être rempli") } + it { expect(response.body).to have_link(first_champ.libelle, href: "##{first_champ.labelledby_id}") } end context 'when dossier has no champ' do @@ -828,41 +832,44 @@ describe Users::DossiersController, type: :controller do end 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(class: ActiveModel::Error, full_message: 'nop', base: first_champ)] - ) - subject + render_views + + context 'classic error' do + before do + expect_any_instance_of(Dossier).to receive(:save).and_return(false) + expect_any_instance_of(Dossier).to receive(:errors).and_return( + [message: 'nop', inner_error: double(base: first_champ)] + ) + subject + end + + it { expect(response).to render_template(:update) } + + it 'does not update the dossier timestamps' do + dossier.reload + expect(dossier.updated_at).not_to eq(now) + expect(dossier.last_champ_updated_at).not_to eq(now) + end + + it 'does not send an email' do + expect(NotificationMailer).not_to receive(:send_en_construction_notification) + + subject + end end - it { expect(response).to render_template(:update) } - it { expect(flash.alert).to eq(["Le champ « #{first_champ.libelle} » nop, #{anchor_to_first_champ}"]) } + context 'iban error' do + let(:value) { 'abc' } - it 'does not update the dossier timestamps' do - dossier.reload - expect(dossier.updated_at).not_to eq(now) - expect(dossier.last_champ_updated_at).not_to eq(now) + 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(response).to have_http_status(:success) } end - - it 'does not send an email' do - expect(NotificationMailer).not_to receive(:send_en_construction_notification) - - subject - end - 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/system/api_particulier/api_particulier_spec.rb b/spec/system/api_particulier/api_particulier_spec.rb index d72ca120c..554c058a2 100644 --- a/spec/system/api_particulier/api_particulier_spec.rb +++ b/spec/system/api_particulier/api_particulier_spec.rb @@ -282,7 +282,7 @@ describe 'fetch API Particulier Data', js: true, retry: 3 do wait_until { cnaf_champ.reload.code_postal == 'wrong_code' } click_on 'Déposer le dossier' - expect(page).to have_content(/Le champ « Champs public code postal » doit posséder 5 caractères/) + expect(page).to have_content("cnaf doit posséder 5 caractères") VCR.use_cassette('api_particulier/success/composition_familiale') do fill_in 'Le code postal', with: code_postal @@ -479,7 +479,7 @@ describe 'fetch API Particulier Data', js: true, retry: 3 do wait_until { dgfip_champ.reload.reference_avis == 'wrong_code' } click_on 'Déposer le dossier' - expect(page).to have_content(/Le champ « Champs public reference avis » doit posséder 13 ou 14 caractères/) + expect(page).to have_content(/dgfip doit posséder 13 ou 14 caractères/) fill_in "La référence d’avis d’imposition", with: reference_avis wait_for_autosave diff --git a/spec/system/users/brouillon_spec.rb b/spec/system/users/brouillon_spec.rb index f20ef195f..78e95b5c1 100644 --- a/spec/system/users/brouillon_spec.rb +++ b/spec/system/users/brouillon_spec.rb @@ -109,7 +109,7 @@ describe 'The user' do fill_individual click_on 'Déposer le dossier' - expect(page).to have_selector("#flash_message") + expect(page).to have_selector("#sumup-errors") all('.error-anchor').map do |link_element| error_anchor = URI(link_element['href']) expect(page).to have_selector("##{error_anchor.fragment}")