From 95f65900d34b7b89d4bb17136fec03a35c468458 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 10 Nov 2022 12:01:15 +0000 Subject: [PATCH] refactor(active_storage): no download on clone --- app/models/attestation_template.rb | 22 +-------------- app/models/procedure.rb | 5 +--- app/services/pieces_justificatives_service.rb | 18 +++++------- spec/models/attestation_template_spec.rb | 6 ++-- spec/models/procedure_spec.rb | 28 +++++++++++-------- 5 files changed, 30 insertions(+), 49 deletions(-) diff --git a/app/models/attestation_template.rb b/app/models/attestation_template.rb index a39f93995..f299d4c7b 100644 --- a/app/models/attestation_template.rb +++ b/app/models/attestation_template.rb @@ -54,27 +54,7 @@ class AttestationTemplate < ApplicationRecord def dup attestation_template = AttestationTemplate.new(title: title, body: body, footer: footer, activated: activated) - - if logo.attached? - attestation_template.logo.attach( - io: StringIO.new(logo.download), - filename: logo.filename.to_s, - content_type: logo.content_type, - # we don't want to run virus scanner on duplicated file - metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } - ) - end - - if signature.attached? - attestation_template.signature.attach( - io: StringIO.new(signature.download), - filename: signature.filename.to_s, - content_type: signature.content_type, - # we don't want to run virus scanner on duplicated file - metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } - ) - end - + PiecesJustificativesService.clone_attachments(self, attestation_template) attestation_template end diff --git a/app/models/procedure.rb b/app/models/procedure.rb index f58be81cf..5f6b12a9a 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -488,10 +488,7 @@ class Procedure < ApplicationRecord } include_list[:groupe_instructeurs] = :instructeurs if !is_different_admin procedure = self.deep_clone(include: include_list) do |original, kopy| - begin - PiecesJustificativesService.clone_attachments(original, kopy) - rescue ActiveStorage::FileNotFoundError, ActiveStorage::IntegrityError - end + PiecesJustificativesService.clone_attachments(original, kopy) end procedure.path = SecureRandom.uuid procedure.aasm_state = :brouillon diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index 648270116..8b9ccebc2 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -56,26 +56,22 @@ class PiecesJustificativesService end def self.clone_attachments(original, kopy) - if original.is_a?(TypeDeChamp) + case original + when TypeDeChamp clone_attachment(original.piece_justificative_template, kopy.piece_justificative_template) - elsif original.is_a?(Procedure) + when Procedure clone_attachment(original.logo, kopy.logo) clone_attachment(original.notice, kopy.notice) clone_attachment(original.deliberation, kopy.deliberation) + when AttestationTemplate + clone_attachment(original.logo, kopy.logo) + clone_attachment(original.signature, kopy.signature) end end def self.clone_attachment(original_attachment, copy_attachment) if original_attachment.attached? - original_attachment.open do |tempfile| - copy_attachment.attach({ - io: File.open(tempfile.path), - filename: original_attachment.filename, - content_type: original_attachment.content_type, - # we don't want to run virus scanner on cloned file - metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } - }) - end + copy_attachment.attach(original_attachment.blob) end end diff --git a/spec/models/attestation_template_spec.rb b/spec/models/attestation_template_spec.rb index 160c59de5..c71bd7e18 100644 --- a/spec/models/attestation_template_spec.rb +++ b/spec/models/attestation_template_spec.rb @@ -71,12 +71,14 @@ describe AttestationTemplate, type: :model do let(:attestation_template) { create(:attestation_template, :with_files) } it do - expect(subject.logo.blob).not_to eq(attestation_template.logo.blob) + expect(subject.logo.attachment).not_to eq(attestation_template.logo.attachment) + expect(subject.logo.blob).to eq(attestation_template.logo.blob) expect(subject.logo.attached?).to be_truthy end it do - expect(subject.signature.blob).not_to eq(attestation_template.signature.blob) + expect(subject.signature.attachment).not_to eq(attestation_template.signature.attachment) + expect(subject.signature.blob).to eq(attestation_template.signature.blob) expect(subject.signature.attached?).to be_truthy end end diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index e95f5236e..e3231587e 100644 --- a/spec/models/procedure_spec.rb +++ b/spec/models/procedure_spec.rb @@ -657,7 +657,23 @@ describe Procedure do let(:procedure) { create(:procedure, :with_notice, received_mail: received_mail, service: service) } it 'should duplicate notice' do - expect(subject.notice.attached?).to be true + expect(subject.notice.attached?).to be_truthy + expect(subject.notice.attachment).not_to eq(procedure.notice.attachment) + expect(subject.notice.attachment.blob).to eq(procedure.notice.attachment.blob) + + subject.notice.attach(logo) + subject.reload + procedure.reload + + expect(subject.notice.attached?).to be_truthy + expect(subject.notice.attachment.blob).not_to eq(procedure.notice.attachment.blob) + + subject.notice.purge + subject.reload + procedure.reload + + expect(subject.notice.attached?).to be_falsey + expect(procedure.notice.attached?).to be_truthy end end @@ -677,16 +693,6 @@ describe Procedure do expect(subject.canonical_procedure).to be_nil end end - - context 'with an pj not found' do - let(:procedure) { create(:procedure) } - - before do - expect(PiecesJustificativesService).to receive(:clone_attachments).at_least(:once).and_raise(ActiveStorage::FileNotFoundError) - end - - it { expect { procedure.clone(administrateur, false) }.not_to raise_error } - end end describe '#publish!' do