From d7454594aac4e3ee9abf15507f4c0351499635e8 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 18 Dec 2019 09:35:44 +0000 Subject: [PATCH 1/8] build(deps): bump excon from 0.68.0 to 0.71.0 Bumps [excon](https://github.com/excon/excon) from 0.68.0 to 0.71.0. - [Release notes](https://github.com/excon/excon/releases) - [Changelog](https://github.com/excon/excon/blob/master/changelog.txt) - [Commits](https://github.com/excon/excon/compare/v0.68.0...v0.71.0) Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index a96a2ea2b..b74054adf 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -201,7 +201,7 @@ GEM ethon (0.11.0) ffi (>= 1.3.0) eventmachine (1.2.7) - excon (0.68.0) + excon (0.71.0) execjs (2.7.0) factory_bot (4.11.1) activesupport (>= 3.0.0) From d0f0533a3262ce5269313e4c00d896cb5ae48dfb Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 27 Nov 2019 15:30:07 +0100 Subject: [PATCH 2/8] Remove unused code --- .../instructeurs/procedures_controller.rb | 19 ---------- config/routes.rb | 1 - .../procedures_controller_spec.rb | 37 ------------------- 3 files changed, 57 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 2af6b0ffe..949ebb778 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -184,25 +184,6 @@ module Instructeurs redirect_back(fallback_location: instructeur_procedure_url(procedure)) end - def download_dossiers - dossiers = current_instructeur.dossiers.for_procedure(procedure) - - respond_to do |format| - format.csv do - send_data(procedure.to_csv(dossiers), - filename: procedure.export_filename(:csv)) - end - format.xlsx do - send_data(procedure.to_xlsx(dossiers), - filename: procedure.export_filename(:xlsx)) - end - format.ods do - send_data(procedure.to_ods(dossiers), - filename: procedure.export_filename(:ods)) - end - end - end - def download_export export_format = params[:export_format] notice_message = "Nous générons cet export. Lorsque celui-ci sera disponible, vous recevrez une notification par email accompagnée d'un lien de téléchargement." diff --git a/config/routes.rb b/config/routes.rb index 5d4908a9e..68d04e52c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -305,7 +305,6 @@ Rails.application.routes.draw do get 'update_sort/:table/:column' => 'procedures#update_sort', as: 'update_sort' post 'add_filter' get 'remove_filter' => 'procedures#remove_filter', as: 'remove_filter' - get 'download_dossiers' get 'download_export' get 'stats' get 'email_notifications' diff --git a/spec/controllers/instructeurs/procedures_controller_spec.rb b/spec/controllers/instructeurs/procedures_controller_spec.rb index 417d58491..0d4a54410 100644 --- a/spec/controllers/instructeurs/procedures_controller_spec.rb +++ b/spec/controllers/instructeurs/procedures_controller_spec.rb @@ -411,43 +411,6 @@ describe Instructeurs::ProceduresController, type: :controller do end end - describe "#download_dossiers" do - let(:instructeur) { create(:instructeur) } - let!(:procedure) { create(:procedure, instructeurs: [instructeur]) } - let!(:gi_2) { procedure.groupe_instructeurs.create(label: '2') } - let!(:dossier_1) { create(:dossier, procedure: procedure) } - let!(:dossier_2) { create(:dossier, groupe_instructeur: gi_2) } - - context "when logged in" do - before do - sign_in(instructeur.user) - end - - context "csv" do - before do - expect_any_instance_of(Procedure).to receive(:to_csv) - .with(instructeur.dossiers.for_procedure(procedure)) - - get :download_dossiers, params: { procedure_id: procedure.id }, format: 'csv' - end - - it { expect(response).to have_http_status(:ok) } - end - - context "xlsx" do - before { get :download_dossiers, params: { procedure_id: procedure.id }, format: 'xlsx' } - - it { expect(response).to have_http_status(:ok) } - end - - context "ods" do - before { get :download_dossiers, params: { procedure_id: procedure.id }, format: 'ods' } - - it { expect(response).to have_http_status(:ok) } - end - end - end - describe '#update_email_notifications' do let(:instructeur) { create(:instructeur) } let!(:procedure) { create(:procedure, instructeurs: [instructeur]) } From d0939ae1a4aa1d084d011106a6d3c424a12fd29c Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 3 Dec 2019 18:36:50 +0100 Subject: [PATCH 3/8] Add Export Model --- app/jobs/export_job.rb | 5 + app/models/export.rb | 97 +++++++++++++++++++ app/models/groupe_instructeur.rb | 1 + db/migrate/20191211101608_create_exports.rb | 9 ++ ...te_export_groupe_instructeur_join_table.rb | 8 ++ db/schema.rb | 15 ++- spec/factories/export.rb | 6 ++ spec/factories/groupe_instructeur.rb | 8 ++ spec/models/export_spec.rb | 58 +++++++++++ 9 files changed, 206 insertions(+), 1 deletion(-) create mode 100644 app/jobs/export_job.rb create mode 100644 app/models/export.rb create mode 100644 db/migrate/20191211101608_create_exports.rb create mode 100644 db/migrate/20191211113341_create_export_groupe_instructeur_join_table.rb create mode 100644 spec/factories/export.rb create mode 100644 spec/factories/groupe_instructeur.rb create mode 100644 spec/models/export_spec.rb diff --git a/app/jobs/export_job.rb b/app/jobs/export_job.rb new file mode 100644 index 000000000..6abd0dcff --- /dev/null +++ b/app/jobs/export_job.rb @@ -0,0 +1,5 @@ +class ExportJob < ApplicationJob + def perform(export) + export.compute + end +end diff --git a/app/models/export.rb b/app/models/export.rb new file mode 100644 index 000000000..820b15ee8 --- /dev/null +++ b/app/models/export.rb @@ -0,0 +1,97 @@ +class Export < ApplicationRecord + MAX_DUREE_CONSERVATION_EXPORT = 15.minutes + + enum format: { + csv: 'csv', + ods: 'ods', + xlsx: 'xlsx' + } + + has_and_belongs_to_many :groupe_instructeurs + + has_one_attached :file + + validates :format, :groupe_instructeurs, presence: true + + scope :stale, -> { where('updated_at < ?', (Time.zone.now - MAX_DUREE_CONSERVATION_EXPORT)) } + + after_create :compute_async + + def compute_async + ExportJob.perform_later(self) + end + + def compute + file.attach( + io: io, + filename: filename, + content_type: content_type + ) + end + + def ready? + file.attached? + end + + def self.find_or_create_export(format, groupe_instructeurs) + export = Export.find_for_format_and_groupe_instructeurs(format, groupe_instructeurs) + + if export.nil? + export = Export.create( + format: format, + groupe_instructeurs: groupe_instructeurs + ) + end + + export + end + + def self.find_for_format_and_groupe_instructeurs(format, groupe_instructeurs) + export_including_gis = Export + .joins(:exports_groupe_instructeurs) + .where( + format: format, + exports_groupe_instructeurs: { groupe_instructeur: groupe_instructeurs } + ) + + export_including_gis.find do |export| + export.groupe_instructeurs.pluck(:id).sort == groupe_instructeurs.map(&:id).sort + end + end + + private + + def filename + procedure_identifier = procedure.path || "procedure-#{id}" + "dossiers_#{procedure_identifier}_#{Time.zone.now.strftime('%Y-%m-%d_%H-%M')}.#{format}" + end + + def io + dossiers = Dossier.where(groupe_instructeur: groupe_instructeurs) + service = ProcedureExportService.new(procedure, dossiers) + + case format.to_sym + when :csv + StringIO.new(service.to_csv) + when :xlsx + StringIO.new(service.to_xlsx) + when :ods + StringIO.new(service.to_ods) + end + end + + def content_type + case format.to_sym + when :csv + 'text/csv' + when :xlsx + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' + when :ods + 'application/vnd.oasis.opendocument.spreadsheet' + end + end + + def procedure + groupe_instructeurs.first.procedure + end +end diff --git a/app/models/groupe_instructeur.rb b/app/models/groupe_instructeur.rb index 8ebd1dd27..546e94cb4 100644 --- a/app/models/groupe_instructeur.rb +++ b/app/models/groupe_instructeur.rb @@ -4,6 +4,7 @@ class GroupeInstructeur < ApplicationRecord has_many :assign_tos has_many :instructeurs, through: :assign_tos, dependent: :destroy has_many :dossiers + has_and_belongs_to_many :exports validates :label, presence: { message: 'doit être renseigné' }, allow_nil: false validates :label, uniqueness: { scope: :procedure, message: 'existe déjà' } diff --git a/db/migrate/20191211101608_create_exports.rb b/db/migrate/20191211101608_create_exports.rb new file mode 100644 index 000000000..ab7ca3374 --- /dev/null +++ b/db/migrate/20191211101608_create_exports.rb @@ -0,0 +1,9 @@ +class CreateExports < ActiveRecord::Migration[5.2] + def change + create_table :exports do |t| + t.string :format, null: false + + t.timestamps + end + end +end diff --git a/db/migrate/20191211113341_create_export_groupe_instructeur_join_table.rb b/db/migrate/20191211113341_create_export_groupe_instructeur_join_table.rb new file mode 100644 index 000000000..57a88fd0b --- /dev/null +++ b/db/migrate/20191211113341_create_export_groupe_instructeur_join_table.rb @@ -0,0 +1,8 @@ +class CreateExportGroupeInstructeurJoinTable < ActiveRecord::Migration[5.2] + create_table "exports_groupe_instructeurs", force: :cascade do |t| + t.bigint "export_id", null: false + t.bigint "groupe_instructeur_id", null: false + + t.timestamps + end +end diff --git a/db/schema.rb b/db/schema.rb index 0ee1c2c04..8400a0987 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_12_09_141641) do +ActiveRecord::Schema.define(version: 2019_12_11_113341) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -317,6 +317,19 @@ ActiveRecord::Schema.define(version: 2019_12_09_141641) do t.datetime "updated_at" end + create_table "exports", force: :cascade do |t| + t.string "format", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + create_table "exports_groupe_instructeurs", force: :cascade do |t| + t.bigint "export_id", null: false + t.bigint "groupe_instructeur_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "feedbacks", force: :cascade do |t| t.bigint "user_id" t.datetime "created_at", null: false diff --git a/spec/factories/export.rb b/spec/factories/export.rb new file mode 100644 index 000000000..30e229a9d --- /dev/null +++ b/spec/factories/export.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :export do + format { :csv } + groupe_instructeurs { [create(:groupe_instructeur)] } + end +end diff --git a/spec/factories/groupe_instructeur.rb b/spec/factories/groupe_instructeur.rb new file mode 100644 index 000000000..413d6c814 --- /dev/null +++ b/spec/factories/groupe_instructeur.rb @@ -0,0 +1,8 @@ +FactoryBot.define do + sequence(:groupe_label) { |n| "label_#{n}" } + + factory :groupe_instructeur do + label { generate(:groupe_label) } + procedure { create(:procedure) } + end +end diff --git a/spec/models/export_spec.rb b/spec/models/export_spec.rb new file mode 100644 index 000000000..a3c75c577 --- /dev/null +++ b/spec/models/export_spec.rb @@ -0,0 +1,58 @@ +require 'rails_helper' + +RSpec.describe Export, type: :model do + describe 'validations' do + let(:groupe_instructeur) { create(:groupe_instructeur) } + + context 'when everything is ok' do + let(:export) { build(:export) } + + it { expect(export.save).to be true } + end + + context 'when groupe instructeurs are missing' do + let(:export) { build(:export, groupe_instructeurs: []) } + + it { expect(export.save).to be false } + end + + context 'when format is missing' do + let(:export) { build(:export, format: nil) } + + it { expect(export.save).to be false } + end + end + + describe '.stale' do + let!(:export) { create(:export) } + let(:stale_date) { Time.zone.now() - (Export::MAX_DUREE_CONSERVATION_EXPORT + 1.minute) } + let!(:stale_export) { create(:export, updated_at: stale_date) } + + it { expect(Export.stale).to match_array([stale_export]) } + end + + describe '.destroy' do + let!(:groupe_instructeur) { create(:groupe_instructeur) } + let!(:export) { create(:export, groupe_instructeurs: [groupe_instructeur]) } + + before { export.destroy! } + + it { expect(Export.count).to eq(0) } + it { expect(groupe_instructeur.reload).to be_present } + end + + describe '.find_by groupe_instructeurs' do + let!(:procedure) { create(:procedure) } + let!(:gi_1) { create(:groupe_instructeur, procedure: procedure) } + let!(:gi_2) { create(:groupe_instructeur, procedure: procedure) } + let!(:gi_3) { create(:groupe_instructeur, procedure: procedure) } + + context 'when an export is made for one groupe instructeur' do + let!(:export) { Export.create(format: :csv, groupe_instructeurs: [gi_1, gi_2]) } + + it { expect(Export.find_for_format_and_groupe_instructeurs(:csv, [gi_1])).to eq(nil) } + it { expect(Export.find_for_format_and_groupe_instructeurs(:csv, [gi_2, gi_1])).to eq(export) } + it { expect(Export.find_for_format_and_groupe_instructeurs(:csv, [gi_1, gi_2, gi_3])).to eq(nil) } + end + end +end From ce7ab899344dda50dc9dd99e4698b0a0629607e1 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 27 Nov 2019 14:56:08 +0100 Subject: [PATCH 4/8] Add purge stale export job --- README.md | 1 + app/jobs/purge_stale_export_jobs.rb | 7 +++++++ .../deployment/20191127135401_enable_export_purge.rake | 8 ++++++++ 3 files changed, 16 insertions(+) create mode 100644 app/jobs/purge_stale_export_jobs.rb create mode 100644 lib/tasks/deployment/20191127135401_enable_export_purge.rake diff --git a/README.md b/README.md index d64b44dbd..abd5ae103 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,7 @@ En local, un utilisateur de test est créé automatiquement, avec les identifian PurgeUnattachedBlobsJob.set(cron: "0 0 * * *").perform_later OperationsSignatureJob.set(cron: "0 6 * * *").perform_later SeekAndDestroyExpiredDossiersJob.set(cron: "0 7 * * *").perform_later + PurgeStaleExportsJob.set(cron: "*/5 * * * *").perform_later ### Voir les emails envoyés en local diff --git a/app/jobs/purge_stale_export_jobs.rb b/app/jobs/purge_stale_export_jobs.rb new file mode 100644 index 000000000..6a40feec6 --- /dev/null +++ b/app/jobs/purge_stale_export_jobs.rb @@ -0,0 +1,7 @@ +class PurgeStaleExportsJob < ApplicationJob + queue_as :cron + + def perform + Export.stale.destroy_all + end +end diff --git a/lib/tasks/deployment/20191127135401_enable_export_purge.rake b/lib/tasks/deployment/20191127135401_enable_export_purge.rake new file mode 100644 index 000000000..50c297cdc --- /dev/null +++ b/lib/tasks/deployment/20191127135401_enable_export_purge.rake @@ -0,0 +1,8 @@ +namespace :after_party do + desc 'Deployment task: enable_export_purge' + task enable_export_purge: :environment do + PurgeStaleExportsJob.set(cron: "*/5 * * * *").perform_later + + AfterParty::TaskRecord.create version: '20191127135401' + end +end From 745086cbb547c98bf27c257228542b4703ce3bab Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 11 Dec 2019 17:42:44 +0100 Subject: [PATCH 5/8] Simplify view --- .../instructeurs/procedures_controller.rb | 9 ++++++ .../procedures/_download_dossiers.html.haml | 30 +++++-------------- .../procedures/download_export.js.erb | 3 +- .../instructeurs/procedures/show.html.haml | 3 +- .../_download_dossiers.html.haml_spec.rb | 2 +- 5 files changed, 21 insertions(+), 26 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 949ebb778..32348f453 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -109,6 +109,8 @@ module Instructeurs @dossiers = @dossiers.sort_by { |d| filtered_sorted_paginated_ids.index(d.id) } kaminarize(page, filtered_sorted_ids.count) + + assign_exports end def update_displayed_fields @@ -226,6 +228,13 @@ module Instructeurs private + def assign_exports + groupe_instructeurs_for_procedure = current_instructeur.groupe_instructeurs.where(procedure: procedure) + @xlsx_export = Export.find_for_format_and_groupe_instructeurs(:xlsx, groupe_instructeurs_for_procedure) + @csv_export = Export.find_for_format_and_groupe_instructeurs(:csv, groupe_instructeurs_for_procedure) + @ods_export = Export.find_for_format_and_groupe_instructeurs(:ods, groupe_instructeurs_for_procedure) + end + def find_field(table, column) procedure_presentation.fields.find { |c| c['table'] == table && c['column'] == column } end diff --git a/app/views/instructeurs/procedures/_download_dossiers.html.haml b/app/views/instructeurs/procedures/_download_dossiers.html.haml index 40fc7a2c1..0249656e6 100644 --- a/app/views/instructeurs/procedures/_download_dossiers.html.haml +++ b/app/views/instructeurs/procedures/_download_dossiers.html.haml @@ -4,27 +4,11 @@ Télécharger tous les dossiers .dropdown-content.fade-in-down{ style: 'width: 330px' } %ul.dropdown-items - %li - - if procedure.xlsx_export_stale? - - if procedure.xlsx_export_queued? - L'export au format .xlsx est en cours de préparation, vous recevrez un email lorsqu'il sera disponible. + - [[xlsx_export, :xlsx], [csv_export, :csv], [ods_export, :ods]].each do |(export, format)| + %li + - if export.nil? + = link_to "Exporter au format .#{format}", download_export_instructeur_procedure_path(procedure, export_format: format), remote: true + - elsif export.ready? + = link_to "Au format .#{format}", url_for(export.file), target: "_blank", rel: "noopener" - else - = link_to "Exporter au format .xlsx", download_export_instructeur_procedure_path(procedure, export_format: :xlsx), remote: true - - else - = link_to "Au format .xlsx", url_for(procedure.xlsx_export_file), target: "_blank", rel: "noopener" - %li - - if procedure.ods_export_stale? - - if procedure.ods_export_queued? - L'export au format .ods est en cours de préparation, vous recevrez un email lorsqu'il sera disponible. - - else - = link_to "Exporter au format .ods", download_export_instructeur_procedure_path(procedure, export_format: :ods), remote: true - - else - = link_to "Au format .ods", url_for(procedure.ods_export_file), target: "_blank", rel: "noopener" - %li - - if procedure.csv_export_stale? - - if procedure.csv_export_queued? - L'export au format .csv est en cours de préparation, vous recevrez un email lorsqu'il sera disponible. - - else - = link_to "Exporter au format .csv", download_export_instructeur_procedure_path(procedure, export_format: :csv), remote: true - - else - = link_to "Au format .csv", url_for(procedure.csv_export_file), target: "_blank", rel: "noopener" + 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 cb6442733..f22ebaf4e 100644 --- a/app/views/instructeurs/procedures/download_export.js.erb +++ b/app/views/instructeurs/procedures/download_export.js.erb @@ -1,2 +1,3 @@ -<%= render_to_element('.procedure-actions', partial: "download_dossiers", locals: { procedure: @procedure }) %> +<%= 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 %> diff --git a/app/views/instructeurs/procedures/show.html.haml b/app/views/instructeurs/procedures/show.html.haml index 0847097fd..828079590 100644 --- a/app/views/instructeurs/procedures/show.html.haml +++ b/app/views/instructeurs/procedures/show.html.haml @@ -49,7 +49,8 @@ badge: @archived_dossiers.count) .procedure-actions - = render partial: "download_dossiers", locals: { procedure: @procedure } + = render partial: "download_dossiers", + locals: { procedure: @procedure, xlsx_export: @xlsx_export, csv_export: @csv_export, ods_export: @ods_export } .container - if @statut == 'a-suivre' diff --git a/spec/views/instructeur/procedures/_download_dossiers.html.haml_spec.rb b/spec/views/instructeur/procedures/_download_dossiers.html.haml_spec.rb index 886de7d3c..45b522742 100644 --- a/spec/views/instructeur/procedures/_download_dossiers.html.haml_spec.rb +++ b/spec/views/instructeur/procedures/_download_dossiers.html.haml_spec.rb @@ -2,7 +2,7 @@ describe 'instructeurs/procedures/_download_dossiers.html.haml', type: :view do let(:current_instructeur) { create(:instructeur) } let(:procedure) { create(:procedure) } - subject { render 'instructeurs/procedures/download_dossiers.html.haml', procedure: procedure } + subject { render 'instructeurs/procedures/download_dossiers.html.haml', procedure: procedure, xlsx_export: nil, csv_export: nil, ods_export: nil } context "when procedure has 0 dossier" do it { is_expected.not_to include("Télécharger tous les dossiers") } From 60538c9c28fcb61b5554d94d4689f4ce6371a1bf Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 11 Dec 2019 17:43:24 +0100 Subject: [PATCH 6/8] Controller use new export model --- .../instructeurs/procedures_controller.rb | 27 ++++--- .../procedures/_download_dossiers.html.haml | 4 +- .../procedures_controller_spec.rb | 76 +++++++++++++++++++ 3 files changed, 94 insertions(+), 13 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 32348f453..998bf5785 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -187,23 +187,28 @@ module Instructeurs end def download_export - export_format = params[:export_format] - notice_message = "Nous générons cet export. Lorsque celui-ci sera disponible, vous recevrez une notification par email accompagnée d'un lien de téléchargement." - if procedure.should_generate_export?(export_format) - procedure.queue_export(current_instructeur, export_format) - flash.notice = notice_message + format = params[:export_format] + groupe_instructeurs = current_instructeur + .groupe_instructeurs + .where(procedure: procedure) + export = Export.find_or_create_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 + end + + format.html do + redirect_to instructeur_procedure_url(procedure), notice: notice_message end - format.all { redirect_to procedure } end - elsif procedure.export_queued?(export_format) - flash.notice = notice_message - redirect_to procedure - else - redirect_to url_for(procedure.export_file(export_format)) end end diff --git a/app/views/instructeurs/procedures/_download_dossiers.html.haml b/app/views/instructeurs/procedures/_download_dossiers.html.haml index 0249656e6..6ad237ddb 100644 --- a/app/views/instructeurs/procedures/_download_dossiers.html.haml +++ b/app/views/instructeurs/procedures/_download_dossiers.html.haml @@ -7,8 +7,8 @@ - [[xlsx_export, :xlsx], [csv_export, :csv], [ods_export, :ods]].each do |(export, format)| %li - if export.nil? - = link_to "Exporter au format .#{format}", download_export_instructeur_procedure_path(procedure, export_format: format), remote: true + = link_to "Demander un export au format .#{format}", download_export_instructeur_procedure_path(procedure, export_format: format), remote: true - elsif export.ready? - = link_to "Au format .#{format}", url_for(export.file), target: "_blank", rel: "noopener" + = link_to "Télécharger l'export au format .#{format}", url_for(export.file), target: "_blank", rel: "noopener" - else L'export au format .#{format} est en cours de préparation diff --git a/spec/controllers/instructeurs/procedures_controller_spec.rb b/spec/controllers/instructeurs/procedures_controller_spec.rb index 0d4a54410..0fdb90e5e 100644 --- a/spec/controllers/instructeurs/procedures_controller_spec.rb +++ b/spec/controllers/instructeurs/procedures_controller_spec.rb @@ -431,4 +431,80 @@ describe Instructeurs::ProceduresController, type: :controller do end end end + + describe '#download_export' do + let(:instructeur) { create(:instructeur) } + let!(:procedure) { create(:procedure) } + let!(:gi_0) { procedure.defaut_groupe_instructeur } + let!(:gi_1) { GroupeInstructeur.create(label: 'gi_1', procedure: procedure, instructeurs: [instructeur]) } + + before { sign_in(instructeur.user) } + + subject do + get :download_export, params: { export_format: :csv, procedure_id: procedure.id } + end + + context 'when the export is does not exist' do + it 'displays an notice' do + is_expected.to redirect_to(instructeur_procedure_url(procedure)) + expect(flash.notice).to be_present + end + + it { expect { subject }.to change(Export, :count).by(1) } + end + + context 'when the export is not ready' do + before do + Export.create(format: :csv, groupe_instructeurs: [gi_1]) + end + + it 'displays an notice' do + is_expected.to redirect_to(instructeur_procedure_url(procedure)) + expect(flash.notice).to be_present + end + end + + context 'when the export is ready' do + let!(:export) do + Export.create(format: :csv, groupe_instructeurs: [gi_1]) + end + + before do + export.file.attach(io: StringIO.new('export'), filename: 'file.csv') + end + + it 'displays the download link' do + subject + expect(response.headers['Location']).to start_with("http://test.host/rails/active_storage/disk") + end + end + + context 'when another export is ready' do + let!(:export) do + Export.create(format: :csv, groupe_instructeurs: [gi_0, gi_1]) + end + + before do + export.file.attach(io: StringIO.new('export'), filename: 'file.csv') + end + + it 'displays an notice' do + is_expected.to redirect_to(instructeur_procedure_url(procedure)) + expect(flash.notice).to be_present + end + end + + context 'when the js format is used' do + before do + post :download_export, + params: { export_format: :csv, procedure_id: procedure.id }, + format: :js + end + + it "responses in the correct format" do + expect(response.content_type).to eq "text/javascript" + expect(response).to have_http_status(:ok) + end + end + end end From c95b7a33fa3e2959a4284be715e3f5bd87688d32 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 12 Dec 2019 16:38:27 +0100 Subject: [PATCH 7/8] Add brakeman exception for a export.file.service_url --- config/brakeman.ignore | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/config/brakeman.ignore b/config/brakeman.ignore index e6b718775..e8efe1367 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -10,7 +10,7 @@ "line": 28, "link": "https://brakemanscanner.org/docs/warning_types/cross_site_scripting", "code": "current_user.dossiers.includes(:procedure).find(params[:id]).procedure.monavis_embed", - "render_path": [{"type":"controller","class":"Users::DossiersController","method":"merci","line":177,"file":"app/controllers/users/dossiers_controller.rb"}], + "render_path": [{"type":"controller","class":"Users::DossiersController","method":"merci","line":181,"file":"app/controllers/users/dossiers_controller.rb"}], "location": { "type": "template", "template": "users/dossiers/merci" @@ -19,6 +19,26 @@ "confidence": "Weak", "note": "" }, + { + "warning_type": "Redirect", + "warning_code": 18, + "fingerprint": "8b22d0fa97c6b32921a3383a60dd63f1d2c0723c48f30bdc2d4abe41fe4abccc", + "check_name": "Redirect", + "message": "Possible unprotected redirect", + "file": "app/controllers/instructeurs/procedures_controller.rb", + "line": 198, + "link": "https://brakemanscanner.org/docs/warning_types/redirect/", + "code": "redirect_to(Export.find_or_create_export(params[:export_format], current_instructeur.groupe_instructeurs.where(:procedure => procedure)).file.service_url)", + "render_path": null, + "location": { + "type": "method", + "class": "Instructeurs::ProceduresController", + "method": "download_export" + }, + "user_input": "Export.find_or_create_export(params[:export_format], current_instructeur.groupe_instructeurs.where(:procedure => procedure)).file.service_url", + "confidence": "High", + "note": "" + }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -46,7 +66,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/procedure_presentation.rb", - "line": 106, + "line": 107, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "((\"self\" == \"self\") ? (dossiers) : (dossiers.includes(\"self\"))).order(\"#{self.class.sanitized_column(\"self\", column)} #{order}\")", "render_path": null, @@ -86,7 +106,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/procedure_presentation.rb", - "line": 102, + "line": 103, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "dossiers.includes(:followers_instructeurs).joins(\"LEFT OUTER JOIN users instructeurs_users ON instructeurs_users.instructeur_id = instructeurs.id\").order(\"instructeurs_users.email #{order}\")", "render_path": null, @@ -100,6 +120,6 @@ "note": "" } ], - "updated": "2019-10-16 16:19:43 +0200", + "updated": "2019-12-12 16:36:32 +0100", "brakeman_version": "4.3.1" } From 2ee559c7485e9d40692b11e948220577931cb919 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 18 Dec 2019 13:35:04 +0100 Subject: [PATCH 8/8] Fix typo in purges_stale_exports_job filename --- .../{purge_stale_export_jobs.rb => purge_stale_exports_job.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename app/jobs/{purge_stale_export_jobs.rb => purge_stale_exports_job.rb} (100%) diff --git a/app/jobs/purge_stale_export_jobs.rb b/app/jobs/purge_stale_exports_job.rb similarity index 100% rename from app/jobs/purge_stale_export_jobs.rb rename to app/jobs/purge_stale_exports_job.rb