Set timezone properly in Schedule\Event

[ibrasho]

An attempt to fix #7573 .
This is very hard to test for and against.

Any ideas for how to create a test for this?

[ibrasho]

Added test after using Carbon::setTestNow() which is pretty nifty.

[taylorotwell]

Can you describe the fix. It looks like you are passing the date as a string every time?

[taylorotwell]

Would like to see this resolved on the cron-expression side of things if possible.

[ibrasho]

I'll try yo explain what was happening and the proposed fix:

Current Event::expressionPasses():

        $date = Carbon::now();

        if ($this->timezone) {
            $date->setTimezone($this->timezone);
        }

        return CronExpression::factory($this->expression)->isDue($date);

• When the Event::expressionPasses() call is made, if the timezone is set it's used to adjust the DateTime object sent to the CronExpression::isDue().
  Assume that the default timezone is UTC, the current time is 2015-01-02 00:00:00 UTC and the event timezone is set to EST. This will send an object set to 2015-01-01 19:00:00 EST to the CronExpression::isDue().
• The CronExpression::isDue() function normalize the $currentTime passed to it by setting its timezone to the default timezone again, so we are back at the 2015-01-02 00:00:00 UTC.

This mean the the DateTime::setTimezone() call made in Event::expressionPasses() is useless.

The fix was to send a modified version of the date. So instead of sending 2015-01-01 19:00:00 EST, we should send 2015-01-01 00:00:00 EST, this is why we get the current time and timezone through:
$date = Carbon::parse(Carbon::now()->toDateTimeString(), $this->timezone);

After the conversion in CronExpression::isDue(), the result will be 2015-01-01 05:00:00 UTC, which should be the time when the cron expression should be evaluated against if we want it to take the timezone into consideration.

This is a hackish fix sadly, the other fix will be to update CronExpression to support timezone.

[ibrasho]

According to this test, this looks like the intended behaviour:
https://github.com/mtdowling/cron-expression/blob/master/tests/Cron/CronExpressionTest.php#L220

[ibrasho]

I've pushed another solution, that only changes one line in the Event::expressionPasses() call.
Instead of sending the DateTime object, if we send only the date time string representation after setting the time zone it will work.

Both fixes are in separate commits.

[taylorotwell]

I'm gonna close this because this needs to be fixed in cron-expression.

[taylorotwell]

Since there has been no movement on the library side, we're gonna have to fix this here. Have you tested this solution?