fix: consider time zone during migration of orders#9
fix: consider time zone during migration of orders#9Dennis Garding (DennisGarding) wants to merge 19 commits into
Conversation
Dennis Garding (DennisGarding)
requested review from
Simon Fiebranz (CR0YD),
Malte Janz (MalteJanz),
Susanne Hartung (ennasus4sun),
Jozsef Damokos (jozsefdamokos),
Lars Kemper (larskemper) and
Nikolas Evers (vintagesucks)
and removed request for
a team
Dennis Garding (DennisGarding)
requested review from
Malte Janz (MalteJanz),
Jozsef Damokos (jozsefdamokos) and
Lars Kemper (larskemper)
|
That linked PR was closed without any comment, can you add a comment to it and point to the correct Assistant PR here? I guess it should be this one shopware/SwagMigrationAssistant#186 ? Also are these draft PRs "ready for review" or do you just want a quick review on the broader architecture / solution and a proper review later? Would be nice if you could write a comment if you request review on draft PRs what your expectations are or put it out of draft mode for a full review. |
Malte Janz (MalteJanz)
left a comment
Malte Janz (MalteJanz)
left a comment
There was a problem hiding this comment.
As already said in the other PR, this solution likely is not feasible unless you find a clean way to support it in the Assistant:
shopware/SwagMigrationAssistant#186 (review)
| public function indexAction() | ||
| { | ||
| $dbConfig = $this->container->getParameter('shopware.db'); | ||
| \assert(\is_array($dbConfig)); |
There was a problem hiding this comment.
\assert might not run in every environment (can be disabled in php.ini), also it shouldn't be used for user facing errors and we don't use it anywhere else in the MigrationConnector.
This should be a proper exception instead
| $dbConfig = $this->container->getParameter('shopware.db'); | ||
| \assert(\is_array($dbConfig)); | ||
|
|
||
| $timezone = $dbConfig['timezone']; |
There was a problem hiding this comment.
What if the array key does not exists?
| $timezone = $dbConfig['timezone']; | |
| $timezone = $dbConfig['timezone'] ?? ''; |
*Not sure which old PHP versions we need to support here, you might have to double check the minimum SW5 version this plugins supports and the minimum PHP version of that, if we can use ??
5 participants
This PR is necessary to complete the issue: shopware/shopware#14186
See PR: shopware/SwagMigrationAssistant#186