From 85ae5dd3583f5ed8f3b1f745bd7efcf175205dfa Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Tue, 10 Dec 2024 15:50:08 +0100 Subject: [PATCH 1/6] fix(dossier/footer): don't say user can submit dossier when he can't --- .../autosave_footer_component.en.yml | 3 +- .../autosave_footer_component.fr.yml | 3 +- .../autosave_footer_component.html.haml | 2 + .../autosave_footer_component_spec.rb | 44 +++++++++++++++++++ 4 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 spec/components/dossiers/autosave_footer_component_spec.rb diff --git a/app/components/dossiers/autosave_footer_component/autosave_footer_component.en.yml b/app/components/dossiers/autosave_footer_component/autosave_footer_component.en.yml index 21ac1dd6eb9..d03a8dd98cc 100644 --- a/app/components/dossiers/autosave_footer_component/autosave_footer_component.en.yml +++ b/app/components/dossiers/autosave_footer_component/autosave_footer_component.en.yml @@ -5,7 +5,8 @@ en: confirmation: Draft saved error: Impossible to save the draft en_construction: - explanation: Your modifications are automatically saved. Submit them when you’re done. + explanation: Your modifications are automatically saved. + submit_them: Submit them when you’re done. confirmation: Modifications saved error: Impossible to save the modifications. annotations: diff --git a/app/components/dossiers/autosave_footer_component/autosave_footer_component.fr.yml b/app/components/dossiers/autosave_footer_component/autosave_footer_component.fr.yml index bbee04894b7..6d962ca27e2 100644 --- a/app/components/dossiers/autosave_footer_component/autosave_footer_component.fr.yml +++ b/app/components/dossiers/autosave_footer_component/autosave_footer_component.fr.yml @@ -5,7 +5,8 @@ fr: confirmation: Brouillon enregistré error: Impossible d’enregistrer le brouillon en_construction: - explanation: Vos modifications sont automatiquement enregistrées. Déposez-les quand vous aurez terminé. + explanation: Vos modifications sont automatiquement enregistrées. + submit_them: Déposez-les quand vous aurez terminé. confirmation: Modifications enregistrées. error: Impossible d’enregistrer les modifications annotations: diff --git a/app/components/dossiers/autosave_footer_component/autosave_footer_component.html.haml b/app/components/dossiers/autosave_footer_component/autosave_footer_component.html.haml index 28039d90c53..879e3a349fe 100644 --- a/app/components/dossiers/autosave_footer_component/autosave_footer_component.html.haml +++ b/app/components/dossiers/autosave_footer_component/autosave_footer_component.html.haml @@ -5,6 +5,8 @@ = t('.annotations.explanation') - elsif dossier.editing_fork? = t('.en_construction.explanation') + - if dossier.can_passer_en_construction? + = t('.en_construction.submit_them') - else = t('.brouillon.explanation') - if !annotation? diff --git a/spec/components/dossiers/autosave_footer_component_spec.rb b/spec/components/dossiers/autosave_footer_component_spec.rb new file mode 100644 index 00000000000..84d7cd76b9a --- /dev/null +++ b/spec/components/dossiers/autosave_footer_component_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Dossiers::AutosaveFooterComponent, type: :component do + subject(:component) { render_inline(described_class.new(dossier:, annotation:)) } + + let(:dossier) { create(:dossier) } + let(:annotation) { false } + + context 'when showing brouillon state (default state)' do + it 'displays brouillon explanation' do + expect(component).to have_text("Votre brouillon") + end + end + + context 'when editing fork and can pass en construction' do + let(:dossier) { create(:dossier, :en_construction).find_or_create_editing_fork(create(:user)) } + + it 'displays en construction explanation' do + expect(component).to have_text("Vos modifications") + expect(component).to have_text("Déposez-les") + end + + context 'when dossier is not eligible' do + before do + allow(dossier).to receive(:can_passer_en_construction?).and_return(false) + end + + it 'displays en construction explanation' do + expect(component).to have_text("Vos modifications") + expect(component).not_to have_text("Déposez-les") + end + end + end + + context 'when showing annotations' do + let(:annotation) { true } + + it 'displays annotations explanation' do + expect(component).to have_text("Vos annotations") + end + end +end From e05d2013b2d7fa92e04a5f8a3da41cad949fcd9c Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Tue, 10 Dec 2024 15:52:17 +0100 Subject: [PATCH 2/6] refactor(dossier): predicates methods must return boolean --- app/models/concerns/dossier_clone_concern.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/concerns/dossier_clone_concern.rb b/app/models/concerns/dossier_clone_concern.rb index e147c9a20d8..84b9ed4818a 100644 --- a/app/models/concerns/dossier_clone_concern.rb +++ b/app/models/concerns/dossier_clone_concern.rb @@ -38,15 +38,15 @@ def editing_fork? end def forked_with_changes? - if forked_diff.present? - forked_diff.values.any?(&:present?) || forked_groupe_instructeur_changed? - end + return false if forked_diff.blank? + + forked_diff.values.any?(&:present?) || forked_groupe_instructeur_changed? end def champ_forked_with_changes?(champ) - if forked_diff.present? - forked_diff.values.any? { |champs| champs.any? { _1.public_id == champ.public_id } } - end + return false if forked_diff.blank? + + forked_diff.values.any? { |champs| champs.any? { _1.public_id == champ.public_id } } end def make_diff(editing_fork) From ac2ec95ccd1101a54f7f2dd7a6c12532d59be580 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Tue, 10 Dec 2024 16:09:09 +0100 Subject: [PATCH 3/6] style(edit footer): fix ineligibilite link wrapping on 2 lines --- app/assets/stylesheets/autosave.scss | 4 ---- app/assets/stylesheets/dossier_edit.scss | 5 ++++- .../autosave_footer_component.html.haml | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/autosave.scss b/app/assets/stylesheets/autosave.scss index 386bedae612..11529620a8f 100644 --- a/app/assets/stylesheets/autosave.scss +++ b/app/assets/stylesheets/autosave.scss @@ -15,10 +15,6 @@ margin-right: 6px; } -.autosave-more-infos { - white-space: nowrap; -} - .autosave-status { // Position the status over the explanation text position: absolute; diff --git a/app/assets/stylesheets/dossier_edit.scss b/app/assets/stylesheets/dossier_edit.scss index 1d4459af897..8225c56470a 100644 --- a/app/assets/stylesheets/dossier_edit.scss +++ b/app/assets/stylesheets/dossier_edit.scss @@ -75,7 +75,6 @@ $dossier-actions-bar-border-width: 1px; border: $dossier-actions-bar-border-width solid #cccccc; border-top-left-radius: 5px; border-top-right-radius: 5px; - border-bottom: none; z-index: 10; // above DSFR btn which are at 1 } @@ -113,4 +112,8 @@ $dossier-actions-bar-border-width: 1px; // to ensure the failed state has room to display its content. flex-grow: 1; } + + a { + white-space: nowrap; + } } diff --git a/app/components/dossiers/autosave_footer_component/autosave_footer_component.html.haml b/app/components/dossiers/autosave_footer_component/autosave_footer_component.html.haml index 879e3a349fe..7cf693d8683 100644 --- a/app/components/dossiers/autosave_footer_component/autosave_footer_component.html.haml +++ b/app/components/dossiers/autosave_footer_component/autosave_footer_component.html.haml @@ -10,7 +10,7 @@ - else = t('.brouillon.explanation') - if !annotation? - = link_to t('.more_information'), t("links.common.faq.autosave_url"), class: 'autosave-more-infos fr-link fr-link--sm', **external_link_attributes + = link_to t('.more_information'), t("links.common.faq.autosave_url"), class: 'fr-link fr-link--sm', **external_link_attributes %p.autosave-status.succeeded.fr-mb-0 = dsfr_icon('fr-icon-checkbox-circle-fill fr-text-default--success autosave-icon') @@ -22,7 +22,7 @@ - else = t('.brouillon.confirmation') - if !annotation? - = link_to t('.more_information'), t("links.common.faq.autosave_url"), class: 'autosave-more-infos fr-link fr-link--sm', **external_link_attributes + = link_to t('.more_information'), t("links.common.faq.autosave_url"), class: 'fr-link fr-link--sm', **external_link_attributes %p.autosave-status.failed.fr-mb-0 %span.autosave-icon ⚠️ From 5c1e07bfb0f952825e2c3317658303943f5f51cb Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Tue, 10 Dec 2024 16:59:56 +0100 Subject: [PATCH 4/6] refactor(admin): ineligibilite submit feedback --- .../pending_republish_component.html.haml | 2 +- .../administrateurs/ineligibilite_rules_controller.rb | 3 ++- .../administrateurs/ineligibilite_rules_controller_spec.rb | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/components/procedure/pending_republish_component/pending_republish_component.html.haml b/app/components/procedure/pending_republish_component/pending_republish_component.html.haml index eab7f62fc87..276955ce1db 100644 --- a/app/components/procedure/pending_republish_component/pending_republish_component.html.haml +++ b/app/components/procedure/pending_republish_component/pending_republish_component.html.haml @@ -1,3 +1,3 @@ -= render Dsfr::AlertComponent.new(state: :warning) do |c| += render Dsfr::AlertComponent.new(state: :warning, extra_class_names: "fr-mb-3w") do |c| - c.with_body do = t('.pending_republish_html', href: admin_procedure_path(@procedure.id)) diff --git a/app/controllers/administrateurs/ineligibilite_rules_controller.rb b/app/controllers/administrateurs/ineligibilite_rules_controller.rb index b21e6a2c20e..50264ff8ec0 100644 --- a/app/controllers/administrateurs/ineligibilite_rules_controller.rb +++ b/app/controllers/administrateurs/ineligibilite_rules_controller.rb @@ -11,7 +11,8 @@ def change draft_revision.assign_attributes(procedure_revision_params) if draft_revision.validate(:ineligibilite_rules_editor) && draft_revision.save - redirect_to edit_admin_procedure_ineligibilite_rules_path(@procedure) + flash[:notice] = "Les conditions d‘inéligibilité ont été modifiées." + redirect_to admin_procedure_path(@procedure) else flash[:alert] = draft_revision.errors.full_messages render :edit diff --git a/spec/controllers/administrateurs/ineligibilite_rules_controller_spec.rb b/spec/controllers/administrateurs/ineligibilite_rules_controller_spec.rb index 92ab88497d0..f1125384beb 100644 --- a/spec/controllers/administrateurs/ineligibilite_rules_controller_spec.rb +++ b/spec/controllers/administrateurs/ineligibilite_rules_controller_spec.rb @@ -243,7 +243,8 @@ draft_revision = procedure.reload.draft_revision expect(draft_revision.ineligibilite_message).to eq('panpan') expect(draft_revision.ineligibilite_enabled).to eq(true) - expect(response).to redirect_to(edit_admin_procedure_ineligibilite_rules_path(procedure)) + expect(response).to redirect_to(admin_procedure_path(procedure)) + expect(flash.notice).not_to be_empty end end end From 98f965f5a57d70bc4630f0ebf2c0d5b6731dbd54 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Tue, 10 Dec 2024 19:13:01 +0100 Subject: [PATCH 5/6] fix(dossier): show ineligibilite message on update --- .../invalid_ineligibilite_rules_component.rb | 10 ++++--- ...id_ineligibilite_rules_component.html.haml | 3 ++- app/controllers/users/dossiers_controller.rb | 4 +-- .../ineligibilite_rules_match_controller.ts | 20 +++++++++++++- app/models/dossier.rb | 12 --------- .../users/dossiers/update.turbo_stream.haml | 7 ++--- .../users/dossiers_controller_spec.rb | 26 +++++++++++++------ .../users/dossier_ineligibilite_spec.rb | 1 + .../shared/dossiers/_edit.html.haml_spec.rb | 2 ++ 9 files changed, 52 insertions(+), 33 deletions(-) diff --git a/app/components/dossiers/invalid_ineligibilite_rules_component.rb b/app/components/dossiers/invalid_ineligibilite_rules_component.rb index 78047042fd6..3e803fe972f 100644 --- a/app/components/dossiers/invalid_ineligibilite_rules_component.rb +++ b/app/components/dossiers/invalid_ineligibilite_rules_component.rb @@ -1,18 +1,22 @@ # frozen_string_literal: true class Dossiers::InvalidIneligibiliteRulesComponent < ApplicationComponent - delegate :can_passer_en_construction?, to: :@dossier + delegate :can_passer_en_construction?, to: :dossier def initialize(dossier:) @dossier = dossier @revision = dossier.revision end + private + + attr_reader :dossier + def render? - !can_passer_en_construction? + dossier.revision.ineligibilite_enabled? end def error_message - @dossier.revision.ineligibilite_message + dossier.revision.ineligibilite_message end end diff --git a/app/components/dossiers/invalid_ineligibilite_rules_component/invalid_ineligibilite_rules_component.html.haml b/app/components/dossiers/invalid_ineligibilite_rules_component/invalid_ineligibilite_rules_component.html.haml index dd39925cd6d..1ce8a16db3c 100644 --- a/app/components/dossiers/invalid_ineligibilite_rules_component/invalid_ineligibilite_rules_component.html.haml +++ b/app/components/dossiers/invalid_ineligibilite_rules_component/invalid_ineligibilite_rules_component.html.haml @@ -1,4 +1,5 @@ -%div{ id: dom_id(@dossier, :ineligibilite_rules_broken), data: { controller: 'ineligibilite-rules-match', turbo_force: :server } } +%div{ id: dom_id(@dossier, :ineligibilite_rules_broken), data: { controller: 'ineligibilite-rules-match', + ineligibilite_rules_match_open_value: (!can_passer_en_construction?).to_s } } %button.fr-sr-only{ aria: {controls: 'modal-eligibilite-rules-dialog' }, data: {'fr-opened': "false" } } show modal diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 626cb0692cd..00160b98458 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -280,9 +280,7 @@ def submit_en_construction def update @dossier = dossier.en_construction? ? dossier.find_editing_fork(dossier.user) : dossier @dossier = dossier_with_champs(pj_template: false) - @can_passer_en_construction_was, @can_passer_en_construction_is = dossier.track_can_passer_en_construction do - update_dossier_and_compute_errors - end + update_dossier_and_compute_errors respond_to do |format| format.turbo_stream do diff --git a/app/javascript/controllers/ineligibilite_rules_match_controller.ts b/app/javascript/controllers/ineligibilite_rules_match_controller.ts index 5b47d79b505..53614585f6e 100644 --- a/app/javascript/controllers/ineligibilite_rules_match_controller.ts +++ b/app/javascript/controllers/ineligibilite_rules_match_controller.ts @@ -10,10 +10,28 @@ declare const window: Window & export class InvalidIneligibiliteRulesController extends ApplicationController { static targets = ['dialog']; + static values = { + open: String + }; declare dialogTarget: HTMLElement; + declare openValue: 'true' | 'false'; connect() { - setTimeout(() => window.dsfr(this.dialogTarget).modal.disclose(), 100); + if (this.openValue == 'true') { + this.openModal(); + } + } + + openValueChanged() { + if (this.openValue == 'true') { + this.openModal(); + } + } + + private openModal() { + setTimeout(() => { + window.dsfr(this.dialogTarget).modal.disclose(); + }, 100); } } diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 19770296337..cad644dfdc7 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -1025,18 +1025,6 @@ def termine_and_accuse_lecture? procedure.accuse_lecture? && termine? end - def track_can_passer_en_construction - if !revision.ineligibilite_enabled - yield - [true, true] # without eligibilite rules, we never reach dossier.champs.visible?, don't cache anything - else - from = can_passer_en_construction? # with eligibilite rules, self.champ[x].visible is cached by passing thru conditions checks - yield - champs.map(&:reset_visible) # we must reset self.champs[x].visible?, because an update occurred and we should re-evaluate champs[x] visibility - [from, can_passer_en_construction?] - end - end - private def build_default_champs diff --git a/app/views/users/dossiers/update.turbo_stream.haml b/app/views/users/dossiers/update.turbo_stream.haml index 2cd14be476c..dbaceb9c72a 100644 --- a/app/views/users/dossiers/update.turbo_stream.haml +++ b/app/views/users/dossiers/update.turbo_stream.haml @@ -1,10 +1,7 @@ = render partial: 'shared/dossiers/update_champs', locals: { to_show: @to_show, to_hide: @to_hide, to_update: @to_update, dossier: @dossier } -- if !params.key?(:validate) - - if @can_passer_en_construction_was && !@can_passer_en_construction_is - = turbo_stream.append('contenu', render(Dossiers::InvalidIneligibiliteRulesComponent.new(dossier: @dossier))) - - else @ineligibilite_rules_is_computable - = turbo_stream.remove(dom_id(@dossier, :ineligibilite_rules_broken)) +- if params[:validate].present? && @dossier.revision.ineligibilite_enabled? + = turbo_stream.update dom_id(@dossier, :ineligibilite_rules_broken), render(Dossiers::InvalidIneligibiliteRulesComponent.new(dossier: @dossier)) - if @update_contact_information = turbo_stream.update "contact_information", partial: 'shared/dossiers/update_contact_information', locals: { dossier: @dossier, procedure: @dossier.procedure } diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 11ce0e20fe5..a1880f73b9e 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -775,9 +775,11 @@ let(:types_de_champ_public) { [{ type: :text }, { type: :integer_number }] } let(:text_champ) { dossier.project_champs_public.first } let(:number_champ) { dossier.project_champs_public.last } + let(:validate) { "true" } let(:submit_payload) do { id: dossier.id, + validate:, dossier: { groupe_instructeur_id: dossier.groupe_instructeur_id, champs_public_attributes: { @@ -805,28 +807,36 @@ end render_views - context 'when it switches from true to false' do + context 'when it becomes invalid' do let(:value) { must_be_greater_than + 1 } it 'raises popup' do subject dossier.reload expect(dossier.can_passer_en_construction?).to be_falsey - expect(assigns(:can_passer_en_construction_was)).to eq(true) - expect(assigns(:can_passer_en_construction_is)).to eq(false) - expect(response.body).to match(ActionView::RecordIdentifier.dom_id(dossier, :ineligibilite_rules_broken)) + expect(response.body).to include("data-ineligibilite-rules-match-open-value='true'") end end - context 'when it stays true' do + context 'when it says valid' do let(:value) { must_be_greater_than - 1 } it 'does nothing' do subject dossier.reload expect(dossier.can_passer_en_construction?).to be_truthy - expect(assigns(:can_passer_en_construction_was)).to eq(true) - expect(assigns(:can_passer_en_construction_is)).to eq(true) - expect(response.body).not_to have_selector("##{ActionView::RecordIdentifier.dom_id(dossier, :ineligibilite_rules_broken)}") + expect(response.body).to include("data-ineligibilite-rules-match-open-value='false'") + end + end + + context 'when not validating' do + let(:validate) { nil } + let(:value) { must_be_greater_than + 1 } + + it 'does not render invalid ineligible modal' do + subject + dossier.reload + expect(dossier.can_passer_en_construction?).to be_falsey + expect(response.body).not_to include("data-ineligibilite-rules-match-open-value") end end end diff --git a/spec/system/users/dossier_ineligibilite_spec.rb b/spec/system/users/dossier_ineligibilite_spec.rb index ef52ae42c4c..eafcc22a289 100644 --- a/spec/system/users/dossier_ineligibilite_spec.rb +++ b/spec/system/users/dossier_ineligibilite_spec.rb @@ -28,6 +28,7 @@ visit brouillon_dossier_path(dossier) # no error while dossier is empty expect(page).to have_selector(:button, text: "Déposer le dossier", disabled: false) + # TODO: modal message is considered as visible but hidden by modal JS expect(page).not_to have_content("Vous ne pouvez pas déposer votre dossier") # does raise error when dossier is filled with condition that does not match diff --git a/spec/views/shared/dossiers/_edit.html.haml_spec.rb b/spec/views/shared/dossiers/_edit.html.haml_spec.rb index 4271d129b28..f59c5a28c20 100644 --- a/spec/views/shared/dossiers/_edit.html.haml_spec.rb +++ b/spec/views/shared/dossiers/_edit.html.haml_spec.rb @@ -160,10 +160,12 @@ before do allow(dossier).to receive(:can_passer_en_construction?).and_return(false) + allow(dossier.revision).to receive(:ineligibilite_enabled?).and_return(true) end it 'renders broken transitions rules dialog' do expect(subject).to have_selector("##{ActionView::RecordIdentifier.dom_id(dossier, :ineligibilite_rules_broken)}") + expect(subject).to include("data-ineligibilite-rules-match-open-value='true'") end end end From dca46ff9e6d326de9bca42aac74371918c192cf9 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Wed, 11 Dec 2024 15:20:59 +0100 Subject: [PATCH 6/6] fix(autosave): validate select onChangeable et on retry --- app/javascript/controllers/autosave_controller.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/javascript/controllers/autosave_controller.ts b/app/javascript/controllers/autosave_controller.ts index 96aca094d19..c7b84485093 100644 --- a/app/javascript/controllers/autosave_controller.ts +++ b/app/javascript/controllers/autosave_controller.ts @@ -80,11 +80,13 @@ export class AutosaveController extends ApplicationController { // Wait next tick so champs having JS can interact // with form elements before extracting form data. setTimeout(() => { - this.enqueueAutosaveRequest(); + this.enqueueAutosaveWithValidationRequest(); this.showConditionnalSpinner(target); }, 0); }, - inputable: (target) => this.enqueueOnInput(target, true), + inputable: (target) => { + this.enqueueOnInput(target, true); + }, hidden: (target) => { // In comboboxes we dispatch a "change" event on hidden inputs to trigger autosave. // We want to debounce them. @@ -152,7 +154,8 @@ export class AutosaveController extends ApplicationController { private didRequestRetry() { if (this.#needsRetry) { - this.enqueueAutosaveRequest(); + console.log('didRequestRetry'); + this.enqueueAutosaveWithValidationRequest(); } }