From dbcf21a555bc84a2a600e5d4941d5868c5a5a27d Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 30 Mar 2022 16:19:14 +0200 Subject: [PATCH] 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?] Update app/dashboards/archive_dashboard.rb Co-authored-by: LeSim --- .../manager/archives_controller.rb | 4 ++ app/dashboards/archive_dashboard.rb | 46 +++++++++++++++++++ app/jobs/archive_creation_job.rb | 8 +++- app/models/archive.rb | 10 +++- app/services/procedure_archive_service.rb | 4 +- config/routes.rb | 2 + spec/jobs/archive_creation_job_spec.rb | 41 +++++++++++++++++ .../procedure_archive_service_spec.rb | 19 ++++---- 8 files changed, 118 insertions(+), 16 deletions(-) create mode 100644 app/controllers/manager/archives_controller.rb create mode 100644 app/dashboards/archive_dashboard.rb create mode 100644 spec/jobs/archive_creation_job_spec.rb diff --git a/app/controllers/manager/archives_controller.rb b/app/controllers/manager/archives_controller.rb new file mode 100644 index 000000000..a8252dbf6 --- /dev/null +++ b/app/controllers/manager/archives_controller.rb @@ -0,0 +1,4 @@ +module Manager + class ArchivesController < Manager::ApplicationController + end +end diff --git a/app/dashboards/archive_dashboard.rb b/app/dashboards/archive_dashboard.rb new file mode 100644 index 000000000..50d53ea38 --- /dev/null +++ b/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 diff --git a/app/jobs/archive_creation_job.rb b/app/jobs/archive_creation_job.rb index 0c5fd90ad..bb1d8fd42 100644 --- a/app/jobs/archive_creation_job.rb +++ b/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 diff --git a/app/models/archive.rb b/app/models/archive.rb index 0f7685e94..7ea337a1a 100644 --- a/app/models/archive.rb +++ b/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? diff --git a/app/services/procedure_archive_service.rb b/app/services/procedure_archive_service.rb index a3fc6fa86..9f244c06c 100644 --- a/app/services/procedure_archive_service.rb +++ b/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) diff --git a/config/routes.rb b/config/routes.rb index 5d067883e..089653643 100644 --- a/config/routes.rb +++ b/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 diff --git a/spec/jobs/archive_creation_job_spec.rb b/spec/jobs/archive_creation_job_spec.rb new file mode 100644 index 000000000..2f4f38425 --- /dev/null +++ b/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 diff --git a/spec/services/procedure_archive_service_spec.rb b/spec/services/procedure_archive_service_spec.rb index d424ee1a7..2528114e9 100644 --- a/spec/services/procedure_archive_service_spec.rb +++ b/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