Apply discounts on guest carts when max_uses_per_user is set - #2470
Draft
glennjacobs wants to merge 1 commit into
Draft
Apply discounts on guest carts when max_uses_per_user is set#2470glennjacobs wants to merge 1 commit into
glennjacobs wants to merge 1 commit into
Conversation
checkDiscountConditions() short-circuited validMaxUses to false on any cart without a user the moment max_uses_per_user was set, which meant guest carts saw the coupon "applied" but got zero discount. The per-user limit can only be enforced when a user is known, so only run the check in that case and otherwise leave the existing validMaxUses value alone. Fixes #2442
Contributor
|
Does this mean that the discount can be used indefinitely by a non-logged in user? |
glennjacobs
marked this pull request as draft
May 19, 2026 09:31
This was referenced Aug 24, 2026
glennjacobs
added a commit
that referenced
this pull request
Aug 25, 2026
A merchant sets up "spend £50, get one free" and it fires on a £20 cart.
A
discount restricted to named customers applies to everyone.
### What happens now
- The minimum spend on a BuyXGetY discount has no effect — the reward is
given
at any cart value.
- A BuyXGetY discount restricted to specific customers applies to every
shopper.
- `max_uses_per_user` is likewise not consulted.
- The quantity condition (`min_qty`) is the only thing actually gating
it.
### What should happen
- The discount applies only when the cart meets the advertised minimum
spend.
- Customer restrictions and per-user limits are respected, as they are
for
`AmountOff`.
### Why
`AbstractDiscountType::checkDiscountConditions()` enforces minimum
spend,
customer restrictions and `max_uses_per_user`. `AmountOff::apply()`
guards on it
first thing:
```php
if (! $this->checkDiscountConditions($cart)) {
return $cart;
}
```
`BuyXGetY::apply()` has no such call — the method is never referenced in
the
class.
Worth noting what is *not* affected, since it narrows the blast radius:
dates,
`max_uses`, the coupon and the channel are all enforced by the query in
`DiscountManager::getDiscounts()` (`active()`, `usable()`, and the
coupon
`when()` clause), so those keep working today. Only the three conditions
checked
solely inside `checkDiscountConditions()` are being skipped.
### The fix
Add the same guard `AmountOff` already uses, at the top of
`BuyXGetY::apply()`.
Nothing else in the class changes, and no condition logic is duplicated
— it
routes through the shared method so the two discount types stay
consistent.
One interaction worth flagging: this means BuyXGetY now also inherits
the
guest-cart behaviour of `max_uses_per_user` that #2470 is addressing.
That seems
right — it is the same rule `AmountOff` already follows — but if #2470
lands
first, both types will pick up its fix together.
### Tests
`tests/core/Unit/DiscountTypes/BuyXGetYConditionsTest.php`:
- **is not applied when the cart is below the minimum spend** — £50
minimum on a
£20 cart. Fails on `1.x` with `Failed asserting that 1000 matches
expected 0`.
- **is not applied when the cart customer is not on the discount** —
same
failure on `1.x`.
- **is applied when the cart meets the minimum spend** — passes before
and after,
so the guard is not simply switching the discount off.
Co-authored-by: Glenn Jacobs <glenn@neondigital.co.uk>
glennjacobs
added a commit
that referenced
this pull request
Aug 25, 2026
A shopper is quoted a total with a single-use coupon applied. Their card
is
declined, they pay with another, and they are charged full price — on
the same
order, with the coupon now spent.
### What happens now
- The first checkout attempt consumes the coupon, before any payment is
taken.
- The retry rebuilds the same draft order, finds the coupon exhausted,
and
rewrites that order without the discount.
- The shopper cannot re-apply the code: as far as the store is concerned
it is
used up.
- A checkout that rebuilds its draft order on each step consumes a use
per step,
so `max_uses` and `max_uses_per_user` drain without a single sale.
### What should happen
- Creating the order again for the same cart prices it the same way.
- One order consumes one use, however many times order creation runs for
it.
### Why
`CreateOrder` records a use on every execution:
```php
$cart->discounts?->each(function ($discount) use ($cart) {
$discount->markAsUsed($cart)->discount->save();
});
```
Nothing records that this cart already consumed the discount. On the
second run
the cart is re-priced first, and both gates that check the use count —
`Discount::scopeUsable()` in `DiscountManager::getDiscounts()`, and the
`max_uses` / `max_uses_per_user` checks in
`AbstractDiscountType::checkDiscountConditions()` — now exclude the very
coupon
this cart spent. The discount never reaches `$cart->discounts`, and
`FillOrderFromCart` and `MapDiscountBreakdown` rewrite the existing
order rows
at full price.
### The fix
A cart's own consumption stops counting against it.
`Cart::consumedDiscountIds()`
reads the discount ids already recorded on that cart's draft order, and
both
gates treat those as satisfying the use limit — **only** the use limit.
Coupon
match, minimum spend, customer restrictions, dates, channel and customer
group
are all still enforced, so a cart that drops below the minimum spend
loses the
discount exactly as it does today.
The counterpart matters as much: `CreateOrder` now skips `markAsUsed()`
for a
discount the draft order already recorded, read *before* the creation
pipeline
runs, since `MapDiscountBreakdown` rewrites that breakdown. Without it
the
relaxed gate would let a retry consume a second use, which is worse than
the
bug.
Deliberately not changed: discounts are still consumed when the draft
order is
created, not when the order is placed. Moving consumption to placement
fixes
abandoned checkouts too, but lets two shoppers both reach payment on the
last
use of a single-use coupon before either is placed — trading a
merchandising
annoyance for over-redemption. That is a bigger decision than this
defect needs,
and a reservation scheme that avoids both needs a table and an expiry
job.
`Cart::consumedDiscountIds()` is additive and not on the `Cart`
contract, so it
is called through the existing `/** @var Cart $cart */` narrowing the
codebase
already uses. `scopeUsable()` takes an optional argument and is
unchanged when
it is omitted. #2470 is currently reworking the same `max_uses_per_user`
branch
for guest carts; the two do not conflict in intent, but whichever lands
second
wants a rebase.
### Tests
`tests/core/Unit/Actions/Carts/CreateOrderTest.php`:
- **can keep the discount when the draft order is created again** — a
single-use
coupon, order created twice. Fails on `1.x` with
`Failed asserting that 0 matches expected 500`, and would fail with
`Failed asserting that 2 matches expected 1` if only the gates were
relaxed.
- **can not reuse a discount another cart has exhausted** — a second
cart still
gets nothing. Passes before and after; it is the guard that the limit is
relaxed for one cart and not simply removed.
- **can still enforce other conditions on a discount the cart consumed**
— the
cart drops under the minimum spend on the retry and loses the discount,
so the
exemption is proved to cover the use count only.
Co-authored-by: Glenn Jacobs <glenn@neondigital.co.uk>
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.
Summary
AbstractDiscountType::checkDiscountConditions()now only runs the per-user usage check when the cart actually has a user. Guest carts withmax_uses_per_userset no longer silently fail the condition.Why
The old condition was:
If the cart had no user attached (guest checkout, or anonymous cart pre-login),
$cart->userwasnull, so$validMaxUsesbecamefalseand the discount was filtered out — but only at calculation time, after the coupon code had already been accepted onto the cart. From the customer's point of view the coupon "worked" but their total never changed. This is exactly what #2442 reported.A per-user limit only makes sense when a user is known, so the new condition skips the check entirely for guest carts and preserves whatever
$validMaxUseswas already set to:Logged-in users still hit the limit correctly — the third test asserts that an attached usage row blocks reuse.
Fixes #2442.
Test plan
vendor/bin/pest --testsuite=core— 506 passingmax_uses_per_user = 1on a guest cart → discount applies to total