fix!: read timezone from config by DennisGarding · Pull Request #186 · shopware/SwagMigrationAssistant

Dennis Garding (DennisGarding) · 2026-05-04T14:02:01Z

Malte Janz (MalteJanz)

The more I think about it, the more I think the extra effort for reading the SW5 configured timezone isn't worth it. Sorry for leading you down this EnvironmentReader path with my previous comment and our short talk.

What if we apply the simplest solution we could come up with here, just a few of my thoughts (feel free to explore a simpler approach on your own / deviate from this):

No reading (or guessing) from the source system at all
Premapping has a option for setting the "storage timezone" of the source system, which is like all other premappings put into the mapping table and can be used from there
possible choices would be all valid timezones
we could think about defaulting / pre-selecting "UTC", if that works for the majority of cases and otherwise the user can still adjust it and migrate again
I still like your approach of using Converter->convertValue(..., TYPE_DATETIME) to do the correct conversion automatically, but I would prefer if we could get rid of the extra Context parameter there and initialize the internal used source timezone differently from the mapping

My reasoning behind this:

Doing a full "read environment" call is expensive and should not really be necessary. I previously thought we might store the values of that somewhere in the DB but I was wrong about that (turns out we store it on the swag_migration_run but at the premapping stage we don't have a run yet)
The other premapping readers also sometimes read from the source system, but they only use a dedicated $gateway->readTable API. In our case we don't need DB data but rather configuration values stored as a file. Introducing extra APIs for that seems not worth the effort
Reading the SW5 config from a PHP file is error prone, e.g. they could also store the timezone in a variable and use that in multiple places 'timezone' => $timezone or other strange things
(Not sure about this one) but is that config.php really the single source of truth for this configuration in SW5? Is it not possible to override / provide config values via environment variables or other ways, where this config.php value could be wrong / unused?
The only reliable way I see would be asking the SW5 backend itself (e.g. they executed config.php and can tell what value is "active"), but that wouldn't work for our local gateway and doesn't seem worth the effort just for giving the user a little more convenience

Lars Kemper (larskemper) · 2026-05-12T11:20:12Z

+                ->format(Defaults::STORAGE_DATE_TIME_FORMAT);
+        } catch (\Throwable $exception) {
+            $this->loggingService->log(
+                MigrationLogBuilder::fromMigrationContext($this->migrationContext)


would be good to add at least entityName here, so its possible to identify

Lars Kemper (larskemper) · 2026-05-12T11:23:15Z

+use SwagMigrationAssistant\Migration\Logging\Log\Builder\AbstractMigrationLogEntry;
+
+#[Package('fundamentals@after-sales')]
+readonly class ConvertDateTimeFailedLog extends AbstractMigrationLogEntry


Would not add a new Log for this. In the end it can simply be a ConvertEntityFailedLog or RunExceptionLog

Lars Kemper (larskemper) · 2026-05-12T11:24:07Z

+        $timezone = null;
+        $premapping = $this->migrationContext->getConnection()->getPremapping();
+        foreach ($premapping ?? [] as $item) {
+            if ($item->getEntity() !== 'source_timezone') {


would be good to make constants in TimezoneReader public and reuse here

Malte Janz (MalteJanz)

Because there aren't any comments / reactions to my previous comments (other than that they are resolved) and there is no meaningful PR description I have no clue what your current goal for a solution is. Looks like you still want to read the source system timezone?

Nevertheless the current implementation still has lots of issues related to that and also didn't follow my suggestion to not read from the source system to keep it simple. Please provide proper solutions to these issues or refactor to not read from the source system.

Malte Janz (MalteJanz) · 2026-05-12T13:38:31Z

+    private function getTimezoneFromPremapping(): ?string
+    {
+        $runId = $this->migrationContext->getRunUuid();
+        if (\array_key_exists($runId, $this->timezoneCache)) {
+            return $this->timezoneCache[$runId];
+        }
+
+        $timezone = null;
+        $premapping = $this->migrationContext->getConnection()->getPremapping();
+        foreach ($premapping ?? [] as $item) {
+            if ($item->getEntity() !== 'source_timezone') {
+                continue;
+            }
+
+            foreach ($item->getMapping() as $mapping) {
+                if ($mapping->getSourceId() === 'timezone') {
+                    $timezone = $mapping->getDestinationUuid() === '' ? null : $mapping->getDestinationUuid();
+                }
+            }


This is not how we read the submitted premapping in our converters, see our documentation on that here:
https://developer.shopware.com/docs/products/extensions/migration-assistant/concept/premapping.html

You just need to do a mapping lookup instead, e.g.:

$mapping = $this->mappingService->getMapping( $this->connectionId, TimezoneReader::getMappingName(), TimezoneReader::SOURCE_ID, $this->context, );

*You likely need to use getValue in this case though, because you don't want to look up a UUID, but instead just a string

SwagMigrationAssistant/src/Migration/Mapping/MappingService.php

Line 260 in d970390

public function getValue(string $connectionId, string $entityName, string $oldIdentifier, Context $context): ?string

Malte Janz (MalteJanz) · 2026-05-12T13:48:58Z


 #[Package('fundamentals@after-sales')]
-abstract class ShopwareConverter extends Converter
+abstract class ShopwareConverter extends Converter implements ResetInterface


Where are all the child classes tagged with kernel.reset now? Otherwise just implementing this Interface is useless if its never called.

You might want to extract that datetime timezone conversion logic into its own service though, that way its stays independent of the ShopwareConverter and could potentially also be used by other profile converters such as Magento 🤷

Malte Janz (MalteJanz) · 2026-05-12T13:58:10Z

+            return null;
+        }
+
+        $timezoneResult = $this->timezoneReader->read($migrationContext);


You can't just use the ApiTimezoneReader here, you need to go through the GatewayRegistry to get the correct gateway for the active connection and the use that to talk to the source system, otherwise this might just crash for connections with local gateway. Just take a look at our existing Premapping readers or our documentation about creating one:
https://developer.shopware.com/docs/products/extensions/migration-assistant/guides/decorating-a-shopware-migration-assistant-converter.html#creating-a-premapping-reader

But as I already said in my last code review, I don't think its worth the effort to read from the source system if that isn't even guaranteed to be reliable. Then its better to just let the user choose and maybe default to UTC if that's commonly set on SW5 by default?

Malte Janz (MalteJanz) · 2026-05-12T14:03:21Z

+
+        $sourceTimezone = $this->readSourceTimezone($migrationContext);
+
+        $description = $sourceTimezone ?? 'No source time zone';


How does this description make sense? It should always say Source system time zone, not Europe/Berlin and then you have to choose Europe/Berlin as well 😕 ?

Malte Janz (MalteJanz) · 2026-05-12T14:06:56Z

+use SwagMigrationAssistant\Migration\MigrationContextInterface;
+
+#[Package('after-sales')]
+class TimezoneReader extends ApiReader


If you really want to do this reading from the source system, where is the equivalent local gateway reader? Also the concept of these Readers are for reading data that is then converted, and are not really for fetching premapping data. For that all other PremappingReaders are using the GatewayInterface->readTable method.

I would again strongly recommend not to fetch the timezone from the source system, unless you find a clean solution that is guaranteed to work and always provides the correct source system timezone for both API and local gateways

Malte Janz (MalteJanz) · 2026-05-12T14:10:45Z

+{
+    public function supports(MigrationContextInterface $migrationContext): bool
+    {
+        return false;


Dead code? Directly using this Reader from dependency injection is an anti-pattern and it should only be retrieved from the corresponding Registry, to support all possible gateways

Read timezone from config

82d7465

Dennis Garding (DennisGarding) requested review from a team, Simon Fiebranz (CR0YD), Malte Janz (MalteJanz), Susanne Hartung (ennasus4sun), Jozsef Damokos (jozsefdamokos), Lars Kemper (larskemper) and Nikolas Evers (vintagesucks) May 4, 2026 14:02

Dennis Garding (DennisGarding) self-assigned this May 4, 2026

Dennis Garding (DennisGarding) commented May 4, 2026

View reviewed changes

Comment thread src/Profile/Shopware/Premapping/TimezoneReader.php Outdated

Dennis Garding (DennisGarding) marked this pull request as draft May 4, 2026 14:09

Malte Janz (MalteJanz) reviewed May 5, 2026

View reviewed changes

Comment thread src/Profile/Shopware/Gateway/Local/Reader/TimezoneReader.php Outdated

Comment thread src/Profile/Shopware/Premapping/TimezoneReader.php Outdated

Fix threads

6f3a0b5

Malte Janz (MalteJanz) mentioned this pull request May 7, 2026

fix: consider time zone during migration of orders shopware/SwagMigrationConnector#9

Open

Dennis Garding (DennisGarding) added 3 commits May 7, 2026 11:51

Fix tests

52bbb67

Fix tests

1bd38be

Fix acceptance tests

8dea1cc

Jozsef Damokos (jozsefdamokos) approved these changes May 7, 2026

View reviewed changes

Comment thread src/Profile/Shopware/Converter/ShopwareConverter.php Outdated

Comment thread src/Profile/Shopware/Gateway/Local/Reader/EnvironmentReader.php Outdated

Comment thread src/Profile/Shopware/Gateway/Local/Reader/EnvironmentReader.php Outdated

Dennis Garding (DennisGarding) and others added 4 commits May 7, 2026 14:48

Fix acceptance tests

9f47c5d

Fix acceptance tests

2ce5808

Merge branch 'trunk' into 14186/time-zone-diff

f630a34

Fix threads

e3bb9c5

Dennis Garding (DennisGarding) marked this pull request as ready for review May 8, 2026 06:13

Lars Kemper (larskemper) reviewed May 8, 2026

View reviewed changes

Comment thread src/Profile/Shopware/Converter/ShopwareConverter.php Outdated

Comment thread src/Profile/Shopware/Converter/ShopwareConverter.php Outdated

Comment thread src/Profile/Shopware/Gateway/Local/Reader/EnvironmentReader.php Outdated

Malte Janz (MalteJanz) self-requested a review May 8, 2026 08:32

Malte Janz (MalteJanz) reviewed May 8, 2026

View reviewed changes

Dennis Garding (DennisGarding) added 2 commits May 12, 2026 07:30

Fix threads

516dbaf

Fix tests

a46760d

Dennis Garding (DennisGarding) requested a review from Lars Kemper (larskemper) May 12, 2026 07:21

Dennis Garding (DennisGarding) requested a review from Malte Janz (MalteJanz) May 12, 2026 07:21

Lars Kemper (larskemper) approved these changes May 12, 2026

View reviewed changes

Malte Janz (MalteJanz) requested changes May 12, 2026

View reviewed changes

Lars Kemper (larskemper) changed the title ~~fix: read timezone from config~~ fix!: read timezone from config May 13, 2026


		$sourceTimezone = $this->readSourceTimezone($migrationContext);

		$description = $sourceTimezone ?? 'No source time zone';

Conversation

Dennis Garding (DennisGarding) commented May 4, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Malte Janz (MalteJanz) left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Lars Kemper (larskemper) May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Lars Kemper (larskemper) May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Lars Kemper (larskemper) May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Malte Janz (MalteJanz) left a comment

Choose a reason for hiding this comment

Uh oh!

Malte Janz (MalteJanz) May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Malte Janz (MalteJanz) May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Malte Janz (MalteJanz) May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Malte Janz (MalteJanz) May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Malte Janz (MalteJanz) May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Malte Janz (MalteJanz) May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Malte Janz (MalteJanz) left a comment •

edited

Loading

Lars Kemper (larskemper) May 12, 2026 •

edited

Loading

Malte Janz (MalteJanz) May 12, 2026 •

edited

Loading