Skip to content

Support variant rewards when automatically adding BuyXGetY rewards - #2631

Merged
glennjacobs merged 2 commits into
lunarphp:1.xfrom
kha333n:fix/buyxgety-variant-rewards
Aug 25, 2026
Merged

Support variant rewards when automatically adding BuyXGetY rewards#2631
glennjacobs merged 2 commits into
lunarphp:1.xfrom
kha333n:fix/buyxgety-variant-rewards

Conversation

@kha333n

@kha333n kha333n commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

A merchant picks a specific variant as a Buy X Get Y reward — which the admin
lets them do — and ticks "automatically add rewards". Every cart that qualifies
then dies with a 500.

What happens now

  • Call to a member function first() on null in
    BuyXGetY::processAutomaticRewards(), thrown during cart calculation.
  • It takes down anything that calculates the cart, so the shopper cannot view
    their basket, not just complete checkout.
  • Product rewards and collection rewards both work. Only the variant reward,
    combined with the automatic-add option, fails.

What should happen

  • A variant reward is added to the cart in the same way a product reward is.

Why

#2214 made variants selectable as rewards, and #2461 added a branch for
collections. The remaining else still assumes the reward is a Product and
reaches for its variants:

} else {
    $purchasable = $selectedRewardItem->variants->first();
}

A ProductVariant has no variants relation, so the property read returns
null and the ->first() on it is fatal. The null guard immediately below
never runs, because the error is thrown before it is reached.

The fix

Add a branch for a reward that is already purchasable, and use it directly.
ProductVariant implements Lunar\Base\Purchasable, and Product does not,
so the check separates the two cases without naming concrete models — the same
approach the existing collection branch takes.

Nothing else in the method changes: the existing product and collection paths
are untouched, and the reward still flows through the normal cart-line pipeline
afterwards.

Worth flagging: #2509 is currently rewriting this method to skip rewards that
cannot be fulfilled. The two changes do not overlap in intent, but whichever
lands second will want a rebase.

Tests

tests/core/Unit/DiscountTypes/BuyXGetYTest.php:

  • can add an eligible variant reward when not in cart — the existing
    can add eligible products when not in cart scenario with the reward
    registered against the variant instead of the product. Fails on 1.x with
    Call to a member function first() on null at BuyXGetY.php:255, and
    asserts the same £12.00 total and single free item afterwards.

@glennjacobs
glennjacobs merged commit 8a1161c into lunarphp:1.x Aug 25, 2026
14 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in Roadmap Aug 25, 2026
glennjacobs added a commit to kha333n/lunar that referenced this pull request Aug 25, 2026
Resolves the test-file conflict with lunarphp#2631 by keeping both new tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ryanmitchell added a commit to ryanmitchell/lunar that referenced this pull request Aug 26, 2026
…kups

The fulfillability filter assumed a reward discountable was always a
Product or Collection, so a ProductVariant reward crashed the same way
lunarphp#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.
ryanmitchell added a commit to ryanmitchell/lunar that referenced this pull request Aug 26, 2026
…kups

The fulfillability filter assumed a reward discountable was always a
Product or Collection, so a ProductVariant reward crashed the same way
lunarphp#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants