From ef3ca9839ba0ec99876967ce78d3254ec89ec34e Mon Sep 17 00:00:00 2001 From: mfo Date: Mon, 3 Jun 2024 07:44:27 +0200 Subject: [PATCH] feat(procedure.validation): extract validation context: types_de_champ_public_editor, types_de_champ_private_editor and publication [combining both contextes]. validate conditions, headers_sections, regexp on type_de_champ_private too. dry validation --- .../procedure/card/champs_component.rb | 5 +- app/components/procedure/errors_summary.rb | 48 +++++++++++ .../errors_summary/errors_summary.html.haml | 9 +++ .../publication_warning_component.rb | 39 --------- .../publication_warning_component.html.haml | 7 -- .../champ_component/champ_component.html.haml | 2 +- .../types_de_champ_editor/editor_component.rb | 8 ++ .../editor_component.html.haml | 2 +- .../types_de_champ_editor/errors_summary.rb | 38 --------- .../errors_summary/errors_summary.fr.yml | 12 --- .../errors_summary/errors_summary.html.haml | 15 ---- .../header_section_component.rb | 2 +- .../header_section_component.html.haml | 2 +- app/models/procedure.rb | 10 ++- app/models/procedure_revision.rb | 48 ----------- app/models/type_de_champ.rb | 8 +- .../types_de_champ/condition_validator.rb | 21 +++++ .../expression_reguliere_validator.rb | 11 +++ .../header_section_consistency_validator.rb | 29 +++++++ .../no_empty_block_validator.rb | 3 +- .../no_empty_drop_down_validator.rb | 3 +- .../conditions/_update.turbo_stream.haml | 2 +- .../procedures/_publication_form.html.haml | 2 +- .../procedures/champs.html.haml | 2 +- .../administrateurs/procedures/show.html.haml | 13 ++- .../types_de_champ/_insert.turbo_stream.haml | 4 +- config/locales/fr.yml | 2 - config/locales/models/procedure/en.yml | 22 +++++ config/locales/models/procedure/fr.yml | 22 +++++ config/locales/models/type_de_champ/fr.yml | 2 +- .../card/annotations_component_spec.rb | 30 +++++++ .../procedures/card/champs_component_spec.rb | 30 +++++++ .../procedures/errors_summary_spec.rb | 80 +++++++++++++++++++ .../editor_component_spec.rb | 26 ++++++ spec/models/procedure_revision_spec.rb | 35 ++++---- .../administrateurs/procedure_publish_spec.rb | 21 ++--- .../administrateurs/types_de_champ_spec.rb | 4 +- 37 files changed, 398 insertions(+), 221 deletions(-) create mode 100644 app/components/procedure/errors_summary.rb create mode 100644 app/components/procedure/errors_summary/errors_summary.html.haml delete mode 100644 app/components/procedure/publication_warning_component.rb delete mode 100644 app/components/procedure/publication_warning_component/publication_warning_component.html.haml delete mode 100644 app/components/types_de_champ_editor/errors_summary.rb delete mode 100644 app/components/types_de_champ_editor/errors_summary/errors_summary.fr.yml delete mode 100644 app/components/types_de_champ_editor/errors_summary/errors_summary.html.haml create mode 100644 app/validators/types_de_champ/condition_validator.rb create mode 100644 app/validators/types_de_champ/expression_reguliere_validator.rb create mode 100644 app/validators/types_de_champ/header_section_consistency_validator.rb create mode 100644 spec/components/procedures/card/annotations_component_spec.rb create mode 100644 spec/components/procedures/card/champs_component_spec.rb create mode 100644 spec/components/procedures/errors_summary_spec.rb create mode 100644 spec/components/types_de_champ_editor/editor_component_spec.rb diff --git a/app/components/procedure/card/champs_component.rb b/app/components/procedure/card/champs_component.rb index d45b2f666..38604c831 100644 --- a/app/components/procedure/card/champs_component.rb +++ b/app/components/procedure/card/champs_component.rb @@ -7,9 +7,6 @@ class Procedure::Card::ChampsComponent < ApplicationComponent private def error_messages - [ - @procedure.errors.messages_for(:draft_types_de_champ_public), - @procedure.errors.messages_for(:draft_revision) - ].flatten.to_sentence + @procedure.errors.messages_for(:draft_types_de_champ_public).to_sentence end end diff --git a/app/components/procedure/errors_summary.rb b/app/components/procedure/errors_summary.rb new file mode 100644 index 000000000..adce8fd64 --- /dev/null +++ b/app/components/procedure/errors_summary.rb @@ -0,0 +1,48 @@ +class Procedure::ErrorsSummary < ApplicationComponent + def initialize(procedure:, validation_context:) + @procedure = procedure + @validation_context = validation_context + end + + def title + case @validation_context + when :types_de_champ_private_editor + "Les annotations privées contiennent des erreurs" + when :types_de_champ_public_editor + "Les champs formulaire contiennent des erreurs" + when :publication + if @procedure.publiee? + "Des problèmes empêchent la publication des modifications" + else + "Des problèmes empêchent la publication de la démarche" + end + end + end + + def invalid? + @procedure.validate(@validation_context) + @procedure.errors.present? + end + + def error_messages + @procedure.errors.map do |error| + [error, error_correction_page(error)] + end + end + + def error_correction_page(error) + case error.attribute + when :draft_types_de_champ_public + tdc = error.options[:type_de_champ] + champs_admin_procedure_path(@procedure, anchor: dom_id(tdc.stable_self, :editor_error)) + when :draft_types_de_champ_private + tdc = error.options[:type_de_champ] + annotations_admin_procedure_path(@procedure, anchor: dom_id(tdc.stable_self, :editor_error)) + when :attestation_template + edit_admin_procedure_attestation_template_path(@procedure) + when :initiated_mail, :received_mail, :closed_mail, :refused_mail, :without_continuation_mail, :re_instructed_mail + klass = "Mails::#{error.attribute.to_s.classify}".constantize + edit_admin_procedure_mail_template_path(@procedure, klass.const_get(:SLUG)) + end + end +end diff --git a/app/components/procedure/errors_summary/errors_summary.html.haml b/app/components/procedure/errors_summary/errors_summary.html.haml new file mode 100644 index 000000000..72780bcd1 --- /dev/null +++ b/app/components/procedure/errors_summary/errors_summary.html.haml @@ -0,0 +1,9 @@ +#errors-summary + - if invalid? + = render Dsfr::AlertComponent.new(state: :error, title: , extra_class_names: 'fr-mb-2w') do |c| + - c.with_body do + - error_messages.each do |(error, path)| + %p.mt-2 + = error.full_message + - if path.present? + = "(#{link_to 'corriger', path, class: 'fr-link'})" diff --git a/app/components/procedure/publication_warning_component.rb b/app/components/procedure/publication_warning_component.rb deleted file mode 100644 index 7af808c3d..000000000 --- a/app/components/procedure/publication_warning_component.rb +++ /dev/null @@ -1,39 +0,0 @@ -class Procedure::PublicationWarningComponent < ApplicationComponent - def initialize(procedure:) - @procedure = procedure - end - - def title - return "Des problèmes empêchent la publication des modifications" if @procedure.publiee? - - "Des problèmes empêchent la publication de la démarche" - end - - private - - def render? - @procedure.validate(:publication) - @procedure.errors.delete(:path) - @procedure.errors.any? - end - - def error_messages - @procedure.errors - .to_hash(full_messages: true) - .map do |attribute, messages| - [messages, error_correction_page(attribute)] - end - end - - def error_correction_page(attribute) - case attribute - when :draft_revision - champs_admin_procedure_path(@procedure) - when :attestation_template - edit_admin_procedure_attestation_template_path(@procedure) - when :initiated_mail, :received_mail, :closed_mail, :refused_mail, :without_continuation_mail, :re_instructed_mail - klass = "Mails::#{attribute.to_s.classify}".constantize - edit_admin_procedure_mail_template_path(@procedure, klass.const_get(:SLUG)) - end - end -end diff --git a/app/components/procedure/publication_warning_component/publication_warning_component.html.haml b/app/components/procedure/publication_warning_component/publication_warning_component.html.haml deleted file mode 100644 index 7d8ff43b7..000000000 --- a/app/components/procedure/publication_warning_component/publication_warning_component.html.haml +++ /dev/null @@ -1,7 +0,0 @@ -= render Dsfr::AlertComponent.new(state: :warning, title:) do |c| - - c.with_body do - - error_messages.each do |(messages, path)| - %p.mt-2 - = messages.to_sentence - - if path.present? - = "(#{link_to 'corriger', path, class: 'fr-link'})" diff --git a/app/components/types_de_champ_editor/champ_component/champ_component.html.haml b/app/components/types_de_champ_editor/champ_component/champ_component.html.haml index 980bd064c..60cf82102 100644 --- a/app/components/types_de_champ_editor/champ_component/champ_component.html.haml +++ b/app/components/types_de_champ_editor/champ_component/champ_component.html.haml @@ -1,5 +1,5 @@ %li.type-de-champ.flex.column.justify-start.fr-mb-5v{ html_options } - .type-de-champ-container + .type-de-champ-container{ id: dom_id(type_de_champ.stable_self, :editor_error) } - if @errors.present? .types-de-champ-errors = @errors diff --git a/app/components/types_de_champ_editor/editor_component.rb b/app/components/types_de_champ_editor/editor_component.rb index 0ea2c76cb..288a2c668 100644 --- a/app/components/types_de_champ_editor/editor_component.rb +++ b/app/components/types_de_champ_editor/editor_component.rb @@ -17,4 +17,12 @@ class TypesDeChampEditor::EditorComponent < ApplicationComponent @revision.revision_types_de_champ_public end end + + def validation_context + if annotations? + :types_de_champ_private_editor + else + :types_de_champ_public_editor + end + end end diff --git a/app/components/types_de_champ_editor/editor_component/editor_component.html.haml b/app/components/types_de_champ_editor/editor_component/editor_component.html.haml index 71df85e46..d161d0ba2 100644 --- a/app/components/types_de_champ_editor/editor_component/editor_component.html.haml +++ b/app/components/types_de_champ_editor/editor_component/editor_component.html.haml @@ -1,6 +1,6 @@ .fr-pb-12w{ 'data-turbo': 'true', id: dom_id(@revision, :types_de_champ_editor) } .types-de-champ-editor.editor-root - = render TypesDeChampEditor::ErrorsSummary.new(revision: @revision) + = render Procedure::ErrorsSummary.new(procedure: @revision.procedure, validation_context:) = render TypesDeChampEditor::BlockComponent.new(block: @revision, coordinates: coordinates) #empty-coordinates{ hidden: coordinates.present? } = render TypesDeChampEditor::AddChampButtonComponent.new(revision: @revision, is_annotation: annotations?) diff --git a/app/components/types_de_champ_editor/errors_summary.rb b/app/components/types_de_champ_editor/errors_summary.rb deleted file mode 100644 index b5367eaf1..000000000 --- a/app/components/types_de_champ_editor/errors_summary.rb +++ /dev/null @@ -1,38 +0,0 @@ -class TypesDeChampEditor::ErrorsSummary < ApplicationComponent - def initialize(revision:) - @revision = revision - end - - def invalid? - @revision.invalid? - end - - def condition_errors? - @revision.errors.include?(:condition) - end - - def header_section_errors? - @revision.errors.include?(:header_section) - end - - def expression_reguliere_errors? - @revision.errors.include?(:expression_reguliere) - end - - private - - def errors_for(key) - @revision.errors.filter { _1.attribute == key } - end - - def error_message_for(key) - errors_for(key) - .map { |error| error.options[:type_de_champ] } - .map { |tdc| tag.li(tdc_anchor(tdc, key)) } - .then { |lis| tag.ul(lis.reduce(&:+)) } - end - - def tdc_anchor(tdc, key) - tag.a(tdc.libelle, href: champs_admin_procedure_path(@revision.procedure_id, anchor: dom_id(tdc.stable_self, key)), data: { turbo: false }) - end -end diff --git a/app/components/types_de_champ_editor/errors_summary/errors_summary.fr.yml b/app/components/types_de_champ_editor/errors_summary/errors_summary.fr.yml deleted file mode 100644 index 08d594931..000000000 --- a/app/components/types_de_champ_editor/errors_summary/errors_summary.fr.yml +++ /dev/null @@ -1,12 +0,0 @@ -fr: - fix_conditional: - one: 'La logique conditionnelle du champ suivant est invalide, veuillez la corriger :' - other: 'La logique conditionnelle des champs suivants sont invalides, veuillez les corriger :' - - fix_header_section: - one: 'Le titre de section suivant est invalide, veuillez le corriger :' - other: 'Les titres de section suivants sont invalides, veuillez les corriger :' - - fix_expressions_regulieres: - one: "L'expression régulière suivante est invalide, veuillez la corriger :" - other: 'Les expressions régulières suivantes sont invalides, veuillez les corriger :' diff --git a/app/components/types_de_champ_editor/errors_summary/errors_summary.html.haml b/app/components/types_de_champ_editor/errors_summary/errors_summary.html.haml deleted file mode 100644 index 46f599ce5..000000000 --- a/app/components/types_de_champ_editor/errors_summary/errors_summary.html.haml +++ /dev/null @@ -1,15 +0,0 @@ -#errors-summary - - if invalid? - = render Dsfr::AlertComponent.new(state: :warning, title: "Le formulaire contient des erreurs", extra_class_names: 'fr-mb-2w') do |c| - - c.with_body do - - if condition_errors? - %p= t('.fix_conditional', count: errors_for(:condition).size) - = error_message_for(:condition) - - - if header_section_errors? - %p= t('.fix_header_section', count: errors_for(:header_section).size) - = error_message_for(:header_section) - - - if expression_reguliere_errors? - %p= t('.fix_expressions_regulieres', count: errors_for(:expression_reguliere).size) - = error_message_for(:expression_reguliere) diff --git a/app/components/types_de_champ_editor/header_section_component.rb b/app/components/types_de_champ_editor/header_section_component.rb index 80271b8e1..39f372dd8 100644 --- a/app/components/types_de_champ_editor/header_section_component.rb +++ b/app/components/types_de_champ_editor/header_section_component.rb @@ -31,7 +31,7 @@ class TypesDeChampEditor::HeaderSectionComponent < ApplicationComponent end def errors? - !errors.empty? + errors.present? end def to_html_list(messages) diff --git a/app/components/types_de_champ_editor/header_section_component/header_section_component.html.haml b/app/components/types_de_champ_editor/header_section_component/header_section_component.html.haml index b40967b26..022c2830b 100644 --- a/app/components/types_de_champ_editor/header_section_component/header_section_component.html.haml +++ b/app/components/types_de_champ_editor/header_section_component/header_section_component.html.haml @@ -1,5 +1,5 @@ %div{ id: dom_id(@tdc.stable_self, :header_section) } - if errors? - .errors-summary= to_html_list(errors) + .errors-summary= errors = @form.label :header_section_level, "Niveau du titre", for: dom_id(@tdc, :header_section_level) = @form.select :header_section_level, header_section_options_for_select, {}, id: dom_id(@tdc, :header_section_level), class: 'fr-select width-33' diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 11cec13d8..af398a991 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -259,13 +259,19 @@ class Procedure < ApplicationRecord validates :lien_dpo, url: { no_local: true, allow_blank: true, accept_email: true } validates :draft_types_de_champ_public, + 'types_de_champ/condition': true, + 'types_de_champ/expression_reguliere': true, + 'types_de_champ/header_section_consistency': true, 'types_de_champ/no_empty_block': true, 'types_de_champ/no_empty_drop_down': true, - on: :publication + on: [:types_de_champ_public_editor, :publication] + validates :draft_types_de_champ_private, + 'types_de_champ/condition': true, + 'types_de_champ/header_section_consistency': true, 'types_de_champ/no_empty_block': true, 'types_de_champ/no_empty_drop_down': true, - on: :publication + on: [:types_de_champ_private_editor, :publication] validate :check_juridique, on: [:create, :publication] diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb index c8daa1bec..fef3516d5 100644 --- a/app/models/procedure_revision.rb +++ b/app/models/procedure_revision.rb @@ -17,10 +17,6 @@ class ProcedureRevision < ApplicationRecord scope :ordered, -> { order(:created_at) } - validate :conditions_are_valid? - validate :header_sections_are_valid? - validate :expressions_regulieres_are_valid? - delegate :path, to: :procedure, prefix: true def build_champs_public @@ -453,48 +449,4 @@ class ProcedureRevision < ApplicationRecord coordinate.update!(type_de_champ: cloned_type_de_champ) cloned_type_de_champ end - - def conditions_are_valid? - public_tdcs = types_de_champ_public.to_a - .flat_map { _1.repetition? ? children_of(_1) : _1 } - - public_tdcs - .map.with_index - .filter_map { |tdc, i| tdc.condition? ? [tdc, i] : nil } - .map do |tdc, i| - [tdc, tdc.condition.errors(public_tdcs.take(i))] - end - .filter { |_tdc, errors| errors.present? } - .each { |tdc, message| errors.add(:condition, message, type_de_champ: tdc) } - end - - def header_sections_are_valid? - public_tdcs = types_de_champ_public.to_a - - root_tdcs_errors = errors_for_header_sections_order(public_tdcs) - repetition_tdcs_errors = public_tdcs - .filter_map { _1.repetition? ? children_of(_1) : nil } - .map { errors_for_header_sections_order(_1) } - - repetition_tdcs_errors + root_tdcs_errors - end - - def expressions_regulieres_are_valid? - types_de_champ_public.to_a - .flat_map { _1.repetition? ? children_of(_1) : _1 } - .each do |tdc| - if tdc.expression_reguliere? && tdc.invalid_regexp? - errors.add(:expression_reguliere, type_de_champ: tdc) - end - end - end - - def errors_for_header_sections_order(tdcs) - tdcs - .map.with_index - .filter_map { |tdc, i| tdc.header_section? ? [tdc, i] : nil } - .map { |tdc, i| [tdc, tdc.check_coherent_header_level(tdcs.take(i))] } - .filter { |_tdc, errors| errors.present? } - .each { |tdc, message| errors.add(:header_section, message, type_de_champ: tdc) } - end end diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index ea2de7468..10e415c05 100644 --- a/app/models/type_de_champ.rb +++ b/app/models/type_de_champ.rb @@ -505,15 +505,15 @@ class TypeDeChamp < ApplicationRecord end def check_coherent_header_level(upper_tdcs) - errs = [] previous_level = previous_section_level(upper_tdcs) - current_level = header_section_level_value.to_i + difference = current_level - previous_level if current_level > previous_level && difference != 1 - errs << I18n.t('activerecord.errors.type_de_champ.attributes.header_section_level.gap_error', level: current_level - previous_level - 1) + I18n.t('activerecord.errors.type_de_champ.attributes.header_section_level.gap_error', level: current_level - previous_level - 1) + else + nil end - errs end def current_section_level(revision) diff --git a/app/validators/types_de_champ/condition_validator.rb b/app/validators/types_de_champ/condition_validator.rb new file mode 100644 index 000000000..74b57c3d5 --- /dev/null +++ b/app/validators/types_de_champ/condition_validator.rb @@ -0,0 +1,21 @@ +class TypesDeChamp::ConditionValidator < ActiveModel::EachValidator + def validate_each(procedure, attribute, types_de_champ) + public_tdcs = types_de_champ.to_a + .flat_map { _1.repetition? ? procedure.draft_revision.children_of(_1) : _1 } + + public_tdcs + .map.with_index + .filter_map { |tdc, i| tdc.condition? ? [tdc, i] : nil } + .map do |tdc, i| + [tdc, tdc.condition.errors(public_tdcs.take(i))] + end + .filter { |_tdc, errors| errors.present? } + .each do |tdc, _error_hash| + procedure.errors.add( + attribute, + procedure.errors.generate_message(attribute, :invalid_condition, { value: tdc.libelle }), + type_de_champ: tdc + ) + end + end +end diff --git a/app/validators/types_de_champ/expression_reguliere_validator.rb b/app/validators/types_de_champ/expression_reguliere_validator.rb new file mode 100644 index 000000000..39262adeb --- /dev/null +++ b/app/validators/types_de_champ/expression_reguliere_validator.rb @@ -0,0 +1,11 @@ +class TypesDeChamp::ExpressionReguliereValidator < ActiveModel::EachValidator + def validate_each(procedure, attribute, types_de_champ) + types_de_champ.to_a + .flat_map { _1.repetition? ? procedure.draft_revision.children_of(_1) : _1 } + .each do |tdc| + if tdc.expression_reguliere? && tdc.invalid_regexp? + procedure.errors.add(:expression_reguliere, type_de_champ: tdc) + end + end + end +end diff --git a/app/validators/types_de_champ/header_section_consistency_validator.rb b/app/validators/types_de_champ/header_section_consistency_validator.rb new file mode 100644 index 000000000..062b27cf0 --- /dev/null +++ b/app/validators/types_de_champ/header_section_consistency_validator.rb @@ -0,0 +1,29 @@ +class TypesDeChamp::HeaderSectionConsistencyValidator < ActiveModel::EachValidator + def validate_each(procedure, attribute, types_de_champ) + public_tdcs = types_de_champ.to_a + + root_tdcs_errors = errors_for_header_sections_order(procedure, attribute, public_tdcs) + repetition_tdcs_errors = public_tdcs + .filter_map { _1.repetition? ? procedure.draft_revision.children_of(_1) : nil } + .map { errors_for_header_sections_order(procedure, attribute, _1) } + + repetition_tdcs_errors + root_tdcs_errors + end + + private + + def errors_for_header_sections_order(procedure, attribute, types_de_champ) + types_de_champ + .map.with_index + .filter_map { |tdc, i| tdc.header_section? ? [tdc, i] : nil } + .map { |tdc, i| [tdc, tdc.check_coherent_header_level(types_de_champ.take(i))] } + .filter { |_tdc, errors| errors.present? } + .each do |tdc, message| + procedure.errors.add( + attribute, + procedure.errors.generate_message(attribute, :inconsistent_header_section, { value: tdc.libelle, custom_message: message }), + type_de_champ: tdc + ) + end + end +end diff --git a/app/validators/types_de_champ/no_empty_block_validator.rb b/app/validators/types_de_champ/no_empty_block_validator.rb index e1ea17739..50356d6ab 100644 --- a/app/validators/types_de_champ/no_empty_block_validator.rb +++ b/app/validators/types_de_champ/no_empty_block_validator.rb @@ -11,7 +11,8 @@ class TypesDeChamp::NoEmptyBlockValidator < ActiveModel::EachValidator if procedure.draft_revision.children_of(parent).empty? procedure.errors.add( attribute, - procedure.errors.generate_message(attribute, :empty_repetition, { value: parent.libelle }) + procedure.errors.generate_message(attribute, :empty_repetition, { value: parent.libelle }), + type_de_champ: parent ) end end diff --git a/app/validators/types_de_champ/no_empty_drop_down_validator.rb b/app/validators/types_de_champ/no_empty_drop_down_validator.rb index bd8fa21dd..0be4e5406 100644 --- a/app/validators/types_de_champ/no_empty_drop_down_validator.rb +++ b/app/validators/types_de_champ/no_empty_drop_down_validator.rb @@ -11,7 +11,8 @@ class TypesDeChamp::NoEmptyDropDownValidator < ActiveModel::EachValidator if drop_down.drop_down_list_enabled_non_empty_options.empty? procedure.errors.add( attribute, - procedure.errors.generate_message(attribute, :empty_drop_down, { value: drop_down.libelle }) + procedure.errors.generate_message(attribute, :empty_drop_down, { value: drop_down.libelle }), + type_de_champ: drop_down ) end end diff --git a/app/views/administrateurs/conditions/_update.turbo_stream.haml b/app/views/administrateurs/conditions/_update.turbo_stream.haml index 3fba14ec8..028fb8c0e 100644 --- a/app/views/administrateurs/conditions/_update.turbo_stream.haml +++ b/app/views/administrateurs/conditions/_update.turbo_stream.haml @@ -4,7 +4,7 @@ ['Configuration des champs']], preview: @procedure.draft_revision.valid? }) -= turbo_stream.replace 'errors-summary', render(TypesDeChampEditor::ErrorsSummary.new(revision: @procedure.draft_revision)) += turbo_stream.replace 'errors-summary', render(Procedure::ErrorsSummary.new(procedure: @procedure, validation_context: @tdc.public? ? :types_de_champ_public_editor : :types_de_champ_private_editor)) - rendered = render @condition_component diff --git a/app/views/administrateurs/procedures/_publication_form.html.haml b/app/views/administrateurs/procedures/_publication_form.html.haml index 85a8f083c..0c9cc8454 100644 --- a/app/views/administrateurs/procedures/_publication_form.html.haml +++ b/app/views/administrateurs/procedures/_publication_form.html.haml @@ -2,7 +2,7 @@ url: admin_procedure_publish_path(procedure_id: procedure.id), method: :put, html: { class: 'form' } do |f| - = render Procedure::PublicationWarningComponent.new(procedure: procedure) + = render Procedure::ErrorsSummary.new(procedure: @procedure, validation_context: :publication) .mt-2 - if procedure.draft_changed? %p.mb-2= t('.draft_changed_procedure_alert') diff --git a/app/views/administrateurs/procedures/champs.html.haml b/app/views/administrateurs/procedures/champs.html.haml index 95ddf2860..4ec783ee1 100644 --- a/app/views/administrateurs/procedures/champs.html.haml +++ b/app/views/administrateurs/procedures/champs.html.haml @@ -9,7 +9,7 @@ .fr-grid-row = render partial: 'champs_summary' .fr-col - = render TypesDeChampEditor::EditorComponent.new(revision: @procedure.draft_revision) + = render TypesDeChampEditor::EditorComponent.new(revision: @procedure.draft_revision, is_annotation: false) .padded-fixed-footer .fixed-footer diff --git a/app/views/administrateurs/procedures/show.html.haml b/app/views/administrateurs/procedures/show.html.haml index a4c1f16d3..0bd65ebf8 100644 --- a/app/views/administrateurs/procedures/show.html.haml +++ b/app/views/administrateurs/procedures/show.html.haml @@ -5,7 +5,7 @@ .fr-container.procedure-admin-container %ul.fr-btns-group.fr-btns-group--inline-sm.fr-btns-group--icon-left - - if @procedure.draft_revision.valid? + - if @procedure.validate(:publication) - if !@procedure.brouillon? = link_to 'Télécharger', admin_procedure_archives_path(@procedure), class: 'fr-btn fr-btn--tertiary fr-btn--icon-left fr-icon-download-line', id: "archive-procedure" @@ -27,15 +27,11 @@ = link_to 'Clore', admin_procedure_close_path(procedure_id: @procedure.id), class: 'fr-btn fr-btn--tertiary fr-btn--icon-left fr-icon-calendar-close-fill', id: "close-procedure-link" .fr-container - = render TypesDeChampEditor::ErrorsSummary.new(revision: @procedure.draft_revision) - -- if @procedure.draft_changed? - .fr-container + - if @procedure.draft_changed? = render Dsfr::CalloutComponent.new(title: t(:has_changes, scope: [:administrateurs, :revision_changes]), icon: "fr-fi-information-line") do |c| - c.with_body do = render Procedure::RevisionChangesComponent.new changes: @procedure.revision_changes, previous_revision: @procedure.published_revision - - = render Procedure::PublicationWarningComponent.new(procedure: @procedure) + = render Procedure::ErrorsSummary.new(procedure: @procedure, validation_context: :publication) - c.with_bottom do %ul.fr-mt-2w.fr-btns-group.fr-btns-group--inline @@ -44,6 +40,9 @@ - else %li= button_to 'Publier les modifications', admin_procedure_publication_path(@procedure), class: 'fr-btn', id: 'publish-procedure-link', data: { disable_with: "Publication..." }, disabled: !@procedure.draft_revision.valid? || @procedure.errors.present?, method: :get %li= button_to "Réinitialiser les modifications", admin_procedure_reset_draft_path(@procedure), class: 'fr-btn fr-btn--secondary fr-mr-2w', data: { confirm: 'Êtes-vous sûr de vouloir réinitialiser les modifications ?' }, method: :put + - else + = render Procedure::ErrorsSummary.new(procedure: @procedure, validation_context: :publication) + - if !@procedure.procedure_expires_when_termine_enabled? = render partial: 'administrateurs/procedures/suggest_expires_when_termine', locals: { procedure: @procedure } diff --git a/app/views/administrateurs/types_de_champ/_insert.turbo_stream.haml b/app/views/administrateurs/types_de_champ/_insert.turbo_stream.haml index 9421b594a..ad121dedd 100644 --- a/app/views/administrateurs/types_de_champ/_insert.turbo_stream.haml +++ b/app/views/administrateurs/types_de_champ/_insert.turbo_stream.haml @@ -10,9 +10,9 @@ locals: { steps: [['Démarches', admin_procedures_path], [@procedure.libelle.truncate_words(10), admin_procedure_path(@procedure)], ['Configuration des champs']], - preview: @procedure.draft_revision.valid? }) + preview: @procedure.validate(@coordinate&.private? ? :types_de_champ_private_editor : :types_de_champ_public_editor) }) -= turbo_stream.replace 'errors-summary', render(TypesDeChampEditor::ErrorsSummary.new(revision: @procedure.draft_revision)) += turbo_stream.replace 'errors-summary', render(Procedure::ErrorsSummary.new(procedure: @procedure, validation_context: @coordinate&.private? ? :types_de_champ_private_editor : :types_de_champ_public_editor)) = turbo_stream.replace 'summary', render(partial: 'administrateurs/procedures/champs_summary') diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 619f5f4f4..07486886c 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -739,8 +739,6 @@ fr: evil_regexp: L'expression régulière que vous avez entrée est potentiellement dangereuse et pourrait entraîner des problèmes de performance mismatch_regexp: L'exemple doit correspondre à l'expression régulière fournie syntax_error_regexp: La syntaxe de l'expression régulière n'est pas valide - empty_repetition: '« %{value} » doit comporter au moins un champ répétable' - empty_drop_down: '« %{value} » doit comporter au moins un choix sélectionnable' # procedure_not_draft: "Cette démarche n’est maintenant plus en brouillon." cadastres_empty: one: "Aucune parcelle cadastrale sur la zone sélectionnée" diff --git a/config/locales/models/procedure/en.yml b/config/locales/models/procedure/en.yml index 2f1d1c8b8..55a9c1ddd 100644 --- a/config/locales/models/procedure/en.yml +++ b/config/locales/models/procedure/en.yml @@ -72,8 +72,30 @@ en: invalid: 'invalid format' draft_types_de_champ_public: format: 'Public field %{message}' + invalid_condition: "« %{value} » have an invalid logic" + empty_repetition: '« %{value} » requires at least one field' + empty_drop_down: '« %{value} » requires at least one option' + inconsistent_header_section: "« %{value} » %{custom_message}" draft_types_de_champ_private: format: 'Private field %{message}' + invalid_condition: "« %{value} » have an invalid logic" + empty_repetition: '« %{value} » requires at least one field' + empty_drop_down: '« %{value} » requires at least one option' + inconsistent_header_section: "« %{value} » %{custom_message}" + attestation_template: + format: "%{attribute} %{message}" + initiated_mail: + format: "%{attribute} %{message}" + received_mail: + format: "%{attribute} %{message}" + closed_mail: + format: "%{attribute} %{message}" + refused_mail: + format: "%{attribute} %{message}" + without_continuation_mail: + format: "%{attribute} %{message}" + re_instructed_mail: + format: "%{attribute} %{message}" lien_dpo: invalid_uri_or_email: "Fill in with an email or a link" sva_svr: diff --git a/config/locales/models/procedure/fr.yml b/config/locales/models/procedure/fr.yml index 78e2bcf26..4a0fdeca5 100644 --- a/config/locales/models/procedure/fr.yml +++ b/config/locales/models/procedure/fr.yml @@ -78,8 +78,30 @@ fr: invalid: 'n’a pas le bon format' draft_types_de_champ_public: format: 'Le champ %{message}' + invalid_condition: "« %{value} » a une logique conditionnelle invalide" + empty_repetition: '« %{value} » doit comporter au moins un champ répétable' + empty_drop_down: '« %{value} » doit comporter au moins un choix sélectionnable' + inconsistent_header_section: "« %{value} » %{custom_message}" draft_types_de_champ_private: format: 'L’annotation privée %{message}' + invalid_condition: "« %{value} » a une logique conditionnelle invalide" + empty_repetition: '« %{value} » doit comporter au moins un champ répétable' + empty_drop_down: '« %{value} » doit comporter au moins un choix sélectionnable' + inconsistent_header_section: "« %{value} » %{custom_message}" + attestation_template: + format: "%{attribute} %{message}" + initiated_mail: + format: "%{attribute} %{message}" + received_mail: + format: "%{attribute} %{message}" + closed_mail: + format: "%{attribute} %{message}" + refused_mail: + format: "%{attribute} %{message}" + without_continuation_mail: + format: "%{attribute} %{message}" + re_instructed_mail: + format: "%{attribute} %{message}" lien_dpo: invalid_uri_or_email: "Veuillez saisir un mail ou un lien" auto_archive_on: diff --git a/config/locales/models/type_de_champ/fr.yml b/config/locales/models/type_de_champ/fr.yml index a6ea09511..71de8c159 100644 --- a/config/locales/models/type_de_champ/fr.yml +++ b/config/locales/models/type_de_champ/fr.yml @@ -61,4 +61,4 @@ fr: type_de_champ: attributes: header_section_level: - gap_error: "Un titre de section avec le niveau %{level} est manquant." + gap_error: "devrait être précédé d'un titre de niveau %{level}" diff --git a/spec/components/procedures/card/annotations_component_spec.rb b/spec/components/procedures/card/annotations_component_spec.rb new file mode 100644 index 000000000..478937124 --- /dev/null +++ b/spec/components/procedures/card/annotations_component_spec.rb @@ -0,0 +1,30 @@ +describe Procedure::Card::AnnotationsComponent, type: :component do + describe 'render' do + let(:procedure) { create(:procedure, id: 1, types_de_champ_private:, types_de_champ_public:) } + let(:types_de_champ_private) { [] } + let(:types_de_champ_public) { [] } + before { procedure.validate(:publication) } + subject { render_inline(described_class.new(procedure: procedure)) } + + context 'when no errors' do + it 'does not render' do + expect(subject).to have_selector('.fr-badge--info', text: 'À configurer') + end + end + + context 'when errors on types_de_champs_public' do + let(:types_de_champ_public) { [{ type: :drop_down_list, options: [] }] } + it 'does not render' do + expect(subject).to have_selector('.fr-badge--info', text: 'À configurer') + end + end + + context 'when errors on types_de_champs_private' do + let(:types_de_champ_private) { [{ type: :drop_down_list, options: [] }] } + + it 'render the template' do + expect(subject).to have_selector('.fr-badge--error', text: 'À modifier') + end + end + end +end diff --git a/spec/components/procedures/card/champs_component_spec.rb b/spec/components/procedures/card/champs_component_spec.rb new file mode 100644 index 000000000..61a5d5762 --- /dev/null +++ b/spec/components/procedures/card/champs_component_spec.rb @@ -0,0 +1,30 @@ +describe Procedure::Card::ChampsComponent, type: :component do + describe 'render' do + let(:procedure) { create(:procedure, id: 1, types_de_champ_private:, types_de_champ_public:) } + let(:types_de_champ_private) { [] } + let(:types_de_champ_public) { [] } + before { procedure.validate(:publication) } + subject { render_inline(described_class.new(procedure: procedure)) } + + context 'when no errors' do + it 'does not render' do + expect(subject).to have_selector('.fr-badge--warning', text: 'À faire') + end + end + + context 'when errors on types_de_champs_public' do + let(:types_de_champ_public) { [{ type: :drop_down_list, options: [] }] } + it 'does not render' do + expect(subject).to have_selector('.fr-badge--error', text: 'À modifier') + end + end + + context 'when errors on types_de_champs_private' do + let(:types_de_champ_private) { [{ type: :drop_down_list, options: [] }] } + + it 'render the template' do + expect(subject).to have_selector('.fr-badge--warning', text: 'À faire') + end + end + end +end diff --git a/spec/components/procedures/errors_summary_spec.rb b/spec/components/procedures/errors_summary_spec.rb new file mode 100644 index 000000000..4c3ef6337 --- /dev/null +++ b/spec/components/procedures/errors_summary_spec.rb @@ -0,0 +1,80 @@ +describe Procedure::ErrorsSummary, type: :component do + subject { render_inline(described_class.new(procedure:, validation_context:)) } + + describe 'validations context' do + let(:procedure) { create(:procedure, types_de_champ_private:, types_de_champ_public:) } + let(:types_de_champ_private) { [{ type: :drop_down_list, options: [], libelle: 'private' }] } + let(:types_de_champ_public) { [{ type: :drop_down_list, options: [], libelle: 'public' }] } + + before { subject } + + context 'when :publication' do + let(:validation_context) { :publication } + + it 'shows errors for public and private tdc' do + expect(page).to have_text("Le champ « public » doit comporter au moins un choix sélectionnable") + expect(page).to have_text("L’annotation privée « private » doit comporter au moins un choix sélectionnable") + end + end + + context 'when :types_de_champ_public_editor' do + let(:validation_context) { :types_de_champ_public_editor } + + it 'shows errors for public only tdc' do + expect(page).to have_text("Le champ « public » doit comporter au moins un choix sélectionnable") + expect(page).not_to have_text("L’annotation privée « private » doit comporter au moins un choix sélectionnable") + end + end + + context 'when :types_de_champ_private_editor' do + let(:validation_context) { :types_de_champ_private_editor } + + it 'shows errors for private only tdc' do + expect(page).not_to have_text("Le champ « public » doit comporter au moins un choix sélectionnable") + expect(page).to have_text("L’annotation privée « private » doit comporter au moins un choix sélectionnable") + end + end + end + + describe 'render all kind of champs errors' do + include Logic + + let(:procedure) do + create(:procedure, id: 1, types_de_champ_public: [ + { libelle: 'repetition requires children', type: :repetition, children: [] }, + { libelle: 'drop down list requires options', type: :drop_down_list, options: [] }, + { libelle: 'invalid condition', type: :text, condition: ds_eq(constant(true), constant(1)) }, + { libelle: 'header sections must have consistent order', type: :header_section, level: 2 } + ]) + end + + let(:validation_context) { :types_de_champ_public_editor } + + before { subject } + + it 'renders all errors on champ' do + expect(page).to have_text("Le champ « drop down list requires options » doit comporter au moins un choix sélectionnable") + expect(page).to have_text("Le champ « repetition requires children » doit comporter au moins un champ répétable") + expect(page).to have_text("Le champ « invalid condition » a une logique conditionnelle invalide") + expect(page).to have_text("Le champ « header sections must have consistent order » devrait être précédé d'un titre de niveau 1") + # TODO, test attestation_template, initiated_mail, :received_mail, :closed_mail, :refused_mail, :without_continuation_mail, :re_instructed_mail + end + end + + describe 'render error for other kind of associated objects' do + let(:validation_context) { :publication } + let(:procedure) { create(:procedure, attestation_template:, initiated_mail:) } + let(:attestation_template) { build(:attestation_template) } + let(:initiated_mail) { build(:initiated_mail) } + + before do + [:attestation_template, :initiated_mail].map { procedure.send(_1).update_column(:body, '--invalidtag--') } + subject + end + + it 'render error nicely' do + expect(page).to have_text("Le modèle d’attestation n'est pas valide") + expect(page).to have_text("L’email de notification de passage de dossier en instruction n'est pas valide") + end + end +end diff --git a/spec/components/types_de_champ_editor/editor_component_spec.rb b/spec/components/types_de_champ_editor/editor_component_spec.rb new file mode 100644 index 000000000..7b4a19e46 --- /dev/null +++ b/spec/components/types_de_champ_editor/editor_component_spec.rb @@ -0,0 +1,26 @@ +describe TypesDeChampEditor::EditorComponent, type: :component do + let(:revision) { procedure.draft_revision } + let(:procedure) { create(:procedure, id: 1, types_de_champ_private:, types_de_champ_public:) } + + let(:types_de_champ_private) { [{ type: :drop_down_list, options: [], libelle: 'private' }] } + let(:types_de_champ_public) { [{ type: :drop_down_list, options: [], libelle: 'public' }] } + + describe 'render' do + subject { render_inline(described_class.new(revision:, is_annotation:)) } + context 'types_de_champ_public' do + let(:is_annotation) { false } + it 'does not render private champs errors' do + expect(subject).not_to have_text("« private » doit comporter au moins un choix sélectionnable") + expect(subject).to have_text("« public » doit comporter au moins un choix sélectionnable") + end + end + + context 'types_de_champ_private' do + let(:is_annotation) { true } + it 'does not render public champs errors' do + expect(subject).to have_text("« private » doit comporter au moins un choix sélectionnable") + expect(subject).not_to have_text("« public » doit comporter au moins un choix sélectionnable") + end + end + end +end diff --git a/spec/models/procedure_revision_spec.rb b/spec/models/procedure_revision_spec.rb index 9d3ea5924..4c7a17ba3 100644 --- a/spec/models/procedure_revision_spec.rb +++ b/spec/models/procedure_revision_spec.rb @@ -828,23 +828,22 @@ describe ProcedureRevision do describe 'conditions_are_valid' do include Logic - def first_champ = procedure.draft_revision.types_de_champ_public.first - - def second_champ = procedure.draft_revision.types_de_champ_public.second - - let(:procedure) do - create(:procedure).tap do |p| - p.draft_revision.add_type_de_champ(type_champ: :integer_number, libelle: 'l1') - p.draft_revision.add_type_de_champ(type_champ: :integer_number, libelle: 'l2') - end + let(:procedure) { create(:procedure, types_de_champ_public:) } + let(:types_de_champ_public) do + [ + { type: :integer_number, libelle: 'l1' }, + { type: :integer_number, libelle: 'l2' } + ] end + def first_champ = procedure.draft_revision.types_de_champ_public.first + def second_champ = procedure.draft_revision.types_de_champ_public.second let(:draft_revision) { procedure.draft_revision } let(:condition) { nil } subject do - draft_revision.save - draft_revision.errors + procedure.validate(:publication) + procedure.errors end context 'when a champ has a valid condition (type)' do @@ -865,7 +864,7 @@ describe ProcedureRevision do before { second_champ.update(condition: condition) } let(:condition) { ds_eq(constant(true), constant(1)) } - it { expect(subject.first.attribute).to eq(:condition) } + it { expect(subject.first.attribute).to eq(:draft_types_de_champ_public) } end context 'when a champ has an invalid condition: needed tdc is down in the forms' do @@ -876,7 +875,7 @@ describe ProcedureRevision do first_champ.update(condition: need_second_champ) end - it { expect(subject.first.attribute).to eq(:condition) } + it { expect(subject.first.attribute).to eq(:draft_types_de_champ_public) } end context 'with a repetition' do @@ -904,7 +903,7 @@ describe ProcedureRevision do context 'when a champ belongs to a repetition' do let(:condition) { ds_eq(champ_value(-1), constant(1)) } - it { expect(subject.first.attribute).to eq(:condition) } + it { expect(subject.first.attribute).to eq(:draft_types_de_champ_public) } end end end @@ -918,8 +917,8 @@ describe ProcedureRevision do let(:draft_revision) { procedure.draft_revision } subject do - draft_revision.save - draft_revision.errors + procedure.validate(:publication) + procedure.errors end it 'find error' do @@ -936,8 +935,8 @@ describe ProcedureRevision do let(:draft_revision) { procedure.draft_revision } subject do - draft_revision.save - draft_revision.errors + procedure.validate(:publication) + procedure.errors end context "When no regexp and no example" do diff --git a/spec/system/administrateurs/procedure_publish_spec.rb b/spec/system/administrateurs/procedure_publish_spec.rb index f1109ec4c..10818971c 100644 --- a/spec/system/administrateurs/procedure_publish_spec.rb +++ b/spec/system/administrateurs/procedure_publish_spec.rb @@ -44,13 +44,10 @@ describe 'Publishing a procedure', js: true do end context 'when a procedure isn’t published yet' do - before do - visit admin_procedures_path(statut: "brouillons") - click_on procedure.libelle - find('#publish-procedure-link').click - end - scenario 'an admin can publish it' do + visit admin_procedure_path(procedure) + find('#publish-procedure-link').click + expect(find_field('procedure_path').value).to eq procedure.path fill_in 'lien_site_web', with: 'http://some.website' within('form') { click_on 'Publier' } @@ -72,10 +69,13 @@ describe 'Publishing a procedure', js: true do end scenario 'an error message prevents the publication' do - expect(page).to have_content('Des problèmes empêchent la publication de la démarche') - expect(page).to have_content("Le champ « Enfants » doit comporter au moins un champ répétable") - expect(page).to have_content("L’annotation privée « Civilité » doit comporter au moins un choix sélectionnable") + visit admin_procedure_path(procedure) + expect(page).to have_content('Des problèmes empêchent la publication de la démarche') + expect(page).to have_content("« Enfants » doit comporter au moins un champ répétable") + expect(page).to have_content("« Civilité » doit comporter au moins un choix sélectionnable") + + visit admin_procedure_publication_path(procedure) expect(find_field('procedure_path').value).to eq procedure.path fill_in 'lien_site_web', with: 'http://some.website' @@ -85,8 +85,9 @@ describe 'Publishing a procedure', js: true do context 'when the procedure has the same path as another procedure from another admin ' do scenario 'an error message prevents the publication' do - expect(find_field('procedure_path').value).to eq procedure.path + visit admin_procedure_publication_path(procedure) fill_in 'procedure_path', with: other_procedure.path + expect(page).to have_content 'vous devez la modifier afin de pouvoir publier votre démarche' fill_in 'lien_site_web', with: 'http://some.website' diff --git a/spec/system/administrateurs/types_de_champ_spec.rb b/spec/system/administrateurs/types_de_champ_spec.rb index 8cbba47a7..28023d9b8 100644 --- a/spec/system/administrateurs/types_de_champ_spec.rb +++ b/spec/system/administrateurs/types_de_champ_spec.rb @@ -228,9 +228,7 @@ describe 'As an administrateur I can edit types de champ', js: true do click_on 'Supprimer' end end - - expect(page).to have_content("Le formulaire contient des erreurs") - expect(page).to have_content("Le titre de section suivant est invalide, veuillez le corriger :") + expect(page).to have_content("devrait être précédé d'un titre de niveau 1") end end