Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.x] Fix schedule:list to display named Jobs #47367

Merged

Conversation

liamkeily
Copy link
Contributor

@liamkeily liamkeily commented Jun 7, 2023

The Issue

I have a project that uses named jobs in the app/Console/Kernel.php (similar to https://laravel.com/docs/10.x/scheduling#naming-unique-jobs)

class Kernel extends ConsoleKernel
{
    protected function schedule(Schedule $schedule): void
    {
        $schedule->job(new TestJob('hourly'))->name('test-job-hourly')->hourly();
        $schedule->job(new TestJob('daily'))->name('test-job-daily')->daily();
    }

The schedule:test and schedule:run commands display these correctly:

image image

However the schedule:list command does not:

image

This Fix

->getSummaryForDisplay() is used by the other commands but not this one, so i have utilised it here while attempting to maintain the extra functionality that was added for Closures and Callbacks.

image

Testing

Possible Simplifications

  • Share this logic via ->getSummaryForDisplay() so that all schedule commands (schedule:run, schedule:list, schedule:test) stay consistent.
  • Adapt $schedule->call so that it gives an accurate description from the start, and avoids us having to piece things back together here.

@liamkeily liamkeily changed the title Hotfix/schedule list named jobs Fix schedule:list to display named jobs Jun 7, 2023
@liamkeily liamkeily changed the title Fix schedule:list to display named jobs Fix schedule:list to display named Jobs Jun 7, 2023
@liamkeily liamkeily changed the title Fix schedule:list to display named Jobs [10.x] Fix schedule:list to display named Jobs Jun 7, 2023
@liamkeily
Copy link
Contributor Author

For context this is the value of getSummaryForDisplay in various scinerios`

Code

dd(
            $schedule->call(function() {})->everyMinute()->getSummaryForDisplay(),
            $schedule->job(function() {})->everyMinute()->getSummaryForDisplay(),
            $schedule->job(Jobs\MyTestJob::class)->everyMinute()->getSummaryForDisplay(),
            $schedule->job(new Jobs\MyTestJob)->everyMinute()->getSummaryForDisplay(),
            $schedule->command(MyTestCommand::class)->everyMinute()->getSummaryForDisplay(),
            $schedule->call(function() {})->everyMinute()->name('named-call')->getSummaryForDisplay(),
            $schedule->job(function() {})->everyMinute()->name('named-job')->getSummaryForDisplay(),
            $schedule->job(Jobs\MyTestJob::class)->everyMinute()->name('named-test-job')->getSummaryForDisplay(),
            $schedule->job(new Jobs\MyTestJob)->everyMinute()->name('named-test-job-instance')->getSummaryForDisplay(),
            $schedule->command(MyTestCommand::class)->name('named-command')->everyMinute()->getSummaryForDisplay(),
        );

Output

"Callback" // app/Console/Kernel.php:55
"Closure" // app/Console/Kernel.php:55
"App\Jobs\MyTestJob" // app/Console/Kernel.php:55
"App\Jobs\MyTestJob" // app/Console/Kernel.php:55
"Command description" // app/Console/Kernel.php:55
"named-call" // app/Console/Kernel.php:55
"named-job" // app/Console/Kernel.php:55
"named-test-job" // app/Console/Kernel.php:55
"named-test-job-instance" // app/Console/Kernel.php:55
"named-command" // app/Console/Kernel.php:55

@taylorotwell taylorotwell merged commit d06f3a9 into laravel:10.x Jun 7, 2023
@liamkeily liamkeily deleted the hotfix/schedule-list-named-jobs branch June 7, 2023 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants