Skip to content

Reload nested cart relations after refresh (#2222) - #2647

Open
wakqasahmed wants to merge 1 commit into
lunarphp:1.xfrom
wakqasahmed:fix/issue-2222-cart-lazy-loading
Open

Reload nested cart relations after refresh (#2222)#2647
wakqasahmed wants to merge 1 commit into
lunarphp:1.xfrom
wakqasahmed:fix/issue-2222-cart-lazy-loading

Conversation

@wakqasahmed

Copy link
Copy Markdown
Contributor

Closes #2222.

The reporter's own diagnosis in the issue was spot on: Cart::add(), remove(), updateLine() and the other cart mutators all call $this->refresh() before recalculating, and Eloquent's refresh() only reloads relations that are already loaded, one level deep. So after the first add, lines.purchasable.prices (and the rest of the nested lunar.cart.eager_load config) gets dropped down to just lines, and the next calculation lazy-loads it. With lazy loading disabled that's the LazyLoadingException from adding a second product.

I added Cart::refreshForCalculation(), which refreshes the cart and then reapplies the full eager_load config, and swapped it in everywhere the old refresh()->recalculate() pattern was used. Made it public since it's a reasonable thing for anyone building custom cart flows to reuse.

I built on the failing test from #2221 (closed) rather than duplicating that diagnostic work — added a regression test that asserts the nested relations survive a refresh, plus the two-different-products-in-one-request scenario from that PR.

One thing worth flagging: I deliberately kept this scoped to the refresh mechanism the issue actually describes. While testing with Model::preventLazyLoading() turned on globally, adding a second product still eventually hits separate, unrelated lazy loads a few frames deeper in the pricing/tax calculation (ProductVariant::taxClass, then TaxClass::taxRateAmounts, then further into discounts). That matches what came up in #2221's thread - alecritson noted supporting preventLazyLoading app-wide would need a rethink of how pricing works, and closed that PR rather than patch around it. This PR doesn't attempt that; it just fixes the specific refresh bug reported here.

Ran ./vendor/bin/pint and the full core test suite locally (Pest, sqlite) - all cart tests pass, 584 passed overall, the only failures are pre-existing environment gaps unrelated to this change (missing GD extension in my local PHP build, hit by MediaObserverTest and HasMediaTraitTest).

…rphp#2222)

Cart::add(), remove(), updateLine() and the other cart mutators all
called $this->refresh() before recalculating. Eloquent's refresh()
only reloads relations that are already loaded on the model, and only
one level deep, so a nested relation configured in
lunar.cart.eager_load (e.g. lines.purchasable.prices) ended up dropped
after the refresh and got lazy-loaded the next time the cart was
calculated. With lazy loading disabled that throws a
LazyLoadingViolationException, which is what happens when a second
product is added to a cart in the same request.

Added Cart::refreshForCalculation(), which refreshes the cart and then
reapplies the full eager_load config so the nested relations survive,
and swapped it in everywhere the old refresh()->recalculate() pattern
was used.

Added a test asserting the nested relations survive a refresh, and
adapted the add-to-cart scenario from lunarphp#2221's failing test into a
regression test for the cart mutators themselves.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

LazyLoadingException exception is thrown when adding multiple products to a cart

2 participants