Skip to content

Memoise the discounts a cart has already consumed - #2638

Merged
glennjacobs merged 3 commits into
lunarphp:1.xfrom
kha333n:fix/memoise-consumed-discount-ids
Aug 26, 2026
Merged

Memoise the discounts a cart has already consumed#2638
glennjacobs merged 3 commits into
lunarphp:1.xfrom
kha333n:fix/memoise-consumed-discount-ids

Conversation

@kha333n

@kha333n kha333n commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #2637, as suggested in review.

Cart::consumedDiscountIds() re-queried the draft order on every call: once when
the 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():

draft-order lookups total queries
before 5 47
after 1 43

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 CreateOrder would then see nothing consumed, and consume the
discount 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 $fresh flag and
CreateOrder passes it:

$alreadyConsumed = $cart->consumedDiscountIds(fresh: true);

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
    uses stays at 1. The existing #2637 tests reload the cart between attempts,
    so none of them cover this.

    It passes on 1.x today, and it is the guard for this change: with the memo
    added and the fresh read left out, it fails with
    Failed asserting that 2 matches expected 1 — the double consumption back
    again. Verified by writing the naive memo first and watching it fail.

@github-project-automation github-project-automation Bot moved this to Todo in Roadmap Aug 25, 2026
@kha333n
kha333n marked this pull request as ready for review August 25, 2026 16:08
glennjacobs and others added 2 commits August 26, 2026 13:23
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
glennjacobs merged commit 2fa7a94 into lunarphp:1.x Aug 26, 2026
14 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in Roadmap Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants