Memoise the discounts a cart has already consumed - #2638
Merged
glennjacobs merged 3 commits intoAug 26, 2026
Conversation
The memo was primed before the creation pipeline wrote the breakdown it reads, so it stayed empty for the rest of the request: a same-request retry read the cart's own coupon as unconsumed in CreateOrder, and as exhausted in the discount conditions, re-pricing the order without it. Invalidating on the one event that changes the answer covers both, and removes the need for the fresh flag on a public model method. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
glennjacobs
approved these changes
Aug 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #2637, as suggested in review.
Cart::consumedDiscountIds()re-queried the draft order on every call: once whenthe discount set is rebuilt, and once per discount while conditions are checked.
That is D+1 single-row lookups on every calculate, paid by every cart including
those that never reach a checkout.
What changes
The result is memoised on the cart instance. Measured on a cart with four active
discounts, for one
calculate():The trap, and how it is handled
A memo primed before the first order exists is empty, and stays empty. On a
same-request retry
CreateOrderwould then see nothing consumed, and consume thediscount a second time — exactly the double-consumption #2637 was written to
prevent.
Order creation is the only thing that invalidates the set, because it writes the
breakdown the method reads. So
consumedDiscountIds()takes a$freshflag andCreateOrderpasses it:Everything else reads the memo. The parameter defaults to false, so no other
caller changes.
Tests
tests/core/Unit/Actions/Carts/CreateOrderTest.php:can not consume a discount twice on one cart instance — two order
creations on the same cart object, with no reload between them, asserting
usesstays at 1. The existing#2637tests reload the cart between attempts,so none of them cover this.
It passes on
1.xtoday, and it is the guard for this change: with the memoadded and the
freshread left out, it fails withFailed asserting that 2 matches expected 1— the double consumption backagain. Verified by writing the naive memo first and watching it fail.