From 53ce0e05f6b440cf17fe2497902a53b06614a866 Mon Sep 17 00:00:00 2001 From: Mathieu Magnin Date: Fri, 6 Dec 2024 16:14:56 +0100 Subject: [PATCH 1/4] Do not notify not visible dossiers --- .../notify_old_brouillon_dossiers_soon_deleted_job.rb | 1 + app/jobs/cron/purge_old_brouillon_dossiers_job.rb | 1 + ...notify_old_brouillon_dossiers_soon_deleted_job_spec.rb | 8 ++++++-- spec/jobs/cron/purge_old_brouillon_dossiers_job_spec.rb | 4 ++++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/app/jobs/cron/notify_old_brouillon_dossiers_soon_deleted_job.rb b/app/jobs/cron/notify_old_brouillon_dossiers_soon_deleted_job.rb index 95e54617c..61dc27654 100644 --- a/app/jobs/cron/notify_old_brouillon_dossiers_soon_deleted_job.rb +++ b/app/jobs/cron/notify_old_brouillon_dossiers_soon_deleted_job.rb @@ -5,6 +5,7 @@ class Cron::NotifyOldBrouillonDossiersSoonDeletedJob < Cron::CronJob def perform Dossier + .visible_by_user .state_brouillon .where(updated_at: ..3.months.ago) .where("notified_soon_deleted_sent_at IS NULL OR notified_soon_deleted_sent_at < updated_at") diff --git a/app/jobs/cron/purge_old_brouillon_dossiers_job.rb b/app/jobs/cron/purge_old_brouillon_dossiers_job.rb index 147b16a29..a53da2910 100644 --- a/app/jobs/cron/purge_old_brouillon_dossiers_job.rb +++ b/app/jobs/cron/purge_old_brouillon_dossiers_job.rb @@ -5,6 +5,7 @@ class Cron::PurgeOldBrouillonDossiersJob < Cron::CronJob def perform Dossier + .visible_by_user .state_brouillon .where(updated_at: ..(3.months + 2.weeks).ago) .find_each do |dossier| diff --git a/spec/jobs/cron/notify_old_brouillon_dossiers_soon_deleted_job_spec.rb b/spec/jobs/cron/notify_old_brouillon_dossiers_soon_deleted_job_spec.rb index ddb3c3bea..de342cf8a 100644 --- a/spec/jobs/cron/notify_old_brouillon_dossiers_soon_deleted_job_spec.rb +++ b/spec/jobs/cron/notify_old_brouillon_dossiers_soon_deleted_job_spec.rb @@ -18,6 +18,8 @@ RSpec.describe Cron::NotifyOldBrouillonDossiersSoonDeletedJob, type: :job do end let!(:recent_draft) { travel_to(2.months.ago) { create(:dossier, :brouillon) } } let!(:old_non_draft) { travel_to(4.months.ago) { create(:dossier, :en_construction) } } + let!(:not_visible_dossier) { travel_to(6.months.ago) { create(:dossier, :brouillon, :hidden_by_user) } } + let!(:not_visible_dossier2) { travel_to(6.months.ago) { create(:dossier, :brouillon, :hidden_by_expired) } } it "sends notifications only for eligible draft dossiers" do expect(DossierMailer).to receive(:notify_old_brouillon_soon_deleted) @@ -30,8 +32,10 @@ RSpec.describe Cron::NotifyOldBrouillonDossiersSoonDeletedJob, type: :job do .and_return(double(deliver_later: true)) .once - expect(DossierMailer).not_to receive(:notify_old_brouillon_soon_deleted) - .with(old_draft_recently_notified) + [old_draft_recently_notified, not_visible_dossier, not_visible_dossier2].each do |dossier| + expect(DossierMailer).not_to receive(:notify_old_brouillon_soon_deleted) + .with(dossier) + end job.perform diff --git a/spec/jobs/cron/purge_old_brouillon_dossiers_job_spec.rb b/spec/jobs/cron/purge_old_brouillon_dossiers_job_spec.rb index 7d5edbb90..e2b9c535b 100644 --- a/spec/jobs/cron/purge_old_brouillon_dossiers_job_spec.rb +++ b/spec/jobs/cron/purge_old_brouillon_dossiers_job_spec.rb @@ -7,6 +7,8 @@ RSpec.describe Cron::PurgeOldBrouillonDossiersJob, type: :job do let!(:old_brouillon) { travel_to(5.months.ago) { create(:dossier, :brouillon, procedure: procedure) } } let!(:very_old_brouillon) { travel_to(6.months.ago) { create(:dossier, :brouillon, procedure: procedure) } } let!(:old_en_construction) { travel_to(5.months.ago) { create(:dossier, :en_construction, procedure: procedure) } } + let!(:not_visible_dossier) { travel_to(6.months.ago) { create(:dossier, :brouillon, :hidden_by_user, procedure: procedure) } } + let!(:not_visible_dossier2) { travel_to(6.months.ago) { create(:dossier, :brouillon, :hidden_by_expired, procedure: procedure) } } subject(:perform_job) { described_class.perform_now } @@ -28,6 +30,8 @@ RSpec.describe Cron::PurgeOldBrouillonDossiersJob, type: :job do expect(DossierMailer).to have_received(:notify_old_brouillon_after_deletion).with(very_old_brouillon).once expect(DossierMailer).not_to have_received(:notify_old_brouillon_after_deletion).with(recent_brouillon) expect(DossierMailer).not_to have_received(:notify_old_brouillon_after_deletion).with(old_en_construction) + expect(DossierMailer).not_to have_received(:notify_old_brouillon_after_deletion).with(not_visible_dossier) + expect(DossierMailer).not_to have_received(:notify_old_brouillon_after_deletion).with(not_visible_dossier2) end it 'sets the correct hidden_by attributes' do From 760963a52d75f47761115f1ecdfd6345aca9074b Mon Sep 17 00:00:00 2001 From: Mathieu Magnin Date: Fri, 6 Dec 2024 16:33:33 +0100 Subject: [PATCH 2/4] Do not hide not notified dossier --- app/jobs/cron/purge_old_brouillon_dossiers_job.rb | 2 +- .../cron/purge_old_brouillon_dossiers_job_spec.rb | 15 +++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/app/jobs/cron/purge_old_brouillon_dossiers_job.rb b/app/jobs/cron/purge_old_brouillon_dossiers_job.rb index a53da2910..a70e17cdc 100644 --- a/app/jobs/cron/purge_old_brouillon_dossiers_job.rb +++ b/app/jobs/cron/purge_old_brouillon_dossiers_job.rb @@ -7,7 +7,7 @@ class Cron::PurgeOldBrouillonDossiersJob < Cron::CronJob Dossier .visible_by_user .state_brouillon - .where(updated_at: ..(3.months + 2.weeks).ago) + .where(updated_at: ..(3.months + 2.weeks).ago, notified_soon_deleted_sent_at: ..2.weeks.ago) .find_each do |dossier| dossier.hide_and_keep_track!(:automatic, :not_modified_for_a_long_time) DossierMailer.notify_old_brouillon_after_deletion(dossier).deliver_later diff --git a/spec/jobs/cron/purge_old_brouillon_dossiers_job_spec.rb b/spec/jobs/cron/purge_old_brouillon_dossiers_job_spec.rb index e2b9c535b..90255dc70 100644 --- a/spec/jobs/cron/purge_old_brouillon_dossiers_job_spec.rb +++ b/spec/jobs/cron/purge_old_brouillon_dossiers_job_spec.rb @@ -3,12 +3,13 @@ RSpec.describe Cron::PurgeOldBrouillonDossiersJob, type: :job do let(:procedure) { create(:procedure) } - let!(:recent_brouillon) { travel_to(3.months.ago) { create(:dossier, :brouillon, procedure: procedure) } } - let!(:old_brouillon) { travel_to(5.months.ago) { create(:dossier, :brouillon, procedure: procedure) } } - let!(:very_old_brouillon) { travel_to(6.months.ago) { create(:dossier, :brouillon, procedure: procedure) } } - let!(:old_en_construction) { travel_to(5.months.ago) { create(:dossier, :en_construction, procedure: procedure) } } - let!(:not_visible_dossier) { travel_to(6.months.ago) { create(:dossier, :brouillon, :hidden_by_user, procedure: procedure) } } - let!(:not_visible_dossier2) { travel_to(6.months.ago) { create(:dossier, :brouillon, :hidden_by_expired, procedure: procedure) } } + let!(:recent_brouillon) { travel_to(3.months.ago) { create(:dossier, :brouillon, procedure: procedure, notified_soon_deleted_sent_at: 3.weeks.ago) } } + let!(:old_brouillon) { travel_to(5.months.ago) { create(:dossier, :brouillon, procedure: procedure, notified_soon_deleted_sent_at: 3.weeks.ago) } } + let!(:very_old_brouillon) { travel_to(6.months.ago) { create(:dossier, :brouillon, procedure: procedure, notified_soon_deleted_sent_at: 3.weeks.ago) } } + let!(:very_old_brouillon_but_not_notified) { travel_to(6.months.ago) { create(:dossier, :brouillon, procedure: procedure, notified_soon_deleted_sent_at: nil) } } + let!(:old_en_construction) { travel_to(5.months.ago) { create(:dossier, :en_construction, procedure: procedure, notified_soon_deleted_sent_at: 3.weeks.ago) } } + let!(:not_visible_dossier) { travel_to(6.months.ago) { create(:dossier, :brouillon, :hidden_by_user, procedure: procedure, notified_soon_deleted_sent_at: 3.weeks.ago) } } + let!(:not_visible_dossier2) { travel_to(6.months.ago) { create(:dossier, :brouillon, :hidden_by_expired, procedure: procedure, notified_soon_deleted_sent_at: 3.weeks.ago) } } subject(:perform_job) { described_class.perform_now } @@ -19,8 +20,6 @@ RSpec.describe Cron::PurgeOldBrouillonDossiersJob, type: :job do it 'hides only old brouillon dossiers' do expect { perform_job }.to change { Dossier.visible_by_user.count }.by(-2) - - expect(Dossier.visible_by_user.pluck(:id)).to match_array([recent_brouillon.id, old_en_construction.id]) end it 'sends notification emails for each hidden dossier' do From 5555da2f902a9dcd28afc03568ffb74e08a6318d Mon Sep 17 00:00:00 2001 From: Mathieu Magnin Date: Fri, 6 Dec 2024 16:34:21 +0100 Subject: [PATCH 3/4] Rename purge => hide --- ...illon_dossiers_job.rb => hide_old_brouillon_dossiers_job.rb} | 2 +- ...iers_job_spec.rb => hide_old_brouillon_dossiers_job_spec.rb} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename app/jobs/cron/{purge_old_brouillon_dossiers_job.rb => hide_old_brouillon_dossiers_job.rb} (89%) rename spec/jobs/cron/{purge_old_brouillon_dossiers_job_spec.rb => hide_old_brouillon_dossiers_job_spec.rb} (97%) diff --git a/app/jobs/cron/purge_old_brouillon_dossiers_job.rb b/app/jobs/cron/hide_old_brouillon_dossiers_job.rb similarity index 89% rename from app/jobs/cron/purge_old_brouillon_dossiers_job.rb rename to app/jobs/cron/hide_old_brouillon_dossiers_job.rb index a70e17cdc..aba3a29a9 100644 --- a/app/jobs/cron/purge_old_brouillon_dossiers_job.rb +++ b/app/jobs/cron/hide_old_brouillon_dossiers_job.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class Cron::PurgeOldBrouillonDossiersJob < Cron::CronJob +class Cron::HideOldBrouillonDossiersJob < Cron::CronJob self.schedule_expression = "every day at 5:30" def perform diff --git a/spec/jobs/cron/purge_old_brouillon_dossiers_job_spec.rb b/spec/jobs/cron/hide_old_brouillon_dossiers_job_spec.rb similarity index 97% rename from spec/jobs/cron/purge_old_brouillon_dossiers_job_spec.rb rename to spec/jobs/cron/hide_old_brouillon_dossiers_job_spec.rb index 90255dc70..de52344a3 100644 --- a/spec/jobs/cron/purge_old_brouillon_dossiers_job_spec.rb +++ b/spec/jobs/cron/hide_old_brouillon_dossiers_job_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe Cron::PurgeOldBrouillonDossiersJob, type: :job do +RSpec.describe Cron::HideOldBrouillonDossiersJob, type: :job do let(:procedure) { create(:procedure) } let!(:recent_brouillon) { travel_to(3.months.ago) { create(:dossier, :brouillon, procedure: procedure, notified_soon_deleted_sent_at: 3.weeks.ago) } } From bfe138fe1a4c1a9b6cd648ab25b6b3bb75b75771 Mon Sep 17 00:00:00 2001 From: Mathieu Magnin Date: Fri, 6 Dec 2024 17:07:31 +0100 Subject: [PATCH 4/4] Change cron hour to avoid race condition with NotifyOldBrouillonDossiersSoonDeletedJob --- app/jobs/cron/hide_old_brouillon_dossiers_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/jobs/cron/hide_old_brouillon_dossiers_job.rb b/app/jobs/cron/hide_old_brouillon_dossiers_job.rb index aba3a29a9..9ffb87026 100644 --- a/app/jobs/cron/hide_old_brouillon_dossiers_job.rb +++ b/app/jobs/cron/hide_old_brouillon_dossiers_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Cron::HideOldBrouillonDossiersJob < Cron::CronJob - self.schedule_expression = "every day at 5:30" + self.schedule_expression = "every day at 21:00" def perform Dossier