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

Weekly Rule with selected Weekdays does not produce the correct dates. #90

Closed
ccc-dbellinger opened this issue Dec 8, 2020 · 4 comments

Comments

@ccc-dbellinger
Copy link

ccc-dbellinger commented Dec 8, 2020

Hello,

When using a weekly frequency with a BYDAY rule, only days between the weekday of DTSTART and the day before WKST (inclusive) are considered. See the following example:

<?php

use RRule\RRule;

include 'vendor/autoload.php';

$dtstart = DateTimeImmutable::createFromFormat('Y-m-d H:i', '2021-01-08 08:00');
$parts = [
    'BYDAY' => [ 'MO', 'WE', 'FR' ],
    'FREQ' => 'WEEKLY',
    'WKST' => 'SU',
    'DTSTART' => $dtstart,
];
$rrule = new RRule($parts);

$start = DateTimeImmutable::createFromFormat('Y-m-d', '2020-01-01');
$end = DateTimeImmutable::createFromFormat('Y-m-d', '2021-12-31');

$occurrences = $rrule->getOccurrencesBetween($start, $end, 10);

foreach ($occurrences as $occurrence) {
    echo $occurrence->format('l, F j, Y H:i T') . "\n";
}

This produces the following output:

Friday, January 8, 2021 08:00 EST
Friday, January 15, 2021 08:00 EST
Friday, January 22, 2021 08:00 EST
Friday, January 29, 2021 08:00 EST
Friday, February 5, 2021 08:00 EST
Friday, February 12, 2021 08:00 EST
Friday, February 19, 2021 08:00 EST
Friday, February 26, 2021 08:00 EST
Friday, March 5, 2021 08:00 EST
Friday, March 12, 2021 08:00 EST

I believe this should include Mondays and Wednesdays after the first week, Thus it should be the following output:

Friday, January 8, 2021 08:00 EST
Monday, January 11, 2021 08:00 EST
Wednesday, January 13, 2021 08:00 EST
Friday, January 15, 2021 08:00 EST
Monday, January 18, 2021 08:00 EST
Wednesday, January 20, 2021 08:00 EST
Friday, January 22, 2021 08:00 EST
Monday, January 25, 2021 08:00 EST
Wednesday, January 27 2021 08:00 EST
Friday, January 29, 2021 08:00 EST

Changing the above DTSTART value to 2021-01-04 08:00 produces the correct dates:

Monday, January 4, 2021 08:00 EST
Wednesday, January 6, 2021 08:00 EST
Friday, January 8, 2021 08:00 EST
Monday, January 11, 2021 08:00 EST
Wednesday, January 13, 2021 08:00 EST
Friday, January 15, 2021 08:00 EST
Monday, January 18, 2021 08:00 EST
Wednesday, January 20, 2021 08:00 EST
Friday, January 22, 2021 08:00 EST
Monday, January 25, 2021 08:00 EST
@pluk77
Copy link

pluk77 commented Dec 9, 2020

After a quick test it seems that this happens when DTSTART is an Immutable DateTime object. Change it to:

$dtstart = DateTime::createFromFormat('Y-m-d H:i', '2021-01-08 08:00');

returned the expected 10 dates.

@pluk77
Copy link

pluk77 commented Dec 9, 2020

I suppose the parseDate function should either throw an exception if an immutable DateTime is used, or it should not directly clone the object but rather convert it to a mutable DateTime object?

@pluk77
Copy link

pluk77 commented Dec 9, 2020

something like this:

static public function parseDate($date)
{
    if (!$date instanceof \DateTime) {
        try {
            if (is_integer($date)) {
                $date = \DateTime::createFromFormat('U', $date);
                $date->setTimezone(new \DateTimeZone('UTC')); // default is +00:00 (see issue #15)
            } elseif ($date instanceof \DateTimeImmutable) {
                if (method_exists(\DateTime::class, 'createFromImmutable')) {
                    $date = \DateTime::createFromImmutable($date);
                } else {
                    $date = new \DateTime($date->format(\DateTime::ATOM));
                }
            } else {
                $date = new \DateTime($date);
            }
        } catch (\Exception $e) { // PHP 5.6
            throw new \InvalidArgumentException(
                    "Failed to parse the date ({$e->getMessage()})"
            );
        } catch (\Throwable $e) { // PHP 7+
            throw new \InvalidArgumentException(
                    "Failed to parse the date ({$e->getMessage()})"
            );
        }
    } else {
        $date = clone $date; // avoid reference problems
    }
    return $date;
}

@rlanvin
Copy link
Owner

rlanvin commented Dec 9, 2020

The fix is available in v2.2.1, thanks for the bug report!

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

No branches or pull requests

3 participants