From a2bbf1b4d605ae4ae12e846499cde4362e3ec016 Mon Sep 17 00:00:00 2001 From: Mathieu Magnin Date: Thu, 3 May 2018 18:30:40 +0200 Subject: [PATCH 1/4] [Fix #1907] A new token should be regenerated, because the one in db is encrypted --- .../administrateurs/activate_before_expiration_job.rb | 2 +- app/mailers/administrateur_mailer.rb | 5 +++-- app/models/administrateur.rb | 10 ++++++++++ .../activate_before_expiration.haml | 2 +- .../activate_before_expiration_job_spec.rb | 6 +++--- 5 files changed, 18 insertions(+), 7 deletions(-) diff --git a/app/jobs/administrateurs/activate_before_expiration_job.rb b/app/jobs/administrateurs/activate_before_expiration_job.rb index d8fd8c27a..21ee027b4 100644 --- a/app/jobs/administrateurs/activate_before_expiration_job.rb +++ b/app/jobs/administrateurs/activate_before_expiration_job.rb @@ -3,7 +3,7 @@ class Administrateurs::ActivateBeforeExpirationJob < ApplicationJob def perform(*args) Administrateur.inactive.where(created_at: 2.days.ago.all_day).each do |a| - AdministrateurMailer.activate_before_expiration(a).deliver_later + a.remind_invitation! end end end diff --git a/app/mailers/administrateur_mailer.rb b/app/mailers/administrateur_mailer.rb index 351206064..a46e46658 100644 --- a/app/mailers/administrateur_mailer.rb +++ b/app/mailers/administrateur_mailer.rb @@ -1,9 +1,10 @@ class AdministrateurMailer < ApplicationMailer layout 'mailers/layout' - def activate_before_expiration(administrateur) + def activate_before_expiration(administrateur, reset_password_token) @administrateur = administrateur - @expiration_date = administrateur.reset_password_sent_at + Devise.reset_password_within + @reset_password_token = reset_password_token + @expiration_date = @administrateur.reset_password_sent_at + Devise.reset_password_within mail(to: administrateur.email, subject: "demarches-simplifiees.fr - N'oubliez pas d'activer votre compte administrateur", reply_to: "contact@demarches-simplifiees.fr") diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index f1aec051c..22ad5db10 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -55,6 +55,16 @@ class Administrateur < ApplicationRecord reset_password_token end + def remind_invitation! + if active? + raise "Impossible d'envoyer un rappel d'invitation à un utilisateur déjà actif !" + end + + reset_password_token = set_reset_password_token + + AdministrateurMailer.activate_before_expiration(self, reset_password_token).deliver_later + end + def invitation_expired? !active && !reset_password_period_valid? end diff --git a/app/views/administrateur_mailer/activate_before_expiration.haml b/app/views/administrateur_mailer/activate_before_expiration.haml index 0fa678348..c44d0c9ff 100644 --- a/app/views/administrateur_mailer/activate_before_expiration.haml +++ b/app/views/administrateur_mailer/activate_before_expiration.haml @@ -8,7 +8,7 @@ Votre compte a été créé mais reste inactif, il arrivera à expiration le #{@ %br %br Afin d’activer votre compte, veuillez cliquer sur le lien ci-dessous : -= link_to(admin_activate_url(token: @administrateur.reset_password_token), admin_activate_url(token: @administrateur.reset_password_token)) += link_to(admin_activate_url(token: @reset_password_token), admin_activate_url(token: @reset_password_token)) %br %br Nous restons à votre disposition si vous avez besoin d’accompagnement. diff --git a/spec/jobs/administrateurs/activate_before_expiration_job_spec.rb b/spec/jobs/administrateurs/activate_before_expiration_job_spec.rb index 1a92e38bf..a3039ef39 100644 --- a/spec/jobs/administrateurs/activate_before_expiration_job_spec.rb +++ b/spec/jobs/administrateurs/activate_before_expiration_job_spec.rb @@ -32,13 +32,13 @@ RSpec.describe Administrateurs::ActivateBeforeExpirationJob, type: :job do it { expect(AdministrateurMailer).not_to have_received(:activate_before_expiration) } end - context "created 2 days ago" do + context "created 3 days ago" do before do - administrateur.update_columns(created_at: DateTime.new(2018, 03, 18, 20, 00)) + administrateur.update_columns(created_at: DateTime.new(2018, 03, 17, 20, 00)) subject end - it { expect(AdministrateurMailer).to have_received(:activate_before_expiration).with(administrateur) } + it { expect(AdministrateurMailer).to have_received(:activate_before_expiration).with(administrateur, kind_of(String)) } end end From dcdd431bc07d296d52d7b0eb2feeaf5c60221020 Mon Sep 17 00:00:00 2001 From: Mathieu Magnin Date: Thu, 3 May 2018 18:31:05 +0200 Subject: [PATCH 2/4] [Fix #1907] set activation reminder delay to 3 days to allow administrateurs to activate their account with the original message after the weekend --- app/jobs/administrateurs/activate_before_expiration_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/jobs/administrateurs/activate_before_expiration_job.rb b/app/jobs/administrateurs/activate_before_expiration_job.rb index 21ee027b4..fbf609828 100644 --- a/app/jobs/administrateurs/activate_before_expiration_job.rb +++ b/app/jobs/administrateurs/activate_before_expiration_job.rb @@ -2,7 +2,7 @@ class Administrateurs::ActivateBeforeExpirationJob < ApplicationJob queue_as :cron def perform(*args) - Administrateur.inactive.where(created_at: 2.days.ago.all_day).each do |a| + Administrateur.inactive.where(created_at: 3.days.ago.all_day).each do |a| a.remind_invitation! end end From 7f4e6eae6b20c1b0cc49036d6151ccddf7713b39 Mon Sep 17 00:00:00 2001 From: Mathieu Magnin Date: Mon, 7 May 2018 12:24:05 +0200 Subject: [PATCH 3/4] No dubious proc. email == Everything is ok || Something went wrong. We should always send dubious procedure email. --- app/jobs/find_dubious_procedures_job.rb | 4 +--- .../dubious_procedures.html.haml | 15 +++++++++------ spec/jobs/find_dubious_procedures_job_spec.rb | 8 +++++--- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/app/jobs/find_dubious_procedures_job.rb b/app/jobs/find_dubious_procedures_job.rb index 9cc4f8ba3..ecfb3b56c 100644 --- a/app/jobs/find_dubious_procedures_job.rb +++ b/app/jobs/find_dubious_procedures_job.rb @@ -24,8 +24,6 @@ class FindDubiousProceduresJob < ApplicationJob .group_by(&:procedure_id) .map { |_procedure_id, tdcs| [tdcs[0].procedure, tdcs] } - if dubious_procedures_and_tdcs.present? - AdministrationMailer.dubious_procedures(dubious_procedures_and_tdcs).deliver_now - end + AdministrationMailer.dubious_procedures(dubious_procedures_and_tdcs).deliver_now end end diff --git a/app/views/administration_mailer/dubious_procedures.html.haml b/app/views/administration_mailer/dubious_procedures.html.haml index f7ce22024..2f90561c2 100644 --- a/app/views/administration_mailer/dubious_procedures.html.haml +++ b/app/views/administration_mailer/dubious_procedures.html.haml @@ -1,8 +1,11 @@ - content_for(:title, 'Liste de procédures douteuses') -%ul - - @procedures_and_type_de_champs.each do |procedure, type_de_champs| - %li - = link_to "Nº #{procedure.id},", manager_procedure_url(procedure) -  #{procedure.libelle} : - %b= type_de_champs.map(&:libelle).join(', ') +- if @procedures_and_type_de_champs.any? + %ul + - @procedures_and_type_de_champs.each do |procedure, type_de_champs| + %li + = link_to "Nº #{procedure.id},", manager_procedure_url(procedure) +  #{procedure.libelle} : + %b= type_de_champs.map(&:libelle).join(', ') +- else + Il n'y a aucune procédure douteuse aujourd'hui diff --git a/spec/jobs/find_dubious_procedures_job_spec.rb b/spec/jobs/find_dubious_procedures_job_spec.rb index d03de6bfc..e8b354f99 100644 --- a/spec/jobs/find_dubious_procedures_job_spec.rb +++ b/spec/jobs/find_dubious_procedures_job_spec.rb @@ -30,25 +30,27 @@ RSpec.describe FindDubiousProceduresJob, type: :job do expect(receive_procedure).to eq(procedure) expect(receive_forbidden_tdcs).to match(forbidden_tdcs) + + expect(AdministrationMailer).to have_received(:dubious_procedures).with(@dubious_procedures_args) end context 'and a whitelisted procedure' do let(:procedure) { create(:procedure, whitelisted_at: DateTime.now) } - it { expect(AdministrationMailer).not_to have_received(:dubious_procedures) } + it { expect(AdministrationMailer).to have_received(:dubious_procedures).with([]) } end context 'and a archived procedure' do let(:procedure) { create(:procedure, archived_at: DateTime.now) } - it { expect(AdministrationMailer).not_to have_received(:dubious_procedures) } + it { expect(AdministrationMailer).to have_received(:dubious_procedures).with([]) } end end context 'with no suspicious champs' do let(:tdcs) { [allowed_tdc] } - it { expect(AdministrationMailer).not_to receive(:dubious_procedures) } + it { expect(AdministrationMailer).to have_received(:dubious_procedures).with([]) } end end end From 5ef129d9606adc8688883047bd3035e17808dc12 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Thu, 8 Mar 2018 17:17:18 +0100 Subject: [PATCH 4/4] [#1563] Remove attestation mail attachment --- .../new_gestionnaire/dossiers_controller.rb | 21 +----------- app/mailers/notification_mailer.rb | 6 +--- app/models/attestation.rb | 6 ---- .../dossiers_controller_spec.rb | 34 ++++--------------- spec/mailers/notification_mailer_spec.rb | 16 +-------- spec/models/attestation_spec.rb | 23 ------------- 6 files changed, 9 insertions(+), 97 deletions(-) delete mode 100644 spec/models/attestation_spec.rb diff --git a/app/controllers/new_gestionnaire/dossiers_controller.rb b/app/controllers/new_gestionnaire/dossiers_controller.rb index 412bb4f29..f3b7928a0 100644 --- a/app/controllers/new_gestionnaire/dossiers_controller.rb +++ b/app/controllers/new_gestionnaire/dossiers_controller.rb @@ -106,18 +106,9 @@ module NewGestionnaire dossier.save - # needed to force Carrierwave to provide dossier.attestation.pdf.read - # when the Flipflop.remote_storage? is true, otherwise pdf.read is a closed stream. - dossier.reload - - attestation_pdf = nil - if check_attestation_emailable - attestation_pdf = dossier.attestation.pdf.read - end - flash.notice = notice - NotificationMailer.send_notification(dossier, template, attestation_pdf).deliver_now! + NotificationMailer.send_notification(dossier, template).deliver_now! redirect_to gestionnaire_dossier_path(procedure, dossier) end @@ -204,16 +195,6 @@ module NewGestionnaire ]) end - def check_attestation_emailable - if dossier&.attestation&.emailable? == false - human_size = number_to_human_size(dossier.attestation.pdf.size) - msg = "the attestation of the dossier #{dossier.id} cannot be mailed because it is too heavy: #{human_size}" - capture_message(msg, level: 'error') - end - - dossier&.attestation&.emailable? - end - def mark_demande_as_read current_gestionnaire.mark_tab_as_seen(dossier, :demande) end diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb index ae1c65cd9..5119693a1 100644 --- a/app/mailers/notification_mailer.rb +++ b/app/mailers/notification_mailer.rb @@ -8,16 +8,12 @@ class NotificationMailer < ApplicationMailer send_notification(dossier, dossier.procedure.received_mail_template) end - def send_notification(dossier, mail_template, attestation = nil) + def send_notification(dossier, mail_template) vars_mailer(dossier) @subject = mail_template.subject_for_dossier dossier @body = mail_template.body_for_dossier dossier - if attestation.present? - attachments['attestation.pdf'] = attestation - end - mail(subject: @subject) { |format| format.html { @body } } end diff --git a/app/models/attestation.rb b/app/models/attestation.rb index 220e2c364..605ed11a4 100644 --- a/app/models/attestation.rb +++ b/app/models/attestation.rb @@ -2,10 +2,4 @@ class Attestation < ApplicationRecord belongs_to :dossier mount_uploader :pdf, AttestationUploader - - MAX_SIZE_EMAILABLE = 2.megabytes - - def emailable? - pdf.size <= MAX_SIZE_EMAILABLE - end end diff --git a/spec/controllers/new_gestionnaire/dossiers_controller_spec.rb b/spec/controllers/new_gestionnaire/dossiers_controller_spec.rb index 5b4a5b7ce..e23bfa5f8 100644 --- a/spec/controllers/new_gestionnaire/dossiers_controller_spec.rb +++ b/spec/controllers/new_gestionnaire/dossiers_controller_spec.rb @@ -152,7 +152,7 @@ describe NewGestionnaire::DossiersController, type: :controller do it 'Notification email is sent' do expect(NotificationMailer).to receive(:send_notification) - .with(dossier, kind_of(Mails::RefusedMail), nil).and_return(NotificationMailer) + .with(dossier, kind_of(Mails::RefusedMail)).and_return(NotificationMailer) expect(NotificationMailer).to receive(:deliver_now!) subject @@ -178,7 +178,7 @@ describe NewGestionnaire::DossiersController, type: :controller do it 'Notification email is sent' do expect(NotificationMailer).to receive(:send_notification) - .with(dossier, kind_of(Mails::WithoutContinuationMail), nil).and_return(NotificationMailer) + .with(dossier, kind_of(Mails::WithoutContinuationMail)).and_return(NotificationMailer) expect(NotificationMailer).to receive(:deliver_now!) subject @@ -188,14 +188,12 @@ describe NewGestionnaire::DossiersController, type: :controller do end context "with accepter" do - let(:expected_attestation) { nil } - before do dossier.en_instruction! sign_in gestionnaire expect(NotificationMailer).to receive(:send_notification) - .with(dossier, kind_of(Mails::ClosedMail), expected_attestation) + .with(dossier, kind_of(Mails::ClosedMail)) .and_return(NotificationMailer) expect(NotificationMailer).to receive(:deliver_now!) @@ -220,34 +218,14 @@ describe NewGestionnaire::DossiersController, type: :controller do before do attestation = Attestation.new allow(attestation).to receive(:pdf).and_return(double(read: 'pdf', size: 2.megabytes)) - allow(attestation).to receive(:emailable?).and_return(emailable) - expect_any_instance_of(Dossier).to receive(:reload) allow_any_instance_of(Dossier).to receive(:build_attestation).and_return(attestation) end - context 'emailable' do - let(:emailable) { true } - let(:expected_attestation) { 'pdf' } + it 'The gestionnaire is sent back to the dossier page' do + subject - it 'Notification email is sent with the attestation' do - subject - - is_expected.to redirect_to redirect_to gestionnaire_dossier_path(procedure, dossier) - end - end - - context 'when the dossier has an attestation not emailable' do - let(:emailable) { false } - let(:expected_attestation) { nil } - - it 'Notification email is sent without the attestation' do - expect(controller).to receive(:capture_message) - - subject - - is_expected.to redirect_to redirect_to gestionnaire_dossier_path(procedure, dossier) - end + is_expected.to redirect_to redirect_to gestionnaire_dossier_path(procedure, dossier) end end diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index 059e219c3..d6d1dd1dd 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -17,25 +17,11 @@ RSpec.describe NotificationMailer, type: :mailer do describe '.send_notification' do let(:email_template) { instance_double('email_template', subject_for_dossier: 'subject', body_for_dossier: 'body') } - let(:attestation) { nil } - subject { described_class.send_notification(dossier, email_template, attestation) } + subject { described_class.send_notification(dossier, email_template) } it { expect(subject.subject).to eq(email_template.subject_for_dossier) } it { expect(subject.body).to eq(email_template.body_for_dossier) } - it { expect(subject.attachments['attestation.pdf']).to eq(nil) } - - context 'when an attestation is provided' do - let(:attestation) { 'attestation' } - - it do - expect(subject.attachments['attestation.pdf'].content_type) - .to eq('application/pdf; filename=attestation.pdf') - - expect(subject.attachments['attestation.pdf'].body).to eq('attestation') - end - end - it_behaves_like "create a commentaire not notified" end diff --git a/spec/models/attestation_spec.rb b/spec/models/attestation_spec.rb deleted file mode 100644 index 5bf9e89f5..000000000 --- a/spec/models/attestation_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -RSpec.describe Attestation, type: :model do - describe 'emailable' do - let(:attestation) do - attestation = Attestation.new - expect(attestation).to receive(:pdf).and_return(double(size: size)) - attestation - end - - subject { attestation.emailable? } - - context 'when the pdf size is acceptable' do - let(:size) { Attestation::MAX_SIZE_EMAILABLE } - - it { is_expected.to be true } - end - - context 'when the pdf size is unacceptable' do - let(:size) { Attestation::MAX_SIZE_EMAILABLE + 1 } - - it { is_expected.to be false } - end - end -end