Conversation
| $customFields = array_merge( | ||
| $customFields, | ||
| [ | ||
| self::ALTAPAY_TRANSACTION_ID_CUSTOM_FIELD => $altaPayTransactionId, | ||
| self::ALTAPAY_TRANSACTION_PAYMENT_SCHEME_NAME_CUSTOM_FIELD => $altaPayPaymentSchemeName, | ||
| self::ALTAPAY_TRANSACTION_PAYMENT_NATURE_CUSTOM_FIELD => $altaPayPaymentNature, | ||
| self::ALTAPAY_PAYMENT_ID_CUSTOM_FIELD => $altaPayPaymentId, | ||
| ] | ||
| ); |
There was a problem hiding this comment.
Is this really necessary?
The way I see it, you just need to insert the custom fields you want to add using the repository:
Rather than loading all the custom fields, merging them, and then inserting them again.
You risk that if the custom fields is changed while this operation is processing, then you will insert custom fields with an older value (race condition)
There was a problem hiding this comment.
@emicha ,
Order custom_fields are stored as JSON, but there is no clear guarantee that writing only part of that JSON will always keep the other existing fields.
There is also a Shopware core issue showing that existing custom_fields can be overwritten when a new array is written instead of merging first:
shopware/shopware#9613
We also need this to behave predictably across different Shopware versions used by merchants.
The safest long-term solution would be to use a separate table/entity instead of relying on the order custom_fields JSON. I have created a separate ticket for that:
https://developer.shopware.com/docs/guides/plugins/plugins/framework/data-handling/add-custom-compl…
So for this change, I think the explicit merge is the safer short-term fix, and the separate entity is the better long-term solution.
Ticket: PL-1598
2 participants
No description provided.