Support variant rewards when automatically adding BuyXGetY rewards - #2631
Merged
Conversation
glennjacobs
approved these changes
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.
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.
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 nullinBuyXGetY::processAutomaticRewards(), thrown during cart calculation.their basket, not just complete checkout.
combined with the automatic-add option, fails.
What should happen
Why
#2214 made variants selectable as rewards, and #2461 added a branch for
collections. The remaining
elsestill assumes the reward is aProductandreaches for its variants:
A
ProductVarianthas novariantsrelation, so the property read returnsnulland the->first()on it is fatal. The null guard immediately belownever 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.
ProductVariantimplementsLunar\Base\Purchasable, andProductdoes 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 eligible products when not in cartscenario with the rewardregistered against the variant instead of the product. Fails on
1.xwithCall to a member function first() on nullatBuyXGetY.php:255, andasserts the same £12.00 total and single free item afterwards.