From 751389bd9ae03974ef28e2082e17474d6acae941 Mon Sep 17 00:00:00 2001 From: mfo Date: Wed, 5 Jun 2024 19:16:41 +0200 Subject: [PATCH] clean(linters): cleanup code, enhance wording and test coverage --- .../ineligibilite_rules_component.html.haml | 38 +++--- .../ineligibilite_dossier_component.fr.yml | 2 +- app/models/dossier.rb | 3 +- app/models/logic/and.rb | 4 +- app/models/logic/or.rb | 1 - app/models/procedure_revision.rb | 7 -- .../ineligibilite_rules/edit.html.haml | 6 +- config/locales/fr.yml | 2 +- config/locales/models/procedure/fr.yml | 2 +- .../locales/models/procedure_revision/fr.yml | 7 ++ .../editor_component_spec.rb | 2 +- .../ineligibilite_rules_controller_spec.rb | 4 +- .../procedure_ineligibilite_spec.rb | 4 +- .../users/dossier_ineligibilite_spec.rb | 118 ++++++++++++++++-- 14 files changed, 152 insertions(+), 48 deletions(-) create mode 100644 config/locales/models/procedure_revision/fr.yml diff --git a/app/components/conditions/ineligibilite_rules_component/ineligibilite_rules_component.html.haml b/app/components/conditions/ineligibilite_rules_component/ineligibilite_rules_component.html.haml index 547a2ad8560..1d657de2d76 100644 --- a/app/components/conditions/ineligibilite_rules_component/ineligibilite_rules_component.html.haml +++ b/app/components/conditions/ineligibilite_rules_component/ineligibilite_rules_component.html.haml @@ -1,10 +1,18 @@ %div{ id: dom_id(@draft_revision, :ineligibilite_rules) } = render Procedure::PendingRepublishComponent.new(procedure: @draft_revision.procedure, render_if: pending_changes?) = render Conditions::ConditionsErrorsComponent.new(conditions: condition_per_row, source_tdcs: @source_tdcs) - %fieldset.fr-fieldset - %legend.fr-mx-1w.fr-label.fr-py-0.fr-mb-1w.fr-mt-2w - Règles d’inéligibilité - %span.fr-hint-text Vous pouvez utiliser 1 ou plusieurs critère pour bloquer le dépot + .fr-fieldset + = form_for(@draft_revision, url: change_admin_procedure_ineligibilite_rules_path(@draft_revision.procedure_id), html: { id: 'ineligibilite_form' }) do |f| + .fr-fieldset__element + .fr-toggle + = f.check_box :ineligibilite_enabled, class: 'fr-toggle__input', data: @opt + = f.label :ineligibilite_enabled, "Inéligibilité des dossiers", data: { 'fr-checked-label': "Actif", 'fr-unchecked-label': "Inactif" }, class: 'fr-toggle__label' + %p.fr-hint-text Passer l’intérrupteur sur actif pour que les conditions d’inéligibilité configurés s'appliquent + .fr-fieldset__element= render Dsfr::InputComponent.new(form: f, attribute: :ineligibilite_message, input_type: :text_area, opts: {rows: 5}) + + .fr-mx-1w.fr-label.fr-py-0.fr-mb-1w.fr-mt-2w + Conditions d’inéligibilité + %span.fr-hint-text Vous pouvez utiliser une ou plusieurs condtions pour bloquer le dépot (si un champ conditionné n'est pas visible, la condition d'inéligibilité ne s'appliquera pas) .fr-fieldset__element = form_tag admin_procedure_ineligibilite_rules_path(@draft_revision.procedure_id), method: :patch, data: { turbo: true, controller: 'autosave' }, class: 'form width-100' do .conditionnel.width-100 @@ -28,15 +36,13 @@ %tr %td.text-right{ colspan: 5 }= add_condition_tag - - - = form_for(@draft_revision, url: change_admin_procedure_ineligibilite_rules_path(@draft_revision.procedure_id)) do |f| - .fr-fieldset__element= render Dsfr::InputComponent.new(form: f, attribute: :ineligibilite_message, input_type: :text_area, opts: {rows: 5}) - .fr-fieldset__element - .fr-toggle - = f.check_box :ineligibilite_enabled, class: 'fr-toggle__input', data: @opt - = f.label :ineligibilite_enabled, "Inéligibilité des dossiers", data: { 'fr-checked-label': "Actif", 'fr-unchecked-label': "Inactif" }, class: 'fr-toggle__label' - %p.fr-hint-text Passer l’intérrupteur sur activé pour que les critères d’inéligibilité configurés s'appliquent - - - = render Procedure::FixedFooterComponent.new(procedure: @draft_revision.procedure, form: f, extra_class_names: 'fr-col-offset-md-2 fr-col-md-8') + .padded-fixed-footer + .fixed-footer + .fr-container + .fr-grid-row.fr-col-offset-md-2.fr-col-md-8 + %div{ class: "fr-col-12" } + %ul.fr-btns-group.fr-btns-group--inline-md + %li + = link_to "Annuler et revenir à l'écran de gestion", admin_procedure_path(id: @draft_revision.procedure), class: 'fr-btn fr-btn--secondary', data: { confirm: 'Si vous avez fait des modifications elles ne seront pas sauvegardées.'} + %li + = button_tag "Enregistrer", class: "fr-btn", form: 'ineligibilite_form' diff --git a/app/components/procedure/card/ineligibilite_dossier_component/ineligibilite_dossier_component.fr.yml b/app/components/procedure/card/ineligibilite_dossier_component/ineligibilite_dossier_component.fr.yml index d65f0d535b5..d7d9e2132ea 100644 --- a/app/components/procedure/card/ineligibilite_dossier_component/ineligibilite_dossier_component.fr.yml +++ b/app/components/procedure/card/ineligibilite_dossier_component/ineligibilite_dossier_component.fr.yml @@ -5,4 +5,4 @@ fr: pending: Champs à configurer ready: À configurer completed: Activé - subtitle: Gérez vos critères d’inéligibilité en fonction des champs du formulaire + subtitle: Gérez vos conditions d’inéligibilité en fonction des champs du formulaire diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 2a3a14d5bb1..899687b0fbf 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -940,8 +940,9 @@ def check_mandatory_and_visible_champs .filter(&:visible?) .filter(&:mandatory_blank?) .map do |champ| - errors.import(champ.errors.add(:value, :missing)) + champ.errors.add(:value, :missing) end + .each { errors.import(_1) } end def ineligibilite_rules_computable? diff --git a/app/models/logic/and.rb b/app/models/logic/and.rb index 1520949090b..8649f1768b0 100644 --- a/app/models/logic/and.rb +++ b/app/models/logic/and.rb @@ -11,8 +11,8 @@ def compute_visible(champs = []) champs_by_stable_id = champs.index_by(&:stable_id) @operands.filter { |operand| operand.sources.all? { |champ_stable_id| champs_by_stable_id[champ_stable_id].visible? } } - .map { |operand| operand.compute(champs) } - .all? + .map { |operand| operand.compute(champs) } + .all? end def computable?(champs = []) diff --git a/app/models/logic/or.rb b/app/models/logic/or.rb index 6b0c51e6acb..1b8d0da9d35 100644 --- a/app/models/logic/or.rb +++ b/app/models/logic/or.rb @@ -8,7 +8,6 @@ def compute(champs = []) end alias compute_visible compute # visible or not, and or is or - def computable?(champs = []) return true if sources.blank? diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb index bb7dbb43e14..7e4f3086029 100644 --- a/app/models/procedure_revision.rb +++ b/app/models/procedure_revision.rb @@ -496,13 +496,6 @@ def ineligibilite_rules_are_valid? end end - def ineligibilite_rules_are_valid? - if ineligibilite_rules - ineligibilite_rules.errors(types_de_champ_for(scope: :public).to_a) - .each { errors.add(:ineligibilite_rules, :invalid) } - end - end - def replace_type_de_champ_by_clone(coordinate) cloned_type_de_champ = coordinate.type_de_champ.deep_clone do |original, kopy| ClonePiecesJustificativesService.clone_attachments(original, kopy) diff --git a/app/views/administrateurs/ineligibilite_rules/edit.html.haml b/app/views/administrateurs/ineligibilite_rules/edit.html.haml index a76a3046829..6eb91d12f1b 100644 --- a/app/views/administrateurs/ineligibilite_rules/edit.html.haml +++ b/app/views/administrateurs/ineligibilite_rules/edit.html.haml @@ -12,17 +12,17 @@ = render Dsfr::AlertComponent.new(title: nil, size: :sm, state: :info, heading_level: 'h2', extra_class_names: 'fr-my-2w') do |c| - c.with_body do %p - Les dossiers répondant à vos critères d’inéligibilité ne pourront pas être déposés. Plus d’informations sur l’inéligibilité des dossiers dans la + Les dossiers répondant à vos conditions d’inéligibilité ne pourront pas être déposés. Plus d’informations sur l’inéligibilité des dossiers dans la = link_to('doc', ELIGIBILITE_URL, title: "Document sur l’inéligibilité des dossiers", **external_link_attributes) - if !@procedure.draft_revision.conditionable_types_de_champ.present? %p.fr-mt-2w.fr-mb-2w - Pour configurer l’inéligibilité des dossiers, votre formulaire doit comporter au moins un champ supportant les critères d’inéligibilité. Il vous faut donc ajouter au moins un des champs suivant à votre formulaire : + Pour configurer l’inéligibilité des dossiers, votre formulaire doit comporter au moins un champ supportant les conditions d’inéligibilité. Il vous faut donc ajouter au moins un des champs suivant à votre formulaire : %ul - Logic::ChampValue::MANAGED_TYPE_DE_CHAMP.values.each do %li= "« #{t(_1, scope: [:activerecord, :attributes, :type_de_champ, :type_champs])} »" %p.fr-mt-2w - = link_to 'Ajouter un champ supportant les critères d’inéligibilité', champs_admin_procedure_path(@procedure), class: 'fr-link fr-icon-arrow-right-line fr-link--icon-right' + = link_to 'Ajouter un champ supportant les conditions d’inéligibilité', champs_admin_procedure_path(@procedure), class: 'fr-link fr-icon-arrow-right-line fr-link--icon-right' = render Procedure::FixedFooterComponent.new(procedure: @procedure) - else = render Conditions::IneligibiliteRulesComponent.new(draft_revision: @procedure.draft_revision) diff --git a/config/locales/fr.yml b/config/locales/fr.yml index f7c0faf058d..f94b4abdd0f 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -609,7 +609,7 @@ fr: otp_attempt: 'Code OTP (uniquement si vous avez déjà activé 2FA)' procedure: zone: La démarche est mise en œuvre par - ineligibilite_rules: "Les règles d’Inéligibilité" + ineligibilite_rules: "Les règles d’inéligibilité" champs: value: Valeur du champ default_mail_attributes: &default_mail_attributes diff --git a/config/locales/models/procedure/fr.yml b/config/locales/models/procedure/fr.yml index 85df92d73dc..5a9dfd9abfc 100644 --- a/config/locales/models/procedure/fr.yml +++ b/config/locales/models/procedure/fr.yml @@ -8,7 +8,7 @@ fr: procedure: hints: description: Décrivez en quelques lignes le contexte, la finalité, etc. - description_target_audience: Décrivez en quelques lignes les destinataires finaux de la démarche, les critères d’éligibilité s’il y en a, les pré-requis, etc. + description_target_audience: Décrivez en quelques lignes les destinataires finaux de la démarche, les conditions d’éligibilité s’il y en a, les pré-requis, etc. description_pj: Décrivez la liste des pièces jointes à fournir s’il y en a lien_site_web: "Il s'agit de la page de votre site web où le lien sera diffusé. Ex: https://exemple.gouv.fr/page_informant_sur_ma_demarche" cadre_juridique: "Exemple: 'https://www.legifrance.gouv.fr/'" diff --git a/config/locales/models/procedure_revision/fr.yml b/config/locales/models/procedure_revision/fr.yml new file mode 100644 index 00000000000..ab3a8ec2271 --- /dev/null +++ b/config/locales/models/procedure_revision/fr.yml @@ -0,0 +1,7 @@ +fr: + activerecord: + attributes: + procedure_revision: + ineligibilite_message: Message d’inéligibilité + hints: + ineligibilite_message: "Ce message sera affiché à l’usager si son dossier correspond à vos conditions d’inéligibilité" diff --git a/spec/components/types_de_champ_editor/editor_component_spec.rb b/spec/components/types_de_champ_editor/editor_component_spec.rb index 5b643995c4a..fb709498363 100644 --- a/spec/components/types_de_champ_editor/editor_component_spec.rb +++ b/spec/components/types_de_champ_editor/editor_component_spec.rb @@ -19,7 +19,7 @@ context 'types_de_champ_private' do let(:is_annotation) { true } it 'does not render public champs errors' do - expect(subject).to have_selector("a", "private") + expect(subject).to have_selector("a", text: "private") expect(subject).to have_text("doit comporter au moins un choix sélectionnable") expect(subject).not_to have_text("public") end diff --git a/spec/controllers/administrateurs/ineligibilite_rules_controller_spec.rb b/spec/controllers/administrateurs/ineligibilite_rules_controller_spec.rb index 5c8f94628de..2a76a054a58 100644 --- a/spec/controllers/administrateurs/ineligibilite_rules_controller_spec.rb +++ b/spec/controllers/administrateurs/ineligibilite_rules_controller_spec.rb @@ -197,14 +197,14 @@ let(:types_de_champ_public) { [] } render_views - it { expect(response.body).to have_link("Ajouter un champ supportant les critères d’inéligibilité") } + it { expect(response.body).to have_link("Ajouter un champ supportant les conditions d’inéligibilité") } end context 'rendered with tdc' do let(:types_de_champ_public) { [{ type: :yes_no }] } render_views - it { expect(response.body).not_to have_link("Ajouter un champ supportant les critères d’inéligibilité") } + it { expect(response.body).not_to have_link("Ajouter un champ supportant les conditions d’inéligibilité") } end end end diff --git a/spec/system/administrateurs/procedure_ineligibilite_spec.rb b/spec/system/administrateurs/procedure_ineligibilite_spec.rb index 9db80cf5957..b5d69e7c3bb 100644 --- a/spec/system/administrateurs/procedure_ineligibilite_spec.rb +++ b/spec/system/administrateurs/procedure_ineligibilite_spec.rb @@ -14,8 +14,8 @@ # explain which champs are compatible visit edit_admin_procedure_ineligibilite_rules_path(procedure) expect(page).to have_content("Inéligibilité des dossiers") - expect(page).to have_content("Pour configurer l’inéligibilité des dossiers, votre formulaire doit comporter au moins un champ supportant les critères d’inéligibilité. Il vous faut donc ajouter au moins un des champs suivant à votre formulaire : ") - click_on "Ajouter un champ supportant les critères d’inéligibilité" + expect(page).to have_content("Pour configurer l’inéligibilité des dossiers, votre formulaire doit comporter au moins un champ supportant les conditions d’inéligibilité. Il vous faut donc ajouter au moins un des champs suivant à votre formulaire : ") + click_on "Ajouter un champ supportant les conditions d’inéligibilité" # setup a compatible champ expect(page).to have_content('Champs du formulaire') diff --git a/spec/system/users/dossier_ineligibilite_spec.rb b/spec/system/users/dossier_ineligibilite_spec.rb index 366dac7803c..6fee2419f45 100644 --- a/spec/system/users/dossier_ineligibilite_spec.rb +++ b/spec/system/users/dossier_ineligibilite_spec.rb @@ -9,7 +9,7 @@ let(:published_revision) { procedure.published_revision } let(:first_tdc) { published_revision.types_de_champ.first } - let(:second_tdc) { published_revision.types_de_champ.last } + let(:second_tdc) { published_revision.types_de_champ.second } let(:ineligibilite_message) { 'sry vous pouvez aps soumettre votre dossier' } let(:eligibilite_params) { { ineligibilite_enabled: true, ineligibilite_message: } } @@ -18,7 +18,7 @@ login_as user, scope: :user end - context 'single condition' do + describe 'ineligibilite_rules with a single BinaryOperator' do let(:types_de_champ_public) { [{ type: :yes_no }] } let(:ineligibilite_rules) { ds_eq(champ_value(first_tdc.stable_id), constant(true)) } @@ -28,24 +28,27 @@ expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: false) expect(page).not_to have_content("Vous ne pouvez pas déposer votre dossier") - # does raise error when dossier is filled with valid condition + # does raise error when dossier is filled with condition that does not match find("label", text: "Non").click expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: false) expect(page).not_to have_content("Vous ne pouvez pas déposer votre dossier") - # raise error when dossier is filled with invalid condition + # raise error when dossier is filled with condition that matches find("label", text: "Oui").click expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: true) expect(page).to have_content("Vous ne pouvez pas déposer votre dossier") - # reload page and see error because it was filled + # reload page and see error visit brouillon_dossier_path(dossier) expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: true) expect(page).to have_content("Vous ne pouvez pas déposer votre dossier") # modal is closable, and we can change our dossier response to be eligible within("#modal-eligibilite-rules-dialog") { click_on "Fermer" } - find("label", text: "Non").click + expect(page).to have_selector("#modal-eligibilite-rules-dialog", visible: false) + within "#champ-#{first_tdc.stable_id}" do + find("label", text: "Non").click + end expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: false) # it works, yay @@ -54,7 +57,7 @@ end end - context 'or condition' do + describe 'ineligibilite_rules with a Or' do let(:types_de_champ_public) { [{ type: :yes_no, libelle: 'l1' }, { type: :drop_down_list, libelle: 'l2', options: ['Paris', 'Marseille'] }] } let(:ineligibilite_rules) do ds_or([ @@ -69,7 +72,7 @@ expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: false) expect(page).not_to have_content("Vous ne pouvez pas déposer votre dossier") - # only one condition is matches, cannot submit dossier and error message is clear + # first condition matches (so ineligible), cannot submit dossier and error message is clear within "#champ-#{first_tdc.stable_id}" do find("label", text: "Oui").click end @@ -77,7 +80,7 @@ expect(page).to have_content("Vous ne pouvez pas déposer votre dossier") within("#modal-eligibilite-rules-dialog") { click_on "Fermer" } - # only one condition does not matches, I can conitnue + # first condition does not matches, I can conitnue within "#champ-#{first_tdc.stable_id}" do find("label", text: "Non").click end @@ -88,7 +91,7 @@ click_on "Accéder à votre dossier" click_on "Modifier le dossier" - # one condition matches, means i'm blocked to send my file. + # first matches, means i'm blocked to send my file. within "#champ-#{first_tdc.stable_id}" do find("label", text: "Oui").click end @@ -116,4 +119,99 @@ wait_until { dossier.reload.en_construction? == true } end end + + describe 'ineligibilite_rules with a And and all visible champs' do + let(:types_de_champ_public) { [{ type: :yes_no, libelle: 'l1' }, { type: :drop_down_list, libelle: 'l2', options: ['Paris', 'Marseille'] }] } + let(:ineligibilite_rules) do + ds_and([ + ds_eq(champ_value(first_tdc.stable_id), constant(true)), + ds_eq(champ_value(second_tdc.stable_id), constant('Paris')) + ]) + end + + scenario 'can submit, can not submit, can edit, etc...' do + visit brouillon_dossier_path(dossier) + # no error while dossier is empty + expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: false) + expect(page).not_to have_content("Vous ne pouvez pas déposer votre dossier") + + # only one condition is matches, can submit dossier + within "#champ-#{first_tdc.stable_id}" do + find("label", text: "Oui").click + end + expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: false) + expect(page).not_to have_content("Vous ne pouvez pas déposer votre dossier") + + # Now test dossier modification + click_on "Déposer le dossier" + click_on "Accéder à votre dossier" + click_on "Modifier le dossier" + + # second condition matches, means i'm blocked to send my file + within "#champ-#{second_tdc.stable_id}" do + find("label", text: 'Paris').click + end + expect(page).to have_selector(:button, text: "Déposer les modifications", disabled: true) + within("#modal-eligibilite-rules-dialog") { click_on "Fermer" } + + # none of conditions matches, i can submit + within "#champ-#{second_tdc.stable_id}" do + find("label", text: 'Marseille').click + end + + # it works, yay + click_on "Déposer les modifications" + wait_until { dossier.reload.en_construction? == true } + end + end + + describe 'ineligibilite_rules with a And applied to champs with condition' do + let(:types_de_champ_public) do + [ + { type: :yes_no, libelle: 'l1', stable_id: 1 }, + # visible if l1 is yes + { type: :drop_down_list, libelle: 'l2', options: ['Paris', 'Marseille'], stable_id: 2, condition: ds_eq(champ_value(1), constant(true)) }, + # visible if l1 is no + { type: :drop_down_list, libelle: 'l3', options: ['Lille', 'Toulouse'], stable_id: 3, condition: ds_eq(champ_value(1), constant(false)) } + ] + end + let(:ineligibilite_rules) do + ds_and([ # none of all conditions care be true at the same time because l2/l3 are shown depending on l1 true or false + ds_eq(champ_value(1), constant(true)), + ds_eq(champ_value(2), constant('Paris')), + ds_eq(champ_value(3), constant('Lille')) + ]) + end + + scenario 'can submit, can not submit, can edit, etc...' do + visit brouillon_dossier_path(dossier) + # no error while dossier is empty + expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: false) + expect(page).not_to have_content("Vous ne pouvez pas déposer votre dossier") + + # only one condition matches, but another one is visible and not filled. So I can submit + within "#champ-#{first_tdc.stable_id}" do + find("label", text: "Oui").click + end + expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: false) + expect(page).not_to have_content("Vous ne pouvez pas déposer votre dossier") + + # second condition matches, means i'm blocked to send my file + within "#champ-#{second_tdc.stable_id}" do + find("label", text: 'Paris').click + end + # binding.irb + expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: true) + within("#modal-eligibilite-rules-dialog") { click_on "Fermer" } + + # change second condition to not match ineligibilite rules, i can send my file + within "#champ-#{second_tdc.stable_id}" do + find("label", text: 'Marseille').click + end + expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: false) + + click_on "Déposer le dossier" + wait_until { dossier.reload.en_construction? == true } + end + end end