Skip to content

feat: Compatible with product bundle #113

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

Open
tamvt wants to merge 3 commits into
base: trunk
Choose a base branch
from
+195 −10
Open

feat: Compatible with product bundle #113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e844b50
feat: Compatible with migration assistant
tamvt Jan 12, 2026
ed932e4
fix: Fix tests
tamvt Jan 13, 2026
fc9865c
Fix: Support fields with Immutable flags
tamvt Jan 16, 2026
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
39 changes: 38 additions & 1 deletion src/DataProvider/Provider/Data/ProductProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@

namespace SwagMigrationAssistant\DataProvider\Provider\Data;

use Doctrine\DBAL\Connection;
use Shopware\Core\Content\Product\ProductCollection;
use Shopware\Core\Framework\Context;
use Shopware\Core\Framework\DataAbstractionLayer\EntityRepository;
use Shopware\Core\Framework\DataAbstractionLayer\Search\Criteria;
use Shopware\Core\Framework\DataAbstractionLayer\Search\Filter\EqualsFilter;
use Shopware\Core\Framework\DataAbstractionLayer\Search\Filter\NotFilter;
use Shopware\Core\Framework\DataAbstractionLayer\Search\Sorting\FieldSorting;
use Shopware\Core\Framework\Log\Package;
use SwagMigrationAssistant\Migration\DataSelection\DefaultEntities;
Expand All @@ -20,12 +23,17 @@
#[Package('fundamentals@after-sales')]
class ProductProvider extends AbstractProvider
{
private const BUNDLE_PRODUCT_TYPE = 'grouped_bundle';

private ?bool $hasTypeColumn = null;

/**
* @param EntityRepository<ProductCollection> $productRepo
*/
public function __construct(
private readonly EntityRepository $productRepo,
private readonly RouterInterface $router,
private readonly Connection $connection,
) {
}

Expand Down Expand Up @@ -55,6 +63,7 @@ public function getProvidedData(int $limit, int $offset, Context $context): arra
new FieldSorting('parentId'), // get 'NULL' parentIds first
new FieldSorting('id')
);
$this->addBundleExclusionFilter($criteria);
$result = $this->productRepo->search($criteria, $context);

$cleanResult = $this->cleanupSearchResult($result, [
Expand Down Expand Up @@ -113,6 +122,34 @@ public function getProvidedData(int $limit, int $offset, Context $context): arra

public function getProvidedTotal(Context $context): int
{
return $this->readTotalFromRepo($this->productRepo, $context);
$criteria = new Criteria();
$this->addBundleExclusionFilter($criteria);

return $this->readTotalFromRepo($this->productRepo, $context, $criteria);
}

private function addBundleExclusionFilter(Criteria $criteria): void
{
if (!$this->hasTypeColumn()) {
return;
}

$criteria->addFilter(
new NotFilter(NotFilter::CONNECTION_AND, [
new EqualsFilter('type', self::BUNDLE_PRODUCT_TYPE),
])
);
}

private function hasTypeColumn(): bool
{
if ($this->hasTypeColumn !== null) {
return $this->hasTypeColumn;
}

$columns = $this->connection->createSchemaManager()->listTableColumns('product');
$this->hasTypeColumn = isset($columns['type']);

return $this->hasTypeColumn;
}
}
1 change: 1 addition & 0 deletions src/DependencyInjection/dataProvider.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
<service id="SwagMigrationAssistant\DataProvider\Provider\Data\ProductProvider">
<argument type="service" id="product.repository"/>
<argument type="service" id="router"/>
<argument type="service" id="Doctrine\DBAL\Connection"/>
<tag name="shopware.dataProvider.provider"/>
</service>

Expand Down
1 change: 1 addition & 0 deletions src/DependencyInjection/shopware6.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@

<service id="SwagMigrationAssistant\Profile\Shopware6\Converter\ProductConverter"
parent="SwagMigrationAssistant\Profile\Shopware6\Converter\ShopwareMediaConverter">
<argument type="service" id="Doctrine\DBAL\Connection"/>
<tag name="shopware.migration.converter"/>
</service>

Expand Down
40 changes: 32 additions & 8 deletions src/Profile/Shopware/Converter/AttributeConverter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ public function convert(array $data, Context $context, MigrationContextInterface
$this->connectionName = $connection->getName();
}

$existingCustomFieldSetMapping = $this->mappingService->getMapping(
$this->connectionId,
DefaultEntities::CUSTOM_FIELD_SET,
$this->getCustomFieldEntityName() . 'CustomFieldSet',
$context
);
$isCustomFieldSetUpdate = $existingCustomFieldSetMapping !== null;

$mapping = $this->mappingService->getOrCreateMapping(
$this->connectionId,
DefaultEntities::CUSTOM_FIELD_SET,
Expand All @@ -57,7 +65,12 @@ public function convert(array $data, Context $context, MigrationContextInterface

$connectionName = ConnectionNameSanitizer::sanitize($this->connectionName);

$converted['name'] = 'migration_' . $connectionName . '_' . $this->getCustomFieldEntityName();
$customFieldSetName = 'migration_' . $connectionName . '_' . $this->getCustomFieldEntityName();

if (!$isCustomFieldSetUpdate) {
$converted['name'] = $customFieldSetName;
}

$converted['config'] = [
'label' => [
$data['_locale'] => \ucfirst($this->getCustomFieldEntityName()) . ' migration custom fields (attributes)',
Expand All @@ -84,6 +97,14 @@ public function convert(array $data, Context $context, MigrationContextInterface
$additionalData['columnType'] = $data['configuration']['column_type'];
}

$existingCustomFieldMapping = $this->mappingService->getMapping(
$this->connectionId,
$this->getCustomFieldEntityName(),
$data['name'],
$context
);
$isCustomFieldUpdate = $existingCustomFieldMapping !== null;

$this->mainMapping = $this->mappingService->getOrCreateMapping(
$this->connectionId,
$this->getCustomFieldEntityName(),
Expand All @@ -93,15 +114,18 @@ public function convert(array $data, Context $context, MigrationContextInterface
$additionalData
);

$converted['customFields'] = [
[
'id' => $this->mainMapping['entityUuid'],
'name' => $converted['name'] . '_' . $data['name'],
'type' => $this->getCustomFieldType($data),
'config' => $this->getCustomFieldConfiguration($data),
],
$customField = [
'id' => $this->mainMapping['entityUuid'],
'config' => $this->getCustomFieldConfiguration($data),
];

if (!$isCustomFieldUpdate) {
$customField['name'] = $customFieldSetName . '_' . $data['name'];
$customField['type'] = $this->getCustomFieldType($data);
}

$converted['customFields'] = [$customField];

unset(
$data['name'],
$data['type'],
Expand Down
31 changes: 31 additions & 0 deletions src/Profile/Shopware6/Converter/CustomFieldSetConverter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,43 @@ protected function convertData(array $data): ConvertStruct
{
$converted = $data;

// Check if custom field set already exists (was previously migrated) before creating mapping
// This is needed because the 'name' field has Immutable flag and can only be set on create
Copy link
Contributor

@MalteJanz MalteJanz Jan 19, 2026

Choose a reason for hiding this comment

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

Sorry for not raising this concern earlier:

Do you really think it's a good idea to have such a ImmutableFlag ? (Was introduced in this PR )

Because I already see the extra work it creates for us here, every entity that uses that flag needs an extra special treatment in the migration and if someone adds that flag to other fields in the future (or introduces such fields), we will run into issues again.

It basically makes "upserts" (insert / update) really difficult to do for such entities, because you want to create them if they don't exists yet, or update them otherwise, so you need the field to be defined but also not defined at the same time... 🤯

I also can't imagine that it's nice to work with such an API, e.g. imagine:

  • You read the data of an existing entity over our admin API
  • You modify some value
  • You want to update that entity with the same payload

I think this was possible in the past (correct me if I'm wrong), but now you need to remove certain fields from your payload to send the update request, which is really annoying.

Of course in our administration codebase this is a non-issue, because there we use some extra logic / magic to build a changeset diff and only send that back to the server, but do you really expect every other API client to do the same or treat it differently like we do here in the migration? 😬

I would suggest either:

  1. Allowing updates in the DAL when the value of the Immutable field stays the same (is unchanged), as it should be in these cases like here in the "upsert" case for migration. Otherwise it would be a valid error if the migration tries to change such a value
  2. Or "silently" ignore that field in the DAL update case, without throwing an error. Maybe it should still create a log entry though 🤷‍♂️

I'm open for other suggestions as well, but I think we should have a better answer for the "upsert" case than just deal with it by throwing extra code at it. The Migration-Assistant makes heave use of "upsert" and basically writes all it's data that way:

SwagMigrationAssistant/src/Migration/Writer/AbstractWriter.php

Line 60 in 5a9afd8

$writeResults = $this->entityWriter->upsert(

cc: @patzick @keulinho what's your opinion on this?

Copy link
Contributor

@vienthuong vienthuong Jan 20, 2026

Choose a reason for hiding this comment

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

That do makes sense, I was also thinking about the update should allow the payload with immutable key if the value doesn't change, but at the query time, we haven't aware of the current value in the database, fetching it before wouldn't be ideal, I will have a look further at this problem and your suggestion and provide an improvement for the flag.

For an external API consumer, I think ideally they don't want to send the whole entities; a subset of what changes is more practical and lightweight. But there might be cases where they want to resend the entire payload and catch the immutable error, as in your mentioned scenario.

Another simple option is to bypass the immutable check for system context scope; it will work for examples like MIG, but not if they update the entities via the admin api.

Copy link
Contributor

@vienthuong vienthuong Jan 20, 2026

Choose a reason for hiding this comment

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

Allowing updates in the DAL when the value of the Immutable field stays the same (is unchanged), as it should be in these cases like here in the "upsert" case for migration. Otherwise it would be a valid error if the migration tries to change such a value

This should be possible, I added a draft PR here: shopware/shopware#14422

$existingMapping = $this->mappingService->getMapping(
$this->connectionId,
DefaultEntities::CUSTOM_FIELD_SET,
$data['id'],
$this->context
);
$isUpdate = $existingMapping !== null;

$this->mainMapping = $this->getOrCreateMappingMainCompleteFacade(
DefaultEntities::CUSTOM_FIELD_SET,
$data['id'],
$converted['id']
);

$this->removeImmutableFields($converted, $isUpdate);

return new ConvertStruct($converted, null, $this->mainMapping['id'] ?? null);
}

/**
* @param array<string, mixed> $converted
*/
private function removeImmutableFields(array &$converted, bool $isUpdate): void
{
if (!$isUpdate) {
return;
}

unset($converted['name']);

if (isset($converted['customFields']) && \is_array($converted['customFields'])) {
foreach ($converted['customFields'] as &$customField) {
unset($customField['name'], $customField['type']);
}
unset($customField);
}
}
}
70 changes: 70 additions & 0 deletions src/Profile/Shopware6/Converter/ProductConverter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,15 @@

namespace SwagMigrationAssistant\Profile\Shopware6\Converter;

use Doctrine\DBAL\Connection;
use Shopware\Core\Content\Product\ProductDefinition;
use Shopware\Core\Defaults;
use Shopware\Core\Framework\Log\Package;
use SwagMigrationAssistant\Migration\Converter\ConvertStruct;
use SwagMigrationAssistant\Migration\DataSelection\DefaultEntities;
use SwagMigrationAssistant\Migration\Logging\LoggingServiceInterface;
use SwagMigrationAssistant\Migration\Mapping\MappingServiceInterface;
use SwagMigrationAssistant\Migration\Media\MediaFileServiceInterface;
use SwagMigrationAssistant\Migration\MigrationContextInterface;
use SwagMigrationAssistant\Profile\Shopware6\DataSelection\DataSet\ProductDataSet;
use SwagMigrationAssistant\Profile\Shopware6\Shopware6MajorProfile;
Expand All @@ -20,6 +25,17 @@ class ProductConverter extends ShopwareMediaConverter
{
private ?string $sourceDefaultCurrencyUuid;

private ?bool $hasTypeColumn = null;

public function __construct(
MappingServiceInterface $mappingService,
LoggingServiceInterface $loggingService,
MediaFileServiceInterface $mediaFileService,
private readonly Connection $connection,
) {
parent::__construct($mappingService, $loggingService, $mediaFileService);
}

public function supports(MigrationContextInterface $migrationContext): bool
{
return $migrationContext->getProfile()->getName() === Shopware6MajorProfile::PROFILE_NAME
Expand Down Expand Up @@ -58,6 +74,16 @@ protected function convertData(array $data): ConvertStruct
{
$converted = $data;

// Check if product already exists (was previously migrated) before creating mapping
// This is needed because the 'type' field has Immutable flag and can only be set on create
$existingMapping = $this->mappingService->getMapping(
$this->connectionId,
DefaultEntities::PRODUCT,
$data['id'],
$this->context
);
$isUpdate = $existingMapping !== null;
Comment on lines +77 to +85
Copy link
Contributor

@MalteJanz MalteJanz Jan 19, 2026

Choose a reason for hiding this comment

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

At first look, this is a clever attempt to determine if this entity will run into an insert or update. But unfortunately it contains a flaw 🙈 .

Yes on the first migration run this will tell you if the product "was previously attempted to migrate" and thus give the the wanted result.

But if the migration for this entity somehow fails:

  • either further down in this converter
  • in the validation layer that comes after - which we currently implement in our feature branch
  • or later while writing the entity to the corresponding SW6 table

it would mean the next migration run (retry) would still consider this entity as an "update", because the mapping already exists, which produces the wrong outcome in this case.

We have to think further about this, to not introduce yet another N+1 Query performance issue 😬


$this->mainMapping = $this->getOrCreateMappingMainCompleteFacade(
DefaultEntities::PRODUCT,
$data['id'],
Expand Down Expand Up @@ -205,6 +231,8 @@ protected function convertData(array $data): ConvertStruct
);
}

$this->convertStatesToType($converted, $isUpdate);

return new ConvertStruct($converted, null, $this->mainMapping['id'] ?? null);
}

Expand Down Expand Up @@ -235,4 +263,46 @@ private function checkDefaultCurrency(array &$source, string $key): void
$source[$key][] = $defaultPrice;
}
}

/**
* @param array<string, mixed> $converted
*/
private function convertStatesToType(array &$converted, bool $isUpdate): void
{
if (!$this->hasTypeColumn()) {
return;
}

if ($isUpdate) {
unset($converted['type']);

return;
}

if (isset($converted['type'])) {
return;
}

if (isset($converted['states']) && \is_array($converted['states'])) {
$converted['type'] = \in_array('is-download', $converted['states'], true)
? ProductDefinition::TYPE_DIGITAL
: ProductDefinition::TYPE_PHYSICAL;

return;
}

$converted['type'] = ProductDefinition::TYPE_PHYSICAL;
}

private function hasTypeColumn(): bool
{
if ($this->hasTypeColumn !== null) {
return $this->hasTypeColumn;
}

$columns = $this->connection->createSchemaManager()->listTableColumns('product');
$this->hasTypeColumn = isset($columns['type']);

return $this->hasTypeColumn;
}
}
14 changes: 13 additions & 1 deletion tests/Profile/Shopware6/Converter/ProductConverterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@

namespace SwagMigrationAssistant\Test\Profile\Shopware6\Converter;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Schema\AbstractSchemaManager;
use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Types\StringType;
use Shopware\Core\Framework\Log\Package;
use SwagMigrationAssistant\Migration\Converter\ConverterInterface;
use SwagMigrationAssistant\Migration\DataSelection\DataSet\DataSet;
Expand All @@ -25,7 +29,15 @@ protected function createConverter(
MediaFileServiceInterface $mediaFileService,
?array $mappingArray = [],
): ConverterInterface {
return new ProductConverter($mappingService, $loggingService, $mediaFileService);
$schemaManager = $this->createMock(AbstractSchemaManager::class);
$schemaManager->method('listTableColumns')->willReturn([
'type' => new Column('type', new StringType()),
]);

$connection = $this->createMock(Connection::class);
$connection->method('createSchemaManager')->willReturn($schemaManager);

return new ProductConverter($mappingService, $loggingService, $mediaFileService, $connection);
}

protected function createDataSet(): DataSet
Expand Down
1 change: 1 addition & 0 deletions tests/_fixtures/Shopware6/Product/01-HappyCaseContainer/output.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,5 @@
'id' => 'd7e9ceac19a948abad07667419424b13',
],
],
'type' => 'physical',
];
1 change: 1 addition & 0 deletions tests/_fixtures/Shopware6/Product/02-HappyCaseVariant/output.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@
'id' => 'bfaf0c7366e6454fb7516ab47435b01a',
],
],
'type' => 'physical',
];
1 change: 1 addition & 0 deletions tests/_fixtures/Shopware6/Product/03-MissingManufacturerMapping/output.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,5 @@
'id' => 'd7e9ceac19a948abad07667419424b13',
],
],
'type' => 'physical',
];
1 change: 1 addition & 0 deletions tests/_fixtures/Shopware6/Product/04-StripUnmigrateableData/output.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,5 @@
'id' => 'd7e9ceac19a948abad07667419424b13',
],
],
'type' => 'physical',
];
1 change: 1 addition & 0 deletions tests/_fixtures/Shopware6/Product/05-AdvancedPrices/output.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,4 +139,5 @@
],
'coverId' => 'bdeb106f47ab4255b2bd5f35d84cae7c',
'id' => 'fb2dbbee297c472c9e916b26952615ff',
'type' => 'physical',
];
1 change: 1 addition & 0 deletions tests/_fixtures/Shopware6/Product/06-DeliveryTimes/output.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,5 @@
'coverId' => 'bdeb106f47ab4255b2bd5f35d84cae7c',
'deliveryTimeId' => 'bdeb106f47ab4255b2bd5f35d84cae7c',
'id' => 'fb2dbbee297c472c9e916b26952615ff',
'type' => 'physical',
];
1 change: 1 addition & 0 deletions tests/_fixtures/Shopware6/Product/07-Media/output.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,5 @@
'coverId' => 'eb5483a9c77c4919b5d110e8d745a1cc',
'deliveryTimeId' => 'bdeb106f47ab4255b2bd5f35d84cae7c',
'id' => 'fb2dbbee297c472c9e916b26952615ff',
'type' => 'physical',
];
1 change: 1 addition & 0 deletions tests/_fixtures/Shopware6/Product/08-MigrateWithoutDefaultCurrency/output.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,4 +209,5 @@
'id' => 'd7e9ceac19a948abad07667419424b13',
],
],
'type' => 'physical',
];
1 change: 1 addition & 0 deletions tests/_fixtures/Shopware6/Product/09-MigrateWithConfiguratorSettings/output.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,4 +121,5 @@
'coverId' => 'eb5483a9c77c4919b5d110e8d745a1cc',
'deliveryTimeId' => 'bdeb106f47ab4255b2bd5f35d84cae7c',
'id' => 'fb2dbbee297c472c9e916b26952615ff',
'type' => 'physical',
];