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

Splitting a day period by 10 minutes on daylight savings roll-back day throws LogicException #68

Closed
isEvrythngTkn opened this issue Nov 27, 2018 · 3 comments
Assignees

Comments

@isEvrythngTkn
Copy link

Bug Report

Information Description
Version 4.0.0
PHP version 7.1.16
OS Platform MacOS High Sierra 10.13.6

Summary

If you have a period that spans the day of November 4, 2018 in a timezone where daylight savings is in effect, if you split that period by 10 or 30 minutes, it throws a Logic Exception with the message: "The ending datepoint must be greater or equal to the starting datepoint".

Standalone code, or other way to reproduce the problem

I pulled the repo and wrote this test to reproduce the bug.

public function testSplitDaylightSavingsDayIntoHours()
{
    date_default_timezone_set('Canada/Central');
    $period = new Period(new DateTime('2018-11-04 00:00:00.000000'), new DateTime('2018-11-05 00:00:00.000000'));
    $splits = $period->split(new DateInterval('PT30M'));
    foreach ($splits as $inner_period) {
        self::assertNotNull($inner_period);
    }
}

Expected result

To not throw an exception when daylight savings is the cause $startDate > $endDate.

Actual result

Throws an exception.

@nyamsprod nyamsprod self-assigned this Nov 28, 2018
nyamsprod added a commit that referenced this issue Nov 28, 2018
nyamsprod added a commit that referenced this issue Nov 28, 2018
@nyamsprod
Copy link
Member

@isEvrythngTkn thanks for reporting this bug. A Patch has landed on the master branch also I've added your test to the test suite and also checked that Period::splitBackward did not suffer from the same issue.

Could you have a look and tell me if it's all good for you ? If no I may release a patch fix this week of the week after

@isEvrythngTkn
Copy link
Author

Thanks very much @nyamsprod. I pulled your latest changes, they look good. Tests all passing. Awesome stuff.

@nyamsprod
Copy link
Member

version 4.0.1 is out with the bug fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants