From 6445337be71694ad56b14f604ce579f7516c4682 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 26 Apr 2024 15:36:30 +0200 Subject: [PATCH] refactor(pj_list): extract pj list in a concern and simplify --- .../concerns/pieces_jointes_list_concern.rb | 39 ++++++++++++++ app/models/procedure.rb | 51 +------------------ app/models/procedure_revision.rb | 1 - .../shared/_procedure_description.html.haml | 30 ++++++----- .../pieces_jointes_list_concern_spec.rb | 50 ++++++++++++++++++ spec/models/procedure_spec.rb | 46 ----------------- .../_procedure_description.html.haml_spec.rb | 2 +- 7 files changed, 107 insertions(+), 112 deletions(-) create mode 100644 app/models/concerns/pieces_jointes_list_concern.rb create mode 100644 spec/models/concerns/pieces_jointes_list_concern_spec.rb diff --git a/app/models/concerns/pieces_jointes_list_concern.rb b/app/models/concerns/pieces_jointes_list_concern.rb new file mode 100644 index 000000000..263b01e9f --- /dev/null +++ b/app/models/concerns/pieces_jointes_list_concern.rb @@ -0,0 +1,39 @@ +module PiecesJointesListConcern + extend ActiveSupport::Concern + + included do + def public_wrapped_partionned_pjs + pieces_jointes_list(public_only: true, wrap_with_parent: true) + .partition { |(pj, _)| pj.condition.nil? } + end + + def pieces_jointes_exportables_list + pieces_jointes_list(exclude_titre_identite: true) + end + + private + + def pieces_jointes_list( + exclude_titre_identite: false, + public_only: false, + wrap_with_parent: false + ) + coordinates = active_revision.revision_types_de_champ + .includes(:type_de_champ, revision_types_de_champ: :type_de_champ) + + coordinates = coordinates.public_only if public_only + + type_champ = ['piece_justificative'] + type_champ << 'titre_identite' if !exclude_titre_identite + + coordinates = coordinates.where(types_de_champ: { type_champ: }) + + return coordinates.map(&:type_de_champ) if !wrap_with_parent + + # we want pj in the form of [[pj1], [pj2, repetition], [pj3, repetition]] + coordinates + .map { |c| c.child? ? [c, c.parent] : [c] } + .map { |a| a.map(&:type_de_champ) } + end + end +end diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 02b1a3d15..11cec13d8 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -5,6 +5,7 @@ class Procedure < ApplicationRecord include ProcedureGroupeInstructeurAPIHackConcern include ProcedureSVASVRConcern include ProcedureChorusConcern + include PiecesJointesListConcern include Discard::Model self.discard_column = :hidden_at @@ -982,28 +983,6 @@ class Procedure < ApplicationRecord end end - def pieces_jointes_list? - pieces_jointes_list_without_conditionnal.present? || pieces_jointes_list_with_conditionnal.present? - end - - def pieces_jointes_list_without_conditionnal - pieces_jointes_list do |base_scope| - base_scope.where(types_de_champ: { condition: nil }) - end - end - - def pieces_jointes_exportables_list - pieces_jointes_list(with_private: true, with_titre_identite: false, with_repetition_parent: false) do |base_scope| - base_scope - end.flatten - end - - def pieces_jointes_list_with_conditionnal - pieces_jointes_list do |base_scope| - base_scope.where.not(types_de_champ: { condition: nil }) - end - end - def toggle_routing update!(routing_enabled: self.groupe_instructeurs.active.many?) end @@ -1031,34 +1010,6 @@ class Procedure < ApplicationRecord private - def pieces_jointes_list(with_private: false, with_titre_identite: true, with_repetition_parent: true) - types_de_champ = with_private ? - active_revision.revision_types_de_champ_private_and_public : - active_revision.revision_types_de_champ_public - - type_champs = ['repetition', 'piece_justificative'] - type_champs << 'titre_identite' if with_titre_identite - - scope = yield types_de_champ - .includes(:type_de_champ, revision_types_de_champ: :type_de_champ) - .where(types_de_champ: { type_champ: [type_champs] }) - - scope.each_with_object([]) do |rtdc, list| - if rtdc.type_de_champ.repetition? - rtdc.revision_types_de_champ.each do |rtdc_in_repetition| - if rtdc_in_repetition.type_de_champ.piece_justificative? - to_add = [] - to_add << rtdc_in_repetition.type_de_champ - to_add << rtdc.type_de_champ if with_repetition_parent - list << to_add - end - end - else - list << [rtdc.type_de_champ] - end - end - end - def validate_auto_archive_on_in_the_future return if auto_archive_on.nil? return if auto_archive_on.future? diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb index 8326c6533..c8daa1bec 100644 --- a/app/models/procedure_revision.rb +++ b/app/models/procedure_revision.rb @@ -8,7 +8,6 @@ class ProcedureRevision < ApplicationRecord has_many :revision_types_de_champ, -> { order(:position, :id) }, class_name: 'ProcedureRevisionTypeDeChamp', foreign_key: :revision_id, dependent: :destroy, inverse_of: :revision has_many :revision_types_de_champ_public, -> { root.public_only.ordered }, class_name: 'ProcedureRevisionTypeDeChamp', foreign_key: :revision_id, dependent: :destroy, inverse_of: :revision has_many :revision_types_de_champ_private, -> { root.private_only.ordered }, class_name: 'ProcedureRevisionTypeDeChamp', foreign_key: :revision_id, dependent: :destroy, inverse_of: :revision - has_many :revision_types_de_champ_private_and_public, -> { root.ordered }, class_name: 'ProcedureRevisionTypeDeChamp', foreign_key: :revision_id, dependent: :destroy, inverse_of: :revision has_many :types_de_champ, through: :revision_types_de_champ, source: :type_de_champ has_many :types_de_champ_public, through: :revision_types_de_champ_public, source: :type_de_champ has_many :types_de_champ_private, through: :revision_types_de_champ_private, source: :type_de_champ diff --git a/app/views/shared/_procedure_description.html.haml b/app/views/shared/_procedure_description.html.haml index 922d790c8..e1cce58eb 100644 --- a/app/views/shared/_procedure_description.html.haml +++ b/app/views/shared/_procedure_description.html.haml @@ -48,21 +48,23 @@ #accordion-116.fr-collapse = h render SimpleFormatComponent.new(procedure.description_pj, allow_a: true) - - elsif procedure.pieces_jointes_list? - %section.fr-accordion.pieces_jointes - %h2.fr-accordion__title - %button.fr-accordion__btn{ "aria-controls" => "accordion-116", "aria-expanded" => "false" } - = t('shared.procedure_description.pieces_jointes') - #accordion-116.fr-collapse - - if procedure.pieces_jointes_list_without_conditionnal.present? - %ul - = render partial: "shared/procedure_pieces_jointes_list", collection: procedure.pieces_jointes_list_without_conditionnal, as: :pj + - else + - pj_without_condition, pj_with_condition = procedure.public_wrapped_partionned_pjs + - if pj_without_condition.present? || pj_with_condition.present? + %section.fr-accordion.pieces_jointes + %h2.fr-accordion__title + %button.fr-accordion__btn{ "aria-controls" => "accordion-116", "aria-expanded" => "false" } + = t('shared.procedure_description.pieces_jointes') + #accordion-116.fr-collapse + - if pj_without_condition.present? + %ul + = render partial: "shared/procedure_pieces_jointes_list", collection: pj_without_condition, as: :pj - - if procedure.pieces_jointes_list_with_conditionnal.present? - %h3.fr-text--sm.fr-mb-0.fr-mt-2w - = t('shared.procedure_description.pieces_jointes_conditionnal_list_title') - %ul - = render partial: "shared/procedure_pieces_jointes_list", collection: procedure.pieces_jointes_list_with_conditionnal, as: :pj + - if pj_with_condition.present? + %h3.fr-text--sm.fr-mb-0.fr-mt-2w + = t('shared.procedure_description.pieces_jointes_conditionnal_list_title') + %ul + = render partial: "shared/procedure_pieces_jointes_list", collection: pj_with_condition, as: :pj - estimated_delay_component = Procedure::EstimatedDelayComponent.new(procedure: procedure) - if estimated_delay_component.render? diff --git a/spec/models/concerns/pieces_jointes_list_concern_spec.rb b/spec/models/concerns/pieces_jointes_list_concern_spec.rb new file mode 100644 index 000000000..bc4bdcd47 --- /dev/null +++ b/spec/models/concerns/pieces_jointes_list_concern_spec.rb @@ -0,0 +1,50 @@ +describe PiecesJointesListConcern do + describe '#pieces_jointes_list' do + include Logic + let(:procedure) { create(:procedure, types_de_champ_public:, types_de_champ_private:) } + let(:types_de_champ_public) do + [ + { type: :integer_number, stable_id: 900 }, + { type: :piece_justificative, libelle: "pj1", stable_id: 910 }, + { type: :piece_justificative, libelle: "pj-cond", stable_id: 911, condition: ds_eq(champ_value(900), constant(1)) }, + { type: :repetition, libelle: "Répétition", stable_id: 920, children: [{ type: :piece_justificative, libelle: "pj2", stable_id: 921 }] }, + { type: :titre_identite, libelle: "pj3", stable_id: 930 } + ] + end + + let(:types_de_champ_private) do + [ + { type: :integer_number, stable_id: 950 }, + { type: :piece_justificative, libelle: "pj5", stable_id: 960 }, + { type: :piece_justificative, libelle: "pj-cond2", stable_id: 961, condition: ds_eq(champ_value(900), constant(1)) }, + { type: :repetition, libelle: "Répétition2", stable_id: 970, children: [{ type: :piece_justificative, libelle: "pj6", stable_id: 971 }] } + ] + end + + let(:types_de_champ) { procedure.active_revision.types_de_champ } + def find_by_stable_id(stable_id) = types_de_champ.find { _1.stable_id == stable_id } + + let(:pj1) { find_by_stable_id(910) } + let(:pjcond) { find_by_stable_id(911) } + let(:repetition) { find_by_stable_id(920) } + let(:pj2) { find_by_stable_id(921) } + let(:pj3) { find_by_stable_id(930) } + + let(:pj5) { find_by_stable_id(960) } + let(:pjcond2) { find_by_stable_id(961) } + let(:repetition2) { find_by_stable_id(970) } + let(:pj6) { find_by_stable_id(971) } + + it "returns the list of pieces jointes without conditional" do + expect(procedure.public_wrapped_partionned_pjs.first).to match_array([[pj1], [pj2, repetition], [pj3]]) + end + + it "returns the list of pieces jointes having conditional" do + expect(procedure.public_wrapped_partionned_pjs.second).to match_array([[pjcond]]) + end + + it "returns the list of pieces jointes with private, without parent repetition, without titre identite" do + expect(procedure.pieces_jointes_exportables_list.map(&:libelle)).to match_array([pj1, pj2, pjcond, pj5, pjcond2, pj6].map(&:libelle)) + end + end +end diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index 8b036cddc..0589985bb 100644 --- a/spec/models/procedure_spec.rb +++ b/spec/models/procedure_spec.rb @@ -1746,52 +1746,6 @@ describe Procedure do end end - describe '#pieces_jointes_list' do - include Logic - let(:procedure) { create(:procedure, types_de_champ_public:, types_de_champ_private:) } - let(:types_de_champ_public) do - [ - { type: :integer_number, stable_id: 900 }, - { type: :piece_justificative, libelle: "PJ", mandatory: true, stable_id: 910 }, - { type: :piece_justificative, libelle: "PJ-cond", mandatory: true, stable_id: 911, condition: ds_eq(champ_value(900), constant(1)) }, - { type: :repetition, libelle: "Répétition", stable_id: 920, children: [{ type: :piece_justificative, libelle: "PJ2", stable_id: 921 }] }, - { type: :titre_identite, libelle: "CNI", mandatory: true, stable_id: 930 } - ] - end - - let(:types_de_champ_private) do - [ - { type: :integer_number, stable_id: 950 }, - { type: :piece_justificative, libelle: "PJ", mandatory: true, stable_id: 960 }, - { type: :piece_justificative, libelle: "PJ-cond", mandatory: true, stable_id: 961, condition: ds_eq(champ_value(900), constant(1)) }, - { type: :repetition, libelle: "Répétition", stable_id: 970, children: [{ type: :piece_justificative, libelle: "PJ2", stable_id: 971 }] } - ] - end - - let(:pj1) { procedure.active_revision.types_de_champ.find { _1.stable_id == 910 } } - let(:pjcond) { procedure.active_revision.types_de_champ.find { _1.stable_id == 911 } } - let(:repetition) { procedure.active_revision.types_de_champ.find { _1.stable_id == 920 } } - let(:pj2) { procedure.active_revision.types_de_champ.find { _1.stable_id == 921 } } - let(:pj3) { procedure.active_revision.types_de_champ.find { _1.stable_id == 930 } } - - let(:pj5) { procedure.active_revision.types_de_champ.find { _1.stable_id == 960 } } - let(:pjcond2) { procedure.active_revision.types_de_champ.find { _1.stable_id == 961 } } - let(:repetition2) { procedure.active_revision.types_de_champ.find { _1.stable_id == 970 } } - let(:pj6) { procedure.active_revision.types_de_champ.find { _1.stable_id == 971 } } - - it "returns the list of pieces jointes without conditional" do - expect(procedure.pieces_jointes_list_without_conditionnal).to match_array([[pj1], [pj2, repetition], [pj3]]) - end - - it "returns the list of pieces jointes having conditional" do - expect(procedure.pieces_jointes_list_with_conditionnal).to match_array([[pjcond]]) - end - - it "returns the list of pieces jointes with private, without parent repetition, without titre identite" do - expect(procedure.pieces_jointes_exportables_list).to match_array([pj1, pj2, pjcond, pj5, pjcond2, pj6]) - end - end - describe "#attestation_template" do let(:procedure) { create(:procedure) } diff --git a/spec/views/shared/_procedure_description.html.haml_spec.rb b/spec/views/shared/_procedure_description.html.haml_spec.rb index 4bb722895..673584aae 100644 --- a/spec/views/shared/_procedure_description.html.haml_spec.rb +++ b/spec/views/shared/_procedure_description.html.haml_spec.rb @@ -110,7 +110,7 @@ describe 'shared/_procedure_description', type: :view do context 'caching', caching: true do it "works" do - expect(procedure).to receive(:pieces_jointes_list?).once + expect(procedure).to receive(:public_wrapped_partionned_pjs).once 2.times { render partial: 'shared/procedure_description', locals: { procedure: } } end