fix: consider time zone during migration of orders#184
fix: consider time zone during migration of orders#184Dennis Garding (DennisGarding) wants to merge 2 commits intotrunkfrom
Conversation
| * @param array<string, mixed> $converted | ||
| * @param array<string, mixed> $data | ||
| */ | ||
| private function convertOrderDateTime(array &$converted, array &$data): void |
There was a problem hiding this comment.
are there more datatime fields that would need something like that?
There was a problem hiding this comment.
Yes I also think there are likely more cases we have to adjust. Just searching through the migration-assistant codebase for datetime in PHP files already reveals some more...
Looks like basically any time we call
$this->convertValue(..., self::TYPE_DATETIME);
// or
$this->convertValue(..., 'datetime');it should consider the source system timezone instead of assume its UTC as SW6. Means we need a more general proper fix for this instead of just fixing this one place for order times.
| $orderEsd = $this->getOrderEsd($migrationContext); | ||
| $orderDetails = $this->getOrderDetails($migrationContext); | ||
| $orderDocuments = $this->getOrderDocuments($migrationContext); | ||
| $timezone = $this->getDatabaseTimezone($migrationContext); |
There was a problem hiding this comment.
Is it not still possible to have a mismatch here? Is the database timezone always aligned with the source shop's timezone?
| $orderDateTime = new \DateTimeImmutable($data['ordertime'], new \DateTimeZone($sourceTimezone)); | ||
| $converted['orderDateTime'] = $orderDateTime | ||
| ->setTimezone(new \DateTimeZone(self::UTC_TIMEZONE)) | ||
| ->format(\DATE_ATOM); |
There was a problem hiding this comment.
Shouldn't this be Defaults::STORAGE_DATE_TIME_FORMAT instead, because thats what SW6 uses right?
| * @param array<string, mixed> $converted | ||
| * @param array<string, mixed> $data | ||
| */ | ||
| private function convertOrderDateTime(array &$converted, array &$data): void |
There was a problem hiding this comment.
Yes I also think there are likely more cases we have to adjust. Just searching through the migration-assistant codebase for datetime in PHP files already reveals some more...
Looks like basically any time we call
$this->convertValue(..., self::TYPE_DATETIME);
// or
$this->convertValue(..., 'datetime');it should consider the source system timezone instead of assume its UTC as SW6. Means we need a more general proper fix for this instead of just fixing this one place for order times.
| <<<'SQL' | ||
| SELECT timeZones.timeZone | ||
| FROM ( | ||
| SELECT @@SESSION.time_zone AS timeZone | ||
| UNION | ||
| SELECT @@system_time_zone AS timeZone | ||
| ) AS timeZones | ||
| WHERE timeZone != 'SYSTEM' | ||
| LIMIT 1 | ||
| SQL |
There was a problem hiding this comment.
Is this the most reliable way to figure out the storage timezone for SW5?
Because in my local dev system running this query on either my SW6 or SW5 database, both return CEST, while SW6 for example clearly stores all datetimes in UTC, means the DB timezone might not matter at all for the application?
Is there maybe some configuration value in SW5 to change it? Or does SW5 store its values with the timezone? I think in SW6 its programmatically hard coded in multiple places like:
| if ($timezone !== null) { | ||
| $order['_timezone'] = $timezone; |
There was a problem hiding this comment.
This looks wrong, why would you add this timezone data to each order while it is the same for every order, or even every entity that comes from SW5.
You should read that once from the source system and store it e.g. in a mapping for easy lookup 🤔
4 participants
closes: #14184
Gets time zone from source db and consider it during migration of orders