Port the v1.5.0 fixes into v2 (spec 0065) - #2655
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ports five 1.5.0 discount fixes: - Guard apply() on checkDiscountConditions(), so min spend, customer restrictions and max_uses_per_user apply as they do for AmountOff (#2624). - Handle a variant reward directly instead of reaching for ->variants, which fatalled and took down cart calculation (#2631). - Write reward lines with getMorphClass(), so they match every other line and stay matchable by the discount on the next calculate (#2632). - Add a multi-quantity reward as one line of N rather than N lines of one, tracking the lines this run created (#2639). - Skip rewards that cannot be fulfilled, and cap allocation against stock (#2509). More consequential in v2 than 1.x: CartLineStock rejects an unfulfillable line, so an out-of-stock automatic reward wedged the cart. Product rewards arrive through the discountable morph, so `variants` is never eager loaded — those reads go through the relation query to stay inside preventLazyLoading(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two 1.5.0 discount fixes: - Key the memoised discount set on the cart state it was built from (id, coupon, customer, line purchasables) rather than reusing any non-empty set (#2623). getDiscounts() filters on the coupon, so a set built before the shopper entered one could never contain their coupon discount — the total stayed unchanged until the next request. Spec 0064 scoped the manager's lifetime, which fixes leakage across requests but not this staleness within one. - Stop a cart's own discount consumption counting against it (#2637, #2638). Order creation records a use as soon as the draft order exists, so a retry after a declined card found its own single-use coupon exhausted and re-priced the order without it. Cart::consumedDiscountIds() reads the draft order's breakdown (memoised per instance, forgotten by CreateOrder once it rewrites the breakdown), Discount::scopeUsable() takes an exempt list, and the use limits in checkDiscountConditions() and the markAsUsed() loop both honour it. Coupon match, min spend, customer restrictions, dates and channel are still enforced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ntrancy Ports five 1.5.0 fixes and scopes the shipping manifest: - Reprice the cart when the session currency changes (#2625). setCurrency() wrote currency_id but left the loaded currency and lines relations in place, and lines carry their own copy of the cart, so the next calculate priced in the old currency. StorefrontSession::setCurrency() — a second switching path in v2 that never touched the cart — now delegates to CartSession, while the boot cascade writes through a non-propagating putCurrency() so resolving a currency per request neither calculates nor creates a cart. - Scope state tax-zone lookups to the address country (#2626). A state name or code is only unique within a country, so Washington's zone hijacked Australian addresses; states with no country still match any. - Guard the shipping-modifier pipeline against re-entry (#2630). A modifier that calculates the cart landed back in getOptions() until the stack was exhausted. The manifest also moves to scoped and joins ServiceLifetimesTest: it holds the options resolved for the cart in hand plus the new flag, so it is per-request state, not a boot-time registry. - Route the default tax zone through TaxZone::getDefault() (#2511), which is Blink-cached — getBreakdown() reaches for it per purchasable. - Annotate getDefault() as null|self (#2634); it returns first(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t_unit Ports #2645. ShipBy summed cart weight in kg regardless of the method's weight_unit, so a method configured in grams compared a kg total against gram tiers and every cart landed in the lowest tier. The constraint path in ShippingRateResolver was already unit-aware; only the tier path was not — this is distinct from #2382, which changed the tier comparison scale rather than its unit. The admin form followed the same assumption: it showed a hardcoded kg suffix and "kilograms" helper text, and silently truncated decimals. Both now read the configured unit, and an integer rule rejects decimals rather than truncating, since tiers are stored as raw integers. The helper text gains a :unit placeholder across all 14 locales. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…type Ports two 1.5.0 fixes: - Give Cart::lines(), Order::lines() and ProductVariant::values() an explicit ORDER BY (#2518). Row order without one is undefined by the SQL standard: InnoDB returns clustered primary-key order by coincidence, PostgreSQL returns heap order, which shifts after an UPDATE. Lunar depends on a stable sequence — GenerateFingerprint reduces $cart->lines in iteration order, and getOption() is snapshotted onto order lines — so it has to be deterministic across engines. Derived relations chain off lines() and inherit the fix. - Match an existing order address on type alone (#2507). Matching on type and postcode meant a shopper changing their postcode before a recalculate got a second address row of the same type; the type column is indexed, not unique, so nothing caught it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…one selects Ports four 1.5.0 admin fixes: - Trim TextInput/Textarea/TagsInput at the form layer (#2642). Livewire skips the TrimStrings middleware, so admin input persisted with leading and trailing whitespace. The Inertia panel needs nothing — its requests go through the normal HTTP stack. - Fix product option mapping and the new-variant tax class (#2640). collect()->search() returns false on no match and PHP coerces $variants[false] to index 0, so sparse matrices showed the first variant's SKU under the wrong option labels. A newly filled permutation also had nothing to copy tax_class_id or a base price from, so the insert violated NOT NULL; it now copies the oldest sibling, with a widget-side fallback. - Keep media custom properties the form did not render (#2598). Create hand-picked name and primary, discarding anything an extension added, and edit wrote the array-cast column wholesale, clobbering unrendered keys. Create now passes the submitted set through and edit merges over what is stored. - Resolve country/state select labels from the submitted values rather than the persisted relationship (#2612). A fresh selection had no label yet, so Filament rejected the field as invalid. Fixes the same defect in ShippingZoneForm's single-country select, which plucked country.name off a relation that points straight at Country and so was always blank. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oice lines Ports five 1.5.0 admin fixes: - Render boolean order meta as localized Yes/No (#2545) instead of PHP's "" and "1" cast. Adds the two keys across all 16 admin locales. - Enforce a unit quantity of at least one (#2547) on the variant form and clamp it in DefaultPriceFormatter, whose unitDecimal() divides by it. - Label the product-options widget actions and empty-state heading from translations (#2555). Filament humanised the action names, so "Save Variants" was hardcoded English in every locale; the new key is translated across all 16 filament locales. The stale duplicate blade under packages/admin is removed rather than fixed — the widget's $view points at the bridge namespace, so nothing could reach it. - Include non-shippable lines on the PDF invoice (#2409). The bug survived the v2 refactor in a new form: the template iterated fulfillableLines, and a non-shippable variant has requires_fulfilment false, so digital lines still vanished from the invoice body. The remaining fulfillableLines call sites are genuine fulfilment concerns. - Close the collection breadcrumb gaps left by the CollectionSelect refactor (#2557): the group name now prefixes the trail, so two same-named collections in different groups are distinguishable, and the condition relation manager gains the breadcrumb description its limitation counterpart already had. Group and ancestors are eager loaded everywhere the label is resolved, including the AttachAction path, which previously N+1'd and would trip preventLazyLoading(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ports #2484. The permission is registered by the shipping package's own migration, but nothing labelled it, so both panels rendered the raw handle "shipping:manage" in the staff and roles UI. The 1.x fix merged the addon's translations into the panel namespace at boot because v1's label key lived somewhere the addon could not write. v2 centralises resolution in the Permission DTO on a flat lunar::auth.permissions.{handle}.* key that both surfaces read, so the keys simply go in core across all 16 locales — one fix, no runtime merging. A third-party addon labels its own permission with Lang::addLines() into the lunar namespace. The v1-era `lunarpanel.shipping` translation namespace is left alone: renaming it would break every call site and any published overrides. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ports #2601, which supersedes the zero-decimal fix already on 2.x (#2467). That one early-returned for everything but HUF/TWD/UGX, so a merchant whose Currency::decimal_places disagreed with Stripe's scale for that currency was charged by a factor of 100 — and it never handled three-decimal currencies at all. toStripeAmount() now de-scales to the major unit using decimal_places and re-scales to whatever sub-unit Stripe expects, using integer arithmetic throughout: float division misrounds half-unit boundaries (145 at 3dp is 0.145, which stores as 0.1449… and rounds down). fromStripeAmount() is the inverse, applied wherever an amount comes back from Stripe. Capture, refund, the intent-total assertion and StoreCharges all convert. Bundled with #2644: once StoreCharges converts, CartBuilder's random faker currency code makes the stored amount diverge from the raw fixture amount whenever it rolls a currency Stripe scales differently — measured at ~14% of rolls — so the currency is pinned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ports #2520. NumberFormatter::CURRENCY and its siblings are int constants, so passing one tripped a coercion deprecation. v2 needs one more file than 1.x did: the pricing refactor widened PriceFormatterInterface to declare the full parameter list, so the implementation and the contract have to change together. That makes it a signature change on a published interface, so RetypeFormatterStyleParamRector retypes a consumer's own formatter methods on upgrade. Marks spec 0065 implemented and moves it to specs/completed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
glennjacobs
marked this pull request as draft
August 27, 2026 14:02
glennjacobs
marked this pull request as ready for review
August 27, 2026 14:02
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
1 participant
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.
Ports the outstanding v1.5.0 fixes to
2.x(spec 0065).v1.5.0 shipped 36 changes on the 1.x line (PRs #2409–#2648). 2.x forked at #2382, so none arrived by cherry-pick, and none could be — namespaces moved to
Lunar\Core\…,Base/was reorganised, actions are constructor-injected with typed signatures, money isPriceValue, admin split intoadmin/filament, and the Inertia panel is a second UI surface. Each fix was re-implemented in its v2 location.Every 1.5.0 commit was compared against the 2.x tree at code level first:
DateTimeImmutablesupport (fix(core): accept DateTimeImmutable in scheduling and scope methods #2648 ≙ fix(core): accept DateTimeImmutable in scheduling and scope methods #2594), the nestedset fork swap (Change to Lunar nested set #2510), the Route imports (Import the Route facade in opayo and paypal route files #2636 ≙ Import Route class #2620).default_value(fix(attributes): expose and apply default_value in the admin panel #2505), deliberately dropped by spec 0019; the Boost skill (Add skill for laravel boost #2491), v1-pinned throughout and already tracked as a rewrite.What's here
One commit per slice, in dependency order.
TaxZone::getDefault()(#2511); nullablegetDefault()(#2634); weight tiers in the configured unit (#2645)formatterStyletypedint(#2520)Where v2 needed more than the 1.x fix
Several of these are worse in v2, or land differently:
preventLazyLoading(spec 0011) is enforced in v2 and not in 1.x. The BuyXGetY fulfillable-reward filter readsvariantsoff a Product arriving through the discountable morph, which threw; those reads now go through the relation query. The same class of problem surfaced inAppliesToExistingSelect, which was N+1'ing to build labels — selectors now declare what their label reads viaoptionLabelRelations().CartLineStockrejects an unfulfillable line, so an out-of-stock automatic reward wedged the cart rather than just being a userland oddity.apply()matches lines on aliases, so an FQCN-typed reward line was never re-matched and the discount kept adding more.fulfillableLines, and a non-shippable variant hasrequires_fulfilment = false, so digital lines still vanished. OtherfulfillableLinescall sites were audited and are genuine fulfilment concerns.decimal_placesdisagreed with Stripe's scale was charged 100x, and three-decimal currencies were never handled.StoreChargesconverts,CartBuilder's random faker currency diverges from the fixture on ~14% of rolls (2000-roll sample) — an intermittent red, so both tests in the file are pinned.PriceFormatterInterfaceto declare the parameter list, so contract and implementation must change together — which makes it a signature change on published surface, handled by a newRetypeFormatterStyleParamRector.Decisions worth a look
StorefrontSession::setCurrency()never touched it, so a region switch left the cart priced in the old currency — a gap 1.x didn't have. It delegates toCartSession, while the boot cascade writes through a non-propagatingputCurrency()so resolving a currency per request neither calculates nor creates a cart.ShippingManifestmoves fromsingletontoscopedand joinsServiceLifetimesTest. It holds the options resolved for the cart in hand plus the new re-entrancy flag — per-request state, not a boot-time registry.mergeTranslationsForPanel()hack existed because v1's key lived where the addon couldn't write. v2's flatPermissionDTO key is read by both surfaces, so the keys simply go in core across 16 locales. A third-party addon usesLang::addLines()into thelunarnamespace — the native API, nothing to build. The v1-eralunarpanel.shippingtranslation namespace is left alone: renaming it breaks every call site and any published override, so it wants its own spec and Rector rule.AdjustStock, and every automatic movement. What v2 needs is spec 0038's unshipped events section, which is new surface rather than a port; it's tracked inTODO.md.packages/admin/resources/views/resources/product-resource/widgets/product-options.blade.php. The widget's$viewpoints at the bridge namespace and nothing references the admin copy, so it was unreachable — but flagging it, since it's a removal from a published package.Migration impact
PriceFormatterInterface::$formatterStyle(string→int), covered by the new Rector rule.:unitin table-rate-shippingrelationmanagers.php(14 locales),yes/noin adminglobal.php(16),save-variants.labelin filamentproductoption.php(16),shipping:managelabel/description in coreauth.php(16). Translated per locale, no English placeholders.Tests
Every ported fix has a test that was confirmed to fail without it. All CI suites pass, plus Pint and PHPStan:
Still open
Recorded in the spec, not blocking:
getDefault()nullable, the typedCartSessionManager::getCurrency()/getChannel()still fail with aTypeErrorwhen no default record exists. Whether that should be a domain exception is its own question.default_valueshould come back to v2 — it would follow the spec-0062 validation-rules template, as its own spec.🤖 Generated with Claude Code