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

Timezone offset is added to the value after upgrading to 1.22 #863

Closed
sisve opened this issue Jan 17, 2017 · 11 comments
Closed

Timezone offset is added to the value after upgrading to 1.22 #863

sisve opened this issue Jan 17, 2017 · 11 comments

Comments

@sisve
Copy link

sisve commented Jan 17, 2017

I'm seeing the timezone value added into the result afteraddDays(1) and subDays(1). The last value ends up at 11:23:45 while still keeping the same timezone. I expect it to end up at 01:23:45.

Output from Laravel's tinker tool:

>>> $input = '2017-01-01T01:23:45+10:00'
=> "2017-01-01T01:23:45+10:00"
>>> $value = \Carbon\Carbon::parse($input);
=> Carbon\Carbon {#1650
     +"date": "2017-01-01 01:23:45.000000",
     +"timezone_type": 1,
     +"timezone": "+10:00",
   }
>>> $value = $value->addDays(1)
=> Carbon\Carbon {#1650
     +"date": "2017-01-02 01:23:45.000000",
     +"timezone_type": 1,
     +"timezone": "+10:00",
   }
>>> $value = $value->subDays(1)
=> Carbon\Carbon {#1650
     +"date": "2017-01-01 11:23:45.000000",
     +"timezone_type": 1,
     +"timezone": "+10:00",
   }

Failing test:

<?php

class CarbonTest extends \PHPUnit\Framework\TestCase {

    public function testCarbonZomg() {
        $input = '2017-01-01T01:23:45+10:00';

        $value = \Carbon\Carbon::parse($input);
        $this->assertEquals('2017-01-01T01:23:45+10:00', $value->toIso8601String());

        $value = $value->addDays(1);
        $this->assertEquals('2017-01-02T01:23:45+10:00', $value->toIso8601String());

        $value = $value->subDays(1);
        $this->assertEquals('2017-01-01T01:23:45+10:00', $value->toIso8601String());
        /**
         * Failed asserting that two strings are equal.
         * --- Expected
         * +++ Actual
         * @@ @@
         * -'2017-01-01T01:23:45+10:00'
         * +'2017-01-01T11:23:45+10:00'
         */
    }

}
@brianseitel
Copy link

I also ran into this problem, same issue. I do a lot of sensitive work with dates that, if wrong, could cost me a lot of money.

Any chance of it getting fixed soon?

@TobiasFroberg
Copy link

This can be reproduced over here as well, running 1.22.1

@lucasmichot
Copy link
Collaborator

Can you check if this related to #861?

@lucasmichot
Copy link
Collaborator

Also have a look at #862 (comment)

@sisve
Copy link
Author

sisve commented Jan 18, 2017

I'm not sure how to do that. The tinker output doesn't call toIso8601String(), I only do it to get a string for display purposes. I see no production-code in Carbon that calls toIso8601String() at all, so it isn't used by subDays()/modify().

@kaufholdr
Copy link

The problem does not exist when you use the add(CarbonInterval interval) method like:

$input = '2017-01-01T01:23:45+10:00'
$value = \Carbon\Carbon::parse($input);
$value = $value->add(CarbonInterval::days(1))
$value = $value->sub(CarbonInterval::days(1))

So I think the problem is how modify is implemented.

@Ovsyanka
Copy link
Contributor

Ovsyanka commented Feb 3, 2017

The problem exactly in "modify" implemented, i think we shoult discuss it in #883 now. There i described my vision how it should work.

@lucasmichot
Copy link
Collaborator

See #884

@sisve
Copy link
Author

sisve commented Mar 5, 2017

This issue is fixed by #884

@ryanwinchester
Copy link

Just a note I also had a bunch of failing tests just using setTimezone

Re: ryanwinchester/carbonize#3

@Glavic
Copy link
Collaborator

Glavic commented Mar 4, 2018

This issue was solved in v1.23.

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

8 participants