Skip to content

Apply discounts on guest carts when max_uses_per_user is set - #2470

Draft
glennjacobs wants to merge 1 commit into
1.xfrom
fix/coupon-max-uses-per-user-no-effect-2442
Draft

Apply discounts on guest carts when max_uses_per_user is set#2470
glennjacobs wants to merge 1 commit into
1.xfrom
fix/coupon-max-uses-per-user-no-effect-2442

Conversation

@glennjacobs

Copy link
Copy Markdown
Contributor

Summary

  • AbstractDiscountType::checkDiscountConditions() now only runs the per-user usage check when the cart actually has a user. Guest carts with max_uses_per_user set no longer silently fail the condition.
  • Added three tests covering guest cart, under-limit user, at-limit user.

Why

The old condition was:

if ($validMaxUses && $this->discount->max_uses_per_user) {
    $validMaxUses = $cart->user && ($this->usesByUser($cart->user) < $this->discount->max_uses_per_user);
}

If the cart had no user attached (guest checkout, or anonymous cart pre-login), $cart->user was null, so $validMaxUses became false and 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 $validMaxUses was already set to:

if ($validMaxUses && $this->discount->max_uses_per_user && $cart->user) {
    $validMaxUses = $this->usesByUser($cart->user) < $this->discount->max_uses_per_user;
}

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 passing
  • Manual: discount with max_uses_per_user = 1 on a guest cart → discount applies to total
  • Manual: same discount on a logged-in user who has already used it → total unchanged

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
@ryanmitchell

Copy link
Copy Markdown
Contributor

Does this mean that the discount can be used indefinitely by a non-logged in user?
If so would it not be better to reject the discount code if there is no logged in user.

@glennjacobs
glennjacobs marked this pull request as draft May 19, 2026 09:31
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Coupon Applies but there is no effect on prices/calculations

2 participants