Revert "When not preserving job records, ensure all prior executions …

…are deleted after successful retry (#719)" (#729)

This reverts commit f726bfb.

app/models/good_job/execution.rb

@@ -66,7 +66,6 @@ def self.queue_parser(string)
     end
 
     belongs_to :job, class_name: 'GoodJob::Job', foreign_key: 'active_job_id', primary_key: 'active_job_id', optional: true, inverse_of: :executions
-    after_destroy -> { self.class.active_job_id(active_job_id).delete_all }, if: -> { @_destroy_job }
 
     # Get executions with given ActiveJob ID
     # @!method active_job_id
@@ -299,11 +298,11 @@ def perform
 
       if result.unhandled_error && GoodJob.retry_on_unhandled_error
         save!
-      elsif GoodJob.preserve_job_records == true || result.retried? || (result.unhandled_error && GoodJob.preserve_job_records == :on_unhandled_error)
+      elsif GoodJob.preserve_job_records == true || (result.unhandled_error && GoodJob.preserve_job_records == :on_unhandled_error)
         self.finished_at = Time.current
         save!
       else
-        destroy_job
+        destroy!
       end
 
       result
@@ -355,14 +354,6 @@ def runtime_latency
       (finished_at || Time.zone.now) - performed_at if performed_at
     end
 
-    # Destroys this execution and all executions within the same job
-    def destroy_job
-      @_destroy_job = true
-      destroy!
-    ensure
-      @_destroy_job = false
-    end
-
     private
 
     def active_job_data
@@ -388,7 +379,7 @@ def execute
           end
           handled_error ||= current_thread.error_on_retry || current_thread.error_on_discard
 
-          ExecutionResult.new(value: value, handled_error: handled_error, retried: current_thread.error_on_retry.present?)
+          ExecutionResult.new(value: value, handled_error: handled_error)
         rescue StandardError => e
           ExecutionResult.new(value: nil, unhandled_error: e)
         end

app/models/good_job/execution_result.rb

@@ -8,18 +8,14 @@ class ExecutionResult
     attr_reader :handled_error
     # @return [Exception, nil]
     attr_reader :unhandled_error
-    # @return [Exception, nil]
-    attr_reader :retried
-    alias retried? retried
 
     # @param value [Object, nil]
     # @param handled_error [Exception, nil]
     # @param unhandled_error [Exception, nil]
-    def initialize(value:, handled_error: nil, unhandled_error: nil, retried: false)
+    def initialize(value:, handled_error: nil, unhandled_error: nil)
       @value = value
       @handled_error = handled_error
       @unhandled_error = unhandled_error
-      @retried = retried
     end
   end
 end

lib/good_job.rb

@@ -57,7 +57,7 @@ module GoodJob
   #   By default, GoodJob deletes job records after the job is completed successfully.
   #   If you want to preserve jobs for latter inspection, set this to +true+.
   #   If you want to preserve only jobs that finished with error for latter inspection, set this to +:on_unhandled_error+.
-  #   @return [Boolean, Symbol, nil]
+  #   @return [Boolean, nil]
   mattr_accessor :preserve_job_records, default: true
 
   # @!attribute [rw] retry_on_unhandled_error

spec/app/models/good_job/execution_spec.rb

@@ -397,25 +397,6 @@ def job_params
       expect { good_job.reload }.to raise_error ActiveRecord::RecordNotFound
     end
 
-    context 'when there are prior executions' do
-      let!(:prior_execution) do
-        described_class.enqueue(active_job).tap do |job|
-          job.update!(
-            error: "TestJob::ExpectedError: Raised expected error",
-            performed_at: Time.current,
-            finished_at: Time.current
-          )
-        end
-      end
-
-      it 'destroys the job and prior executions when preserving record only on error' do
-        allow(GoodJob).to receive(:preserve_job_records).and_return(:on_unhandled_error)
-        good_job.perform
-        expect { good_job.reload }.to raise_error ActiveRecord::RecordNotFound
-        expect { prior_execution.reload }.to raise_error ActiveRecord::RecordNotFound
-      end
-    end
-
     it 'raises an error if the job is attempted to be re-run' do
       good_job.update!(finished_at: Time.current)
       expect { good_job.perform }.to raise_error described_class::PreviouslyPerformedError
@@ -523,19 +504,6 @@ def job_params
     end
   end
 
-  describe '#destroy_job' do
-    let!(:execution) { described_class.create! active_job_id: SecureRandom.uuid }
-    let!(:prior_execution) { described_class.create! active_job_id: execution.active_job_id }
-    let!(:other_job) { described_class.create! active_job_id: SecureRandom.uuid }
-
-    it 'destroys all of the job executions' do
-      execution.destroy_job
-      expect { execution.reload }.to raise_error ActiveRecord::RecordNotFound
-      expect { prior_execution.reload }.to raise_error ActiveRecord::RecordNotFound
-      expect { other_job.reload }.not_to raise_error
-    end
-  end
-
   describe '#queue_latency' do
     let(:execution) { described_class.create! }
 

spec/test_app/config/environments/test.rb

@@ -4,8 +4,8 @@
 # and recreated between test runs. Don't rely on the data there!
 
 Rails.application.configure do
-  config.cache_classes = true
-  config.eager_load = true
+  config.cache_classes = false
+  config.eager_load = false
 
   # Raises error for missing translations.
   if Gem::Version.new(Rails.version) < Gem::Version.new('6.1')