refactor(pj_service): explicit options with_{bills,champs_private} because expert & instructeurs does not need the same

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")

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")

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)

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
 

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)

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

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

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