Merge pull request #7115 from betagouv/really_faster_list_of_documents

accélere *30  la liste des documents par archive

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

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

app/lib/active_storage/downloadable_file.rb

@@ -1,24 +1,26 @@
-
 class ActiveStorage::DownloadableFile
-  def self.create_list_from_dossier(dossier, for_expert = false)
+  def self.create_list_from_dossiers(dossiers, for_expert = false)
-    dossier_export = PiecesJustificativesService.generate_dossier_export(dossier)
+    dossiers
-    pjs = [dossier_export] + PiecesJustificativesService.liste_documents(dossier, for_expert)
+      .map { |d| pj_and_path(d.id, PiecesJustificativesService.generate_dossier_export(d)) } +
-    pjs.map do |piece_justificative|
+    PiecesJustificativesService.liste_documents(dossiers, for_expert)
-      [
-        piece_justificative,
-        "dossier-#{dossier.id}/#{self.timestamped_filename(piece_justificative)}"
-      ]
-    end
-  end
-
-  def self.create_list_from_dossiers(dossiers)
-    dossiers.flat_map do |dossier|
-      create_list_from_dossier(dossier)
-    end
   end
 
   private
 
+  def self.bill_and_path(bill)
+    [
+      bill,
+      "bills/#{self.timestamped_filename(bill)}"
+    ]
+  end
+
+  def self.pj_and_path(dossier_id, 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
@@ -47,8 +49,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

app/services/pieces_justificatives_service.rb

@@ -1,10 +1,30 @@
 class PiecesJustificativesService
-  def self.liste_documents(dossier, for_expert)
+  def self.liste_documents(dossiers, for_expert)
-    pjs_champs = pjs_for_champs(dossier, for_expert)
+    bill_ids = []
-    pjs_commentaires = pjs_for_commentaires(dossier)
-    pjs_dossier = pjs_for_dossier(dossier, for_expert)
 
-    pjs_champs + pjs_commentaires + pjs_dossier
+    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
+        # 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)
@@ -106,82 +126,117 @@ 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)
+  def self.pjs_for_commentaires(dossiers)
-    commentaires = Commentaire
+    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)
+  def self.pjs_for_dossier(dossiers)
-    pjs = motivation(dossier) +
+    motivations(dossiers) +
-      attestation(dossier) +
+      attestations(dossiers) +
-      etablissement(dossier)
+      etablissements(dossiers)
-
-    if !for_expert
-      pjs += operation_logs_and_signatures(dossier)
-    end
-
-    pjs
   end
 
-  def self.etablissement(dossier)
+  def self.etablissements(dossiers)
-    etablissement = Etablissement.where(dossier: dossier)
+    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)
+  def self.attestations(dossiers)
-    attestation = Attestation
+    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)
+  def self.operation_logs_and_signature_ids(dossiers)
-    dol_ids_bill_id = DossierOperationLog
+    dol_id_dossier_id_bill_id = DossierOperationLog
-      .where(dossier: dossier)
+      .where(dossier: dossiers)
-      .pluck(:id, :bill_signature_id)
+      .pluck(:id, :dossier_id, :bill_signature_id)
 
-    dol_ids = dol_ids_bill_id.map(&:first)
+    dol_id_dossier_id = dol_id_dossier_id_bill_id
-    bill_ids = dol_ids_bill_id.map(&:second).uniq.compact
+      .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
+    [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

spec/lib/active_storage/downloadable_file_spec.rb

@@ -1,74 +1,12 @@
 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" }
     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_dossier(dossier, 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

spec/services/pieces_justificatives_service_spec.rb

@@ -1,43 +1,178 @@
 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, false) }
+    let(:for_expert) { false }
 
-    it "doesn't return sensitive documents like titre_identite" do
+    subject do
-      expect(champ_identite.piece_justificative_file).to be_attached
+      PiecesJustificativesService
-      expect(subject.any? { |piece| piece.name == 'piece_justificative_file' }).to be_falsy
+        .liste_documents(Dossier.where(id: dossier.id), for_expert)
+        .map(&:first)
     end
 
-    it "returns operation logs of the dossier" do
+    context 'with a pj champ' do
-      expect(subject.any? { |piece| piece.name == 'serialized' }).to be_truthy
+      let(:procedure) { create(:procedure, :with_piece_justificative) }
+      let(:dossier) { create(:dossier, procedure: procedure) }
+      let(:witness) { create(:dossier, procedure: procedure) }
+
+      let(:pj_champ) { -> (d) { d.champs.find { |c| c.type == 'Champs::PieceJustificativeChamp' } } }
+
+      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) { -> (d) { d.champs_private.find { |c| c.type == 'Champs::PieceJustificativeChamp' } } }
+
+      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.call(dossier).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(: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) }
+
+      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(: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
+
+    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 }
+
+      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)
+      end
+
+      it { expect(subject).to match_array([attestation_sociale.attachment, attestation_fiscale.attachment]) }
+    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
+
+      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 }
+
+      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(:dossier) { create(:dossier) }
+      let(:witness) { create(:dossier) }
+
+      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) }
+
+      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
+      expect { subject }.not_to change { dossier.updated_at }
-      subject
-      expect(dossier.updated_at).to be <= before_export
     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

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.id, p) } }
         before do
           allow(PiecesJustificativesService).to receive(:liste_documents).and_return(documents)
         end