Merge pull request #4651 from betagouv/dev

2019-12-18-01

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)

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
 

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
@@ -184,43 +186,29 @@ 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]
+      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."
+      groupe_instructeurs = current_instructeur
-      if procedure.should_generate_export?(export_format)
+        .groupe_instructeurs
-        procedure.queue_export(current_instructeur, export_format)
+        .where(procedure: procedure)
-        flash.notice = notice_message
 
+      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
-          end
+            assign_exports
-          format.all { redirect_to procedure }
-        end
-      elsif procedure.export_queued?(export_format)
             flash.notice = notice_message
-        redirect_to procedure
+          end
-      else
+
-        redirect_to url_for(procedure.export_file(export_format))
+          format.html do
+            redirect_to instructeur_procedure_url(procedure), notice: notice_message
+          end
+        end
       end
     end
 
@@ -245,6 +233,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

app/jobs/export_job.rb

@@ -0,0 +1,5 @@
+class ExportJob < ApplicationJob
+  def perform(export)
+    export.compute
+  end
+end

app/jobs/purge_stale_exports_job.rb

@@ -0,0 +1,7 @@
+class PurgeStaleExportsJob < ApplicationJob
+  queue_as :cron
+
+  def perform
+    Export.stale.destroy_all
+  end
+end

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

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à' }

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
+        - [[xlsx_export, :xlsx], [csv_export, :csv], [ods_export, :ods]].each do |(export, format)|
           %li
-          - if procedure.xlsx_export_stale?
+            - if export.nil?
-            - if procedure.xlsx_export_queued?
+              = link_to "Demander un export au format .#{format}", download_export_instructeur_procedure_path(procedure, export_format: format), remote: true
-              L'export au format .xlsx est en cours de préparation, vous recevrez un email lorsqu'il sera disponible.
+            - elsif export.ready?
+              = link_to "Télécharger l'export 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
+              L'export au format .#{format} est en cours de préparation
-          - 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"

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 %>

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'

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"
 }

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'

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

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

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

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

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]) }
@@ -468,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

spec/factories/export.rb

@@ -0,0 +1,6 @@
+FactoryBot.define do
+  factory :export do
+    format { :csv }
+    groupe_instructeurs { [create(:groupe_instructeur)] }
+  end
+end

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

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

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") }