Match cart lines on the purchasable type when merging carts - #2681
Open
kha333n wants to merge 1 commit into
Open
Match cart lines on the purchasable type when merging carts#2681kha333n wants to merge 1 commit into
kha333n wants to merge 1 commit into
Conversation
|
Thanks for the PR! I couldn't find a linked issue or Discussion in the description. For anything beyond a typo or small isolated fix, we ask that the change is accepted before the PR is opened — for bug fixes that means a linked issue, and for features or enhancements a linked Discussion (features are not tracked as issues in this repo). See Non-trivial changes need acceptance first. It exists so you never spend a weekend on an approach we were always going to turn down. If there is one and I missed it, just link it here and ignore me — I'm a bot and this is advisory. |
kha333n
marked this pull request as ready for review
August 28, 2026 15:48
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 shopper puts four gift cards in their basket as a guest, then logs in. The
gift cards are gone, and the one shampoo they already had is now five.
Fixes #2682.
The same code is unchanged on
2.x, so this wants a forward port.What happens now
from the purchasable id and the meta.
and product variant 1 both exist. Nothing coordinates them, and nothing needs
to.
unrelated line absorbs its quantity.
merged_idimmediately after, so the lost itemcannot be recovered. No exception, nothing in the log.
What should happen
Why
MergeCartcompares the id and the meta, and never the type:GetExistingCartLine— Lunar's other answer to "is this the same line?" —does compare it, via
wherePurchasableType(). The two disagree about what makestwo lines equal, and only one of them can be right.
The fix
Compare
purchasable_typeas well, which is the comparisonGetExistingCartLinealready makes. Nothing else changes: the branch thatcreates a missing line already writes the type correctly, so once the match is
right the rest of the method behaves.
Tests
tests/core/Unit/Actions/Carts/MergeCartTest.php:can merge lines of different purchasable types that share an id — an
account cart with one variant, a guest cart with four gift cards carrying the
same id, merged. Fails on
1.xwith:The defect is only observable with two purchasable types in play, and the suite
has no Eloquent purchasable —
TestPurchasableis a plain class and cannot bethe target of a morph. So the test adds
TestGiftCard, a minimal modelimplementing
Purchasable, and creates its table in the test. It is the sort ofcustom purchasable the docs invite — a gift card, a bundle, a subscription.