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.
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
Order Item Repository saving and deleting items on the null
key in registry
#36172
base: 2.4-develop
Are you sure you want to change the base?
Order Item Repository saving and deleting items on the null
key in registry
#36172
Conversation
Hi @ioiste. Thank you for your contribution
❗ Automated tests can be triggered manually with an appropriate comment:
You can find more information about the builds here ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review. For more details, review the Magento Contributor Guide documentation. 🕙 You can find the schedule on the Magento Community Calendar page. 📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket. ✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel |
…leting-items-on-null-in-registry
Failed to run the builds. Please try to re-run them later. |
@magento run all tests |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
@magento run Functional Tests CE, Functional Tests EE |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
…leting-items-on-null-in-registry
…leting-items-on-null-in-registry
…leting-items-on-null-in-registry
…leting-items-on-null-in-registry
…leting-items-on-null-in-registry
…leting-items-on-null-in-registry
Description (*)
Hi, Magento 2 team! We noticed an issue. While saving or deleting Order Items via
Magento/Sales/Model/Order/ItemRepository
, the entities are stored in the$this->registry
array using$entity->getEntityId()
as the key.However,
entity_id
($entity->getEntityId()
) does not exist on the Order Item and, while saving or deleting,it would set the Order Item object in
$this->registry
on thenull
key. This might cause issues in some cases.Consider the following scenario:
Later on, using the exact same
ItemRepository
instance, you want to get the order item with id1234
, so you do:$orderItem = ItemRepository->get(1234);
- It checks theregistry
for that item, finds it for the1234
key and then returns that entity. That entity won't be the one saved earlier (since that one is on thenull
key), thus being outdated.$orderItem->getQtyCanceled()
will not be the expected2
.The proper field to use as the key would be
item_id
($entity->getItemId()
).It returns the Order Item ID, which, besides fixing the issue described above, it makes sure the repository can properly handle saving multiple subsequent elements in the
$this->registry
array (since before you would override the previous object in registry because you'd set the new object on the same key -null
).You can also see the issue if you debug through the code:
Please let me know if this is not clear enough. I can try to explain better.
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
I'm not sure how to manually test this, since you'd have to write code to reproduce what I described in the scenario above. I don't know places in Magento 2 CE where that really happens.
We came across this issue while developing something custom.
Questions or comments
Contribution checklist (*)