Modify with time on instance with timezone doesn't respect timezone

[tpraxl]

Calling modify with a time part doesn't respect the timezone of the instance it is applied to.

I added a unit test to prove that in my fork: https://github.com/tpraxl/Carbon/tree/prove-timezone-not-respected-bug:

    public function testModifyWithTimeRespectsTimezone()
    {
        // Given we have a Carbon Date in a certain timezone
        $b = new Carbon('2018-02-03 00:00:00', 'Europe/Berlin');
        // And we call modify with a time part
        $b->modify('14:00');
        // Then we expect it represent that time in the given timezone
        $this->assertEquals(new Carbon('2018-02-03 14:00:00', 'Europe/Berlin'), $b);
    }

This fails with:

Failed asserting that two DateTime objects are equal.
--- Expected
+++ Actual
@@ @@
-2018-02-03T14:00:00.000000+0100
+2018-02-02T15:00:00.000000+0100

This is most likely caused by the implementation of \Carbon\Carbon::modify, which sets the timezone to UTC before applying modify:

        $timezone = $this->getTimezone();
        $this->setTimezone('UTC');
        $instance = parent::modify($modify);
        $this->setTimezone($timezone);

I understand that this works for new instances – and is somehow required to work in that context – but it doesn't seem to work when applied to existing instances.

I also understand that this works when the timezone is considered local (see the other added unit test, testModifyWithTimeRespectsTimezoneWhenLocal, which passes).

But when dealing with multiple timezones in your application, this should not be the way to go.

Simplified Usecase:

An application that configures relative scheduling dates in different timezones.

$config = ['de' => ['event-reminder' => '-2 weeks 14:00']];

// imagine an iteration over a bunch of dates with different timezones
$country = 'de'
$dueDate = $myEventDateWithTimezone->copy()->modify($config[$country]['event-reminder']);

[Ovsyanka]

Look at #883

[Glavic]

I don't know why Carbon team decided to change modify() method in the way they did: change TZ to UTC before applying modification, and then changing TZ back to original one...

Like you already wrote, what happens here is:

• you have 2018-02-03 00:00:00 EU\Berlin (UTC+01:00)
• Carbon changes TZ before modification of time to 2018-02-02 23:00:00 UTC
• method modify('14:00') sets time to 2018-02-02 14:00:00 UTC
• Carbon changes TZ back to original one 2018-02-02 15:00:00 EU\Berlin

That is why you receive back 2018-02-02T15:00:00.000000+0100 which is correct (for what Carbon does).

You could use ->setTime(14, 0) on your object for your issue?

[tpraxl]

@Glavic Thanks for your comment. This is exactly what we did, but I consider it a workaround. And I consider Carbon's behavior a bug (although I didn't take the time to research the reason for it – there seems to be a good reason).

First of all, it means that we have to set two separate configuration values per country for a single purpose.

Second: The whole reason for applying this configuration to a non-utc-date was to enable us to trigger events independent of DST. Afterwards, the final date is persisted for scheduling as a UTC date anyways.

[Ovsyanka]

@Glavic @tpraxl look at the #883. There is explanation where this code came from (issue #88). And it is not correct code and I suggested to remove it and made a pull-request for that. All this info available in #883.

[Glavic]

Working on solution in #1070 PR. TY