From ef864021f7dd6fe98b6d0268044bfa92a2abb811 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Mon, 30 Jan 2023 17:42:03 +0100 Subject: [PATCH] refactor(pj_service): explicit options with_{bills,champs_private} because expert & instructeurs does not need the same --- app/controllers/experts/avis_controller.rb | 2 +- .../instructeurs/dossiers_controller.rb | 2 +- app/lib/active_storage/downloadable_file.rb | 6 +++--- app/services/pieces_justificatives_service.rb | 12 ++++++------ app/services/procedure_archive_service.rb | 2 +- app/services/procedure_export_service.rb | 2 +- .../active_storage/downloadable_file_spec.rb | 2 +- .../pieces_justificatives_service_spec.rb | 17 +++++++++-------- 8 files changed, 23 insertions(+), 22 deletions(-) diff --git a/app/controllers/experts/avis_controller.rb b/app/controllers/experts/avis_controller.rb index 4971fc676..2801c3343 100644 --- a/app/controllers/experts/avis_controller.rb +++ b/app/controllers/experts/avis_controller.rb @@ -140,7 +140,7 @@ module Experts end def telecharger_pjs - files = ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: @dossier.id), true) + files = ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: @dossier.id)) cleaned_files = ActiveStorage::DownloadableFile.cleanup_list_from_dossier(files) zipline(cleaned_files, "dossier-#{@dossier.id}.zip") diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb index db2f996f3..17d051f73 100644 --- a/app/controllers/instructeurs/dossiers_controller.rb +++ b/app/controllers/instructeurs/dossiers_controller.rb @@ -235,7 +235,7 @@ module Instructeurs end def telecharger_pjs - files = ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: dossier.id), true) + files = ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: dossier.id), with_champs_private: true) cleaned_files = ActiveStorage::DownloadableFile.cleanup_list_from_dossier(files) zipline(cleaned_files, "dossier-#{dossier.id}.zip") diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index 4f84c7940..09713dadb 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -1,7 +1,7 @@ class ActiveStorage::DownloadableFile - def self.create_list_from_dossiers(dossiers, for_expert = false) - PiecesJustificativesService.generate_dossier_export(dossiers, include_infos_administration: !for_expert) + - PiecesJustificativesService.liste_documents(dossiers, for_expert) + def self.create_list_from_dossiers(dossiers, with_bills: false, with_champs_private: false, include_infos_administration: false) + PiecesJustificativesService.generate_dossier_export(dossiers, include_infos_administration:) + + PiecesJustificativesService.liste_documents(dossiers, with_bills:, with_champs_private:) end def self.cleanup_list_from_dossier(files) diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index f7bb33861..1b18ea4ca 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -1,13 +1,13 @@ class PiecesJustificativesService - def self.liste_documents(dossiers, for_expert) + def self.liste_documents(dossiers, with_bills:, with_champs_private:) bill_ids = [] docs = dossiers.in_batches.flat_map do |batch| - pjs = pjs_for_champs(batch, for_expert) + + pjs = pjs_for_champs(batch, with_champs_private:) + pjs_for_commentaires(batch) + pjs_for_dossier(batch) - if !for_expert + if with_bills # some bills are shared among operations # so first, all the bill_ids are fetched operation_logs, some_bill_ids = operation_logs_and_signature_ids(batch) @@ -19,7 +19,7 @@ class PiecesJustificativesService pjs end - if !for_expert + if with_bills # then the bills are retrieved without duplication docs += signatures(bill_ids.uniq) end @@ -117,12 +117,12 @@ class PiecesJustificativesService private - def self.pjs_for_champs(dossiers, for_expert = false) + def self.pjs_for_champs(dossiers, with_champs_private:) champs = Champ .joins(:piece_justificative_file_attachments) .where(type: "Champs::PieceJustificativeChamp", dossier: dossiers) - if for_expert + if !with_champs_private champs = champs.where(private: false) end diff --git a/app/services/procedure_archive_service.rb b/app/services/procedure_archive_service.rb index 594801961..db87c0785 100644 --- a/app/services/procedure_archive_service.rb +++ b/app/services/procedure_archive_service.rb @@ -15,7 +15,7 @@ class ProcedureArchiveService dossiers.processed_in_month(archive.month) end - attachments = ActiveStorage::DownloadableFile.create_list_from_dossiers(dossiers) + attachments = ActiveStorage::DownloadableFile.create_list_from_dossiers(dossiers, with_bills: true, with_champs_private: true) DownloadableFileService.download_and_zip(@procedure, attachments, zip_root_folder(archive)) do |zip_filepath| ArchiveUploader.new(procedure: @procedure, filename: archive.filename(@procedure), filepath: zip_filepath) diff --git a/app/services/procedure_export_service.rb b/app/services/procedure_export_service.rb index def935f06..ee24e574c 100644 --- a/app/services/procedure_export_service.rb +++ b/app/services/procedure_export_service.rb @@ -35,7 +35,7 @@ class ProcedureExportService end def to_zip - attachments = ActiveStorage::DownloadableFile.create_list_from_dossiers(dossiers, true) + attachments = ActiveStorage::DownloadableFile.create_list_from_dossiers(dossiers, with_champs_private: true) DownloadableFileService.download_and_zip(procedure, attachments, base_filename) do |zip_filepath| ArchiveUploader.new(procedure: procedure, filename: filename(:zip), filepath: zip_filepath).blob diff --git a/spec/lib/active_storage/downloadable_file_spec.rb b/spec/lib/active_storage/downloadable_file_spec.rb index 93fcccc98..93ae1378a 100644 --- a/spec/lib/active_storage/downloadable_file_spec.rb +++ b/spec/lib/active_storage/downloadable_file_spec.rb @@ -1,7 +1,7 @@ describe ActiveStorage::DownloadableFile do let(:dossier) { create(:dossier, :en_construction) } - subject(:list) { ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: dossier.id)) } + subject(:list) { ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: dossier.id), with_bills: true, with_champs_private: true) } describe 'create_list_from_dossiers' do context 'when no piece_justificative is present' do diff --git a/spec/services/pieces_justificatives_service_spec.rb b/spec/services/pieces_justificatives_service_spec.rb index fb58bcc54..bdfbda7a3 100644 --- a/spec/services/pieces_justificatives_service_spec.rb +++ b/spec/services/pieces_justificatives_service_spec.rb @@ -1,10 +1,11 @@ describe PiecesJustificativesService do describe '.liste_documents' do - let(:for_expert) { false } + let(:with_champs_private) { true } + let(:with_bills) { true } subject do PiecesJustificativesService - .liste_documents(Dossier.where(id: dossier.id), for_expert) + .liste_documents(Dossier.where(id: dossier.id), with_bills:, with_champs_private:) .map(&:first) end @@ -59,8 +60,8 @@ describe PiecesJustificativesService do it { expect(subject).to match_array(private_pj_champ.call(dossier).piece_justificative_file.attachments) } - context 'for expert' do - let(:for_expert) { true } + context 'without private champ' do + let(:with_champs_private) { false } it { expect(subject).to be_empty } end @@ -171,8 +172,8 @@ describe PiecesJustificativesService do expect(subject).to match_array([dossier_bs.serialized.attachment, dossier_bs.signature.attachment]) end - context 'for expert' do - let(:for_expert) { true } + context 'without bills' do + let(:with_bills) { false } it { expect(subject).to be_empty } end @@ -192,8 +193,8 @@ describe PiecesJustificativesService do it { expect(subject).to match_array(dol.serialized.attachment) } - context 'for expert' do - let(:for_expert) { true } + context 'without bills' do + let(:with_bills) { false } it { expect(subject).to be_empty } end