From 6a0379086d958b1424c91e9709542fef5a800695 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Sat, 29 Oct 2022 16:08:59 +0200 Subject: [PATCH] chore(piece_justificative): continue attachments refactor --- app/components/attachment/edit_component.rb | 8 +++--- .../edit_component/edit_component.html.haml | 2 +- .../multiple_component.html.haml | 2 +- .../titre_identite_component.html.haml | 2 +- .../shared/activestorage/auto-upload.ts | 6 ++--- .../piece_justificative/_show.html.haml | 3 ++- .../attachments_controller_spec.rb | 4 +-- .../piece_justificative_controller_spec.rb | 2 +- spec/serializers/champ_serializer_spec.rb | 4 ++- .../shared/attachment/_show.html.haml_spec.rb | 26 +++++++++---------- .../attachment/_update.html.haml_spec.rb | 9 ++++--- 11 files changed, 36 insertions(+), 32 deletions(-) diff --git a/app/components/attachment/edit_component.rb b/app/components/attachment/edit_component.rb index e0e50d6cc..9b932eed3 100644 --- a/app/components/attachment/edit_component.rb +++ b/app/components/attachment/edit_component.rb @@ -4,16 +4,16 @@ class Attachment::EditComponent < ApplicationComponent delegate :persisted?, to: :attachment, allow_nil: true - def initialize(form:, attached_file:, attachment: nil, user_can_destroy: false, direct_upload: true, id: nil, index: 0) + def initialize(form:, attached_file:, user_can_destroy: false, direct_upload: true, id: nil, index: 0, **kwargs) @form = form @attached_file = attached_file - @attachment = if attachment - attachment + @attachment = if kwargs.key?(:attachment) + kwargs[:attachment] elsif attached_file.respond_to?(:attachment) attached_file.attachment else - # multiple attachments: attachment kwarg is expected, unless adding a new attachment + fail ArgumentError, "You must pass an `attachment` kwarg when not using as single attachment like in #{attached_file.name}. Set it to nil for a new attachment." end @user_can_destroy = user_can_destroy diff --git a/app/components/attachment/edit_component/edit_component.html.haml b/app/components/attachment/edit_component/edit_component.html.haml index f91503777..4efad0744 100644 --- a/app/components/attachment/edit_component/edit_component.html.haml +++ b/app/components/attachment/edit_component/edit_component.html.haml @@ -1,4 +1,4 @@ -.fr-mb-2w +.fr-mb-2w.attachment - if persisted? .attachment-actions{ id: dom_id(attachment, :actions) } .attachment-action diff --git a/app/components/attachment/multiple_component/multiple_component.html.haml b/app/components/attachment/multiple_component/multiple_component.html.haml index 53a8b76b6..b8c214127 100644 --- a/app/components/attachment/multiple_component/multiple_component.html.haml +++ b/app/components/attachment/multiple_component/multiple_component.html.haml @@ -6,7 +6,7 @@ = render Attachment::EditComponent.new(form:, attached_file:, attachment:, user_can_destroy:, direct_upload:, id:, index:) %div{class: [attachments_empty? ? nil : "hidden"], data: { "#{stimulus_controller_name}-target": "empty" }} - = render Attachment::EditComponent.new(form:, attached_file:, user_can_destroy:, direct_upload:, id:, index: attachments_count) + = render Attachment::EditComponent.new(form:, attached_file:, attachment: nil, user_can_destroy:, direct_upload:, id:, index: attachments_count) - if can_attach_next? %button.fr-btn.fr-btn--tertiary.fr-btn--sm{ data: { "#{stimulus_controller_name}-target": "buttonAdd", action: "click->attachment-multiple#add" }} Ajouter un fichier diff --git a/app/components/editable_champ/titre_identite_component/titre_identite_component.html.haml b/app/components/editable_champ/titre_identite_component/titre_identite_component.html.haml index 7548f0ecd..d8e3d4f8d 100644 --- a/app/components/editable_champ/titre_identite_component/titre_identite_component.html.haml +++ b/app/components/editable_champ/titre_identite_component/titre_identite_component.html.haml @@ -1,2 +1,2 @@ - user_can_destroy = !@champ.mandatory? || @champ.dossier.brouillon? -= render Attachment::EditComponent.new(form: @form, attached_file: @champ.piece_justificative_file, user_can_destroy: user_can_destroy) += render Attachment::EditComponent.new(form: @form, attached_file: @champ.piece_justificative_file, attachment: @champ.piece_justificative_file[0], user_can_destroy: user_can_destroy) diff --git a/app/javascript/shared/activestorage/auto-upload.ts b/app/javascript/shared/activestorage/auto-upload.ts index 4bb41a615..fc9a24cbb 100644 --- a/app/javascript/shared/activestorage/auto-upload.ts +++ b/app/javascript/shared/activestorage/auto-upload.ts @@ -118,9 +118,9 @@ export class AutoUpload { } get errorElement() { - return this.#input.parentElement?.querySelector( - '.attachment-error' - ); + return this.#input + .closest('.attachment') + ?.querySelector('.attachment-error'); } get errorTitleElement() { diff --git a/app/views/shared/champs/piece_justificative/_show.html.haml b/app/views/shared/champs/piece_justificative/_show.html.haml index c57cf367e..c4cd50bfe 100644 --- a/app/views/shared/champs/piece_justificative/_show.html.haml +++ b/app/views/shared/champs/piece_justificative/_show.html.haml @@ -1,5 +1,6 @@ - pj = champ.piece_justificative_file - if pj.attached? - = render Attachment::ShowComponent.new(attachment: pj.attachment) + - pj.attachments.each do |attachment| + = render Attachment::ShowComponent.new(attachment:) - else Pièce justificative non fournie diff --git a/spec/controllers/attachments_controller_spec.rb b/spec/controllers/attachments_controller_spec.rb index 8b1f8dff8..140244825 100644 --- a/spec/controllers/attachments_controller_spec.rb +++ b/spec/controllers/attachments_controller_spec.rb @@ -1,6 +1,6 @@ describe AttachmentsController, type: :controller do let(:user) { create(:user) } - let(:attachment) { champ.piece_justificative_file.attachment } + let(:attachment) { champ.piece_justificative_file.attachments.first } let(:dossier) { create(:dossier, user: user) } let(:champ) { create(:champ_piece_justificative, dossier_id: dossier.id) } let(:signed_id) { attachment.blob.signed_id } @@ -45,7 +45,7 @@ describe AttachmentsController, type: :controller do describe '#destroy' do render_views - let(:attachment) { champ.piece_justificative_file.attachment } + let(:attachment) { champ.piece_justificative_file.attachments.first } let(:dossier) { create(:dossier, user: user) } let(:champ) { create(:champ_piece_justificative, dossier_id: dossier.id) } let(:signed_id) { attachment.blob.signed_id } diff --git a/spec/controllers/champs/piece_justificative_controller_spec.rb b/spec/controllers/champs/piece_justificative_controller_spec.rb index cedd547b0..1b1dd077f 100644 --- a/spec/controllers/champs/piece_justificative_controller_spec.rb +++ b/spec/controllers/champs/piece_justificative_controller_spec.rb @@ -23,7 +23,7 @@ describe Champs::PieceJustificativeController, type: :controller do subject champ.reload expect(champ.piece_justificative_file.attached?).to be true - expect(champ.piece_justificative_file.filename).to eq('piece_justificative_0.pdf') + expect(champ.piece_justificative_file[0].filename).to eq('piece_justificative_0.pdf') end it 'renders the attachment template as Javascript' do diff --git a/spec/serializers/champ_serializer_spec.rb b/spec/serializers/champ_serializer_spec.rb index 69de2722d..6ca6dcf7c 100644 --- a/spec/serializers/champ_serializer_spec.rb +++ b/spec/serializers/champ_serializer_spec.rb @@ -6,7 +6,9 @@ describe ChampSerializer do context 'when type champ is piece justificative' do let(:champ) { create(:champ_piece_justificative) } - it { expect(subject[:value]).to match('/rails/active_storage/disk/') } + it { + expect(subject[:value]).to match_array([a_string_matching('/rails/active_storage/disk/')]) + } end context 'when type champ is not piece justificative' do diff --git a/spec/views/shared/attachment/_show.html.haml_spec.rb b/spec/views/shared/attachment/_show.html.haml_spec.rb index 6bd7b4586..ebb9e3d5f 100644 --- a/spec/views/shared/attachment/_show.html.haml_spec.rb +++ b/spec/views/shared/attachment/_show.html.haml_spec.rb @@ -3,16 +3,16 @@ describe 'shared/attachment/_show.html.haml', type: :view do let(:virus_scan_result) { nil } before do - champ.piece_justificative_file.blob.update(metadata: champ.piece_justificative_file.blob.metadata.merge(virus_scan_result: virus_scan_result)) + champ.piece_justificative_file[0].blob.update(metadata: champ.piece_justificative_file[0].blob.metadata.merge(virus_scan_result: virus_scan_result)) end - subject { render Attachment::ShowComponent.new(attachment: champ.piece_justificative_file.attachment) } + subject { render Attachment::ShowComponent.new(attachment: champ.piece_justificative_file.attachments.first) } context 'when there is no anti-virus scan' do let(:virus_scan_result) { nil } it 'allows to download the file' do - expect(subject).to have_link(champ.piece_justificative_file.filename.to_s) + expect(subject).to have_link(champ.piece_justificative_file[0].filename.to_s) expect(subject).to have_text('ce fichier n’a pas été analysé par notre antivirus') end end @@ -21,8 +21,8 @@ describe 'shared/attachment/_show.html.haml', type: :view do let(:virus_scan_result) { ActiveStorage::VirusScanner::PENDING } it 'displays the filename, but doesn’t allow to download the file' do - expect(subject).to have_text(champ.piece_justificative_file.filename.to_s) - expect(subject).not_to have_link(champ.piece_justificative_file.filename.to_s) + expect(subject).to have_text(champ.piece_justificative_file[0].filename.to_s) + expect(subject).not_to have_link(champ.piece_justificative_file[0].filename.to_s) expect(subject).to have_text('analyse antivirus en cours') end end @@ -31,9 +31,9 @@ describe 'shared/attachment/_show.html.haml', type: :view do let(:champ) { create(:champ_titre_identite) } it 'displays the filename, but doesn’t allow to download the file' do - pp champ.piece_justificative_file.attachment.watermark_pending? - expect(subject).to have_text(champ.piece_justificative_file.filename.to_s) - expect(subject).not_to have_link(champ.piece_justificative_file.filename.to_s) + expect(champ.piece_justificative_file.attachments[0].watermark_pending?).to be_truthy + expect(subject).to have_text(champ.piece_justificative_file[0].filename.to_s) + expect(subject).not_to have_link(champ.piece_justificative_file[0].filename.to_s) expect(subject).to have_text('traitement de la pièce en cours') end end @@ -42,7 +42,7 @@ describe 'shared/attachment/_show.html.haml', type: :view do let(:virus_scan_result) { ActiveStorage::VirusScanner::SAFE } it 'allows to download the file' do - expect(subject).to have_link(champ.piece_justificative_file.filename.to_s) + expect(subject).to have_link(champ.piece_justificative_file[0].filename.to_s) end end @@ -50,8 +50,8 @@ describe 'shared/attachment/_show.html.haml', type: :view do let(:virus_scan_result) { ActiveStorage::VirusScanner::INFECTED } it 'displays the filename, but doesn’t allow to download the file' do - expect(subject).to have_text(champ.piece_justificative_file.filename.to_s) - expect(subject).not_to have_link(champ.piece_justificative_file.filename.to_s) + expect(subject).to have_text(champ.piece_justificative_file[0].filename.to_s) + expect(subject).not_to have_link(champ.piece_justificative_file[0].filename.to_s) expect(subject).to have_text('virus détecté') end end @@ -60,8 +60,8 @@ describe 'shared/attachment/_show.html.haml', type: :view do let(:virus_scan_result) { ActiveStorage::VirusScanner::INTEGRITY_ERROR } it 'displays the filename, but doesn’t allow to download the file' do - expect(subject).to have_text(champ.piece_justificative_file.filename.to_s) - expect(subject).not_to have_link(champ.piece_justificative_file.filename.to_s) + expect(subject).to have_text(champ.piece_justificative_file[0].filename.to_s) + expect(subject).not_to have_link(champ.piece_justificative_file[0].filename.to_s) expect(subject).to have_text('corrompu') end end diff --git a/spec/views/shared/attachment/_update.html.haml_spec.rb b/spec/views/shared/attachment/_update.html.haml_spec.rb index 85ce3564f..cfeea90e7 100644 --- a/spec/views/shared/attachment/_update.html.haml_spec.rb +++ b/spec/views/shared/attachment/_update.html.haml_spec.rb @@ -1,12 +1,12 @@ describe 'shared/attachment/_update.html.haml', type: :view do - let(:champ) { build(:champ_piece_justificative, dossier: create(:dossier)) } + let(:champ) { build(:champ_titre_identite, dossier: create(:dossier)) } let(:attached_file) { champ.piece_justificative_file } let(:user_can_destroy) { false } let(:template) { nil } subject do form_for(champ.dossier) do |form| - view.render Attachment::EditComponent.new(form: form, attached_file: attached_file, user_can_destroy: true, direct_upload: true) + view.render Attachment::EditComponent.new(form: form, attached_file: attached_file, attachment: attached_file[0], user_can_destroy: true, direct_upload: true) end end @@ -26,7 +26,7 @@ describe 'shared/attachment/_update.html.haml', type: :view do end it 'does not renders a link to the unsaved file' do - expect(subject).not_to have_content(attached_file.filename.to_s) + expect(subject).not_to have_content(attached_file.attachments[0].filename.to_s) end it 'does not render action buttons' do @@ -37,7 +37,7 @@ describe 'shared/attachment/_update.html.haml', type: :view do before { champ.save! } it 'renders a link to the file' do - expect(subject).to have_content(attached_file.filename.to_s) + expect(subject).to have_content(attached_file.attachments[0].filename.to_s) end it 'hides the form field by default' do @@ -55,6 +55,7 @@ describe 'shared/attachment/_update.html.haml', type: :view do form_for(champ.dossier) do |form| render Attachment::EditComponent.new(form: form, attached_file: attached_file, + attachment: attached_file[0], user_can_destroy: user_can_destroy, direct_upload: true) end