Merge pull request #9575 from colinux/exports-fix-dossiers-count

ETQ instructeur: corrige le décompte du nombre de dossiers exportés une fois l'export généré

app/controllers/administrateurs/exports_controller.rb

@@ -4,7 +4,7 @@ module Administrateurs
     before_action :ensure_not_super_admin!
 
     def download
-      export = Export.find_or_create_fresh_export(export_format, all_groupe_instructeurs, **export_options)
+      export = Export.find_or_create_fresh_export(export_format, all_groupe_instructeurs, current_administrateur.instructeur, **export_options)
       @dossiers_count = export.count
 
       if export.available?

app/controllers/instructeurs/procedures_controller.rb

@@ -175,7 +175,7 @@ module Instructeurs
         .visible_by_administration
         .exists?(groupe_instructeur_id: groupe_instructeur_ids) && !instructeur_as_manager?
 
-      export = Export.find_or_create_fresh_export(export_format, groupe_instructeurs, **export_options)
+      export = Export.find_or_create_fresh_export(export_format, groupe_instructeurs, current_instructeur, **export_options)
 
       @procedure = procedure
       @statut = export_options[:statut]

app/models/export.rb

@@ -29,6 +29,7 @@ class Export < ApplicationRecord
 
   has_and_belongs_to_many :groupe_instructeurs
   belongs_to :procedure_presentation, optional: true
+  belongs_to :instructeur, optional: true
 
   has_one_attached :file
 
@@ -50,6 +51,7 @@ class Export < ApplicationRecord
   end
 
   def compute
+    self.dossiers_count = dossiers_for_export.count
     load_snapshot!
 
     file.attach(blob.signed_id) # attaching a blob directly might run identify/virus scanner and wipe it
@@ -63,7 +65,7 @@ class Export < ApplicationRecord
     procedure_presentation_id.present?
   end
 
-  def self.find_or_create_fresh_export(format, groupe_instructeurs, time_span_type: time_span_types.fetch(:everything), statut: statuts.fetch(:tous), procedure_presentation: nil)
+  def self.find_or_create_fresh_export(format, groupe_instructeurs, instructeur, time_span_type: time_span_types.fetch(:everything), statut: statuts.fetch(:tous), procedure_presentation: nil)
     attributes = {
       format:,
       time_span_type:,
@@ -79,6 +81,7 @@ class Export < ApplicationRecord
     return recent_export if recent_export.present?
 
     create!(**attributes, groupe_instructeurs:,
+                          instructeur:,
                           procedure_presentation:,
                           procedure_presentation_snapshot: procedure_presentation&.snapshot)
   end
@@ -107,9 +110,10 @@ class Export < ApplicationRecord
   end
 
   def count
-    if procedure_presentation_id.present?
-      dossiers_for_export.count
-    end
+    return dossiers_count if !dossiers_count.nil? # export generated
+    return dossiers_for_export.count if procedure_presentation_id.present?
+
+    nil
   end
 
   def procedure

db/migrate/20231009070354_add_context_to_exports.rb

@@ -0,0 +1,11 @@
+class AddContextToExports < ActiveRecord::Migration[7.0]
+  disable_ddl_transaction!
+
+  def change
+    safety_assured do
+      add_reference :exports, :instructeur, foreign_key: true, null: true, default: nil, index: { algorithm: :concurrently }
+    end
+
+    add_column :exports, :dossiers_count, :integer, null: true, default: nil
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_09_28_083809) do
+ActiveRecord::Schema[7.0].define(version: 2023_10_09_070354) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
@@ -533,7 +533,9 @@ ActiveRecord::Schema[7.0].define(version: 2023_09_28_083809) do
 
   create_table "exports", force: :cascade do |t|
     t.datetime "created_at", precision: 6, null: false
+    t.integer "dossiers_count"
     t.string "format", null: false
+    t.bigint "instructeur_id"
     t.string "job_status", default: "pending", null: false
     t.text "key", null: false
     t.bigint "procedure_presentation_id"
@@ -541,6 +543,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_09_28_083809) do
     t.string "statut", default: "tous"
     t.string "time_span_type", default: "everything", null: false
     t.datetime "updated_at", precision: 6, null: false
+    t.index ["instructeur_id"], name: "index_exports_on_instructeur_id"
     t.index ["key"], name: "index_exports_on_key"
     t.index ["procedure_presentation_id"], name: "index_exports_on_procedure_presentation_id"
   end
@@ -1102,6 +1105,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_09_28_083809) do
   add_foreign_key "experts", "users"
   add_foreign_key "experts_procedures", "experts"
   add_foreign_key "experts_procedures", "procedures"
+  add_foreign_key "exports", "instructeurs"
   add_foreign_key "france_connect_informations", "users"
   add_foreign_key "geo_areas", "champs"
   add_foreign_key "groupe_instructeurs", "procedures"

spec/components/dossiers/export_link_component_spec.rb

@@ -1,17 +1,19 @@
 RSpec.describe Dossiers::ExportLinkComponent, type: :component do
   let(:procedure) { create(:procedure) }
-  let(:groupe_instructeur) { create(:groupe_instructeur, procedure: procedure) }
-  let(:export) { create(:export, groupe_instructeurs: [groupe_instructeur], updated_at: 5.minutes.ago, created_at: 10.minutes.ago) }
+  let(:groupe_instructeur) { create(:groupe_instructeur, procedure: procedure, instructeurs: [build(:instructeur)]) }
   let(:export_url) { double("ExportUrl", call: "/some/fake/path") }
 
+  let(:assign_to) { create(:assign_to, procedure: procedure, instructeur: groupe_instructeur.instructeurs.first) }
+  let(:procedure_presentation) { create(:procedure_presentation, procedure: procedure, assign_to: assign_to) }
+
   let(:component) { described_class.new(procedure:, exports: [export], export_url:) }
 
   describe "rendering" do
     subject { render_inline(component).to_html }
 
     context "when the export is available" do
+      let(:export) { create(:export, :generated, groupe_instructeurs: [groupe_instructeur], updated_at: 5.minutes.ago, created_at: 10.minutes.ago) }
       before do
-        allow(export).to receive(:available?).and_return(true)
         attachment = ActiveStorage::Attachment.new(name: "export", record: export, blob: ActiveStorage::Blob.new(byte_size: 10.kilobytes, content_type: "text/csv", filename: "export.csv"))
         allow(export).to receive(:file).and_return(attachment)
       end
@@ -25,12 +27,29 @@ RSpec.describe Dossiers::ExportLinkComponent, type: :component do
         expect(subject).to include("CSV")
         expect(subject).to include("10 ko")
       end
+
+      context 'when export is for everything' do
+        it 'not display the exact dossiers count' do
+          expect(subject).to include("tous les dossiers")
+        end
+      end
+
+      context 'when export is for a presentation' do
+        before do
+          export.update!(procedure_presentation: procedure_presentation)
+        end
+
+        it 'display the persisted dossiers count' do
+          expect(subject).to include("10 dossiers")
+        end
+      end
     end
 
     context "when the export is not available" do
+      let(:export) { create(:export, :pending, groupe_instructeurs: [groupe_instructeur], procedure_presentation: procedure_presentation, created_at: 10.minutes.ago) }
+
       before do
-        allow(export).to receive(:available?).and_return(false)
-        allow(export).to receive(:failed?).and_return(false)
+        create_list(:dossier, 3, :en_construction, procedure: procedure, groupe_instructeur: groupe_instructeur)
       end
 
       it "displays the pending label" do
@@ -40,12 +59,14 @@ RSpec.describe Dossiers::ExportLinkComponent, type: :component do
       it "displays a refresh page button" do
         expect(subject).to include("Recharger")
       end
+
+      it 'displays the current dossiers count' do
+        expect(subject).to include("3 dossiers")
+      end
     end
 
     context "when the export has failed" do
-      before do
-        allow(export).to receive(:failed?).and_return(true)
-      end
+      let(:export) { create(:export, :failed) }
 
       it "displays the refresh old export button" do
         expect(subject).to include("Regénérer")

spec/factories/export.rb

@@ -7,6 +7,8 @@ FactoryBot.define do
 
     after(:build) do |export, _evaluator|
       export.key = Export.generate_cache_key(export.groupe_instructeurs.map(&:id), export.procedure_presentation)
+      export.instructeur = export.groupe_instructeurs.first&.instructeurs&.first
+      export.dossiers_count = 10 if !export.pending?
     end
   end
 end

spec/models/export_spec.rb

@@ -81,28 +81,29 @@ RSpec.describe Export, type: :model do
 
   describe '.find_or_create_fresh_export' do
     let!(:procedure) { create(:procedure) }
-    let!(:gi_1) { create(:groupe_instructeur, procedure: procedure, instructeurs: [create(:instructeur)]) }
+    let(:instructeur) { create(:instructeur) }
+    let!(:gi_1) { create(:groupe_instructeur, procedure: procedure, instructeurs: [instructeur]) }
     let!(:pp) { gi_1.instructeurs.first.procedure_presentation_and_errors_for_procedure_id(procedure.id).first }
     before { pp.add_filter('tous', 'self/created_at', '10/12/2021') }
 
     context 'with procedure_presentation having different filters' do
       it 'works once' do
-        expect { Export.find_or_create_fresh_export(:zip, [gi_1], time_span_type: Export.time_span_types.fetch(:everything), statut: Export.statuts.fetch(:tous), procedure_presentation: pp) }
+        expect { Export.find_or_create_fresh_export(:zip, [gi_1], instructeur, time_span_type: Export.time_span_types.fetch(:everything), statut: Export.statuts.fetch(:tous), procedure_presentation: pp) }
           .to change { Export.count }.by(1)
       end
 
       it 'works once, changes procedure_presentation, recreate a new' do
-        expect { Export.find_or_create_fresh_export(:zip, [gi_1], time_span_type: Export.time_span_types.fetch(:everything), statut: Export.statuts.fetch(:tous), procedure_presentation: pp) }
+        expect { Export.find_or_create_fresh_export(:zip, [gi_1], instructeur, time_span_type: Export.time_span_types.fetch(:everything), statut: Export.statuts.fetch(:tous), procedure_presentation: pp) }
           .to change { Export.count }.by(1)
         pp.add_filter('tous', 'self/updated_at', '10/12/2021')
-        expect { Export.find_or_create_fresh_export(:zip, [gi_1], time_span_type: Export.time_span_types.fetch(:everything), statut: Export.statuts.fetch(:tous), procedure_presentation: pp) }
+        expect { Export.find_or_create_fresh_export(:zip, [gi_1], instructeur, time_span_type: Export.time_span_types.fetch(:everything), statut: Export.statuts.fetch(:tous), procedure_presentation: pp) }
           .to change { Export.count }.by(1)
       end
     end
 
     context 'with existing matching export' do
       def find_or_create =
-        Export.find_or_create_fresh_export(:zip, [gi_1], time_span_type: Export.time_span_types.fetch(:everything), statut: Export.statuts.fetch(:tous), procedure_presentation: pp)
+        Export.find_or_create_fresh_export(:zip, [gi_1], instructeur, time_span_type: Export.time_span_types.fetch(:everything), statut: Export.statuts.fetch(:tous), procedure_presentation: pp)
 
       context 'freshly generate export' do
         before { find_or_create.update!(job_status: :generated, updated_at: 1.second.ago) }
@@ -197,4 +198,31 @@ RSpec.describe Export, type: :model do
       expect(results.count).to eq(1)
     end
   end
+
+  describe '.dossiers_count' do
+    let(:export) { create(:export, :pending) }
+
+    before do
+      blob_double = instance_double("ActiveStorage::Blob", signed_id: "some_signed_id_value")
+      attachment_double = instance_double("ActiveStorage::Attached::One", attach: true)
+
+      allow(export).to receive(:blob).and_return(blob_double)
+      allow(export).to receive(:file).and_return(attachment_double)
+
+      create_list(:dossier, 3, :en_construction, groupe_instructeur: export.groupe_instructeurs.first)
+    end
+
+    it 'is not set until generation' do
+      expect(export.dossiers_count).to be_nil
+    end
+
+    it 'is persisted after generation' do
+      export.compute_with_safe_stale_for_purge do
+        export.compute
+      end
+
+      export.reload
+      expect(export.dossiers_count).to eq(3)
+    end
+  end
 end