From 474d206ee119c32b7bc756df835f14525d73cb34 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 26 Nov 2024 22:14:47 +0100 Subject: [PATCH] refactor(champs): remove orphaned and invisible champs after submit --- app/models/concerns/dossier_state_concern.rb | 71 +++++++-- app/models/dossier.rb | 13 -- ...ece_justificative_file_not_visible_task.rb | 17 -- .../concerns/dossier_state_concern_spec.rb | 149 ++++++++++++++++++ spec/models/dossier_spec.rb | 10 +- spec/system/users/brouillon_spec.rb | 17 +- ...ustificative_file_not_visible_task_spec.rb | 30 ---- 7 files changed, 225 insertions(+), 82 deletions(-) delete mode 100644 app/tasks/maintenance/remove_piece_justificative_file_not_visible_task.rb create mode 100644 spec/models/concerns/dossier_state_concern_spec.rb delete mode 100644 spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb diff --git a/app/models/concerns/dossier_state_concern.rb b/app/models/concerns/dossier_state_concern.rb index a9ea4c741..8401d0bb9 100644 --- a/app/models/concerns/dossier_state_concern.rb +++ b/app/models/concerns/dossier_state_concern.rb @@ -11,9 +11,7 @@ module DossierStateConcern resolve_pending_correction! process_sva_svr! - remove_piece_justificative_file_not_visible! - - editing_forks.each(&:destroy_editing_fork!) + clean_champs_after_submit! end def after_passer_en_construction @@ -41,7 +39,8 @@ module DossierStateConcern groupe_instructeur.instructeurs.with_instant_email_dossier_notifications.each do |instructeur| DossierMailer.notify_new_dossier_depose_to_instructeur(self, instructeur.email).deliver_later end - remove_piece_justificative_file_not_visible! + + clean_champs_after_submit! end def after_passer_en_instruction(h) @@ -154,7 +153,7 @@ module DossierStateConcern end send_dossier_decision_to_experts(self) - remove_titres_identite! + clean_champs_after_instruction! end def after_accepter_automatiquement @@ -189,7 +188,7 @@ module DossierStateConcern NotificationMailer.send_notification_for_tiers(self).deliver_later if self.for_tiers? send_dossier_decision_to_experts(self) - remove_titres_identite! + clean_champs_after_instruction! end def after_refuser(h) @@ -226,7 +225,7 @@ module DossierStateConcern end send_dossier_decision_to_experts(self) - remove_titres_identite! + clean_champs_after_instruction! end def after_refuser_automatiquement @@ -254,7 +253,7 @@ module DossierStateConcern NotificationMailer.send_notification_for_tiers(self).deliver_later if self.for_tiers? send_dossier_decision_to_experts(self) - remove_titres_identite! + clean_champs_after_instruction! end def after_classer_sans_suite(h) @@ -291,7 +290,7 @@ module DossierStateConcern end send_dossier_decision_to_experts(self) - remove_titres_identite! + clean_champs_after_instruction! end def after_repasser_en_instruction(h) @@ -329,4 +328,58 @@ module DossierStateConcern rebase_later end + + def clean_champs_after_submit! + remove_discarded_rows! + remove_not_visible_rows! + remove_not_visible_or_empty_champs! + editing_forks.each(&:destroy_editing_fork!) + end + + def clean_champs_after_instruction! + remove_discarded_rows! + remove_titres_identite! + end + + private + + def remove_discarded_rows! + row_to_remove_ids = champs.filter { _1.row? && _1.discarded? }.map(&:row_id) + + return if row_to_remove_ids.empty? + champs.where(row_id: row_to_remove_ids).destroy_all + end + + def remove_not_visible_or_empty_champs! + repetition_to_keep_stable_ids, champ_to_keep_public_ids = project_champs_public_all + .reject { _1.blank? || !_1.visible? } + .partition(&:repetition?) + .then { |(repetitions, champs)| [repetitions.to_set(&:stable_id), champs.to_set(&:public_id)] } + + rows_public, champs_public = champs + .filter(&:public?) + .partition(&:row?) + + champs_to_remove = champs_public.reject { champ_to_keep_public_ids.member?(_1.public_id) } + champs_to_remove += rows_public.reject { repetition_to_keep_stable_ids.member?(_1.stable_id) } + + return if champs_to_remove.empty? + champs.where(id: champs_to_remove).destroy_all + end + + def remove_not_visible_rows! + row_to_remove_ids = project_champs_public + .filter { _1.repetition? && !_1.visible? } + .flat_map(&:row_ids) + + return if row_to_remove_ids.empty? + champs.where(row_id: row_to_remove_ids).destroy_all + end + + def remove_titres_identite! + champ_to_remove_ids = filled_champs.filter(&:titre_identite?).map(&:id) + + return if champ_to_remove_ids.empty? + champs.where(id: champ_to_remove_ids).destroy_all + end end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 44f3b973d..197702963 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -905,19 +905,6 @@ class Dossier < ApplicationRecord traitements.any?(&:termine?) end - def remove_titres_identite! - champs.filter(&:titre_identite?).map(&:piece_justificative_file).each(&:purge_later) - end - - def remove_piece_justificative_file_not_visible! - champs.each do |champ| - next unless champ.piece_justificative_file.attached? - next if champ.visible? - - champ.piece_justificative_file.purge_later - end - end - def check_mandatory_and_visible_champs project_champs_public.filter(&:visible?).each do |champ| if champ.mandatory_blank? diff --git a/app/tasks/maintenance/remove_piece_justificative_file_not_visible_task.rb b/app/tasks/maintenance/remove_piece_justificative_file_not_visible_task.rb deleted file mode 100644 index 473b5698c..000000000 --- a/app/tasks/maintenance/remove_piece_justificative_file_not_visible_task.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -module Maintenance - class RemovePieceJustificativeFileNotVisibleTask < MaintenanceTasks::Task - attribute :procedure_id, :string - validates :procedure_id, presence: true - - def collection - procedure = Procedure.with_discarded.find(procedure_id.strip.to_i) - procedure.dossiers.state_not_brouillon - end - - def process(dossier) - dossier.remove_piece_justificative_file_not_visible! - end - end -end diff --git a/spec/models/concerns/dossier_state_concern_spec.rb b/spec/models/concerns/dossier_state_concern_spec.rb new file mode 100644 index 000000000..88a39157c --- /dev/null +++ b/spec/models/concerns/dossier_state_concern_spec.rb @@ -0,0 +1,149 @@ +# frozen_string_literal: true + +RSpec.describe DossierStateConcern do + include Logic + + let(:procedure) { create(:procedure, :published, :for_individual, types_de_champ_public:, declarative_with_state:) } + let(:types_de_champ_public) do + [ + { type: :text, stable_id: 90 }, + { type: :text, stable_id: 91 }, + { type: :piece_justificative, stable_id: 92, condition: ds_eq(constant(true), constant(false)) }, + { type: :titre_identite, stable_id: 93, condition: ds_eq(constant(true), constant(false)) }, + { type: :repetition, stable_id: 94, children: [{ type: :text, stable_id: 941 }, { type: :text, stable_id: 942 }] }, + { type: :repetition, stable_id: 95, children: [{ type: :text, stable_id: 951 }] }, + { type: :repetition, stable_id: 96, children: [{ type: :text, stable_id: 961 }], condition: ds_eq(constant(true), constant(false)) }, + { type: :text, stable_id: 97, condition: ds_eq(constant(true), constant(false)) }, + { type: :titre_identite, stable_id: 98 } + ] + end + let(:declarative_with_state) { nil } + let(:dossier_state) { :brouillon } + let(:dossier) do + create(:dossier, dossier_state, :with_individual, :with_populated_champs, procedure:).tap do |dossier| + procedure.draft_revision.remove_type_de_champ(91) + procedure.draft_revision.remove_type_de_champ(95) + procedure.draft_revision.remove_type_de_champ(942) + procedure.publish_revision! + perform_enqueued_jobs + dossier.reload + champ_repetition = dossier.project_champs_public.find { _1.stable_id == 94 } + row_id = champ_repetition.row_ids.first + champ_repetition.remove_row(row_id, updated_by: 'test') + end + end + + describe 'submit brouillon' do + it do + expect(dossier.champs.size).to eq(20) + expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2) + expect(dossier.champs.filter { _1.row? && _1.discarded? }.size).to eq(1) + expect(dossier.champs.filter { _1.row? && _1.stable_id.in?([95, 96]) }.size).to eq(4) + expect(dossier.champs.filter { _1.stable_id.in?([90, 92, 93, 97, 961, 951]) }.size).to eq(8) + + champ_text = dossier.project_champs_public.find { _1.stable_id == 90 } + champ_text.update(value: '') + + dossier.passer_en_construction! + dossier.reload + + expect(dossier.champs.size).to eq(3) + expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1) + expect(dossier.champs.filter { _1.row? && _1.discarded? }.size).to eq(0) + expect(dossier.champs.filter { _1.row? && _1.stable_id.in?([95, 96]) }.size).to eq(0) + expect(dossier.champs.filter { _1.stable_id.in?([90, 92, 93, 97, 961, 951]) }.size).to eq(0) + end + end + + describe 'submit en construction' do + let(:dossier_state) { :en_construction } + + it do + expect(dossier.champs.size).to eq(20) + expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2) + expect(dossier.champs.filter { _1.row? && _1.discarded? }.size).to eq(1) + expect(dossier.champs.filter { _1.row? && _1.stable_id.in?([95, 96]) }.size).to eq(4) + expect(dossier.champs.filter { _1.stable_id.in?([92, 93, 97, 961, 951]) }.size).to eq(7) + + dossier.submit_en_construction! + dossier.reload + + expect(dossier.champs.size).to eq(4) + expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1) + expect(dossier.champs.filter { _1.row? && _1.discarded? }.size).to eq(0) + expect(dossier.champs.filter { _1.row? && _1.stable_id.in?([95, 96]) }.size).to eq(0) + expect(dossier.champs.filter { _1.stable_id.in?([92, 93, 97, 961, 951]) }.size).to eq(0) + end + end + + describe 'accepter' do + let(:dossier_state) { :en_instruction } + + it do + expect(dossier.champs.size).to eq(20) + expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2) + expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(2) + + dossier.accepter!(motivation: 'test') + dossier.reload + + expect(dossier.champs.size).to eq(15) + expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1) + expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(0) + end + end + + describe 'refuser' do + let(:dossier_state) { :en_instruction } + + it do + expect(dossier.champs.size).to eq(20) + expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2) + expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(2) + + dossier.refuser!(motivation: 'test') + dossier.reload + + expect(dossier.champs.size).to eq(15) + expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1) + expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(0) + end + end + + describe 'classer_sans_suite' do + let(:dossier_state) { :en_instruction } + + it do + expect(dossier.champs.size).to eq(20) + expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2) + expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(2) + + dossier.classer_sans_suite!(motivation: 'test') + dossier.reload + + expect(dossier.champs.size).to eq(15) + expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1) + expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(0) + end + end + + describe 'automatiquement' do + let(:dossier_state) { :en_construction } + let(:declarative_with_state) { Dossier.states.fetch(:accepte) } + + describe 'accepter' do + it do + expect(dossier.champs.size).to eq(20) + expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2) + expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(2) + + dossier.accepter_automatiquement! + dossier.reload + + expect(dossier.champs.size).to eq(15) + expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1) + expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(0) + end + end + end +end diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index cba6e5188..dcc9cd685 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -501,15 +501,13 @@ describe Dossier, type: :model do let(:procedure) { create(:procedure, types_de_champ_public:) } let(:dossier) { create(:dossier, :brouillon, :with_populated_champs, procedure:) } - before { expect(champ).to receive(:visible?).and_return(visible) } - context 'when piece_justificative' do - let(:types_de_champ_public) { [{ type: :piece_justificative }] } + let(:types_de_champ_public) { [{ type: :piece_justificative, condition: ds_eq(constant(true), constant(visible)) }] } let(:champ) { dossier.project_champs_public.find(&:piece_justificative?) } context 'when not visible' do let(:visible) { false } - it { expect { subject }.to change { champ.reload.piece_justificative_file.attached? } } + it { expect { subject }.to change { Champ.exists?(champ.id) } } end context 'when visible' do @@ -519,12 +517,12 @@ describe Dossier, type: :model do end context 'when titre identite' do - let(:types_de_champ_public) { [{ type: :titre_identite }] } + let(:types_de_champ_public) { [{ type: :titre_identite, condition: ds_eq(constant(true), constant(visible)) }] } let(:champ) { dossier.project_champs_public.find(&:titre_identite?) } context 'when not visible' do let(:visible) { false } - it { expect { subject }.to change { champ.reload.piece_justificative_file.attached? } } + it { expect { subject }.to change { Champ.exists?(champ.id) } } end context 'when visible' do diff --git a/spec/system/users/brouillon_spec.rb b/spec/system/users/brouillon_spec.rb index 98c414e4b..a53bc09a3 100644 --- a/spec/system/users/brouillon_spec.rb +++ b/spec/system/users/brouillon_spec.rb @@ -569,6 +569,16 @@ describe 'The user', js: true do expect(page).to have_no_css('legend', text: 'permis de conduire', visible: true) expect(page).to have_no_css('label', text: 'tonnage', visible: true) + fill_in('age du candidat', with: '18') + wait_for_autosave + + # the champ keeps their previous value so they are all displayed + expect(page).to have_css('legend', text: 'permis de conduire', visible: true) + expect(page).to have_css('label', text: 'tonnage', visible: true) + + fill_in('age du candidat', with: '2') + wait_for_autosave + click_on 'Déposer le dossier' click_on 'Accéder à votre dossier' click_on 'Modifier mon dossier' @@ -576,13 +586,6 @@ describe 'The user', js: true do expect(page).to have_css('label', text: 'age du candidat', visible: true) expect(page).to have_no_css('legend', text: 'permis de conduire', visible: true) expect(page).to have_no_css('label', text: 'tonnage', visible: true) - - fill_in('age du candidat', with: '18') - wait_for_autosave - - # the champ keeps their previous value so they are all displayed - expect(page).to have_css('legend', text: 'permis de conduire', visible: true) - expect(page).to have_css('label', text: 'tonnage', visible: true) end end end diff --git a/spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb b/spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb deleted file mode 100644 index e7e68ff2b..000000000 --- a/spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -module Maintenance - RSpec.describe RemovePieceJustificativeFileNotVisibleTask do - describe "#process" do - subject(:process) { described_class.process(dossier) } - - let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :piece_justificative }]) } - let(:dossier) { create(:dossier, :en_construction, :with_populated_champs, procedure:) } - - before { expect(champ).to receive(:visible?).and_return(visible) } - - context 'when piece_justificative' do - let(:champ) { dossier.filled_champs_public.find(&:piece_justificative?) } - - context 'when not visible' do - let(:visible) { false } - it { expect { subject }.to change { champ.reload.piece_justificative_file.attached? } } - end - - context 'when visible' do - let(:visible) { true } - it { expect { subject }.not_to change { champ.reload.piece_justificative_file.attached? } } - end - end - end - end -end