From 5897464224fefe62d07fa7f56d0a9fa00b842b9d Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 24 Sep 2019 06:57:23 +0200 Subject: [PATCH 01/17] Build Dossier champ only once --- app/models/dossier.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 1204534c2..497ddd0f4 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -197,7 +197,7 @@ class Dossier < ApplicationRecord before_validation :update_state_dates, if: -> { state_changed? } - before_save :build_default_champs, if: Proc.new { groupe_instructeur_id_changed? } + before_save :build_default_champs, if: Proc.new { groupe_instructeur_id_was.nil? } before_save :build_default_individual, if: Proc.new { procedure.for_individual? } before_save :update_search_terms From 6b8cefa551be740bc8cd3ba37b9e846bbaf6ba66 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 15 Oct 2019 09:28:21 +0200 Subject: [PATCH 02/17] Procedure: defaut_groupe_instructeur is the first group Especially useful when the defaut groupe is renamed ... --- app/models/procedure.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 550a5e32d..1d24b7ae4 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -29,7 +29,7 @@ class Procedure < ApplicationRecord has_one :refused_mail, class_name: "Mails::RefusedMail", dependent: :destroy has_one :without_continuation_mail, class_name: "Mails::WithoutContinuationMail", dependent: :destroy - has_one :defaut_groupe_instructeur, -> { where(label: GroupeInstructeur::DEFAULT_LABEL) }, class_name: 'GroupeInstructeur', inverse_of: :procedure + has_one :defaut_groupe_instructeur, -> { order(:id) }, class_name: 'GroupeInstructeur', inverse_of: :procedure has_one_attached :logo has_one_attached :notice From b7434c313296b51a5aa68c8333f7ea55cff9709b Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 24 Sep 2019 06:31:08 +0200 Subject: [PATCH 03/17] User can choose its gi --- app/controllers/users/dossiers_controller.rb | 9 +++++---- app/views/shared/dossiers/_edit.html.haml | 7 +++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 71fd1c713..23cb10410 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -282,13 +282,14 @@ module Users end # FIXME: require(:dossier) when all the champs are united - def champs_params - params.permit(dossier: { + def champs_and_groupe_instructeurs_params + params.permit(dossier: [ + :groupe_instructeur_id, champs_attributes: [ :id, :value, :primary_value, :secondary_value, :piece_justificative_file, value: [], champs_attributes: [:id, :_destroy, :value, :primary_value, :secondary_value, :piece_justificative_file, value: []] ] - }) + ]) end def dossier @@ -302,7 +303,7 @@ module Users def update_dossier_and_compute_errors errors = [] - if champs_params[:dossier] && !@dossier.update(champs_params[:dossier]) + if champs_and_groupe_instructeurs_params[:dossier] && !@dossier.update(champs_and_groupe_instructeurs_params[:dossier]) errors += @dossier.errors.full_messages end diff --git a/app/views/shared/dossiers/_edit.html.haml b/app/views/shared/dossiers/_edit.html.haml index 7da3f5bc5..c617a2450 100644 --- a/app/views/shared/dossiers/_edit.html.haml +++ b/app/views/shared/dossiers/_edit.html.haml @@ -26,6 +26,13 @@ %hr + - if dossier.procedure.routee? + = f.label :groupe_instructeur_id, dossier.procedure.routing_criteria_name + = f.select :groupe_instructeur_id, + dossier.procedure.groupe_instructeurs.order(:label).map { |gi| [gi.label, gi.id] }, + {}, + required: true + = f.fields_for :champs, dossier.champs do |champ_form| - champ = champ_form.object = render partial: "shared/dossiers/editable_champs/editable_champ", From 18de25fac7823ca4105d8d73a25d6a3db33d4132 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 15 Oct 2019 10:29:07 +0200 Subject: [PATCH 04/17] Display the dossier.groupe_instructeur.label on various screens --- .../instructeurs/dossiers/print.html.haml | 2 +- app/views/shared/dossiers/_champs.html.haml | 4 ++++ app/views/shared/dossiers/_demande.html.haml | 4 ++-- spec/factories/procedure.rb | 6 ++++++ .../shared/dossiers/_champs.html.haml_spec.rb | 18 +++++++++++++++++- 5 files changed, 30 insertions(+), 4 deletions(-) diff --git a/app/views/instructeurs/dossiers/print.html.haml b/app/views/instructeurs/dossiers/print.html.haml index 7702f8ed4..25bfa2aec 100644 --- a/app/views/instructeurs/dossiers/print.html.haml +++ b/app/views/instructeurs/dossiers/print.html.haml @@ -14,7 +14,7 @@ %h2 Formulaire - champs = @dossier.champs -- if champs.any? +- if champs.any? || @dossier.procedure.routee? = render partial: "shared/dossiers/champs", locals: { champs: champs, dossier: @dossier, demande_seen_at: nil, profile: 'instructeur' } %h2 Annotations privées diff --git a/app/views/shared/dossiers/_champs.html.haml b/app/views/shared/dossiers/_champs.html.haml index 1a511730f..969c489d5 100644 --- a/app/views/shared/dossiers/_champs.html.haml +++ b/app/views/shared/dossiers/_champs.html.haml @@ -1,3 +1,7 @@ %table.table.vertical.dossier-champs %tbody + - if dossier.procedure.routee? + %th= dossier.procedure.routing_criteria_name + %td= dossier.groupe_instructeur.label + %td = render partial: "shared/dossiers/champ_row", locals: { champs: champs, demande_seen_at: demande_seen_at, profile: profile, repetition: false } diff --git a/app/views/shared/dossiers/_demande.html.haml b/app/views/shared/dossiers/_demande.html.haml index d04cc1918..55870541c 100644 --- a/app/views/shared/dossiers/_demande.html.haml +++ b/app/views/shared/dossiers/_demande.html.haml @@ -23,6 +23,6 @@ .tab-title Formulaire - champs = dossier.champs.includes(:type_de_champ) - - if champs.any? + - if champs.any? || dossier.procedure.routee? .card - = render partial: "shared/dossiers/champs", locals: { champs: champs, demande_seen_at: demande_seen_at, profile: profile } + = render partial: "shared/dossiers/champs", locals: { champs: champs, dossier: dossier, demande_seen_at: demande_seen_at, profile: profile } diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb index 3cfdd52dc..43dc38211 100644 --- a/spec/factories/procedure.rb +++ b/spec/factories/procedure.rb @@ -74,6 +74,12 @@ FactoryBot.define do end end + trait :routee do + after(:create) do |procedure, _evaluator| + procedure.groupe_instructeurs.create(label: '2nd groupe') + end + end + trait :for_individual do after(:build) do |procedure, _evaluator| procedure.for_individual = true diff --git a/spec/views/shared/dossiers/_champs.html.haml_spec.rb b/spec/views/shared/dossiers/_champs.html.haml_spec.rb index dfce7ae25..9efff5573 100644 --- a/spec/views/shared/dossiers/_champs.html.haml_spec.rb +++ b/spec/views/shared/dossiers/_champs.html.haml_spec.rb @@ -8,7 +8,7 @@ describe 'shared/dossiers/champs.html.haml', type: :view do allow(view).to receive(:current_instructeur).and_return(instructeur) end - subject { render 'shared/dossiers/champs.html.haml', champs: champs, demande_seen_at: demande_seen_at, profile: nil } + subject { render 'shared/dossiers/champs.html.haml', champs: champs, dossier: dossier, demande_seen_at: demande_seen_at, profile: nil } context "there are some champs" do let(:dossier) { create(:dossier) } @@ -54,6 +54,21 @@ describe 'shared/dossiers/champs.html.haml', type: :view do end end + context "with a routed procedure" do + let(:procedure) do + create(:procedure, + :routee, + routing_criteria_name: 'departement') + end + let(:dossier) { create(:dossier, procedure: procedure) } + let(:champs) { [] } + + it "renders the routing criteria name and its value" do + expect(subject).to include(procedure.routing_criteria_name) + expect(subject).to include(dossier.groupe_instructeur.label) + end + end + context "with a dossier champ, but we are not authorized to acces the dossier" do let(:dossier) { create(:dossier) } let(:champ) { create(:champ, :dossier_link, value: dossier.id) } @@ -65,6 +80,7 @@ describe 'shared/dossiers/champs.html.haml', type: :view do end context "with a dossier_link champ but without value" do + let(:dossier) { create(:dossier) } let(:champ) { create(:champ, :dossier_link, value: nil) } let(:champs) { [champ] } From 1e8e45232a946d55b15c82b1ac792fb6f49b730c Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 17 Oct 2019 16:28:28 +0200 Subject: [PATCH 05/17] Setup a timeout on long query --- config/database.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/config/database.yml b/config/database.yml index 1f4b1a8f2..6308f93bf 100644 --- a/config/database.yml +++ b/config/database.yml @@ -3,6 +3,11 @@ default: &default encoding: unicode pool: <%= ENV.fetch("DB_POOL") { 5 } %> timeout: 5000 + # sql queries will be killed after 60s + # we should reduce this number + # A bigger timeout can be set for jobs + variables: + statement_timeout: <%= ENV['PG_STATEMENT_TIMEOUT'] || 60000 %> development: <<: *default From 0807fed258e15be3db4937fdf9bb2952c652559e Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 17 Oct 2019 16:51:42 +0200 Subject: [PATCH 06/17] specs: rename `instructeur_spec.rb` to `instruction_spec.rb` This clarifies the difference between this spec and the other instructeur specs in the same directory. --- .../instructeurs/{instructeur_spec.rb => instruction_spec.rb} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename spec/features/instructeurs/{instructeur_spec.rb => instruction_spec.rb} (99%) diff --git a/spec/features/instructeurs/instructeur_spec.rb b/spec/features/instructeurs/instruction_spec.rb similarity index 99% rename from spec/features/instructeurs/instructeur_spec.rb rename to spec/features/instructeurs/instruction_spec.rb index 4514b9084..e724e31dc 100644 --- a/spec/features/instructeurs/instructeur_spec.rb +++ b/spec/features/instructeurs/instruction_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'The instructeur part' do +feature 'Instructing a dossier:' do include ActiveJob::TestHelper let(:password) { 'démarches-simplifiées-pwd' } From a56dc0f65341a1f4d2a16b18a474046f2f590809 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 21 Oct 2019 11:17:23 +0200 Subject: [PATCH 07/17] specs: re-order method in feature helpers --- spec/support/feature_helpers.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/support/feature_helpers.rb b/spec/support/feature_helpers.rb index 6e11bea16..4282db2ea 100644 --- a/spec/support/feature_helpers.rb +++ b/spec/support/feature_helpers.rb @@ -63,6 +63,13 @@ module FeatureHelpers expect(page).to have_content(procedure.service.email) end + def click_reset_password_link_for(email) + reset_password_email = open_email(email) + token_params = reset_password_email.body.match(/reset_password_token=[^"]+/) + + visit "/users/password/edit?#{token_params}" + end + def blur page.find('body').click end @@ -78,13 +85,6 @@ module FeatureHelpers value end end - - def click_reset_password_link_for(email) - reset_password_email = open_email(email) - token_params = reset_password_email.body.match(/reset_password_token=[^"]+/) - - visit "/users/password/edit?#{token_params}" - end end RSpec.configure do |config| From ec6ec6f4aa892f0ac6237c22494dd0c10f3a2eff Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 21 Oct 2019 09:45:03 +0000 Subject: [PATCH 08/17] specs: improve avis factory The instructeur, dossier and claimant where created regardless of wether they already existed or not. With this commit: - Associations are declarated in a more idiomatic way - They are correctly not created if they are provided when creating the object. --- spec/factories/avis.rb | 25 ++++++------------- .../avis/instruction.html.haml_spec.rb | 6 +++-- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/spec/factories/avis.rb b/spec/factories/avis.rb index aeec523d3..fedf3c6c7 100644 --- a/spec/factories/avis.rb +++ b/spec/factories/avis.rb @@ -1,27 +1,16 @@ FactoryBot.define do + sequence(:expert_email) { |n| "expert#{n}@expert.com" } + factory :avis do + email { generate(:expert_email) } introduction { 'Bonjour, merci de me donner votre avis sur ce dossier' } + confidentiel { false } - before(:create) do |avis, _evaluator| - if !avis.instructeur - avis.instructeur = create :instructeur - end - end - - before(:create) do |avis, _evaluator| - if !avis.dossier - avis.dossier = create :dossier - end - end - - before(:create) do |avis, _evaluator| - if !avis.claimant - avis.claimant = create :instructeur - end - end + association :dossier + association :claimant, factory: :instructeur trait :with_answer do - answer { "Mon avis se décompose en deux points :\n- La demande semble pertinente\n- Le demandeur remplit les conditions." } + answer { "Mon avis se décompose en deux points :\n- La demande semble pertinente\n- Le demandeur remplit les conditions." } end end end diff --git a/spec/views/instructeur/avis/instruction.html.haml_spec.rb b/spec/views/instructeur/avis/instruction.html.haml_spec.rb index 5d5800b4b..411d93f6c 100644 --- a/spec/views/instructeur/avis/instruction.html.haml_spec.rb +++ b/spec/views/instructeur/avis/instruction.html.haml_spec.rb @@ -1,9 +1,11 @@ describe 'instructeurs/avis/instruction.html.haml', type: :view do - let(:avis) { create(:avis, confidentiel: confidentiel) } + let(:expert) { create(:instructeur) } + let(:avis) { create(:avis, confidentiel: confidentiel, email: expert.email) } before do assign(:avis, avis) - @dossier = create(:dossier, :accepte) + assign(:new_avis, Avis.new) + assign(:dossier, avis.dossier) allow(view).to receive(:current_instructeur).and_return(avis.instructeur) end From f0c2599d1851d793f4d90f3b139ddb294a8d7319 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 21 Oct 2019 12:16:26 +0200 Subject: [PATCH 09/17] specs: improve feature spec for experts - Move the avis feature spec to a separate file - Add more test cases --- spec/features/instructeurs/expert_spec.rb | 127 ++++++++++++++++++ .../features/instructeurs/instruction_spec.rb | 53 -------- 2 files changed, 127 insertions(+), 53 deletions(-) create mode 100644 spec/features/instructeurs/expert_spec.rb diff --git a/spec/features/instructeurs/expert_spec.rb b/spec/features/instructeurs/expert_spec.rb new file mode 100644 index 000000000..c938c2f6d --- /dev/null +++ b/spec/features/instructeurs/expert_spec.rb @@ -0,0 +1,127 @@ +require 'spec_helper' + +feature 'Inviting an expert:' do + include ActiveJob::TestHelper + include ActionView::Helpers + + let(:instructeur) { create(:instructeur, password: 'démarches-simplifiées-pwd') } + let(:expert) { create(:instructeur, password: expert_password) } + let(:expert_password) { 'mot de passe d’expert' } + let(:procedure) { create(:procedure, :published, instructeurs: [instructeur]) } + let(:dossier) { create(:dossier, state: Dossier.states.fetch(:en_construction), procedure: procedure) } + + context 'as an Instructeur' do + scenario 'I can invite an expert' do + login_as instructeur.user, scope: :user + visit instructeur_dossier_path(procedure, dossier) + + click_on 'Avis externes' + expect(page).to have_current_path(avis_instructeur_dossier_path(procedure, dossier)) + + fill_in 'avis_emails', with: 'expert1@exemple.fr, expert2@exemple.fr' + fill_in 'avis_introduction', with: 'Bonjour, merci de me donner votre avis sur ce dossier.' + page.select 'confidentiel', from: 'avis_confidentiel' + + perform_enqueued_jobs do + click_on 'Demander un avis' + end + + expect(page).to have_content('Une demande d\'avis a été envoyée') + expect(page).to have_content('Avis des invités') + within('.list-avis') do + expect(page).to have_content('expert1@exemple.fr') + expect(page).to have_content('expert2@exemple.fr') + expect(page).to have_content('Bonjour, merci de me donner votre avis sur ce dossier.') + end + + invitation_email = open_email('expert2@exemple.fr') + avis = Avis.find_by(email: 'expert2@exemple.fr') + sign_up_link = sign_up_instructeur_avis_path(avis.id, avis.email) + expect(invitation_email.body).to include(sign_up_link) + end + + context 'when experts submitted their answer' do + let!(:answered_avis) { create(:avis, :with_answer, dossier: dossier, claimant: instructeur, email: expert.email) } + + scenario 'I can read the expert answer' do + login_as instructeur.user, scope: :user + visit instructeur_dossier_path(procedure, dossier) + + click_on 'Avis externes' + + expect(page).to have_content(expert.email) + answered_avis.answer.split("\n").each do |answer_line| + expect(page).to have_content(answer_line) + end + end + end + end + + context 'as an invited Expert' do + let(:avis_email) { expert.email } + let(:avis) { create(:avis, dossier: dossier, claimant: instructeur, email: avis_email, confidentiel: true) } + + context 'when I don’t already have an account' do + let(:avis_email) { 'not-signed-up-expert@exemple.fr' } + + scenario 'I can sign up' do + visit sign_up_instructeur_avis_path(avis.id, avis_email) + + expect(page).to have_field('Email', with: avis_email, disabled: true) + fill_in 'Mot de passe', with: 'This is a very complicated password !' + click_on 'Créer un compte' + + expect(page).to have_current_path(instructeur_avis_index_path) + expect(page).to have_text('avis à donner 1') + end + end + + context 'when I already have an existing account' do + let(:avis_email) { expert.email } + + scenario 'I can sign in' do + visit sign_up_instructeur_avis_path(avis.id, avis_email) + + expect(page).to have_current_path(new_user_session_path) + sign_in_with(expert.email, expert_password) + + expect(page).to have_current_path(instructeur_avis_index_path) + expect(page).to have_text('avis à donner 1') + end + end + + scenario 'I can give an answer' do + avis # create avis + login_as expert.user, scope: :user + + visit instructeur_avis_index_path + expect(page).to have_text('avis à donner 1') + expect(page).to have_text('avis donnés 0') + + click_on avis.dossier.user.email + + within('.tabs') { click_on 'Avis' } + expect(page).to have_text("Demandeur : #{instructeur.email}") + expect(page).to have_text('Cet avis est confidentiel') + + fill_in 'avis_answer', with: 'Ma réponse d’expert : c’est un oui.' + find('.piece-justificative input[type=file]').attach_file(Rails.root + 'spec/fixtures/files/RIB.pdf') + click_on 'Envoyer votre avis' + + expect(page).to have_content('Votre réponse est enregistrée') + expect(page).to have_content('Ma réponse d’expert : c’est un oui.') + expect(page).to have_content('RIB.pdf') + + within('.new-header') { click_on 'Avis' } + expect(page).to have_text('avis à donner 0') + expect(page).to have_text('avis donné 1') + end + + # TODO + # scenario 'I can read other experts advices' do + # end + + # scenario 'I can invite other experts' do + # end + end +end diff --git a/spec/features/instructeurs/instruction_spec.rb b/spec/features/instructeurs/instruction_spec.rb index e724e31dc..085b39417 100644 --- a/spec/features/instructeurs/instruction_spec.rb +++ b/spec/features/instructeurs/instruction_spec.rb @@ -93,59 +93,6 @@ feature 'Instructing a dossier:' do expect(page).to have_text('Aucun dossier') end - scenario 'A instructeur can use avis' do - log_in(instructeur.email, password) - - click_on procedure.libelle - click_on dossier.user.email - - click_on 'Avis externes' - expect(page).to have_current_path(avis_instructeur_dossier_path(procedure, dossier)) - - expert_email = 'expert@tps.com' - - perform_enqueued_jobs do - ask_confidential_avis(expert_email, 'a good introduction') - end - - log_out - - avis = dossier.avis.first - test_mail(expert_email, sign_up_instructeur_avis_path(avis, expert_email)) - - avis_sign_up(avis, expert_email) - - expect(page).to have_current_path(instructeur_avis_index_path) - expect(page).to have_text('avis à donner 1') - expect(page).to have_text('avis donnés 0') - - click_on dossier.user.email - expect(page).to have_current_path(instructeur_avis_path(dossier.avis.first)) - - within(:css, '.tabs') do - click_on 'Avis' - end - expect(page).to have_current_path(instruction_instructeur_avis_path(dossier.avis.first)) - - within(:css, '.give-avis') do - expect(page).to have_text("Demandeur : #{instructeur.email}") - expect(page).to have_text('a good introduction') - expect(page).to have_text('Cet avis est confidentiel') - fill_in 'avis_answer', with: 'a great answer' - click_on 'Envoyer votre avis' - end - - log_out - - log_in(instructeur.email, password, check_email: false) - - click_on procedure.libelle - click_on dossier.user.email - click_on 'Avis externes' - - expect(page).to have_text('a great answer') - end - scenario 'A instructeur can see the personnes impliquées' do instructeur2 = FactoryBot.create(:instructeur, password: password) From 1af2b63ed150d997c52902faf595ca1b017395dc Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Mon, 30 Sep 2019 11:57:21 +0200 Subject: [PATCH 10/17] initial implementation of async export --- .../instructeurs/procedures_controller.rb | 8 +++++ app/jobs/download_dossiers_job.rb | 32 +++++++++++++++++++ app/mailers/instructeur_mailer.rb | 8 +++++ .../download_procedure.html.haml | 8 +++++ .../procedures/_download_dossiers.html.haml | 6 ++++ config/routes.rb | 1 + 6 files changed, 63 insertions(+) create mode 100644 app/jobs/download_dossiers_job.rb create mode 100644 app/views/instructeur_mailer/download_procedure.html.haml diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 139d311ed..4284eaf9e 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -205,6 +205,14 @@ module Instructeurs end end + def download_dossiers_mail + options = params.permit(:format, tables: []) + DownloadDossiersJob.perform_later(procedure, options, current_instructeur) + + flash.notice = "Le dossier va vous être envoyé par mail" + redirect_to procedure + end + def email_notifications @procedure = procedure @assign_to = assign_to diff --git a/app/jobs/download_dossiers_job.rb b/app/jobs/download_dossiers_job.rb new file mode 100644 index 000000000..8d3401eaf --- /dev/null +++ b/app/jobs/download_dossiers_job.rb @@ -0,0 +1,32 @@ +class DownloadDossiersJob < ApplicationJob + def perform(procedure, options, instructeur) + dossiers = instructeur.dossiers.for_procedure(procedure) + format = options[:format] + options.delete(:format) + + case format + when 'csv' + filename = procedure.export_filename(:csv) + data = procedure.to_csv(dossiers, options) + when 'xlsx' + filename = procedure.export_filename(:xlsx) + data = procedure.to_xlsx(dossiers, options) + when 'ods' + filename = procedure.export_filename(:ods) + data = procedure.to_ods(dossiers, options) + end + + file_path = File.join('/tmp/', filename) + IO.write(file_path, data) + + File.open(file_path) do |io| + blob = ActiveStorage::Blob.create_after_upload!( + io: io, + filename: filename + ) + + InstructeurMailer.download_procedure(instructeur, procedure, blob).deliver_now + File.delete(file_path) + end + end +end diff --git a/app/mailers/instructeur_mailer.rb b/app/mailers/instructeur_mailer.rb index aa28622d2..8aa2d9a83 100644 --- a/app/mailers/instructeur_mailer.rb +++ b/app/mailers/instructeur_mailer.rb @@ -42,4 +42,12 @@ class InstructeurMailer < ApplicationMailer mail(to: instructeur.email, subject: subject) end + + def download_procedure(instructeur, procedure, blob) + @procedure = procedure + @lien_telechargement = url_for(blob) + subject = "Votre export de la procédure #{procedure.id} est disponible" + + mail(to: instructeur.email, subject: subject) + end end diff --git a/app/views/instructeur_mailer/download_procedure.html.haml b/app/views/instructeur_mailer/download_procedure.html.haml new file mode 100644 index 000000000..9c8108f6c --- /dev/null +++ b/app/views/instructeur_mailer/download_procedure.html.haml @@ -0,0 +1,8 @@ +%p + Bonjour, + +%p + Vous avez demandé un export des dossiers de la procédure nº #{@procedure.id} « #{@procedure.libelle} ». Cliquez sur le lien ci-dessous pour le télécharger : + = link_to('Télécharger les dossiers', @lien_telechargement) + += render partial: "layouts/mailers/signature" diff --git a/app/views/instructeurs/procedures/_download_dossiers.html.haml b/app/views/instructeurs/procedures/_download_dossiers.html.haml index 074752555..387759623 100644 --- a/app/views/instructeurs/procedures/_download_dossiers.html.haml +++ b/app/views/instructeurs/procedures/_download_dossiers.html.haml @@ -20,3 +20,9 @@ = link_to "Au format .ods #{old_format_message}", procedure_dossiers_download_path(procedure, format: :ods, version: 'v1'), target: "_blank", rel: "noopener" %li = link_to "Au format .csv #{old_format_message}", procedure_dossiers_download_path(procedure, format: :csv, version: 'v1'), target: "_blank", rel: "noopener" + %li + = link_to "Par mail, au format .xlsx", download_dossiers_mail_instructeur_procedure_path(procedure, format: :xlsx, version: 'v2') + %li + = link_to "Par mail, au format .ods", download_dossiers_mail_instructeur_procedure_path(procedure, format: :ods, version: 'v2') + %li + = link_to "Par mail, au format .csv", download_dossiers_mail_instructeur_procedure_path(procedure, format: :csv, version: 'v2') diff --git a/config/routes.rb b/config/routes.rb index 595299fcd..7ddf6c7c7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -293,6 +293,7 @@ Rails.application.routes.draw do post 'add_filter' get 'remove_filter' => 'procedures#remove_filter', as: 'remove_filter' get 'download_dossiers' + get 'download_dossiers_mail' get 'stats' get 'email_notifications' patch 'update_email_notifications' From 70ea5e167efcf60823137f3ad9b149ee0ffdebcc Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Wed, 2 Oct 2019 15:51:37 +0200 Subject: [PATCH 11/17] procedure download is performed through a controller in order not to leak the URL --- .../instructeurs/procedures_controller.rb | 12 ++++++++++-- ...oad_dossiers_job.rb => export_procedure_job.rb} | 14 +++++++------- app/mailers/instructeur_mailer.rb | 3 +-- app/models/procedure.rb | 1 + ...ml.haml => download_procedure_export.html.haml} | 2 +- config/routes.rb | 1 + 6 files changed, 21 insertions(+), 12 deletions(-) rename app/jobs/{download_dossiers_job.rb => export_procedure_job.rb} (58%) rename app/views/instructeur_mailer/{download_procedure.html.haml => download_procedure_export.html.haml} (70%) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 4284eaf9e..271ea602b 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -206,13 +206,21 @@ module Instructeurs end def download_dossiers_mail - options = params.permit(:format, tables: []) - DownloadDossiersJob.perform_later(procedure, options, current_instructeur) + ExportProcedureJob.perform_later(procedure, current_instructeur, params[:format]) flash.notice = "Le dossier va vous être envoyé par mail" redirect_to procedure end + def download_export + if procedure.export_file.attachment.created_at < 1.day.ago + flash.alert = "Cet export n'est plus disponible. Vous devez en générer un nouveau qui vous sera transmis par mail" + redirect_to instructeur_procedure_url(procedure) + else + redirect_to url_for(procedure.export_file) + end + end + def email_notifications @procedure = procedure @assign_to = assign_to diff --git a/app/jobs/download_dossiers_job.rb b/app/jobs/export_procedure_job.rb similarity index 58% rename from app/jobs/download_dossiers_job.rb rename to app/jobs/export_procedure_job.rb index 8d3401eaf..194c13ebe 100644 --- a/app/jobs/download_dossiers_job.rb +++ b/app/jobs/export_procedure_job.rb @@ -1,10 +1,9 @@ -class DownloadDossiersJob < ApplicationJob - def perform(procedure, options, instructeur) +class ExportProcedureJob < ApplicationJob + def perform(procedure, instructeur, export_format) dossiers = instructeur.dossiers.for_procedure(procedure) - format = options[:format] - options.delete(:format) + options = { :version => 'v2', :tables => [:dossiers, :etablissements] } - case format + case export_format when 'csv' filename = procedure.export_filename(:csv) data = procedure.to_csv(dossiers, options) @@ -20,12 +19,13 @@ class DownloadDossiersJob < ApplicationJob IO.write(file_path, data) File.open(file_path) do |io| - blob = ActiveStorage::Blob.create_after_upload!( + # todo: add a TTL to the uploaded file, even though it's checked for in the controller too + procedure.export_file = ActiveStorage::Blob.create_after_upload!( io: io, filename: filename ) - InstructeurMailer.download_procedure(instructeur, procedure, blob).deliver_now + InstructeurMailer.download_procedure_export(instructeur, procedure).deliver_now File.delete(file_path) end end diff --git a/app/mailers/instructeur_mailer.rb b/app/mailers/instructeur_mailer.rb index 8aa2d9a83..52c4fe429 100644 --- a/app/mailers/instructeur_mailer.rb +++ b/app/mailers/instructeur_mailer.rb @@ -43,9 +43,8 @@ class InstructeurMailer < ApplicationMailer mail(to: instructeur.email, subject: subject) end - def download_procedure(instructeur, procedure, blob) + def download_procedure_export(instructeur, procedure) @procedure = procedure - @lien_telechargement = url_for(blob) subject = "Votre export de la procédure #{procedure.id} est disponible" mail(to: instructeur.email, subject: subject) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 1d24b7ae4..843e2ce47 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -34,6 +34,7 @@ class Procedure < ApplicationRecord has_one_attached :logo has_one_attached :notice has_one_attached :deliberation + has_one_attached :export_file accepts_nested_attributes_for :types_de_champ, reject_if: proc { |attributes| attributes['libelle'].blank? }, allow_destroy: true accepts_nested_attributes_for :types_de_champ_private, reject_if: proc { |attributes| attributes['libelle'].blank? }, allow_destroy: true diff --git a/app/views/instructeur_mailer/download_procedure.html.haml b/app/views/instructeur_mailer/download_procedure_export.html.haml similarity index 70% rename from app/views/instructeur_mailer/download_procedure.html.haml rename to app/views/instructeur_mailer/download_procedure_export.html.haml index 9c8108f6c..251f6668f 100644 --- a/app/views/instructeur_mailer/download_procedure.html.haml +++ b/app/views/instructeur_mailer/download_procedure_export.html.haml @@ -3,6 +3,6 @@ %p Vous avez demandé un export des dossiers de la procédure nº #{@procedure.id} « #{@procedure.libelle} ». Cliquez sur le lien ci-dessous pour le télécharger : - = link_to('Télécharger les dossiers', @lien_telechargement) + = link_to('Télécharger les dossiers', download_export_instructeur_procedure_url(@procedure)) = render partial: "layouts/mailers/signature" diff --git a/config/routes.rb b/config/routes.rb index 7ddf6c7c7..51cb1b35e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -294,6 +294,7 @@ Rails.application.routes.draw do get 'remove_filter' => 'procedures#remove_filter', as: 'remove_filter' get 'download_dossiers' get 'download_dossiers_mail' + get 'download_export' get 'stats' get 'email_notifications' patch 'update_email_notifications' From 43424e4f4e84d8db90a4f03e5012cf2ed7730815 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 3 Oct 2019 15:35:31 +0200 Subject: [PATCH 12/17] merge with the work of paul, using 3 links --- .../instructeurs/procedures_controller.rb | 19 +-- app/jobs/export_procedure_job.rb | 30 +--- app/mailers/instructeur_mailer.rb | 5 +- app/models/procedure.rb | 90 +++++++++- .../download_procedure_export.html.haml | 8 - ...otify_procedure_export_available.html.haml | 11 ++ .../procedures/_download_dossiers.html.haml | 41 +++-- config/initializers/flipper.rb | 1 + config/routes.rb | 1 - ...9140415_add_export_queued_to_procedures.rb | 7 + db/schema.rb | 3 + spec/factories/procedure.rb | 42 +++++ spec/models/procedure_spec.rb | 161 ++++++++++++++++++ 13 files changed, 356 insertions(+), 63 deletions(-) delete mode 100644 app/views/instructeur_mailer/download_procedure_export.html.haml create mode 100644 app/views/instructeur_mailer/notify_procedure_export_available.html.haml create mode 100644 db/migrate/20190709140415_add_export_queued_to_procedures.rb diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 271ea602b..37a0f2c2c 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -205,19 +205,16 @@ module Instructeurs end end - def download_dossiers_mail - ExportProcedureJob.perform_later(procedure, current_instructeur, params[:format]) - - flash.notice = "Le dossier va vous être envoyé par mail" - redirect_to procedure - end - def download_export - if procedure.export_file.attachment.created_at < 1.day.ago - flash.alert = "Cet export n'est plus disponible. Vous devez en générer un nouveau qui vous sera transmis par mail" - redirect_to instructeur_procedure_url(procedure) + export_format = params[:export_format] + + if procedure.should_generate_export?(export_format) + procedure.queue_export(current_instructeur, export_format) + + flash.now.notice = "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." + redirect_to procedure else - redirect_to url_for(procedure.export_file) + redirect_to url_for(procedure.export_file(export_format)) end end diff --git a/app/jobs/export_procedure_job.rb b/app/jobs/export_procedure_job.rb index 194c13ebe..7e935d8ac 100644 --- a/app/jobs/export_procedure_job.rb +++ b/app/jobs/export_procedure_job.rb @@ -1,32 +1,6 @@ class ExportProcedureJob < ApplicationJob def perform(procedure, instructeur, export_format) - dossiers = instructeur.dossiers.for_procedure(procedure) - options = { :version => 'v2', :tables => [:dossiers, :etablissements] } - - case export_format - when 'csv' - filename = procedure.export_filename(:csv) - data = procedure.to_csv(dossiers, options) - when 'xlsx' - filename = procedure.export_filename(:xlsx) - data = procedure.to_xlsx(dossiers, options) - when 'ods' - filename = procedure.export_filename(:ods) - data = procedure.to_ods(dossiers, options) - end - - file_path = File.join('/tmp/', filename) - IO.write(file_path, data) - - File.open(file_path) do |io| - # todo: add a TTL to the uploaded file, even though it's checked for in the controller too - procedure.export_file = ActiveStorage::Blob.create_after_upload!( - io: io, - filename: filename - ) - - InstructeurMailer.download_procedure_export(instructeur, procedure).deliver_now - File.delete(file_path) - end + procedure.prepare_export_download(export_format) + InstructeurMailer.notify_procedure_export_available(instructeur, procedure, export_format).deliver_later end end diff --git a/app/mailers/instructeur_mailer.rb b/app/mailers/instructeur_mailer.rb index 52c4fe429..d0aae6b81 100644 --- a/app/mailers/instructeur_mailer.rb +++ b/app/mailers/instructeur_mailer.rb @@ -43,9 +43,10 @@ class InstructeurMailer < ApplicationMailer mail(to: instructeur.email, subject: subject) end - def download_procedure_export(instructeur, procedure) + def notify_procedure_export_available(instructeur, procedure, export_format) @procedure = procedure - subject = "Votre export de la procédure #{procedure.id} est disponible" + @export_format = export_format + subject = "Votre export de la procédure nº #{procedure.id} est disponible" mail(to: instructeur.email, subject: subject) end diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 843e2ce47..240fc3998 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -34,7 +34,10 @@ class Procedure < ApplicationRecord has_one_attached :logo has_one_attached :notice has_one_attached :deliberation - has_one_attached :export_file + + has_one_attached :csv_export_file + has_one_attached :xlsx_export_file + has_one_attached :ods_export_file accepts_nested_attributes_for :types_de_champ, reject_if: proc { |attributes| attributes['libelle'].blank? }, allow_destroy: true accepts_nested_attributes_for :types_de_champ_private, reject_if: proc { |attributes| attributes['libelle'].blank? }, allow_destroy: true @@ -129,11 +132,88 @@ class Procedure < ApplicationRecord end end + def csv_export_stale? + !csv_export_file.attached? || csv_export_file.created_at < 3.hours.ago + end + + def xlsx_export_stale? + !xlsx_export_file.attached? || xlsx_export_file.created_at < 3.hours.ago + end + + def ods_export_stale? + !ods_export_file.attached? || ods_export_file.created_at < 3.hours.ago + end + + def should_generate_export?(format) + case format.to_sym + when :csv + return csv_export_stale? && !csv_export_queued? + when :xlsx + return xlsx_export_stale? && !xlsx_export_queued? + when :ods + return ods_export_stale? && !ods_export_queued? + end + false + end + + def export_file(export_format) + case export_format.to_sym + when :csv + csv_export_file + when :xlsx + xlsx_export_file + when :ods + ods_export_file + end + end + + def queue_export(instructeur, export_format) + ExportProcedureJob.perform_later(procedure, instructeur, export_format) + case export_format.to_sym + when :csv + update(csv_export_queued: true) + when :xlsx + update(xlsx_export_queued: true) + when :ods + update(ods_export_queued: true) + end + end + + def prepare_export_download(format) + service = ProcedureExportV2Service.new(self, self.dossiers) + filename = export_filename(format) + + case format.to_sym + when :csv + csv_export_file.attach( + io: StringIO.new(service.to_csv), + filename: filename, + content_type: 'text/csv' + ) + update(csv_export_queued: false) + when :xlsx + xlsx_export_file.attach( + io: StringIO.new(service.to_xlsx), + filename: filename, + content_type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' + ) + update(xlsx_export_queued: false) + when :ods + ods_export_file.attach( + io: StringIO.new(service.to_ods), + filename: filename, + content_type: 'application/vnd.oasis.opendocument.spreadsheet' + ) + update(ods_export_queued: false) + end + end + def reset! if locked? raise "Can not reset a locked procedure." else groupe_instructeurs.each { |gi| gi.dossiers.destroy_all } + purge_export_files end end @@ -175,6 +255,12 @@ class Procedure < ApplicationRecord procedure.blank? || administrateur.owns?(procedure) end + def purge_export_files + xlsx_export_file.purge_later + ods_export_file.purge_later + csv_export_file.purge_later + end + def locked? publiee_ou_archivee? end @@ -514,12 +600,14 @@ class Procedure < ApplicationRecord def after_archive update!(archived_at: Time.zone.now) + purge_export_files end def after_hide now = Time.zone.now update!(hidden_at: now) dossiers.update_all(hidden_at: now) + purge_export_files end def after_draft diff --git a/app/views/instructeur_mailer/download_procedure_export.html.haml b/app/views/instructeur_mailer/download_procedure_export.html.haml deleted file mode 100644 index 251f6668f..000000000 --- a/app/views/instructeur_mailer/download_procedure_export.html.haml +++ /dev/null @@ -1,8 +0,0 @@ -%p - Bonjour, - -%p - Vous avez demandé un export des dossiers de la procédure nº #{@procedure.id} « #{@procedure.libelle} ». Cliquez sur le lien ci-dessous pour le télécharger : - = link_to('Télécharger les dossiers', download_export_instructeur_procedure_url(@procedure)) - -= render partial: "layouts/mailers/signature" diff --git a/app/views/instructeur_mailer/notify_procedure_export_available.html.haml b/app/views/instructeur_mailer/notify_procedure_export_available.html.haml new file mode 100644 index 000000000..cc9425645 --- /dev/null +++ b/app/views/instructeur_mailer/notify_procedure_export_available.html.haml @@ -0,0 +1,11 @@ +%p + Bonjour, + +%p + Vous avez demandé un export des dossiers de la procédure nº #{@procedure.id} « #{@procedure.libelle} » au format #{@export_format}. + +%p + Cliquez sur le lien ci-dessous pour le télécharger : + = link_to('Télécharger l\'export des dossiers', download_export_instructeur_procedure_url(@procedure, :export_format => @export_format)) + += render partial: "layouts/mailers/signature" diff --git a/app/views/instructeurs/procedures/_download_dossiers.html.haml b/app/views/instructeurs/procedures/_download_dossiers.html.haml index 387759623..2eab6536e 100644 --- a/app/views/instructeurs/procedures/_download_dossiers.html.haml +++ b/app/views/instructeurs/procedures/_download_dossiers.html.haml @@ -4,14 +4,37 @@ Télécharger tous les dossiers - old_format_limit_date = Date.parse("Oct 31 2019") - export_v1_enabled = old_format_limit_date > Time.zone.today + - export_v2_enabled = Flipper[:procedure_export_v2_enabled] || !export_v1_enabled .dropdown-content.fade-in-down{ style: !export_v1_enabled ? 'width: 330px' : '' } %ul.dropdown-items - %li - = link_to "Au format .xlsx", procedure_dossiers_download_path(procedure, format: :xlsx, version: 'v2'), target: "_blank", rel: "noopener" - %li - = link_to "Au format .ods", procedure_dossiers_download_path(procedure, format: :ods, version: 'v2'), target: "_blank", rel: "noopener" - %li - = link_to "Au format .csv", procedure_dossiers_download_path(procedure, format: :csv, version: 'v2'), target: "_blank", rel: "noopener" + - if export_v2_enabled + %li + - if procedure.xlsx_export_stale? + - if procedure.xlsx_export_queued? + L'export au format .xlsx est en cours de préparation + = link_to "rafraichir", download_export_instructeur_procedure_path(procedure, export_format: :xlsx), remote: true + - else + = link_to "Préparer le téléchargement de l'export 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 + = link_to "rafraichir", download_export_instructeur_procedure_path(procedure, export_format: :ods), remote: true + - else + = link_to "Préparer le téléchargement de l'export 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 + = link_to "rafraichir", download_export_instructeur_procedure_path(procedure, export_format: :csv), remote: true + - else + = link_to "Préparer le téléchargement de l'export 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" - if export_v1_enabled - old_format_message = "(ancien format, jusqu’au #{old_format_limit_date.strftime('%d/%m/%Y')})" %li @@ -20,9 +43,3 @@ = link_to "Au format .ods #{old_format_message}", procedure_dossiers_download_path(procedure, format: :ods, version: 'v1'), target: "_blank", rel: "noopener" %li = link_to "Au format .csv #{old_format_message}", procedure_dossiers_download_path(procedure, format: :csv, version: 'v1'), target: "_blank", rel: "noopener" - %li - = link_to "Par mail, au format .xlsx", download_dossiers_mail_instructeur_procedure_path(procedure, format: :xlsx, version: 'v2') - %li - = link_to "Par mail, au format .ods", download_dossiers_mail_instructeur_procedure_path(procedure, format: :ods, version: 'v2') - %li - = link_to "Par mail, au format .csv", download_dossiers_mail_instructeur_procedure_path(procedure, format: :csv, version: 'v2') diff --git a/config/initializers/flipper.rb b/config/initializers/flipper.rb index e8172d0da..0718c0e6a 100644 --- a/config/initializers/flipper.rb +++ b/config/initializers/flipper.rb @@ -34,6 +34,7 @@ features = [ :mini_profiler, :operation_log_serialize_subject, :pre_maintenance_mode, + :procedure_export_v2_enabled, :xray ] diff --git a/config/routes.rb b/config/routes.rb index 51cb1b35e..d60cd0c65 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -293,7 +293,6 @@ Rails.application.routes.draw do post 'add_filter' get 'remove_filter' => 'procedures#remove_filter', as: 'remove_filter' get 'download_dossiers' - get 'download_dossiers_mail' get 'download_export' get 'stats' get 'email_notifications' diff --git a/db/migrate/20190709140415_add_export_queued_to_procedures.rb b/db/migrate/20190709140415_add_export_queued_to_procedures.rb new file mode 100644 index 000000000..471d61fd0 --- /dev/null +++ b/db/migrate/20190709140415_add_export_queued_to_procedures.rb @@ -0,0 +1,7 @@ +class AddExportQueuedToProcedures < ActiveRecord::Migration[5.2] + def change + add_column :procedures, :csv_export_queued, :boolean + add_column :procedures, :xlsx_export_queued, :boolean + add_column :procedures, :ods_export_queued, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index a9b5f57a9..3736ce621 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -487,6 +487,9 @@ ActiveRecord::Schema.define(version: 2019_10_14_160538) do t.string "declarative_with_state" t.text "monavis_embed" t.text "routing_criteria_name" + t.boolean "csv_export_queued" + t.boolean "xlsx_export_queued" + t.boolean "ods_export_queued" t.index ["declarative_with_state"], name: "index_procedures_on_declarative_with_state" t.index ["hidden_at"], name: "index_procedures_on_hidden_at" t.index ["parent_procedure_id"], name: "index_procedures_on_parent_procedure_id" diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb index 43dc38211..b392caba3 100644 --- a/spec/factories/procedure.rb +++ b/spec/factories/procedure.rb @@ -242,5 +242,47 @@ FactoryBot.define do end end end + + trait :with_csv_export_file do + after(:create) do |procedure, _evaluator| + procedure.csv_export_file.attach(io: StringIO.new("some csv data"), filename: "export.csv", content_type: "text/plain") + procedure.csv_export_file.update(created_at: 5.minutes.ago) + end + end + + trait :with_stale_csv_export_file do + after(:create) do |procedure, _evaluator| + procedure.csv_export_file.attach(io: StringIO.new("some csv data"), filename: "export.csv", content_type: "text/plain") + procedure.csv_export_file.update(created_at: 4.hours.ago) + end + end + + trait :with_ods_export_file do + after(:create) do |procedure, _evaluator| + procedure.ods_export_file.attach(io: StringIO.new("some ods data"), filename: "export.ods", content_type: "text/plain") + procedure.ods_export_file.update(created_at: 5.minutes.ago) + end + end + + trait :with_stale_ods_export_file do + after(:create) do |procedure, _evaluator| + procedure.ods_export_file.attach(io: StringIO.new("some ods data"), filename: "export.ods", content_type: "text/plain") + procedure.ods_export_file.update(created_at: 4.hours.ago) + end + end + + trait :with_xlsx_export_file do + after(:create) do |procedure, _evaluator| + procedure.xlsx_export_file.attach(io: StringIO.new("some xlsx data"), filename: "export.xlsx", content_type: "text/plain") + procedure.xlsx_export_file.update(created_at: 5.minutes.ago) + end + end + + trait :with_stale_xlsx_export_file do + after(:create) do |procedure, _evaluator| + procedure.xlsx_export_file.attach(io: StringIO.new("some xlsx data"), filename: "export.xlsx", content_type: "text/plain") + procedure.xlsx_export_file.update(created_at: 4.hours.ago) + end + end end end diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index c95879de4..d63ff1670 100644 --- a/spec/models/procedure_spec.rb +++ b/spec/models/procedure_spec.rb @@ -954,4 +954,165 @@ describe Procedure do it { is_expected.to be false } end end + + describe '.ods_export_stale?' do + subject { procedure.ods_export_stale? } + + context 'with no ods export' do + let(:procedure) { create(:procedure) } + it { is_expected.to be true } + end + + context 'with a recent ods export' do + let(:procedure) { create(:procedure, :with_ods_export_file) } + it { is_expected.to be false } + end + + context 'with an old ods export' do + let(:procedure) { create(:procedure, :with_stale_ods_export_file) } + it { is_expected.to be true } + end + end + + describe '.csv_export_stale?' do + subject { procedure.csv_export_stale? } + + context 'with no csv export' do + let(:procedure) { create(:procedure) } + it { is_expected.to be true } + end + + context 'with a recent csv export' do + let(:procedure) { create(:procedure, :with_csv_export_file) } + it { is_expected.to be false } + end + + context 'with an old csv export' do + let(:procedure) { create(:procedure, :with_stale_csv_export_file) } + it { is_expected.to be true } + end + end + + describe '.xlsx_export_stale?' do + subject { procedure.xlsx_export_stale? } + + context 'with no xlsx export' do + let(:procedure) { create(:procedure) } + it { is_expected.to be true } + end + + context 'with a recent xlsx export' do + let(:procedure) { create(:procedure, :with_xlsx_export_file) } + it { is_expected.to be false } + end + + context 'with an old xlsx export' do + let(:procedure) { create(:procedure, :with_stale_xlsx_export_file) } + it { is_expected.to be true } + end + end + + describe '.should_generate_export?' do + context 'xlsx' do + subject { procedure.should_generate_export?('xlsx') } + context 'with no export' do + let(:procedure) { create(:procedure) } + it { is_expected.to be true } + end + + context 'with a recent export' do + context 'when its not queued' do + let(:procedure) { create(:procedure, :with_xlsx_export_file, xlsx_export_queued: false) } + it { is_expected.to be false } + end + + context 'when its already queued' do + let(:procedure) { create(:procedure, :with_xlsx_export_file, xlsx_export_queued: true) } + it { expect(procedure.xlsx_export_queued).to be true } + it { is_expected.to be false } + end + end + + context 'with an old export' do + context 'when its not queued' do + let(:procedure) { create(:procedure, :with_stale_xlsx_export_file, xlsx_export_queued: false) } + it { is_expected.to be true } + end + + context 'when its already queued' do + let(:procedure) { create(:procedure, :with_stale_xlsx_export_file, xlsx_export_queued: true) } + it { expect(procedure.xlsx_export_queued).to be true } + it { is_expected.to be false } + end + end + end + + context 'csv' do + subject { procedure.should_generate_export?('csv') } + context 'with no export' do + let(:procedure) { create(:procedure) } + it { is_expected.to be true } + end + + context 'with a recent export' do + context 'when its not queued' do + let(:procedure) { create(:procedure, :with_csv_export_file, csv_export_queued: false) } + it { is_expected.to be false } + end + + context 'when its already queued' do + let(:procedure) { create(:procedure, :with_csv_export_file, csv_export_queued: true) } + it { expect(procedure.csv_export_queued).to be true } + it { is_expected.to be false } + end + end + + context 'with an old export' do + context 'when its not queued' do + let(:procedure) { create(:procedure, :with_stale_csv_export_file, csv_export_queued: false) } + it { is_expected.to be true } + end + + context 'when its already queued' do + let(:procedure) { create(:procedure, :with_stale_csv_export_file, csv_export_queued: true) } + it { expect(procedure.csv_export_queued).to be true } + it { is_expected.to be false } + end + end + end + + context 'ods' do + subject { procedure.should_generate_export?('ods') } + context 'with no export' do + let(:procedure) { create(:procedure) } + it { is_expected.to be true } + end + + context 'with a recent export' do + context 'when its not queued' do + let(:procedure) { create(:procedure, :with_ods_export_file, ods_export_queued: false) } + it { is_expected.to be false } + end + + context 'when its already queued' do + let(:procedure) { create(:procedure, :with_ods_export_file, ods_export_queued: true) } + it { expect(procedure.ods_export_queued).to be true } + it { is_expected.to be false } + end + end + + context 'with an old export' do + context 'when its not queued' do + let(:procedure) { create(:procedure, :with_stale_ods_export_file, ods_export_queued: false) } + it { is_expected.to be true } + end + + context 'when its already queued' do + let(:procedure) { create(:procedure, :with_stale_ods_export_file, ods_export_queued: true) } + it { expect(procedure.ods_export_queued).to be true } + it { is_expected.to be false } + end + end + end + end end From cdab08b19832c699ced0b1416934c35943ff2531 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Wed, 16 Oct 2019 11:40:19 +0200 Subject: [PATCH 13/17] UI work --- .../instructeurs/procedures_controller.rb | 8 ++++++-- app/mailers/instructeur_mailer.rb | 2 +- app/models/procedure.rb | 4 +++- .../notify_procedure_export_available.html.haml | 2 +- .../procedures/_download_dossiers.html.haml | 13 +++++-------- .../instructeurs/procedures/download_export.js.erb | 2 ++ 6 files changed, 18 insertions(+), 13 deletions(-) create mode 100644 app/views/instructeurs/procedures/download_export.js.erb diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 37a0f2c2c..9b76145ff 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -211,8 +211,12 @@ module Instructeurs if procedure.should_generate_export?(export_format) procedure.queue_export(current_instructeur, export_format) - flash.now.notice = "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." - redirect_to procedure + respond_to do |format| + format.js do + flash.notice = "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." + @procedure = procedure + end + end else redirect_to url_for(procedure.export_file(export_format)) end diff --git a/app/mailers/instructeur_mailer.rb b/app/mailers/instructeur_mailer.rb index d0aae6b81..d132c0005 100644 --- a/app/mailers/instructeur_mailer.rb +++ b/app/mailers/instructeur_mailer.rb @@ -46,7 +46,7 @@ class InstructeurMailer < ApplicationMailer def notify_procedure_export_available(instructeur, procedure, export_format) @procedure = procedure @export_format = export_format - subject = "Votre export de la procédure nº #{procedure.id} est disponible" + subject = "Votre export de la démarche nº #{procedure.id} est disponible" mail(to: instructeur.email, subject: subject) end diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 240fc3998..4e14e7f94 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -168,7 +168,7 @@ class Procedure < ApplicationRecord end def queue_export(instructeur, export_format) - ExportProcedureJob.perform_later(procedure, instructeur, export_format) + ExportProcedureJob.perform_now(self, instructeur, export_format) case export_format.to_sym when :csv update(csv_export_queued: true) @@ -259,6 +259,8 @@ class Procedure < ApplicationRecord xlsx_export_file.purge_later ods_export_file.purge_later csv_export_file.purge_later + + update(csv_export_queued: false, xlsx_export_queued: false, ods_export_queued: false) end def locked? diff --git a/app/views/instructeur_mailer/notify_procedure_export_available.html.haml b/app/views/instructeur_mailer/notify_procedure_export_available.html.haml index cc9425645..678dfd08b 100644 --- a/app/views/instructeur_mailer/notify_procedure_export_available.html.haml +++ b/app/views/instructeur_mailer/notify_procedure_export_available.html.haml @@ -2,7 +2,7 @@ Bonjour, %p - Vous avez demandé un export des dossiers de la procédure nº #{@procedure.id} « #{@procedure.libelle} » au format #{@export_format}. + Votre export des dossiers de la démarche nº #{@procedure.id} « #{@procedure.libelle} » au format #{@export_format} est prêt. %p Cliquez sur le lien ci-dessous pour le télécharger : diff --git a/app/views/instructeurs/procedures/_download_dossiers.html.haml b/app/views/instructeurs/procedures/_download_dossiers.html.haml index 2eab6536e..10902baae 100644 --- a/app/views/instructeurs/procedures/_download_dossiers.html.haml +++ b/app/views/instructeurs/procedures/_download_dossiers.html.haml @@ -11,26 +11,23 @@ %li - if procedure.xlsx_export_stale? - if procedure.xlsx_export_queued? - L'export au format .xlsx est en cours de préparation - = link_to "rafraichir", download_export_instructeur_procedure_path(procedure, export_format: :xlsx), remote: true + L'export au format .xlsx est en cours de préparation, vous recevrez un email lorsqu'il sera disponible. - else - = link_to "Préparer le téléchargement de l'export au format .xlsx", download_export_instructeur_procedure_path(procedure, export_format: :xlsx), remote: true + = 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 - = link_to "rafraichir", download_export_instructeur_procedure_path(procedure, export_format: :ods), remote: true + L'export au format .ods est en cours de préparation, vous recevrez un email lorsqu'il sera disponible. - else - = link_to "Préparer le téléchargement de l'export au format .ods", download_export_instructeur_procedure_path(procedure, export_format: :ods)#, remote: true + = link_to "Préparer le téléchargement de l'export 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 - = link_to "rafraichir", download_export_instructeur_procedure_path(procedure, export_format: :csv), remote: true + L'export au format .csv est en cours de préparation, vous recevrez un email lorsqu'il sera disponible. - else = link_to "Préparer le téléchargement de l'export au format .csv", download_export_instructeur_procedure_path(procedure, export_format: :csv), remote: true - else diff --git a/app/views/instructeurs/procedures/download_export.js.erb b/app/views/instructeurs/procedures/download_export.js.erb new file mode 100644 index 000000000..cb6442733 --- /dev/null +++ b/app/views/instructeurs/procedures/download_export.js.erb @@ -0,0 +1,2 @@ +<%= render_to_element('.procedure-actions', partial: "download_dossiers", locals: { procedure: @procedure }) %> +<%= render_flash %> From 87741f4b4903bf18dba78b5c2d53d57f2e55fb8d Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Mon, 21 Oct 2019 17:14:56 +0200 Subject: [PATCH 14/17] introduce constant for exports --- app/models/procedure.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 4e14e7f94..d4703b465 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -6,6 +6,7 @@ class Procedure < ApplicationRecord include ProcedureStatsConcern MAX_DUREE_CONSERVATION = 36 + MAX_DUREE_CONSERVATION_EXPORT = 3.hours has_many :types_de_champ, -> { root.public_only.ordered }, inverse_of: :procedure, dependent: :destroy has_many :types_de_champ_private, -> { root.private_only.ordered }, class_name: 'TypeDeChamp', inverse_of: :procedure, dependent: :destroy @@ -133,15 +134,15 @@ class Procedure < ApplicationRecord end def csv_export_stale? - !csv_export_file.attached? || csv_export_file.created_at < 3.hours.ago + !csv_export_file.attached? || csv_export_file.created_at < MAX_DUREE_CONSERVATION_EXPORT.ago end def xlsx_export_stale? - !xlsx_export_file.attached? || xlsx_export_file.created_at < 3.hours.ago + !xlsx_export_file.attached? || xlsx_export_file.created_at < MAX_DUREE_CONSERVATION_EXPORT.ago end def ods_export_stale? - !ods_export_file.attached? || ods_export_file.created_at < 3.hours.ago + !ods_export_file.attached? || ods_export_file.created_at < MAX_DUREE_CONSERVATION_EXPORT.ago end def should_generate_export?(format) From ad785278af52904bafe6aaba621c921d3082a224 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Mon, 21 Oct 2019 17:38:45 +0200 Subject: [PATCH 15/17] add mailer test --- spec/mailers/instructeur_mailer_spec.rb | 14 ++++++++++++++ .../mailers/previews/instructeur_mailer_preview.rb | 4 ++++ 2 files changed, 18 insertions(+) diff --git a/spec/mailers/instructeur_mailer_spec.rb b/spec/mailers/instructeur_mailer_spec.rb index aafaf85d2..ffcda1f25 100644 --- a/spec/mailers/instructeur_mailer_spec.rb +++ b/spec/mailers/instructeur_mailer_spec.rb @@ -46,4 +46,18 @@ RSpec.describe InstructeurMailer, type: :mailer do end end end + + describe '#notify_procedure_export_available' do + let(:instructeur) { create(:instructeur) } + let(:procedure) { create(:procedure, :published, instructeurs: [instructeur]) } + let(:dossier) { create(:dossier, procedure: procedure) } + let(:format) { 'xlsx' } + + context 'when the mail is sent' do + subject { described_class.notify_procedure_export_available(instructeur, procedure, format) } + it 'contains a download link' do + expect(subject.body).to include download_export_instructeur_procedure_url(procedure, :export_format => format) + end + end + end end diff --git a/spec/mailers/previews/instructeur_mailer_preview.rb b/spec/mailers/previews/instructeur_mailer_preview.rb index b7b1c879f..0932eab6a 100644 --- a/spec/mailers/previews/instructeur_mailer_preview.rb +++ b/spec/mailers/previews/instructeur_mailer_preview.rb @@ -37,6 +37,10 @@ class InstructeurMailerPreview < ActionMailer::Preview InstructeurMailer.send_notifications(instructeur, data) end + def notify_procedure_export_available + InstructeurMailer.notify_procedure_export_available(instructeur, procedure, 'xlsx') + end + private def instructeur From 4a6893d88b4c3dc99441b91037e8aae767e4b029 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Tue, 1 Oct 2019 17:28:06 +0200 Subject: [PATCH 16/17] migrate sendinblue API to v3 --- app/lib/sendinblue/api.rb | 14 +++++++------- .../administrateur_usage_statistics_service.rb | 2 +- config/initializers/urls.rb | 1 + config/secrets.yml | 1 + 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/lib/sendinblue/api.rb b/app/lib/sendinblue/api.rb index d1939f2d4..89d9a821f 100644 --- a/app/lib/sendinblue/api.rb +++ b/app/lib/sendinblue/api.rb @@ -15,8 +15,8 @@ class Sendinblue::Api client_key.present? end - def identify(email, attributes = {}) - req = api_request('identify', email: email, attributes: attributes) + def update_contact(email, attributes = {}) + req = post_api_request('contacts', email: email, attributes: attributes, updateEnabled: true) req.on_complete do |response| if !response.success? push_failure("Error while updating identity for administrateur '#{email}' in Sendinblue: #{response.response_code} '#{response.body}'") @@ -34,7 +34,7 @@ class Sendinblue::Api private def hydra - @hydra ||= Typhoeus::Hydra.new + @hydra ||= Typhoeus::Hydra.new(max_concurrency: 50) end def push_failure(failure) @@ -49,8 +49,8 @@ class Sendinblue::Api end end - def api_request(path, body) - url = "#{SENDINBLUE_API_URL}/#{path}" + def post_api_request(path, body) + url = "#{SENDINBLUE_API_V3_URL}/#{path}" Typhoeus::Request.new( url, @@ -62,12 +62,12 @@ class Sendinblue::Api def headers { - 'ma-key': client_key, + 'api-key': client_key, 'Content-Type': 'application/json; charset=UTF-8' } end def client_key - Rails.application.secrets.sendinblue[:client_key] + Rails.application.secrets.sendinblue[:api_v3_key] end end diff --git a/app/services/administrateur_usage_statistics_service.rb b/app/services/administrateur_usage_statistics_service.rb index c197f1b7d..c283b388c 100644 --- a/app/services/administrateur_usage_statistics_service.rb +++ b/app/services/administrateur_usage_statistics_service.rb @@ -10,7 +10,7 @@ class AdministrateurUsageStatisticsService def update_administrateurs Administrateur.includes(:user).find_each do |administrateur| stats = administrateur_stats(administrateur) - api.identify(administrateur.email, stats) + api.update_contact(administrateur.email, stats) end api.run end diff --git a/config/initializers/urls.rb b/config/initializers/urls.rb index 19b442250..53aca743e 100644 --- a/config/initializers/urls.rb +++ b/config/initializers/urls.rb @@ -7,6 +7,7 @@ API_GEO_SANDBOX_URL = ENV.fetch("API_GEO_SANDBOX_URL", "https://sandbox.geo.api. HELPSCOUT_API_URL = ENV.fetch("HELPSCOUT_API_URL", "https://api.helpscout.net/v2") PIPEDRIVE_API_URL = ENV.fetch("PIPEDRIVE_API_URL", "https://api.pipedrive.com/v1") SENDINBLUE_API_URL = ENV.fetch("SENDINBLUE_API_URL", "https://in-automate.sendinblue.com/api/v2") +SENDINBLUE_API_V3_URL = ENV.fetch("SENDINBLUE_API_V3_URL", "https://api.sendinblue.com/v3") UNIVERSIGN_API_URL = ENV.fetch("UNIVERSIGN_API_URL", "https://ws.universign.eu/tsa/post/") # Internal URLs diff --git a/config/secrets.yml b/config/secrets.yml index 11001e502..7ad675ffe 100644 --- a/config/secrets.yml +++ b/config/secrets.yml @@ -55,6 +55,7 @@ defaults: &defaults sendinblue: enabled: <%= ENV['SENDINBLUE_ENABLED'] == 'enabled' %> client_key: <%= ENV['SENDINBLUE_CLIENT_KEY'] %> + api_v3_key: <%= ENV['SENDINBLUE_API_V3_KEY'] %> matomo: enabled: <%= ENV['MATOMO_ENABLED'] == 'enabled' %> client_key: <%= ENV['MATOMO_ID'] %> From 72902146249a5fdab74349f9302edf640fdc7de7 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Tue, 22 Oct 2019 12:15:49 +0200 Subject: [PATCH 17/17] uniformize export wording in UI --- .../instructeurs/procedures/_download_dossiers.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/instructeurs/procedures/_download_dossiers.html.haml b/app/views/instructeurs/procedures/_download_dossiers.html.haml index 10902baae..228c07e64 100644 --- a/app/views/instructeurs/procedures/_download_dossiers.html.haml +++ b/app/views/instructeurs/procedures/_download_dossiers.html.haml @@ -21,7 +21,7 @@ - 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 "Préparer le téléchargement de l'export au format .ods", download_export_instructeur_procedure_path(procedure, export_format: :ods), remote: true + = 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 @@ -29,7 +29,7 @@ - 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 "Préparer le téléchargement de l'export au format .csv", download_export_instructeur_procedure_path(procedure, export_format: :csv), remote: true + = 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" - if export_v1_enabled