GoodJob.cleanup_preserved_jobs: add :include_discarded option (#1550)

This change allows the caller of GoodJob.cleanup_preserved_jobs to
specify the :include_discarded option, rather than always relying on the
value that is set in the configuration.

This would enable us to e.g. set
config.cleanup_preserved_jobs_before_seconds_ago to a very low value
(e.g. 10 seconds), but also set config.cleanup_discarded_jobs = false so
that we can retain discarded jobs for longer in case we want to inspect
them or manually retry then.

We would then set up a separate maintenance job to call
GoodJob.cleanup_preserved_jobs(older_than: some_longer_period,
include_discarded: true).

Co-authored-by: Ben Sheldon [he/him] <[email protected]>

lib/good_job.rb

@@ -203,11 +203,11 @@ def self._shutdown_all(executables, method_name = :shutdown, timeout: -1, after:
   # If you are preserving job records this way, use this method regularly to
   # destroy old records and preserve space in your database.
   # @param older_than [nil,Numeric,ActiveSupport::Duration] Jobs older than this will be destroyed (default: +86400+).
+  # @param include_discarded [Boolean] Whether or not to destroy discarded jobs (default: per +cleanup_discarded_jobs+ config option)
   # @return [Integer] Number of job execution records and batches that were destroyed.
-  def self.cleanup_preserved_jobs(older_than: nil, in_batches_of: 1_000)
+  def self.cleanup_preserved_jobs(older_than: nil, in_batches_of: 1_000, include_discarded: GoodJob.configuration.cleanup_discarded_jobs?)
     older_than ||= GoodJob.configuration.cleanup_preserved_jobs_before_seconds_ago
     timestamp = Time.current - older_than
-    include_discarded = GoodJob.configuration.cleanup_discarded_jobs?
 
     ActiveSupport::Notifications.instrument("cleanup_preserved_jobs.good_job", { older_than: older_than, timestamp: timestamp }) do |payload|
       deleted_jobs_count = 0

spec/lib/good_job_spec.rb

@@ -111,6 +111,20 @@
       expect { old_batch.reload }.to raise_error ActiveRecord::RecordNotFound
     end
 
+    it "can override cleanup_discarded_jobs? configuration" do
+      allow(described_class.configuration).to receive(:env).and_return ENV.to_hash.merge({ 'GOOD_JOB_CLEANUP_DISCARDED_JOBS' => 'false' })
+      destroyed_records_count = described_class.cleanup_preserved_jobs(include_discarded: true)
+
+      expect(destroyed_records_count).to eq 4
+
+      expect { recent_job.reload }.not_to raise_error
+      expect { old_unfinished_job.reload }.not_to raise_error
+      expect { old_finished_job.reload }.to raise_error ActiveRecord::RecordNotFound
+      expect { old_finished_job_execution.reload }.to raise_error ActiveRecord::RecordNotFound
+      expect { old_discarded_job.reload }.to raise_error ActiveRecord::RecordNotFound
+      expect { old_batch.reload }.to raise_error ActiveRecord::RecordNotFound
+    end
+
     it "does not delete batches until their callbacks have finished" do
       old_batch.update!(finished_at: nil)
       described_class.cleanup_preserved_jobs