From 235f3f6dbde39c81e4a87ad7c9dda6d7b16f0d87 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Mon, 1 Jun 2026 09:21:28 +0100 Subject: [PATCH 1/3] Ensure automatic discounts can be fulfilled --- packages/core/src/DiscountTypes/BuyXGetY.php | 39 ++- .../core/Unit/DiscountTypes/BuyXGetYTest.php | 305 ++++++++++++++++++ 2 files changed, 337 insertions(+), 7 deletions(-) diff --git a/packages/core/src/DiscountTypes/BuyXGetY.php b/packages/core/src/DiscountTypes/BuyXGetY.php index 8f53db4c38..0e5f48b6a6 100644 --- a/packages/core/src/DiscountTypes/BuyXGetY.php +++ b/packages/core/src/DiscountTypes/BuyXGetY.php @@ -249,18 +249,37 @@ private function processAutomaticRewards(CartContract $cart, int $remainingRewar // we have lines to add if ($remainingRewardQty > 0) { - while ($remainingRewardQty > 0) { - $selectedRewardItem = $this->discount->discountableRewards->random()->discountable; + $fulfillableRewards = $this->discount->discountableRewards->filter(function ($discountableReward) { + $rewardItem = $discountableReward->discountable; - if (! $selectedRewardItem) { - $remainingRewardQty--; + if (! $rewardItem) { + return false; + } - continue; + if ($rewardItem instanceof LunarCollection) { + return $rewardItem->products() + ->with('variants') + ->get() + ->some(fn ($p) => $p->variants->first()?->canBeFulfilledAtQuantity(1)); } + return (bool) $rewardItem->variants->first()?->canBeFulfilledAtQuantity(1); + }); + + if ($fulfillableRewards->isEmpty()) { + return [$affectedLines, $discountTotal]; + } + + while ($remainingRewardQty > 0) { + $selectedRewardItem = $fulfillableRewards->random()->discountable; + if ($selectedRewardItem instanceof LunarCollection) { - $product = $selectedRewardItem->products()->inRandomOrder()->first(); - $purchasable = $product?->variants()->first(); + $product = $selectedRewardItem->products() + ->inRandomOrder() + ->with('variants') + ->get() + ->first(fn ($p) => $p->variants->first()?->canBeFulfilledAtQuantity(1)); + $purchasable = $product?->variants->first(); $selectedRewardItem = $product; } elseif ($selectedRewardItem instanceof Purchasable) { $purchasable = $selectedRewardItem; @@ -274,6 +293,12 @@ private function processAutomaticRewards(CartContract $cart, int $remainingRewar continue; } + if (! $purchasable->canBeFulfilledAtQuantity(1)) { + $remainingRewardQty--; + + continue; + } + $rewardKey = $purchasable->getMorphClass().':'.$purchasable->id; // is it already in cart? diff --git a/tests/core/Unit/DiscountTypes/BuyXGetYTest.php b/tests/core/Unit/DiscountTypes/BuyXGetYTest.php index a8cc12dd6d..da98c569b3 100644 --- a/tests/core/Unit/DiscountTypes/BuyXGetYTest.php +++ b/tests/core/Unit/DiscountTypes/BuyXGetYTest.php @@ -1736,6 +1736,311 @@ expect($rewardLine->purchasable_type)->toEqual($purchasableB->getMorphClass()); }); +test('does not automatically add reward when product variant has no stock', function () { + $customerGroup = CustomerGroup::factory()->create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + $currency = Currency::factory()->create(['code' => 'GBP']); + + $productA = Product::factory()->create(); + $productB = Product::factory()->create(); + + $purchasableA = ProductVariant::factory()->create(['product_id' => $productA->id]); + $purchasableB = ProductVariant::factory()->create([ + 'product_id' => $productB->id, + 'purchasable' => 'in_stock', + 'stock' => 0, + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + foreach ([$purchasableA, $purchasableB] as $purchasable) { + 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' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Automatic Reward Out Of Stock', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 1, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + $discount->discountableRewards()->create([ + 'discountable_type' => $productB->getMorphClass(), + 'discountable_id' => $productB->id, + 'type' => 'reward', + ]); + + $cart = $cart->calculate(); + + expect($cart->freeItems)->toBeNull(); +}); + +test('does not automatically add collection reward when all products are out of stock', function () { + $customerGroup = CustomerGroup::factory()->create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + $currency = Currency::factory()->create(['code' => 'GBP']); + + $collection = Collection::factory()->create(); + + $productA = Product::factory()->create(); + $productB = Product::factory()->create(); + + $productB->collections()->sync($collection); + + $purchasableA = ProductVariant::factory()->create(['product_id' => $productA->id]); + $purchasableB = ProductVariant::factory()->create([ + 'product_id' => $productB->id, + 'purchasable' => 'in_stock', + 'stock' => 0, + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + foreach ([$purchasableA, $purchasableB] as $purchasable) { + 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' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Automatic Collection Reward Out Of Stock', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 1, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + $discount->discountableRewards()->create([ + 'discountable_type' => $collection->getMorphClass(), + 'discountable_id' => $collection->id, + 'type' => 'reward', + ]); + + $cart = $cart->calculate(); + + expect($cart->freeItems)->toBeNull(); +}); + +test('automatically adds collection reward selecting in-stock product when others are out of stock', function () { + $customerGroup = CustomerGroup::factory()->create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + $currency = Currency::factory()->create(['code' => 'GBP']); + + $collection = Collection::factory()->create(); + + $productA = Product::factory()->create(); + $productB = Product::factory()->create(); // out of stock + $productC = Product::factory()->create(); // in stock + + $productB->collections()->sync($collection); + $productC->collections()->sync($collection); + + $purchasableA = ProductVariant::factory()->create(['product_id' => $productA->id]); + $purchasableB = ProductVariant::factory()->create([ + 'product_id' => $productB->id, + 'purchasable' => 'in_stock', + 'stock' => 0, + ]); + $purchasableC = ProductVariant::factory()->create([ + 'product_id' => $productC->id, + 'purchasable' => 'in_stock', + 'stock' => 5, + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + foreach ([$purchasableA, $purchasableB, $purchasableC] as $purchasable) { + 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' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Automatic Collection Reward Mixed Stock', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 1, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + $discount->discountableRewards()->create([ + 'discountable_type' => $collection->getMorphClass(), + 'discountable_id' => $collection->id, + 'type' => 'reward', + ]); + + $cart = $cart->calculate(); + + // Only the in-stock product (C) should be added as a free item + expect($cart->freeItems)->toHaveCount(1); + expect($cart->freeItems->first()->id)->toEqual($productC->id); +}); + +test('automatically adds in-stock product reward when another reward product is out of stock', function () { + $customerGroup = CustomerGroup::factory()->create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + $currency = Currency::factory()->create(['code' => 'GBP']); + + $productA = Product::factory()->create(); // condition + $productB = Product::factory()->create(); // reward — out of stock + $productC = Product::factory()->create(); // reward — in stock + + $purchasableA = ProductVariant::factory()->create(['product_id' => $productA->id]); + $purchasableB = ProductVariant::factory()->create([ + 'product_id' => $productB->id, + 'purchasable' => 'in_stock', + 'stock' => 0, + ]); + $purchasableC = ProductVariant::factory()->create([ + 'product_id' => $productC->id, + 'purchasable' => 'in_stock', + 'stock' => 5, + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + foreach ([$purchasableA, $purchasableB, $purchasableC] as $purchasable) { + 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' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Automatic Reward Mixed Stock', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 1, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + // Both B (no stock) and C (in stock) are rewards + $discount->discountableRewards()->createMany([ + ['discountable_type' => $productB->getMorphClass(), 'discountable_id' => $productB->id, 'type' => 'reward'], + ['discountable_type' => $productC->getMorphClass(), 'discountable_id' => $productC->id, 'type' => 'reward'], + ]); + + $cart = $cart->calculate(); + + // Product C (in stock) must be added; product B (out of stock) must never be added + expect($cart->freeItems)->toHaveCount(1); + expect($cart->freeItems->first()->id)->toEqual($productC->id); +}); + test('can add a multi quantity reward as a single line', function () { $customerGroup = CustomerGroup::factory()->create([ 'default' => true, From 6fc10eb863c175d0ab23232c46f6ff7b13376200 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Wed, 26 Aug 2026 12:00:53 +0100 Subject: [PATCH 2/3] Fix fulfillability check for variant rewards and cache collection lookups The fulfillability filter assumed a reward discountable was always a Product or Collection, so a ProductVariant reward crashed the same way #2631 fixed for the reward-selection code. Mirror that instanceof Purchasable branch here. Also hydrate each collection reward's products once per apply() call instead of once in the filter and again per remaining reward unit in the selection loop, and collapse the two identical purchasable/ fulfillability guards into one. --- packages/core/src/DiscountTypes/BuyXGetY.php | 33 ++++++++++---------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/packages/core/src/DiscountTypes/BuyXGetY.php b/packages/core/src/DiscountTypes/BuyXGetY.php index 0e5f48b6a6..bf13568d18 100644 --- a/packages/core/src/DiscountTypes/BuyXGetY.php +++ b/packages/core/src/DiscountTypes/BuyXGetY.php @@ -249,7 +249,11 @@ private function processAutomaticRewards(CartContract $cart, int $remainingRewar // we have lines to add if ($remainingRewardQty > 0) { - $fulfillableRewards = $this->discount->discountableRewards->filter(function ($discountableReward) { + // Fulfillable products per collection reward, hydrated once here rather + // than re-queried on every iteration of the allocation loop below. + $fulfillableCollectionProducts = []; + + $fulfillableRewards = $this->discount->discountableRewards->filter(function ($discountableReward) use (&$fulfillableCollectionProducts) { $rewardItem = $discountableReward->discountable; if (! $rewardItem) { @@ -257,10 +261,17 @@ private function processAutomaticRewards(CartContract $cart, int $remainingRewar } if ($rewardItem instanceof LunarCollection) { - return $rewardItem->products() + $fulfillableCollectionProducts[$rewardItem->id] = $rewardItem->products() ->with('variants') ->get() - ->some(fn ($p) => $p->variants->first()?->canBeFulfilledAtQuantity(1)); + ->filter(fn ($p) => $p->variants->first()?->canBeFulfilledAtQuantity(1)) + ->values(); + + return $fulfillableCollectionProducts[$rewardItem->id]->isNotEmpty(); + } + + if ($rewardItem instanceof Purchasable) { + return $rewardItem->canBeFulfilledAtQuantity(1); } return (bool) $rewardItem->variants->first()?->canBeFulfilledAtQuantity(1); @@ -274,12 +285,8 @@ private function processAutomaticRewards(CartContract $cart, int $remainingRewar $selectedRewardItem = $fulfillableRewards->random()->discountable; if ($selectedRewardItem instanceof LunarCollection) { - $product = $selectedRewardItem->products() - ->inRandomOrder() - ->with('variants') - ->get() - ->first(fn ($p) => $p->variants->first()?->canBeFulfilledAtQuantity(1)); - $purchasable = $product?->variants->first(); + $product = $fulfillableCollectionProducts[$selectedRewardItem->id]->random(); + $purchasable = $product->variants->first(); $selectedRewardItem = $product; } elseif ($selectedRewardItem instanceof Purchasable) { $purchasable = $selectedRewardItem; @@ -287,13 +294,7 @@ private function processAutomaticRewards(CartContract $cart, int $remainingRewar $purchasable = $selectedRewardItem->variants->first(); } - if (! $purchasable) { - $remainingRewardQty--; - - continue; - } - - if (! $purchasable->canBeFulfilledAtQuantity(1)) { + if (! $purchasable || ! $purchasable->canBeFulfilledAtQuantity(1)) { $remainingRewardQty--; continue; From 5a77c08a22f44ae3c5726624afea67d5b7772895 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Wed, 26 Aug 2026 12:54:25 +0100 Subject: [PATCH 3/3] Track cumulative allocation when checking reward fulfillability canBeFulfilledAtQuantity(1) was checked on every pass through the loop, so a reward_qty of 3 against a stock-1 variant still allocated all 3 units onto the one line #2639 now collapses them into. Check against what this run has already put on that line instead. --- packages/core/src/DiscountTypes/BuyXGetY.php | 13 +++- .../core/Unit/DiscountTypes/BuyXGetYTest.php | 76 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/packages/core/src/DiscountTypes/BuyXGetY.php b/packages/core/src/DiscountTypes/BuyXGetY.php index bf13568d18..840b32203c 100644 --- a/packages/core/src/DiscountTypes/BuyXGetY.php +++ b/packages/core/src/DiscountTypes/BuyXGetY.php @@ -294,7 +294,7 @@ private function processAutomaticRewards(CartContract $cart, int $remainingRewar $purchasable = $selectedRewardItem->variants->first(); } - if (! $purchasable || ! $purchasable->canBeFulfilledAtQuantity(1)) { + if (! $purchasable) { $remainingRewardQty--; continue; @@ -302,6 +302,17 @@ private function processAutomaticRewards(CartContract $cart, int $remainingRewar $rewardKey = $purchasable->getMorphClass().':'.$purchasable->id; + // How many units of this reward this run has already allocated, + // since canBeFulfilledAtQuantity below must check against that + // running total rather than a fixed quantity of 1 each time. + $allocated = $addedRewardLines[$rewardKey]->quantity ?? 0; + + if (! $purchasable->canBeFulfilledAtQuantity($allocated + 1)) { + $remainingRewardQty--; + + continue; + } + // is it already in cart? $rewardLine = $addedRewardLines[$rewardKey] ?? $cart->lines->first(function ($line) use ($purchasable) { return $line->purchasable->id == $purchasable->id; diff --git a/tests/core/Unit/DiscountTypes/BuyXGetYTest.php b/tests/core/Unit/DiscountTypes/BuyXGetYTest.php index da98c569b3..96e2245ef2 100644 --- a/tests/core/Unit/DiscountTypes/BuyXGetYTest.php +++ b/tests/core/Unit/DiscountTypes/BuyXGetYTest.php @@ -2131,6 +2131,82 @@ expect($rewardLines->first()->quantity)->toEqual(3); }); +test('does not allocate a multi quantity reward beyond stock', function () { + $customerGroup = CustomerGroup::factory()->create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + $currency = Currency::factory()->create(['code' => 'GBP']); + + $productA = Product::factory()->create(); + $productB = Product::factory()->create(); + + $purchasableA = ProductVariant::factory()->create(['product_id' => $productA->id]); + $purchasableB = ProductVariant::factory()->create([ + 'product_id' => $productB->id, + 'purchasable' => 'in_stock', + 'stock' => 1, + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + foreach ([$purchasableA, $purchasableB] as $purchasable) { + 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' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Automatic Reward Limited Stock', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 3, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + $discount->discountableRewards()->create([ + 'discountable_type' => $productB->getMorphClass(), + 'discountable_id' => $productB->id, + 'type' => 'reward', + ]); + + $cart->calculate(); + + $rewardLines = CartLine::where('cart_id', $cart->id) + ->where('purchasable_id', $purchasableB->id) + ->get(); + + // Only one unit is in stock, so only one is allocated despite reward_qty 3. + expect($rewardLines)->toHaveCount(1); + expect($rewardLines->first()->quantity)->toEqual(1); +}); + test('can leave a reward line the shopper added at their own quantity', function () { $customerGroup = CustomerGroup::factory()->create([ 'default' => true,