From 08400cdd4c0cbe19b54003b938e56b232af0c8ee Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 29 Jan 2020 12:16:38 +0100 Subject: [PATCH 1/2] Poll for export readiness updates --- .../instructeurs/procedures_controller.rb | 27 ++++++++++---- app/javascript/packs/application.js | 2 +- ...attachment-checker.js => remote-poller.js} | 35 +++++++++++++------ .../procedures/_download_dossiers.html.haml | 5 +-- .../procedures/download_export.js.erb | 9 ++++- app/views/shared/attachment/_show.html.haml | 2 +- .../procedures_controller_spec.rb | 2 +- 7 files changed, 58 insertions(+), 24 deletions(-) rename app/javascript/shared/{activestorage/attachment-checker.js => remote-poller.js} (56%) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index d619a267d..277365c4c 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -187,22 +187,35 @@ module Instructeurs end def download_export - format = params[:export_format] + export_format = params[:export_format] groupe_instructeurs = current_instructeur .groupe_instructeurs .where(procedure: procedure) - export = Export.find_or_create_export(format, groupe_instructeurs) + export = Export.find_or_create_export(export_format, groupe_instructeurs) if export.ready? - redirect_to export.file.service_url - else respond_to do |format| - notice_message = "Nous générons cet export. Veuillez revenir dans quelques minutes pour le télécharger." format.js do @procedure = procedure assign_exports - flash.notice = notice_message + flash.notice = "L’export au format \"#{export_format}\" est prêt." + end + + format.html do + redirect_to export.file.service_url + end + end + else + respond_to do |format| + notice_message = "Nous générons cet export. Veuillez revenir dans quelques minutes pour le télécharger." + + format.js do + @procedure = procedure + assign_exports + if !params[:no_progress_notification] + flash.notice = notice_message + end end format.html do @@ -262,7 +275,7 @@ module Instructeurs def ensure_ownership! if !current_instructeur.procedures.include?(procedure) - flash[:alert] = "Vous n'avez pas accès à cette démarche" + flash[:alert] = "Vous n’avez pas accès à cette démarche" redirect_to root_path end end diff --git a/app/javascript/packs/application.js b/app/javascript/packs/application.js index 0e567c58b..d19723244 100644 --- a/app/javascript/packs/application.js +++ b/app/javascript/packs/application.js @@ -10,7 +10,7 @@ import ReactRailsUJS from 'react_ujs'; import '../shared/page-update-event'; import '../shared/activestorage/ujs'; -import '../shared/activestorage/attachment-checker'; +import '../shared/remote-poller'; import '../shared/rails-ujs-fix'; import '../shared/safari-11-file-xhr-workaround'; import '../shared/remote-input'; diff --git a/app/javascript/shared/activestorage/attachment-checker.js b/app/javascript/shared/remote-poller.js similarity index 56% rename from app/javascript/shared/activestorage/attachment-checker.js rename to app/javascript/shared/remote-poller.js index 724e1a379..a04607212 100644 --- a/app/javascript/shared/activestorage/attachment-checker.js +++ b/app/javascript/shared/remote-poller.js @@ -1,32 +1,42 @@ import { ajax, delegate } from '@utils'; addEventListener('turbolinks:load', () => { - checker.deactivate(); + attachementPoller.deactivate(); + exportPoller.deactivate(); - const attachments = document.querySelectorAll('[data-attachment-check-url]'); + const attachments = document.querySelectorAll('[data-attachment-poll-url]'); + const exports = document.querySelectorAll('[data-export-poll-url]'); - for (let attachment of attachments) { - checker.add(attachment.dataset.attachmentCheckUrl); + for (let { dataset } of attachments) { + attachementPoller.add(dataset.attachmentPollUrl); + } + + for (let { dataset } of exports) { + exportPoller.add(dataset.exportPollUrl); } }); addEventListener('attachment:update', ({ detail: { url } }) => { - checker.add(url); + attachementPoller.add(url); +}); + +addEventListener('export:update', ({ detail: { url } }) => { + exportPoller.add(url); }); delegate('click', '[data-attachment-refresh]', event => { event.preventDefault(); - checker.check(); + attachementPoller.check(); }); -class AttachmentChecker { +class RemotePoller { urls = new Set(); timeout; checks = 0; constructor(settings = {}) { - this.interval = settings.interval || 5000; - this.maxChecks = settings.maxChecks || 5; + this.interval = settings.interval; + this.maxChecks = settings.maxChecks; } get isEnabled() { @@ -58,12 +68,14 @@ class AttachmentChecker { clearTimeout(this.timeout); this.timeout = setTimeout(() => { this.checks++; + this.currentInterval = this.interval * 1.5; this.check(); - }, this.interval); + }, this.currentInterval); } deactivate() { this.checks = 0; + this.currentInterval = this.interval; this.reset(); } @@ -74,4 +86,5 @@ class AttachmentChecker { } } -const checker = new AttachmentChecker(); +const attachementPoller = new RemotePoller({ interval: 2000, maxChecks: 5 }); +const exportPoller = new RemotePoller({ interval: 4000, maxChecks: 10 }); diff --git a/app/views/instructeurs/procedures/_download_dossiers.html.haml b/app/views/instructeurs/procedures/_download_dossiers.html.haml index 6ad237ddb..f07fd6c03 100644 --- a/app/views/instructeurs/procedures/_download_dossiers.html.haml +++ b/app/views/instructeurs/procedures/_download_dossiers.html.haml @@ -9,6 +9,7 @@ - if export.nil? = link_to "Demander un export au format .#{format}", download_export_instructeur_procedure_path(procedure, export_format: format), remote: true - elsif export.ready? - = link_to "Télécharger l'export au format .#{format}", url_for(export.file), target: "_blank", rel: "noopener" + = link_to "Télécharger l'export au format .#{format}", export.file.service_url, target: "_blank", rel: "noopener" - else - L'export au format .#{format} est en cours de préparation + %span{ 'data-export-poll-url': download_export_instructeur_procedure_path(procedure, export_format: format, no_progress_notification: true) } + L'export au format .#{format} est en cours de préparation diff --git a/app/views/instructeurs/procedures/download_export.js.erb b/app/views/instructeurs/procedures/download_export.js.erb index f22ebaf4e..fe8b67bb9 100644 --- a/app/views/instructeurs/procedures/download_export.js.erb +++ b/app/views/instructeurs/procedures/download_export.js.erb @@ -1,3 +1,10 @@ <%= render_to_element('.procedure-actions', partial: "download_dossiers", locals: { procedure: @procedure, xlsx_export: @xlsx_export, csv_export: @csv_export, ods_export: @ods_export }) %> -<%= render_flash %> + +<% [[@xlsx_export, :xlsx], [@csv_export, :csv], [@ods_export, :ods]].each do |(export, format)| %> + <% if export && !export.ready? %> + <%= fire_event('export:update', { url: download_export_instructeur_procedure_path(@procedure, export_format: format, no_progress_notification: true) }.to_json ) %> + <% end %> +<% end %> + +<%= render_flash(timeout: 7000) %> diff --git a/app/views/shared/attachment/_show.html.haml b/app/views/shared/attachment/_show.html.haml index 41c747055..6c5564ee9 100644 --- a/app/views/shared/attachment/_show.html.haml +++ b/app/views/shared/attachment/_show.html.haml @@ -5,7 +5,7 @@ - else - attachment_check_url = attachment_url(attachment.id, { signed_id: attachment.blob.signed_id, user_can_upload: user_can_upload }) -.attachment-link{ 'data-attachment-id': attachment.id, 'data-attachment-check-url': attachment_check_url } +.attachment-link{ 'data-attachment-id': attachment.id, 'data-attachment-poll-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.attached diff --git a/spec/controllers/instructeurs/procedures_controller_spec.rb b/spec/controllers/instructeurs/procedures_controller_spec.rb index a8168d821..fe614cee5 100644 --- a/spec/controllers/instructeurs/procedures_controller_spec.rb +++ b/spec/controllers/instructeurs/procedures_controller_spec.rb @@ -37,7 +37,7 @@ describe Instructeurs::ProceduresController, type: :controller do it "redirects and flash" do expect(@controller).to have_received(:redirect_to).with(root_path) - expect(flash.alert).to eq("Vous n'avez pas accès à cette démarche") + expect(flash.alert).to eq("Vous n’avez pas accès à cette démarche") end end end From eb20dd9153157c995f581a38f5d3005d71838018 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 30 Jan 2020 15:14:33 +0100 Subject: [PATCH 2/2] only load unscoped custom rule in development --- lib/cops/unscoped.rb | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/cops/unscoped.rb b/lib/cops/unscoped.rb index 8374c2116..2cbfe5f98 100644 --- a/lib/cops/unscoped.rb +++ b/lib/cops/unscoped.rb @@ -1,16 +1,18 @@ -module RuboCop - module Cop - module DS - class Unscoped < Cop - MSG = "Avoid using `unscoped`. Instead unscope specific clauses by using `unscope(where: :attribute)`." +if ENV["RAILS_ENV"] == "development" + module RuboCop + module Cop + module DS + class Unscoped < Cop + MSG = "Avoid using `unscoped`. Instead unscope specific clauses by using `unscope(where: :attribute)`." - def_node_matcher :unscoped?, <<-END - (send _ :unscoped) - END + def_node_matcher :unscoped?, <<-END + (send _ :unscoped) + END - def on_send(node) - return unless unscoped?(node) - add_offense(node) + def on_send(node) + return unless unscoped?(node) + add_offense(node) + end end end end