Skip to content

Match cart lines on the purchasable type when merging carts - #2681

Open
kha333n wants to merge 1 commit into
lunarphp:1.xfrom
kha333n:fix/merge-cart-purchasable-type
Open

Match cart lines on the purchasable type when merging carts#2681
kha333n wants to merge 1 commit into
lunarphp:1.xfrom
kha333n:fix/merge-cart-purchasable-type

Conversation

@kha333n

@kha333n kha333n commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

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

  • Merging a guest cart into an account cart decides two lines are the same item
    from the purchasable id and the meta.
  • Ids of different purchasable types are independent sequences, so gift card 1
    and product variant 1 both exist. Nothing coordinates them, and nothing needs
    to.
  • When they collide, the source line is never created in the target cart and an
    unrelated line absorbs its quantity.
  • The source cart is stamped merged_id immediately after, so the lost item
    cannot be recovered. No exception, nothing in the log.

What should happen

  • Two lines are the same item only when the type and the id both match.

Why

MergeCart compares the id and the meta, and never the type:

return $targetLine->purchasable_id == $line->purchasable_id &&
json_encode($targetLine->meta) == json_encode($line->meta);

GetExistingCartLine — Lunar's other answer to "is this the same line?" —
does compare it, via wherePurchasableType(). The two disagree about what makes
two lines equal, and only one of them can be right.

The fix

Compare purchasable_type as well, which is the comparison
GetExistingCartLine already makes. Nothing else changes: the branch that
creates 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.x with:

    -    0 => 'Lunar\Tests\Core\Stubs\TestGiftCard x4'
    -    1 => 'product_variant x1'
    +    0 => 'product_variant x5'
    

The defect is only observable with two purchasable types in play, and the suite
has no Eloquent purchasable — TestPurchasable is a plain class and cannot be
the target of a morph. So the test adds TestGiftCard, a minimal model
implementing Purchasable, and creates its table in the test. It is the sort of
custom purchasable the docs invite — a gift card, a bundle, a subscription.

@github-project-automation github-project-automation Bot moved this to Todo in Roadmap Aug 28, 2026
@github-actions github-actions Bot added 1.x high needs-issue PR has no linked issue or Discussion labels Aug 28, 2026
@github-actions

Copy link
Copy Markdown

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
kha333n marked this pull request as ready for review August 28, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.x high needs-issue PR has no linked issue or Discussion

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Merging carts matches lines on the purchasable id only, so one item is deleted and another inflated

1 participant