Skip to content

PHPORM-143 Ensure date are read using local timezone #2739

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 1, 2024
Merged

PHPORM-143 Ensure date are read using local timezone #2739

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# Changelog
All notable changes to this project will be documented in this file.

## [4.1.3] - unreleased

* Fix the timezone of `datetime` fields when they are read from the database by @GromNaN in [#2739](https://github.com/mongodb/laravel-mongodb/pull/2739)

## [4.1.2] - 2024-02-22

* Fix support for subqueries using the query builder by @GromNaN in [#2717](https://github.com/mongodb/laravel-mongodb/pull/2717)
Expand Down
5 changes: 4 additions & 1 deletion src/Eloquent/Model.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Carbon\CarbonInterface;
use DateTimeInterface;
use DateTimeZone;
use Illuminate\Contracts\Queue\QueueableCollection;
use Illuminate\Contracts\Queue\QueueableEntity;
use Illuminate\Contracts\Support\Arrayable;
Expand All @@ -30,6 +31,7 @@
use function array_values;
use function class_basename;
use function count;
use function date_default_timezone_get;
use function explode;
use function func_get_args;
use function in_array;
Expand Down Expand Up @@ -137,7 +139,8 @@ protected function asDateTime($value)
{
// Convert UTCDateTime instances to Carbon.
if ($value instanceof UTCDateTime) {
return Date::instance($value->toDateTime());
return Date::instance($value->toDateTime())
->setTimezone(new DateTimeZone(date_default_timezone_get()));
}

return parent::asDateTime($value);
Expand Down
30 changes: 30 additions & 0 deletions tests/ModelTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,16 @@
use function abs;
use function array_keys;
use function array_merge;
use function date_default_timezone_set;
use function get_debug_type;
use function hex2bin;
use function sleep;
use function sort;
use function strlen;
use function time;

use const DATE_ATOM;

class ModelTest extends TestCase
{
public function tearDown(): void
Expand Down Expand Up @@ -670,6 +673,33 @@ public function testUnsetDotAttributesAndSet(): void
$this->assertSame(['note2' => 'DEF', 'note1' => 'ABC'], $user->notes);
}

public function testDateUseLocalTimeZone(): void
{
// The default timezone is reset to UTC before every test in OrchestraTestCase
$tz = 'Australia/Sydney';
date_default_timezone_set($tz);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test permanently alters the state of the environment. Would it be preferable to capture the current timezone first and then restore it at the end of the test?

If you're concerned about a failure interrupting things, you can use tearDown(), but I don't think that's necessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timezone is reset before each test by this: https://github.com/orchestral/testbench-core/blob/bf3de053bc6566210c0c336635d03e97e270f205/src/Concerns/CreatesApplication.php#L389-L391

But I can explicitly reset the timezone to prevent any regression.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't realize the tests in this project used OrchestraTestCase instead of PHPUnit directly. I think a comment explaining that would be sufficient.


$date = new DateTime('1965/03/02 15:30:10');
$user = User::create(['birthday' => $date]);
$this->assertInstanceOf(Carbon::class, $user->birthday);
$this->assertEquals($tz, $user->birthday->getTimezone()->getName());
$user->save();

$user = User::find($user->_id);
$this->assertEquals($date, $user->birthday);
$this->assertEquals($tz, $user->birthday->getTimezone()->getName());
$this->assertSame('1965-03-02T15:30:10+10:00', $user->birthday->format(DATE_ATOM));

$tz = 'America/New_York';
date_default_timezone_set($tz);
$user = User::find($user->_id);
$this->assertEquals($date, $user->birthday);
$this->assertEquals($tz, $user->birthday->getTimezone()->getName());
$this->assertSame('1965-03-02T00:30:10-05:00', $user->birthday->format(DATE_ATOM));

date_default_timezone_set('UTC');
}

public function testDates(): void
{
$user = User::create(['name' => 'John Doe', 'birthday' => new DateTime('1965/1/1')]);
Expand Down