[5.7] When scheduling a job, respect the already set queue

[petervmeijgaard]

Problem description

When creating a job, we sometimes want to define in which queue the job should run.
Sometimes that's something that we want to define in the job itself.

When creating a job, we often specify which queue it should be stored at in the constructor:

// MyCustomJob-class

public function __construct()
{
    $this->onQueue('my-custom-queue');
}

But when scheduling this job, we need to specify a queue in which the job should run.

// Will add the job to the 'default'-queue.
$schedule->job(MyCustomJob::class)->everyMinute();

// Will add the job to the 'my-custom-queue'-queue.
$schedule->job(MyCustomJob::class, 'my-custom-queue')->everyMinute();

This results in having to specify the queue multiple times instead of just once.

Proposed solution

It would be a good solution to respect the already defined queue on the custom job class.
Using the example above, it'll result in a solution something like this:

// MyCustomJob-class

public function __construct()
{
    $this->onQueue('my-custom-queue');
}

// Will add the job to the 'my-custom-queue'-queue.
$schedule->job(MyCustomJob::class)->everyMinute();

// Will add the job to the 'my-other-queue'-queue.
$schedule->job(MyCustomJob::class, 'my-other-queue')->everyMinute();

// Will still add the job to the 'default'-queue.
$schedule->job(MyCustomJob::class, 'default')->everyMinute();

[sisve]

Could you add a test for this?

[petervmeijgaard]

Hi @sisve,

I tried to, but I am struggling to fire a scheduled job in my unit test.
If you have the time, could you take a look at what I may am missing?
https://github.com/laravel/framework/pull/25159/files#diff-977603a2c6bc453a498ed4a1b18ec275R39

[X-Coder264]

@petervmeijgaard I didn't try this, but off the top of my head...

$events = $scheduler->events();
foreach ($events as $event) {
    $event->run($container);
}

[taylorotwell]

The code doesn't even look correct anymore? Your original PR code looked correct. I don't know why you are setting $queue to a booelan now?

[petervmeijgaard]

Oh my apologies, I was tinkering around with trying different approaches by setting the queue different ways. What I intended to do was $queue = $queue !== null ? $queue : $job->queue.

Coming from a JavaScript perspective, I thought that the code above would result in $queue = $queue || $job->queue, but I was mistaken. I've reverted the code change.

[deleugpn]

$queue = $queue ?? $job->queue;

[sisve]

Is an empty string a valid queue name?

[sisve]

Never mind, I had a brain-fart. I was thinking of the ?: operator. ?? seem to work as expected.

[taylorotwell]

Tests fail. Please re-submit with working tests.