From e423da93195035c900c7b3cb23af10613bfb8f39 Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Fri, 17 May 2019 17:34:41 +0200 Subject: [PATCH 01/15] Fix unclosed div tags in manager views --- app/views/manager/administrateurs/show.html.erb | 2 +- app/views/manager/dossiers/show.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/manager/administrateurs/show.html.erb b/app/views/manager/administrateurs/show.html.erb index 600809365..8d152a733 100644 --- a/app/views/manager/administrateurs/show.html.erb +++ b/app/views/manager/administrateurs/show.html.erb @@ -36,7 +36,7 @@ as well as a link to its edit page. <% if page.resource.invitation_expired? %> <%= link_to "renvoyer l'invitation", reinvite_manager_administrateur_path(page.resource), method: :post, class: "button" %> <% end %> -
+
diff --git a/app/views/manager/dossiers/show.html.erb b/app/views/manager/dossiers/show.html.erb index 636962cd6..e2eacd19c 100644 --- a/app/views/manager/dossiers/show.html.erb +++ b/app/views/manager/dossiers/show.html.erb @@ -34,7 +34,7 @@ as well as a link to its edit page. <% if dossier.hidden_at.nil? %> <%= link_to 'Supprimer le dossier', hide_manager_dossier_path(dossier), method: :post, class: 'button', data: { confirm: "Confirmez vous la suppression du dossier ?" } %> <% end %> -
+
From 855d4d024533d5e1ec392962b1b681919da89715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chai=CC=88b=20Martinez?= Date: Wed, 15 May 2019 14:43:27 +0200 Subject: [PATCH 02/15] [fix #3867] Add legal text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Chaïb Martinez --- app/views/admin/procedures/show.html.haml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/app/views/admin/procedures/show.html.haml b/app/views/admin/procedures/show.html.haml index ea2c586b4..6c3782092 100644 --- a/app/views/admin/procedures/show.html.haml +++ b/app/views/admin/procedures/show.html.haml @@ -67,7 +67,24 @@ %p Toute personne ayant la connaissance de ce lien pourra ainsi remplir des dossiers de test sur votre démarche. %br + %h4 Protection des Données personnelles + %p + À ce moment du processus de création, vous devez informer votre Délégué à la Protection des Données personnelles (DPD). + ( + %a{ href:'https://www.cnil.fr/fr/protection-des-donnees-les-bons-reflexes', target:'_blank' } + https://www.cnil.fr/fr/protection-des-donnees-les-bons-reflexes + ) + Si votre démarche propose de collecter des données personnelles, vous devez informer votre DPD. Chaque organisme en a un. + %p + Ce dernier pourra vous aider dans la finalisation de votre démarche, et vous inviter à vous interroger sur les données collectées, et sur la pertinence de ses dernières. + N'oubliez pas : toutes les démarches qui contiennent des données personnelles doivent être consignées dans un registre des traitements : + %a{ href:'https://www.cnil.fr/fr/RGDP-le-registre-des-activites-de-traitement', target:'_blank' } + https://www.cnil.fr/fr/RGDP-le-registre-des-activites-de-traitement + %p + Comment faire : + vous pouvez soit lui communiquer par email le lien vers la démarche en test, ou bien le nommer « administrateur ». Dans tous les cas, ne publiez votre démarche qu’après avoir eu son avis. + %br %h4 Ce que vous pouvez faire lorsque vous êtes en test %p Profitez de la phase de test pour tester la saisie de dossiers, ainsi que toutes les fonctionnalités associées (instruction, emails automatiques, attestations, etc.). From 49f01273008de9635c83424dfed4166c4027975f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chai=CC=88b=20Martinez?= Date: Mon, 13 May 2019 12:23:01 +0200 Subject: [PATCH 03/15] [fix #3856] Add form contact admin link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Chaïb Martinez --- app/views/users/sessions/link_sent.html.haml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/users/sessions/link_sent.html.haml b/app/views/users/sessions/link_sent.html.haml index 18b26242a..348a71f0d 100644 --- a/app/views/users/sessions/link_sent.html.haml +++ b/app/views/users/sessions/link_sent.html.haml @@ -17,4 +17,6 @@ Si vous voyez cette page trop souvent, consultez notre aide : #{link_to 'https://faq.demarches-simplifiees.fr/article/34-je-dois-confirmer-mon-compte-a-chaque-connexion', 'https://faq.demarches-simplifiees.fr/article/34-je-dois-confirmer-mon-compte-a-chaque-connexion', target: '_blank', rel: 'noopener' } %br %br - En cas de difficultés, nous restons joignables sur #{link_to CONTACT_EMAIL, "mailto:#{CONTACT_EMAIL}"}. + En cas de difficultés, nous restons joignables + = succeed '.' do + = link_to("via ce formulaire", contact_admin_url) From 4a9ef5d12ec9817d362df941ce8044a5741486d6 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 21 May 2019 11:36:19 +0200 Subject: [PATCH 04/15] Always use purge_later --- app/controllers/gestionnaires/dossiers_controller.rb | 2 +- app/controllers/users/dossiers_controller.rb | 2 +- app/models/champs/piece_justificative_champ.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/gestionnaires/dossiers_controller.rb b/app/controllers/gestionnaires/dossiers_controller.rb index d7f7d06fa..02512b173 100644 --- a/app/controllers/gestionnaires/dossiers_controller.rb +++ b/app/controllers/gestionnaires/dossiers_controller.rb @@ -143,7 +143,7 @@ module Gestionnaires def purge_champ_piece_justificative @champ = dossier.champs_private.find(params[:champ_id]) - @champ.piece_justificative_file.purge + @champ.piece_justificative_file.purge_later flash.notice = 'La pièce jointe a bien été supprimée.' end diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 9f3bce5e9..420a81891 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -239,7 +239,7 @@ module Users def purge_champ_piece_justificative @champ = dossier.champs.find(params[:champ_id]) - @champ.piece_justificative_file.purge + @champ.piece_justificative_file.purge_later flash.notice = 'La pièce jointe a bien été supprimée.' end diff --git a/app/models/champs/piece_justificative_champ.rb b/app/models/champs/piece_justificative_champ.rb index 00ec3a559..27bda0eb8 100644 --- a/app/models/champs/piece_justificative_champ.rb +++ b/app/models/champs/piece_justificative_champ.rb @@ -40,7 +40,7 @@ class Champs::PieceJustificativeChamp < Champ end if errors.present? - piece_justificative_file.purge + piece_justificative_file.purge_later end errors From f833f57e457debf323ef240aa26d0a97fc48d3d4 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 21 May 2019 11:42:21 +0200 Subject: [PATCH 05/15] Add PurgeUnattachedBlobsJob --- README.md | 1 + app/jobs/purge_unattached_blobs_job.rb | 9 +++++++++ 2 files changed, 10 insertions(+) create mode 100644 app/jobs/purge_unattached_blobs_job.rb diff --git a/README.md b/README.md index ed501dad8..3590f6833 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,7 @@ En local, un utilisateur de test est créé automatiquement, avec les identifian Administrateurs::ActivateBeforeExpirationJob.set(cron: "0 8 * * *").perform_later WarnExpiringDossiersJob.set(cron: "0 0 1 * *").perform_later GestionnaireEmailNotificationJob.set(cron: "0 10 * * 1,2,3,4,5,6").perform_later + PurgeUnattachedBlobsJob.set("0 0 * * *").perform_later ### Voir les emails envoyés en local diff --git a/app/jobs/purge_unattached_blobs_job.rb b/app/jobs/purge_unattached_blobs_job.rb new file mode 100644 index 000000000..fb5d5804e --- /dev/null +++ b/app/jobs/purge_unattached_blobs_job.rb @@ -0,0 +1,9 @@ +class PurgeUnattachedBlobsJob < ApplicationJob + queue_as :cron + + def perform(*args) + ActiveStorage::Blob.unattached + .where('active_storage_blobs.created_at < ?', 24.hours.ago) + .find_each(&:purge_later) + end +end From f6421e081aaa7ac92fbc2438d407235129208078 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 21 May 2019 14:21:13 +0200 Subject: [PATCH 06/15] Refactor attachment view --- .../gestionnaires/avis/instruction.html.haml | 2 +- .../gestionnaires/shared/avis/_list.html.haml | 3 ++- .../show/_download_justificatif.html.haml | 14 ++-------- app/views/shared/attachment/_show.html.haml | 26 +++++++++++++++++++ .../_update.html.haml} | 2 +- .../piece_justificative/_show.html.haml | 2 +- .../dossiers/_infos_generales.html.haml | 2 +- .../_piece_justificative.html.haml | 2 +- .../shared/piece_jointe/_pj_link.html.haml | 20 -------------- .../_show.html.haml_spec.rb} | 4 +-- 10 files changed, 37 insertions(+), 40 deletions(-) create mode 100644 app/views/shared/attachment/_show.html.haml rename app/views/shared/{piece_jointe/_pj_upload_field.haml => attachment/_update.html.haml} (80%) delete mode 100644 app/views/shared/piece_jointe/_pj_link.html.haml rename spec/views/shared/{piece_jointe/_pj_link.html.haml_spec.rb => attachment/_show.html.haml_spec.rb} (90%) diff --git a/app/views/gestionnaires/avis/instruction.html.haml b/app/views/gestionnaires/avis/instruction.html.haml index 3d81ac7b5..25f03d299 100644 --- a/app/views/gestionnaires/avis/instruction.html.haml +++ b/app/views/gestionnaires/avis/instruction.html.haml @@ -13,7 +13,7 @@ = form_for @avis, url: gestionnaire_avis_path(@avis), html: { class: 'form' } do |f| = f.text_area :answer, rows: 3, placeholder: 'Votre avis', required: true - = render partial: "shared/piece_jointe/pj_upload_field", locals: { pj: @avis.piece_justificative_file, object: @avis, form: f } + = render partial: "shared/attachment/update", locals: { pj: @avis.piece_justificative_file, object: @avis, form: f } .flex.justify-between.align-baseline %p.confidentiel.flex diff --git a/app/views/gestionnaires/shared/avis/_list.html.haml b/app/views/gestionnaires/shared/avis/_list.html.haml index 25f6866ed..cc3c25282 100644 --- a/app/views/gestionnaires/shared/avis/_list.html.haml +++ b/app/views/gestionnaires/shared/avis/_list.html.haml @@ -28,6 +28,7 @@ Réponse donnée le #{l(avis.updated_at, format: '%d/%m/%y à %H:%M')} - else %span.waiting En attente de réponse - = render partial: 'shared/piece_jointe/pj_link', locals: { pj: avis.piece_justificative_file, user_can_upload: false } + - if avis.piece_justificative_file.attached? + = render partial: 'shared/attachment/show', locals: { attachment: avis.piece_justificative_file.attachment } .answer-body = simple_format(avis.answer) diff --git a/app/views/new_user/dossiers/show/_download_justificatif.html.haml b/app/views/new_user/dossiers/show/_download_justificatif.html.haml index 3df94f6bd..ce596c299 100644 --- a/app/views/new_user/dossiers/show/_download_justificatif.html.haml +++ b/app/views/new_user/dossiers/show/_download_justificatif.html.haml @@ -1,12 +1,2 @@ -- if dossier.present? - - justificatif = dossier.justificatif_motivation - - if justificatif.attached? - - if justificatif.virus_scanner.safe? - .action - = link_to (justificatif), target: '_blank', class: 'button primary' do - %span.icon.download - Télécharger le justificatif - - elsif justificatif.virus_scanner.pending? - %p - Un justificatif a été joint. L'analyse antivirus de ce document est en cours. - = link_to "rafraichir", request.path +- if dossier.present? && dossier.justificatif_motivation.attached? + = render partial: "shared/attachment/show", locals: { attachment: dossier.justificatif_motivation.attachment } diff --git a/app/views/shared/attachment/_show.html.haml b/app/views/shared/attachment/_show.html.haml new file mode 100644 index 000000000..501a6da7d --- /dev/null +++ b/app/views/shared/attachment/_show.html.haml @@ -0,0 +1,26 @@ +- should_display_link = attachment.virus_scanner.safe? || !attachment.virus_scanner.started? +- user_can_upload = defined?(user_can_upload) ? user_can_upload : false +- if should_display_link + - attachment_check_url = false +- else + - attachment_check_url = attachment_url(attachment.id, { signed_id: attachment.blob.signed_id, user_can_upload: user_can_upload }) + +.pj-link{ 'data-attachment-id': attachment.id, 'data-attachment-check-url': attachment_check_url } + - if should_display_link + = link_to url_for(attachment.blob), target: '_blank', rel: 'noopener', title: "Télécharger la pièce jointe" do + %span.icon.attachment + = attachment.filename.to_s + - if !attachment.virus_scanner.started? + (ce fichier n’a pas été analysé par notre antivirus, téléchargez-le avec précaution) + + - else + = attachment.filename.to_s + - if attachment.virus_scanner.pending? + (analyse antivirus en cours + = link_to "rafraichir", request.path + ) + - elsif attachment.virus_scanner.infected? + - if user_can_upload + (virus détecté, merci d’envoyer un autre fichier) + - else + (virus détecté, le téléchargement de ce fichier est bloqué) diff --git a/app/views/shared/piece_jointe/_pj_upload_field.haml b/app/views/shared/attachment/_update.html.haml similarity index 80% rename from app/views/shared/piece_jointe/_pj_upload_field.haml rename to app/views/shared/attachment/_update.html.haml index 8faf0d3de..eb5c9dc83 100644 --- a/app/views/shared/piece_jointe/_pj_upload_field.haml +++ b/app/views/shared/attachment/_update.html.haml @@ -2,7 +2,7 @@ - if pj.attached? .piece-justificative-actions{ id: "piece_justificative_#{object.id}" } .piece-justificative-action - = render partial: "shared/piece_jointe/pj_link", locals: { pj: pj, user_can_upload: true } + = render partial: "shared/attachment/show", locals: { attachment: pj.attachment, user_can_upload: true } .piece-justificative-action = button_tag 'Remplacer', type: 'button', class: 'button small', data: { 'toggle-target': "#champs_#{object.id}" } diff --git a/app/views/shared/champs/piece_justificative/_show.html.haml b/app/views/shared/champs/piece_justificative/_show.html.haml index c02510d25..a9d7ae19c 100644 --- a/app/views/shared/champs/piece_justificative/_show.html.haml +++ b/app/views/shared/champs/piece_justificative/_show.html.haml @@ -1,5 +1,5 @@ - pj = champ.piece_justificative_file - if pj.attached? - = render partial: "shared/piece_jointe/pj_link", locals: { pj: pj, user_can_upload: false } + = render partial: "shared/attachment/show", locals: { attachment: pj.attachment } - else Pièce justificative non fournie diff --git a/app/views/shared/dossiers/_infos_generales.html.haml b/app/views/shared/dossiers/_infos_generales.html.haml index 9d2748899..e4b139f72 100644 --- a/app/views/shared/dossiers/_infos_generales.html.haml +++ b/app/views/shared/dossiers/_infos_generales.html.haml @@ -8,4 +8,4 @@ %th.libelle Justificatif : %td .action - = render partial: 'shared/piece_jointe/pj_link', locals: { object: dossier, pj: dossier.justificatif_motivation } + = render partial: 'shared/attachment/show', locals: { attachment: dossier.justificatif_motivation.attachment } diff --git a/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml b/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml index 63cbc411b..346317669 100644 --- a/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml +++ b/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml @@ -9,7 +9,7 @@ - if pj.attached? .piece-justificative-actions{ id: "piece_justificative_#{champ.id}" } .piece-justificative-action - = render partial: "shared/piece_jointe/pj_link", locals: { pj: pj, user_can_upload: true } + = render partial: "shared/attachment/show", locals: { attachment: pj.attachment, user_can_upload: true } .piece-justificative-action - if champ.private? = link_to 'Supprimer', gestionnaire_champ_purge_champ_piece_justificative_path(procedure_id: champ.dossier.procedure_id, dossier_id: champ.dossier_id, champ_id: champ.id), remote: true, method: :delete, class: 'button small danger' diff --git a/app/views/shared/piece_jointe/_pj_link.html.haml b/app/views/shared/piece_jointe/_pj_link.html.haml deleted file mode 100644 index 1821ebf70..000000000 --- a/app/views/shared/piece_jointe/_pj_link.html.haml +++ /dev/null @@ -1,20 +0,0 @@ -- if pj.attached? - .pj-link - - if pj.virus_scanner.safe? || !pj.virus_scanner.started? - = link_to url_for(pj), target: '_blank', rel: 'noopener', title: "Télécharger la pièce jointe" do - %span.icon.attachment - = pj.filename.to_s - - if !pj.virus_scanner.started? - (ce fichier n’a pas été analysé par notre antivirus, téléchargez-le avec précaution) - - - else - = pj.filename.to_s - - if pj.virus_scanner.pending? - (analyse antivirus en cours - = link_to "rafraichir", request.path - ) - - elsif pj.virus_scanner.infected? - - if user_can_upload - (virus détecté, merci d’envoyer un autre fichier) - - else - (virus détecté, le téléchargement de ce fichier est bloqué) diff --git a/spec/views/shared/piece_jointe/_pj_link.html.haml_spec.rb b/spec/views/shared/attachment/_show.html.haml_spec.rb similarity index 90% rename from spec/views/shared/piece_jointe/_pj_link.html.haml_spec.rb rename to spec/views/shared/attachment/_show.html.haml_spec.rb index 04fbe495b..81cf9024d 100644 --- a/spec/views/shared/piece_jointe/_pj_link.html.haml_spec.rb +++ b/spec/views/shared/attachment/_show.html.haml_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -describe 'shared/piece_jointe/_pj_link.html.haml', type: :view do +describe 'shared/attachment/_show.html.haml', type: :view do let(:champ) { create(:champ_piece_justificative) } let(:virus_scan_result) { nil } @@ -8,7 +8,7 @@ describe 'shared/piece_jointe/_pj_link.html.haml', type: :view do champ.piece_justificative_file.blob.update(metadata: champ.piece_justificative_file.blob.metadata.merge(virus_scan_result: virus_scan_result)) end - subject { render 'shared/piece_jointe/pj_link', pj: champ.piece_justificative_file, user_can_upload: false } + subject { render 'shared/attachment/show', attachment: champ.piece_justificative_file.attachment } context 'when there is no anti-virus scan' do let(:virus_scan_result) { nil } From cc4eba2b369082506658110832bcfd2bb38fdf60 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 21 May 2019 14:21:42 +0200 Subject: [PATCH 07/15] Less mokey patching --- config/initializers/active_storage.rb | 18 ------------------ spec/models/champ_spec.rb | 6 ------ 2 files changed, 24 deletions(-) diff --git a/config/initializers/active_storage.rb b/config/initializers/active_storage.rb index 121d0c3f1..e9fb875dd 100644 --- a/config/initializers/active_storage.rb +++ b/config/initializers/active_storage.rb @@ -1,21 +1,3 @@ ActiveStorage::Service.url_expires_in = 1.hour -# We want to run the virus scan on every ActiveStorage attachment, -# regardless of its type (user-uploaded document, instructeur-uploaded attestation, form template, etc.) -# -# To do this, the best place to work on is the ActiveStorage::Attachment -# objects themselves. -# -# We have to monkey patch ActiveStorage in order to always run an analyzer. -# The way analyzable blob interface work is by running the first accepted analyzer. -# This is not what we want for the virus scan. Using analyzer interface is still beneficial -# as it takes care of downloading the blob. -ActiveStorage::Attached::One.class_eval do - def virus_scanner - if attached? - ActiveStorage::VirusScanner.new(attachment.blob) - end - end -end - ActiveSupport.on_load(:active_storage_blob) { include BlobVirusScanner } diff --git a/spec/models/champ_spec.rb b/spec/models/champ_spec.rb index 73845b53a..9aff08c62 100644 --- a/spec/models/champ_spec.rb +++ b/spec/models/champ_spec.rb @@ -390,12 +390,6 @@ describe Champ do it { expect(champ.piece_justificative_file.virus_scanner.started?).to be_truthy } end - - context 'and there is no blob' do - before { champ.save } - - it { expect(champ.piece_justificative_file.virus_scanner).to be_nil } - end end end From 6a3413018a440dea0959c34f4658f98c50a039ff Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 21 May 2019 14:21:55 +0200 Subject: [PATCH 08/15] Refresh attachments with virus scan result --- app/controllers/attachments_controller.rb | 9 +++ app/javascript/packs/application.js | 1 + .../activestorage/attachment-checker.js | 66 +++++++++++++++++++ app/views/attachments/show.js.erb | 8 +++ config/routes.rb | 2 + 5 files changed, 86 insertions(+) create mode 100644 app/controllers/attachments_controller.rb create mode 100644 app/javascript/shared/activestorage/attachment-checker.js create mode 100644 app/views/attachments/show.js.erb diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb new file mode 100644 index 000000000..c2edd378c --- /dev/null +++ b/app/controllers/attachments_controller.rb @@ -0,0 +1,9 @@ +class AttachmentsController < ApplicationController + before_action :authenticate_logged_user! + include ActiveStorage::SetBlob + + def show + @attachment = @blob.attachments.find(params[:id]) + @user_can_upload = params[:user_can_upload] + end +end diff --git a/app/javascript/packs/application.js b/app/javascript/packs/application.js index ba665e4bb..6995b655b 100644 --- a/app/javascript/packs/application.js +++ b/app/javascript/packs/application.js @@ -9,6 +9,7 @@ import ReactUJS from '../shared/react-ujs'; import reactComponents from '../shared/react-components'; import '../shared/activestorage/ujs'; +import '../shared/activestorage/attachment-checker'; import '../shared/rails-ujs-fix'; import '../shared/safari-11-file-xhr-workaround'; import '../shared/autocomplete'; diff --git a/app/javascript/shared/activestorage/attachment-checker.js b/app/javascript/shared/activestorage/attachment-checker.js new file mode 100644 index 000000000..326c6240a --- /dev/null +++ b/app/javascript/shared/activestorage/attachment-checker.js @@ -0,0 +1,66 @@ +import Rails from 'rails-ujs'; + +addEventListener('turbolinks:load', () => { + checker.deactivate(); + + const attachments = document.querySelectorAll('[data-attachment-check-url]'); + + for (let attachment of attachments) { + checker.add(attachment.dataset.attachmentCheckUrl); + } +}); + +addEventListener('attachment:update', ({ detail: { url } }) => { + checker.add(url); +}); + +class AttachmentChecker { + urls = new Set(); + timeout; + checks = 0; + + constructor(settings = {}) { + this.interval = settings.interval || 5000; + this.maxChecks = settings.maxChecks || 5; + } + + get isEnabled() { + return this.checks <= this.maxChecks; + } + + get isActive() { + return this.timeout !== undefined; + } + + add(url) { + if (this.isEnabled) { + if (!this.isActive) { + this.activate(); + } + this.urls.add(url); + } + } + + activate() { + this.timeout = setTimeout(() => { + for (let url of this.urls) { + Rails.ajax({ url, type: 'get' }); + } + this.checks++; + this.reset(); + }, this.interval); + } + + deactivate() { + this.checks = 0; + this.reset(); + } + + reset() { + clearTimeout(this.timeout); + this.urls = new Set(); + this.timeout = undefined; + } +} + +const checker = new AttachmentChecker(); diff --git a/app/views/attachments/show.js.erb b/app/views/attachments/show.js.erb new file mode 100644 index 000000000..fb65942b8 --- /dev/null +++ b/app/views/attachments/show.js.erb @@ -0,0 +1,8 @@ +<%= render_to_element(".pj-link[data-attachment-id=\"#{@attachment.id}\"]", + partial: 'shared/attachment/show', + outer: true, + locals: { attachment: @attachment, user_can_upload: @user_can_upload }) %> + +<% if @attachment.virus_scanner.pending? %> + <%= fire_event('attachment:update', { url: attachment_url(@attachment.id, { signed_id: @attachment.blob.signed_id, user_can_upload: @user_can_upload }) }.to_json ) %> +<% end %> diff --git a/config/routes.rb b/config/routes.rb index 00979c57a..16c0d618a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -133,6 +133,8 @@ Rails.application.routes.draw do post ':position/repetition', to: 'repetition#show', as: :repetition end + get 'attachments/:id', to: 'attachments#show', as: :attachment + get 'tour-de-france' => 'root#tour_de_france' get "patron" => "root#patron" get "accessibilite" => "root#accessibilite" From 6797c01b693301d3c449cbce85cb8b39b0e6b083 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 22 May 2019 12:00:05 +0200 Subject: [PATCH 09/15] Properly handle justificatif_motivation case --- .../activestorage/attachment-checker.js | 21 ++++++++++++++----- app/javascript/shared/utils.js | 2 +- .../dossiers/_state_button.html.haml | 2 +- .../dossiers/_state_button_refresh.js.erb | 5 +++++ app/views/shared/attachment/_show.html.haml | 2 +- .../show/_download_justificatif.html.haml | 0 .../dossiers/show/_status_overview.html.haml | 6 +++--- 7 files changed, 27 insertions(+), 11 deletions(-) rename app/views/{new_user => users}/dossiers/show/_download_justificatif.html.haml (100%) diff --git a/app/javascript/shared/activestorage/attachment-checker.js b/app/javascript/shared/activestorage/attachment-checker.js index 326c6240a..724e1a379 100644 --- a/app/javascript/shared/activestorage/attachment-checker.js +++ b/app/javascript/shared/activestorage/attachment-checker.js @@ -1,4 +1,4 @@ -import Rails from 'rails-ujs'; +import { ajax, delegate } from '@utils'; addEventListener('turbolinks:load', () => { checker.deactivate(); @@ -14,6 +14,11 @@ addEventListener('attachment:update', ({ detail: { url } }) => { checker.add(url); }); +delegate('click', '[data-attachment-refresh]', event => { + event.preventDefault(); + checker.check(); +}); + class AttachmentChecker { urls = new Set(); timeout; @@ -41,13 +46,19 @@ class AttachmentChecker { } } + check() { + let urls = this.urls; + this.reset(); + for (let url of urls) { + ajax({ url, type: 'get' }); + } + } + activate() { + clearTimeout(this.timeout); this.timeout = setTimeout(() => { - for (let url of this.urls) { - Rails.ajax({ url, type: 'get' }); - } this.checks++; - this.reset(); + this.check(); }, this.interval); } diff --git a/app/javascript/shared/utils.js b/app/javascript/shared/utils.js index cb1e62c9d..f78c376fc 100644 --- a/app/javascript/shared/utils.js +++ b/app/javascript/shared/utils.js @@ -3,7 +3,7 @@ import $ from 'jquery'; import debounce from 'debounce'; export { debounce }; -export const { fire } = Rails; +export const { fire, ajax } = Rails; export function show({ classList }) { classList.remove('hidden'); diff --git a/app/views/gestionnaires/dossiers/_state_button.html.haml b/app/views/gestionnaires/dossiers/_state_button.html.haml index 17986293e..18d0a42be 100644 --- a/app/views/gestionnaires/dossiers/_state_button.html.haml +++ b/app/views/gestionnaires/dossiers/_state_button.html.haml @@ -60,7 +60,7 @@ - if dossier.motivation.present? %h4 Motivation %p.dossier-motivation= dossier.motivation - = render partial: 'new_user/dossiers/show/download_justificatif', locals: { dossier: dossier } + = render partial: 'users/dossiers/show/download_justificatif', locals: { dossier: dossier } - if dossier.attestation.present? %h4 Attestation diff --git a/app/views/gestionnaires/dossiers/_state_button_refresh.js.erb b/app/views/gestionnaires/dossiers/_state_button_refresh.js.erb index 73945d956..1a24de77b 100644 --- a/app/views/gestionnaires/dossiers/_state_button_refresh.js.erb +++ b/app/views/gestionnaires/dossiers/_state_button_refresh.js.erb @@ -1,2 +1,7 @@ <%= render_flash %> <%= render_to_element('.state-button', partial: "state_button", locals: { dossier: dossier }) %> + +<% attachment = dossier.justificatif_motivation.attachment %> +<% if attachment && attachment.virus_scanner.pending? %> + <%= fire_event('attachment:update', { url: attachment_url(attachment.id, { signed_id: attachment.blob.signed_id }) }.to_json ) %> +<% end %> diff --git a/app/views/shared/attachment/_show.html.haml b/app/views/shared/attachment/_show.html.haml index 501a6da7d..1da36cdbd 100644 --- a/app/views/shared/attachment/_show.html.haml +++ b/app/views/shared/attachment/_show.html.haml @@ -17,7 +17,7 @@ = attachment.filename.to_s - if attachment.virus_scanner.pending? (analyse antivirus en cours - = link_to "rafraichir", request.path + = link_to "rafraichir", request.path, data: { 'attachment-refresh': true } ) - elsif attachment.virus_scanner.infected? - if user_can_upload diff --git a/app/views/new_user/dossiers/show/_download_justificatif.html.haml b/app/views/users/dossiers/show/_download_justificatif.html.haml similarity index 100% rename from app/views/new_user/dossiers/show/_download_justificatif.html.haml rename to app/views/users/dossiers/show/_download_justificatif.html.haml diff --git a/app/views/users/dossiers/show/_status_overview.html.haml b/app/views/users/dossiers/show/_status_overview.html.haml index e2d16b5db..b5504237f 100644 --- a/app/views/users/dossiers/show/_status_overview.html.haml +++ b/app/views/users/dossiers/show/_status_overview.html.haml @@ -50,7 +50,7 @@ %h3 Motif de l’acceptation %blockquote= dossier.motivation - = render partial: 'new_user/dossiers/show/download_justificatif', locals: { dossier: dossier } + = render partial: 'users/dossiers/show/download_justificatif', locals: { dossier: dossier } - if dossier.attestation.present? .action @@ -71,7 +71,7 @@ %h3 Motif du refus %blockquote= dossier.motivation - = render partial: 'new_user/dossiers/show/download_justificatif', locals: { dossier: @dossier } + = render partial: 'users/dossiers/show/download_justificatif', locals: { dossier: dossier } .action = link_to 'Envoyer un message à l’administration', messagerie_dossier_url(dossier, anchor: 'new_commentaire'), class: 'button' @@ -83,7 +83,7 @@ = succeed '.' do %strong sans suite - = render partial: 'new_user/dossiers/show/download_justificatif', locals: { dossier: @dossier } + = render partial: 'users/dossiers/show/download_justificatif', locals: { dossier: dossier } - if dossier.motivation.present? %h3 Motif du classement sans suite From f06ae1631f2df448b5b03dc07f5954220f6726a4 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Wed, 22 May 2019 14:34:46 +0200 Subject: [PATCH 10/15] no crash when q is missing on RechercheController::index --- app/services/dossier_search_service.rb | 3 ++- .../gestionnaires/recherche_controller_spec.rb | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index 8bb952085..b2947edd5 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -38,7 +38,8 @@ class DossierSearchService end def self.to_tsquery(search_terms) - search_terms.strip + (search_terms || "") + .strip .gsub(/['?\\:&|!<>\(\)]/, "") # drop disallowed characters .split(/\s+/) # split words .map { |x| "#{x}:*" } # enable prefix matching diff --git a/spec/controllers/gestionnaires/recherche_controller_spec.rb b/spec/controllers/gestionnaires/recherche_controller_spec.rb index 6d33e190a..a54c97c1d 100644 --- a/spec/controllers/gestionnaires/recherche_controller_spec.rb +++ b/spec/controllers/gestionnaires/recherche_controller_spec.rb @@ -48,5 +48,16 @@ describe Gestionnaires::RechercheController, type: :controller do end end end + + context 'with no query param it does not crash' do + subject { get :index, params: {} } + + it { is_expected.to have_http_status(200) } + + it 'returns 0 dossier' do + subject + expect(assigns(:dossiers).count).to eq(0) + end + end end end From 920c8c0c32b625e801ef2e074643c28fe229c905 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chai=CC=88b=20Martinez?= Date: Mon, 27 May 2019 16:30:03 +0200 Subject: [PATCH 11/15] Add Gestionnaire ID and ROLES MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Chaïb Martinez --- app/controllers/application_controller.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5dbfd7a61..c87f47a4d 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -231,7 +231,9 @@ class ApplicationController < ActionController::Base DS_SIGN_IN_COUNT: current_administrateur&.sign_in_count, DS_CREATED_AT: current_administrateur&.created_at, DS_ACTIVE: current_administrateur&.active, - DS_ID: current_administrateur&.id + DS_ID: current_administrateur&.id, + DS_GESTIONNAIRE_ID: current_gestionnaire&.id, + DS_ROLES: logged_user_roles } } } From 21ac60ad04fdb0abca6e6ab5ea73615b4a385980 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 16 May 2019 15:26:58 +0200 Subject: [PATCH 12/15] tasks: add a task to migrate pjs of procedures in batches --- ...to_champ_piece_jointe_migration_service.rb | 5 +++ .../2019_03_13_migrate_pjs_to_champs.rake | 8 ----- lib/tasks/pieces_justificatives.rake | 28 ++++++++++++++++ spec/lib/tasks/pieces_justificatives_spec.rb | 32 +++++++++++++++++++ 4 files changed, 65 insertions(+), 8 deletions(-) delete mode 100644 lib/tasks/2019_03_13_migrate_pjs_to_champs.rake create mode 100644 lib/tasks/pieces_justificatives.rake create mode 100644 spec/lib/tasks/pieces_justificatives_spec.rb diff --git a/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb b/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb index 0778454f3..52314b697 100644 --- a/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb +++ b/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb @@ -9,6 +9,11 @@ class PieceJustificativeToChampPieceJointeMigrationService storage_service.ensure_openstack_copy_possible!(PieceJustificativeUploader) end + def procedures_with_pjs_in_range(ids_range) + procedures_with_pj = Procedure.unscope(where: :hidden_at).joins(:types_de_piece_justificative).distinct + procedures_with_pj.where(id: ids_range) + end + def convert_procedure_pjs_to_champ_pjs(procedure) types_de_champ_pj = PiecesJustificativesService.types_pj_as_types_de_champ(procedure) populate_champs_pjs!(procedure, types_de_champ_pj) diff --git a/lib/tasks/2019_03_13_migrate_pjs_to_champs.rake b/lib/tasks/2019_03_13_migrate_pjs_to_champs.rake deleted file mode 100644 index ad8194116..000000000 --- a/lib/tasks/2019_03_13_migrate_pjs_to_champs.rake +++ /dev/null @@ -1,8 +0,0 @@ -namespace :'2019_03_13_migrate_pjs_to_champs' do - task run: :environment do - procedure_id = ENV['PROCEDURE_ID'] - service = PieceJustificativeToChampPieceJointeMigrationService.new - service.ensure_correct_storage_configuration! - service.convert_procedure_pjs_to_champ_pjs(Procedure.find(procedure_id)) - end -end diff --git a/lib/tasks/pieces_justificatives.rake b/lib/tasks/pieces_justificatives.rake new file mode 100644 index 000000000..a30855090 --- /dev/null +++ b/lib/tasks/pieces_justificatives.rake @@ -0,0 +1,28 @@ +require Rails.root.join("lib", "tasks", "task_helper") + +namespace :pieces_justificatives do + task migrate_procedure_to_champs: :environment do + procedure_id = ENV['PROCEDURE_ID'] + service = PieceJustificativeToChampPieceJointeMigrationService.new + service.ensure_correct_storage_configuration! + service.convert_procedure_pjs_to_champ_pjs(Procedure.find(procedure_id)) + end + + task migrate_procedures_range_to_champs: :environment do + if ENV['RANGE_START'].nil? || ENV['RANGE_END'].nil? + fail "RANGE_START and RANGE_END must be specified" + end + procedures_range = ENV['RANGE_START']..ENV['RANGE_END'] + + service = PieceJustificativeToChampPieceJointeMigrationService.new + service.ensure_correct_storage_configuration! + procedures_to_migrate = service.procedures_with_pjs_in_range(procedures_range) + + procedures_to_migrate.find_each do |procedure| + rake_puts '' + rake_puts "Migrating procedure #{procedure.id}…" + + service.convert_procedure_pjs_to_champ_pjs(procedure) + end + end +end diff --git a/spec/lib/tasks/pieces_justificatives_spec.rb b/spec/lib/tasks/pieces_justificatives_spec.rb new file mode 100644 index 000000000..5031c772d --- /dev/null +++ b/spec/lib/tasks/pieces_justificatives_spec.rb @@ -0,0 +1,32 @@ +describe 'pieces_justificatives' do + describe 'migrate_procedures_range_to_champs' do + let(:rake_task) { Rake::Task['pieces_justificatives:migrate_procedures_range_to_champs'] } + let(:procedure_in_range_1) { create(:procedure, :with_two_type_de_piece_justificative) } + let(:procedure_in_range_2) { create(:procedure, :with_two_type_de_piece_justificative) } + let(:procedure_out_of_range) { create(:procedure, :with_two_type_de_piece_justificative) } + + before do + procedure_in_range_1 + procedure_in_range_2 + procedure_out_of_range + + ENV['RANGE_START'] = procedure_in_range_1.id.to_s + ENV['RANGE_END'] = procedure_in_range_2.id.to_s + + allow_any_instance_of(PieceJustificativeToChampPieceJointeMigrationService).to receive(:ensure_correct_storage_configuration!) + + rake_task.invoke + end + + after { rake_task.reenable } + + it 'migrates procedures in the ids range' do + expect(procedure_in_range_1.reload.types_de_piece_justificative).to be_empty + expect(procedure_in_range_2.reload.types_de_piece_justificative).to be_empty + end + + it 'doesn’t migrate procedures not in the range' do + expect(procedure_out_of_range.reload.types_de_piece_justificative).to be_present + end + end +end From 4cf54e0d28acec96bb8bb17df6ebc8c8e2902891 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 16 May 2019 13:37:33 +0000 Subject: [PATCH 13/15] tasks: add progress report to the pjs migration task Progress is indicated per migrated champ. --- ...to_champ_piece_jointe_migration_service.rb | 12 +++- lib/tasks/pieces_justificatives.rake | 22 +++++++- spec/lib/tasks/pieces_justificatives_spec.rb | 19 +++++++ ...amp_piece_jointe_migration_service_spec.rb | 56 +++++++++++++------ 4 files changed, 88 insertions(+), 21 deletions(-) diff --git a/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb b/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb index 52314b697..30b025201 100644 --- a/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb +++ b/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb @@ -14,9 +14,13 @@ class PieceJustificativeToChampPieceJointeMigrationService procedures_with_pj.where(id: ids_range) end - def convert_procedure_pjs_to_champ_pjs(procedure) + def number_of_champs_to_migrate(procedure) + (procedure.types_de_piece_justificative.count + 1) * procedure.dossiers.unscope(where: :hidden_at).count + end + + def convert_procedure_pjs_to_champ_pjs(procedure, &progress) types_de_champ_pj = PiecesJustificativesService.types_pj_as_types_de_champ(procedure) - populate_champs_pjs!(procedure, types_de_champ_pj) + populate_champs_pjs!(procedure, types_de_champ_pj, &progress) # Only destroy the old types PJ once everything has been safely migrated to # champs PJs. Destroying the types PJ will cascade and destroy the PJs, @@ -29,7 +33,7 @@ class PieceJustificativeToChampPieceJointeMigrationService @storage_service ||= CarrierwaveActiveStorageMigrationService.new end - def populate_champs_pjs!(procedure, types_de_champ_pj) + def populate_champs_pjs!(procedure, types_de_champ_pj, &progress) procedure.types_de_champ += types_de_champ_pj # Unscope to make sure all dossiers are migrated, even the soft-deleted ones @@ -54,6 +58,8 @@ class PieceJustificativeToChampPieceJointeMigrationService created_at: dossier.created_at ) end + + yield if block_given? end end rescue diff --git a/lib/tasks/pieces_justificatives.rake b/lib/tasks/pieces_justificatives.rake index a30855090..49fe00193 100644 --- a/lib/tasks/pieces_justificatives.rake +++ b/lib/tasks/pieces_justificatives.rake @@ -3,9 +3,18 @@ require Rails.root.join("lib", "tasks", "task_helper") namespace :pieces_justificatives do task migrate_procedure_to_champs: :environment do procedure_id = ENV['PROCEDURE_ID'] + procedure = Procedure.find(procedure_id) + service = PieceJustificativeToChampPieceJointeMigrationService.new service.ensure_correct_storage_configuration! - service.convert_procedure_pjs_to_champ_pjs(Procedure.find(procedure_id)) + + progress = ProgressReport.new(service.number_of_champs_to_migrate(procedure)) + + service.convert_procedure_pjs_to_champ_pjs(procedure) do + progress.inc + end + + progress.finish end task migrate_procedures_range_to_champs: :environment do @@ -18,11 +27,20 @@ namespace :pieces_justificatives do service.ensure_correct_storage_configuration! procedures_to_migrate = service.procedures_with_pjs_in_range(procedures_range) + total_number_of_champs_to_migrate = procedures_to_migrate + .map { |p| service.number_of_champs_to_migrate(p) } + .sum + progress = ProgressReport.new(total_number_of_champs_to_migrate) + procedures_to_migrate.find_each do |procedure| rake_puts '' rake_puts "Migrating procedure #{procedure.id}…" - service.convert_procedure_pjs_to_champ_pjs(procedure) + service.convert_procedure_pjs_to_champ_pjs(procedure) do + progress.inc + end end + + progress.finish end end diff --git a/spec/lib/tasks/pieces_justificatives_spec.rb b/spec/lib/tasks/pieces_justificatives_spec.rb index 5031c772d..fc929d51d 100644 --- a/spec/lib/tasks/pieces_justificatives_spec.rb +++ b/spec/lib/tasks/pieces_justificatives_spec.rb @@ -1,4 +1,23 @@ describe 'pieces_justificatives' do + describe 'migrate_procedure_to_champs' do + let(:rake_task) { Rake::Task['pieces_justificatives:migrate_procedure_to_champs'] } + let(:procedure) { create(:procedure, :with_two_type_de_piece_justificative) } + + before do + ENV['PROCEDURE_ID'] = procedure.id.to_s + + allow_any_instance_of(PieceJustificativeToChampPieceJointeMigrationService).to receive(:ensure_correct_storage_configuration!) + + rake_task.invoke + end + + after { rake_task.reenable } + + it 'migrates the procedure' do + expect(procedure.reload.types_de_piece_justificative).to be_empty + end + end + describe 'migrate_procedures_range_to_champs' do let(:rake_task) { Rake::Task['pieces_justificatives:migrate_procedures_range_to_champs'] } let(:procedure_in_range_1) { create(:procedure, :with_two_type_de_piece_justificative) } diff --git a/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb b/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb index 9f7f83e34..bbd94d8ef 100644 --- a/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb +++ b/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb @@ -9,16 +9,17 @@ describe PieceJustificativeToChampPieceJointeMigrationService do let(:procedure) { create(:procedure, types_de_piece_justificative: types_pj) } let(:types_pj) { [create(:type_de_piece_justificative)] } - let!(:dossier) do - create( - :dossier, - procedure: procedure, - pieces_justificatives: pjs - ) - end + let!(:dossier) { make_dossier } let(:pjs) { [] } + def make_dossier(hidden: false) + create(:dossier, + procedure: procedure, + pieces_justificatives: pjs, + hidden_at: hidden ? Time.zone.now : nil) + end + def make_pjs types_pj.map do |tpj| create(:piece_justificative, :contrat, type_de_piece_justificative: tpj) @@ -31,6 +32,22 @@ describe PieceJustificativeToChampPieceJointeMigrationService do expect(storage_service).to receive(:make_attachment) end + describe '.number_of_champs_to_migrate' do + let!(:other_dossier) { make_dossier } + + it 'reports the numbers of champs to be migrated' do + expect(service.number_of_champs_to_migrate(procedure)).to eq(4) + end + + context 'when the procedure has hidden dossiers' do + let!(:hidden_dossier) { make_dossier(hidden: true) } + + it 'reports the numbers of champs including those of hidden dossiers' do + expect(service.number_of_champs_to_migrate(procedure)).to eq(6) + end + end + end + context 'when conversion succeeds' do context 'for the procedure' do it 'types de champ are created for the "pièces jointes" header and for each PJ' do @@ -114,19 +131,26 @@ describe PieceJustificativeToChampPieceJointeMigrationService do .to change { dossier.pieces_justificatives.count } .to(0) end + + context 'when the procedure has several dossiers' do + let!(:other_dossier) { make_dossier } + + it 'sends progress callback for each migrated champ' do + number_of_champs_to_migrate = service.number_of_champs_to_migrate(procedure) + + progress_count = 0 + service.convert_procedure_pjs_to_champ_pjs(procedure) do + progress_count += 1 + end + + expect(progress_count).to eq(number_of_champs_to_migrate) + end + end end context 'when the dossier is soft-deleted it still gets converted' do let(:pjs) { make_pjs } - - let!(:dossier) do - create( - :dossier, - procedure: procedure, - pieces_justificatives: pjs, - hidden_at: Time.zone.now - ) - end + let!(:dossier) { make_dossier(hidden: true) } before { expect_storage_service_to_convert_object } From 52b7a829323a1d8ddca039d7b4d00f3033888122 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 28 May 2019 08:45:24 +0000 Subject: [PATCH 14/15] services: mark attachments migrated from CarrierWave as safe This avoids to enqueue thousands of scans when migrating the PJs of a whole procedure. --- ...erwave_active_storage_migration_service.rb | 5 +-- ...e_active_storage_migration_service_spec.rb | 32 +++++++++++++++++-- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/app/services/carrierwave_active_storage_migration_service.rb b/app/services/carrierwave_active_storage_migration_service.rb index 640643042..9d80f3be3 100644 --- a/app/services/carrierwave_active_storage_migration_service.rb +++ b/app/services/carrierwave_active_storage_migration_service.rb @@ -75,14 +75,15 @@ class CarrierwaveActiveStorageMigrationService # but when the first attachment that references this blob is created. def make_blob(uploader, created_at, filename: nil, identify: false) content_type = uploader.content_type + identified = content_type.present? && !identify ActiveStorage::Blob.create( filename: filename || uploader.filename, content_type: uploader.content_type, - identified: content_type.present? && !identify, byte_size: uploader.size, checksum: checksum(uploader), - created_at: created_at + created_at: created_at, + metadata: { identified: identified, virus_scan_result: ActiveStorage::VirusScanner::SAFE } ) end diff --git a/spec/services/carrierwave_active_storage_migration_service_spec.rb b/spec/services/carrierwave_active_storage_migration_service_spec.rb index 7d3d470b0..a3bb2b042 100644 --- a/spec/services/carrierwave_active_storage_migration_service_spec.rb +++ b/spec/services/carrierwave_active_storage_migration_service_spec.rb @@ -1,9 +1,37 @@ require 'spec_helper' describe CarrierwaveActiveStorageMigrationService do - describe '#hex_to_base64' do - let(:service) { CarrierwaveActiveStorageMigrationService.new } + let(:service) { CarrierwaveActiveStorageMigrationService.new } + describe '#hex_to_base64' do it { expect(service.hex_to_base64('deadbeef')).to eq('3q2+7w==') } end + + describe '.make_blob' do + let(:pj) { create(:piece_justificative, :rib) } + let(:identify) { false } + + before do + allow(service).to receive(:checksum).and_return('cafe') + end + + subject(:blob) { service.make_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename, identify: identify) } + + it 'marks the blob as already scanned by the antivirus' do + expect(blob.metadata[:virus_scan_result]).to eq(ActiveStorage::VirusScanner::SAFE) + end + + it 'sets the blob MIME type from the file' do + expect(blob.identified).to be true + expect(blob.content_type).to eq 'application/pdf' + end + + context 'when asking for explicit MIME type identification' do + let(:identify) { true } + + it 'marks the file as needing MIME type detection' do + expect(blob.identified).to be false + end + end + end end From d410e3134452c952300473a3f4bac5b2dc50b9de Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 28 May 2019 11:35:42 +0200 Subject: [PATCH 15/15] active_storage: document the virus scan hooks --- app/models/concerns/blob_virus_scanner.rb | 4 ++++ config/initializers/active_storage.rb | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/app/models/concerns/blob_virus_scanner.rb b/app/models/concerns/blob_virus_scanner.rb index 4cd5f8476..c6b0966c9 100644 --- a/app/models/concerns/blob_virus_scanner.rb +++ b/app/models/concerns/blob_virus_scanner.rb @@ -1,3 +1,7 @@ +# TODO: once we're using Rails 6, use the hooks on attachments creation +# (rather than on blob creation). +# This will help to avoid cloberring metadata accidentally (as metadata +# are more stable on attachment creation than on blob creation). module BlobVirusScanner extend ActiveSupport::Concern diff --git a/config/initializers/active_storage.rb b/config/initializers/active_storage.rb index e9fb875dd..7357c9267 100644 --- a/config/initializers/active_storage.rb +++ b/config/initializers/active_storage.rb @@ -1,3 +1,9 @@ ActiveStorage::Service.url_expires_in = 1.hour +# In Rails 5.2, we have to hook at `on_load` on the blob themeselves, which is +# not ideal. +# +# Rails 6 adds support for `.on_load(:active_storage_attachment)`, which is +# cleaner (as it allows to enqueue the virus scan on attachment creation, rather +# than on blob creation). ActiveSupport.on_load(:active_storage_blob) { include BlobVirusScanner }