From b8296c6d4df4b289a520633a531bf57b6b2f6e66 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Tue, 25 Oct 2022 14:14:24 +0200 Subject: [PATCH] feat(piece_justificative): supports multiple files Closes #7924 --- app/components/attachment/edit_component.rb | 47 +++++++++++------ .../edit_component/edit_component.html.haml | 20 ++------ .../attachment/multiple_component.rb | 43 ++++++++++++++++ .../multiple_component.html.haml | 13 +++++ .../piece_justificative_component.html.haml | 14 +++++- .../champs/piece_justificative_controller.rb | 10 ++++ app/helpers/champ_helper.rb | 4 +- .../attachment_multiple_controller.ts | 50 +++++++++++++++++++ .../champs/piece_justificative_champ.rb | 10 ++-- .../attachments/destroy.turbo_stream.haml | 1 + .../show.turbo_stream.haml | 4 +- .../piece_justificative_component_spec.rb | 32 ++++++++++++ spec/factories/champ.rb | 7 ++- spec/models/champ_spec.rb | 12 ++--- .../champs/piece_justificative_champ_spec.rb | 14 +++--- .../pieces_justificatives_service_spec.rb | 15 +++++- .../attachment/_update.html.haml_spec.rb | 24 +-------- 17 files changed, 242 insertions(+), 78 deletions(-) create mode 100644 app/components/attachment/multiple_component.rb create mode 100644 app/components/attachment/multiple_component/multiple_component.html.haml create mode 100644 app/javascript/controllers/attachment_multiple_controller.ts create mode 100644 spec/components/editable_champ/piece_justificative_component/piece_justificative_component_spec.rb diff --git a/app/components/attachment/edit_component.rb b/app/components/attachment/edit_component.rb index 38a98bda3..25ec4e2ea 100644 --- a/app/components/attachment/edit_component.rb +++ b/app/components/attachment/edit_component.rb @@ -1,16 +1,27 @@ # Display a widget for uploading, editing and deleting a file attachment class Attachment::EditComponent < ApplicationComponent - def initialize(form:, attached_file:, template: nil, user_can_destroy: false, direct_upload: true, id: nil) + attr_reader :template, :form, :attachment + + 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) @form = form @attached_file = attached_file - @template = template + + @attachment = if attachment + attachment + elsif attached_file.respond_to?(:attachment) + attached_file.attachment + else + # multiple attachments: attachment kwarg is expected, unless adding a new attachment + end + @user_can_destroy = user_can_destroy @direct_upload = direct_upload @id = id + @index = index end - attr_reader :template, :form - def max_file_size file_size_validator.options[:less_than] end @@ -19,26 +30,18 @@ class Attachment::EditComponent < ApplicationComponent @user_can_destroy end - def attachment - @attached_file.attachment - end - def attachment_path helpers.attachment_path attachment.id, { signed_id: attachment.blob.signed_id } end def attachment_id - @attachment_id ||= attachment ? attachment.id : SecureRandom.uuid + @attachment_id ||= (attachment&.id || SecureRandom.uuid) end def attachment_input_class "attachment-input-#{attachment_id}" end - def persisted? - attachment&.persisted? - end - def champ @form.object.is_a?(Champ) ? @form.object : nil end @@ -51,13 +54,25 @@ class Attachment::EditComponent < ApplicationComponent id: input_id(@id), aria: { describedby: champ&.describedby_id }, data: { - auto_attach_url: helpers.auto_attach_url(form.object) - }.merge(has_file_size_validator? ? { max_file_size: max_file_size } : {}) + auto_attach_url: + }.merge(has_file_size_validator? ? { max_file_size: } : {}) }.merge(has_content_type_validator? ? { accept: accept_content_type } : {}) end + def auto_attach_url + helpers.auto_attach_url(form.object, replace_attachment_id: persisted? ? attachment_id : nil) + end + def input_id(given_id) - [given_id, champ&.input_id, file_field_name].reject(&:blank?).compact.first + return given_id if given_id.present? + + if champ.present? + # Single or first attachment input must match label "for" attribute. Others must remain unique. + return champ.input_id if @index.zero? + return "#{champ.input_id}_#{attachment_id}" + end + + file_field_name end def file_field_name diff --git a/app/components/attachment/edit_component/edit_component.html.haml b/app/components/attachment/edit_component/edit_component.html.haml index 42520586d..fc3e60515 100644 --- a/app/components/attachment/edit_component/edit_component.html.haml +++ b/app/components/attachment/edit_component/edit_component.html.haml @@ -1,15 +1,4 @@ -.attachment - - if template&.attached? - %p.mb-1 - Veuillez télécharger, remplir et joindre - = link_to(url_for(template), download: "", class: "fr-link fr-link--icon-right fr-icon-download-line") do - le modèle suivant - - - if helpers.administrateur_signed_in? - %span.ml-2.fr-text--xs.fr-text-mention--grey.visible-on-previous-hover - %span.fr-text-action-high--blue-france.fr-icon-questionnaire-line{ "aria-hidden": "true" } - = t('shared.ephemeral_link') - +.fr-mb-2w - if persisted? .attachment-actions{ id: dom_id(attachment, :actions) } .attachment-action @@ -30,8 +19,9 @@ %span.icon.retry Ré-essayer + - if !persisted? + %label.text-sm.font-weight-normal{ for: file_field_options[:id] } + = t('.max_file_size', max_file_size: number_to_human_size(max_file_size)) - %label.text-sm.font-weight-normal{ for: file_field_options[:id] } - = t('.max_file_size', max_file_size: number_to_human_size(max_file_size)) + %p= form.file_field(file_field_name, **file_field_options) - = form.file_field(file_field_name, **file_field_options) diff --git a/app/components/attachment/multiple_component.rb b/app/components/attachment/multiple_component.rb new file mode 100644 index 000000000..fe9f42135 --- /dev/null +++ b/app/components/attachment/multiple_component.rb @@ -0,0 +1,43 @@ +# Display a widget for uploading, editing and deleting a file attachment +class Attachment::MultipleComponent < ApplicationComponent + renders_one :template + + attr_reader :form + attr_reader :attached_file + attr_reader :direct_upload + attr_reader :id + attr_reader :user_can_destroy + + delegate :count, :empty?, to: :attachments, prefix: true + + def initialize(form:, attached_file:, user_can_destroy: false, direct_upload: true, id: nil) + @form = form + @attached_file = attached_file + @user_can_destroy = user_can_destroy + @direct_upload = direct_upload + @id = id + + @attachments = attached_file.attachments || [] + end + + def each_attachment(&block) + @attachments.each_with_index(&block) + end + + def can_attach_next? + return false if @attachments.empty? + return false if !@attachments.last.persisted? + + true + end + + def stimulus_controller_name + "attachment-multiple" + end + + private + + def attachments + @attachments + end +end diff --git a/app/components/attachment/multiple_component/multiple_component.html.haml b/app/components/attachment/multiple_component/multiple_component.html.haml new file mode 100644 index 000000000..53a8b76b6 --- /dev/null +++ b/app/components/attachment/multiple_component/multiple_component.html.haml @@ -0,0 +1,13 @@ +.fr-mb-4w{ data: { controller: stimulus_controller_name }} + = template + + - each_attachment do |attachment, index| + %div{id: dom_id(attachment)} + = 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) + + - 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/piece_justificative_component/piece_justificative_component.html.haml b/app/components/editable_champ/piece_justificative_component/piece_justificative_component.html.haml index 9ad12d070..9805b2050 100644 --- a/app/components/editable_champ/piece_justificative_component/piece_justificative_component.html.haml +++ b/app/components/editable_champ/piece_justificative_component/piece_justificative_component.html.haml @@ -1,2 +1,14 @@ - user_can_destroy = !@champ.mandatory? || @champ.dossier.brouillon? -= render Attachment::EditComponent.new(form: @form, attached_file: @champ.piece_justificative_file, template: @champ.type_de_champ.piece_justificative_template, user_can_destroy: user_can_destroy) += render Attachment::MultipleComponent.new(form: @form, attached_file: @champ.piece_justificative_file, user_can_destroy: user_can_destroy) do |c| + - if @champ.type_de_champ.piece_justificative_template&.attached? + - c.with_template do + %p + Veuillez télécharger, remplir et joindre + = link_to(url_for(@champ.type_de_champ.piece_justificative_template), download: "", class: "fr-link fr-link--icon-right fr-icon-download-line") do + le modèle suivant + + - if helpers.administrateur_signed_in? + %span.fr-ml-2w.fr-text--xs.fr-text-mention--grey.visible-on-previous-hover + %span.fr-text-action-high--blue-france.fr-icon-questionnaire-line{ "aria-hidden": "true" } + = t('shared.ephemeral_link') + diff --git a/app/controllers/champs/piece_justificative_controller.rb b/app/controllers/champs/piece_justificative_controller.rb index 1eed530b8..807bb70d2 100644 --- a/app/controllers/champs/piece_justificative_controller.rb +++ b/app/controllers/champs/piece_justificative_controller.rb @@ -13,6 +13,9 @@ class Champs::PieceJustificativeController < ApplicationController def attach_piece_justificative @champ = policy_scope(Champ).find(params[:champ_id]) + + purge_replaced_attachment + @champ.piece_justificative_file.attach(params[:blob_signed_id]) save_succeed = @champ.save @champ.dossier.update(last_champ_updated_at: Time.zone.now.utc) if save_succeed @@ -24,4 +27,11 @@ class Champs::PieceJustificativeController < ApplicationController rescue ActiveRecord::StaleObjectError attach_piece_justificative end + + def purge_replaced_attachment + return if params[:replace_attachment_id].blank? + + attachment = @champ.piece_justificative_file.attachments.find { _1.id == params[:replace_attachment_id].to_i } + attachment&.purge_later + end end diff --git a/app/helpers/champ_helper.rb b/app/helpers/champ_helper.rb index 2a66461bf..d57caaf8b 100644 --- a/app/helpers/champ_helper.rb +++ b/app/helpers/champ_helper.rb @@ -7,9 +7,9 @@ module ChampHelper simple_format(auto_linked_text, {}, sanitize: false) end - def auto_attach_url(object) + def auto_attach_url(object, url_options = {}) if object.is_a?(Champ) - champs_piece_justificative_url(object.id) + champs_piece_justificative_url(object.id, url_options) elsif object.is_a?(TypeDeChamp) piece_justificative_template_admin_procedure_type_de_champ_url(stable_id: object.stable_id, procedure_id: object.procedure.id) end diff --git a/app/javascript/controllers/attachment_multiple_controller.ts b/app/javascript/controllers/attachment_multiple_controller.ts new file mode 100644 index 000000000..a71bed991 --- /dev/null +++ b/app/javascript/controllers/attachment_multiple_controller.ts @@ -0,0 +1,50 @@ +import {} from '@hotwired/stimulus'; +import { show, hide } from '~/shared/utils'; +import { ApplicationController } from './application_controller'; + +type AttachementDestroyedEvent = CustomEvent<{ target_id: string }>; + +export class AttachmentMultipleController extends ApplicationController { + static targets = ['buttonAdd', 'empty']; + + declare readonly emptyTarget: HTMLDivElement; + declare readonly buttonAddTarget: HTMLButtonElement; + + connect() { + this.onGlobal('attachment:destroyed', (event: AttachementDestroyedEvent) => + this.onAttachmentDestroy(event) + ); + } + + add(event: Event) { + event.preventDefault(); + + hide(this.buttonAddTarget); + + show(this.emptyTarget); + + const inputFile = this.emptyTarget.querySelector( + 'input[type=file]' + ) as HTMLInputElement; + + inputFile.click(); + } + + onAttachmentDestroy(event: AttachementDestroyedEvent) { + const { detail } = event; + + const attachmentWrapper = document.getElementById(detail.target_id); + + // Remove this attachment row when there is at least another attachment. + if (attachmentWrapper && this.attachmentsCount() > 1) { + attachmentWrapper.parentNode?.removeChild(attachmentWrapper); + } else { + hide(this.buttonAddTarget); + } + } + + attachmentsCount() { + // Don't count the hidden "empty" attachment + return this.element.querySelectorAll('.attachment-input').length - 1; + } +} diff --git a/app/models/champs/piece_justificative_champ.rb b/app/models/champs/piece_justificative_champ.rb index dfde878e5..708f17d24 100644 --- a/app/models/champs/piece_justificative_champ.rb +++ b/app/models/champs/piece_justificative_champ.rb @@ -43,12 +43,16 @@ class Champs::PieceJustificativeChamp < Champ end def for_export - piece_justificative_file.filename.to_s if piece_justificative_file.attached? + piece_justificative_file.map { _1.filename.to_s } end def for_api - if piece_justificative_file.attached? && (piece_justificative_file.virus_scanner.safe? || piece_justificative_file.virus_scanner.pending?) - piece_justificative_file.service_url + return nil unless piece_justificative_file.attached? + + piece_justificative_file.filter_map do |attachment| + if attachment.virus_scanner.safe? || attachment.virus_scanner.pending? + attachment.service_url + end end end end diff --git a/app/views/attachments/destroy.turbo_stream.haml b/app/views/attachments/destroy.turbo_stream.haml index 323dd8f03..3d4600326 100644 --- a/app/views/attachments/destroy.turbo_stream.haml +++ b/app/views/attachments/destroy.turbo_stream.haml @@ -1,2 +1,3 @@ = turbo_stream.remove dom_id(@attachment, :actions) += turbo_stream.dispatch "attachment:destroyed", { target_id: dom_id(@attachment) } = turbo_stream.show_all ".attachment-input-#{@attachment.id}" diff --git a/app/views/champs/piece_justificative/show.turbo_stream.haml b/app/views/champs/piece_justificative/show.turbo_stream.haml index c170e5cb5..42c362406 100644 --- a/app/views/champs/piece_justificative/show.turbo_stream.haml +++ b/app/views/champs/piece_justificative/show.turbo_stream.haml @@ -2,6 +2,6 @@ = turbo_stream.morph @champ.input_group_id do = render EditableChamp::EditableChampComponent.new champ: @champ, form: form -- if @champ.piece_justificative_file.attached? - - attachment = @champ.piece_justificative_file.attachment + +- @champ.piece_justificative_file.attachments.each do |attachment| = turbo_stream.focus_all "button[data-toggle-target=\".attachment-input-#{attachment.id}\"]" diff --git a/spec/components/editable_champ/piece_justificative_component/piece_justificative_component_spec.rb b/spec/components/editable_champ/piece_justificative_component/piece_justificative_component_spec.rb new file mode 100644 index 000000000..705984960 --- /dev/null +++ b/spec/components/editable_champ/piece_justificative_component/piece_justificative_component_spec.rb @@ -0,0 +1,32 @@ +describe EditableChamp::PieceJustificativeComponent, type: :component do + let(:champ) { build(:champ_piece_justificative, dossier: create(:dossier)) } + let(:component) { + described_class.new(form: instance_double(ActionView::Helpers::FormBuilder, object: champ.dossier, file_field: ""), champ:) + } + + let(:subject) { + render_inline(component).to_html + } + + context 'when there is a template' do + let(:template) { champ.type_de_champ.piece_justificative_template } + let(:profil) { :user } + + before do + allow_any_instance_of(ApplicationController).to receive(:administrateur_signed_in?).and_return(profil == :administrateur) + end + + it 'renders a link to template' do + expect(subject).to have_link('le modèle suivant') + expect(subject).not_to have_text("éphémère") + end + + context 'as an administrator' do + let(:profil) { :administrateur } + it 'warn about ephemeral template url' do + expect(subject).to have_link('le modèle suivant') + expect(subject).to have_text("éphémère") + end + end + end +end diff --git a/spec/factories/champ.rb b/spec/factories/champ.rb index d531fd980..a52f096f8 100644 --- a/spec/factories/champ.rb +++ b/spec/factories/champ.rb @@ -162,8 +162,13 @@ FactoryBot.define do factory :champ_titre_identite, class: 'Champs::TitreIdentiteChamp' do type_de_champ { association :type_de_champ_titre_identite, procedure: dossier.procedure } + transient do + skip_default_attachment { false } + end + + after(:build) do |champ, evaluator| + next if evaluator.skip_default_attachment - after(:build) do |champ, _evaluator| champ.piece_justificative_file.attach( io: StringIO.new("toto"), filename: "toto.png", diff --git a/spec/models/champ_spec.rb b/spec/models/champ_spec.rb index a5f3641d8..6ef81bde5 100644 --- a/spec/models/champ_spec.rb +++ b/spec/models/champ_spec.rb @@ -452,13 +452,13 @@ describe Champ do end it 'marks the file as pending virus scan' do - expect(subject.piece_justificative_file.virus_scanner.started?).to be_truthy + expect(subject.piece_justificative_file.first.virus_scanner.started?).to be_truthy end it 'marks the file as safe once the scan completes' do subject perform_enqueued_jobs - expect(champ.reload.piece_justificative_file.virus_scanner.safe?).to be_truthy + expect(champ.reload.piece_justificative_file.first.virus_scanner.safe?).to be_truthy end end end @@ -467,7 +467,7 @@ describe Champ do describe '#enqueue_watermark_job' do context 'when type_champ is type_de_champ_titre_identite' do let(:type_de_champ) { create(:type_de_champ_titre_identite) } - let(:champ) { build(:champ_titre_identite, type_de_champ: type_de_champ) } + let(:champ) { build(:champ_titre_identite, type_de_champ: type_de_champ, skip_default_attachment: true) } before do allow(ClamavService).to receive(:safe_file?).and_return(true) @@ -480,14 +480,14 @@ describe Champ do end it 'marks the file as needing watermarking' do - expect(subject.piece_justificative_file.watermark_pending?).to be_truthy + expect(subject.piece_justificative_file.first.watermark_pending?).to be_truthy end it 'watermarks the file' do subject perform_enqueued_jobs - expect(champ.reload.piece_justificative_file.watermark_pending?).to be_falsy - expect(champ.reload.piece_justificative_file.blob.watermark_done?).to be_truthy + expect(champ.reload.piece_justificative_file.first.watermark_pending?).to be_falsy + expect(champ.reload.piece_justificative_file.first.blob.watermark_done?).to be_truthy end end end diff --git a/spec/models/champs/piece_justificative_champ_spec.rb b/spec/models/champs/piece_justificative_champ_spec.rb index 9d3d239b1..1e0c9c80c 100644 --- a/spec/models/champs/piece_justificative_champ_spec.rb +++ b/spec/models/champs/piece_justificative_champ_spec.rb @@ -43,35 +43,35 @@ describe Champs::PieceJustificativeChamp do let(:champ_pj) { create(:champ_piece_justificative) } subject { champ_pj.for_export } - it { is_expected.to eq('toto.txt') } + it { is_expected.to match_array(['toto.txt']) } context 'without attached file' do before { champ_pj.piece_justificative_file.purge } - it { is_expected.to eq(nil) } + it { is_expected.to eq([]) } end end describe '#for_api' do let(:champ_pj) { create(:champ_piece_justificative) } - let(:metadata) { champ_pj.piece_justificative_file.blob.metadata } + let(:metadata) { champ_pj.piece_justificative_file.first.blob.metadata } - before { champ_pj.piece_justificative_file.blob.update(metadata: metadata.merge(virus_scan_result: status)) } + before { champ_pj.piece_justificative_file.first.blob.update(metadata: metadata.merge(virus_scan_result: status)) } subject { champ_pj.for_api } context 'when file is safe' do let(:status) { ActiveStorage::VirusScanner::SAFE } - it { is_expected.to include("/rails/active_storage/disk/") } + it { is_expected.to match_array([include("/rails/active_storage/disk/")]) } end context 'when file is not scanned' do let(:status) { ActiveStorage::VirusScanner::PENDING } - it { is_expected.to include("/rails/active_storage/disk/") } + it { is_expected.to match_array([include("/rails/active_storage/disk/")]) } end context 'when file is infected' do let(:status) { ActiveStorage::VirusScanner::INFECTED } - it { is_expected.to be_nil } + it { is_expected.to eq([]) } end end end diff --git a/spec/services/pieces_justificatives_service_spec.rb b/spec/services/pieces_justificatives_service_spec.rb index 882d6f035..fb58bcc54 100644 --- a/spec/services/pieces_justificatives_service_spec.rb +++ b/spec/services/pieces_justificatives_service_spec.rb @@ -20,7 +20,18 @@ describe PiecesJustificativesService do attach_file_to_champ(pj_champ.call(witness)) end - it { expect(subject).to match_array([pj_champ.call(dossier).piece_justificative_file.attachment]) } + context 'with a single attachment' do + it { expect(subject).to match_array(pj_champ.call(dossier).piece_justificative_file.attachments) } + end + + context 'with a multiple attachments' do + before do + attach_file_to_champ(pj_champ.call(dossier)) + end + + it { expect(subject.count).to eq(2) } + it { expect(subject).to match_array(pj_champ.call(dossier).piece_justificative_file.attachments) } + end end context 'with a pj not safe on a champ' do @@ -46,7 +57,7 @@ describe PiecesJustificativesService do attach_file_to_champ(private_pj_champ.call(witness)) end - it { expect(subject).to match_array([private_pj_champ.call(dossier).piece_justificative_file.attachment]) } + it { expect(subject).to match_array(private_pj_champ.call(dossier).piece_justificative_file.attachments) } context 'for expert' do let(:for_expert) { true } diff --git a/spec/views/shared/attachment/_update.html.haml_spec.rb b/spec/views/shared/attachment/_update.html.haml_spec.rb index 2f120f12d..10d689243 100644 --- a/spec/views/shared/attachment/_update.html.haml_spec.rb +++ b/spec/views/shared/attachment/_update.html.haml_spec.rb @@ -6,7 +6,7 @@ describe 'shared/attachment/_update.html.haml', type: :view do 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, template:) + view.render Attachment::EditComponent.new(form: form, attached_file: attached_file, user_can_destroy: true, direct_upload: true) end end @@ -65,26 +65,4 @@ describe 'shared/attachment/_update.html.haml', type: :view do is_expected.not_to have_link('Supprimer') end end - - context 'when champ has a template' do - let(:profil) { :user } - let(:template) { champ.type_de_champ.piece_justificative_template } - - before do - allow_any_instance_of(ActionView::Base).to receive(:administrateur_signed_in?).and_return(profil == :administrateur) - end - - it 'renders a link to template' do - expect(subject).to have_link('le modèle suivant') - expect(subject).not_to have_text("éphémère") - end - - context 'as an administrator' do - let(:profil) { :administrateur } - it 'warn about ephemeral template url' do - expect(subject).to have_link('le modèle suivant') - expect(subject).to have_text("éphémère") - end - end - end end