cron-expression icon indicating copy to clipboard operation
cron-expression copied to clipboard

Offset from cron date

Open bilogic opened this issue 3 years ago • 11 comments

Hi,

  1. I need to collect on invoices every quarter, i.e. 1st of Jan/Apr/Jul/Oct.
  2. I would like to send out a reminder, say 7 days prior.
  3. I was thinking of adding shift() and dependency on https://github.com/briannesbitt/Carbon
$cron = new Cron\CronExpression('@quarterly');
echo $cron->shift('-7 days')->getNextRunDate(null, 2)->format('Y-m-d H:i:s');
  1. Would this be an acceptable PR?

bilogic avatar Jul 05 '21 06:07 bilogic

@dragonmantank would love to hear your opinions :)

bilogic avatar Jul 06 '21 08:07 bilogic

@DerekCresswell what do you think? Thanks.

bilogic avatar Jul 11 '21 17:07 bilogic

Sounds like an interesting addition to me. However, I don't think such a thing would be accepted. The maintainer of the repository considers it feature complete. So such a change is unlikely to be added. I would just say extend the class yourself and add this functionality to it.

DerekCresswell avatar Jul 11 '21 20:07 DerekCresswell

@DerekCresswell

Thanks, I mistook you as the maintainer as only your name appeared when I looked through the recent commits.

bilogic avatar Jul 12 '21 02:07 bilogic

@dragonmantank possible to say a yes or no? (preferrably yes 🤣)

bilogic avatar Sep 18 '21 14:09 bilogic

Thanks for the suggestion.

As @DerekCresswell mentioned this library tries to stick to the original C implementation as much as possible, so I'm not sure I would add this. I will gladly leave open for others to see if they would like it though.

dragonmantank avatar Jan 05 '22 04:01 dragonmantank

Thanks @dragonmantank

How do we vote?

bilogic avatar Jan 05 '22 07:01 bilogic

I just leave it open for anyone else to chime in. I added the label to make it easier for people to find when looking through issues, because we have a few feature requests that are like this (might make life easier, but diverge from the original implementation).

dragonmantank avatar Jan 05 '22 15:01 dragonmantank

@bilogic correct me if I am wrong but can't you not rewrite the code this way ?

$cron = new Cron\CronExpression('@quarterly');
echo $cron->getNextRunDate(null, 2)->sub(DateInterval::createFromDateString('7 days'))->format('Y-m-d H:i:s');

Seems to me that nothing needs to be added to the library to achieve what you are looking for 🤔 Or did I miss something ?

nyamsprod avatar Jan 08 '22 22:01 nyamsprod

@nyamsprod

I can't remember my exact thought process, but I'm using this library via Laravel's schedule. I vaguely recall being able to expose an instance of this library, and thus wanted to be able to shift(...) it so that when Laravel calls it's getNextRunDate(), it gets the shifted date.

bilogic avatar Jan 19 '22 12:01 bilogic

@nyamsprod I revisited this, this repo's isDue() needs to return true for the shifted date. As far as I can tell, your code doesn't do that. Basically the shifting logic must be contained within getNextRunDate() otherwise lots of code needs to be modified.

bilogic avatar Sep 24 '22 09:09 bilogic