From 8790ac49784c1720ea88fa8abefabd5a6bc54fe8 Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 3 Nov 2023 13:52:37 +0100 Subject: [PATCH] amelioration(ExpiredUsersDeletionService): ajoute le rate limit a l'envoie des mails --- app/services/concerns/mail_rate_limitable.rb | 13 +++++++++ .../expired_dossiers_deletion_service.rb | 8 +----- .../expired_users_deletion_service.rb | 28 +++++++++---------- .../expired_users_deletion_service_spec.rb | 23 ++++++++------- 4 files changed, 41 insertions(+), 31 deletions(-) create mode 100644 app/services/concerns/mail_rate_limitable.rb diff --git a/app/services/concerns/mail_rate_limitable.rb b/app/services/concerns/mail_rate_limitable.rb new file mode 100644 index 000000000..89ff0f301 --- /dev/null +++ b/app/services/concerns/mail_rate_limitable.rb @@ -0,0 +1,13 @@ +module MailRateLimitable + extend ActiveSupport::Concern + + included do + def initialize(rate_limiter: MailRateLimiter.new(limit: 200, window: 10.minutes)) + @rate_limiter = rate_limiter + end + + def safe_send_email(mail) + @rate_limiter.send_with_delay(mail) + end + end +end diff --git a/app/services/expired_dossiers_deletion_service.rb b/app/services/expired_dossiers_deletion_service.rb index 534a68f8f..13ed8eb0e 100644 --- a/app/services/expired_dossiers_deletion_service.rb +++ b/app/services/expired_dossiers_deletion_service.rb @@ -1,7 +1,5 @@ class ExpiredDossiersDeletionService - def initialize(rate_limiter: MailRateLimiter.new(limit: 200, window: 10.minutes)) - @rate_limiter = rate_limiter - end + include MailRateLimitable def process_expired_dossiers_brouillon send_brouillon_expiration_notices @@ -18,10 +16,6 @@ class ExpiredDossiersDeletionService delete_expired_termine_and_notify end - def safe_send_email(mail) - @rate_limiter.send_with_delay(mail) - end - def send_brouillon_expiration_notices dossiers_close_to_expiration = Dossier .brouillon_close_to_expiration diff --git a/app/services/expired_users_deletion_service.rb b/app/services/expired_users_deletion_service.rb index a54713f80..bf0ada968 100644 --- a/app/services/expired_users_deletion_service.rb +++ b/app/services/expired_users_deletion_service.rb @@ -1,52 +1,52 @@ class ExpiredUsersDeletionService - def self.expirable_after - 2.years.ago - end - RETENTION_AFTER_NOTICE_IN_WEEK = 2 + include MailRateLimitable - def self.process_expired + RETENTION_AFTER_NOTICE_IN_WEEK = 2 + EXPIRABLE_AFTER_IN_YEAR = 2 + + def process_expired [expiring_users_without_dossiers, expiring_users_with_dossiers].map do |expiring_segment| delete_expired_users(expiring_segment) send_inactive_close_to_expiration_notice(expiring_segment) end end - def self.send_inactive_close_to_expiration_notice(users) + def send_inactive_close_to_expiration_notice(users) to_notify_only(users).in_batches do |batch| batch.each do |user| - UserMailer.notify_inactive_close_to_deletion(user).perform_later + safe_send_email(UserMailer.notify_inactive_close_to_deletion(user)) end batch.update_all(inactive_close_to_expiration_notice_sent_at: Time.zone.now.utc) end end - def self.delete_expired_users(users) + def delete_expired_users(users) to_delete_only(users).find_each do |user| user.delete_and_keep_track_dossiers_also_delete_user(nil) end end # rubocop:disable DS/Unscoped - def self.expiring_users_with_dossiers + def expiring_users_with_dossiers User.unscoped # avoid default_scope eager_loading :export, :instructeur, :administrateur .joins(:dossiers) - .having('MAX(dossiers.created_at) < ?', EXPIRABLE_AFTER) + .having('MAX(dossiers.created_at) < ?', EXPIRABLE_AFTER_IN_YEAR.years.ago) .group('users.id') end - def self.expiring_users_without_dossiers + def expiring_users_without_dossiers User.unscoped .where .missing(:dossiers) - .where(last_sign_in_at: ..EXPIRABLE_AFTER) + .where(last_sign_in_at: ..EXPIRABLE_AFTER_IN_YEAR.years.ago) end # rubocop:enable DS/Unscoped - def self.to_notify_only(users) + def to_notify_only(users) users.where(inactive_close_to_expiration_notice_sent_at: nil) end - def self.to_delete_only(users) + def to_delete_only(users) users.where.not(inactive_close_to_expiration_notice_sent_at: RETENTION_AFTER_NOTICE_IN_WEEK.weeks.ago..) end end diff --git a/spec/services/expired_users_deletion_service_spec.rb b/spec/services/expired_users_deletion_service_spec.rb index 9b6a71e2d..2744bc97c 100644 --- a/spec/services/expired_users_deletion_service_spec.rb +++ b/spec/services/expired_users_deletion_service_spec.rb @@ -1,15 +1,23 @@ describe ExpiredUsersDeletionService do + let(:mail_double) do + dbl = double() + expect(dbl).to receive(:deliver_later).with(wait: 0) + dbl + end + before { user && dossier } + describe '#process_expired' do + subject { ExpiredUsersDeletionService.new.process_expired } + context 'when user has an expirable dossier' do let(:dossier) { create(:dossier, user:, created_at: 3.years.ago) } context 'when user was not notified' do let(:user) { create(:user, inactive_close_to_expiration_notice_sent_at: nil) } - subject { ExpiredUsersDeletionService.process_expired } it 'update user.inactive_close_to_expiration_notice_sent_at ' do - expect(UserMailer).to receive(:notify_inactive_close_to_deletion).with(user).and_return(double(perform_later: true)) + expect(UserMailer).to receive(:notify_inactive_close_to_deletion).with(user).and_return(mail_double) expect { subject } .to change { user.reload.inactive_close_to_expiration_notice_sent_at } .from(nil).to(anything) @@ -18,7 +26,6 @@ describe ExpiredUsersDeletionService do context 'user has been notified 1 week ago' do let(:user) { create(:user, inactive_close_to_expiration_notice_sent_at: 1.week.ago) } - subject { ExpiredUsersDeletionService.process_expired } it 'do nothing' do expect { subject }.not_to change { Dossier.count } @@ -28,7 +35,6 @@ describe ExpiredUsersDeletionService do context 'user has been notified 3 weeks ago' do let(:user) { create(:user, inactive_close_to_expiration_notice_sent_at: 3.weeks.ago) } - subject { ExpiredUsersDeletionService.process_expired } it 'destroys user and dossier' do expect { subject }.to change { Dossier.count }.by(-1) @@ -42,10 +48,9 @@ describe ExpiredUsersDeletionService do context 'when user was not notified' do let(:user) { create(:user, last_sign_in_at: 3.years.ago, inactive_close_to_expiration_notice_sent_at: nil) } - subject { ExpiredUsersDeletionService.process_expired } it 'update user.inactive_close_to_expiration_notice_sent_at ' do - expect(UserMailer).to receive(:notify_inactive_close_to_deletion).with(user).and_return(double(perform_later: true)) + expect(UserMailer).to receive(:notify_inactive_close_to_deletion).with(user).and_return(mail_double) expect { subject } .to change { user.reload.inactive_close_to_expiration_notice_sent_at } .from(nil).to(anything) @@ -54,7 +59,6 @@ describe ExpiredUsersDeletionService do context 'when user has been notified 1 week ago' do let(:user) { create(:user, last_sign_in_at: 3.years.ago, inactive_close_to_expiration_notice_sent_at: 1.week.ago) } - subject { ExpiredUsersDeletionService.process_expired } it 'do nothing' do expect { subject }.not_to change { Dossier.count } @@ -64,7 +68,6 @@ describe ExpiredUsersDeletionService do context 'when user has been notified 3 weeks ago' do let(:user) { create(:user, last_sign_in_at: 3.years.ago, inactive_close_to_expiration_notice_sent_at: 3.weeks.ago) } - subject { ExpiredUsersDeletionService.process_expired } it 'destroys user and dossier' do subject @@ -76,7 +79,7 @@ describe ExpiredUsersDeletionService do describe '#expiring_users_without_dossiers' do let(:dossier) { nil } - subject { ExpiredUsersDeletionService.expiring_users_without_dossiers } + subject { ExpiredUsersDeletionService.new.expiring_users_without_dossiers } context 'when user last_sign_in_at is 1 year ago and has no dossier' do let(:user) { create(:user, last_sign_in_at: 1.year.ago) } @@ -91,7 +94,7 @@ describe ExpiredUsersDeletionService do describe '#expiring_users_with_dossiers' do let(:user) { create(:user) } - subject { ExpiredUsersDeletionService.expiring_users_with_dossiers } + subject { ExpiredUsersDeletionService.new.expiring_users_with_dossiers } context 'when user has a dossier created 1 year ago' do let(:dossier) { create(:dossier, user:, created_at: 1.year.ago) }