Skip to content

Conversation

@MnkyArts
Copy link

1. Why is this change necessary?

closes shopware/shopware#14051

The Migration Assistant does not populate the primaryOrderDeliveryId and primaryOrderTransactionId fields when migrating orders from Shopware 5 to Shopware 6. These fields were introduced in Shopware 6.7 and are required for proper order handling.

2. What does this change do, exactly?

This change updates both OrderConverters to set the primary order delivery and transaction IDs:

  • Shopware 5 → 6 (Profile/Shopware/Converter/OrderConverter.php): Sets primaryOrderDeliveryId to the first delivery's ID and primaryOrderTransactionId to the first transaction's ID after they are created.

  • Shopware 6 → 6 (Profile/Shopware6/Converter/OrderConverter.php): Sets primaryOrderDeliveryId and primaryOrderTransactionId from the first delivery/transaction if deliveries and transactions exist.

This mirrors the behavior of Shopware 6's core OrderConverter which sets these fields when converting a cart to an order.

3. Describe each step to reproduce the issue or behaviour.

  1. Set up a Shopware 5 shop with existing orders
  2. Install the Migration Assistant in a Shopware 6.7+ shop
  3. Run a migration for orders
  4. Check the migrated orders in the database: SELECT id, primary_order_delivery_id, primary_order_transaction_id FROM \order``
  5. Before fix: Both fields are NULL
  6. After fix: Both fields are populated with the correct delivery/transaction IDs

5. Checklist

  • I have created the PR in draft status and only open it when it's ready for review
  • If the change is large: I have thought about splitting it up into smaller PRs
  • I have written tests and if it's a bug fix verified that they fail without my change or that new code is covered by tests
  • I have updated developer-facing release notes if this change affects external developers
  • I have written or adjusted the documentation according to my changes
  • I added the correct package annotation to each src file (not strictly necessary for tests)
  • This change has code comments where appropriate, especially for non-obvious lines of code
  • Ensure the pull request title as well as first commit follows conventional commits
  • I have thought about well-defined extension points and marked everything else as @internal / @private

Copy link
Contributor

@MalteJanz MalteJanz left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution 🚀

$converted['primaryOrderDeliveryId'] = $converted['deliveries'][0]['id'];
}
if (!isset($converted['primaryOrderTransactionId']) && !empty($converted['transactions'])) {
$converted['primaryOrderTransactionId'] = $converted['transactions'][0]['id'];
Copy link
Contributor

@MalteJanz MalteJanz Dec 19, 2025

Choose a reason for hiding this comment

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

Looks like PHPStan isn't happy about this, because it thinks the array has a shape of array<string,mixed> as defined here:

$this->updateAssociationIds(
$converted['transactions'],
DefaultEntities::PAYMENT_METHOD,
'paymentMethodId',
DefaultEntities::ORDER
);

* Replaces every id (associationIdKey) in the specified array of entities with the right mapping dependent one.
*
* @param array<string, mixed> $associationArray
*/
protected function updateAssociationIds(array &$associationArray, string $entity, string $associationIdKey, string $sourceEntity, bool $logMissing = true, bool $unsetMissing = false): void

But like in this case, in practice it can also have integer keys. Would be nice if you could fix this to get the CI checks green 🙂

Copy link
Author

Choose a reason for hiding this comment

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

Gonna need to find some time, but going to do it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external-contribution A PR contributed by a community member.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Order overview don't show payment state, delivery state and delivery address

6 participants