From 155fa00305fc8e1a4466e5be56da9141a913841b Mon Sep 17 00:00:00 2001
From: Rosa Gutierrez <rosa@37signals.com>
Date: Mon, 21 Jul 2025 21:11:12 +0200
Subject: [PATCH] Set `successfully_enqueued` in `Job.enqueue`

Even though this will be overwritten by Active Job when called through
`perform_later`.

This is necessary for recurring tasks, where we run the `enqueue`
callbacks manually and call `Job.enqueue`. We rely on `successfully_enqueued?`
to know whether we need to record the recurring execution. We were setting
that no matter what after calling `Job.enqueue` because we were relying on
that to raise (which would be the case for Active Record errors).

In the case of discarding it because of concurrency controls, the job is simply
not persisted but no error is raised, which means we were trying to
record a recurring execution without a persisted job, which would raise.

Fixes #598
---
 app/models/solid_queue/job.rb                 |  1 +
 app/models/solid_queue/recurring_task.rb      |  1 -
 .../models/solid_queue/recurring_task_test.rb | 34 ++++++++++++++++---
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/app/models/solid_queue/job.rb b/app/models/solid_queue/job.rb
index 234e5eb4..6a6a6fa3 100644
--- a/app/models/solid_queue/job.rb
+++ b/app/models/solid_queue/job.rb
@@ -30,6 +30,7 @@ def enqueue(active_job, scheduled_at: Time.current)
 
         create_from_active_job(active_job).tap do |enqueued_job|
           active_job.provider_job_id = enqueued_job.id if enqueued_job.persisted?
+          active_job.successfully_enqueued = enqueued_job.persisted?
         end
       end
 
diff --git a/app/models/solid_queue/recurring_task.rb b/app/models/solid_queue/recurring_task.rb
index 5363f0a7..b5ba67fd 100644
--- a/app/models/solid_queue/recurring_task.rb
+++ b/app/models/solid_queue/recurring_task.rb
@@ -130,7 +130,6 @@ def enqueue_and_record(run_at:)
             active_job.run_callbacks(:enqueue) do
               Job.enqueue(active_job)
             end
-            active_job.successfully_enqueued = true
           end
         end
       end
diff --git a/test/models/solid_queue/recurring_task_test.rb b/test/models/solid_queue/recurring_task_test.rb
index 2f65cb01..ec31dd69 100644
--- a/test/models/solid_queue/recurring_task_test.rb
+++ b/test/models/solid_queue/recurring_task_test.rb
@@ -41,6 +41,14 @@ def perform
     end
   end
 
+  class JobWithConcurrencyControlsAndDiscard < ApplicationJob
+    limits_concurrency key: -> { true }, on_conflict: :discard
+
+    def perform
+      JobBuffer.add "job_with_concurrency_controls"
+    end
+  end
+
   test "job without arguments" do
     task = recurring_task_with(class_name: "JobWithoutArguments")
     enqueue_and_assert_performed_with_result task, "job_without_arguments"
@@ -88,6 +96,20 @@ def perform
     end
   end
 
+  test "error when enqueuing job because of concurrency controls and discard" do
+    JobWithConcurrencyControlsAndDiscard.perform_later
+
+    task = recurring_task_with(class_name: "JobWithConcurrencyControlsAndDiscard")
+    assert_no_difference -> { SolidQueue::Job.count } do
+      task.enqueue(at: Time.now)
+    end
+
+    # Run the enqueued job that was preventing a new one from being enqueued
+    run_all_jobs_inline
+
+    enqueue_and_assert_performed_with_result task, "job_with_concurrency_controls"
+  end
+
   test "error when enqueuing job using another adapter that raises ActiveJob::EnqueueError" do
     ActiveJob::QueueAdapters::AsyncAdapter.any_instance.stubs(:enqueue).raises(ActiveJob::EnqueueError)
     previous_size = JobBuffer.size
@@ -174,10 +196,7 @@ def enqueue_and_assert_performed_with_result(task, result)
       end
 
       assert_difference -> { JobBuffer.size }, +1 do
-        SolidQueue::Worker.new(queues: "*").tap do |worker|
-          worker.mode = :inline
-          worker.start
-        end
+        run_all_jobs_inline
       end
 
       assert_equal result, JobBuffer.last_value
@@ -192,4 +211,11 @@ def recurring_task_with(class_name: nil, **options)
 
       SolidQueue::RecurringTask.from_configuration("task-id", **options)
     end
+
+    def run_all_jobs_inline
+      SolidQueue::Worker.new(queues: "*").tap do |worker|
+        worker.mode = :inline
+        worker.start
+      end
+    end
 end