Merge pull request #7092 from betagouv/US/archive-enhance-usage-understanding

feat(archive): extract archive status management within job to simplify the main service as well as to isolate this part for a merge with exports csv/xslx [maybe?]

app/controllers/manager/archives_controller.rb

@@ -0,0 +1,4 @@
+module Manager
+  class ArchivesController < Manager::ApplicationController
+  end
+end

app/dashboards/archive_dashboard.rb

@@ -0,0 +1,46 @@
+require "administrate/base_dashboard"
+
+class ArchiveDashboard < Administrate::BaseDashboard
+  # ATTRIBUTE_TYPES
+  # a hash that describes the type of each of the model's fields.
+  #
+  # Each different type represents an Administrate::Field object,
+  # which determines how the attribute is displayed
+  # on pages throughout the dashboard.
+  ATTRIBUTE_TYPES = {
+    id: Field::Number,
+    created_at: Field::DateTime,
+    updated_at: Field::DateTime,
+    status: Field::String,
+    file: Field::HasOne
+  }.freeze
+
+  # COLLECTION_ATTRIBUTES
+  # an array of attributes that will be displayed on the model's index page.
+  #
+  # By default, it's limited to four items to reduce clutter on index pages.
+  # Feel free to add, remove, or rearrange items.
+  COLLECTION_ATTRIBUTES = [
+    :id,
+    :created_at,
+    :updated_at,
+    :status
+  ].freeze
+
+  # SHOW_PAGE_ATTRIBUTES
+  # an array of attributes that will be displayed on the model's show page.
+  SHOW_PAGE_ATTRIBUTES = [
+    :id,
+    :created_at,
+    :updated_at,
+    :status,
+    :file
+  ].freeze
+
+  # Overwrite this method to customize how users are displayed
+  # across all pages of the admin dashboard.
+  #
+  def display_resource(archive)
+    "Archive : #{archive&.file.&byte_size}"
+  end
+end

app/jobs/archive_creation_job.rb

@@ -2,8 +2,14 @@ class ArchiveCreationJob < ApplicationJob
   queue_as :archives
 
   def perform(procedure, archive, instructeur)
+    archive.restart! if archive.failed? # restart for AASM
     ProcedureArchiveService
       .new(procedure)
-      .collect_files_archive(archive, instructeur)
+      .make_and_upload_archive(archive, instructeur)
+    archive.make_available!
+    InstructeurMailer.send_archive(instructeur, procedure, archive).deliver_later
+  rescue => e
+    archive.fail! # fail for observability
+    raise e       # re-raise for retryable behaviour
   end
 end

app/models/archive.rb

@@ -35,16 +35,24 @@ class Archive < ApplicationRecord
 
   enum status: {
     pending: 'pending',
-    generated: 'generated'
+    generated: 'generated',
+    failed: 'failed'
   }
 
   aasm whiny_persistence: true, column: :status, enum: true do
     state :pending, initial: true
     state :generated
+    state :failed
 
     event :make_available do
       transitions from: :pending, to: :generated
     end
+    event :restart do
+      transitions from: :failed, to: :pending
+    end
+    event :fail do
+      transitions from: :pending, to: :failed
+    end
   end
 
   def available?

app/services/procedure_archive_service.rb

@@ -15,7 +15,7 @@ class ProcedureArchiveService
     Archive.find_or_create_archive(type, month, groupe_instructeurs)
   end
 
-  def collect_files_archive(archive, instructeur)
+  def make_and_upload_archive(archive, instructeur)
     dossiers = Dossier.visible_by_administration
       .where(groupe_instructeur: archive.groupe_instructeurs)
 
@@ -30,8 +30,6 @@ class ProcedureArchiveService
       ArchiveUploader.new(procedure: @procedure, archive: archive, filepath: zip_filepath)
         .upload
     end
-    archive.make_available!
-    InstructeurMailer.send_archive(instructeur, @procedure, archive).deliver_later
   end
 
   def self.procedure_files_size(procedure)

config/routes.rb

@@ -20,6 +20,8 @@ Rails.application.routes.draw do
       get 'export_mail_brouillons', on: :member
     end
 
+    resources :archives, only: [:index, :show]
+
     resources :dossiers, only: [:index, :show] do
       post 'discard', on: :member
       post 'restore', on: :member

spec/jobs/archive_creation_job_spec.rb

@@ -0,0 +1,41 @@
+describe ArchiveCreationJob, type: :job do
+  describe 'perform' do
+    let(:archive) { create(:archive, status: status, groupe_instructeurs: [procedure.groupe_instructeurs.first]) }
+    let(:instructeur) { create(:instructeur) }
+    let(:procedure) { create(:procedure, instructeurs: [instructeur]) }
+    let(:job) { ArchiveCreationJob.new(procedure, archive, instructeur) }
+
+    context 'when it fails' do
+      let(:status) { :pending }
+      let(:mailer) { double('mailer', deliver_later: true) }
+      before { expect(InstructeurMailer).not_to receive(:send_archive) }
+
+      it 'does not send email and forward error for retry' do
+        allow_any_instance_of(ProcedureArchiveService).to receive(:download_and_zip).and_raise(StandardError, "kaboom")
+        expect { job.perform_now }.to raise_error(StandardError, "kaboom")
+        expect(archive.reload.failed?).to eq(true)
+      end
+    end
+
+    context 'when it works' do
+      let(:mailer) { double('mailer', deliver_later: true) }
+      before do
+        allow_any_instance_of(ProcedureArchiveService).to receive(:download_and_zip).and_return(true)
+        expect(InstructeurMailer).to receive(:send_archive).and_return(mailer)
+      end
+
+      context 'when archive failed previously' do
+        let(:status) { :failed }
+        it 'restarts and works from failed states' do
+          expect { job.perform_now }.to change { archive.reload.failed? }.from(true).to(false)
+        end
+      end
+      context 'when archive start from pending state' do
+        let(:status) { :pending }
+        it 'restarts and works from failed states' do
+          expect { job.perform_now }.to change { archive.reload.generated? }.from(false).to(true)
+        end
+      end
+    end
+  end
+end

spec/services/procedure_archive_service_spec.rb

@@ -33,7 +33,7 @@ describe ProcedureArchiveService do
     end
   end
 
-  describe '#collect_files_archive' do
+  describe '#make_and_upload_archive' do
     let!(:dossier) { create_dossier_for_month(year, month) }
     let!(:dossier_2020) { create_dossier_for_month(2020, month) }
 
@@ -42,13 +42,12 @@ describe ProcedureArchiveService do
     context 'for a specific month' do
       let(:archive) { create(:archive, time_span_type: 'monthly', status: 'pending', month: date_month, groupe_instructeurs: groupe_instructeurs) }
       let(:year) { 2021 }
-      let(:mailer) { double('mailer', deliver_later: true) }
 
       it 'collects files with success' do
         allow_any_instance_of(ActiveStorage::Attached::One).to receive(:url).and_return("https://opengraph.githubassets.com/d0e7862b24d8026a3c03516d865b28151eb3859029c6c6c2e86605891fbdcd7a/socketry/async-io")
-        expect(InstructeurMailer).to receive(:send_archive).and_return(mailer)
+
         VCR.use_cassette('archive/new_file_to_get_200') do
-          service.collect_files_archive(archive, instructeur)
+          service.make_and_upload_archive(archive, instructeur)
         end
 
         archive.file.open do |f|
@@ -68,9 +67,9 @@ describe ProcedureArchiveService do
 
       it 'retry errors files with errors' do
         allow_any_instance_of(ActiveStorage::Attached::One).to receive(:url).and_return("https://www.demarches-simplifiees.fr/error_1")
-        expect(InstructeurMailer).to receive(:send_archive).and_return(mailer)
+
         VCR.use_cassette('archive/new_file_to_get_400.html') do
-          service.collect_files_archive(archive, instructeur)
+          service.make_and_upload_archive(archive, instructeur)
         end
         archive.file.open do |f|
           files = ZipTricks::FileReader.read_zip_structure(io: f)
@@ -113,11 +112,11 @@ describe ProcedureArchiveService do
         end
 
         it 'collect files without raising exception' do
-          expect { service.collect_files_archive(archive, instructeur) }.not_to raise_exception
+          expect { service.make_and_upload_archive(archive, instructeur) }.not_to raise_exception
         end
 
         it 'add bug report to archive' do
-          service.collect_files_archive(archive, instructeur)
+          service.make_and_upload_archive(archive, instructeur)
 
           archive.file.open do |f|
             zip_entries = ZipTricks::FileReader.read_zip_structure(io: f)
@@ -144,14 +143,12 @@ describe ProcedureArchiveService do
 
     context 'for all months' do
       let(:archive) { create(:archive, time_span_type: 'everything', status: 'pending', groupe_instructeurs: groupe_instructeurs) }
-      let(:mailer) { double('mailer', deliver_later: true) }
 
       it 'collect files' do
         allow_any_instance_of(ActiveStorage::Attached::One).to receive(:url).and_return("https://opengraph.githubassets.com/5e61989aecb78e369c93674f877d7bf4ecde378850114a9563cdf8b6a2472536/typhoeus/typhoeus/issues/110")
-        expect(InstructeurMailer).to receive(:send_archive).and_return(mailer)
 
         VCR.use_cassette('archive/old_file_to_get_200') do
-          service.collect_files_archive(archive, instructeur)
+          service.make_and_upload_archive(archive, instructeur)
         end
 
         archive = Archive.last