Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Spun off from #2934647: The order can stay unplaced after an offsite payment.
The problem is that the "order paid" event is fired too late in the preSave process for us to be able to modify the order state. That causes only the post_transition events to fire.
An entity should not be modified after its field hooks have already fired. This can lead to critical issues.
Currently both the order paid event and the order refresh event fire after the field hooks have already fired.
The preSave process needs to go like this:
1. Default properties
2. Order refresh
3. Recalculate total
4. Order paid in full
5. Presave hooks
We let Order::preSave() handle the default properties. We keep 2-4 in the storage cause it's cleaner (DI instead of \Drupal in the already-too-huge entity class).
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 3008959-6.patch | 5.09 KB | bojanz |
| |||
| #2 | 3008959-2.patch | 5.85 KB | bojanz |
Comments
Comment #2
bojanz CreditAttribution: bojanz at Centarro commentedLet's see what the tests say.
Comment #4
zaporylieIt seems like test fails for newly-created-but-already-paid order. $this->isPaid() returns NULL, because recalculateTotalPrice() have never been triggered on the order (it happens too late - in OrderStorage::doPreSave). For that very reason OrderEvents::ORDER_PAID event was never dispatched which made the test fail.
Comment #5
bojanz CreditAttribution: bojanz at Centarro commentedUpdating issue summary. The problem is larger than I initially thought.
The test fail from above could have been worked around by recalculating the order total before the event is fired, but that still leaves two fundamental problems:
1) The order refresh can change the order total, making the paid check unprecise
2) The order refresh process also happens after the field hooks have been invoked, which is potentially very problematic
Comment #6
bojanz CreditAttribution: bojanz at Centarro commentedComment #8
bojanz CreditAttribution: bojanz at Centarro commentedCommitted.
Comment #9
agoradesign CreditAttribution: agoradesign commentedyessssssss :)