Set scheduled_at in enqueue_all

[toydestroyer]

👋

We've noticed inconsistent behavior between jobs enqueued normally and those enqueued in batches using ActiveJob.perform_all_later. When using batch enqueue, the job attribute scheduled_at is missing (unless wait or wait_until is used).

Besides being inconsistent, this causes a problem with Mission Control, as you can't view the details of a finished job due to this line: https://github.com/rails/mission_control-jobs/blob/7ac38357e33778099b8b76b5476bf36ad3b53bf2/lib/active_job/job_proxy.rb#L30, which throws the following exception:

can't convert NilClass into an exact number (TypeError)

      minus_without_duration(other)

This PR makes the behavior consistent and adds tests.

[rosa]

Good catch! I'd do the change differently, though, since that line looks out of place there. I'd add it on attributes_from_active_job, like this:

diff --git a/app/models/solid_queue/job.rb b/app/models/solid_queue/job.rb
index 6a6a6fa..37893c0 100644
--- a/app/models/solid_queue/job.rb
+++ b/app/models/solid_queue/job.rb
@@ -58,7 +58,7 @@ def attributes_from_active_job(active_job)
             queue_name: active_job.queue_name || DEFAULT_QUEUE_NAME,
             active_job_id: active_job.job_id,
             priority: active_job.priority || DEFAULT_PRIORITY,
-            scheduled_at: active_job.scheduled_at,
+            scheduled_at: active_job.scheduled_at || Time.current,
             class_name: active_job.class.name,
             arguments: active_job.serialize,
             concurrency_key: active_job.concurrency_key

It's consistent with other attributes where a default value is provided.

[toydestroyer]

I see and agree with that, but I was actually trying to make it consistent with how it's done in the enqueue method.

I wonder if it makes sense to remove it from there as well 🤔. But I don't see an easy way, because it still has to accept a timestamp when called from ActiveJob's enqueue_at.

As for attributes_from_active_job, although it works, that creates another inconsistency: it only sets the value for the scheduled_at column, but the scheduled_at in the attributes column remains null. However, that doesn't break Mission Control, so maybe it's less critical.

Please let me know what you think and how I should proceed.

[rosa]

I see and agree with that, but I was actually trying to make it consistent with how it's done in the enqueue method.

Ahhh, yes, you're right. It's a bit different because in that case, the scheduled_at value is provided as well. Active Job requires that method signature. This is not the first time I have encountered inconsistencies between enqueue and enqueue_all in Active Job, TBH.

Ok, then let's proceed with the change as it is now. Thank you!