From ba69edffc5dae74938d128f723fc4bafea026826 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 28 Apr 2021 12:54:09 +0200 Subject: [PATCH 01/11] add conservation extension to dossiers --- .../20210428104228_add_conservation_extension_to_dossiers.rb | 5 +++++ db/schema.rb | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20210428104228_add_conservation_extension_to_dossiers.rb diff --git a/db/migrate/20210428104228_add_conservation_extension_to_dossiers.rb b/db/migrate/20210428104228_add_conservation_extension_to_dossiers.rb new file mode 100644 index 000000000..909ac22f0 --- /dev/null +++ b/db/migrate/20210428104228_add_conservation_extension_to_dossiers.rb @@ -0,0 +1,5 @@ +class AddConservationExtensionToDossiers < ActiveRecord::Migration[6.1] + def change + add_column :dossiers, :conservation_extension, :interval, default: 0.days + end +end diff --git a/db/schema.rb b/db/schema.rb index eefe0112f..44108f050 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_04_27_124500) do +ActiveRecord::Schema.define(version: 2021_04_28_104228) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -274,6 +274,7 @@ ActiveRecord::Schema.define(version: 2021_04_27_124500) do t.datetime "last_avis_updated_at" t.datetime "last_commentaire_updated_at" t.string "api_entreprise_job_exceptions", array: true + t.interval "conservation_extension", default: "PT0S" t.index "to_tsvector('french'::regconfig, (search_terms || private_search_terms))", name: "index_dossiers_on_search_terms_private_search_terms", using: :gin t.index "to_tsvector('french'::regconfig, search_terms)", name: "index_dossiers_on_search_terms", using: :gin t.index ["archived"], name: "index_dossiers_on_archived" From b2a867266a522a12b639cca0f02b4ffdba2b9c93 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 28 Apr 2021 12:50:58 +0200 Subject: [PATCH 02/11] Allow users to extend conservation on drafts --- app/controllers/users/dossiers_controller.rb | 2 +- app/models/dossier.rb | 24 ++++++++++++++----- .../users/dossiers/show/_header.html.haml | 16 +++++++++---- .../notify_brouillon_near_deletion/fr.yml | 4 ++-- spec/models/dossier_spec.rb | 11 ++++++++- 5 files changed, 42 insertions(+), 15 deletions(-) diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index f03d28f19..cdc030978 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -161,7 +161,7 @@ module Users end def extend_conservation - dossier.update(en_construction_conservation_extension: dossier.en_construction_conservation_extension + 1.month) + dossier.update(conservation_extension: dossier.conservation_extension + 1.month) flash[:notice] = 'Votre dossier sera conservé un mois supplémentaire' redirect_to dossier_path(@dossier) end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 8b8a1e60e..f8023d563 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -7,6 +7,7 @@ # archived :boolean default(FALSE) # autorisation_donnees :boolean # brouillon_close_to_expiration_notice_sent_at :datetime +# conservation_extension :interval default(0 seconds) # en_construction_at :datetime # en_construction_close_to_expiration_notice_sent_at :datetime # en_construction_conservation_extension :interval default(0 seconds) @@ -253,21 +254,22 @@ class Dossier < ApplicationRecord scope :brouillon_close_to_expiration, -> do state_brouillon .joins(:procedure) - .where("dossiers.created_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + .where("dossiers.created_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) end scope :en_construction_close_to_expiration, -> do state_en_construction .joins(:procedure) - .where("dossiers.en_construction_at + dossiers.en_construction_conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + .where("dossiers.en_construction_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) end scope :en_instruction_close_to_expiration, -> do state_en_instruction .joins(:procedure) .where("dossiers.en_instruction_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) end - def self.termine_close_to_expiration - dossier_ids = Traitement.termine_close_to_expiration.pluck(:dossier_id).uniq - Dossier.where(id: dossier_ids) + scope :termine_close_to_expiration, -> do + state_termine + .joins(:procedure) + .where(id: Traitement.termine_close_to_expiration.pluck(:dossier_id).uniq) end scope :brouillon_expired, -> do @@ -431,7 +433,15 @@ class Dossier < ApplicationRecord end def en_construction_close_to_expiration? - Dossier.en_construction_close_to_expiration.where(id: self).present? + self.class.en_construction_close_to_expiration.exists?(id: self) + end + + def brouillon_close_to_expiration? + self.class.brouillon_close_to_expiration.exists?(id: self) + end + + def close_to_expiration? + en_construction_close_to_expiration? || brouillon_close_to_expiration? end def show_groupe_instructeur_details? @@ -609,6 +619,7 @@ class Dossier < ApplicationRecord end def after_passer_en_construction + update!(conservation_extension: 0.days) update!(en_construction_at: Time.zone.now) if self.en_construction_at.nil? end @@ -625,6 +636,7 @@ class Dossier < ApplicationRecord end def after_repasser_en_construction(instructeur) + update!(conservation_extension: 0.days) log_dossier_operation(instructeur, :repasser_en_construction) end diff --git a/app/views/users/dossiers/show/_header.html.haml b/app/views/users/dossiers/show/_header.html.haml index 905f34f42..ceed29f78 100644 --- a/app/views/users/dossiers/show/_header.html.haml +++ b/app/views/users/dossiers/show/_header.html.haml @@ -22,13 +22,19 @@ %li = link_to "Tout le dossier", dossier_path(dossier, format: :pdf), target: "_blank", rel: "noopener", class: "menu-item menu-link" - - if dossier.en_construction_close_to_expiration? + - if dossier.close_to_expiration? .card.warning .card-title Votre dossier va expirer - %p - Votre dossier a été déposé, mais va bientôt expirer. Cela signifie qu'il va bientôt être supprimé sans avoir été traité par l’administration. - Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver - un mois de plus en cliquant sur le bouton ci-dessous. + - if dossier.brouillon? + %p + Votre dossier est en brouillon, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été déposé. + Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver + un mois de plus en cliquant sur le bouton ci-dessous. + - else + %p + Votre dossier a été déposé, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été traité par l’administration. + Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver + un mois de plus en cliquant sur le bouton ci-dessous. %br = button_to 'Repousser sa suppression', users_dossier_repousser_expiration_path(dossier), class: 'button secondary' diff --git a/config/locales/views/dossier_mailer/notify_brouillon_near_deletion/fr.yml b/config/locales/views/dossier_mailer/notify_brouillon_near_deletion/fr.yml index f4ccef4e0..883a7130a 100644 --- a/config/locales/views/dossier_mailer/notify_brouillon_near_deletion/fr.yml +++ b/config/locales/views/dossier_mailer/notify_brouillon_near_deletion/fr.yml @@ -8,5 +8,5 @@ fr: one: "Afin de limiter la conservation de vos données personnelles, le dossier en brouillon suivant sera bientôt automatiquement supprimé :" other: "Afin de limiter la conservation de vos données personnelles, les dossiers en brouillon suivant seront bientôt automatiquement supprimés :" footer: - one: "Si vous souhaitez toujours déposer ce dossier, vous pouvez retrouver votre brouillon pendant encore un mois. Et sinon, vous n’avez rien à faire." - other: "Si vous souhaitez toujours déposer ces dossiers, vous pouvez retrouver vos brouillons pendant encore un mois. Et sinon, vous n’avez rien à faire." + one: "Si vous souhaitez toujours déposer ce dossier, vous pouvez retrouver votre brouillon pendant encore un mois. Si vous souhaitez conserver votre dossier plus longtemps, vous pouvez prolonger sa durée de conservation dans l'interface. Et sinon, vous n’avez rien à faire." + other: "Si vous souhaitez toujours déposer ces dossiers, vous pouvez retrouver vos brouillons pendant encore un mois. Si vous souhaitez conserver vos dossiers plus longtemps, vous pouvez prolonger leur durée de conservation au cas par cas dans l'interface. Et sinon, vous n’avez rien à faire." diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 4386a504a..310a372b1 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -52,6 +52,15 @@ describe Dossier do is_expected.to include(just_expired_dossier) is_expected.to include(long_expired_dossier) end + + context 'does not include an expiring dossier that has been postponed' do + before do + expiring_dossier.update(conservation_extension: 1.month) + expiring_dossier.reload + end + + it { is_expected.not_to include(expiring_dossier) } + end end describe 'en_construction_close_to_expiration' do @@ -72,7 +81,7 @@ describe Dossier do context 'does not include an expiring dossier that has been postponed' do before do - expiring_dossier.update(en_construction_conservation_extension: 1.month) + expiring_dossier.update(conservation_extension: 1.month) expiring_dossier.reload end From 88db6fb6611758cfdd819dac2196de9b270f3d8c Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 29 Apr 2021 19:38:16 +0200 Subject: [PATCH 03/11] rename en_construction_conservation_extension in conservation_extension --- ...0429172327_rename_conservation_extension.rake | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 lib/tasks/deployment/20210429172327_rename_conservation_extension.rake diff --git a/lib/tasks/deployment/20210429172327_rename_conservation_extension.rake b/lib/tasks/deployment/20210429172327_rename_conservation_extension.rake new file mode 100644 index 000000000..16d83125b --- /dev/null +++ b/lib/tasks/deployment/20210429172327_rename_conservation_extension.rake @@ -0,0 +1,16 @@ +namespace :after_party do + desc 'Deployment task: rename_conservation_extension' + task rename_conservation_extension: :environment do + puts "Running deploy task 'rename_conservation_extension'" + + dossiers = Dossier.state_en_construction.where.not(en_construction_conservation_extension: 0.days) + dossiers.find_each do |dossier| + dossier.update_column(:conservation_extension, dossier.en_construction_conservation_extension) + end + + # Update task as completed. If you remove the line below, the task will + # run with every deploy (or every time you call after_party:run). + AfterParty::TaskRecord + .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp + end +end From ea087af5d92ca853f913481021c4cda170d6711d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 30 Apr 2021 20:30:05 +0000 Subject: [PATCH 04/11] Bump rexml from 3.2.4 to 3.2.5 Bumps [rexml](https://github.com/ruby/rexml) from 3.2.4 to 3.2.5. - [Release notes](https://github.com/ruby/rexml/releases) - [Changelog](https://github.com/ruby/rexml/blob/master/NEWS.md) - [Commits](https://github.com/ruby/rexml/compare/v3.2.4...v3.2.5) Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 3256d54dc..43e8b9b5e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -556,7 +556,7 @@ GEM http-cookie (>= 1.0.2, < 2.0) mime-types (>= 1.16, < 4.0) netrc (~> 0.8) - rexml (3.2.4) + rexml (3.2.5) rgeo (2.2.0) rgeo-geojson (2.1.1) rgeo (>= 1.0.0) From cb717aede235ce1b7b49f7417ec198fe2b4c4ccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Vantomme?= Date: Wed, 14 Apr 2021 21:56:36 +0200 Subject: [PATCH 05/11] Fix (GraphQL): raise exception when blob is missing Failures: 1) API::V2::GraphqlController when authenticated mutations dossierEnvoyerMessage upload error should fail Failure/Error: expect(gql_errors).to eq(nil) expected: nil got: [{:backtrace=>["/usr/local/bundle/ruby/2.7.0/gems/graphql-1.12.5/lib/graphql/backtrace/tracer.rb:64:i...] --- app/graphql/mutations/base_mutation.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/graphql/mutations/base_mutation.rb b/app/graphql/mutations/base_mutation.rb index aea9ca2a8..652046a06 100644 --- a/app/graphql/mutations/base_mutation.rb +++ b/app/graphql/mutations/base_mutation.rb @@ -5,6 +5,8 @@ module Mutations def validate_blob(blob_id) begin blob = ActiveStorage::Blob.find_signed(blob_id) + raise ActiveSupport::MessageVerifier::InvalidSignature if blob.nil? + # open downloads the file and checks its hash blob.open { |f| } true From 13d2364fd5b1f9108c9b7e049eb23c5825dfae62 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 4 May 2021 08:57:33 +0200 Subject: [PATCH 06/11] jobs: add Excon::Error::Socket to the list of transient errors In ActiveStorage::Purge job we see a lot of SSL errors: > Connection reset by peer - SSL_connect (Errno::ECONNRESET) These errors seem transient, and resolve themselves after a while. --- app/lib/active_job/retry_on_transient_errors.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/lib/active_job/retry_on_transient_errors.rb b/app/lib/active_job/retry_on_transient_errors.rb index b3790359c..cf714174a 100644 --- a/app/lib/active_job/retry_on_transient_errors.rb +++ b/app/lib/active_job/retry_on_transient_errors.rb @@ -4,7 +4,8 @@ module ActiveJob::RetryOnTransientErrors TRANSIENT_ERRORS = [ Excon::Error::InternalServerError, Excon::Error::GatewayTimeout, - Excon::Error::BadRequest + Excon::Error::BadRequest, + Excon::Error::Socket ] included do From e240dd6d2ced9c6d2a609db81db7e63476972cf2 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 4 May 2021 10:26:31 +0200 Subject: [PATCH 07/11] remove NewAttestationMailer --- app/mailers/new_attestation_mailer.rb | 32 --------------------------- 1 file changed, 32 deletions(-) delete mode 100644 app/mailers/new_attestation_mailer.rb diff --git a/app/mailers/new_attestation_mailer.rb b/app/mailers/new_attestation_mailer.rb deleted file mode 100644 index 20c645b2b..000000000 --- a/app/mailers/new_attestation_mailer.rb +++ /dev/null @@ -1,32 +0,0 @@ -# Preview all emails at http://localhost:3000/rails/mailers/new_attestation_mailer -class NewAttestationMailer < ApplicationMailer - include Rails.application.routes.url_helpers - - def new_attestation(dossier) - to = dossier.user.email - subject = "Nouvelle attestation pour votre dossier nº #{dossier.id}" - - mail(to: to, subject: subject, body: body(dossier)) - end - - private - - def body(dossier) - <<~HEREDOC - Bonjour, - - Votre dossier nº #{dossier.id} (démarche "#{dossier.procedure.libelle}") a subi, à un moment, un "aller-retour" : - - Acceptation de votre dossier - - Passage en instruction du dossier car besoin de le modifier - - Seconde acceptation de votre dossier - - Suite à cette opération, l'attestation liée à votre dossier n'a pas été regénérée. - Ce problème est désormais reglé, votre nouvelle attestation est disponible à l'adresse suivante : - #{attestation_dossier_url(dossier)} - - Cordialement, - - L’équipe #{APPLICATION_NAME} - HEREDOC - end -end From a4fd629f4ae145a785bd9c5613e1f17cde44622e Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Sat, 1 May 2021 12:20:24 +0200 Subject: [PATCH 08/11] Enable user destruction --- .../manager/procedures_controller.rb | 2 +- app/graphql/types/dossier_type.rb | 6 +- app/mailers/dossier_mailer.rb | 8 +-- app/mailers/notification_mailer.rb | 8 ++- app/models/dossier.rb | 41 +++++++++--- app/models/user.rb | 14 +++-- app/views/dossiers/show.pdf.prawn | 4 +- app/views/experts/avis/procedure.html.haml | 2 +- .../instructeurs/avis/procedure.html.haml | 2 +- .../dossiers/_state_button.html.haml | 29 +++++---- .../instructeurs/recherche/index.html.haml | 2 +- ...leted_user_email_never_send_to_dossiers.rb | 5 ++ db/schema.rb | 3 +- spec/models/user_spec.rb | 63 ++++++++++--------- 14 files changed, 126 insertions(+), 63 deletions(-) create mode 100644 db/migrate/20210419100831_add_deleted_user_email_never_send_to_dossiers.rb diff --git a/app/controllers/manager/procedures_controller.rb b/app/controllers/manager/procedures_controller.rb index dd0dcf0f1..9ca75b503 100644 --- a/app/controllers/manager/procedures_controller.rb +++ b/app/controllers/manager/procedures_controller.rb @@ -41,7 +41,7 @@ module Manager def export_mail_brouillons dossiers = procedure.dossiers.state_brouillon.includes(:user) - emails = dossiers.map { |d| d.user.email }.sort.uniq + emails = dossiers.map { |dossier| dossier.user_email_for(:display) }.sort.uniq date = Time.zone.now.strftime('%d-%m-%Y') send_data(emails.join("\n"), :filename => "brouillons-#{procedure.id}-au-#{date}.csv") end diff --git a/app/graphql/types/dossier_type.rb b/app/graphql/types/dossier_type.rb index 7f340ffc3..57811a75e 100644 --- a/app/graphql/types/dossier_type.rb +++ b/app/graphql/types/dossier_type.rb @@ -54,7 +54,11 @@ module Types end def usager - Loaders::Record.for(User).load(object.user_id) + if object.user_deleted? + { email: object.user_email_for(:display), id: -1 } + else + Loaders::Record.for(User).load(object.user_id) + end end def groupe_instructeur diff --git a/app/mailers/dossier_mailer.rb b/app/mailers/dossier_mailer.rb index 285802797..29e035ffc 100644 --- a/app/mailers/dossier_mailer.rb +++ b/app/mailers/dossier_mailer.rb @@ -13,7 +13,7 @@ class DossierMailer < ApplicationMailer subject = "Retrouvez votre brouillon pour la démarche « #{dossier.procedure.libelle} »" - mail(from: NO_REPLY_EMAIL, to: dossier.user.email, subject: subject) do |format| + mail(from: NO_REPLY_EMAIL, to: dossier.user_email_for(:notification), subject: subject) do |format| format.html { render layout: 'mailers/notifications_layout' } end end @@ -25,7 +25,7 @@ class DossierMailer < ApplicationMailer subject = "Nouveau message pour votre dossier nº #{dossier.id} (#{dossier.procedure.libelle})" - mail(from: NO_REPLY_EMAIL, to: dossier.user.email, subject: subject) do |format| + mail(from: NO_REPLY_EMAIL, to: dossier.user_email_for(:notification), subject: subject) do |format| format.html { render layout: 'mailers/notifications_layout' } end end @@ -49,7 +49,7 @@ class DossierMailer < ApplicationMailer subject = "Votre dossier nº #{@dossier.id} est en train d'être réexaminé" - mail(from: NO_REPLY_EMAIL, to: dossier.user.email, subject: subject) do |format| + mail(from: NO_REPLY_EMAIL, to: dossier.user_email_for(:notification), subject: subject) do |format| format.html { render layout: 'mailers/notifications_layout' } end end @@ -139,7 +139,7 @@ class DossierMailer < ApplicationMailer @subject = "Attention : votre dossier n'est pas déposé." @dossier = dossier - mail(to: dossier.user.email, subject: @subject) + mail(to: dossier.user_email_for(:notification), subject: @subject) end protected diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb index 829c7ce47..d958d781b 100644 --- a/app/mailers/notification_mailer.rb +++ b/app/mailers/notification_mailer.rb @@ -8,6 +8,8 @@ class NotificationMailer < ApplicationMailer include ActionView::Helpers::SanitizeHelper + before_action :prevent_delivery_to_deleted_users + helper ServiceHelper helper MailerHelper @@ -36,8 +38,12 @@ class NotificationMailer < ApplicationMailer private + def prevent_delivery_to_deleted_users + !@dossier.user_deleted? + end + def send_notification(dossier, mail_template) - email = dossier.user.email + email = dossier.user_email_for(:notification) subject = mail_template.subject_for_dossier(dossier) body = mail_template.body_for_dossier(dossier) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index f8023d563..53ec3ff38 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -8,6 +8,7 @@ # autorisation_donnees :boolean # brouillon_close_to_expiration_notice_sent_at :datetime # conservation_extension :interval default(0 seconds) +# deleted_user_email_never_send :string # en_construction_at :datetime # en_construction_close_to_expiration_notice_sent_at :datetime # en_construction_conservation_extension :interval default(0 seconds) @@ -137,9 +138,9 @@ class Dossier < ApplicationRecord end event :repasser_en_instruction, after: :after_repasser_en_instruction do - transitions from: :refuse, to: :en_instruction - transitions from: :sans_suite, to: :en_instruction - transitions from: :accepte, to: :en_instruction + transitions from: :refuse, to: :en_instruction, guard: :can_repasser_en_instruction? + transitions from: :sans_suite, to: :en_instruction, guard: :can_repasser_en_instruction? + transitions from: :accepte, to: :en_instruction, guard: :can_repasser_en_instruction? end end @@ -249,6 +250,7 @@ class Dossier < ApplicationRecord states = opts[:notify_on_closed] ? [:publiee, :close, :depubliee] : [:publiee, :depubliee] joins(:procedure) .where(procedures: { aasm_state: states }) + .where.not(user_id: nil) end scope :brouillon_close_to_expiration, -> do @@ -350,6 +352,22 @@ class Dossier < ApplicationRecord validates :individual, presence: true, if: -> { revision.procedure.for_individual? } validates :groupe_instructeur, presence: true, if: -> { !brouillon? } + def user_deleted? + user_id.nil? + end + + def user_email_for(use) + if user_deleted? + if use == :display + deleted_user_email_never_send + else + raise "Can not send email to discarded user" + end + else + user.email + end + end + def motivation return nil if !termine? traitements.any? ? traitements.last.motivation : read_attribute(:motivation) @@ -416,6 +434,10 @@ class Dossier < ApplicationRecord brouillon? && procedure.dossier_can_transition_to_en_construction? end + def can_repasser_en_instruction? + termine? && !user_deleted? + end + def can_be_updated_by_user? brouillon? || en_construction? end @@ -429,7 +451,7 @@ class Dossier < ApplicationRecord end def messagerie_available? - !brouillon? && !archived + !brouillon? && !user_deleted? && !archived end def en_construction_close_to_expiration? @@ -590,13 +612,18 @@ class Dossier < ApplicationRecord administration_emails.each do |email| DossierMailer.notify_deletion_to_administration(deleted_dossier, email).deliver_later end - DossierMailer.notify_deletion_to_user(deleted_dossier, user.email).deliver_later + + if !user_deleted? + DossierMailer.notify_deletion_to_user(deleted_dossier, user_email_for(:notification)).deliver_later + end log_dossier_operation(author, :supprimer, self) elsif termine? deleted_dossier = DeletedDossier.create_from_dossier(self, reason) - DossierMailer.notify_instructeur_deletion_to_user(deleted_dossier, user.email).deliver_later + if !user_deleted? + DossierMailer.notify_instructeur_deletion_to_user(deleted_dossier, user_email_for(:notification)).deliver_later + end log_dossier_operation(author, :supprimer, self) end @@ -751,7 +778,7 @@ class Dossier < ApplicationRecord def spreadsheet_columns(with_etablissement: false, types_de_champ:, types_de_champ_private:) columns = [ ['ID', id.to_s], - ['Email', user.email] + ['Email', user_email_for(:display)] ] if procedure.for_individual? diff --git a/app/models/user.rb b/app/models/user.rb index f9f6fa372..1e40adb85 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -170,18 +170,24 @@ class User < ApplicationRecord end def can_be_deleted? - !administrateur? && !instructeur? && !expert? && dossiers.with_discarded.state_instruction_commencee.empty? + !administrateur? && !instructeur? && !expert? end def delete_and_keep_track_dossiers(administration) if !can_be_deleted? - raise "Cannot delete this user because instruction has started for some dossiers" + raise "Cannot delete this user because they are also instructeur, expert or administrateur" end - dossiers.each do |dossier| + Invite.where(dossier: dossiers.with_discarded).destroy_all + dossiers.state_en_construction.each do |dossier| dossier.discard_and_keep_track!(administration, :user_removed) end - dossiers.with_discarded.destroy_all + DossierOperationLog + .where(dossier: dossiers.with_discarded.discarded) + .where.not(operation: DossierOperationLog.operations.fetch(:supprimer)) + .destroy_all + dossiers.with_discarded.discarded.destroy_all + dossiers.update_all(deleted_user_email_never_send: email, user_id: nil) destroy! end diff --git a/app/views/dossiers/show.pdf.prawn b/app/views/dossiers/show.pdf.prawn index da28de24d..087502214 100644 --- a/app/views/dossiers/show.pdf.prawn +++ b/app/views/dossiers/show.pdf.prawn @@ -174,7 +174,7 @@ def add_message(pdf, message) if message.sent_by_system? sender = 'Email automatique' elsif message.sent_by?(@dossier.user) - sender = @dossier.user.email + sender = @dossier.user_email_for(:display) end format_in_2_lines(pdf, "#{sender}, #{try_format_date(message.created_at)}", @@ -233,7 +233,7 @@ prawn_document(page_size: "A4") do |pdf| format_in_2_columns(pdf, 'Informations France Connect', "Le dossier a été déposé par le compte de #{@dossier.france_connect_information.given_name} #{@dossier.france_connect_information.family_name}, authentifié par France Connect le #{@dossier.france_connect_information.updated_at.strftime('%d/%m/%Y')}") end - format_in_2_columns(pdf, "Email", @dossier.user.email) + format_in_2_columns(pdf, "Email", @dossier.user_email_for(:display)) if @dossier.individual.present? add_identite_individual(pdf, @dossier.individual) diff --git a/app/views/experts/avis/procedure.html.haml b/app/views/experts/avis/procedure.html.haml index fc79795d2..8abe0f3d4 100644 --- a/app/views/experts/avis/procedure.html.haml +++ b/app/views/experts/avis/procedure.html.haml @@ -38,7 +38,7 @@ = link_to(instructeur_avis_path(avis.procedure, avis), class: 'cell-link') do %span.icon.folder #{avis.dossier.id} - %td= link_to(avis.dossier.user.email, instructeur_avis_path(avis.procedure, avis), class: 'cell-link') + %td= link_to(avis.dossier.user_email_for(:display), instructeur_avis_path(avis.procedure, avis), class: 'cell-link') %td= link_to(avis.procedure.libelle, instructeur_avis_path(avis.procedure, avis), class: 'cell-link') = paginate(@avis) - else diff --git a/app/views/instructeurs/avis/procedure.html.haml b/app/views/instructeurs/avis/procedure.html.haml index f38d6cde3..1505eef58 100644 --- a/app/views/instructeurs/avis/procedure.html.haml +++ b/app/views/instructeurs/avis/procedure.html.haml @@ -38,7 +38,7 @@ = link_to(instructeur_avis_path(avis.procedure, avis), class: 'cell-link') do %span.icon.folder #{avis.dossier.id} - %td= link_to(avis.dossier.user.email, instructeur_avis_path(avis.procedure, avis), class: 'cell-link') + %td= link_to(avis.dossier.user_email_for(:display), instructeur_avis_path(avis.procedure, avis), class: 'cell-link') %td= link_to(avis.procedure.libelle, instructeur_avis_path(avis.procedure, avis), class: 'cell-link') = paginate(@avis) - else diff --git a/app/views/instructeurs/dossiers/_state_button.html.haml b/app/views/instructeurs/dossiers/_state_button.html.haml index 2b4d5851c..3d7b8e7bc 100644 --- a/app/views/instructeurs/dossiers/_state_button.html.haml +++ b/app/views/instructeurs/dossiers/_state_button.html.haml @@ -99,17 +99,24 @@ %h4 Voir l’attestation %p Cette attestation a été envoyée automatiquement au demandeur. - %li - = link_to repasser_en_instruction_instructeur_dossier_path(dossier.procedure, dossier), method: :post, data: { remote:true, confirm: "Voulez vous remettre le dossier #{dossier.id} en instruction ?" } do - %span.icon.in-progress - .dropdown-description - %h4 Repasser en instruction - L’usager sera notifié que son dossier est réexaminé. - - if dossier.termine? + - if dossier.can_repasser_en_instruction? %li - = link_to supprimer_dossier_instructeur_dossier_path(dossier.procedure, dossier), method: :patch, data: { confirm: "Voulez vous vraiment supprimer le dossier #{dossier.id} ? Cette action est irréversible. \nNous vous suggérons de télécharger le dossier au format PDF au préalable." } do - %span.icon.delete + = link_to repasser_en_instruction_instructeur_dossier_path(dossier.procedure, dossier), method: :post, data: { remote:true, confirm: "Voulez vous remettre le dossier #{dossier.id} en instruction ?" } do + %span.icon.in-progress .dropdown-description - %h4 Supprimer le dossier - L’usager sera notifié que son dossier est supprimé. + %h4 Repasser en instruction + L’usager sera notifié que son dossier est réexaminé. + - elsif dossier.user_deleted? + %li + %span.icon.info + .dropdown-description + %h4 En attente d‘archivage + L'usager a supprimé son compte. Vous pouvez archiver puis supprimer le dossier. + + %li + = link_to supprimer_dossier_instructeur_dossier_path(dossier.procedure, dossier), method: :patch, data: { confirm: "Voulez vous vraiment supprimer le dossier #{dossier.id} ? Cette action est irréversible. \nNous vous suggérons de télécharger le dossier au format PDF au préalable." } do + %span.icon.delete + .dropdown-description + %h4 Supprimer le dossier + L’usager sera notifié que son dossier est supprimé. diff --git a/app/views/instructeurs/recherche/index.html.haml b/app/views/instructeurs/recherche/index.html.haml index 7c7c3ce89..b08a849b5 100644 --- a/app/views/instructeurs/recherche/index.html.haml +++ b/app/views/instructeurs/recherche/index.html.haml @@ -27,7 +27,7 @@ = link_to(dossier_linked_path(current_instructeur, dossier), class: 'cell-link') do = dossier.id %td= link_to(dossier.procedure.libelle, dossier_linked_path(current_instructeur, dossier), class: 'cell-link') - %td= link_to(dossier.user.email, dossier_linked_path(current_instructeur, dossier), class: 'cell-link') + %td= link_to(dossier.user_email_for(:display), dossier_linked_path(current_instructeur, dossier), class: 'cell-link') %td.status-col = link_to(dossier_linked_path(current_instructeur, dossier), class: 'cell-link') do = status_badge(dossier.state) diff --git a/db/migrate/20210419100831_add_deleted_user_email_never_send_to_dossiers.rb b/db/migrate/20210419100831_add_deleted_user_email_never_send_to_dossiers.rb new file mode 100644 index 000000000..8238f3e9c --- /dev/null +++ b/db/migrate/20210419100831_add_deleted_user_email_never_send_to_dossiers.rb @@ -0,0 +1,5 @@ +class AddDeletedUserEmailNeverSendToDossiers < ActiveRecord::Migration[6.1] + def change + add_column :dossiers, :deleted_user_email_never_send, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 44108f050..09d1a2239 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_04_28_104228) do +ActiveRecord::Schema.define(version: 2021_04_28_114228) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -275,6 +275,7 @@ ActiveRecord::Schema.define(version: 2021_04_28_104228) do t.datetime "last_commentaire_updated_at" t.string "api_entreprise_job_exceptions", array: true t.interval "conservation_extension", default: "PT0S" + t.string "deleted_user_email_never_send" t.index "to_tsvector('french'::regconfig, (search_terms || private_search_terms))", name: "index_dossiers_on_search_terms_private_search_terms", using: :gin t.index "to_tsvector('french'::regconfig, search_terms)", name: "index_dossiers_on_search_terms", using: :gin t.index ["archived"], name: "index_dossiers_on_archived" diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 459fbae5d..6ad1040b9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -263,13 +263,16 @@ describe User, type: :model do describe '#can_be_deleted?' do let(:user) { create(:user) } + let(:administrateur) { create(:administrateur) } + let(:instructeur) { create(:instructeur) } + let(:expert) { create(:expert) } subject { user.can_be_deleted? } context 'when the user has a dossier in instruction' do let!(:dossier) { create(:dossier, :en_instruction, user: user) } - it { is_expected.to be false } + it { is_expected.to be true } end context 'when the user has no dossier in instruction' do @@ -278,19 +281,19 @@ describe User, type: :model do context 'when the user is an administrateur' do it 'cannot be deleted' do - administrateur = create(:administrateur) - user = administrateur.user - - expect(user.can_be_deleted?).to be_falsy + expect(administrateur.user.can_be_deleted?).to be_falsy end end context 'when the user is an instructeur' do it 'cannot be deleted' do - instructeur = create(:instructeur) - user = instructeur.user + expect(instructeur.user.can_be_deleted?).to be_falsy + end + end - expect(user.can_be_deleted?).to be_falsy + context 'when the user is an expert' do + it 'cannot be deleted' do + expect(expert.user.can_be_deleted?).to be_falsy end end end @@ -299,14 +302,7 @@ describe User, type: :model do let(:super_admin) { create(:super_admin) } let(:user) { create(:user) } - context 'with a dossier in instruction' do - let!(:dossier_en_instruction) { create(:dossier, :en_instruction, user: user) } - it 'raises' do - expect { user.delete_and_keep_track_dossiers(super_admin) }.to raise_error(RuntimeError) - end - end - - context 'without a dossier in instruction' do + context 'without a dossier with processing strted' do let!(:dossier_en_construction) { create(:dossier, :en_construction, user: user) } let!(:dossier_brouillon) { create(:dossier, user: user) } @@ -321,28 +317,39 @@ describe User, type: :model do end context 'with a discarded dossier' do - let!(:dossier_cache) do - create(:dossier, :en_construction, user: user) - end - let!(:dossier_from_another_user) do - create(:dossier, :en_construction, user: create(:user)) - end + let(:dossier_to_discard) { create(:dossier, :en_construction, user: user) } + let!(:dossier_from_another_user) { create(:dossier, :en_construction, user: create(:user)) } it "keep track of dossiers and delete user" do - dossier_cache.discard_and_keep_track!(super_admin, :user_request) + dossier_to_discard.discard_and_keep_track!(super_admin, :user_request) user.delete_and_keep_track_dossiers(super_admin) expect(DeletedDossier.find_by(dossier_id: dossier_en_construction)).to be_present expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_nil + expect(Dossier.find_by(id: dossier_from_another_user.id)).to be_present expect(User.find_by(id: user.id)).to be_nil end + end + end - it "doesn't destroy dossiers of another user" do - dossier_cache.discard_and_keep_track!(super_admin, :user_request) - user.delete_and_keep_track_dossiers(super_admin) + context 'with dossiers with processing strted' do + let!(:dossier_en_instruction) { create(:dossier, :en_instruction, user: user) } + let!(:dossier_termine) { create(:dossier, :accepte, user: user) } - expect(Dossier.find_by(id: dossier_from_another_user.id)).to be_present - end + it "keep track of dossiers and delete user" do + user.delete_and_keep_track_dossiers(super_admin) + + expect(dossier_en_instruction.reload).to be_present + expect(dossier_en_instruction.user).to be_nil + expect(dossier_en_instruction.user_email_for(:display)).to eq(user.email) + expect { dossier_en_instruction.user_email_for(:notification) }.to raise_error(RuntimeError) + + expect(dossier_termine.reload).to be_present + expect(dossier_termine.user).to be_nil + expect(dossier_termine.user_email_for(:display)).to eq(user.email) + expect { dossier_termine.user_email_for(:notification) }.to raise_error(RuntimeError) + + expect(User.find_by(id: user.id)).to be_nil end end end From f6508899deb952b5a3d7d835de27c45be607d7f5 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 29 Apr 2021 19:10:22 +0200 Subject: [PATCH 09/11] Refactor NotificationMailer --- app/controllers/users/dossiers_controller.rb | 2 +- app/mailers/notification_mailer.rb | 69 ++++++++++--------- app/models/dossier.rb | 14 ++-- app/models/procedure.rb | 17 +++++ .../instructeurs/dossiers_controller_spec.rb | 6 +- .../users/dossiers_controller_spec.rb | 8 +-- spec/mailers/notification_mailer_spec.rb | 31 ++++++++- .../previews/notification_mailer_preview.rb | 37 ++++++---- spec/models/dossier_spec.rb | 14 ++-- 9 files changed, 128 insertions(+), 70 deletions(-) diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index cdc030978..791049a78 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -143,7 +143,7 @@ module Users if passage_en_construction? && errors.blank? @dossier.passer_en_construction! - NotificationMailer.send_initiated_notification(@dossier).deliver_later + NotificationMailer.send_en_construction_notification(@dossier).deliver_later @dossier.groupe_instructeur.instructeurs.with_instant_email_dossier_notifications.each do |instructeur| DossierMailer.notify_new_dossier_depose_to_instructeur(@dossier, instructeur.email).deliver_later end diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb index d958d781b..056240dc2 100644 --- a/app/mailers/notification_mailer.rb +++ b/app/mailers/notification_mailer.rb @@ -8,7 +8,8 @@ class NotificationMailer < ApplicationMailer include ActionView::Helpers::SanitizeHelper - before_action :prevent_delivery_to_deleted_users + before_action :set_dossier + after_action :create_commentaire_for_notification helper ServiceHelper helper MailerHelper @@ -16,52 +17,54 @@ class NotificationMailer < ApplicationMailer layout 'mailers/notifications_layout' default from: NO_REPLY_EMAIL - def send_dossier_received(dossier) - send_notification(dossier, dossier.procedure.received_mail_template) + def send_notification + @service = @dossier.procedure.service + @logo_url = attach_logo(@dossier.procedure) + @rendered_template = sanitize(@body) + + mail(subject: @subject, to: @email, template_name: 'send_notification') end - def send_initiated_notification(dossier) - send_notification(dossier, dossier.procedure.initiated_mail_template) + def self.send_en_construction_notification(dossier) + with(dossier: dossier, state: Dossier.states.fetch(:en_construction)).send_notification end - def send_closed_notification(dossier) - send_notification(dossier, dossier.procedure.closed_mail_template) + def self.send_en_instruction_notification(dossier) + with(dossier: dossier, state: Dossier.states.fetch(:en_instruction)).send_notification end - def send_refused_notification(dossier) - send_notification(dossier, dossier.procedure.refused_mail_template) + def self.send_accepte_notification(dossier) + with(dossier: dossier, state: Dossier.states.fetch(:accepte)).send_notification end - def send_without_continuation_notification(dossier) - send_notification(dossier, dossier.procedure.without_continuation_mail_template) + def self.send_refuse_notification(dossier) + with(dossier: dossier, state: Dossier.states.fetch(:refuse)).send_notification + end + + def self.send_sans_suite_notification(dossier) + with(dossier: dossier, state: Dossier.states.fetch(:sans_suite)).send_notification end private - def prevent_delivery_to_deleted_users - !@dossier.user_deleted? + def set_dossier + @dossier = params[:dossier] + + if @dossier.user_deleted? + mail.perform_deliveries = false + else + mail_template = @dossier.procedure.mail_template_for(params[:state]) + + @email = @dossier.user_email_for(:notification) + @subject = mail_template.subject_for_dossier(@dossier) + @body = mail_template.body_for_dossier(@dossier) + @actions = mail_template.actions_for_dossier(@dossier) + end end - def send_notification(dossier, mail_template) - email = dossier.user_email_for(:notification) - - subject = mail_template.subject_for_dossier(dossier) - body = mail_template.body_for_dossier(dossier) - - create_commentaire_for_notification(dossier, subject, body) - - @dossier = dossier - @service = dossier.procedure.service - @logo_url = attach_logo(dossier.procedure) - @rendered_template = sanitize(body) - @actions = mail_template.actions_for_dossier(dossier) - - mail(subject: subject, to: email, template_name: 'send_notification') - end - - def create_commentaire_for_notification(dossier, subject, body) - params = { body: ["[#{subject}]", body].join("

") } - commentaire = CommentaireService.build_with_email(CONTACT_EMAIL, dossier, params) + def create_commentaire_for_notification + body = ["[#{@subject}]", @body].join("

") + commentaire = CommentaireService.build_with_email(CONTACT_EMAIL, @dossier, body: body) commentaire.save! end end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 53ec3ff38..cca2baa97 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -344,7 +344,7 @@ class Dossier < ApplicationRecord before_save :build_default_champs, if: Proc.new { revision_id_was.nil? } before_save :update_search_terms - after_save :send_dossier_received + after_save :send_dossier_en_instruction after_save :send_web_hook after_create_commit :send_draft_notification_email @@ -690,7 +690,7 @@ class Dossier < ApplicationRecord save! remove_titres_identite! - NotificationMailer.send_closed_notification(self).deliver_later + NotificationMailer.send_accepte_notification(self).deliver_later send_dossier_decision_to_experts(self) log_dossier_operation(instructeur, :accepter, self) end @@ -705,7 +705,7 @@ class Dossier < ApplicationRecord save! remove_titres_identite! - NotificationMailer.send_closed_notification(self).deliver_later + NotificationMailer.send_accepte_notification(self).deliver_later log_automatic_dossier_operation(:accepter, self) end @@ -718,7 +718,7 @@ class Dossier < ApplicationRecord save! remove_titres_identite! - NotificationMailer.send_refused_notification(self).deliver_later + NotificationMailer.send_refuse_notification(self).deliver_later send_dossier_decision_to_experts(self) log_dossier_operation(instructeur, :refuser, self) end @@ -732,7 +732,7 @@ class Dossier < ApplicationRecord save! remove_titres_identite! - NotificationMailer.send_without_continuation_notification(self).deliver_later + NotificationMailer.send_sans_suite_notification(self).deliver_later send_dossier_decision_to_experts(self) log_dossier_operation(instructeur, :classer_sans_suite, self) end @@ -949,9 +949,9 @@ class Dossier < ApplicationRecord end end - def send_dossier_received + def send_dossier_en_instruction if saved_change_to_state? && en_instruction? && !procedure.declarative_accepte? - NotificationMailer.send_dossier_received(self).deliver_later + NotificationMailer.send_en_instruction_notification(self).deliver_later end end diff --git a/app/models/procedure.rb b/app/models/procedure.rb index b4760c353..0e6a11783 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -500,6 +500,23 @@ class Procedure < ApplicationRecord without_continuation_mail || Mails::WithoutContinuationMail.default_for_procedure(self) end + def mail_template_for(state) + case state + when Dossier.states.fetch(:en_construction) + initiated_mail_template + when Dossier.states.fetch(:en_instruction) + received_mail_template + when Dossier.states.fetch(:accepte) + closed_mail_template + when Dossier.states.fetch(:refuse) + refused_mail_template + when Dossier.states.fetch(:sans_suite) + without_continuation_mail_template + else + raise "Unknown dossier state: #{state}" + end + end + def self.default_sort { 'table' => 'self', diff --git a/spec/controllers/instructeurs/dossiers_controller_spec.rb b/spec/controllers/instructeurs/dossiers_controller_spec.rb index ab6d9ff5a..1521c7dd3 100644 --- a/spec/controllers/instructeurs/dossiers_controller_spec.rb +++ b/spec/controllers/instructeurs/dossiers_controller_spec.rb @@ -210,7 +210,7 @@ describe Instructeurs::DossiersController, type: :controller do end it 'Notification email is sent' do - expect(NotificationMailer).to receive(:send_refused_notification) + expect(NotificationMailer).to receive(:send_refuse_notification) .with(dossier).and_return(NotificationMailer) expect(NotificationMailer).to receive(:deliver_later) @@ -250,7 +250,7 @@ describe Instructeurs::DossiersController, type: :controller do end it 'Notification email is sent' do - expect(NotificationMailer).to receive(:send_without_continuation_notification) + expect(NotificationMailer).to receive(:send_sans_suite_notification) .with(dossier).and_return(NotificationMailer) expect(NotificationMailer).to receive(:deliver_later) @@ -280,7 +280,7 @@ describe Instructeurs::DossiersController, type: :controller do dossier.passer_en_instruction!(instructeur) sign_in(instructeur.user) - expect(NotificationMailer).to receive(:send_closed_notification) + expect(NotificationMailer).to receive(:send_accepte_notification) .with(dossier) .and_return(NotificationMailer) diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index d69c83670..fd3a0c73d 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -476,12 +476,12 @@ describe Users::DossiersController, type: :controller do delivery = double expect(delivery).to receive(:deliver_later).with(no_args) - expect(NotificationMailer).to receive(:send_initiated_notification) + expect(NotificationMailer).to receive(:send_en_construction_notification) .and_return(delivery) subject - expect(NotificationMailer).not_to receive(:send_initiated_notification) + expect(NotificationMailer).not_to receive(:send_en_construction_notification) subject end @@ -499,7 +499,7 @@ describe Users::DossiersController, type: :controller do it { expect(flash.alert).to eq(['nop']) } it 'does not send an email' do - expect(NotificationMailer).not_to receive(:send_initiated_notification) + expect(NotificationMailer).not_to receive(:send_en_construction_notification) subject end @@ -687,7 +687,7 @@ describe Users::DossiersController, type: :controller do end it 'does not send an email' do - expect(NotificationMailer).not_to receive(:send_initiated_notification) + expect(NotificationMailer).not_to receive(:send_en_construction_notification) subject end diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index 667fc4eaf..95e66b4db 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -1,19 +1,21 @@ RSpec.describe NotificationMailer, type: :mailer do + let(:administrateur) { create(:administrateur) } let(:user) { create(:user) } let(:procedure) { create(:simple_procedure) } - let(:dossier) { create(:dossier, :en_construction, :with_individual, :with_service, user: user, procedure: procedure) } - describe '.send_dossier_received' do + describe 'send_en_instruction_notification' do + let(:dossier) { create(:dossier, :en_construction, :with_individual, :with_service, user: user, procedure: procedure) } let(:email_template) { create(:received_mail, subject: 'Email subject', body: 'Your dossier was processed. Thanks.') } before do dossier.procedure.received_mail = email_template end - subject(:mail) { described_class.send_dossier_received(dossier) } + subject(:mail) { described_class.send_en_instruction_notification(dossier) } it 'creates a commentaire in the messagerie' do expect { subject.deliver_now }.to change { Commentaire.count }.by(1) + expect(subject.perform_deliveries).to be_truthy commentaire = Commentaire.last expect(commentaire.body).to include(email_template.subject_for_dossier(dossier), email_template.body_for_dossier(dossier)) @@ -59,4 +61,27 @@ RSpec.describe NotificationMailer, type: :mailer do expect(subject.from.first).to eq(Mail::Address.new(NO_REPLY_EMAIL).address) end end + + describe 'send_accepte_notification' do + let(:dossier) { create(:dossier, :en_instruction, :with_individual, :with_service, user: user, procedure: procedure) } + let(:email_template) { create(:closed_mail, subject: 'Email subject', body: 'Your dossier was accepted. Thanks.') } + + before do + dossier.procedure.closed_mail = email_template + end + + subject(:mail) { described_class.send_accepte_notification(dossier) } + + context 'when dossier user is deleted' do + before do + dossier.user.delete_and_keep_track_dossiers(administrateur) + dossier.reload + end + + it 'should not send notification' do + expect { subject.deliver_now }.not_to change { Commentaire.count } + expect(subject.perform_deliveries).to be_falsey + end + end + end end diff --git a/spec/mailers/previews/notification_mailer_preview.rb b/spec/mailers/previews/notification_mailer_preview.rb index dff8bbed9..8d7dbf075 100644 --- a/spec/mailers/previews/notification_mailer_preview.rb +++ b/spec/mailers/previews/notification_mailer_preview.rb @@ -1,23 +1,36 @@ class NotificationMailerPreview < ActionMailer::Preview - def send_initiated_notification - p = Procedure.where(id: Mails::InitiatedMail.where("body like ?", "% Date: Tue, 4 May 2021 10:31:13 +0200 Subject: [PATCH 10/11] prend en compte uniquement les pj pour estimer la taille d'un dossier --- .../instructeurs/dossiers_controller.rb | 2 +- app/lib/active_storage/downloadable_file.rb | 2 +- app/models/dossier.rb | 5 +- app/services/pieces_justificatives_service.rb | 10 +++- .../dossiers/_header_actions.html.haml | 19 ++++---- spec/models/dossier_spec.rb | 12 ++--- .../pieces_justificatives_service_spec.rb | 47 ++++++++++++++----- 7 files changed, 60 insertions(+), 37 deletions(-) diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb index e6fb16d21..0a2d6f8ea 100644 --- a/app/controllers/instructeurs/dossiers_controller.rb +++ b/app/controllers/instructeurs/dossiers_controller.rb @@ -211,7 +211,7 @@ module Instructeurs end def telecharger_pjs - return head(:forbidden) if !dossier.attachments_downloadable? + return head(:forbidden) if !dossier.export_and_attachments_downloadable? files = ActiveStorage::DownloadableFile.create_list_from_dossier(dossier) diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index 25813202b..bdfe5760b 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -1,7 +1,7 @@ class ActiveStorage::DownloadableFile def self.create_list_from_dossier(dossier) dossier_export = PiecesJustificativesService.generate_dossier_export(dossier) - pjs = [dossier_export] + PiecesJustificativesService.liste_pieces_justificatives(dossier) + pjs = [dossier_export] + PiecesJustificativesService.liste_documents(dossier) pjs.map do |piece_justificative| [ piece_justificative, diff --git a/app/models/dossier.rb b/app/models/dossier.rb index cca2baa97..50f4553a5 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -873,9 +873,8 @@ class Dossier < ApplicationRecord end end - def attachments_downloadable? - PiecesJustificativesService.liste_pieces_justificatives(self).present? \ - && PiecesJustificativesService.pieces_justificatives_total_size(self) < Dossier::TAILLE_MAX_ZIP + def export_and_attachments_downloadable? + PiecesJustificativesService.pieces_justificatives_total_size(self) < Dossier::TAILLE_MAX_ZIP end def linked_dossiers_for(instructeur_or_expert) diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index 3c20ab32d..03c173599 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -1,5 +1,5 @@ class PiecesJustificativesService - def self.liste_pieces_justificatives(dossier) + def self.liste_documents(dossier) pjs_champs = pjs_for_champs(dossier) pjs_commentaires = pjs_for_commentaires(dossier) pjs_dossier = pjs_for_dossier(dossier) @@ -8,6 +8,14 @@ class PiecesJustificativesService .filter(&:attached?) end + def self.liste_pieces_justificatives(dossier) + pjs_champs = pjs_for_champs(dossier) + pjs_commentaires = pjs_for_commentaires(dossier) + + (pjs_champs + pjs_commentaires) + .filter(&:attached?) + end + def self.pieces_justificatives_total_size(dossier) liste_pieces_justificatives(dossier) .sum(&:byte_size) diff --git a/app/views/instructeurs/dossiers/_header_actions.html.haml b/app/views/instructeurs/dossiers/_header_actions.html.haml index 9a5e871ea..ca50a1164 100644 --- a/app/views/instructeurs/dossiers/_header_actions.html.haml +++ b/app/views/instructeurs/dossiers/_header_actions.html.haml @@ -12,16 +12,15 @@ %li = link_to "Export GeoJSON", geo_data_instructeur_dossier_path(dossier.procedure, dossier), target: "_blank", rel: "noopener", class: "menu-item menu-link" -- if PiecesJustificativesService.liste_pieces_justificatives(dossier).present? - %span.dropdown.print-menu-opener - %button.button.dropdown-button.icon-only{ 'aria-expanded' => 'false', 'aria-controls' => 'print-pj-menu' } - %span.icon.attached - %ul#print-pj-menu.print-menu.dropdown-content - %li - - if PiecesJustificativesService.pieces_justificatives_total_size(dossier) < Dossier::TAILLE_MAX_ZIP - = link_to "Télécharger le dossier et toutes ses pièces jointes", telecharger_pjs_instructeur_dossier_path(dossier.procedure, dossier), target: "_blank", rel: "noopener", class: "menu-item menu-link" - - else - %p.menu-item Le téléchargement des pièces jointes est désactivé pour les dossiers de plus de #{number_to_human_size Dossier::TAILLE_MAX_ZIP}. +%span.dropdown.print-menu-opener + %button.button.dropdown-button.icon-only{ 'aria-expanded' => 'false', 'aria-controls' => 'print-pj-menu' } + %span.icon.attached + %ul#print-pj-menu.print-menu.dropdown-content + %li + - if dossier.export_and_attachments_downloadable? + = link_to "Télécharger le dossier et toutes ses pièces jointes", telecharger_pjs_instructeur_dossier_path(dossier.procedure, dossier), target: "_blank", rel: "noopener", class: "menu-item menu-link" + - else + %p.menu-item Le téléchargement des pièces jointes est désactivé pour les dossiers de plus de #{number_to_human_size Dossier::TAILLE_MAX_ZIP}. = render partial: "instructeurs/procedures/dossier_actions", locals: { procedure_id: dossier.procedure.id, diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index e92b9ec4d..f4db5f69f 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -1135,30 +1135,26 @@ describe Dossier do after { Timecop.return } end - describe '#attachments_downloadable?' do + describe '#export_and_attachments_downloadable?' do let(:dossier) { create(:dossier, user: user) } - # subject { dossier.attachments_downloadable? } context "no attachments" do it { - expect(PiecesJustificativesService).to receive(:liste_pieces_justificatives).and_return([]) - expect(dossier.attachments_downloadable?).to be false + expect(dossier.export_and_attachments_downloadable?).to be true } end context "with a small attachment" do it { - expect(PiecesJustificativesService).to receive(:liste_pieces_justificatives).and_return([Champ.new]) expect(PiecesJustificativesService).to receive(:pieces_justificatives_total_size).and_return(4.megabytes) - expect(dossier.attachments_downloadable?).to be true + expect(dossier.export_and_attachments_downloadable?).to be true } end context "with a too large attachment" do it { - expect(PiecesJustificativesService).to receive(:liste_pieces_justificatives).and_return([Champ.new]) expect(PiecesJustificativesService).to receive(:pieces_justificatives_total_size).and_return(100.megabytes) - expect(dossier.attachments_downloadable?).to be false + expect(dossier.export_and_attachments_downloadable?).to be false } end end diff --git a/spec/services/pieces_justificatives_service_spec.rb b/spec/services/pieces_justificatives_service_spec.rb index 13d843563..b4e7b01a9 100644 --- a/spec/services/pieces_justificatives_service_spec.rb +++ b/spec/services/pieces_justificatives_service_spec.rb @@ -1,20 +1,24 @@ describe PiecesJustificativesService do + let(:procedure) { create(:procedure, :with_titre_identite) } + let(:dossier) { create(:dossier, procedure: procedure) } + let(:champ_identite) { dossier.champs.find { |c| c.type == 'Champs::TitreIdentiteChamp' } } + let(:bill_signature) do + bs = build(:bill_signature, :with_serialized, :with_signature) + bs.save(validate: false) + bs + end + + before do + champ_identite + .piece_justificative_file + .attach(io: StringIO.new("toto"), filename: "toto.png", content_type: "image/png") + create(:dossier_operation_log, dossier: dossier, bill_signature: bill_signature) + end + describe '.liste_pieces_justificatives' do - let(:procedure) { create(:procedure, :with_titre_identite) } - let(:dossier) { create(:dossier, procedure: procedure) } - let(:champ_identite) { dossier.champs.find { |c| c.type == 'Champs::TitreIdentiteChamp' } } - - before do - champ_identite - .piece_justificative_file - .attach(io: StringIO.new("toto"), filename: "toto.png", content_type: "image/png") - end - subject { PiecesJustificativesService.liste_pieces_justificatives(dossier) } - # titre identite is too sensitive - # to be exported - it 'ensures no titre identite is given' do + it "doesn't return sensitive documents like titre_identite" do expect(champ_identite.piece_justificative_file).to be_attached expect(subject.any? { |piece| piece.name == 'piece_justificative_file' }).to be_falsy end @@ -22,5 +26,22 @@ describe PiecesJustificativesService do it "doesn't return export pdf of the dossier" do expect(subject.any? { |piece| piece.name == 'pdf_export_for_instructeur' }).to be_falsy end + + it "doesn't return operation logs of the dossier" do + expect(subject.any? { |piece| piece.name == 'serialized' }).to be_falsy + end + end + + describe '.liste_documents' do + subject { PiecesJustificativesService.liste_documents(dossier) } + + it "doesn't return sensitive documents like titre_identite" do + expect(champ_identite.piece_justificative_file).to be_attached + expect(subject.any? { |piece| piece.name == 'piece_justificative_file' }).to be_falsy + end + + it "returns operation logs of the dossier" do + expect(subject.any? { |piece| piece.name == 'serialized' }).to be_truthy + end end end From bcbfcdc537937ee926860278ee959c98888c68b8 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 4 May 2021 16:03:29 +0200 Subject: [PATCH 11/11] Revert "Merge pull request #6142 from tchak/enable_brouillon_extend_conservation" This reverts commit 48eb4d977877ea7191ec4dfca0badd319ceba6af, reversing changes made to 5539d5cb8c6c3ebe2ff5ec988e7134ea06699c1c. # Conflicts: # app/models/dossier.rb # db/schema.rb --- app/controllers/users/dossiers_controller.rb | 2 +- app/models/dossier.rb | 24 +++++-------------- .../users/dossiers/show/_header.html.haml | 16 ++++--------- .../notify_brouillon_near_deletion/fr.yml | 4 ++-- ..._add_conservation_extension_to_dossiers.rb | 5 ---- db/schema.rb | 1 - ...9172327_rename_conservation_extension.rake | 16 ------------- spec/models/dossier_spec.rb | 11 +-------- 8 files changed, 15 insertions(+), 64 deletions(-) delete mode 100644 db/migrate/20210428104228_add_conservation_extension_to_dossiers.rb delete mode 100644 lib/tasks/deployment/20210429172327_rename_conservation_extension.rake diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 791049a78..987939cc6 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -161,7 +161,7 @@ module Users end def extend_conservation - dossier.update(conservation_extension: dossier.conservation_extension + 1.month) + dossier.update(en_construction_conservation_extension: dossier.en_construction_conservation_extension + 1.month) flash[:notice] = 'Votre dossier sera conservé un mois supplémentaire' redirect_to dossier_path(@dossier) end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 50f4553a5..9d7b53986 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -7,7 +7,6 @@ # archived :boolean default(FALSE) # autorisation_donnees :boolean # brouillon_close_to_expiration_notice_sent_at :datetime -# conservation_extension :interval default(0 seconds) # deleted_user_email_never_send :string # en_construction_at :datetime # en_construction_close_to_expiration_notice_sent_at :datetime @@ -256,22 +255,21 @@ class Dossier < ApplicationRecord scope :brouillon_close_to_expiration, -> do state_brouillon .joins(:procedure) - .where("dossiers.created_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + .where("dossiers.created_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) end scope :en_construction_close_to_expiration, -> do state_en_construction .joins(:procedure) - .where("dossiers.en_construction_at + dossiers.conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) + .where("dossiers.en_construction_at + dossiers.en_construction_conservation_extension + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) end scope :en_instruction_close_to_expiration, -> do state_en_instruction .joins(:procedure) .where("dossiers.en_instruction_at + (duree_conservation_dossiers_dans_ds * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION }) end - scope :termine_close_to_expiration, -> do - state_termine - .joins(:procedure) - .where(id: Traitement.termine_close_to_expiration.pluck(:dossier_id).uniq) + def self.termine_close_to_expiration + dossier_ids = Traitement.termine_close_to_expiration.pluck(:dossier_id).uniq + Dossier.where(id: dossier_ids) end scope :brouillon_expired, -> do @@ -455,15 +453,7 @@ class Dossier < ApplicationRecord end def en_construction_close_to_expiration? - self.class.en_construction_close_to_expiration.exists?(id: self) - end - - def brouillon_close_to_expiration? - self.class.brouillon_close_to_expiration.exists?(id: self) - end - - def close_to_expiration? - en_construction_close_to_expiration? || brouillon_close_to_expiration? + Dossier.en_construction_close_to_expiration.where(id: self).present? end def show_groupe_instructeur_details? @@ -646,7 +636,6 @@ class Dossier < ApplicationRecord end def after_passer_en_construction - update!(conservation_extension: 0.days) update!(en_construction_at: Time.zone.now) if self.en_construction_at.nil? end @@ -663,7 +652,6 @@ class Dossier < ApplicationRecord end def after_repasser_en_construction(instructeur) - update!(conservation_extension: 0.days) log_dossier_operation(instructeur, :repasser_en_construction) end diff --git a/app/views/users/dossiers/show/_header.html.haml b/app/views/users/dossiers/show/_header.html.haml index ceed29f78..905f34f42 100644 --- a/app/views/users/dossiers/show/_header.html.haml +++ b/app/views/users/dossiers/show/_header.html.haml @@ -22,19 +22,13 @@ %li = link_to "Tout le dossier", dossier_path(dossier, format: :pdf), target: "_blank", rel: "noopener", class: "menu-item menu-link" - - if dossier.close_to_expiration? + - if dossier.en_construction_close_to_expiration? .card.warning .card-title Votre dossier va expirer - - if dossier.brouillon? - %p - Votre dossier est en brouillon, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été déposé. - Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver - un mois de plus en cliquant sur le bouton ci-dessous. - - else - %p - Votre dossier a été déposé, mais va bientôt expirer. Cela signifie qu’il va bientôt être supprimé sans avoir été traité par l’administration. - Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver - un mois de plus en cliquant sur le bouton ci-dessous. + %p + Votre dossier a été déposé, mais va bientôt expirer. Cela signifie qu'il va bientôt être supprimé sans avoir été traité par l’administration. + Si vous souhaitez le conserver afin de poursuivre la démarche, vous pouvez le conserver + un mois de plus en cliquant sur le bouton ci-dessous. %br = button_to 'Repousser sa suppression', users_dossier_repousser_expiration_path(dossier), class: 'button secondary' diff --git a/config/locales/views/dossier_mailer/notify_brouillon_near_deletion/fr.yml b/config/locales/views/dossier_mailer/notify_brouillon_near_deletion/fr.yml index 883a7130a..f4ccef4e0 100644 --- a/config/locales/views/dossier_mailer/notify_brouillon_near_deletion/fr.yml +++ b/config/locales/views/dossier_mailer/notify_brouillon_near_deletion/fr.yml @@ -8,5 +8,5 @@ fr: one: "Afin de limiter la conservation de vos données personnelles, le dossier en brouillon suivant sera bientôt automatiquement supprimé :" other: "Afin de limiter la conservation de vos données personnelles, les dossiers en brouillon suivant seront bientôt automatiquement supprimés :" footer: - one: "Si vous souhaitez toujours déposer ce dossier, vous pouvez retrouver votre brouillon pendant encore un mois. Si vous souhaitez conserver votre dossier plus longtemps, vous pouvez prolonger sa durée de conservation dans l'interface. Et sinon, vous n’avez rien à faire." - other: "Si vous souhaitez toujours déposer ces dossiers, vous pouvez retrouver vos brouillons pendant encore un mois. Si vous souhaitez conserver vos dossiers plus longtemps, vous pouvez prolonger leur durée de conservation au cas par cas dans l'interface. Et sinon, vous n’avez rien à faire." + one: "Si vous souhaitez toujours déposer ce dossier, vous pouvez retrouver votre brouillon pendant encore un mois. Et sinon, vous n’avez rien à faire." + other: "Si vous souhaitez toujours déposer ces dossiers, vous pouvez retrouver vos brouillons pendant encore un mois. Et sinon, vous n’avez rien à faire." diff --git a/db/migrate/20210428104228_add_conservation_extension_to_dossiers.rb b/db/migrate/20210428104228_add_conservation_extension_to_dossiers.rb deleted file mode 100644 index 909ac22f0..000000000 --- a/db/migrate/20210428104228_add_conservation_extension_to_dossiers.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddConservationExtensionToDossiers < ActiveRecord::Migration[6.1] - def change - add_column :dossiers, :conservation_extension, :interval, default: 0.days - end -end diff --git a/db/schema.rb b/db/schema.rb index 09d1a2239..c33baa8f5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -274,7 +274,6 @@ ActiveRecord::Schema.define(version: 2021_04_28_114228) do t.datetime "last_avis_updated_at" t.datetime "last_commentaire_updated_at" t.string "api_entreprise_job_exceptions", array: true - t.interval "conservation_extension", default: "PT0S" t.string "deleted_user_email_never_send" t.index "to_tsvector('french'::regconfig, (search_terms || private_search_terms))", name: "index_dossiers_on_search_terms_private_search_terms", using: :gin t.index "to_tsvector('french'::regconfig, search_terms)", name: "index_dossiers_on_search_terms", using: :gin diff --git a/lib/tasks/deployment/20210429172327_rename_conservation_extension.rake b/lib/tasks/deployment/20210429172327_rename_conservation_extension.rake deleted file mode 100644 index 16d83125b..000000000 --- a/lib/tasks/deployment/20210429172327_rename_conservation_extension.rake +++ /dev/null @@ -1,16 +0,0 @@ -namespace :after_party do - desc 'Deployment task: rename_conservation_extension' - task rename_conservation_extension: :environment do - puts "Running deploy task 'rename_conservation_extension'" - - dossiers = Dossier.state_en_construction.where.not(en_construction_conservation_extension: 0.days) - dossiers.find_each do |dossier| - dossier.update_column(:conservation_extension, dossier.en_construction_conservation_extension) - end - - # Update task as completed. If you remove the line below, the task will - # run with every deploy (or every time you call after_party:run). - AfterParty::TaskRecord - .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp - end -end diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index f4db5f69f..ac82dd094 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -52,15 +52,6 @@ describe Dossier do is_expected.to include(just_expired_dossier) is_expected.to include(long_expired_dossier) end - - context 'does not include an expiring dossier that has been postponed' do - before do - expiring_dossier.update(conservation_extension: 1.month) - expiring_dossier.reload - end - - it { is_expected.not_to include(expiring_dossier) } - end end describe 'en_construction_close_to_expiration' do @@ -81,7 +72,7 @@ describe Dossier do context 'does not include an expiring dossier that has been postponed' do before do - expiring_dossier.update(conservation_extension: 1.month) + expiring_dossier.update(en_construction_conservation_extension: 1.month) expiring_dossier.reload end