Memoise the discounts a cart has already consumed

packages/core/src/Actions/Carts/CreateOrder.php

@@ -60,6 +60,10 @@ public function execute(
                 $discount->markAsUsed($cart)->discount->save();
             });
 
+            // The breakdown has been rewritten, so anything still holding this
+            // cart must not read a set memoised before the order existed.
+            $cart->forgetConsumedDiscountIds();
+
             $cart->save();
 
             MarkAsNewCustomer::dispatch($order->id);

packages/core/src/Models/Cart.php

@@ -212,6 +212,11 @@ protected static function newFactory()
         'coupon_code' => CouponString::class,
     ];
 
+    /**
+     * Memoised result of {@see self::consumedDiscountIds()}.
+     */
+    protected ?Collection $consumedDiscountIds = null;
+
     public function lines(): HasMany
     {
         return $this->hasMany(CartLine::modelClass(), 'cart_id', 'id')->orderBy('id');
@@ -291,20 +296,46 @@ public function draftOrder(?int $draftOrderId = null): HasOne
      * checkout that runs it a second time - a declined card and a retry - would
      * otherwise find its own coupon exhausted and re-price that same order
      * without it. A cart's own consumption must not count against it.
+     *
+     * Memoised per instance, because this is read once when the discount set is
+     * rebuilt and once per discount while conditions are checked - a fresh query
+     * each time costs a single-row lookup per discount on every calculate,
+     * including for carts that never reach a checkout. Order creation is the
+     * only thing that changes the answer, so it forgets the memo.
+     *
+     * @see self::forgetConsumedDiscountIds()
      */
     public function consumedDiscountIds(): Collection
     {
+        if ($this->consumedDiscountIds !== null) {
+            return $this->consumedDiscountIds;
+        }
+
         // Read the raw column: the cast hydrates an OrderLine per breakdown
         // line, which is a lot of work to reach an id.
         $breakdown = $this->draftOrder()->first()?->getRawOriginal('discount_breakdown');
 
-        return collect(json_decode($breakdown ?: '[]', true) ?: [])
+        return $this->consumedDiscountIds = collect(json_decode($breakdown ?: '[]', true) ?: [])
             ->pluck('discount_id')
             ->filter()
             ->unique()
             ->values();
     }
 
+    /**
+     * Forget the memoised consumed discount ids.
+     *
+     * Order creation writes the breakdown consumedDiscountIds() reads, so a set
+     * memoised before it ran is stale afterwards: on a same-request retry the
+     * cart's own coupon would look unconsumed to CreateOrder and be consumed
+     * twice, and exhausted to the discount conditions, re-pricing the order
+     * without it.
+     */
+    public function forgetConsumedDiscountIds(): void
+    {
+        $this->consumedDiscountIds = null;
+    }
+
     public function currentDraftOrder(?int $draftOrderId = null)
     {
         return $this->calculate()

tests/core/Unit/Actions/Carts/CreateOrderTest.php

@@ -618,3 +618,166 @@ function can_update_draft_order()
     expect($orderB->sub_total->value)->toEqual(1000);
     expect($orderB->discount_total->value)->toEqual(0);
 });
+
+test('can not consume a discount twice on one cart instance', function () {
+    TaxClass::factory()->create([
+        'default' => true,
+    ]);
+
+    $customerGroup = CustomerGroup::factory()->create([
+        'default' => true,
+    ]);
+
+    $channel = Channel::factory()->create([
+        'default' => true,
+    ]);
+
+    $currency = Currency::factory()->create([
+        'decimal_places' => 2,
+    ]);
+
+    $cart = Cart::factory()->create([
+        'currency_id' => $currency->id,
+        'channel_id' => $channel->id,
+        'coupon_code' => 'SAVE10',
+    ]);
+
+    $purchasable = ProductVariant::factory()->create();
+
+    Price::factory()->create([
+        'price' => 1000,
+        'min_quantity' => 1,
+        'currency_id' => $currency->id,
+        'priceable_type' => $purchasable->getMorphClass(),
+        'priceable_id' => $purchasable->id,
+    ]);
+
+    $cart->lines()->create([
+        'purchasable_type' => $purchasable->getMorphClass(),
+        'purchasable_id' => $purchasable->id,
+        'quantity' => 2,
+    ]);
+
+    $discount = Discount::factory()->create([
+        'type' => AmountOff::class,
+        'name' => 'Ten off',
+        'coupon' => 'SAVE10',
+        'uses' => 0,
+        'max_uses' => 1,
+        'data' => [
+            'fixed_value' => true,
+            'fixed_values' => [
+                $currency->code => 500,
+            ],
+        ],
+    ]);
+
+    $discount->customerGroups()->sync([
+        $customerGroup->id => ['enabled' => true, 'starts_at' => now()->subHour()],
+    ]);
+
+    $discount->channels()->sync([
+        $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()],
+    ]);
+
+    // The same instance throughout: no reload between the two attempts. This is
+    // what a checkout that retries in one request looks like, and it is the case
+    // any memoisation of consumedDiscountIds() has to survive - a set cached
+    // before the first order exists would still be empty for the second.
+    $cart->calculate();
+
+    $orderA = (new CreateOrder)->execute($cart)->then(fn ($order) => $order->refresh());
+
+    expect($orderA->discount_total->value)->toEqual(500);
+    expect($discount->refresh()->uses)->toEqual(1);
+
+    $cart->calculate();
+
+    $orderB = (new CreateOrder)->execute($cart)->then(fn ($order) => $order->refresh());
+
+    expect($orderB->id)->toEqual($orderA->id);
+    expect($orderB->discount_total->value)->toEqual(500);
+    expect($discount->refresh()->uses)->toEqual(1);
+});
+
+test('keeps its own discount when a cart is priced again after order creation', function () {
+    TaxClass::factory()->create([
+        'default' => true,
+    ]);
+
+    $customerGroup = CustomerGroup::factory()->create([
+        'default' => true,
+    ]);
+
+    $channel = Channel::factory()->create([
+        'default' => true,
+    ]);
+
+    $currency = Currency::factory()->create([
+        'decimal_places' => 2,
+    ]);
+
+    $cart = Cart::factory()->create([
+        'currency_id' => $currency->id,
+        'channel_id' => $channel->id,
+        'coupon_code' => 'SAVE10',
+    ]);
+
+    $purchasable = ProductVariant::factory()->create();
+
+    Price::factory()->create([
+        'price' => 1000,
+        'min_quantity' => 1,
+        'currency_id' => $currency->id,
+        'priceable_type' => $purchasable->getMorphClass(),
+        'priceable_id' => $purchasable->id,
+    ]);
+
+    $cart->lines()->create([
+        'purchasable_type' => $purchasable->getMorphClass(),
+        'purchasable_id' => $purchasable->id,
+        'quantity' => 2,
+    ]);
+
+    $discount = Discount::factory()->create([
+        'type' => AmountOff::class,
+        'name' => 'Ten off',
+        'coupon' => 'SAVE10',
+        'uses' => 0,
+        'max_uses' => 1,
+        'data' => [
+            'fixed_value' => true,
+            'fixed_values' => [
+                $currency->code => 500,
+            ],
+        ],
+    ]);
+
+    $discount->customerGroups()->sync([
+        $customerGroup->id => ['enabled' => true, 'starts_at' => now()->subHour()],
+    ]);
+
+    $discount->channels()->sync([
+        $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()],
+    ]);
+
+    $cart->calculate();
+
+    $orderA = (new CreateOrder)->execute($cart)->then(fn ($order) => $order->refresh());
+
+    expect($orderA->discount_total->value)->toEqual(500);
+    expect($discount->refresh()->uses)->toEqual(1);
+
+    // Priced again on the same instance, with the discount set rebuilt: the
+    // cart's own use must not read as an exhausted coupon, or the retry is
+    // re-priced without the discount the shopper was quoted.
+    Discounts::resetDiscounts();
+
+    $cart->recalculate();
+
+    $orderB = (new CreateOrder)->execute($cart)->then(fn ($order) => $order->refresh());
+
+    expect($orderB->id)->toEqual($orderA->id);
+    expect($orderB->discount_total->value)->toEqual(500);
+    expect($discount->refresh()->uses)->toEqual(1);
+});