From 72ef163e4a95a675cc1e00ba8f92f58a400ca535 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Wojciech=20Wn=C4=99trzak?= <w.wnetrzak@gmail.com>
Date: Thu, 17 Sep 2020 09:36:34 +0200
Subject: [PATCH] Remove arguments from perform method

Follow up to https://github.com/bensheldon/good_job/issues/136#issuecomment-693451491
---
 lib/good_job/job.rb           | 20 ++++++--------------
 spec/lib/good_job/job_spec.rb | 32 ++++++++++++++++++++++++++------
 2 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/lib/good_job/job.rb b/lib/good_job/job.rb
index 53fa0131b..9d4fd8796 100644
--- a/lib/good_job/job.rb
+++ b/lib/good_job/job.rb
@@ -177,25 +177,17 @@ def self.enqueue(active_job, scheduled_at: nil, create_with_advisory_lock: false
     end
 
     # Execute the ActiveJob job this {Job} represents.
-    # @param destroy_after [Boolean]
-    #   Whether to destroy the {Job} record after executing it if the job did
-    #   not need to be reperformed. Defaults to the value of
-    #   {GoodJob.preserve_job_records}.
-    # @param reperform_on_standard_error [Boolean]
-    #   Whether to re-queue the job to execute again if it raised an instance
-    #   of +StandardError+. Defaults to the value of
-    #   {GoodJob.reperform_jobs_on_standard_error}.
     # @return [Array<(Object, Exception)>]
     #   An array of the return value of the job's +#perform+ method and the
     #   exception raised by the job, if any. If the job completed successfully,
     #   the second array entry (the exception) will be +nil+ and vice versa.
-    def perform(destroy_after: !GoodJob.preserve_job_records, reperform_on_standard_error: GoodJob.reperform_jobs_on_standard_error)
+    def perform
       raise PreviouslyPerformedError, 'Cannot perform a job that has already been performed' if finished_at
 
       GoodJob::CurrentExecution.reset
 
       self.performed_at = Time.current
-      save! unless destroy_after
+      save! if GoodJob.preserve_job_records
 
       result, rescued_error = execute
 
@@ -215,15 +207,15 @@ def perform(destroy_after: !GoodJob.preserve_job_records, reperform_on_standard_
       error_message = "#{error.class}: #{error.message}" if error
       self.error = error_message
 
-      if rescued_error && reperform_on_standard_error
+      if rescued_error && GoodJob.reperform_jobs_on_standard_error
         save!
       else
         self.finished_at = Time.current
 
-        if destroy_after
-          destroy!
-        else
+        if GoodJob.preserve_job_records
           save!
+        else
+          destroy!
         end
       end
 
diff --git a/spec/lib/good_job/job_spec.rb b/spec/lib/good_job/job_spec.rb
index 5912947c3..80bb517ec 100644
--- a/spec/lib/good_job/job_spec.rb
+++ b/spec/lib/good_job/job_spec.rb
@@ -202,7 +202,9 @@ def perform(result_value = nil, raise_error: false)
     end
 
     it 'can preserve the job' do
-      good_job.perform(destroy_after: false)
+      allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+
+      good_job.perform
       expect(good_job.reload).to have_attributes(
         performed_at: within(1.second).of(Time.current),
         finished_at: within(1.second).of(Time.current)
@@ -235,7 +237,9 @@ def perform(result_value = nil, raise_error: false)
       end
 
       it 'can preserve the job' do
-        good_job.perform(destroy_after: false)
+        allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+
+        good_job.perform
 
         expect(good_job.reload).to have_attributes(
           error: "ExpectedError: Raised expected error",
@@ -257,8 +261,14 @@ def perform(result_value = nil, raise_error: false)
 
       describe 'GoodJob.reperform_jobs_on_standard_error behavior' do
         context 'when true' do
+          before do
+            allow(GoodJob).to receive(:reperform_jobs_on_standard_error).and_return(true)
+          end
+
           it 'leaves the job record unfinished' do
-            good_job.perform(destroy_after: false)
+            allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+
+            good_job.perform
 
             expect(good_job.reload).to have_attributes(
               error: "ExpectedError: Raised expected error",
@@ -268,19 +278,29 @@ def perform(result_value = nil, raise_error: false)
           end
 
           it 'does not destroy the job record' do
-            good_job.perform(destroy_after: true)
+            allow(GoodJob).to receive(:preserve_job_records).and_return(false)
+
+            good_job.perform
             expect { good_job.reload }.not_to raise_error
           end
         end
 
         context 'when false' do
+          before do
+            allow(GoodJob).to receive(:reperform_jobs_on_standard_error).and_return(false)
+          end
+
           it 'destroys the job' do
-            good_job.perform(destroy_after: true, reperform_on_standard_error: false)
+            allow(GoodJob).to receive(:preserve_job_records).and_return(false)
+
+            good_job.perform
             expect { good_job.reload }.to raise_error ActiveRecord::RecordNotFound
           end
 
           it 'can preserve the job' do
-            good_job.perform(destroy_after: false, reperform_on_standard_error: false)
+            allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+
+            good_job.perform
 
             expect(good_job.reload).to have_attributes(
               error: "ExpectedError: Raised expected error",