Clear broken order items reference on presave

+++ b/modules/order/tests/src/Kernel/Entity/OrderTest.php
@@ -342,6 +342,30 @@ class OrderTest extends OrderKernelTestBase {
+    // Assert that saving the order fixes references to a deleted order item.
+    $order->save();

Core prevents references to non-existent (possibly deleted) items in the entity validation step. It is always necessary to call validate() and check if there any violations before saving an entity.

Given that, I'm not sure the fix is actually fixing anything..

@jsacksick you've actually committed new test code + some comments in the order storage, as you've commented out the fix:

//$order->set('order_items', $order->getItems());

@jsacksick, the problem with the test is that it's not testing a real-world scenario. You need to change the test so it calls validate()/code> before <code>save(), and don't try to save if there are any violations found.

You're welcome :)

Why isn't that real-world? If the test doesn't lead to having validate() called, why should that be guaranteed in real world? I'm unsure baout that assumption

@agoradesign, because the entity system does not (and can not) guarantee data integrity if you don't validate an entity before saving it, that's how the Entity API works. Any real world code that calls save() without validating before is broken and needs to be fixed.

Well to me the fix is non controversial

IMO the fix is not ok because it prevents the actual bugs from being discovered at all by hiding the problem under the rug. This means that there are way less chances to find instances of broken code and open issues for them.

And this isn't the only usecase the patch is addressing. If an order item is being deleted, the next time the order will be saved, the reference will remain, and that code fixes that.

If removing an order item outside of a "cart context" is a problem, then the deletion should be prevented by the order item storage.