Reset the shipping breakdown on every cart calculate#2476
Merged
Conversation
ApplyShipping reused the cart's existing ShippingBreakdown and only cleared its items when a shippingOptionOverride was set. Switching shipping options via CartSession::setShippingOption() therefore appended the new option to the breakdown while keeping the old one, so the shipping subtotal summed both prices until the next page load. Start each run with a fresh ShippingBreakdown — the option is always re-resolved from the shipping address (or the override), and the breakdown is purely a derived value from that single option. Fixes #2199
Collaborator
|
In this case, what happens if a developer adds a custom pipeline class that needs to add additional shipping costs/breakdowns before they are calculated? |
Contributor
Author
The post-ApplyShipping slot is the supported extension point for additional shipping costs; that still works. The pre-ApplyShipping slot was never reliable (override path already cleared it) and is the right thing to break in service of fixing the duplicate-option bug. If they want a documented hook for pre-resolution adjustments, that's a separate feature, not something to revert this fix for. |
alecritson
approved these changes
May 19, 2026
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
2 participants
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.
Summary
ApplyShippingnow constructs a freshShippingBreakdownon each run rather than mutating the cart's previously-cached one.if (\$cart->shippingOptionOverride) { \$shippingBreakdown->items = collect(); }clear is removed — it was a partial workaround for the same issue.Why
ApplyShippingwas seeded from\$cart->shippingBreakdown ?: new ShippingBreakdownand thenput()'d the resolved option into the existing items collection. CallingCartSession::setShippingOption()(or otherwise recalculating the cart) within the same request added the new option to the items collection alongside the previous one —\$shippingBreakdown->items->sum('price.value')then summed both. The page refresh fix that reporters noticed was becauseshippingBreakdownis runtime-only and disappears between requests.The breakdown is purely a derived view of the single currently-selected option, so starting fresh every time is both correct and simpler.
Fixes #2199.
Test plan
vendor/bin/pest --testsuite=core— 504 passingCartSession::setShippingOption()twice in a row with different options → cart total reflects only the latest