Merge pull request #8723 from colinux/fix-failing-jobs

Attempts to fix timeout and errors cron jobs

app/jobs/cron/fix_missing_antivirus_analysis_job.rb

@@ -3,10 +3,7 @@ class Cron::FixMissingAntivirusAnalysisJob < Cron::CronJob
 
   def perform
     ActiveStorage::Blob.where(virus_scan_result: ActiveStorage::VirusScanner::PENDING).find_each do |blob|
-      begin
-        VirusScannerJob.perform_now(blob)
-      rescue ActiveStorage::IntegrityError
-      end
+      VirusScannerJob.perform_later(blob)
     end
   end
 end

app/jobs/cron/purge_unattached_blobs_job.rb

@@ -1,9 +1,16 @@
 class Cron::PurgeUnattachedBlobsJob < Cron::CronJob
   self.schedule_expression = "every day at midnight"
 
-  def perform(*args)
-    ActiveStorage::Blob.unattached
-      .where('active_storage_blobs.created_at < ?', 24.hours.ago)
-      .find_each(&:purge_later)
+  def perform
+    # .in_batches { _1.each... } is more efficient in this case that in_batches.each_record or find_each
+    # because it plucks only ids in a preliminary query, then load records with selected columns in batches by ids.
+    # This is faster than other batch strategies, which load at once selected columns with an ORDER BY in the same query, triggering timeouts.
+    ActiveStorage::Blob.unattached.select(:id, :service_name, :created_at).in_batches do |relation|
+      relation.each do |blob|
+        return if blob.created_at > 24.hours.ago # not in where() because it's not an indexed column
+
+        blob.purge_later
+      end
+    end
   end
 end

app/jobs/cron/weekly_overview_job.rb

@@ -1,13 +1,16 @@
 class Cron::WeeklyOverviewJob < Cron::CronJob
   self.schedule_expression = "every monday at 7 am"
 
-  def perform(*args)
+  def perform
     # Feature flipped to avoid mails in staging due to unprocessed dossier
-    if Rails.application.config.ds_weekly_overview
-      Instructeur.all
-        .map { |instructeur| [instructeur, instructeur.last_week_overview] }
-        .reject { |_, overview| overview.nil? }
-        .each { |instructeur, _| InstructeurMailer.last_week_overview(instructeur).deliver_later }
+    return unless Rails.application.config.ds_weekly_overview
+
+    Instructeur.find_each do |instructeur|
+      # NOTE: it's not exactly accurate because rate limit is not shared between jobs processes
+      Dolist::API.sleep_until_limit_reset if Dolist::API.near_rate_limit?
+
+      # mailer won't send anything if overview if empty
+      InstructeurMailer.last_week_overview(instructeur)&.deliver_later
     end
   end
 end

app/services/expired_dossiers_deletion_service.rb

@@ -21,7 +21,7 @@ class ExpiredDossiersDeletionService
 
     user_notifications = group_by_user_email(dossiers_close_to_expiration)
 
-    dossiers_close_to_expiration.update_all(brouillon_close_to_expiration_notice_sent_at: Time.zone.now)
+    dossiers_close_to_expiration.in_batches.update_all(brouillon_close_to_expiration_notice_sent_at: Time.zone.now)
 
     user_notifications.each do |(email, dossiers)|
       DossierMailer.notify_brouillon_near_deletion(

lib/tasks/deployment/20230302161322_fix_again_hidden_by_reason_nil.rake

@@ -0,0 +1,14 @@
+namespace :after_party do
+  desc 'Deployment task: fix_again_hidden_by_reason_nil'
+  task fix_again_hidden_by_reason_nil: :environment do
+    puts "Running deploy task 'fix_again_hidden_by_reason_nil'"
+
+    Dossier.en_construction_expired_to_delete.where(hidden_by_reason: nil).update_all(hidden_by_reason: :user_request)
+    Dossier.termine_expired_to_delete.where(hidden_by_reason: nil).update_all(hidden_by_reason: :user_request)
+
+    # 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

spec/jobs/cron/weekly_overview_job_spec.rb

@@ -2,7 +2,6 @@ RSpec.describe Cron::WeeklyOverviewJob, type: :job do
   describe 'perform' do
     let!(:instructeur) { create(:instructeur) }
     let(:overview) { double('overview') }
-    let(:mailer_double) { double('mailer', deliver_later: true) }
 
     context 'if the feature is enabled' do
       before do
@@ -12,11 +11,14 @@ RSpec.describe Cron::WeeklyOverviewJob, type: :job do
         Rails.application.config.ds_weekly_overview = false
       end
 
+      subject(:run_job) { Cron::WeeklyOverviewJob.new.perform }
+      # See also spec/mailers/instructeur_mailer_spec.rb
+
       context 'with one instructeur with one overview' do
+        let(:mailer_double) { double('mailer', deliver_later: true) }
         before do
-          expect_any_instance_of(Instructeur).to receive(:last_week_overview).and_return(overview)
           allow(InstructeurMailer).to receive(:last_week_overview).and_return(mailer_double)
-          Cron::WeeklyOverviewJob.new.perform
+          run_job
         end
 
         it { expect(InstructeurMailer).to have_received(:last_week_overview).with(instructeur) }
@@ -25,22 +27,22 @@ RSpec.describe Cron::WeeklyOverviewJob, type: :job do
 
       context 'with one instructeur with no overviews' do
         before do
-          expect_any_instance_of(Instructeur).to receive(:last_week_overview).and_return(nil)
-          allow(InstructeurMailer).to receive(:last_week_overview)
-          Cron::WeeklyOverviewJob.new.perform
+          allow(InstructeurMailer).to receive(:last_week_overview).and_return(nil)
+          run_job
         end
 
-        it { expect(InstructeurMailer).not_to have_received(:last_week_overview) }
+        it { expect(InstructeurMailer).to have_received(:last_week_overview).with(instructeur) }
+        it { expect { run_job }.not_to raise_error(NoMethodError) }
       end
     end
 
     context 'if the feature is disabled' do
       before do
-        allow(Instructeur).to receive(:all)
+        allow(Instructeur).to receive(:find_each)
         Cron::WeeklyOverviewJob.new.perform
       end
 
-      it { expect(Instructeur).not_to receive(:all) }
+      it { expect(Instructeur).not_to receive(:find_each) }
     end
   end
 end