From 3f7693ae1311971fefbc350622e5893409f1b0cd Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Apr 2022 21:51:09 +0200 Subject: [PATCH 01/17] style remove blank line --- app/lib/active_storage/downloadable_file.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index 8c3855ad4..413662fc3 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -1,4 +1,3 @@ - class ActiveStorage::DownloadableFile def self.create_list_from_dossier(dossier, for_expert = false) dossier_export = PiecesJustificativesService.generate_dossier_export(dossier) From cf79e340ef3f7de76cf1f484f810a142dda4754d Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Apr 2022 22:05:18 +0200 Subject: [PATCH 02/17] remove unused method --- app/lib/active_storage/downloadable_file.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index 413662fc3..15895ccc6 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -46,8 +46,4 @@ class ActiveStorage::DownloadableFile 'pieces_justificatives/' end end - - def using_local_backend? - [:local, :local_test, :test].include?(Rails.application.config.active_storage.service) - end end From 51b71aaa01b13781056f5bbd988956953fe4be5d Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Apr 2022 14:20:59 +0200 Subject: [PATCH 03/17] always call create_list_from_dossiers --- app/controllers/experts/avis_controller.rb | 2 +- app/controllers/instructeurs/dossiers_controller.rb | 2 +- app/lib/active_storage/downloadable_file.rb | 4 ++-- spec/lib/active_storage/downloadable_file_spec.rb | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/experts/avis_controller.rb b/app/controllers/experts/avis_controller.rb index 6523fcd2c..a298d4485 100644 --- a/app/controllers/experts/avis_controller.rb +++ b/app/controllers/experts/avis_controller.rb @@ -139,7 +139,7 @@ module Experts end def telecharger_pjs - files = ActiveStorage::DownloadableFile.create_list_from_dossier(@dossier, true) + files = ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: @dossier.id), true) zipline(files, "dossier-#{@dossier.id}.zip") end diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb index 8612359ea..e941f50e3 100644 --- a/app/controllers/instructeurs/dossiers_controller.rb +++ b/app/controllers/instructeurs/dossiers_controller.rb @@ -218,7 +218,7 @@ module Instructeurs end def telecharger_pjs - files = ActiveStorage::DownloadableFile.create_list_from_dossier(dossier) + files = ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: dossier.id)) zipline(files, "dossier-#{dossier.id}.zip") end diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index 15895ccc6..85a3c586f 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -10,9 +10,9 @@ class ActiveStorage::DownloadableFile end end - def self.create_list_from_dossiers(dossiers) + def self.create_list_from_dossiers(dossiers, for_expert = false) dossiers.flat_map do |dossier| - create_list_from_dossier(dossier) + create_list_from_dossier(dossier, for_expert) end end diff --git a/spec/lib/active_storage/downloadable_file_spec.rb b/spec/lib/active_storage/downloadable_file_spec.rb index 755a0643c..011614265 100644 --- a/spec/lib/active_storage/downloadable_file_spec.rb +++ b/spec/lib/active_storage/downloadable_file_spec.rb @@ -1,9 +1,9 @@ describe ActiveStorage::DownloadableFile do let(:dossier) { create(:dossier, :en_construction) } - subject(:list) { ActiveStorage::DownloadableFile.create_list_from_dossier(dossier) } + subject(:list) { ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: dossier.id)) } - describe 'create_list_from_dossier' do + describe 'create_list_from_dossiers' do context 'when no piece_justificative is present' do it { expect(list.length).to eq 1 } it { expect(list.first[0].name).to eq "pdf_export_for_instructeur" } @@ -60,7 +60,7 @@ describe ActiveStorage::DownloadableFile do let(:champ) { dossier.champs.first } let(:avis) { create(:avis, dossier: dossier, claimant: instructeur, experts_procedure: experts_procedure, confidentiel: true) } - subject(:list) { ActiveStorage::DownloadableFile.create_list_from_dossier(dossier, true) } + subject(:list) { ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: dossier.id), true) } before do dossier.champs_private << create(:champ_piece_justificative, :with_piece_justificative_file, private: true, dossier: dossier) From 97443e2ff66e58ff2958fd0c50b91de61de6ade5 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Apr 2022 14:22:23 +0200 Subject: [PATCH 04/17] inline create_list_from_dossier into caller --- app/lib/active_storage/downloadable_file.rb | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index 85a3c586f..98ef3994e 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -1,18 +1,14 @@ class ActiveStorage::DownloadableFile - def self.create_list_from_dossier(dossier, for_expert = false) - dossier_export = PiecesJustificativesService.generate_dossier_export(dossier) - pjs = [dossier_export] + PiecesJustificativesService.liste_documents(dossier, for_expert) - pjs.map do |piece_justificative| - [ - piece_justificative, - "dossier-#{dossier.id}/#{self.timestamped_filename(piece_justificative)}" - ] - end - end - def self.create_list_from_dossiers(dossiers, for_expert = false) dossiers.flat_map do |dossier| - create_list_from_dossier(dossier, for_expert) + dossier_export = PiecesJustificativesService.generate_dossier_export(dossier) + pjs = [dossier_export] + PiecesJustificativesService.liste_documents(dossier, for_expert) + pjs.map do |piece_justificative| + [ + piece_justificative, + "dossier-#{dossier.id}/#{self.timestamped_filename(piece_justificative)}" + ] + end end end From d5241381eb816749b3d17415a31756c0671ce729 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Apr 2022 22:16:33 +0200 Subject: [PATCH 05/17] extract pj_path method --- app/lib/active_storage/downloadable_file.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index 98ef3994e..2cbbd4c75 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -3,17 +3,19 @@ class ActiveStorage::DownloadableFile dossiers.flat_map do |dossier| dossier_export = PiecesJustificativesService.generate_dossier_export(dossier) pjs = [dossier_export] + PiecesJustificativesService.liste_documents(dossier, for_expert) - pjs.map do |piece_justificative| - [ - piece_justificative, - "dossier-#{dossier.id}/#{self.timestamped_filename(piece_justificative)}" - ] - end + pjs.map { |piece_justificative| pj_and_path(dossier, piece_justificative) } end end private + def self.pj_and_path(dossier, pj) + [ + pj, + "dossier-#{dossier.id}/#{self.timestamped_filename(pj)}" + ] + end + def self.timestamped_filename(attachment) # we pad the original file name with a timestamp # and a short id in order to help identify multiple versions and avoid name collisions From 51a6145d96a6479f1bda3031476c4dfb1810341b Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Apr 2022 22:24:14 +0200 Subject: [PATCH 06/17] put dossier_pdf apart --- app/lib/active_storage/downloadable_file.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index 2cbbd4c75..41713ac0b 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -1,10 +1,13 @@ class ActiveStorage::DownloadableFile def self.create_list_from_dossiers(dossiers, for_expert = false) - dossiers.flat_map do |dossier| - dossier_export = PiecesJustificativesService.generate_dossier_export(dossier) - pjs = [dossier_export] + PiecesJustificativesService.liste_documents(dossier, for_expert) + pj_and_paths = dossiers.map { |d| pj_and_path(d, PiecesJustificativesService.generate_dossier_export(d)) } + + pj_and_paths += dossiers.flat_map do |dossier| + pjs = PiecesJustificativesService.liste_documents(dossier, for_expert) pjs.map { |piece_justificative| pj_and_path(dossier, piece_justificative) } end + + pj_and_paths end private From 7ac1288905b88deee0056a22bd6ae84f94d6b9e0 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Apr 2022 22:53:06 +0200 Subject: [PATCH 07/17] pj_service take a dossier collection ! --- app/lib/active_storage/downloadable_file.rb | 11 +++-------- app/services/pieces_justificatives_service.rb | 12 +++++++----- spec/services/pieces_justificatives_service_spec.rb | 6 +++--- spec/services/procedure_archive_service_spec.rb | 2 +- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index 41713ac0b..93842d5cb 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -1,13 +1,8 @@ class ActiveStorage::DownloadableFile def self.create_list_from_dossiers(dossiers, for_expert = false) - pj_and_paths = dossiers.map { |d| pj_and_path(d, PiecesJustificativesService.generate_dossier_export(d)) } - - pj_and_paths += dossiers.flat_map do |dossier| - pjs = PiecesJustificativesService.liste_documents(dossier, for_expert) - pjs.map { |piece_justificative| pj_and_path(dossier, piece_justificative) } - end - - pj_and_paths + dossiers + .map { |d| pj_and_path(d, PiecesJustificativesService.generate_dossier_export(d)) } + + PiecesJustificativesService.liste_documents(dossiers, for_expert) end private diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index 63192f350..f836d9989 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -1,10 +1,12 @@ class PiecesJustificativesService - def self.liste_documents(dossier, for_expert) - pjs_champs = pjs_for_champs(dossier, for_expert) - pjs_commentaires = pjs_for_commentaires(dossier) - pjs_dossier = pjs_for_dossier(dossier, for_expert) + def self.liste_documents(dossiers, for_expert) + dossiers.flat_map do |dossier| + pjs = pjs_for_champs(dossier, for_expert) + + pjs_for_commentaires(dossier) + + pjs_for_dossier(dossier, for_expert) - pjs_champs + pjs_commentaires + pjs_dossier + pjs.map { |piece_justificative| ActiveStorage::DownloadableFile.pj_and_path(dossier, piece_justificative) } + end end def self.serialize_types_de_champ_as_type_pj(revision) diff --git a/spec/services/pieces_justificatives_service_spec.rb b/spec/services/pieces_justificatives_service_spec.rb index 8fbdd43eb..0f2dd55a9 100644 --- a/spec/services/pieces_justificatives_service_spec.rb +++ b/spec/services/pieces_justificatives_service_spec.rb @@ -16,15 +16,15 @@ describe PiecesJustificativesService do end describe '.liste_documents' do - subject { PiecesJustificativesService.liste_documents(dossier, false) } + subject { PiecesJustificativesService.liste_documents(Dossier.where(id: dossier.id), false) } it "doesn't return sensitive documents like titre_identite" do expect(champ_identite.piece_justificative_file).to be_attached - expect(subject.any? { |piece| piece.name == 'piece_justificative_file' }).to be_falsy + expect(subject.any? { |piece, _| piece.name == 'piece_justificative_file' }).to be_falsy end it "returns operation logs of the dossier" do - expect(subject.any? { |piece| piece.name == 'serialized' }).to be_truthy + expect(subject.any? { |piece, _| piece.name == 'serialized' }).to be_truthy end end diff --git a/spec/services/procedure_archive_service_spec.rb b/spec/services/procedure_archive_service_spec.rb index cb7e62712..e0ea87ccc 100644 --- a/spec/services/procedure_archive_service_spec.rb +++ b/spec/services/procedure_archive_service_spec.rb @@ -106,7 +106,7 @@ describe ProcedureArchiveService do ) end - let(:documents) { [pj, bad_pj] } + let(:documents) { [pj, bad_pj].map { |p| ActiveStorage::DownloadableFile.pj_and_path(dossier, p) } } before do allow(PiecesJustificativesService).to receive(:liste_documents).and_return(documents) end From 34b0578d70b55deeeb82c12b87561f632490d247 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Apr 2022 23:00:54 +0200 Subject: [PATCH 08/17] pj_and_path only take dossier id --- app/lib/active_storage/downloadable_file.rb | 6 +++--- app/services/pieces_justificatives_service.rb | 2 +- spec/services/procedure_archive_service_spec.rb | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index 93842d5cb..0cbacfe95 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -1,16 +1,16 @@ class ActiveStorage::DownloadableFile def self.create_list_from_dossiers(dossiers, for_expert = false) dossiers - .map { |d| pj_and_path(d, PiecesJustificativesService.generate_dossier_export(d)) } + + .map { |d| pj_and_path(d.id, PiecesJustificativesService.generate_dossier_export(d)) } + PiecesJustificativesService.liste_documents(dossiers, for_expert) end private - def self.pj_and_path(dossier, pj) + def self.pj_and_path(dossier_id, pj) [ pj, - "dossier-#{dossier.id}/#{self.timestamped_filename(pj)}" + "dossier-#{dossier_id}/#{self.timestamped_filename(pj)}" ] end diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index f836d9989..749c2037a 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -5,7 +5,7 @@ class PiecesJustificativesService pjs_for_commentaires(dossier) + pjs_for_dossier(dossier, for_expert) - pjs.map { |piece_justificative| ActiveStorage::DownloadableFile.pj_and_path(dossier, piece_justificative) } + pjs.map { |piece_justificative| ActiveStorage::DownloadableFile.pj_and_path(dossier.id, piece_justificative) } end end diff --git a/spec/services/procedure_archive_service_spec.rb b/spec/services/procedure_archive_service_spec.rb index e0ea87ccc..72c99a62a 100644 --- a/spec/services/procedure_archive_service_spec.rb +++ b/spec/services/procedure_archive_service_spec.rb @@ -106,7 +106,7 @@ describe ProcedureArchiveService do ) end - let(:documents) { [pj, bad_pj].map { |p| ActiveStorage::DownloadableFile.pj_and_path(dossier, p) } } + let(:documents) { [pj, bad_pj].map { |p| ActiveStorage::DownloadableFile.pj_and_path(dossier.id, p) } } before do allow(PiecesJustificativesService).to receive(:liste_documents).and_return(documents) end From 5631141a467c558eac7fa4e137b7f7f6cd5cdba9 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Apr 2022 23:08:01 +0200 Subject: [PATCH 09/17] fetch pjs_for_champ by dossiers --- app/services/pieces_justificatives_service.rb | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index 749c2037a..c6e94c8b0 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -1,12 +1,15 @@ class PiecesJustificativesService def self.liste_documents(dossiers, for_expert) - dossiers.flat_map do |dossier| - pjs = pjs_for_champs(dossier, for_expert) + - pjs_for_commentaires(dossier) + + pj_and_paths = pjs_for_champs(dossiers, for_expert) + + pj_and_paths += dossiers.flat_map do |dossier| + pjs = pjs_for_commentaires(dossier) + pjs_for_dossier(dossier, for_expert) pjs.map { |piece_justificative| ActiveStorage::DownloadableFile.pj_and_path(dossier.id, piece_justificative) } end + + pj_and_paths end def self.serialize_types_de_champ_as_type_pj(revision) @@ -108,18 +111,26 @@ class PiecesJustificativesService private - def self.pjs_for_champs(dossier, for_expert = false) + def self.pjs_for_champs(dossiers, for_expert = false) champs = Champ .joins(:piece_justificative_file_attachment) - .where(type: "Champs::PieceJustificativeChamp", dossier: dossier) + .where(type: "Champs::PieceJustificativeChamp", dossier: dossiers) if for_expert champs = champs.where(private: false) end + champ_id_dossier_id = champs + .pluck(:id, :dossier_id) + .to_h + ActiveStorage::Attachment .includes(:blob) - .where(record_type: "Champ", record_id: champs.ids) + .where(record_type: "Champ", record_id: champ_id_dossier_id.keys) + .map do |a| + dossier_id = champ_id_dossier_id[a.record_id] + ActiveStorage::DownloadableFile.pj_and_path(dossier_id, a) + end end def self.pjs_for_commentaires(dossier) From 11aedb2dc86e570ecc745b318b36f8aad4d882cb Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Apr 2022 23:15:10 +0200 Subject: [PATCH 10/17] fetch pjs_for_commentaires by dossiers --- app/services/pieces_justificatives_service.rb | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index c6e94c8b0..c36dbfada 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -1,12 +1,11 @@ class PiecesJustificativesService def self.liste_documents(dossiers, for_expert) - pj_and_paths = pjs_for_champs(dossiers, for_expert) + pj_and_paths = pjs_for_champs(dossiers, for_expert) + + pjs_for_commentaires(dossiers) pj_and_paths += dossiers.flat_map do |dossier| - pjs = pjs_for_commentaires(dossier) + - pjs_for_dossier(dossier, for_expert) - - pjs.map { |piece_justificative| ActiveStorage::DownloadableFile.pj_and_path(dossier.id, piece_justificative) } + pjs_for_dossier(dossier, for_expert) + .map { |piece_justificative| ActiveStorage::DownloadableFile.pj_and_path(dossier.id, piece_justificative) } end pj_and_paths @@ -133,14 +132,20 @@ class PiecesJustificativesService end end - def self.pjs_for_commentaires(dossier) - commentaires = Commentaire + def self.pjs_for_commentaires(dossiers) + commentaire_id_dossier_id = Commentaire .joins(:piece_jointe_attachment) - .where(dossier: dossier) + .where(dossier: dossiers) + .pluck(:id, :dossier_id) + .to_h ActiveStorage::Attachment .includes(:blob) - .where(record_type: "Commentaire", record_id: commentaires.ids) + .where(record_type: "Commentaire", record_id: commentaire_id_dossier_id.keys) + .map do |a| + dossier_id = commentaire_id_dossier_id[a.record_id] + ActiveStorage::DownloadableFile.pj_and_path(dossier_id, a) + end end def self.pjs_for_dossier(dossier, for_expert = false) From e1afb35ca256b5f21493bb4c562c7e31a2daff37 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Apr 2022 23:40:20 +0200 Subject: [PATCH 11/17] fetch pjs_for_dossier by dossiers put all bills in bills folder --- app/lib/active_storage/downloadable_file.rb | 7 ++ app/services/pieces_justificatives_service.rb | 79 ++++++++++++------- 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index 0cbacfe95..3f38007be 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -7,6 +7,13 @@ class ActiveStorage::DownloadableFile private + def self.bill_and_path(bill) + [ + bill, + "bills/#{self.timestamped_filename(bill)}" + ] + end + def self.pj_and_path(dossier_id, pj) [ pj, diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index c36dbfada..424db58b2 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -1,14 +1,8 @@ class PiecesJustificativesService def self.liste_documents(dossiers, for_expert) - pj_and_paths = pjs_for_champs(dossiers, for_expert) + - pjs_for_commentaires(dossiers) - - pj_and_paths += dossiers.flat_map do |dossier| - pjs_for_dossier(dossier, for_expert) - .map { |piece_justificative| ActiveStorage::DownloadableFile.pj_and_path(dossier.id, piece_justificative) } - end - - pj_and_paths + pjs_for_champs(dossiers, for_expert) + + pjs_for_commentaires(dossiers) + + pjs_for_dossier(dossiers, for_expert) end def self.serialize_types_de_champ_as_type_pj(revision) @@ -148,57 +142,82 @@ class PiecesJustificativesService end end - def self.pjs_for_dossier(dossier, for_expert = false) - pjs = motivation(dossier) + - attestation(dossier) + - etablissement(dossier) + def self.pjs_for_dossier(dossiers, for_expert = false) + pjs = motivations(dossiers) + + attestations(dossiers) + + etablissements(dossiers) if !for_expert - pjs += operation_logs_and_signatures(dossier) + pjs += operation_logs_and_signatures(dossiers) end pjs end - def self.etablissement(dossier) - etablissement = Etablissement.where(dossier: dossier) + def self.etablissements(dossiers) + etablissement_id_dossier_id = Etablissement + .where(dossier: dossiers) + .pluck(:id, :dossier_id) + .to_h ActiveStorage::Attachment .includes(:blob) - .where(record_type: "Etablissement", record_id: etablissement) + .where(record_type: "Etablissement", record_id: etablissement_id_dossier_id.keys) + .map do |a| + dossier_id = etablissement_id_dossier_id[a.record_id] + ActiveStorage::DownloadableFile.pj_and_path(dossier_id, a) + end end - def self.motivation(dossier) + def self.motivations(dossiers) ActiveStorage::Attachment .includes(:blob) - .where(record_type: "Dossier", name: "justificatif_motivation", record_id: dossier) + .where(record_type: "Dossier", name: "justificatif_motivation", record_id: dossiers) + .map do |a| + dossier_id = a.record_id + ActiveStorage::DownloadableFile.pj_and_path(dossier_id, a) + end end - def self.attestation(dossier) - attestation = Attestation + def self.attestations(dossiers) + attestation_id_dossier_id = Attestation .joins(:pdf_attachment) - .where(dossier: dossier) + .where(dossier: dossiers) + .pluck(:id, :dossier_id) + .to_h ActiveStorage::Attachment .includes(:blob) - .where(record_type: "Attestation", record_id: attestation) + .where(record_type: "Attestation", record_id: attestation_id_dossier_id.keys) + .map do |a| + dossier_id = attestation_id_dossier_id[a.record_id] + ActiveStorage::DownloadableFile.pj_and_path(dossier_id, a) + end end - def self.operation_logs_and_signatures(dossier) - dol_ids_bill_id = DossierOperationLog - .where(dossier: dossier) - .pluck(:id, :bill_signature_id) + def self.operation_logs_and_signatures(dossiers) + dol_id_dossier_id_bill_id = DossierOperationLog + .where(dossier: dossiers) + .pluck(:id, :dossier_id, :bill_signature_id) - dol_ids = dol_ids_bill_id.map(&:first) - bill_ids = dol_ids_bill_id.map(&:second).uniq.compact + dol_id_dossier_id = dol_id_dossier_id_bill_id + .map { |dol_id, dossier_id, _| [dol_id, dossier_id] } + .to_h + + bill_ids = dol_id_dossier_id_bill_id.map(&:third).uniq.compact serialized_dols = ActiveStorage::Attachment .includes(:blob) - .where(record_type: "DossierOperationLog", record_id: dol_ids) + .where(record_type: "DossierOperationLog", record_id: dol_id_dossier_id.keys) + .map do |a| + dossier_id = dol_id_dossier_id[a.record_id] + ActiveStorage::DownloadableFile.pj_and_path(dossier_id, a) + end bill_docs = ActiveStorage::Attachment .includes(:blob) .where(record_type: "BillSignature", record_id: bill_ids) + .map { |bill| ActiveStorage::DownloadableFile.bill_and_path(bill) } serialized_dols + bill_docs end From c27d9a01f23c4ef71e3b407c268f14dc3ea2f5fb Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Apr 2022 23:53:29 +0200 Subject: [PATCH 12/17] fetch documents by batch to avoid huge memory load --- app/services/pieces_justificatives_service.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index 424db58b2..2879890f3 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -1,8 +1,10 @@ class PiecesJustificativesService def self.liste_documents(dossiers, for_expert) - pjs_for_champs(dossiers, for_expert) + - pjs_for_commentaires(dossiers) + - pjs_for_dossier(dossiers, for_expert) + dossiers.in_batches.flat_map do |batch| + pjs_for_champs(batch, for_expert) + + pjs_for_commentaires(batch) + + pjs_for_dossier(batch, for_expert) + end end def self.serialize_types_de_champ_as_type_pj(revision) From f24f6ee105e6180dfe5e3f2d27fb46e8f52c991f Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Apr 2022 15:10:22 +0200 Subject: [PATCH 13/17] extract operation_logs_and_signature from pjs_for_dossiers --- app/services/pieces_justificatives_service.rb | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index 2879890f3..98a119107 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -1,9 +1,15 @@ class PiecesJustificativesService def self.liste_documents(dossiers, for_expert) dossiers.in_batches.flat_map do |batch| - pjs_for_champs(batch, for_expert) + + pjs = pjs_for_champs(batch, for_expert) + pjs_for_commentaires(batch) + - pjs_for_dossier(batch, for_expert) + pjs_for_dossier(batch) + + if !for_expert + pjs += operation_logs_and_signatures(dossiers) + end + + pjs end end @@ -144,16 +150,10 @@ class PiecesJustificativesService end end - def self.pjs_for_dossier(dossiers, for_expert = false) - pjs = motivations(dossiers) + + def self.pjs_for_dossier(dossiers) + motivations(dossiers) + attestations(dossiers) + etablissements(dossiers) - - if !for_expert - pjs += operation_logs_and_signatures(dossiers) - end - - pjs end def self.etablissements(dossiers) From e2a54e3ee3b56e936759eedcfdd36f7845c45bfa Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Apr 2022 15:30:23 +0200 Subject: [PATCH 14/17] extract bill_ids per batch, and catch them all afterwards to avoid duplicate --- app/services/pieces_justificatives_service.rb | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index 98a119107..0bc94b5ad 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -1,16 +1,30 @@ class PiecesJustificativesService def self.liste_documents(dossiers, for_expert) - dossiers.in_batches.flat_map do |batch| + bill_ids = [] + + docs = dossiers.in_batches.flat_map do |batch| pjs = pjs_for_champs(batch, for_expert) + pjs_for_commentaires(batch) + pjs_for_dossier(batch) if !for_expert - pjs += operation_logs_and_signatures(dossiers) + # 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) + + pjs += operation_logs + bill_ids += some_bill_ids end pjs end + + if !for_expert + # then the bills are retrieved without duplication + docs += signatures(bill_ids.uniq) + end + + docs end def self.serialize_types_de_champ_as_type_pj(revision) @@ -197,7 +211,7 @@ class PiecesJustificativesService end end - def self.operation_logs_and_signatures(dossiers) + def self.operation_logs_and_signature_ids(dossiers) dol_id_dossier_id_bill_id = DossierOperationLog .where(dossier: dossiers) .pluck(:id, :dossier_id, :bill_signature_id) @@ -216,11 +230,13 @@ class PiecesJustificativesService ActiveStorage::DownloadableFile.pj_and_path(dossier_id, a) end - bill_docs = ActiveStorage::Attachment + [serialized_dols, bill_ids] + end + + def self.signatures(bill_ids) + ActiveStorage::Attachment .includes(:blob) .where(record_type: "BillSignature", record_id: bill_ids) .map { |bill| ActiveStorage::DownloadableFile.bill_and_path(bill) } - - serialized_dols + bill_docs end end From 951d96470100f73f2f158f6c275185c2e04a1e52 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 7 Apr 2022 10:39:54 +0200 Subject: [PATCH 15/17] clean pj_service test --- .../pieces_justificatives_service_spec.rb | 149 ++++++++++++++---- 1 file changed, 121 insertions(+), 28 deletions(-) diff --git a/spec/services/pieces_justificatives_service_spec.rb b/spec/services/pieces_justificatives_service_spec.rb index 0f2dd55a9..8b27a2b33 100644 --- a/spec/services/pieces_justificatives_service_spec.rb +++ b/spec/services/pieces_justificatives_service_spec.rb @@ -1,43 +1,136 @@ describe PiecesJustificativesService do - let(:procedure) { create(:procedure, :with_titre_identite) } - let(:dossier) { create(:dossier, procedure: procedure) } - let(:champ_identite) { dossier.champs.find { |c| c.type == 'Champs::TitreIdentiteChamp' } } - let(:bill_signature) do - bs = build(:bill_signature, :with_serialized, :with_signature) - bs.save(validate: false) - bs - end - - before do - champ_identite - .piece_justificative_file - .attach(io: StringIO.new("toto"), filename: "toto.png", content_type: "image/png") - create(:dossier_operation_log, dossier: dossier, bill_signature: bill_signature) - end - describe '.liste_documents' do - subject { PiecesJustificativesService.liste_documents(Dossier.where(id: dossier.id), false) } + let(:procedure) { create(:procedure) } + let(:dossier) { create(:dossier, procedure: procedure) } + let(:for_expert) { false } - it "doesn't return sensitive documents like titre_identite" do - expect(champ_identite.piece_justificative_file).to be_attached - expect(subject.any? { |piece, _| piece.name == 'piece_justificative_file' }).to be_falsy + subject do + PiecesJustificativesService + .liste_documents(Dossier.where(id: dossier.id), for_expert) + .map(&:first) end - it "returns operation logs of the dossier" do - expect(subject.any? { |piece, _| piece.name == 'serialized' }).to be_truthy + context 'with a pj champ' do + let(:procedure) { create(:procedure, :with_piece_justificative) } + let(:pj_champ) { dossier.champs.find { |c| c.type == 'Champs::PieceJustificativeChamp' } } + + before { attach_file_to_champ(pj_champ) } + + it { expect(subject).to match_array([pj_champ.piece_justificative_file.attachment]) } + end + + context 'with a private pj champ' do + let(:procedure) { create(:procedure) } + let!(:private_pj) { create(:type_de_champ_piece_justificative, procedure: procedure, private: true) } + let(:private_pj_champ) { dossier.champs_private.find { |c| c.type == 'Champs::PieceJustificativeChamp' } } + + before { attach_file_to_champ(private_pj_champ) } + + it { expect(subject).to match_array([private_pj_champ.piece_justificative_file.attachment]) } + + context 'for expert' do + let(:for_expert) { true } + + it { expect(subject).to be_empty } + end + end + + context 'with a identite champ pj' do + let(:procedure) { create(:procedure, :with_titre_identite) } + let(:champ_identite) { dossier.champs.find { |c| c.type == 'Champs::TitreIdentiteChamp' } } + + before { attach_file_to_champ(champ_identite) } + + it "doesn't return sensitive documents like titre_identite" do + expect(champ_identite.piece_justificative_file).to be_attached + expect(subject).to be_empty + end + end + + context 'with a pj on an commentaire' do + let!(:commentaire) { create(:commentaire, :with_file, dossier: dossier) } + + it { expect(subject).to match_array(dossier.commentaires.first.piece_jointe.attachment) } + end + + context 'with a motivation' do + let(:dossier) { create(:dossier, :with_justificatif) } + + it { expect(subject).to match_array(dossier.justificatif_motivation.attachment) } + end + + context 'with an attestation' do + let(:dossier) { create(:dossier, :with_attestation) } + + it { expect(subject).to match_array(dossier.attestation.pdf.attachment) } + end + + context 'with an etablissement' do + let(:dossier) { create(:dossier, :with_entreprise) } + let(:attestation_sociale) { dossier.etablissement.entreprise_attestation_sociale } + let(:attestation_fiscale) { dossier.etablissement.entreprise_attestation_fiscale } + + before do + attach_file(attestation_sociale) + attach_file(attestation_fiscale) + end + + it { expect(subject).to match_array([attestation_sociale.attachment, attestation_fiscale.attachment]) } + end + + context 'with a bill' do + let(:bill_signature) do + bs = build(:bill_signature, :with_serialized, :with_signature) + bs.save(validate: false) + bs + end + + before { create(:dossier_operation_log, dossier: dossier, bill_signature: bill_signature) } + + let(:dossier_bs) { dossier.dossier_operation_logs.first.bill_signature } + + it "returns serialized bill and signature" do + expect(subject).to match_array([dossier_bs.serialized.attachment, dossier_bs.signature.attachment]) + end + + context 'for expert' do + let(:for_expert) { true } + + it { expect(subject).to be_empty } + end + end + + context 'with a dol' do + let(:dol) { create(:dossier_operation_log, dossier: dossier) } + + before { attach_file(dol.serialized) } + + it { expect(subject).to match_array(dol.serialized.attachment) } + + context 'for expert' do + let(:for_expert) { true } + + it { expect(subject).to be_empty } + end end end describe '.generate_dossier_export' do + let(:dossier) { create(:dossier) } + subject { PiecesJustificativesService.generate_dossier_export(dossier) } - it "generates pdf export for instructeur" do - subject - end it "doesn't update dossier" do - before_export = Time.zone.now - subject - expect(dossier.updated_at).to be <= before_export + expect { subject }.not_to change { dossier.updated_at } end end + + def attach_file_to_champ(champ) + attach_file(champ.piece_justificative_file) + end + + def attach_file(attachable) + attachable + .attach(io: StringIO.new("toto"), filename: "toto.png", content_type: "image/png") + end end From e2130cc2d3db7882e48a3441f6eee748174c4edc Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 7 Apr 2022 12:02:20 +0200 Subject: [PATCH 16/17] add witness dossier --- .../pieces_justificatives_service_spec.rb | 64 +++++++++++++++---- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/spec/services/pieces_justificatives_service_spec.rb b/spec/services/pieces_justificatives_service_spec.rb index 8b27a2b33..8d0ca2e56 100644 --- a/spec/services/pieces_justificatives_service_spec.rb +++ b/spec/services/pieces_justificatives_service_spec.rb @@ -1,7 +1,5 @@ describe PiecesJustificativesService do describe '.liste_documents' do - let(:procedure) { create(:procedure) } - let(:dossier) { create(:dossier, procedure: procedure) } let(:for_expert) { false } subject do @@ -12,21 +10,33 @@ describe PiecesJustificativesService do context 'with a pj champ' do let(:procedure) { create(:procedure, :with_piece_justificative) } - let(:pj_champ) { dossier.champs.find { |c| c.type == 'Champs::PieceJustificativeChamp' } } + let(:dossier) { create(:dossier, procedure: procedure) } + let(:witness) { create(:dossier, procedure: procedure) } - before { attach_file_to_champ(pj_champ) } + let(:pj_champ) { -> (d) { d.champs.find { |c| c.type == 'Champs::PieceJustificativeChamp' } } } - it { expect(subject).to match_array([pj_champ.piece_justificative_file.attachment]) } + before do + attach_file_to_champ(pj_champ.call(dossier)) + attach_file_to_champ(pj_champ.call(witness)) + end + + it { expect(subject).to match_array([pj_champ.call(dossier).piece_justificative_file.attachment]) } end context 'with a private pj champ' do let(:procedure) { create(:procedure) } + let(:dossier) { create(:dossier, procedure: procedure) } + let(:witness) { create(:dossier, procedure: procedure) } + let!(:private_pj) { create(:type_de_champ_piece_justificative, procedure: procedure, private: true) } - let(:private_pj_champ) { dossier.champs_private.find { |c| c.type == 'Champs::PieceJustificativeChamp' } } + let(:private_pj_champ) { -> (d) { d.champs_private.find { |c| c.type == 'Champs::PieceJustificativeChamp' } } } - before { attach_file_to_champ(private_pj_champ) } + before do + attach_file_to_champ(private_pj_champ.call(dossier)) + attach_file_to_champ(private_pj_champ.call(witness)) + end - it { expect(subject).to match_array([private_pj_champ.piece_justificative_file.attachment]) } + it { expect(subject).to match_array([private_pj_champ.call(dossier).piece_justificative_file.attachment]) } context 'for expert' do let(:for_expert) { true } @@ -37,6 +47,9 @@ describe PiecesJustificativesService do context 'with a identite champ pj' do let(:procedure) { create(:procedure, :with_titre_identite) } + let(:dossier) { create(:dossier, procedure: procedure) } + let(:witness) { create(:dossier, procedure: procedure) } + let(:champ_identite) { dossier.champs.find { |c| c.type == 'Champs::TitreIdentiteChamp' } } before { attach_file_to_champ(champ_identite) } @@ -48,19 +61,25 @@ describe PiecesJustificativesService do end context 'with a pj on an commentaire' do + let(:dossier) { create(:dossier) } + let(:witness) { create(:dossier) } + let!(:commentaire) { create(:commentaire, :with_file, dossier: dossier) } + let!(:witness_commentaire) { create(:commentaire, :with_file, dossier: witness) } it { expect(subject).to match_array(dossier.commentaires.first.piece_jointe.attachment) } end context 'with a motivation' do let(:dossier) { create(:dossier, :with_justificatif) } + let!(:witness) { create(:dossier, :with_justificatif) } it { expect(subject).to match_array(dossier.justificatif_motivation.attachment) } end context 'with an attestation' do let(:dossier) { create(:dossier, :with_attestation) } + let!(:witness) { create(:dossier, :with_attestation) } it { expect(subject).to match_array(dossier.attestation.pdf.attachment) } end @@ -70,6 +89,10 @@ describe PiecesJustificativesService do let(:attestation_sociale) { dossier.etablissement.entreprise_attestation_sociale } let(:attestation_fiscale) { dossier.etablissement.entreprise_attestation_fiscale } + let!(:witness) { create(:dossier, :with_entreprise) } + let!(:witness_attestation_sociale) { witness.etablissement.entreprise_attestation_sociale } + let!(:witness_attestation_fiscale) { witness.etablissement.entreprise_attestation_fiscale } + before do attach_file(attestation_sociale) attach_file(attestation_fiscale) @@ -79,13 +102,25 @@ describe PiecesJustificativesService do end context 'with a bill' do + let(:dossier) { create(:dossier) } + let(:witness) { create(:dossier) } + let(:bill_signature) do bs = build(:bill_signature, :with_serialized, :with_signature) bs.save(validate: false) bs end - before { create(:dossier_operation_log, dossier: dossier, bill_signature: bill_signature) } + let(:witness_bill_signature) do + bs = build(:bill_signature, :with_serialized, :with_signature) + bs.save(validate: false) + bs + end + + before do + create(:dossier_operation_log, dossier: dossier, bill_signature: bill_signature) + create(:dossier_operation_log, dossier: witness, bill_signature: witness_bill_signature) + end let(:dossier_bs) { dossier.dossier_operation_logs.first.bill_signature } @@ -101,9 +136,16 @@ describe PiecesJustificativesService do end context 'with a dol' do - let(:dol) { create(:dossier_operation_log, dossier: dossier) } + let(:dossier) { create(:dossier) } + let(:witness) { create(:dossier) } - before { attach_file(dol.serialized) } + let(:dol) { create(:dossier_operation_log, dossier: dossier) } + let(:witness_dol) { create(:dossier_operation_log, dossier: witness) } + + before do + attach_file(dol.serialized) + attach_file(witness_dol.serialized) + end it { expect(subject).to match_array(dol.serialized.attachment) } From 4f73c6105fc34660621ea3fbfaf9865e40cd37e0 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 7 Apr 2022 12:05:58 +0200 Subject: [PATCH 17/17] remove duplicate spec --- .../active_storage/downloadable_file_spec.rb | 62 ------------------- 1 file changed, 62 deletions(-) diff --git a/spec/lib/active_storage/downloadable_file_spec.rb b/spec/lib/active_storage/downloadable_file_spec.rb index 011614265..f2e35f767 100644 --- a/spec/lib/active_storage/downloadable_file_spec.rb +++ b/spec/lib/active_storage/downloadable_file_spec.rb @@ -8,67 +8,5 @@ describe ActiveStorage::DownloadableFile do it { expect(list.length).to eq 1 } it { expect(list.first[0].name).to eq "pdf_export_for_instructeur" } end - - context 'when there is a piece_justificative' do - before do - dossier.champs << create(:champ_piece_justificative, :with_piece_justificative_file, dossier: dossier) - end - - it { expect(list.length).to eq 2 } - end - - context 'when there is a private piece_justificative' do - before do - dossier.champs_private << create(:champ_piece_justificative, :with_piece_justificative_file, private: true, dossier: dossier) - end - - it { expect(list.length).to eq 2 } - end - - context 'when there is a repetition bloc' do - before do - dossier.champs << create(:champ_repetition_with_piece_jointe, dossier: dossier) - end - - it 'should have 4 piece_justificatives' do - expect(list.size).to eq 5 - end - end - - context 'when there is a message with no attachment' do - before do - dossier.commentaires << create(:commentaire, dossier: dossier) - end - - it { expect(list.length).to eq 1 } - end - - context 'when there is a message with an attachment' do - before do - dossier.commentaires << create(:commentaire, :with_file, dossier: dossier) - end - - it { expect(list.length).to eq 2 } - end - - context 'when the files are asked by an expert with piece justificative and private piece justificative' do - let(:expert) { create(:expert) } - let(:instructeur) { create(:instructeur) } - let(:procedure) { create(:procedure, :published, :with_piece_justificative, instructeurs: [instructeur]) } - let(:experts_procedure) { create(:experts_procedure, expert: expert, procedure: procedure) } - let(:dossier) { create(:dossier, :en_construction, :with_dossier_link, procedure: procedure) } - let(:champ) { dossier.champs.first } - let(:avis) { create(:avis, dossier: dossier, claimant: instructeur, experts_procedure: experts_procedure, confidentiel: true) } - - subject(:list) { ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: dossier.id), true) } - - before do - dossier.champs_private << create(:champ_piece_justificative, :with_piece_justificative_file, private: true, dossier: dossier) - - dossier.champs << create(:champ_piece_justificative, :with_piece_justificative_file, dossier: dossier) - end - - it { expect(list.length).to eq 2 } - end end end