[12.x] Fixes for cron expressions when using `between()`
[!NOTE]
As part of #50670 I was asked by @driesvints to reattempt my previous PR (https://github.com/laravel/framework/pull/47983) for Laravel 12, so here we go! PR body is the same so I've copied it over
Hey,
I noticed something weird with some of the expressions of my scheduled commands when I run run php artisan schedule list, so I decided to do some digging. It seems like if you're using ->between(), this doesn't get included in the cron expression that shows.
For example, let's take a scheduled command that's meant to run every minute between 10pm and 11pm:
$schedule->command('example')
->everyMinute()
->timezone('BST')
->between('22:00', '23:00');
If I run php artisan schedule:list before 10pm, for example at the time of typing this, it's 21:21, I get:
* * * * * php artisan example ............................... Next Due: 20 seconds from now
But that's not correct, because it's actually next due in 36 minutes. With the changes in this PR, if I run php artisan schedule:list again, I get:
* 22-23 * * * php artisan example ........................ Next Due: 36 minutes from now
I also noticed that if you try and be more specific about the minutes you want to run, e.g.:
$schedule->command('example')
->everyMinute()
->timezone('BST')
->between('22:30', '23:00');
You get the same as before, where it will give me the wrong expression and count down:
* * * * * php artisan example ............................... Next Due: 20 seconds from now
But with this PR, it changes it to the below, which includes the hours and minutes.
30-59 22-23 * * * php artisan example ........................ Next Due: 1 hour from now
I've also amended the output to show if, for example, you've specified it to run every 2 minutes, 10 minutes etc...:
30-35/2 22-23 * * * php artisan example ..................... Next Due: 1 hour from now
I haven't really delved into this part of the code in the framework before so I'm definitely open to being educated 😄
I've added some supporting tests to cover this new behaviour.
Thanks!
Hey @driesvints 👋
Apologies it took me so long to get round to this, I actually had this issue in some more recent work I was doing and then remembered about this! You asked if I would be up for creating this again the L12 branch as a part of https://github.com/laravel/framework/pull/47983, so here we go 😄
From a previous attempt:
For example, if an event is scheduled every minute from 1:30-2:30, there is no way to express this in a cron expression.
Please mark as ready for review if you want me to take a look.
@taylorotwell I'm gonna leave this as a draft for now, I want to do some more testing with some more timings
Hey @taylorotwell
Going to mark this for review as I believe these changes so far bring benefit, but need to have a further discussion regarding the below (I'm also not sure you get notifications for draft PR's):
For example, if an event is scheduled every minute from 1:30-2:30, there is no way to express this in a cron expression.
That is correct, there's no way to express this is crontab, the only thing I can think of doing is making the framework able to split something like this into two lines when you run php artisan schedule:list, so taking that example above, the output could be:
30-59 1 * * * php artisan inspire ...................... Next Due: 1 hour from now
0-29 2 * * * php artisan inspire ....................... Next Due: 1 hour from now
instead of
* 01-02 * * * php artisan inspire .................. Next Due: 39 minutes from now
I'm not 100% how I'd go about doing the above, it's merely just a thought at this point.
Yeah, I'm not really sure - I'm not sure trying to convert to cron expressions is the best way to solve this "Next Due" problem.
I'm going to go back to the drawing board on this one and try and come up with a new way of handling the issue. I'll close this PR for now so it's not just sat there!