From b675ef2bccc014f52982c4647f5c359f2f2b80dc Mon Sep 17 00:00:00 2001 From: Glenn Jacobs Date: Wed, 26 Aug 2026 17:52:22 +0100 Subject: [PATCH 01/11] =?UTF-8?q?docs:=20spec=200065=20=E2=80=94=20port=20?= =?UTF-8?q?the=20v1.5.0=20fixes=20into=20v2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 (1M context) --- TODO.md | 1 + specs/0065-port-v1.5-fixes.md | 110 ++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 specs/0065-port-v1.5-fixes.md diff --git a/TODO.md b/TODO.md index b095cff894..04888d7178 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,7 @@ Items tagged _(judgement)_ are genuine line-calls worth revisiting. - Order print templates — Print dropdown of selectable PDF templates, ships an Advice Note (spec 0027) - Cart/order line grouping — grouping key on the `*_lines` tables _(judgement)_ - Cart totals caching in the database — additive performance optimisation +- Port the v1.5.0 fixes into v2 (spec 0065) - Add Boost guidelines to packages ## Ideas diff --git a/specs/0065-port-v1.5-fixes.md b/specs/0065-port-v1.5-fixes.md new file mode 100644 index 0000000000..afd1924767 --- /dev/null +++ b/specs/0065-port-v1.5-fixes.md @@ -0,0 +1,110 @@ +# 0065 — Port the v1.5.0 fixes into v2 + +- Status: draft +- Author: Glenn +- Created: 2026-08-26 +- TODO item: Port the v1.5.0 fixes into v2 + +## Problem + +Lunar 1.5.0 shipped 36 changes on the 1.x line (PRs #2409–#2648, tagged `1.4.0..1.5.0`). The 2.x branch forked at #2382, so none of them are on 2.x by cherry-pick — a code-level comparison of each 1.x commit against the current 2.x tree shows: + +- **4 already landed independently**: the order-line de-morph (#2643 is the backport of spec [[0045-optional-purchasables-and-shipping-de-morph]]), `DateTimeImmutable` support (#2648 backports 2.x #2594), the nestedset fork swap (#2510, on 2.x as `2f38c2a2`), and the Route facade imports (#2636 ≙ 2.x #2620). +- **2 are N/A by design**: attribute `default_value` (#2505) — the column was deliberately dropped by spec 0019; re-adding it is a new spec, not a port. The Boost skill (#2491) is v1-pinned throughout (namespaces, doc paths, contract advice) and is covered by the existing "Add Boost guidelines to packages" TODO item as a rewrite. +- **The remaining fixes are all still live bugs on 2.x.** Every one was verified against the 2.x working tree: the vulnerable code is present, usually byte-identical to the pre-fix 1.x shape, sometimes with v2-specific amplifiers (e.g. real inventory makes an unfulfillable BuyXGetY reward line an uncalculable cart; morph-alias matching makes the FQCN reward-line bug self-perpetuating). + +Straight cherry-picks don't apply: namespaces moved to `Lunar\Core\…`, `Base/` was reorganised, actions are constructor-injected with typed signatures, money is `PriceValue`, admin code split into `admin`/`filament`, and the Inertia panel is a second UI surface. Each fix needs re-implementing in its v2 location. + +## Proposal + +Re-implement the outstanding 1.5.0 fixes on 2.x, grouped below. Each item lists the 1.x source PR and the v2 target. Behavioural intent is preserved; the mechanics adapt to v2. + +### A. BuyXGetY — conditions and automatic rewards (`core/src/DiscountTypes/BuyXGetY.php`) + +The four `processAutomaticRewards()` fixes are one textual stack and must land in order; the conditions guard is independent. + +1. **Honour discount conditions** (#2624) — add the `checkDiscountConditions($cart)` guard at the top of `apply()`, matching `AmountOff` and `ShippingDiscount`. Without it, min spend, customer restrictions and `max_uses_per_user` are skipped for BuyXGetY. +2. **Variant rewards** (#2631) — add the `instanceof Purchasable` branch in `processAutomaticRewards()`; today a `ProductVariant` reward fatals on `->variants->first()`. v2's admin already offers variants as rewards, so the crash is reachable. +3. **Morph-map reward lines** (#2632) — `get_class($purchasable)` → `$purchasable->getMorphClass()`. Worse in v2 than 1.x: `apply()` matches lines on morph aliases, so an FQCN-typed reward line is never re-matched on the next calculate. +4. **Multi-quantity rewards as one line** (#2639) — port the `$addedRewardLines` map and the quantity-increment branch. Adapt the money maths to `PriceValue` (`$rewardLine->unitPrice->multiply($rewardLine->quantity)`); drop the now-dead `$unitQuantity` local. +5. **Only fulfillable rewards** (#2509) — filter `discountableRewards` through `canBeFulfilledAtQuantity()` and cap allocation against stock. Higher stakes in v2: `CartLineStock` validation rejects unfulfillable lines, so an out-of-stock automatic reward currently wedges the cart. Check whether the spec-0038 inventory surface offers a more idiomatic availability check before hand-rolling. + +### B. Discount memoisation correctness (`core/src/Managers/DiscountManager.php`, `Models/Cart.php`, `Actions/Carts/CreateOrder.php`) + +6. **Scope the memoised discount set to the cart it was built for** (#2623) — port the `$discountsKey` cache key (cart id + coupon + customer + line purchasables) into `DiscountManager::apply()`/`resetDiscounts()`. Spec [[0064-scoped-service-lifetimes]] fixed cross-request leakage only; this intra-request staleness bug (coupon accepted, total unchanged until reload) is fully present. Note v2 has no `resetDiscounts()` callers, so the key is the only invalidation a storefront gets. +7. **A cart's own consumption doesn't count against it** (#2637 + follow-up #2638) — port as one unit: `Cart::consumedDiscountIds()` (memoised, with `forgetConsumedDiscountIds()` invalidated from `CreateOrder`), the `$exempt` parameter on `Discount::scopeUsable()`, the consumed-aware `checkDiscountConditions()`, and the `markAsUsed()` skip in `CreateOrder`. Port #2638's final shape (invalidate on order creation), not its intermediate `fresh:` parameter. Prerequisites (`discount_breakdown` raw JSON with `discount_id`) exist in v2 unchanged. + +### C. Cart, tax and shipping (core + table-rate-shipping) + +8. **Reprice the cart when the session currency changes** (#2625) — `unsetRelation('currency')`/`unsetRelation('lines')` in `CartSessionManager::setCurrency()`. v2-only decision: `StorefrontSessionManager::setCurrency()` (spec 0040) is a second currency-switching path that never touches the cart — the port must cover it or a region switch leaves the cart priced in the old currency (see open questions). +9. **Scope state tax-zone lookups to the address country** (#2626) — thread `?int $countryId` from `GetTaxZone` into `GetTaxZoneState` with the `whereNull('country_id') OR country_id = ?` constraint. `GetTaxZoneState` has no contract of its own, so no interface change. Region (spec 0039) left tax-zone resolution intact. +10. **Shipping-modifier re-entrancy guard** (#2630) — the `$resolving` flag + `finally` in `ShippingManifest::getOptions()`. Note the manifest is a `singleton` in v2, so the `finally` is load-bearing under Octane — a throwing modifier would otherwise wedge the whole worker. +11. **Weight tiers in the method's configured `weight_unit`** (#2645) — both halves: `ShipBy::resolve()` sums in `$shippingMethod->weight_unit ?: 'kg'` (the constraint path in `ShippingRateResolver` is already unit-aware; the tier path is not — the exact split #2645 fixed), and `ManageShippingRates` gets the unit-aware suffix/helper-text plus the `integer` rule. 14 table-rate-shipping locales gain the `:unit` placeholder. Keep the three existing #2382 regression tests green (they use methods with no `weight_unit`, so the `?: 'kg'` default preserves them). +12. **`SystemTaxDriver` uses `TaxZone::getDefault()`** (#2511) — one line; bigger payoff in v2 because `defaultTaxZone()` runs per purchasable inside `getBreakdown()`. +13. **Annotate `getDefault()` as `null|self`** (#2634) — docblock on `Models/Concerns/HasDefaultRecord`. v2's typed `CartSessionManager::getCurrency(): Currency` / `getChannel(): Channel` make the missing-default case a hard `TypeError`; whether that deserves a domain exception is an open question, not part of this port. + +### D. Core models and orders + +14. **Deterministic ordering for PostgreSQL** (#2518) — `Cart::lines()->orderBy('id')`, `Order::lines()->orderBy('id')`, `ProductVariant::values()->orderBy('position')->orderByPivot('id')`. Fingerprints and the snapshotted variant option label depend on iteration order; heap order is unstable on Postgres. Derived relations (`productLines()`, `shippingLines()`, …) inherit the fix. +15. **One order address per type** (#2507) — drop the `postcode` clause from the match in `CreateOrderAddresses`; a postcode change on recalculation currently duplicates the address row (no unique index to catch it). +16. **Stock events** (#2606 / #2226) — do **not** port the 1.x shape (an admin-namespaced event on a Filament page). The gap is spec 0038's documented-but-unshipped events section: dispatch `StockMovementRecorded` from `RecordStockMovement` and a variant-level stock-changed event from `RecomputeStockRollup` when `stock_available` actually changes (dirty-guarded), so both panels, `AdjustStock`, and the automatic order/fulfilment movements all emit for free. Wire the Scout re-index spec 0038 calls for, or record it as a follow-up. + +### E. Filament admin + +17. **Trim whitespace from form input** (#2642) — `TextInput`/`Textarea`/`TagsInput` `configureUsing(... ->trim())` in `LunarPanelManager::register()` (Livewire skips the global `TrimStrings` middleware). The Inertia panel needs nothing — its requests go through the normal HTTP stack. +18. **Product option mapping + new-variant tax class** (#2640) — fix the `search() === false` coercion and add the `copied_id`-from-oldest-sibling behaviour in the now-core `Actions/Products/MapVariantsToProductOptions.php`, plus the widget-side fallback in `ProductOptionsWidget`. The panel's `GenerateProductVariants` path is already structurally correct. +19. **Stop dropping media custom properties** (#2598) — `MediaRelationManager`: pass `custom_properties` wholesale on create, `array_merge` on edit. The panel's `AddMedia`/`UpdateMedia` actions already merge per key. +20. **Country/state selects reject fresh selections** (#2612) — retype the `getOptionLabelsUsing()` closures to `array $values` with direct `whereIn` lookups in `TaxZoneForm` (bridge) and `ShippingZoneForm` (table-rate-shipping). Fix the latent v2-only bug in the same pass: `ShippingZoneForm::getCountryComponent()` plucks `country.name` off a relation that doesn't exist, so its labels are always empty. +21. **Localize boolean meta values** (#2545) — the `is_bool` ternary in `ManageOrder::getDefaultAdditionalInfoSection()` plus `yes`/`no` keys in `global.php` across all 16 `packages/admin` locales (this file still resolves through `lunarpanel::` — don't move namespaces as part of the port). Keep the stringification in `getStateUsing` so the `strlen()` tooltip closure downstream stays safe. +22. **Unit quantity minimum of 1** (#2547) — `->minValue(1)` on `ProductVariantForm::getUnitQtyComponent()` and the `max(1, $unitQty)` clamp in `DefaultPriceFormatter` (division-by-zero guard). The panel already enforces `min:1` in `VariantFields`. Consider the sibling `min_quantity`/`quantity_increment` components while there. +23. **Translate product-options widget labels** (#2555) — `->label()` on the `addSharedOption` and `saveVariants` actions, the empty-state heading in the widget blade (delete the stale duplicate blade under `packages/admin` rather than fixing both), and a new `save-variants.label` key across all 16 `packages/filament` locales. +24. **Shipping permission labels** (#2484) — do **not** port the 1.x `mergeTranslationsForPanel()` merge hack. v2 centralises label resolution in the `Permission` DTO on `lunar::auth.permissions.{handle}.*`, consumed by both the Filament staff form and the panel role/staff controllers — so either add the two `shipping:manage` keys to `packages/core` `auth.php` (16 locales), or have `ShippingServiceProvider` call `Lang::addLines()` into the `lunar` namespace. The latter doubles as the documented seam for third-party add-ons. Revisit the v1-era `lunarpanel.shipping` translation namespace in the same change. +25. **Collection breadcrumb gaps in discount selection** (#2557) — mostly covered by `CollectionSelect` already; close the deltas: prepend the collection group name to `optionLabel()` (+ eager-load `group`), give `CollectionConditionRelationManager`'s table column the breadcrumb `description()` + `morphWith` eager load for parity with the limitation manager, and eager-load `ancestors` in `AppliesToExistingSelect::applyTo()` (an N+1 that trips `preventLazyLoading()`). +26. **Digital lines on the PDF invoice** (#2599) — the bug survived the v2 refactor in a new form: `pdf/order.blade.php` iterates `fulfillableLines`, and non-shippable variants have `requires_fulfilment = false`, so they vanish from the invoice body. Switch to `productLines`. Audit other `fulfillableLines` call sites to confirm the invoice is the only mis-application. + +### F. Payments and pricing + +27. **Stripe amounts by currency decimal places** (#2601) — port the full conversion layer (`toStripeAmount()` rewrite, new `fromStripeAmount()`, zero/three-decimal and special-case currency tables, integer-only rounding) and apply it at capture, refund, intent-total assertion and `StoreCharges`. v2 adaptations: keep the concrete `Currency` type (v2's stripe package doesn't use model contracts); preserve v2's plain-int `$order->total` in `assertIntentMatchesTotal()`; port the supporting test infra (`MockClient` request capture + refund fixture, `CartBuilder` `currencyParams`, `StripeFake` scaling). Bundle the #2644 currency pin for `StoreChargesTest` — without it, this port introduces the random CI flake 1.x hit. +28. **`formatterStyle` is an `int`** (#2520) — `string` → `int` on the three `DefaultPriceFormatter` signatures **and** on `PriceFormatterInterface` (v2 widened the interface to declare the full parameter list, so both files must change together). + +## Alternatives considered + +- **Cherry-pick from 1.x**: rejected — the fork predates all 36 changes and v2 moved or reshaped every target (namespaces, `Base/` reorganisation, DI-typed actions, `PriceValue`, the admin/filament split, the panel). Each pick would conflict into a rewrite anyway. +- **Port everything verbatim, including shape**: rejected for the three items where 1.x fixed the symptom at the wrong layer for v2 — stock events (#2606) belong on the core write seam per spec 0038, permission labels (#2484) belong on the `Permission` DTO key convention, and the Stripe test pin (#2644) is meaningless without #2601. +- **Do nothing / wait for bug reports**: rejected — several of these are worse in v2 than in 1.x (unfulfillable reward lines wedge carts under real inventory; FQCN reward lines defeat v2's alias-based matching; the manifest re-entrancy guard protects a process-lifetime singleton). + +## Migration impact + +- **Database**: none. Every schema-adjacent 1.x change (nullable purchasable morph, weight-unit column) is already in the v2 baseline. +- **Public contract surface**: additive only — `Discount::scopeUsable()` gains an optional `$exempt` parameter, `Cart` gains `consumedDiscountIds()`/`forgetConsumedDiscountIds()`, `GetTaxZoneState::execute()` gains an optional `$countryId`, new stock events in `Lunar\Core\Events\Products`. `PriceFormatterInterface`'s `formatterStyle` retype is a signature change on a published interface; downstream implementations that copied the buggy `string` type need the same one-word fix (worth a Rector note in the upgrade package). +- **Upgrade path**: no new v1 → v2 data transformations. +- **Translations**: four locale batches — `:unit` placeholder in table-rate-shipping `relationmanagers.php` (14 locales), `yes`/`no` in admin `global.php` (16), `save-variants.label` in filament `productoption.php` (16), `shipping:manage` permission label/description (16, location per item 24). Translate values per locale; no English placeholders. +- **Filament / admin**: items 17–26 above; no new screens, all fixes to existing surfaces. Panel is unaffected except where noted (already correct or section not yet built — the orders/discounts panel sections should pick up items 21 and 25 as acceptance criteria when they land). + +## Open questions + +- **Currency switch propagation** (item 8): should `StorefrontSessionManager::setCurrency()` propagate to `CartSession::setCurrency()` so region switches reprice the cart, or is repeating the relation reset enough? Owner: Glenn. +- **Missing default records** (item 13): after annotating `getDefault()` nullable, the typed `CartSessionManager::getCurrency()/getChannel()` fail with a `TypeError` when no default exists. Throw a domain exception instead? Also confirm the annotation is CI-neutral at the current PHPStan level before landing. Owner: Glenn. +- **`ShippingManifest` lifetime**: should the manifest move to `scoped` alongside the spec-0064 set once it carries the `$resolving` flag? Adjacent to item 10, not blocking it. Owner: Glenn. +- **Stock events shape** (item 16): event names/payloads, and whether the Scout re-index on `stock_available` change ships in the same slice or as a follow-up. Owner: Glenn. +- **Attribute `default_value`**: wanted back in v2? If so it follows the spec-0062 validation-rules template (core field-type concept + both panels) as its own spec. Owner: Glenn. + +## References + +- 1.5.0 release: https://github.com/lunarphp/lunar/releases/tag/1.5.0 (range `1.4.0..1.5.0`, 36 first-parent merges) +- Already on 2.x: #2594 (≙ #2648), #2620 (≙ #2636), `2f38c2a2` (≙ #2510), spec [[0045-optional-purchasables-and-shipping-de-morph]] (≙ #2643) +- Related specs: [[0038-inventory-fundamentals]] (item 16 implements its events section), [[0064-scoped-service-lifetimes]] (orthogonal to item 6), 0019/0062 (attribute `default_value` history) + +## Implementation plan + +- [ ] Slice 1 — BuyXGetY: conditions guard + automatic-rewards stack (items 1–5, in order) +- [ ] Slice 2 — Discount memoisation: cache key + consumed-discount exemption (items 6–7) +- [ ] Slice 3 — Cart/tax/shipping core: currency reprice, state tax zones, manifest re-entrancy, `getDefault()` (items 8–10, 12–13) +- [ ] Slice 4 — Weight-unit tiers incl. 14 locales (item 11) +- [ ] Slice 5 — Core model fixes: deterministic ordering, one address per type (items 14–15) +- [ ] Slice 6 — Stock events per spec 0038 (item 16) +- [ ] Slice 7 — Filament admin batch 1: trim, option mapping, media properties, country/state selects (items 17–20) +- [ ] Slice 8 — Filament admin batch 2: boolean meta, unit qty, widget labels, breadcrumb gaps, PDF invoice, incl. locales (items 21–23, 25–26) +- [ ] Slice 9 — Shipping permission label seam incl. 16 locales (item 24) +- [ ] Slice 10 — Stripe amount conversion + test pin (item 27) +- [ ] Slice 11 — Pricing formatter: `formatterStyle` retype + unit-qty clamp (item 28 + the core half of item 22) From 63a9af341c48887ef33fcf22ef2d553bedaf10ef Mon Sep 17 00:00:00 2001 From: Glenn Jacobs Date: Wed, 26 Aug 2026 17:58:01 +0100 Subject: [PATCH 02/11] fix(core): honour conditions and fix automatic rewards in BuyXGetY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/core/src/DiscountTypes/BuyXGetY.php | 88 ++- .../DiscountTypes/BuyXGetYConditionsTest.php | 133 ++++ .../core/Unit/DiscountTypes/BuyXGetYTest.php | 655 ++++++++++++++++++ 3 files changed, 864 insertions(+), 12 deletions(-) create mode 100644 tests/core/Unit/DiscountTypes/BuyXGetYConditionsTest.php diff --git a/packages/core/src/DiscountTypes/BuyXGetY.php b/packages/core/src/DiscountTypes/BuyXGetY.php index b4d39bdbf6..7438c3c420 100644 --- a/packages/core/src/DiscountTypes/BuyXGetY.php +++ b/packages/core/src/DiscountTypes/BuyXGetY.php @@ -4,6 +4,7 @@ use Illuminate\Pipeline\Pipeline; use Illuminate\Support\Collection; +use Lunar\Core\Contracts\Purchasable; use Lunar\Core\DataObjects\PriceValue; use Lunar\Core\Models\Cart; use Lunar\Core\Models\CartLine; @@ -50,6 +51,10 @@ public function getRewardQuantity($linesQuantity, $minQty, $rewardQty, $maxRewar */ public function apply(Cart $cart): Cart { + if (! $this->checkDiscountConditions($cart)) { + return $cart; + } + $data = $this->discount->data; $minQty = $data['min_qty'] ?? null; @@ -224,23 +229,57 @@ public function apply(Cart $cart): Cart private function processAutomaticRewards(Cart $cart, int $remainingRewardQty, Collection $affectedLines, int $discountTotal) { + // Reward lines this run has added, keyed by purchasable. The check below + // reads $cart->lines, which never receives a line made here, so without + // this a reward quantity of three opens three lines of one rather than + // one line of three. + $addedRewardLines = []; + // we have lines to add if ($remainingRewardQty > 0) { - while ($remainingRewardQty > 0) { - $selectedRewardItem = $this->discount->discountableRewards->random()->discountable; + // Fulfillable products per collection reward, hydrated once here rather + // than re-queried on every iteration of the allocation loop below. + $fulfillableCollectionProducts = []; - if (! $selectedRewardItem) { - $remainingRewardQty--; + $fulfillableRewards = $this->discount->discountableRewards->filter(function ($discountableReward) use (&$fulfillableCollectionProducts) { + $rewardItem = $discountableReward->discountable; - continue; + if (! $rewardItem) { + return false; + } + + if ($rewardItem instanceof LunarCollection) { + $fulfillableCollectionProducts[$rewardItem->id] = $rewardItem->products() + ->with('variants') + ->get() + ->filter(fn ($p) => $p->variants->first()?->canBeFulfilledAtQuantity(1)) + ->values(); + + return $fulfillableCollectionProducts[$rewardItem->id]->isNotEmpty(); + } + + if ($rewardItem instanceof Purchasable) { + return $rewardItem->canBeFulfilledAtQuantity(1); } + return (bool) $rewardItem->variants()->first()?->canBeFulfilledAtQuantity(1); + }); + + if ($fulfillableRewards->isEmpty()) { + return [$affectedLines, $discountTotal]; + } + + while ($remainingRewardQty > 0) { + $selectedRewardItem = $fulfillableRewards->random()->discountable; + if ($selectedRewardItem instanceof LunarCollection) { - $product = $selectedRewardItem->products()->inRandomOrder()->first(); - $purchasable = $product?->variants()->first(); + $product = $fulfillableCollectionProducts[$selectedRewardItem->id]->random(); + $purchasable = $product->variants->first(); $selectedRewardItem = $product; + } elseif ($selectedRewardItem instanceof Purchasable) { + $purchasable = $selectedRewardItem; } else { - $purchasable = $selectedRewardItem->variants->first(); + $purchasable = $selectedRewardItem->variants()->first(); } if (! $purchasable) { @@ -249,14 +288,39 @@ private function processAutomaticRewards(Cart $cart, int $remainingRewardQty, Co continue; } + $rewardKey = $purchasable->getMorphClass().':'.$purchasable->id; + + // How many units of this reward this run has already allocated, + // since canBeFulfilledAtQuantity below must check against that + // running total rather than a fixed quantity of 1 each time. + $allocated = $addedRewardLines[$rewardKey]->quantity ?? 0; + + if (! $purchasable->canBeFulfilledAtQuantity($allocated + 1)) { + $remainingRewardQty--; + + continue; + } + // is it already in cart? - $rewardLine = $cart->lines->first(function ($line) use ($purchasable) { + $rewardLine = $addedRewardLines[$rewardKey] ?? $cart->lines->first(function ($line) use ($purchasable) { return $line->purchasable->id == $purchasable->id; }); + if ($rewardLine && isset($addedRewardLines[$rewardKey])) { + // Another unit of a reward this run already added: raise the + // quantity on that line. A line the shopper put in the cart + // themselves is left at the quantity they chose, as before. + $rewardLine->quantity++; + + $lineTotal = $rewardLine->unitPrice->multiply($rewardLine->quantity); + + $rewardLine->subTotal = $lineTotal; + $rewardLine->total = $lineTotal; + } + if (! $rewardLine) { $rewardLine = $cart->lines()->make([ - 'purchasable_type' => get_class($purchasable), + 'purchasable_type' => $purchasable->getMorphClass(), 'purchasable_id' => $purchasable->id, 'quantity' => 1, ]); @@ -279,11 +343,11 @@ private function processAutomaticRewards(Cart $cart, int $remainingRewardQty, Co return $cartLine; }); - $unitQuantity = $purchasable->getUnitQuantity(); - $rewardLine->subTotal = new PriceValue($rewardLine->unitPrice->value, $cart->currency); $rewardLine->taxAmount = new PriceValue(0, $cart->currency); $rewardLine->total = new PriceValue($rewardLine->unitPrice->value, $cart->currency); + + $addedRewardLines[$rewardKey] = $rewardLine; } $meta = $rewardLine->meta ?? json_decode('{}'); diff --git a/tests/core/Unit/DiscountTypes/BuyXGetYConditionsTest.php b/tests/core/Unit/DiscountTypes/BuyXGetYConditionsTest.php new file mode 100644 index 0000000000..7dc43b32f3 --- /dev/null +++ b/tests/core/Unit/DiscountTypes/BuyXGetYConditionsTest.php @@ -0,0 +1,133 @@ +create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + $currency = Currency::factory()->create(['code' => 'GBP', 'default' => true]); + + $conditionProduct = Product::factory()->create(); + $rewardProduct = Product::factory()->create(); + + $condition = ProductVariant::factory()->create(['product_id' => $conditionProduct->id]); + $reward = ProductVariant::factory()->create(['product_id' => $rewardProduct->id]); + + foreach ([$condition, $reward] as $variant) { + Price::factory()->create([ + 'price' => 1000, // £10 + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $variant->getMorphClass(), + 'priceable_id' => $variant->id, + ]); + } + + $discount = Discount::factory()->create(array_merge([ + 'type' => BuyXGetY::class, + 'name' => 'Buy one get one', + 'starts_at' => now()->subDay(), + 'data' => array_merge([ + 'min_qty' => 1, + 'reward_qty' => 1, + 'automatically_add_rewards' => false, + ], $discountData), + ], $discountAttributes)); + + $discount->channels()->attach([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subDay()], + ]); + + $discount->customerGroups()->attach([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()->subDay()], + ]); + + $discount->discountables()->create([ + 'discountable_type' => $conditionProduct->getMorphClass(), + 'discountable_id' => $conditionProduct->id, + 'type' => 'condition', + ]); + + $discount->discountables()->create([ + 'discountable_type' => $rewardProduct->getMorphClass(), + 'discountable_id' => $rewardProduct->id, + 'type' => 'reward', + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + 'coupon_code' => null, + ]); + + // £10 of the condition product and £10 of the reward product: £20 total. + $cart->lines()->create([ + 'purchasable_type' => $condition->getMorphClass(), + 'purchasable_id' => $condition->id, + 'quantity' => 1, + ]); + + $cart->lines()->create([ + 'purchasable_type' => $reward->getMorphClass(), + 'purchasable_id' => $reward->id, + 'quantity' => 1, + ]); + + return $cart->refresh(); +} + +test('is not applied when the cart is below the minimum spend', function () { + // £50 minimum against a £20 cart. + $cart = buyXGetYConditionCart([ + 'min_prices' => ['GBP' => 5000], + ]); + + $cart->calculate(); + + expect($cart->discountTotal->value)->toEqual(0); +}); + +test('is applied when the cart meets the minimum spend', function () { + // £10 minimum against a £20 cart. + $cart = buyXGetYConditionCart([ + 'min_prices' => ['GBP' => 1000], + ]); + + $cart->calculate(); + + expect($cart->discountTotal->value)->toBeGreaterThan(0); +}); + +test('is not applied when the cart customer is not on the discount', function () { + $cart = buyXGetYConditionCart([]); + + // Restrict the discount to a customer this cart does not belong to. + $discount = Discount::first(); + $discount->customers()->attach(Customer::factory()->create()->id); + + $cart->refresh()->calculate(); + + expect($cart->discountTotal->value)->toEqual(0); +}); diff --git a/tests/core/Unit/DiscountTypes/BuyXGetYTest.php b/tests/core/Unit/DiscountTypes/BuyXGetYTest.php index a2c7ec1b6c..dafe759d87 100644 --- a/tests/core/Unit/DiscountTypes/BuyXGetYTest.php +++ b/tests/core/Unit/DiscountTypes/BuyXGetYTest.php @@ -3,7 +3,9 @@ use Illuminate\Foundation\Testing\RefreshDatabase; use Lunar\Core\DiscountTypes\AmountOff; use Lunar\Core\DiscountTypes\BuyXGetY; +use Lunar\Core\Enums\SellingPolicy; use Lunar\Core\Models\Cart; +use Lunar\Core\Models\CartLine; use Lunar\Core\Models\Channel; use Lunar\Core\Models\Collection; use Lunar\Core\Models\Currency; @@ -1548,3 +1550,656 @@ // productC (not in collection) should not be discounted expect($lineC->discountTotal->value)->toEqual(0); }); + +test('can store an automatically added reward line using the morph map', function () { + $customerGroup = CustomerGroup::factory()->create([ + 'default' => true, + ]); + + $channel = Channel::factory()->create([ + 'default' => true, + ]); + + $currency = Currency::factory()->create([ + 'code' => 'GBP', + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + $productA = Product::factory()->create(); + $productB = Product::factory()->create(); + + $purchasableA = ProductVariant::factory()->create([ + 'product_id' => $productA->id, + ]); + $purchasableB = ProductVariant::factory()->create([ + 'product_id' => $productB->id, + ]); + + Price::factory()->create([ + 'price' => 1000, // £10 + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasableA->getMorphClass(), + 'priceable_id' => $purchasableA->id, + ]); + + Price::factory()->create([ + 'price' => 1000, // £10 + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasableB->getMorphClass(), + 'priceable_id' => $purchasableB->id, + ]); + + $cart->lines()->create([ + 'purchasable_type' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Product Discount', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 1, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => [ + 'enabled' => true, + 'starts_at' => now(), + ], + ]); + + $discount->channels()->sync([ + $channel->id => [ + 'enabled' => true, + 'starts_at' => now()->subHour(), + ], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + $discount->discountableRewards()->create([ + 'discountable_type' => $productB->getMorphClass(), + 'discountable_id' => $productB->id, + 'type' => 'reward', + ]); + + $cart->calculate(); + + $rewardLine = CartLine::where('cart_id', $cart->id) + ->where('purchasable_id', $purchasableB->id) + ->first(); + + expect($rewardLine->purchasable_type)->toEqual($purchasableB->getMorphClass()); +}); + +test('can add a multi quantity reward as a single line', function () { + $customerGroup = CustomerGroup::factory()->create([ + 'default' => true, + ]); + + $channel = Channel::factory()->create([ + 'default' => true, + ]); + + $currency = Currency::factory()->create([ + 'code' => 'GBP', + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + $productA = Product::factory()->create(); + $productB = Product::factory()->create(); + + $purchasableA = ProductVariant::factory()->create([ + 'product_id' => $productA->id, + ]); + $purchasableB = ProductVariant::factory()->create([ + 'product_id' => $productB->id, + ]); + + foreach ([$purchasableA, $purchasableB] as $purchasable) { + Price::factory()->create([ + 'price' => 1000, // £10 + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasable->getMorphClass(), + 'priceable_id' => $purchasable->id, + ]); + } + + $cart->lines()->create([ + 'purchasable_type' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Product Discount', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 3, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => [ + 'enabled' => true, + 'starts_at' => now(), + ], + ]); + + $discount->channels()->sync([ + $channel->id => [ + 'enabled' => true, + 'starts_at' => now()->subHour(), + ], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + $discount->discountableRewards()->create([ + 'discountable_type' => $productB->getMorphClass(), + 'discountable_id' => $productB->id, + 'type' => 'reward', + ]); + + $cart->calculate(); + + $rewardLines = CartLine::where('cart_id', $cart->id) + ->where('purchasable_id', $purchasableB->id) + ->get(); + + // Three of one thing is one line of three, not three lines of one. + expect($rewardLines)->toHaveCount(1); + expect($rewardLines->first()->quantity)->toEqual(3); +}); + +test('can leave a reward line the shopper added at their own quantity', function () { + $customerGroup = CustomerGroup::factory()->create([ + 'default' => true, + ]); + + $channel = Channel::factory()->create([ + 'default' => true, + ]); + + $currency = Currency::factory()->create([ + 'code' => 'GBP', + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + $productA = Product::factory()->create(); + $productB = Product::factory()->create(); + + $purchasableA = ProductVariant::factory()->create([ + 'product_id' => $productA->id, + ]); + $purchasableB = ProductVariant::factory()->create([ + 'product_id' => $productB->id, + ]); + + foreach ([$purchasableA, $purchasableB] as $purchasable) { + Price::factory()->create([ + 'price' => 1000, // £10 + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasable->getMorphClass(), + 'priceable_id' => $purchasable->id, + ]); + } + + $cart->lines()->create([ + 'purchasable_type' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + // The shopper already put the reward product in the cart themselves. + $cart->lines()->create([ + 'purchasable_type' => $purchasableB->getMorphClass(), + 'purchasable_id' => $purchasableB->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Product Discount', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 3, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => [ + 'enabled' => true, + 'starts_at' => now(), + ], + ]); + + $discount->channels()->sync([ + $channel->id => [ + 'enabled' => true, + 'starts_at' => now()->subHour(), + ], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + $discount->discountableRewards()->create([ + 'discountable_type' => $productB->getMorphClass(), + 'discountable_id' => $productB->id, + 'type' => 'reward', + ]); + + $cart->calculate(); + + $rewardLines = CartLine::where('cart_id', $cart->id) + ->where('purchasable_id', $purchasableB->id) + ->get(); + + expect($rewardLines)->toHaveCount(1); + expect($rewardLines->first()->quantity)->toEqual(1); +}); +test('does not automatically add reward when product variant has no stock', function () { + $customerGroup = CustomerGroup::factory()->create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + $currency = Currency::factory()->create(['code' => 'GBP']); + + $productA = Product::factory()->create(); + $productB = Product::factory()->create(); + + $purchasableA = ProductVariant::factory()->create(['product_id' => $productA->id]); + $purchasableB = ProductVariant::factory()->create([ + 'product_id' => $productB->id, + 'selling_policy' => SellingPolicy::InStock, + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + foreach ([$purchasableA, $purchasableB] as $purchasable) { + Price::factory()->create([ + 'price' => 1000, + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasable->getMorphClass(), + 'priceable_id' => $purchasable->id, + ]); + } + + $cart->lines()->create([ + 'purchasable_type' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Automatic Reward Out Of Stock', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 1, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + $discount->discountableRewards()->create([ + 'discountable_type' => $productB->getMorphClass(), + 'discountable_id' => $productB->id, + 'type' => 'reward', + ]); + + $cart = $cart->calculate(); + + expect($cart->freeItems)->toBeNull(); +}); + +test('does not automatically add collection reward when all products are out of stock', function () { + $customerGroup = CustomerGroup::factory()->create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + $currency = Currency::factory()->create(['code' => 'GBP']); + + $collection = Collection::factory()->create(); + + $productA = Product::factory()->create(); + $productB = Product::factory()->create(); + + $productB->collections()->sync($collection); + + $purchasableA = ProductVariant::factory()->create(['product_id' => $productA->id]); + $purchasableB = ProductVariant::factory()->create([ + 'product_id' => $productB->id, + 'selling_policy' => SellingPolicy::InStock, + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + foreach ([$purchasableA, $purchasableB] as $purchasable) { + Price::factory()->create([ + 'price' => 1000, + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasable->getMorphClass(), + 'priceable_id' => $purchasable->id, + ]); + } + + $cart->lines()->create([ + 'purchasable_type' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Automatic Collection Reward Out Of Stock', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 1, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + $discount->discountableRewards()->create([ + 'discountable_type' => $collection->getMorphClass(), + 'discountable_id' => $collection->id, + 'type' => 'reward', + ]); + + $cart = $cart->calculate(); + + expect($cart->freeItems)->toBeNull(); +}); + +test('automatically adds collection reward selecting in-stock product when others are out of stock', function () { + $customerGroup = CustomerGroup::factory()->create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + $currency = Currency::factory()->create(['code' => 'GBP']); + + $collection = Collection::factory()->create(); + + $productA = Product::factory()->create(); + $productB = Product::factory()->create(); // out of stock + $productC = Product::factory()->create(); // in stock + + $productB->collections()->sync($collection); + $productC->collections()->sync($collection); + + $purchasableA = ProductVariant::factory()->create(['product_id' => $productA->id]); + $purchasableB = ProductVariant::factory()->create([ + 'product_id' => $productB->id, + 'selling_policy' => SellingPolicy::InStock, + ]); + $purchasableC = ProductVariant::factory()->inStock(5)->create([ + 'product_id' => $productC->id, + 'selling_policy' => SellingPolicy::InStock, + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + foreach ([$purchasableA, $purchasableB, $purchasableC] as $purchasable) { + Price::factory()->create([ + 'price' => 1000, + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasable->getMorphClass(), + 'priceable_id' => $purchasable->id, + ]); + } + + $cart->lines()->create([ + 'purchasable_type' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Automatic Collection Reward Mixed Stock', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 1, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + $discount->discountableRewards()->create([ + 'discountable_type' => $collection->getMorphClass(), + 'discountable_id' => $collection->id, + 'type' => 'reward', + ]); + + $cart = $cart->calculate(); + + // Only the in-stock product (C) should be added as a free item + expect($cart->freeItems)->toHaveCount(1); + expect($cart->freeItems->first()->id)->toEqual($productC->id); +}); + +test('automatically adds in-stock product reward when another reward product is out of stock', function () { + $customerGroup = CustomerGroup::factory()->create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + $currency = Currency::factory()->create(['code' => 'GBP']); + + $productA = Product::factory()->create(); // condition + $productB = Product::factory()->create(); // reward — out of stock + $productC = Product::factory()->create(); // reward — in stock + + $purchasableA = ProductVariant::factory()->create(['product_id' => $productA->id]); + $purchasableB = ProductVariant::factory()->create([ + 'product_id' => $productB->id, + 'selling_policy' => SellingPolicy::InStock, + ]); + $purchasableC = ProductVariant::factory()->inStock(5)->create([ + 'product_id' => $productC->id, + 'selling_policy' => SellingPolicy::InStock, + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + foreach ([$purchasableA, $purchasableB, $purchasableC] as $purchasable) { + Price::factory()->create([ + 'price' => 1000, + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasable->getMorphClass(), + 'priceable_id' => $purchasable->id, + ]); + } + + $cart->lines()->create([ + 'purchasable_type' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Automatic Reward Mixed Stock', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 1, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + // Both B (no stock) and C (in stock) are rewards + $discount->discountableRewards()->createMany([ + ['discountable_type' => $productB->getMorphClass(), 'discountable_id' => $productB->id, 'type' => 'reward'], + ['discountable_type' => $productC->getMorphClass(), 'discountable_id' => $productC->id, 'type' => 'reward'], + ]); + + $cart = $cart->calculate(); + + // Product C (in stock) must be added; product B (out of stock) must never be added + expect($cart->freeItems)->toHaveCount(1); + expect($cart->freeItems->first()->id)->toEqual($productC->id); +}); + +test('does not allocate a multi quantity reward beyond stock', function () { + $customerGroup = CustomerGroup::factory()->create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + $currency = Currency::factory()->create(['code' => 'GBP']); + + $productA = Product::factory()->create(); + $productB = Product::factory()->create(); + + $purchasableA = ProductVariant::factory()->create(['product_id' => $productA->id]); + $purchasableB = ProductVariant::factory()->inStock(1)->create([ + 'product_id' => $productB->id, + 'selling_policy' => SellingPolicy::InStock, + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $currency->id, + ]); + + foreach ([$purchasableA, $purchasableB] as $purchasable) { + Price::factory()->create([ + 'price' => 1000, + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasable->getMorphClass(), + 'priceable_id' => $purchasable->id, + ]); + } + + $cart->lines()->create([ + 'purchasable_type' => $purchasableA->getMorphClass(), + 'purchasable_id' => $purchasableA->id, + 'quantity' => 1, + ]); + + $discount = Discount::factory()->create([ + 'type' => BuyXGetY::class, + 'name' => 'Test Automatic Reward Limited Stock', + 'data' => [ + 'min_qty' => 1, + 'reward_qty' => 3, + 'automatically_add_rewards' => true, + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->discountableConditions()->create([ + 'discountable_type' => $productA->getMorphClass(), + 'discountable_id' => $productA->id, + ]); + + $discount->discountableRewards()->create([ + 'discountable_type' => $productB->getMorphClass(), + 'discountable_id' => $productB->id, + 'type' => 'reward', + ]); + + $cart->calculate(); + + $rewardLines = CartLine::where('cart_id', $cart->id) + ->where('purchasable_id', $purchasableB->id) + ->get(); + + // Only one unit is in stock, so only one is allocated despite reward_qty 3. + expect($rewardLines)->toHaveCount(1); + expect($rewardLines->first()->quantity)->toEqual(1); +}); From 46165bef453705d3a9591b18510e571c089b1689 Mon Sep 17 00:00:00 2001 From: Glenn Jacobs Date: Wed, 26 Aug 2026 18:02:00 +0100 Subject: [PATCH 03/11] fix(core): scope the memoised discount set and exempt a cart's own uses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../core/src/Actions/Carts/CreateOrder.php | 17 +- .../DiscountTypes/AbstractDiscountType.php | 12 +- .../core/src/Managers/DiscountManager.php | 29 +- packages/core/src/Models/Cart.php | 52 +++ packages/core/src/Models/Discount.php | 17 +- .../Unit/Actions/Carts/CreateOrderTest.php | 422 ++++++++++++++++++ .../DiscountManagerMemoisationTest.php | 124 +++++ 7 files changed, 665 insertions(+), 8 deletions(-) create mode 100644 tests/core/Unit/Managers/DiscountManagerMemoisationTest.php diff --git a/packages/core/src/Actions/Carts/CreateOrder.php b/packages/core/src/Actions/Carts/CreateOrder.php index 39df4221d6..de211eea5f 100644 --- a/packages/core/src/Actions/Carts/CreateOrder.php +++ b/packages/core/src/Actions/Carts/CreateOrder.php @@ -26,6 +26,11 @@ public function execute( /** @var Cart $cart */ $order = $cart->draftOrder($orderIdToUpdate)->first() ?: App::make(Order::class); + // Read before the creation pipeline runs: MapDiscountBreakdown + // rewrites the order's breakdown, so afterwards every discount would + // look as though it had already been consumed. + $alreadyConsumed = $cart->consumedDiscountIds(); + if ($cart->hasCompletedOrders() && ! $allowMultipleOrders) { throw new DisallowMultipleCartOrdersException; } @@ -43,10 +48,20 @@ public function execute( return $order; }); - $cart->discounts?->each(function ($discount) use ($cart) { + // Creating the order again for the same cart — a declined card and a + // retry — must not consume a second use of the same discount. + $cart->discounts?->each(function ($discount) use ($cart, $alreadyConsumed) { + if ($alreadyConsumed->contains($discount->discount->id)) { + return; + } + $discount->markAsUsed($cart)->discount->save(); }); + // The breakdown has been rewritten, so anything still holding this + // cart must not read a set memoised before the order existed. + $cart->forgetConsumedDiscountIds(); + $cart->save(); MarkAsNewCustomer::dispatch($order->id); diff --git a/packages/core/src/DiscountTypes/AbstractDiscountType.php b/packages/core/src/DiscountTypes/AbstractDiscountType.php index d3cd683b26..f0973c0393 100644 --- a/packages/core/src/DiscountTypes/AbstractDiscountType.php +++ b/packages/core/src/DiscountTypes/AbstractDiscountType.php @@ -89,9 +89,17 @@ protected function checkDiscountConditions(Cart $cart): bool $lines = $this->getEligibleLines($cart); $validMinSpend = $minSpend ? $minSpend < $lines->sum('subTotal.value') : true; - $validMaxUses = $this->discount->max_uses ? $this->discount->uses < $this->discount->max_uses : true; + // A cart that already consumed this discount when its draft order was + // created must not then be blocked by its own use. Otherwise creating + // that order again — a declined card and a retry — re-prices it without + // the discount the shopper was quoted. + $alreadyConsumed = $cart->consumedDiscountIds()->contains($this->discount->id); - if ($validMaxUses && $this->discount->max_uses_per_user) { + $validMaxUses = $this->discount->max_uses + ? ($alreadyConsumed || $this->discount->uses < $this->discount->max_uses) + : true; + + if (! $alreadyConsumed && $validMaxUses && $this->discount->max_uses_per_user) { $validMaxUses = $cart->user && ($this->usesByUser($cart->user) < $this->discount->max_uses_per_user); } diff --git a/packages/core/src/Managers/DiscountManager.php b/packages/core/src/Managers/DiscountManager.php index 633398ec16..82db4953f6 100644 --- a/packages/core/src/Managers/DiscountManager.php +++ b/packages/core/src/Managers/DiscountManager.php @@ -35,6 +35,11 @@ class DiscountManager implements DiscountManagerContract */ protected ?Collection $discounts = null; + /** + * Identifies the cart state $discounts was built from. + */ + protected ?string $discountsKey = null; + /** * The available discount types * @@ -132,7 +137,7 @@ public function getDiscounts(?Cart $cart = null): Collection } return Discount::active() - ->usable() + ->usable($cart?->consumedDiscountIds() ?? []) ->channel($this->channels) ->customerGroup($this->customerGroups) ->with([ @@ -217,8 +222,11 @@ public function getApplied(): Collection public function apply(Cart $cart): Cart { - if (! $this->discounts || $this->discounts?->isEmpty()) { + $key = $this->getDiscountsCacheKey($cart); + + if ($this->discounts === null || $this->discountsKey !== $key) { $this->discounts = $this->getDiscounts($cart); + $this->discountsKey = $key; } foreach ($this->discounts as $discount) { @@ -236,9 +244,26 @@ public function apply(Cart $cart): Cart return $cart; } + /** + * Build the cache key for the memoised discounts. + * + * getDiscounts() filters on the cart's coupon code and on the purchasables + * in its lines, so a set built for one cart state is not valid for another. + */ + protected function getDiscountsCacheKey(Cart $cart): string + { + return implode('|', [ + $cart->id, + $cart->coupon_code ?? '', + $cart->customer_id ?? '', + $cart->lines->map(fn ($line) => $line->purchasable_type.':'.$line->purchasable_id)->sort()->implode(','), + ]); + } + public function resetDiscounts(): self { $this->discounts = null; + $this->discountsKey = null; return $this; } diff --git a/packages/core/src/Models/Cart.php b/packages/core/src/Models/Cart.php index 73d3bd1845..45811f5b6b 100644 --- a/packages/core/src/Models/Cart.php +++ b/packages/core/src/Models/Cart.php @@ -300,6 +300,11 @@ public function scopeActive(Builder $query): Builder }); } + /** + * Memoised result of {@see self::consumedDiscountIds()}. + */ + protected ?Collection $consumedDiscountIds = null; + /** * Return the draft order relationship. */ @@ -311,6 +316,53 @@ public function draftOrder(?int $draftOrderId = null): HasOne })->whereNull('placed_at'); } + /** + * The ids of any discounts this cart has already consumed. + * + * Order creation records a use as soon as the draft order exists, so a + * checkout that runs it a second time — a declined card and a retry — would + * otherwise find its own coupon exhausted and re-price that same order + * without it. A cart's own consumption must not count against it. + * + * Memoised per instance, because this is read once when the discount set is + * rebuilt and once per discount while conditions are checked — a fresh query + * each time costs a single-row lookup per discount on every calculate, + * including for carts that never reach a checkout. Order creation is the + * only thing that changes the answer, so it forgets the memo. + * + * @see self::forgetConsumedDiscountIds() + */ + public function consumedDiscountIds(): Collection + { + if ($this->consumedDiscountIds !== null) { + return $this->consumedDiscountIds; + } + + // Read the raw column: the cast hydrates an OrderLine per breakdown + // line, which is a lot of work to reach an id. + $breakdown = $this->draftOrder()->first()?->getRawOriginal('discount_breakdown'); + + return $this->consumedDiscountIds = collect(json_decode($breakdown ?: '[]', true) ?: []) + ->pluck('discount_id') + ->filter() + ->unique() + ->values(); + } + + /** + * Forget the memoised consumed discount ids. + * + * Order creation writes the breakdown consumedDiscountIds() reads, so a set + * memoised before it ran is stale afterwards: on a same-request retry the + * cart's own coupon would look unconsumed to CreateOrder and be consumed + * twice, and exhausted to the discount conditions, re-pricing the order + * without it. + */ + public function forgetConsumedDiscountIds(): void + { + $this->consumedDiscountIds = null; + } + public function currentDraftOrder(?int $draftOrderId = null) { return $this->calculate() diff --git a/packages/core/src/Models/Discount.php b/packages/core/src/Models/Discount.php index 7257f8ba7b..1f01771e0a 100644 --- a/packages/core/src/Models/Discount.php +++ b/packages/core/src/Models/Discount.php @@ -270,11 +270,22 @@ public function scopeProductVariants(Builder $query, iterable $variantIds = [], ); } - public function scopeUsable(Builder $query): Builder + /** + * @param iterable $exempt Discount ids that stay usable whatever their + * use count — a cart that already consumed a + * discount must still be able to re-price with it. + */ + public function scopeUsable(Builder $query, iterable $exempt = []): Builder { - return $query->where(function ($subQuery) { + $exempt = collect($exempt)->filter()->values(); + + return $query->where(function ($subQuery) use ($exempt) { $subQuery->whereRaw('uses < max_uses') - ->orWhereNull('max_uses'); + ->orWhereNull('max_uses') + ->when( + $exempt->isNotEmpty(), + fn ($subQuery) => $subQuery->orWhereIn('id', $exempt) + ); }); } } diff --git a/tests/core/Unit/Actions/Carts/CreateOrderTest.php b/tests/core/Unit/Actions/Carts/CreateOrderTest.php index 8bd62866a4..65358478fd 100644 --- a/tests/core/Unit/Actions/Carts/CreateOrderTest.php +++ b/tests/core/Unit/Actions/Carts/CreateOrderTest.php @@ -4,14 +4,18 @@ use Lunar\Core\Actions\Carts\CreateOrder; use Lunar\Core\DataObjects\PriceValue as PriceDataType; use Lunar\Core\DataTypes\ShippingOption; +use Lunar\Core\DiscountTypes\AmountOff; use Lunar\Core\Exceptions\DisallowMultipleCartOrdersException; +use Lunar\Core\Facades\Discounts; use Lunar\Core\Facades\ShippingManifest; use Lunar\Core\Models\Cart; use Lunar\Core\Models\CartAddress; +use Lunar\Core\Models\Channel; use Lunar\Core\Models\Country; use Lunar\Core\Models\Currency; use Lunar\Core\Models\Customer; use Lunar\Core\Models\CustomerGroup; +use Lunar\Core\Models\Discount; use Lunar\Core\Models\Order; use Lunar\Core\Models\OrderAddress; use Lunar\Core\Models\OrderLine; @@ -347,3 +351,421 @@ function can_update_draft_order() $this->assertDatabaseHas((new Order)->getTable(), $datacheck); }); + +test('can keep the discount when the draft order is created again', function () { + TaxClass::factory()->create([ + 'default' => true, + ]); + + $customerGroup = CustomerGroup::factory()->create([ + 'default' => true, + ]); + + $channel = Channel::factory()->create([ + 'default' => true, + ]); + + $currency = Currency::factory()->create([ + 'decimal_places' => 2, + ]); + + $cart = Cart::factory()->create([ + 'currency_id' => $currency->id, + 'channel_id' => $channel->id, + 'coupon_code' => 'SAVE10', + ]); + + $purchasable = ProductVariant::factory()->create(); + + Price::factory()->create([ + 'price' => 1000, + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasable->getMorphClass(), + 'priceable_id' => $purchasable->id, + ]); + + $cart->lines()->create([ + 'purchasable_type' => $purchasable->getMorphClass(), + 'purchasable_id' => $purchasable->id, + 'quantity' => 2, + ]); + + // A single-use coupon, which is the ordinary shape of a promotional code. + $discount = Discount::factory()->create([ + 'type' => AmountOff::class, + 'name' => 'Ten off', + 'coupon' => 'SAVE10', + 'uses' => 0, + 'max_uses' => 1, + 'data' => [ + 'fixed_value' => true, + 'fixed_values' => [ + $currency->code => 500, + ], + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $cart->calculate(); + + $orderA = (new CreateOrder)->execute($cart)->refresh(); + + expect($orderA->discount_total)->toEqual(500); + expect($discount->refresh()->uses)->toEqual(1); + + // The card is declined and the shopper tries another one. That is a fresh + // request, so nothing is memoised from the first attempt. + Discounts::resetDiscounts(); + + $cart = Cart::find($cart->id); + $cart->calculate(); + + $orderB = (new CreateOrder)->execute($cart)->refresh(); + + expect($orderB->id)->toEqual($orderA->id); + expect($orderB->discount_total)->toEqual(500); + + // The retry must not consume a second use of a single-use coupon. + expect($discount->refresh()->uses)->toEqual(1); +}); + +test('can not reuse a discount another cart has exhausted', function () { + TaxClass::factory()->create([ + 'default' => true, + ]); + + $customerGroup = CustomerGroup::factory()->create([ + 'default' => true, + ]); + + $channel = Channel::factory()->create([ + 'default' => true, + ]); + + $currency = Currency::factory()->create([ + 'decimal_places' => 2, + ]); + + $purchasable = ProductVariant::factory()->create(); + + Price::factory()->create([ + 'price' => 1000, + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasable->getMorphClass(), + 'priceable_id' => $purchasable->id, + ]); + + $discount = Discount::factory()->create([ + 'type' => AmountOff::class, + 'name' => 'Ten off', + 'coupon' => 'SAVE10', + 'uses' => 0, + 'max_uses' => 1, + 'data' => [ + 'fixed_value' => true, + 'fixed_values' => [ + $currency->code => 500, + ], + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $makeCart = function () use ($currency, $channel, $purchasable) { + $cart = Cart::factory()->create([ + 'currency_id' => $currency->id, + 'channel_id' => $channel->id, + 'coupon_code' => 'SAVE10', + ]); + + $cart->lines()->create([ + 'purchasable_type' => $purchasable->getMorphClass(), + 'purchasable_id' => $purchasable->id, + 'quantity' => 2, + ]); + + return $cart; + }; + + $cartA = $makeCart(); + $cartA->calculate(); + $orderA = (new CreateOrder)->execute($cartA)->refresh(); + + expect($orderA->discount_total)->toEqual(500); + expect($discount->refresh()->uses)->toEqual(1); + + // A different shopper, with the last use already spent. + Discounts::resetDiscounts(); + + $cartB = $makeCart(); + $cartB->calculate(); + $orderB = (new CreateOrder)->execute($cartB)->refresh(); + + expect($orderB->id)->not->toEqual($orderA->id); + expect($orderB->discount_total)->toEqual(0); + expect($discount->refresh()->uses)->toEqual(1); +}); + +test('can still enforce other conditions on a discount the cart consumed', function () { + TaxClass::factory()->create([ + 'default' => true, + ]); + + $customerGroup = CustomerGroup::factory()->create([ + 'default' => true, + ]); + + $channel = Channel::factory()->create([ + 'default' => true, + ]); + + $currency = Currency::factory()->create([ + 'decimal_places' => 2, + ]); + + $cart = Cart::factory()->create([ + 'currency_id' => $currency->id, + 'channel_id' => $channel->id, + 'coupon_code' => 'SAVE10', + ]); + + $purchasable = ProductVariant::factory()->create(); + + Price::factory()->create([ + 'price' => 1000, + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasable->getMorphClass(), + 'priceable_id' => $purchasable->id, + ]); + + $line = $cart->lines()->create([ + 'purchasable_type' => $purchasable->getMorphClass(), + 'purchasable_id' => $purchasable->id, + 'quantity' => 2, + ]); + + // Spend at least 15.00 to qualify. Two units is 20.00, one is 10.00. + $discount = Discount::factory()->create([ + 'type' => AmountOff::class, + 'name' => 'Ten off', + 'coupon' => 'SAVE10', + 'uses' => 0, + 'max_uses' => 1, + 'data' => [ + 'fixed_value' => true, + 'fixed_values' => [ + $currency->code => 500, + ], + 'min_prices' => [ + $currency->code => 1500, + ], + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $cart->calculate(); + + $orderA = (new CreateOrder)->execute($cart)->refresh(); + + expect($orderA->discount_total)->toEqual(500); + + // The shopper drops a unit, taking the cart under the minimum spend. Being + // the cart that consumed the discount must not exempt it from that. + Discounts::resetDiscounts(); + + $cart = Cart::find($cart->id); + $cart->updateLine($line->id, 1); + $cart->calculate(); + + $orderB = (new CreateOrder)->execute($cart)->refresh(); + + expect($orderB->id)->toEqual($orderA->id); + expect($orderB->sub_total)->toEqual(1000); + expect($orderB->discount_total)->toEqual(0); +}); + +test('can not consume a discount twice on one cart instance', function () { + TaxClass::factory()->create([ + 'default' => true, + ]); + + $customerGroup = CustomerGroup::factory()->create([ + 'default' => true, + ]); + + $channel = Channel::factory()->create([ + 'default' => true, + ]); + + $currency = Currency::factory()->create([ + 'decimal_places' => 2, + ]); + + $cart = Cart::factory()->create([ + 'currency_id' => $currency->id, + 'channel_id' => $channel->id, + 'coupon_code' => 'SAVE10', + ]); + + $purchasable = ProductVariant::factory()->create(); + + Price::factory()->create([ + 'price' => 1000, + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasable->getMorphClass(), + 'priceable_id' => $purchasable->id, + ]); + + $cart->lines()->create([ + 'purchasable_type' => $purchasable->getMorphClass(), + 'purchasable_id' => $purchasable->id, + 'quantity' => 2, + ]); + + $discount = Discount::factory()->create([ + 'type' => AmountOff::class, + 'name' => 'Ten off', + 'coupon' => 'SAVE10', + 'uses' => 0, + 'max_uses' => 1, + 'data' => [ + 'fixed_value' => true, + 'fixed_values' => [ + $currency->code => 500, + ], + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + // The same instance throughout: no reload between the two attempts. This is + // what a checkout that retries in one request looks like, and it is the case + // any memoisation of consumedDiscountIds() has to survive - a set cached + // before the first order exists would still be empty for the second. + $cart->calculate(); + + $orderA = (new CreateOrder)->execute($cart)->refresh(); + + expect($orderA->discount_total)->toEqual(500); + expect($discount->refresh()->uses)->toEqual(1); + + $cart->calculate(); + + $orderB = (new CreateOrder)->execute($cart)->refresh(); + + expect($orderB->id)->toEqual($orderA->id); + expect($orderB->discount_total)->toEqual(500); + expect($discount->refresh()->uses)->toEqual(1); +}); + +test('keeps its own discount when a cart is priced again after order creation', function () { + TaxClass::factory()->create([ + 'default' => true, + ]); + + $customerGroup = CustomerGroup::factory()->create([ + 'default' => true, + ]); + + $channel = Channel::factory()->create([ + 'default' => true, + ]); + + $currency = Currency::factory()->create([ + 'decimal_places' => 2, + ]); + + $cart = Cart::factory()->create([ + 'currency_id' => $currency->id, + 'channel_id' => $channel->id, + 'coupon_code' => 'SAVE10', + ]); + + $purchasable = ProductVariant::factory()->create(); + + Price::factory()->create([ + 'price' => 1000, + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $purchasable->getMorphClass(), + 'priceable_id' => $purchasable->id, + ]); + + $cart->lines()->create([ + 'purchasable_type' => $purchasable->getMorphClass(), + 'purchasable_id' => $purchasable->id, + 'quantity' => 2, + ]); + + $discount = Discount::factory()->create([ + 'type' => AmountOff::class, + 'name' => 'Ten off', + 'coupon' => 'SAVE10', + 'uses' => 0, + 'max_uses' => 1, + 'data' => [ + 'fixed_value' => true, + 'fixed_values' => [ + $currency->code => 500, + ], + ], + ]); + + $discount->customerGroups()->sync([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $discount->channels()->sync([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subHour()], + ]); + + $cart->calculate(); + + $orderA = (new CreateOrder)->execute($cart)->refresh(); + + expect($orderA->discount_total)->toEqual(500); + expect($discount->refresh()->uses)->toEqual(1); + + // Priced again on the same instance, with the discount set rebuilt: the + // cart's own use must not read as an exhausted coupon, or the retry is + // re-priced without the discount the shopper was quoted. + Discounts::resetDiscounts(); + + $cart->recalculate(); + + $orderB = (new CreateOrder)->execute($cart)->refresh(); + + expect($orderB->id)->toEqual($orderA->id); + expect($orderB->discount_total)->toEqual(500); + expect($discount->refresh()->uses)->toEqual(1); +}); diff --git a/tests/core/Unit/Managers/DiscountManagerMemoisationTest.php b/tests/core/Unit/Managers/DiscountManagerMemoisationTest.php new file mode 100644 index 0000000000..864944a331 --- /dev/null +++ b/tests/core/Unit/Managers/DiscountManagerMemoisationTest.php @@ -0,0 +1,124 @@ +create([ + 'code' => 'GBP', + 'default' => true, + ]); + + $customerGroup = CustomerGroup::factory()->create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + + $variant = ProductVariant::factory()->create(); + + Price::factory()->create([ + 'price' => 1000, + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $variant->getMorphClass(), + 'priceable_id' => $variant->id, + ]); + + $attach = function (Discount $discount) use ($channel, $customerGroup) { + $discount->channels()->attach([ + $channel->id => ['enabled' => true, 'starts_at' => now()->subDay()], + ]); + + $discount->customerGroups()->attach([ + $customerGroup->id => ['enabled' => true, 'starts_at' => now()->subDay()], + ]); + }; + + // An always-on discount is required to reproduce this: apply() re-queries + // whenever the memoised set is empty, so the staleness only shows once the + // first calculation already found something. + $attach(Discount::factory()->create([ + 'type' => AmountOff::class, + 'name' => 'Always on', + 'coupon' => null, + 'starts_at' => now()->subDay(), + 'data' => ['fixed_value' => true, 'fixed_values' => ['GBP' => 1]], + ])); + + $attach(Discount::factory()->create([ + 'type' => AmountOff::class, + 'name' => 'Coupon discount', + 'coupon' => 'SAVE', + 'starts_at' => now()->subDay(), + 'data' => ['fixed_value' => true, 'fixed_values' => ['GBP' => 5]], + ])); + + $cart = Cart::factory()->create([ + 'currency_id' => $currency->id, + 'channel_id' => $channel->id, + 'coupon_code' => null, + ]); + + $cart->lines()->create([ + 'purchasable_type' => $variant->getMorphClass(), + 'purchasable_id' => $variant->id, + 'quantity' => 2, + ]); + + return $cart->refresh(); +} + +test('a coupon applied after the cart has been calculated is applied', function () { + $cart = discountMemoFixture(); + + // The storefront renders the cart, which calculates it. The cart has no + // coupon yet, so the memoised set cannot contain the coupon discount. + $cart->recalculate(); + + $withoutCoupon = $cart->total->value; + + $cart->coupon_code = 'SAVE'; + $cart->save(); + $cart->recalculate(); + + expect($cart->coupon_code)->toBe('SAVE') + ->and($cart->total->value)->toBeLessThan($withoutCoupon); +}); + +test('resetting the discount manager applies the same coupon', function () { + $cart = discountMemoFixture(); + + $cart->recalculate(); + $withoutCoupon = $cart->total->value; + + $cart->coupon_code = 'SAVE'; + $cart->save(); + + // Same cart, same coupon, only the memoised set dropped. + Discounts::resetDiscounts(); + $cart->recalculate(); + + expect($cart->total->value)->toBeLessThan($withoutCoupon); +}); From a6aef97ec2a48f5cc4467b4f392b4446c12568f8 Mon Sep 17 00:00:00 2001 From: Glenn Jacobs Date: Wed, 26 Aug 2026 18:10:08 +0100 Subject: [PATCH 04/11] fix(core): cart currency repricing, state tax zones and modifier re-entrancy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- TODO.md | 1 + .../core/src/Actions/Taxes/GetTaxZone.php | 2 +- .../src/Actions/Taxes/GetTaxZoneState.php | 30 +++-- packages/core/src/Drivers/SystemTaxDriver.php | 2 +- packages/core/src/LunarServiceProvider.php | 4 +- .../core/src/Managers/CartSessionManager.php | 7 ++ .../src/Managers/StorefrontSessionManager.php | 27 +++- .../core/src/Manifests/ShippingManifest.php | 29 ++++- .../src/Models/Concerns/HasDefaultRecord.php | 5 +- specs/0065-port-v1.5-fixes.md | 30 +++-- .../Stubs/TestRecursiveShippingModifier.php | 32 +++++ .../Actions/Taxes/GetTaxZoneStateTest.php | 80 ++++++++++++ .../Unit/Actions/Taxes/GetTaxZoneTest.php | 40 ++++++ tests/core/Unit/Base/ShippingManifestTest.php | 32 +++++ .../Unit/Base/Traits/HasDefaultRecordTest.php | 8 ++ .../Managers/CartSessionSetCurrencyTest.php | 115 ++++++++++++++++++ tests/core/Unit/ServiceLifetimesTest.php | 2 + 17 files changed, 413 insertions(+), 33 deletions(-) create mode 100644 tests/core/Stubs/TestRecursiveShippingModifier.php create mode 100644 tests/core/Unit/Managers/CartSessionSetCurrencyTest.php diff --git a/TODO.md b/TODO.md index 04888d7178..f2128c9e5f 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,7 @@ Items tagged _(judgement)_ are genuine line-calls worth revisiting. - Cache toolkit follow-ons — outbound webhooks, change feed / sync cursor, internal derived caches (ride spec 0043 events) - Cache-invalidation deferrals — media as a tracked satellite; a `CartLinesUpdated` event (from spec 0043) +- Stock events — `StockMovementRecorded`, a variant stock-changed event and the Scout re-index on `stock_available` change (spec 0038's unshipped events section; split out of spec 0065) - Location-scoped availability & stock routing — per-location stock selection + sell-time routing/splitting (follow-on to 0038) - Checkout stock reservations — hold stock via the `ReservesStock` seam at checkout start (follow-on to 0038) - Selling-policy rework — declarative model: deny-oversell, sell-against-incoming, continue-selling boolean (follow-on to 0038) diff --git a/packages/core/src/Actions/Taxes/GetTaxZone.php b/packages/core/src/Actions/Taxes/GetTaxZone.php index 4e8fa70ae7..caba7ac12b 100644 --- a/packages/core/src/Actions/Taxes/GetTaxZone.php +++ b/packages/core/src/Actions/Taxes/GetTaxZone.php @@ -24,7 +24,7 @@ public function execute(?Addressable $address = null): ?TaxZone } if ($address && $address->state) { - $stateZone = $this->getTaxZoneState->execute($address->state); + $stateZone = $this->getTaxZoneState->execute($address->state, $address->country_id); if ($stateZone) { return $stateZone->taxZone; } diff --git a/packages/core/src/Actions/Taxes/GetTaxZoneState.php b/packages/core/src/Actions/Taxes/GetTaxZoneState.php index d930693ff6..1e6b0f914b 100644 --- a/packages/core/src/Actions/Taxes/GetTaxZoneState.php +++ b/packages/core/src/Actions/Taxes/GetTaxZoneState.php @@ -12,9 +12,9 @@ class GetTaxZoneState * @param string $state * @return null|TaxZoneState */ - public function execute($state) + public function execute($state, ?int $countryId = null) { - $stateZone = $this->getZoneMatches($state); + $stateZone = $this->getZoneMatches($state, $countryId); if ($stateZone instanceof TaxZoneState) { return $stateZone; @@ -26,17 +26,33 @@ public function execute($state) /** * Return the zone or zones which match the given state name/code. * + * A state name or code is only unique within a country — WA is both + * Washington and Western Australia — so a country narrows the match. States + * with no country stay matchable by any address. + * * @param string $state * @return null|TaxZoneState */ - protected function getZoneMatches($state) + protected function getZoneMatches($state, ?int $countryId = null) { $state = (string) $state; - $stateZone = TaxZoneState::whereHas('state', function ($query) use ($state) { - return $query - ->where('name', $state) - ->orWhere('code', $state); + $stateZone = TaxZoneState::whereHas('state', function ($query) use ($state, $countryId) { + $query->where(function ($query) use ($state) { + return $query + ->where('name', $state) + ->orWhere('code', $state); + }); + + if ($countryId) { + $query->where(function ($query) use ($countryId) { + return $query + ->whereNull('country_id') + ->orWhere('country_id', $countryId); + }); + } + + return $query; })->whereHas('taxZone', function ($query) { return $query->where('active', true); })->first(); diff --git a/packages/core/src/Drivers/SystemTaxDriver.php b/packages/core/src/Drivers/SystemTaxDriver.php index 27e8061a49..659c36ed5c 100644 --- a/packages/core/src/Drivers/SystemTaxDriver.php +++ b/packages/core/src/Drivers/SystemTaxDriver.php @@ -172,6 +172,6 @@ public function getBreakdown($subTotal): TaxBreakdown protected function defaultTaxZone() { - return TaxZone::where('default', '=', 1)->first(); + return TaxZone::getDefault(); } } diff --git a/packages/core/src/LunarServiceProvider.php b/packages/core/src/LunarServiceProvider.php index 4267a10d5d..564d86fdf3 100644 --- a/packages/core/src/LunarServiceProvider.php +++ b/packages/core/src/LunarServiceProvider.php @@ -405,7 +405,9 @@ protected function registerServices(): void return new ShippingModifiers; }); - $this->app->singleton(ShippingManifest::class, function ($app) { + // Holds the resolved options for the cart in hand plus a re-entrancy + // flag, so it is per-request state, not a boot-time registry. + $this->app->scoped(ShippingManifest::class, function ($app) { return $app->make(ShippingManifestImpl::class); }); diff --git a/packages/core/src/Managers/CartSessionManager.php b/packages/core/src/Managers/CartSessionManager.php index 5198f69319..c1c2e84573 100644 --- a/packages/core/src/Managers/CartSessionManager.php +++ b/packages/core/src/Managers/CartSessionManager.php @@ -220,6 +220,13 @@ public function setCurrency(Currency $currency): void $this->cart->update([ 'currency_id' => $currency->id, ]); + + // Prices are resolved from the cart's loaded relations, and the + // lines carry their own copy of the cart (see lunar.cart.eager_load). + // Without dropping these the next calculate() prices in the old + // currency. + $this->cart->unsetRelation('currency'); + $this->cart->unsetRelation('lines'); } } diff --git a/packages/core/src/Managers/StorefrontSessionManager.php b/packages/core/src/Managers/StorefrontSessionManager.php index afd5a665e7..bc37bb918c 100644 --- a/packages/core/src/Managers/StorefrontSessionManager.php +++ b/packages/core/src/Managers/StorefrontSessionManager.php @@ -7,6 +7,7 @@ use Illuminate\Session\SessionManager; use Illuminate\Support\Collection; use Lunar\Core\Contracts\Actions\Storefront\ResolvesStorefrontContext; +use Lunar\Core\Contracts\CartSession; use Lunar\Core\Contracts\StorefrontSession; use Lunar\Core\DataObjects\StorefrontContext; use Lunar\Core\Exceptions\CustomerNotBelongsToUserException; @@ -32,6 +33,7 @@ public function __construct( protected SessionManager $sessionManager, protected AuthManager $authManager, protected ResolvesStorefrontContext $resolveStorefrontContext, + protected CartSession $cartSession, ) { $this->customerGroups = new Collection; @@ -142,6 +144,25 @@ public function getCurrency(): Currency } public function setCurrency(Currency $currency): static + { + $this->putCurrency($currency); + + // The cart carries its own currency_id and is priced from it, so a + // storefront switch has to reach the cart or the shopper keeps seeing + // the old currency. CartSession owns that write and the relation resets + // it needs; it is a no-op when the visitor has no cart. + $this->cartSession->setCurrency($currency); + + return $this; + } + + /** + * Store the currency without reaching for the cart. + * + * The boot cascade resolves a currency on every request, and must not + * calculate — or create — a cart to do it. + */ + protected function putCurrency(Currency $currency): void { $this->sessionManager->put( $this->getSessionKey().'_currency', @@ -149,8 +170,6 @@ public function setCurrency(Currency $currency): static ); $this->currency = $currency; - - return $this; } public function getCustomer(): ?Customer @@ -259,12 +278,12 @@ public function initCurrency(): void if ($sessionCurrency) { $currency = Currency::query()->where('code', $sessionCurrency)->firstOrFail(); - $this->setCurrency($currency); + $this->putCurrency($currency); return; } - $this->setCurrency($this->region?->currency ?? Currency::getDefault()); + $this->putCurrency($this->region?->currency ?? Currency::getDefault()); } public function initCustomer(): void diff --git a/packages/core/src/Manifests/ShippingManifest.php b/packages/core/src/Manifests/ShippingManifest.php index b7f96945fc..10b1176a7a 100644 --- a/packages/core/src/Manifests/ShippingManifest.php +++ b/packages/core/src/Manifests/ShippingManifest.php @@ -19,6 +19,11 @@ class ShippingManifest implements ShippingManifestContract public ?Closure $getOptionUsing = null; + /** + * Whether the shipping modifiers are currently being run. + */ + protected bool $resolving = false; + /** * Initiate the class. */ @@ -79,11 +84,25 @@ public function getOptionUsing(Closure $closure): self */ public function getOptions(Cart $cart): Collection { - app(Pipeline::class) - ->send($cart) - ->through( - app(ShippingModifiers::class)->getModifiers()->toArray() - )->thenReturn(); + // A modifier is free to calculate the cart, which runs ApplyShipping + // and lands back here. Hand back what has been resolved so far rather + // than running the modifiers again. The manifest is a singleton, so the + // flag must clear even when a modifier throws. + if ($this->resolving) { + return $this->options; + } + + $this->resolving = true; + + try { + app(Pipeline::class) + ->send($cart) + ->through( + app(ShippingModifiers::class)->getModifiers()->toArray() + )->thenReturn(); + } finally { + $this->resolving = false; + } return $this->options; } diff --git a/packages/core/src/Models/Concerns/HasDefaultRecord.php b/packages/core/src/Models/Concerns/HasDefaultRecord.php index 94f1c1d110..0906b69da3 100644 --- a/packages/core/src/Models/Concerns/HasDefaultRecord.php +++ b/packages/core/src/Models/Concerns/HasDefaultRecord.php @@ -22,7 +22,10 @@ public function scopeDefault($query, $default = true) /** * Get the default record. * - * @return self + * Null when nothing is marked as default — an install that has not been + * seeded, or a record that was unset. + * + * @return null|self */ public static function getDefault() { diff --git a/specs/0065-port-v1.5-fixes.md b/specs/0065-port-v1.5-fixes.md index afd1924767..2a9f06eb2c 100644 --- a/specs/0065-port-v1.5-fixes.md +++ b/specs/0065-port-v1.5-fixes.md @@ -36,9 +36,9 @@ The four `processAutomaticRewards()` fixes are one textual stack and must land i ### C. Cart, tax and shipping (core + table-rate-shipping) -8. **Reprice the cart when the session currency changes** (#2625) — `unsetRelation('currency')`/`unsetRelation('lines')` in `CartSessionManager::setCurrency()`. v2-only decision: `StorefrontSessionManager::setCurrency()` (spec 0040) is a second currency-switching path that never touches the cart — the port must cover it or a region switch leaves the cart priced in the old currency (see open questions). +8. **Reprice the cart when the session currency changes** (#2625) — `unsetRelation('currency')`/`unsetRelation('lines')` in `CartSessionManager::setCurrency()`. v2 also has a second currency-switching path: `StorefrontSessionManager::setCurrency()` (spec 0040) never touches the cart at all, so a region switch left it priced in the old currency. It now delegates to `CartSession::setCurrency()`, while the boot cascade writes through a non-propagating `putCurrency()` so resolving a currency per request never calculates or creates a cart. 9. **Scope state tax-zone lookups to the address country** (#2626) — thread `?int $countryId` from `GetTaxZone` into `GetTaxZoneState` with the `whereNull('country_id') OR country_id = ?` constraint. `GetTaxZoneState` has no contract of its own, so no interface change. Region (spec 0039) left tax-zone resolution intact. -10. **Shipping-modifier re-entrancy guard** (#2630) — the `$resolving` flag + `finally` in `ShippingManifest::getOptions()`. Note the manifest is a `singleton` in v2, so the `finally` is load-bearing under Octane — a throwing modifier would otherwise wedge the whole worker. +10. **Shipping-modifier re-entrancy guard** (#2630) — the `$resolving` flag + `finally` in `ShippingManifest::getOptions()`. The manifest also moves from `singleton` 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, and the `finally` keeps a throwing modifier from wedging the flag within that request. 11. **Weight tiers in the method's configured `weight_unit`** (#2645) — both halves: `ShipBy::resolve()` sums in `$shippingMethod->weight_unit ?: 'kg'` (the constraint path in `ShippingRateResolver` is already unit-aware; the tier path is not — the exact split #2645 fixed), and `ManageShippingRates` gets the unit-aware suffix/helper-text plus the `integer` rule. 14 table-rate-shipping locales gain the `:unit` placeholder. Keep the three existing #2382 regression tests green (they use methods with no `weight_unit`, so the `?: 'kg'` default preserves them). 12. **`SystemTaxDriver` uses `TaxZone::getDefault()`** (#2511) — one line; bigger payoff in v2 because `defaultTaxZone()` runs per purchasable inside `getBreakdown()`. 13. **Annotate `getDefault()` as `null|self`** (#2634) — docblock on `Models/Concerns/HasDefaultRecord`. v2's typed `CartSessionManager::getCurrency(): Currency` / `getChannel(): Channel` make the missing-default case a hard `TypeError`; whether that deserves a domain exception is an open question, not part of this port. @@ -47,7 +47,7 @@ The four `processAutomaticRewards()` fixes are one textual stack and must land i 14. **Deterministic ordering for PostgreSQL** (#2518) — `Cart::lines()->orderBy('id')`, `Order::lines()->orderBy('id')`, `ProductVariant::values()->orderBy('position')->orderByPivot('id')`. Fingerprints and the snapshotted variant option label depend on iteration order; heap order is unstable on Postgres. Derived relations (`productLines()`, `shippingLines()`, …) inherit the fix. 15. **One order address per type** (#2507) — drop the `postcode` clause from the match in `CreateOrderAddresses`; a postcode change on recalculation currently duplicates the address row (no unique index to catch it). -16. **Stock events** (#2606 / #2226) — do **not** port the 1.x shape (an admin-namespaced event on a Filament page). The gap is spec 0038's documented-but-unshipped events section: dispatch `StockMovementRecorded` from `RecordStockMovement` and a variant-level stock-changed event from `RecomputeStockRollup` when `stock_available` actually changes (dirty-guarded), so both panels, `AdjustStock`, and the automatic order/fulfilment movements all emit for free. Wire the Scout re-index spec 0038 calls for, or record it as a follow-up. +16. **Stock events** (#2606 / #2226) — **out of scope, deferred to its own spec.** The 1.x shape (an admin-namespaced event dispatched from a Filament page) is the wrong layer for v2 and would miss the panel controller, `AdjustStock`, and every automatic order/fulfilment movement. What v2 actually needs is spec 0038's documented-but-unshipped events section — `StockMovementRecorded` from `RecordStockMovement`, a variant-level stock-changed event from `RecomputeStockRollup`, and the Scout re-index on `stock_available` change. That is new surface rather than a port, so it belongs in a spec of its own against 0038 (tracked in `TODO.md`). ### E. Filament admin @@ -83,10 +83,15 @@ The four `processAutomaticRewards()` fixes are one textual stack and must land i ## Open questions -- **Currency switch propagation** (item 8): should `StorefrontSessionManager::setCurrency()` propagate to `CartSession::setCurrency()` so region switches reprice the cart, or is repeating the relation reset enough? Owner: Glenn. -- **Missing default records** (item 13): after annotating `getDefault()` nullable, the typed `CartSessionManager::getCurrency()/getChannel()` fail with a `TypeError` when no default exists. Throw a domain exception instead? Also confirm the annotation is CI-neutral at the current PHPStan level before landing. Owner: Glenn. -- **`ShippingManifest` lifetime**: should the manifest move to `scoped` alongside the spec-0064 set once it carries the `$resolving` flag? Adjacent to item 10, not blocking it. Owner: Glenn. -- **Stock events shape** (item 16): event names/payloads, and whether the Scout re-index on `stock_available` change ships in the same slice or as a follow-up. Owner: Glenn. +Resolved during implementation: + +- **Currency switch propagation** (item 8): `StorefrontSessionManager::setCurrency()` propagates to `CartSession::setCurrency()`, which owns the cart row write and the relation resets. The boot cascade (`initCurrency()`, and `initRegion()` which deliberately bypasses `setRegion()`) writes through a non-propagating `putCurrency()`, so resolving a currency on every request never calculates — or creates — a cart. +- **`ShippingManifest` lifetime**: moved to `scoped` and added to `ServiceLifetimesTest`. It holds the resolved options for the cart in hand plus the new `$resolving` flag, so it is per-request state rather than a boot-time registry. +- **Stock events** (item 16): deferred to its own spec against 0038. + +Still open: + +- **Missing default records** (item 13): the annotation lands as a docblock-only change (PHPStan level 0 is unaffected). Separately, the typed `CartSessionManager::getCurrency()/getChannel()` still fail with a `TypeError` when no default exists — whether that should be a domain exception is its own question. Owner: Glenn. - **Attribute `default_value`**: wanted back in v2? If so it follows the spec-0062 validation-rules template (core field-type concept + both panels) as its own spec. Owner: Glenn. ## References @@ -102,9 +107,8 @@ The four `processAutomaticRewards()` fixes are one textual stack and must land i - [ ] Slice 3 — Cart/tax/shipping core: currency reprice, state tax zones, manifest re-entrancy, `getDefault()` (items 8–10, 12–13) - [ ] Slice 4 — Weight-unit tiers incl. 14 locales (item 11) - [ ] Slice 5 — Core model fixes: deterministic ordering, one address per type (items 14–15) -- [ ] Slice 6 — Stock events per spec 0038 (item 16) -- [ ] Slice 7 — Filament admin batch 1: trim, option mapping, media properties, country/state selects (items 17–20) -- [ ] Slice 8 — Filament admin batch 2: boolean meta, unit qty, widget labels, breadcrumb gaps, PDF invoice, incl. locales (items 21–23, 25–26) -- [ ] Slice 9 — Shipping permission label seam incl. 16 locales (item 24) -- [ ] Slice 10 — Stripe amount conversion + test pin (item 27) -- [ ] Slice 11 — Pricing formatter: `formatterStyle` retype + unit-qty clamp (item 28 + the core half of item 22) +- [ ] Slice 6 — Filament admin batch 1: trim, option mapping, media properties, country/state selects (items 17–20) +- [ ] Slice 7 — Filament admin batch 2: boolean meta, unit qty, widget labels, breadcrumb gaps, PDF invoice, incl. locales (items 21–23, 25–26) +- [ ] Slice 8 — Shipping permission label seam incl. 16 locales (item 24) +- [ ] Slice 9 — Stripe amount conversion + test pin (item 27) +- [ ] Slice 10 — Pricing formatter: `formatterStyle` retype + unit-qty clamp (item 28 + the core half of item 22) diff --git a/tests/core/Stubs/TestRecursiveShippingModifier.php b/tests/core/Stubs/TestRecursiveShippingModifier.php new file mode 100644 index 0000000000..d5f2ab5599 --- /dev/null +++ b/tests/core/Stubs/TestRecursiveShippingModifier.php @@ -0,0 +1,32 @@ +calculate(); + } + + return $next($cart); + } +} diff --git a/tests/core/Unit/Actions/Taxes/GetTaxZoneStateTest.php b/tests/core/Unit/Actions/Taxes/GetTaxZoneStateTest.php index e749a37838..fa0f4c3d4e 100644 --- a/tests/core/Unit/Actions/Taxes/GetTaxZoneStateTest.php +++ b/tests/core/Unit/Actions/Taxes/GetTaxZoneStateTest.php @@ -2,6 +2,7 @@ use Illuminate\Foundation\Testing\RefreshDatabase; use Lunar\Core\Actions\Taxes\GetTaxZoneState; +use Lunar\Core\Models\Country; use Lunar\Core\Models\State; use Lunar\Core\Models\TaxZoneState; use Lunar\Tests\Core\TestCase; @@ -85,3 +86,82 @@ $this->assertNotEquals($al->id, $zone?->id); }); + +test('can match a state in the given country', function () { + $australia = Country::factory()->create([ + 'name' => 'Australia', + ]); + + $unitedStates = Country::factory()->create([ + 'name' => 'United States', + ]); + + $westernAustralia = State::factory()->create([ + 'country_id' => $australia->id, + 'code' => 'WA', + 'name' => 'Western Australia', + ]); + + $washington = State::factory()->create([ + 'country_id' => $unitedStates->id, + 'code' => 'WA', + 'name' => 'Washington', + ]); + + // Created first, so an unscoped lookup returns this one. + TaxZoneState::factory()->create([ + 'state_id' => $washington->id, + ]); + + $auZone = TaxZoneState::factory()->create([ + 'state_id' => $westernAustralia->id, + ]); + + $zone = app(GetTaxZoneState::class)->execute('WA', $australia->id); + + expect($zone?->id)->toEqual($auZone->id); +}); + +test('can mismatch a state in another country', function () { + $australia = Country::factory()->create([ + 'name' => 'Australia', + ]); + + $unitedStates = Country::factory()->create([ + 'name' => 'United States', + ]); + + $washington = State::factory()->create([ + 'country_id' => $unitedStates->id, + 'code' => 'WA', + 'name' => 'Washington', + ]); + + TaxZoneState::factory()->create([ + 'state_id' => $washington->id, + ]); + + $zone = app(GetTaxZoneState::class)->execute('WA', $australia->id); + + expect($zone)->toBeNull(); +}); + +test('can match a state which is not assigned to a country', function () { + $australia = Country::factory()->create([ + 'name' => 'Australia', + ]); + + $alabama = State::factory()->create([ + 'country_id' => null, + 'code' => 'AL', + 'name' => 'Alabama', + ]); + + $alZone = TaxZoneState::factory()->create([ + 'state_id' => $alabama->id, + ]); + + $zone = app(GetTaxZoneState::class)->execute('AL', $australia->id); + + expect($zone?->id)->toEqual($alZone->id); +}); diff --git a/tests/core/Unit/Actions/Taxes/GetTaxZoneTest.php b/tests/core/Unit/Actions/Taxes/GetTaxZoneTest.php index 7a57cfaac5..d1049887ff 100644 --- a/tests/core/Unit/Actions/Taxes/GetTaxZoneTest.php +++ b/tests/core/Unit/Actions/Taxes/GetTaxZoneTest.php @@ -89,3 +89,43 @@ expect($zone3->id)->toEqual($defaultTaxZone->id); }); + +test('can ignore a state tax zone from another country', function () { + $australia = Country::factory()->create([ + 'name' => 'Australia', + ]); + + $unitedStates = Country::factory()->create([ + 'name' => 'United States', + ]); + + $washington = State::factory()->create([ + 'country_id' => $unitedStates->id, + 'code' => 'WA', + 'name' => 'Washington', + ]); + + TaxZoneState::factory()->create([ + 'tax_zone_id' => TaxZone::factory(['default' => false]), + 'state_id' => $washington->id, + ]); + + $auZone = TaxZoneCountry::factory()->create([ + 'tax_zone_id' => TaxZone::factory(['default' => false]), + 'country_id' => $australia->id, + ]); + + TaxZone::factory(['default' => true])->create(); + + // Western Australia is also "WA", but has no tax zone of its own, so the + // Australian country zone should apply. + $address = Address::factory()->create([ + 'postcode' => '6000', + 'state' => 'WA', + 'country_id' => $australia->id, + ]); + + $zone = app(GetTaxZone::class)->execute($address); + + expect($zone->id)->toEqual($auZone->tax_zone_id); +}); diff --git a/tests/core/Unit/Base/ShippingManifestTest.php b/tests/core/Unit/Base/ShippingManifestTest.php index 29dcb66438..60ea450834 100644 --- a/tests/core/Unit/Base/ShippingManifestTest.php +++ b/tests/core/Unit/Base/ShippingManifestTest.php @@ -12,6 +12,8 @@ use Lunar\Core\Models\ProductVariant; use Lunar\Core\Models\TaxClass; use Lunar\Core\Models\TaxRateAmount; +use Lunar\Core\Modifiers\ShippingModifiers; +use Lunar\Tests\Core\Stubs\TestRecursiveShippingModifier; use Lunar\Tests\Core\TestCase; uses(TestCase::class); @@ -253,3 +255,33 @@ expect(ShippingManifest::getShippingOption($this->cart))->toBe($option); }); + +test('can not re-enter shipping modifiers while resolving options', function () { + TestRecursiveShippingModifier::$calls = 0; + + $taxClass = TaxClass::factory()->create(); + + CartAddress::factory()->create([ + 'cart_id' => $this->cart->id, + 'type' => 'shipping', + 'shipping_option' => 'BASDEL', + ]); + + ShippingManifest::addOption( + new ShippingOption( + name: 'Basic Delivery', + description: 'Basic Delivery', + identifier: 'BASDEL', + price: new PriceValue(500, $this->cart->currency), + taxClass: $taxClass + ) + ); + + app(ShippingModifiers::class)->add(TestRecursiveShippingModifier::class); + + $this->cart->calculate(); + + // ApplyShipping and CalculateTax each resolve the shipping option once, so + // a single calculate runs the modifiers twice. Neither run may re-enter. + expect(TestRecursiveShippingModifier::$calls)->toEqual(2); +}); diff --git a/tests/core/Unit/Base/Traits/HasDefaultRecordTest.php b/tests/core/Unit/Base/Traits/HasDefaultRecordTest.php index 10929f18d3..a86cb84b09 100644 --- a/tests/core/Unit/Base/Traits/HasDefaultRecordTest.php +++ b/tests/core/Unit/Base/Traits/HasDefaultRecordTest.php @@ -31,3 +31,11 @@ expect(Channel::getDefault()->id)->toEqual($defaultChannel->id); }); + +test('can return null when no record is marked as default', function () { + Channel::factory(3)->create([ + 'default' => false, + ]); + + expect(Channel::getDefault())->toBeNull(); +}); diff --git a/tests/core/Unit/Managers/CartSessionSetCurrencyTest.php b/tests/core/Unit/Managers/CartSessionSetCurrencyTest.php new file mode 100644 index 0000000000..1cb1627c6d --- /dev/null +++ b/tests/core/Unit/Managers/CartSessionSetCurrencyTest.php @@ -0,0 +1,115 @@ +create(['default' => true]); + $channel = Channel::factory()->create(['default' => true]); + + $gbp = Currency::factory()->create(['code' => 'GBP', 'default' => true, 'exchange_rate' => 1]); + $usd = Currency::factory()->create(['code' => 'USD', 'default' => false, 'exchange_rate' => 1]); + + $variant = ProductVariant::factory()->create(); + + Price::factory()->create([ + 'price' => 1000, // £10 + 'min_quantity' => 1, + 'currency_id' => $gbp->id, + 'priceable_type' => $variant->getMorphClass(), + 'priceable_id' => $variant->id, + ]); + + Price::factory()->create([ + 'price' => 2500, // $25 — deliberately unrelated to the GBP price + 'min_quantity' => 1, + 'currency_id' => $usd->id, + 'priceable_type' => $variant->getMorphClass(), + 'priceable_id' => $variant->id, + ]); + + $cart = Cart::factory()->create([ + 'channel_id' => $channel->id, + 'currency_id' => $gbp->id, + ]); + + $cart->lines()->create([ + 'purchasable_type' => $variant->getMorphClass(), + 'purchasable_id' => $variant->id, + 'quantity' => 1, + ]); + + CartSession::use($cart->refresh()); + + return [$cart, $gbp, $usd]; +} + +test('switching the session currency reprices the cart', function () { + [$cart, $gbp, $usd] = currencySwitchCart(); + + $cart->calculate(); + expect($cart->subTotal->value)->toEqual(1000); + + CartSession::setCurrency($usd); + + $cart->calculate(); + + expect($cart->subTotal->value)->toEqual(2500); +}); + +test('switching the session currency reprices a cart read back from the session', function () { + [$cart, $gbp, $usd] = currencySwitchCart(); + + $cart->calculate(); + + CartSession::setCurrency($usd); + + expect(CartSession::current()->subTotal->value)->toEqual(2500); +}); + +test('switching the storefront currency reprices the cart', function () { + [$cart, $gbp, $usd] = currencySwitchCart(); + + $cart->calculate(); + expect($cart->subTotal->value)->toEqual(1000); + + // The storefront session owns the visitor's currency; the cart carries its + // own currency_id, so the switch has to reach it. + StorefrontSession::setCurrency($usd); + + expect(CartSession::current()->subTotal->value)->toEqual(2500); +}); + +test('switching the storefront region reprices the cart', function () { + [$cart, $gbp, $usd] = currencySwitchCart(); + + $cart->calculate(); + + $region = Region::factory()->create([ + 'currency_id' => $usd->id, + ]); + + StorefrontSession::setRegion($region); + + expect(CartSession::current()->subTotal->value)->toEqual(2500); +}); diff --git a/tests/core/Unit/ServiceLifetimesTest.php b/tests/core/Unit/ServiceLifetimesTest.php index eadf6b6511..71a026825f 100644 --- a/tests/core/Unit/ServiceLifetimesTest.php +++ b/tests/core/Unit/ServiceLifetimesTest.php @@ -7,6 +7,7 @@ use Lunar\Core\Contracts\CacheInvalidator; use Lunar\Core\Contracts\CartSession; use Lunar\Core\Contracts\DiscountManager; +use Lunar\Core\Contracts\ShippingManifest; use Lunar\Core\Contracts\StorefrontSession; use Lunar\Core\Models\Channel; use Lunar\Core\Models\Currency; @@ -42,6 +43,7 @@ 'discount manager' => [DiscountManager::class], 'cache invalidator' => [CacheInvalidator::class], 'attribute cache' => [AttributeCache::class], + 'shipping manifest' => [ShippingManifest::class], 'access control manifest' => ['lunar-access-control'], ]); From 6242bfc6743634214276444de908e202746c29f7 Mon Sep 17 00:00:00 2001 From: Glenn Jacobs Date: Wed, 26 Aug 2026 18:12:01 +0100 Subject: [PATCH 05/11] fix(shipping): evaluate weight tiers in the method's configured weight_unit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../resources/lang/ar/relationmanagers.php | 2 +- .../resources/lang/bg/relationmanagers.php | 2 +- .../resources/lang/en/relationmanagers.php | 2 +- .../resources/lang/es/relationmanagers.php | 2 +- .../resources/lang/fa/relationmanagers.php | 2 +- .../resources/lang/fr/relationmanagers.php | 2 +- .../resources/lang/hr/relationmanagers.php | 2 +- .../resources/lang/hu/relationmanagers.php | 2 +- .../resources/lang/mn/relationmanagers.php | 2 +- .../resources/lang/pl/relationmanagers.php | 2 +- .../resources/lang/pt_BR/relationmanagers.php | 2 +- .../resources/lang/ro/relationmanagers.php | 2 +- .../resources/lang/tr/relationmanagers.php | 2 +- .../resources/lang/vi/relationmanagers.php | 2 +- .../src/Drivers/ShippingMethods/ShipBy.php | 6 +- .../Pages/ManageShippingRates.php | 25 +++++- .../Drivers/ShippingMethods/ShipByTest.php | 76 +++++++++++++++++++ 17 files changed, 118 insertions(+), 17 deletions(-) diff --git a/packages/table-rate-shipping/resources/lang/ar/relationmanagers.php b/packages/table-rate-shipping/resources/lang/ar/relationmanagers.php index 256ce25143..6358388299 100644 --- a/packages/table-rate-shipping/resources/lang/ar/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/ar/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'الحد الأدنى للوزن', - 'helper_text' => 'أدخل الوزن بالكيلوغرام', + 'helper_text' => 'أدخل الوزن بوحدة :unit', ], 'price' => [ 'label' => 'السعر', diff --git a/packages/table-rate-shipping/resources/lang/bg/relationmanagers.php b/packages/table-rate-shipping/resources/lang/bg/relationmanagers.php index 4b3c6d6981..4ccfa41d32 100644 --- a/packages/table-rate-shipping/resources/lang/bg/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/bg/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'Мин. тегло', - 'helper_text' => 'Въведете теглото в килограми', + 'helper_text' => 'Въведете теглото в :unit', ], 'price' => [ 'label' => 'Цена', diff --git a/packages/table-rate-shipping/resources/lang/en/relationmanagers.php b/packages/table-rate-shipping/resources/lang/en/relationmanagers.php index f0790ab92a..26f975c442 100644 --- a/packages/table-rate-shipping/resources/lang/en/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/en/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'Min. Weight', - 'helper_text' => 'Enter weight in kilograms', + 'helper_text' => 'Enter weight in :unit', ], 'price' => [ 'label' => 'Price', diff --git a/packages/table-rate-shipping/resources/lang/es/relationmanagers.php b/packages/table-rate-shipping/resources/lang/es/relationmanagers.php index 1cd276e8d1..c675104917 100644 --- a/packages/table-rate-shipping/resources/lang/es/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/es/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'Peso Mín.', - 'helper_text' => 'Introduce el peso en kilogramos', + 'helper_text' => 'Introduce el peso en :unit', ], 'price' => [ 'label' => 'Precio', diff --git a/packages/table-rate-shipping/resources/lang/fa/relationmanagers.php b/packages/table-rate-shipping/resources/lang/fa/relationmanagers.php index 725ac597d3..f23e0b9d46 100644 --- a/packages/table-rate-shipping/resources/lang/fa/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/fa/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'حداقل وزن', - 'helper_text' => 'وزن را به کیلوگرم وارد کنید', + 'helper_text' => 'وزن را به :unit وارد کنید', ], 'price' => [ 'label' => 'قیمت', diff --git a/packages/table-rate-shipping/resources/lang/fr/relationmanagers.php b/packages/table-rate-shipping/resources/lang/fr/relationmanagers.php index 3199e8ea0c..572e002c2d 100644 --- a/packages/table-rate-shipping/resources/lang/fr/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/fr/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'Poids Min.', - 'helper_text' => 'Saisir le poids en kilogrammes', + 'helper_text' => 'Saisir le poids en :unit', ], 'price' => [ 'label' => 'Prix', diff --git a/packages/table-rate-shipping/resources/lang/hr/relationmanagers.php b/packages/table-rate-shipping/resources/lang/hr/relationmanagers.php index ac73534201..7757dacc3f 100644 --- a/packages/table-rate-shipping/resources/lang/hr/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/hr/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'Min. težina', - 'helper_text' => 'Unesite težinu u kilogramima', + 'helper_text' => 'Unesite težinu u :unit', ], 'price' => [ 'label' => 'Cijena', diff --git a/packages/table-rate-shipping/resources/lang/hu/relationmanagers.php b/packages/table-rate-shipping/resources/lang/hu/relationmanagers.php index 05314ffd37..8955480f2a 100644 --- a/packages/table-rate-shipping/resources/lang/hu/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/hu/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'Min. súly', - 'helper_text' => 'Adja meg a súlyt kilogrammban', + 'helper_text' => 'Adja meg a súlyt :unit egységben', ], 'price' => [ 'label' => 'Ár', diff --git a/packages/table-rate-shipping/resources/lang/mn/relationmanagers.php b/packages/table-rate-shipping/resources/lang/mn/relationmanagers.php index cf2a9b58df..5420e7dc45 100644 --- a/packages/table-rate-shipping/resources/lang/mn/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/mn/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'Хамгийн бага жин', - 'helper_text' => 'Жинг килограммаар оруулна уу', + 'helper_text' => 'Жинг :unit-ээр оруулна уу', ], 'price' => [ 'label' => 'Үнэ', diff --git a/packages/table-rate-shipping/resources/lang/pl/relationmanagers.php b/packages/table-rate-shipping/resources/lang/pl/relationmanagers.php index 475f684949..ae3ebf30cd 100644 --- a/packages/table-rate-shipping/resources/lang/pl/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/pl/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'Min. Waga', - 'helper_text' => 'Podaj wagę w kilogramach', + 'helper_text' => 'Podaj wagę w :unit', ], 'price' => [ 'label' => 'Cena', diff --git a/packages/table-rate-shipping/resources/lang/pt_BR/relationmanagers.php b/packages/table-rate-shipping/resources/lang/pt_BR/relationmanagers.php index 6c0a3bd5a0..bcc7d1a72f 100644 --- a/packages/table-rate-shipping/resources/lang/pt_BR/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/pt_BR/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'Peso mín.', - 'helper_text' => 'Insira o peso em quilogramas', + 'helper_text' => 'Insira o peso em :unit', ], 'price' => [ 'label' => 'Preço', diff --git a/packages/table-rate-shipping/resources/lang/ro/relationmanagers.php b/packages/table-rate-shipping/resources/lang/ro/relationmanagers.php index ab9b971ba2..41f7c038e8 100644 --- a/packages/table-rate-shipping/resources/lang/ro/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/ro/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'Greutate min.', - 'helper_text' => 'Introduceți greutatea în kilograme', + 'helper_text' => 'Introduceți greutatea în :unit', ], 'price' => [ 'label' => 'Preț', diff --git a/packages/table-rate-shipping/resources/lang/tr/relationmanagers.php b/packages/table-rate-shipping/resources/lang/tr/relationmanagers.php index 0491f3be96..2bab315bf1 100644 --- a/packages/table-rate-shipping/resources/lang/tr/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/tr/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'Min. Ağırlık', - 'helper_text' => 'Ağırlığı kilogram olarak girin', + 'helper_text' => 'Ağırlığı :unit olarak girin', ], 'price' => [ 'label' => 'Fiyat', diff --git a/packages/table-rate-shipping/resources/lang/vi/relationmanagers.php b/packages/table-rate-shipping/resources/lang/vi/relationmanagers.php index 8fdb3b11de..9909f70367 100644 --- a/packages/table-rate-shipping/resources/lang/vi/relationmanagers.php +++ b/packages/table-rate-shipping/resources/lang/vi/relationmanagers.php @@ -39,7 +39,7 @@ ], 'min_weight' => [ 'label' => 'K.lượng T.thiểu', - 'helper_text' => 'Nhập trọng lượng theo kilôgam', + 'helper_text' => 'Nhập trọng lượng theo :unit', ], 'price' => [ 'label' => 'Giá', diff --git a/packages/table-rate-shipping/src/Drivers/ShippingMethods/ShipBy.php b/packages/table-rate-shipping/src/Drivers/ShippingMethods/ShipBy.php index 20b1ade7bf..545adbe292 100644 --- a/packages/table-rate-shipping/src/Drivers/ShippingMethods/ShipBy.php +++ b/packages/table-rate-shipping/src/Drivers/ShippingMethods/ShipBy.php @@ -72,8 +72,12 @@ public function resolve(ShippingOptionRequest $shippingOptionRequest): ?Shipping $tier = $subTotal; if ($chargeBy == 'weight') { + // Tiers are stored raw in the method's weight unit; each line's + // weight converts from its own purchasable unit. + $weightUnit = $shippingMethod->weight_unit ?: 'kg'; + $tier = $cart->lines->sum( - fn ($line) => $line->purchasable->weight->to('weight.kg')->convert()->getValue() * $line->quantity + fn ($line) => $line->purchasable->weight->to("weight.{$weightUnit}")->convert()->getValue() * $line->quantity ); } diff --git a/packages/table-rate-shipping/src/Filament/Resources/ShippingZoneResource/Pages/ManageShippingRates.php b/packages/table-rate-shipping/src/Filament/Resources/ShippingZoneResource/Pages/ManageShippingRates.php index 37ca4f26f7..8494d5db25 100644 --- a/packages/table-rate-shipping/src/Filament/Resources/ShippingZoneResource/Pages/ManageShippingRates.php +++ b/packages/table-rate-shipping/src/Filament/Resources/ShippingZoneResource/Pages/ManageShippingRates.php @@ -122,12 +122,20 @@ function () { : __('lunarpanel.shipping::relationmanagers.shipping_rates.form.prices.repeater.min_spend.label') ) ->helperText(fn (Get $get) => static::isWeightCharge($get) - ? __('lunarpanel.shipping::relationmanagers.shipping_rates.form.prices.repeater.min_weight.helper_text') + ? __('lunarpanel.shipping::relationmanagers.shipping_rates.form.prices.repeater.min_weight.helper_text', [ + 'unit' => static::getShippingWeightUnit($get('../../shipping_method_id')), + ]) : null ) // Unit symbol — intentionally not translated. - ->suffix(fn (Get $get) => static::isWeightCharge($get) ? 'kg' : null) + ->suffix(fn (Get $get) => static::isWeightCharge($get) + ? static::getShippingWeightUnit($get('../../shipping_method_id')) + : null + ) ->numeric() + // Weight tiers are stored as raw integers in the method's + // weight unit — reject decimals instead of truncating them. + ->rules(fn (Get $get) => static::isWeightCharge($get) ? ['integer'] : []) ->required(), ])->afterStateHydrated( static function (Repeater $component, ?Model $record = null): void { @@ -228,6 +236,19 @@ private static function isWeightCharge(Get $get): bool return static::getShippingChargeBy($get('../../shipping_method_id')) === 'weight'; } + private static function getShippingWeightUnit(ShippingMethod|int|null $method): string + { + if (blank($method)) { + return 'kg'; + } + + if (! $method instanceof ShippingMethod) { + $method = ShippingMethod::find($method); + } + + return $method?->weight_unit ?: 'kg'; + } + protected static function saveShippingRate(?ShippingRate $shippingRate = null, array $data = []): void { $shippingRate->basePrices()->delete(); diff --git a/tests/shipping/Unit/Drivers/ShippingMethods/ShipByTest.php b/tests/shipping/Unit/Drivers/ShippingMethods/ShipByTest.php index eee17d6524..5c2acf2066 100644 --- a/tests/shipping/Unit/Drivers/ShippingMethods/ShipByTest.php +++ b/tests/shipping/Unit/Drivers/ShippingMethods/ShipByTest.php @@ -577,3 +577,79 @@ expect($option)->toBeInstanceOf(ShippingOption::class) ->and($option->price->value)->toEqual(400); }); + +test('weight tiers are evaluated in the shipping method configured weight_unit', function () { + $currency = Currency::factory()->create(['default' => true]); + TaxClass::factory()->create(['default' => true]); + + $shippingZone = ShippingZone::factory()->create(['type' => 'countries']); + + // Grams give merchants sub-kilogram tier precision with raw integer storage. + $shippingMethod = ShippingMethod::factory()->create([ + 'driver' => 'ship-by', + 'data' => ['charge_by' => 'weight'], + 'weight_unit' => 'g', + ]); + + $shippingRate = ShippingRate::factory()->create([ + 'shipping_method_id' => $shippingMethod->id, + 'shipping_zone_id' => $shippingZone->id, + ]); + + $shippingRate->prices()->createMany([ + ['price' => 1000, 'min_quantity' => 1, 'currency_id' => $currency->id], + ['price' => 600, 'min_quantity' => 500, 'currency_id' => $currency->id], + ['price' => 200, 'min_quantity' => 1000, 'currency_id' => $currency->id], + ]); + + $makeCart = function (array $lines) use ($currency): Cart { + $cart = Cart::factory()->create(['currency_id' => $currency->id]); + + foreach ($lines as [$weightValue, $weightUnit]) { + $variant = ProductVariant::factory()->create([ + 'weight_value' => $weightValue, + 'weight_unit' => $weightUnit, + ]); + + Price::factory()->create([ + 'price' => 500, + 'min_quantity' => 1, + 'currency_id' => $currency->id, + 'priceable_type' => $variant->getMorphClass(), + 'priceable_id' => $variant->id, + ]); + + $cart->lines()->create([ + 'purchasable_type' => $variant->getMorphClass(), + 'purchasable_id' => $variant->id, + 'quantity' => 1, + ]); + } + + return $cart->calculate(); + }; + + // 300g cart sits below the 500g tier. + $option = (new ShipBy)->resolve(new ShippingOptionRequest( + shippingRate: $shippingRate, + cart: $makeCart([[0.3, 'kg']]), + )); + expect($option)->toBeInstanceOf(ShippingOption::class) + ->and($option->price->value)->toEqual(1000); + + // A 0.5kg product converts to 500g and matches the 500g tier exactly. + $option = (new ShipBy)->resolve(new ShippingOptionRequest( + shippingRate: $shippingRate, + cart: $makeCart([[0.5, 'kg']]), + )); + expect($option)->toBeInstanceOf(ShippingOption::class) + ->and($option->price->value)->toEqual(600); + + // Mixed units: 0.4kg + 600g = 1000g hits the top tier. + $option = (new ShipBy)->resolve(new ShippingOptionRequest( + shippingRate: $shippingRate, + cart: $makeCart([[0.4, 'kg'], [600.0, 'g']]), + )); + expect($option)->toBeInstanceOf(ShippingOption::class) + ->and($option->price->value)->toEqual(200); +}); From af4d7636e0284adf4e557e772fd1697aaf8a3de4 Mon Sep 17 00:00:00 2001 From: Glenn Jacobs Date: Wed, 26 Aug 2026 18:14:21 +0100 Subject: [PATCH 06/11] fix(core): deterministic relation ordering and one order address per type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/core/src/Models/Cart.php | 2 +- packages/core/src/Models/Order.php | 2 +- packages/core/src/Models/ProductVariant.php | 4 +- .../Order/Creation/CreateOrderAddresses.php | 3 +- tests/core/Unit/Models/CartTest.php | 30 +++++++++++++ tests/core/Unit/Models/OrderTest.php | 23 ++++++++++ tests/core/Unit/Models/ProductVariantTest.php | 21 +++++++++ .../Creation/CreateOrderAddressesTest.php | 44 +++++++++++++++++++ 8 files changed, 124 insertions(+), 5 deletions(-) diff --git a/packages/core/src/Models/Cart.php b/packages/core/src/Models/Cart.php index 45811f5b6b..c2fe81828a 100644 --- a/packages/core/src/Models/Cart.php +++ b/packages/core/src/Models/Cart.php @@ -218,7 +218,7 @@ protected static function newFactory() public function lines(): HasMany { - return $this->hasMany(CartLine::class, 'cart_id', 'id'); + return $this->hasMany(CartLine::class, 'cart_id', 'id')->orderBy('id'); } public function currency(): BelongsTo diff --git a/packages/core/src/Models/Order.php b/packages/core/src/Models/Order.php index a2eca591b9..023c8bf39f 100644 --- a/packages/core/src/Models/Order.php +++ b/packages/core/src/Models/Order.php @@ -208,7 +208,7 @@ public function cart(): BelongsTo public function lines(): HasMany { - return $this->hasMany(OrderLine::class); + return $this->hasMany(OrderLine::class)->orderBy('id'); } public function fulfilments(): HasMany diff --git a/packages/core/src/Models/ProductVariant.php b/packages/core/src/Models/ProductVariant.php index d4e53e6d02..1cb9692f14 100644 --- a/packages/core/src/Models/ProductVariant.php +++ b/packages/core/src/Models/ProductVariant.php @@ -144,7 +144,9 @@ public function values(): BelongsToMany "{$prefix}product_option_value_product_variant", 'variant_id', 'value_id' - )->withTimestamps(); + )->withTimestamps() + ->orderBy('position') + ->orderByPivot('id'); } public function getPrices(): Collection diff --git a/packages/core/src/Pipelines/Order/Creation/CreateOrderAddresses.php b/packages/core/src/Pipelines/Order/Creation/CreateOrderAddresses.php index c8745352ca..33a552ac26 100644 --- a/packages/core/src/Pipelines/Order/Creation/CreateOrderAddresses.php +++ b/packages/core/src/Pipelines/Order/Creation/CreateOrderAddresses.php @@ -20,8 +20,7 @@ public function handle(Order $order, Closure $next): mixed foreach ($order->cart->addresses as $address) { /** @var OrderAddress $addressModel */ $addressModel = $orderAddresses->first(function ($orderAddress) use ($address) { - return $orderAddress->type == $address->type && - $orderAddress->postcode == $address->postcode; + return $orderAddress->type == $address->type; }) ?: App::make(OrderAddress::class); $addressModel->fill( diff --git a/tests/core/Unit/Models/CartTest.php b/tests/core/Unit/Models/CartTest.php index e289d9b172..e1241720a6 100644 --- a/tests/core/Unit/Models/CartTest.php +++ b/tests/core/Unit/Models/CartTest.php @@ -1342,3 +1342,33 @@ ->and($context->customer->id)->toBe($customer->id) ->and($context->customerGroups->pluck('id')->all())->toBe([$trade->id]); }); + +test('orders cart lines by id', function () { + $cart = Cart::factory()->create(); + + // Cart::lines() must carry an explicit ordering contract. Without an + // ORDER BY, row order is undefined by the SQL standard: MySQL/InnoDB + // returns clustered primary-key order by coincidence, but PostgreSQL + // returns heap order, which changes after an UPDATE. Lunar relies on a + // stable line sequence (e.g. GenerateFingerprint reduces $cart->lines in + // iteration order), so it must be deterministic across engines. + expect($cart->lines()->toBase()->orders) + ->toBe([['column' => 'id', 'direction' => 'asc']]); +}); + +test('can retrieve cart lines in ascending id order', function () { + $currency = Currency::factory()->create(); + + $cart = Cart::factory()->create([ + 'currency_id' => $currency->id, + ]); + + $lines = CartLine::factory() + ->count(5) + ->create(['cart_id' => $cart->id]); + + $expectedOrder = $lines->pluck('id')->sort()->values()->all(); + + expect($cart->load('lines')->lines->pluck('id')->all()) + ->toBe($expectedOrder); +}); diff --git a/tests/core/Unit/Models/OrderTest.php b/tests/core/Unit/Models/OrderTest.php index b131fb12d6..abcdcd048f 100644 --- a/tests/core/Unit/Models/OrderTest.php +++ b/tests/core/Unit/Models/OrderTest.php @@ -422,3 +422,26 @@ public function orderContactType(): string expect($order->routeNotificationForMail(new SelfRoutingOrderNotification(null))) ->toBe('shipping@example.com'); }); + +test('orders order lines by id', function () { + $order = Order::factory()->create(); + + // Order::lines() must carry an explicit ordering contract so line order is + // deterministic across database engines. PostgreSQL does not return rows in + // insertion order without an ORDER BY, which shifts invoice/history display. + expect($order->lines()->toBase()->orders) + ->toBe([['column' => 'id', 'direction' => 'asc']]); +}); + +test('can retrieve order lines in ascending id order', function () { + $order = Order::factory()->create(); + + $lines = OrderLine::factory() + ->count(5) + ->create(['order_id' => $order->id]); + + $expectedOrder = $lines->pluck('id')->sort()->values()->all(); + + expect($order->load('lines')->lines->pluck('id')->all()) + ->toBe($expectedOrder); +}); diff --git a/tests/core/Unit/Models/ProductVariantTest.php b/tests/core/Unit/Models/ProductVariantTest.php index 8b565f46a6..a6794d2292 100644 --- a/tests/core/Unit/Models/ProductVariantTest.php +++ b/tests/core/Unit/Models/ProductVariantTest.php @@ -8,6 +8,8 @@ use Lunar\Core\Models\CustomerGroup; use Lunar\Core\Models\Price; use Lunar\Core\Models\Product; +use Lunar\Core\Models\ProductOption; +use Lunar\Core\Models\ProductOptionValue; use Lunar\Core\Models\ProductVariant; use Lunar\Core\Models\TaxClass; use Lunar\Core\Models\TaxRate; @@ -285,3 +287,22 @@ expect($product->variants()->enabled()->pluck('id')->all())->toBe([$enabled->id]); }); + +test('returns variant option values ordered by position', function () { + $variant = ProductVariant::factory()->create(); + $option = ProductOption::factory()->create(); + + // Attached deliberately out of position order. Without an explicit ORDER BY + // the pivot rows would come back in attachment order ([3, 1, 2]); the + // relationship must order by the option value's position so getOption() + // (snapshotted onto order lines) is deterministic across database engines. + $values = collect([3, 1, 2])->map(fn ($position) => ProductOptionValue::factory()->create([ + 'product_option_id' => $option->id, + 'position' => $position, + ])); + + $variant->values()->attach($values->pluck('id')->all()); + + expect($variant->load('values')->values->pluck('position')->all()) + ->toBe([1, 2, 3]); +}); diff --git a/tests/core/Unit/Pipelines/Order/Creation/CreateOrderAddressesTest.php b/tests/core/Unit/Pipelines/Order/Creation/CreateOrderAddressesTest.php index 9555944a60..3a55b12760 100644 --- a/tests/core/Unit/Pipelines/Order/Creation/CreateOrderAddressesTest.php +++ b/tests/core/Unit/Pipelines/Order/Creation/CreateOrderAddressesTest.php @@ -82,3 +82,47 @@ expect($order->addresses)->toHaveCount($cart->addresses->count()); }); + +test('does not duplicate addresses when the cart address postcode changes', function () { + $currency = Currency::factory()->create(); + + $cart = Cart::factory()->create([ + 'currency_id' => $currency->id, + ]); + + CartAddress::factory()->create([ + 'type' => 'billing', + 'cart_id' => $cart->id, + 'postcode' => 'N1 1TW', + ]); + + CartAddress::factory()->create([ + 'type' => 'shipping', + 'cart_id' => $cart->id, + 'postcode' => 'N2 2TW', + ]); + + $order = Order::factory()->create([ + 'cart_id' => $cart->id, + ]); + + $billing = OrderAddress::factory()->create([ + 'type' => 'billing', + 'order_id' => $order->id, + 'postcode' => 'SW1 1AA', + ]); + + $shipping = OrderAddress::factory()->create([ + 'type' => 'shipping', + 'order_id' => $order->id, + 'postcode' => 'SW2 2AA', + ]); + + app(CreateOrderAddresses::class)->handle($order, fn ($order) => $order); + + expect($order->refresh()->addresses)->toHaveCount(2) + ->and($order->billingAddress->id)->toBe($billing->id) + ->and($order->billingAddress->postcode)->toBe('N1 1TW') + ->and($order->shippingAddress->id)->toBe($shipping->id) + ->and($order->shippingAddress->postcode)->toBe('N2 2TW'); +}); From ab83924fe9555aace29f72480682af78a16c4dea Mon Sep 17 00:00:00 2001 From: Glenn Jacobs Date: Wed, 26 Aug 2026 18:18:54 +0100 Subject: [PATCH 07/11] fix(admin,filament): trim input, variant mapping, media properties, zone selects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/admin/src/LunarPanelManager.php | 8 ++ .../RelationManagers/MediaRelationManager.php | 18 ++- .../Products/MapVariantsToProductOptions.php | 14 ++- .../src/Schemas/TaxZone/TaxZoneForm.php | 17 ++- .../Widgets/Products/ProductOptionsWidget.php | 7 ++ .../Schemas/ShippingZoneForm.php | 20 ++- .../MediaRelationManagerTest.php | 85 +++++++++++++ .../MapVariantsToProductOptionsTest.php | 115 ++++++++++++++++++ .../filament/Unit/Schemas/TaxZoneFormTest.php | 67 ++++++++++ 9 files changed, 321 insertions(+), 30 deletions(-) create mode 100644 tests/filament/Unit/Schemas/TaxZoneFormTest.php diff --git a/packages/admin/src/LunarPanelManager.php b/packages/admin/src/LunarPanelManager.php index 89cdb843e4..60480e49f0 100644 --- a/packages/admin/src/LunarPanelManager.php +++ b/packages/admin/src/LunarPanelManager.php @@ -5,6 +5,9 @@ use Closure; use Filament\Auth\MultiFactor\App\AppAuthentication; use Filament\Facades\Filament; +use Filament\Forms\Components\TagsInput; +use Filament\Forms\Components\Textarea; +use Filament\Forms\Components\TextInput; use Filament\Http\Middleware\Authenticate; use Filament\Http\Middleware\DisableBladeIconComponents; use Filament\Http\Middleware\DispatchServingFilamentEvent; @@ -201,6 +204,11 @@ public function register(): self Grid::configureUsing(fn (Grid $grid) => $grid->columnSpanFull()); Fieldset::configureUsing(fn (Fieldset $fieldset) => $fieldset->columnSpanFull()); + // Livewire skips the TrimStrings middleware, so trim at the form layer instead. + TextInput::configureUsing(fn (TextInput $input) => $input->trim()); + Textarea::configureUsing(fn (Textarea $textarea) => $textarea->trim()); + TagsInput::configureUsing(fn (TagsInput $tagsInput) => $tagsInput->trim()); + return $this; } diff --git a/packages/admin/src/Support/RelationManagers/MediaRelationManager.php b/packages/admin/src/Support/RelationManagers/MediaRelationManager.php index 917064df0e..a4afac8a14 100644 --- a/packages/admin/src/Support/RelationManagers/MediaRelationManager.php +++ b/packages/admin/src/Support/RelationManagers/MediaRelationManager.php @@ -98,16 +98,24 @@ public function getDefaultTable(Table $table): Table ->usingFileName( $data['media']->getClientOriginalName() ) - ->withCustomProperties([ - 'name' => $data['custom_properties']['name'], - 'primary' => $data['custom_properties']['primary'], - ]) + ->withCustomProperties($data['custom_properties'] ?? []) ->preservingOriginal() ->toMediaCollection($this->mediaCollection); }), ]) ->recordActions([ - EditAction::make(), + EditAction::make() + ->mutateDataUsing(function (array $data, Media $record): array { + // The column is array-cast, so writing the submitted set + // wholesale drops any key this form did not render — an + // extension's own properties, for instance. + $data['custom_properties'] = array_merge( + $record->custom_properties ?? [], + $data['custom_properties'] ?? [], + ); + + return $data; + }), DeleteAction::make(), Action::make('view_open') ->label(__('lunarpanel::relationmanagers.medias.actions.view.label')) diff --git a/packages/core/src/Actions/Products/MapVariantsToProductOptions.php b/packages/core/src/Actions/Products/MapVariantsToProductOptions.php index 085ade9cdf..ba88ba6de3 100644 --- a/packages/core/src/Actions/Products/MapVariantsToProductOptions.php +++ b/packages/core/src/Actions/Products/MapVariantsToProductOptions.php @@ -45,7 +45,7 @@ public function execute(array $options, array $variants, bool $fillMissing = tru $valueDifference = array_diff_assoc($permutation, $variant['values']); if (! count($valueDifference)) { - return $variant; + return true; } $amountMatched = count($permutation) - count($valueDifference); @@ -53,13 +53,23 @@ public function execute(array $options, array $variants, bool $fillMissing = tru return $amountMatched === count($variant['values']); }); - $variant = $variants[$variantIndex] ?? null; + // search() returns false when nothing matches; PHP coerces false to index 0. + $variant = $variantIndex === false ? null : ($variants[$variantIndex] ?? null); $variantId = $variant['id'] ?? null; $sku = $variant['sku'] ?? null; $copiedFrom = null; $shouldFill = true; + if (! $variant && ! $fillMissing) { + $shouldFill = false; + } + + // New option combinations still need tax_class_id on insert; copy the oldest sibling. + if (! $variant && $fillMissing) { + $copiedFrom = collect($variants)->min('id'); + } + if ($variant) { $existing = collect($variantPermutations) ->first(fn ($p) => $p['variant_id'] === $variant['id']); diff --git a/packages/filament/src/Schemas/TaxZone/TaxZoneForm.php b/packages/filament/src/Schemas/TaxZone/TaxZoneForm.php index 3f1fc4f8b0..a7dd3880a4 100644 --- a/packages/filament/src/Schemas/TaxZone/TaxZoneForm.php +++ b/packages/filament/src/Schemas/TaxZone/TaxZoneForm.php @@ -90,11 +90,9 @@ public static function getZoneTypeCountriesComponent(): Component ->map(static fn ($key): string => strval($key)) ->toArray(), ); - })->getOptionLabelsUsing(static function (Model $record): array { - $record->loadMissing('countries.country'); - - return $record->countries - ->pluck('country.name', 'country.iso3') + })->getOptionLabelsUsing(static function (array $values): array { + return Country::whereIn('iso3', $values) + ->pluck('name', 'iso3') ->toArray(); }) ->saveRelationshipsUsing(static function (Model $record, $state) { @@ -151,11 +149,10 @@ public static function getZoneTypeStatesComponent(): Component ->map(static fn ($key): string => strval($key)) ->toArray(), ); - })->getOptionLabelsUsing(static function (Model $record): array { - $record->loadMissing('states.state'); - - return $record->states - ->pluck('state.name', 'state.code') + })->getOptionLabelsUsing(static function (array $values, $get): array { + return State::where('country_id', $get('zone_country')) + ->whereIn('code', $values) + ->pluck('name', 'code') ->toArray(); }) ->saveRelationshipsUsing(static function (Model $record, $state, $get) { diff --git a/packages/filament/src/Widgets/Products/ProductOptionsWidget.php b/packages/filament/src/Widgets/Products/ProductOptionsWidget.php index 01a18c2d13..37ea17329a 100644 --- a/packages/filament/src/Widgets/Products/ProductOptionsWidget.php +++ b/packages/filament/src/Widgets/Products/ProductOptionsWidget.php @@ -414,6 +414,13 @@ protected function saveVariants(): void $basePrice = $variant->basePrices->first(); } + // A permutation the mapper could not match has no variant to update and + // nothing to copy tax_class_id or a base price from, so fall back to + // the oldest sibling. + if (empty($variantData['variant_id']) && empty($variantData['copied_id'])) { + $variantData['copied_id'] = $this->record->variants()->orderBy('id')->value('id'); + } + if (! empty($variantData['copied_id'])) { $copiedVariant = ProductVariant::find( $variantData['copied_id'] diff --git a/packages/table-rate-shipping/src/Filament/Resources/ShippingZoneResource/Schemas/ShippingZoneForm.php b/packages/table-rate-shipping/src/Filament/Resources/ShippingZoneResource/Schemas/ShippingZoneForm.php index 85a40b056c..41755e9cdc 100644 --- a/packages/table-rate-shipping/src/Filament/Resources/ShippingZoneResource/Schemas/ShippingZoneForm.php +++ b/packages/table-rate-shipping/src/Filament/Resources/ShippingZoneResource/Schemas/ShippingZoneForm.php @@ -83,11 +83,9 @@ public static function getCountryComponent(): Component $country = $record->countries->first(); $component->state($country?->id); - })->getOptionLabelsUsing(static function (Model $record): array { - $record->loadMissing('countries.country'); - - return $record->countries - ->pluck('country.name', 'country.id') + })->getOptionLabelsUsing(static function (array $values): array { + return Country::whereIn('id', $values) + ->pluck('name', 'id') ->toArray(); }) ->saveRelationshipsUsing(static function (Model $record, $state) { @@ -116,10 +114,8 @@ public static function getCountriesComponent(): Component ->map(static fn ($key): string => strval($key)) ->toArray(), ); - })->getOptionLabelsUsing(static function (Model $record): array { - $record->loadMissing('countries'); - - return $record->countries + })->getOptionLabelsUsing(static function (array $values): array { + return Country::whereIn('id', $values) ->pluck('name', 'id') ->toArray(); }) @@ -148,10 +144,8 @@ public static function getStatesComponent(): Component ->map(static fn ($key): string => strval($key)) ->toArray(), ); - })->getOptionLabelsUsing(static function (Model $record): array { - $record->loadMissing('states'); - - return $record->states + })->getOptionLabelsUsing(static function (array $values): array { + return State::whereIn('id', $values) ->pluck('name', 'id') ->toArray(); }) diff --git a/tests/admin/Feature/Support/RelationManagers/MediaRelationManagerTest.php b/tests/admin/Feature/Support/RelationManagers/MediaRelationManagerTest.php index 20a29f18f0..89294e0f1e 100644 --- a/tests/admin/Feature/Support/RelationManagers/MediaRelationManagerTest.php +++ b/tests/admin/Feature/Support/RelationManagers/MediaRelationManagerTest.php @@ -1,8 +1,15 @@ components([ + ...$schema->getComponents(true), + TextInput::make('custom_properties.credits'), + ]); + + return $schema; + } + }; + + LunarPanel::extensions([ + MediaRelationManager::class => $class::class, + ]); + + $this->asStaff(); + + Language::factory()->create([ + 'default' => true, + ]); + + $brand = Brand::factory()->create(); + + Livewire::test(MediaRelationManager::class, [ + 'ownerRecord' => $brand, + 'pageClass' => ManageBrandMedia::class, + ])->callTableAction(CreateAction::class, data: [ + 'custom_properties.name' => 'Test image', + 'custom_properties.credits' => 'Jane Doe', + 'media' => UploadedFile::fake()->image('foobar.jpg'), + ])->assertHasNoTableActionErrors(); + + $media = $brand->fresh()->getFirstMedia('default'); + + expect($media)->not->toBeNull() + ->and($media->getCustomProperty('name'))->toBe('Test image') + ->and($media->getCustomProperty('credits'))->toBe('Jane Doe'); +}); + +it('preserves existing custom properties not present on the edit form', function () { + $this->asStaff(); + + Language::factory()->create([ + 'default' => true, + ]); + + $brand = Brand::factory()->create(); + + $media = $brand + ->addMedia(UploadedFile::fake()->image('foobar.jpg')) + ->preservingOriginal() + ->withCustomProperties([ + 'name' => 'Original name', + 'sha1' => 'abc123', + ]) + ->toMediaCollection('default'); + + Livewire::test(MediaRelationManager::class, [ + 'ownerRecord' => $brand, + 'pageClass' => ManageBrandMedia::class, + ])->mountTableAction(EditAction::class, $media) + ->setTableActionData([ + 'custom_properties' => [ + 'name' => 'Updated name', + 'primary' => false, + ], + ])->callMountedTableAction() + ->assertHasNoTableActionErrors(); + + $media->refresh(); + + expect($media->getCustomProperty('name'))->toBe('Updated name') + ->and($media->getCustomProperty('sha1'))->toBe('abc123'); +}); diff --git a/tests/admin/Unit/Actions/Products/MapVariantsToProductOptionsTest.php b/tests/admin/Unit/Actions/Products/MapVariantsToProductOptionsTest.php index df14d0a29d..e0f4004972 100644 --- a/tests/admin/Unit/Actions/Products/MapVariantsToProductOptionsTest.php +++ b/tests/admin/Unit/Actions/Products/MapVariantsToProductOptionsTest.php @@ -78,3 +78,118 @@ expect($result)->toHaveCount(4); }); + +it('does not assign the first variant sku to unmatched sparse permutations', function () { + $optionValues = [ + 'Size' => [ + 'XS', + 'L', + ], + 'Colour' => [ + 'Navy', + 'White', + ], + 'Length' => [ + 'Long', + 'Short', + ], + 'Fit' => [ + 'Slim', + ], + ]; + + $variants = [ + [ + 'id' => 1, + 'sku' => 'TEE-L-NAVY-SHORT', + 'price' => 0, + 'stock' => 0, + 'values' => [ + 'Size' => 'L', + 'Colour' => 'Navy', + 'Length' => 'Short', + 'Fit' => 'Slim', + ], + ], + [ + 'id' => 2, + 'sku' => 'TEE-XS-WHITE-LONG', + 'price' => 0, + 'stock' => 0, + 'values' => [ + 'Size' => 'XS', + 'Colour' => 'White', + 'Length' => 'Long', + 'Fit' => 'Slim', + ], + ], + ]; + + $result = MapVariantsToProductOptions::map($optionValues, $variants, fillMissing: false); + + expect($result)->toHaveCount(2); + + $mappedBySku = collect($result)->keyBy('sku'); + + expect($mappedBySku['TEE-L-NAVY-SHORT']['values'])->toBe([ + 'Size' => 'L', + 'Colour' => 'Navy', + 'Length' => 'Short', + 'Fit' => 'Slim', + ])->and($mappedBySku['TEE-XS-WHITE-LONG']['values'])->toBe([ + 'Size' => 'XS', + 'Colour' => 'White', + 'Length' => 'Long', + 'Fit' => 'Slim', + ]); + + // First cartesian permutation is XS/Navy/Long — must not steal the first variant SKU. + expect( + collect($result)->contains(fn (array $row) => $row['sku'] === 'TEE-L-NAVY-SHORT' + && $row['values']['Size'] === 'XS') + )->toBeFalse(); +}); + +it('keeps unmatched permutations without binding the first variant when fillMissing is enabled', function () { + $optionValues = [ + 'Size' => [ + 'Small', + 'Large', + ], + 'Colour' => [ + 'Red', + 'Blue', + ], + ]; + + $variants = [ + [ + 'id' => 10, + 'sku' => 'SMALL-RED', + 'price' => 1, + 'stock' => 5, + 'values' => [ + 'Size' => 'Small', + 'Colour' => 'Red', + ], + ], + ]; + + $result = MapVariantsToProductOptions::map($optionValues, $variants, fillMissing: true); + + expect($result)->toHaveCount(4); + + $exact = collect($result)->first( + fn (array $row) => $row['values'] === ['Size' => 'Small', 'Colour' => 'Red'] + ); + + expect($exact['sku'])->toBe('SMALL-RED') + ->and($exact['variant_id'])->toBe(10) + ->and($exact['copied_id'])->toBeNull(); + + $unmatched = collect($result) + ->reject(fn (array $row) => $row['variant_id'] === 10); + + expect($unmatched->pluck('sku')->all())->each->toBeNull(); + expect($unmatched->pluck('copied_id')->unique()->all())->toBe([10]); +}); diff --git a/tests/filament/Unit/Schemas/TaxZoneFormTest.php b/tests/filament/Unit/Schemas/TaxZoneFormTest.php new file mode 100644 index 0000000000..229d7739a2 --- /dev/null +++ b/tests/filament/Unit/Schemas/TaxZoneFormTest.php @@ -0,0 +1,67 @@ +statePath('data') + ->components([$component]); + + $schema->fill([...$siblingState, $component->getName() => $state]); + + return $schema->getComponents(withHidden: true)[0]; +} + +it('resolves country labels from the selected values, not the saved relationship', function () { + $selected = Country::factory()->count(2)->create(); + Country::factory()->create(); + + // Nothing is attached to a tax zone — these are fresh selections. + $component = mountedSelect( + TaxZoneForm::getZoneTypeCountriesComponent(), + $selected->pluck('iso3')->all(), + ); + + expect($component->getOptionLabels()) + ->toEqual($selected->pluck('name', 'iso3')->all()); +}); + +it('resolves state labels scoped to the chosen country', function () { + $country = Country::factory()->create(); + $other = Country::factory()->create(); + + $selected = State::factory()->count(2)->create(['country_id' => $country->id]); + $elsewhere = State::factory()->create(['country_id' => $other->id]); + + $component = mountedSelect( + TaxZoneForm::getZoneTypeStatesComponent(), + $selected->pluck('code')->push($elsewhere->code)->all(), + ['zone_country' => $country->id], + ); + + // Scoped to zone_country, so a state of the same code in another country is + // not labelled. + expect($component->getOptionLabels()) + ->toEqual($selected->pluck('name', 'code')->all()); +}); From 0d4e8c3b88117847362dab1c727f42d0ad687aa4 Mon Sep 17 00:00:00 2001 From: Glenn Jacobs Date: Wed, 26 Aug 2026 18:24:32 +0100 Subject: [PATCH 08/11] fix(admin,filament): meta booleans, unit quantity, widget labels, invoice lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/admin/resources/lang/ar/global.php | 4 + packages/admin/resources/lang/bg/global.php | 4 + packages/admin/resources/lang/de/global.php | 4 + packages/admin/resources/lang/en/global.php | 3 + packages/admin/resources/lang/es/global.php | 4 + packages/admin/resources/lang/fa/global.php | 4 + packages/admin/resources/lang/fr/global.php | 4 + packages/admin/resources/lang/hr/global.php | 4 + packages/admin/resources/lang/hu/global.php | 4 + packages/admin/resources/lang/mn/global.php | 4 + packages/admin/resources/lang/nl/global.php | 4 + packages/admin/resources/lang/pl/global.php | 4 + .../admin/resources/lang/pt_BR/global.php | 4 + packages/admin/resources/lang/ro/global.php | 4 + packages/admin/resources/lang/tr/global.php | 4 + packages/admin/resources/lang/vi/global.php | 4 + .../admin/resources/views/pdf/order.blade.php | 2 +- .../widgets/product-options.blade.php | 247 ------------------ .../OrderResource/Pages/ManageOrder.php | 4 +- .../src/Pricing/DefaultPriceFormatter.php | 6 + .../resources/lang/ar/productoption.php | 3 + .../resources/lang/bg/productoption.php | 3 + .../resources/lang/de/productoption.php | 3 + .../resources/lang/en/productoption.php | 3 + .../resources/lang/es/productoption.php | 3 + .../resources/lang/fa/productoption.php | 3 + .../resources/lang/fr/productoption.php | 3 + .../resources/lang/hr/productoption.php | 3 + .../resources/lang/hu/productoption.php | 3 + .../resources/lang/mn/productoption.php | 3 + .../resources/lang/nl/productoption.php | 3 + .../resources/lang/pl/productoption.php | 3 + .../resources/lang/pt_BR/productoption.php | 3 + .../resources/lang/ro/productoption.php | 3 + .../resources/lang/tr/productoption.php | 3 + .../resources/lang/vi/productoption.php | 3 + .../products/product-options.blade.php | 2 +- .../src/Forms/Components/CollectionSelect.php | 15 +- .../Concerns/AppliesToExistingSelect.php | 16 +- .../CollectionConditionRelationManager.php | 9 + .../CollectionLimitationRelationManager.php | 7 +- .../ProductVariant/ProductVariantForm.php | 3 +- .../Widgets/Products/ProductOptionsWidget.php | 6 + .../OrderResource/Pages/ManageOrderTest.php | 34 +++ tests/core/Unit/Models/OrderTest.php | 46 ++++ .../Pricing/DefaultPriceFormatterTest.php | 36 +++ .../ProductVariant/ProductVariantFormTest.php | 12 + .../ProductOptionsWidgetLabelsTest.php | 20 ++ 48 files changed, 318 insertions(+), 258 deletions(-) delete mode 100644 packages/admin/resources/views/resources/product-resource/widgets/product-options.blade.php create mode 100644 tests/core/Unit/Pricing/DefaultPriceFormatterTest.php create mode 100644 tests/filament/Unit/Schemas/ProductVariant/ProductVariantFormTest.php create mode 100644 tests/filament/Unit/Widgets/ProductOptionsWidgetLabelsTest.php diff --git a/packages/admin/resources/lang/ar/global.php b/packages/admin/resources/lang/ar/global.php index a0febf3705..84b292a1a2 100644 --- a/packages/admin/resources/lang/ar/global.php +++ b/packages/admin/resources/lang/ar/global.php @@ -7,4 +7,8 @@ 'reports' => 'التقارير', 'settings' => 'الإعدادات', ], + + 'yes' => 'نعم', + 'no' => 'لا', + ]; diff --git a/packages/admin/resources/lang/bg/global.php b/packages/admin/resources/lang/bg/global.php index 241a10bfb5..1efad9b09a 100644 --- a/packages/admin/resources/lang/bg/global.php +++ b/packages/admin/resources/lang/bg/global.php @@ -7,4 +7,8 @@ 'reports' => 'Отчети', 'settings' => 'Настройки', ], + + 'yes' => 'Да', + 'no' => 'Не', + ]; diff --git a/packages/admin/resources/lang/de/global.php b/packages/admin/resources/lang/de/global.php index b3507c6537..885ac42e35 100644 --- a/packages/admin/resources/lang/de/global.php +++ b/packages/admin/resources/lang/de/global.php @@ -7,4 +7,8 @@ 'reports' => 'Berichte', 'settings' => 'Einstellungen', ], + + 'yes' => 'Ja', + 'no' => 'Nein', + ]; diff --git a/packages/admin/resources/lang/en/global.php b/packages/admin/resources/lang/en/global.php index 3b06a0c094..db5a41b34a 100644 --- a/packages/admin/resources/lang/en/global.php +++ b/packages/admin/resources/lang/en/global.php @@ -9,4 +9,7 @@ 'settings' => 'Settings', ], + 'yes' => 'Yes', + 'no' => 'No', + ]; diff --git a/packages/admin/resources/lang/es/global.php b/packages/admin/resources/lang/es/global.php index 4fa2a64d35..3530affbe5 100644 --- a/packages/admin/resources/lang/es/global.php +++ b/packages/admin/resources/lang/es/global.php @@ -7,4 +7,8 @@ 'reports' => 'Informes', 'settings' => 'Configuraciones', ], + + 'yes' => 'Sí', + 'no' => 'No', + ]; diff --git a/packages/admin/resources/lang/fa/global.php b/packages/admin/resources/lang/fa/global.php index 86c67d065e..1942f79322 100644 --- a/packages/admin/resources/lang/fa/global.php +++ b/packages/admin/resources/lang/fa/global.php @@ -7,4 +7,8 @@ 'reports' => 'Reports', 'settings' => 'Settings', ], + + 'yes' => 'بله', + 'no' => 'خیر', + ]; diff --git a/packages/admin/resources/lang/fr/global.php b/packages/admin/resources/lang/fr/global.php index ba730a9966..b5b9ff406f 100644 --- a/packages/admin/resources/lang/fr/global.php +++ b/packages/admin/resources/lang/fr/global.php @@ -7,4 +7,8 @@ 'reports' => 'Rapports', 'settings' => 'Paramètres', ], + + 'yes' => 'Oui', + 'no' => 'Non', + ]; diff --git a/packages/admin/resources/lang/hr/global.php b/packages/admin/resources/lang/hr/global.php index 616ccdebb2..1746e7b8f2 100644 --- a/packages/admin/resources/lang/hr/global.php +++ b/packages/admin/resources/lang/hr/global.php @@ -7,4 +7,8 @@ 'reports' => 'Izvještaji', 'settings' => 'Postavke', ], + + 'yes' => 'Da', + 'no' => 'Ne', + ]; diff --git a/packages/admin/resources/lang/hu/global.php b/packages/admin/resources/lang/hu/global.php index e4864827a9..bd399d144d 100644 --- a/packages/admin/resources/lang/hu/global.php +++ b/packages/admin/resources/lang/hu/global.php @@ -7,4 +7,8 @@ 'reports' => 'Jelentések', 'settings' => 'Beállítások', ], + + 'yes' => 'Igen', + 'no' => 'Nem', + ]; diff --git a/packages/admin/resources/lang/mn/global.php b/packages/admin/resources/lang/mn/global.php index b8643ea28b..892879f69f 100644 --- a/packages/admin/resources/lang/mn/global.php +++ b/packages/admin/resources/lang/mn/global.php @@ -7,4 +7,8 @@ 'reports' => 'Тайлан', 'settings' => 'Тохиргоо', ], + + 'yes' => 'Тийм', + 'no' => 'Үгүй', + ]; diff --git a/packages/admin/resources/lang/nl/global.php b/packages/admin/resources/lang/nl/global.php index 9a32bfda0e..70a63213c3 100644 --- a/packages/admin/resources/lang/nl/global.php +++ b/packages/admin/resources/lang/nl/global.php @@ -7,4 +7,8 @@ 'reports' => 'Rapporten', 'settings' => 'Instellingen', ], + + 'yes' => 'Ja', + 'no' => 'Nee', + ]; diff --git a/packages/admin/resources/lang/pl/global.php b/packages/admin/resources/lang/pl/global.php index ea9a649922..822d2d8e5f 100644 --- a/packages/admin/resources/lang/pl/global.php +++ b/packages/admin/resources/lang/pl/global.php @@ -7,4 +7,8 @@ 'reports' => 'Raporty', 'settings' => 'Ustawienia', ], + + 'yes' => 'Tak', + 'no' => 'Nie', + ]; diff --git a/packages/admin/resources/lang/pt_BR/global.php b/packages/admin/resources/lang/pt_BR/global.php index 83357873c1..1231bb7e2c 100644 --- a/packages/admin/resources/lang/pt_BR/global.php +++ b/packages/admin/resources/lang/pt_BR/global.php @@ -7,4 +7,8 @@ 'reports' => 'Relatórios', 'settings' => 'Configurações', ], + + 'yes' => 'Sim', + 'no' => 'Não', + ]; diff --git a/packages/admin/resources/lang/ro/global.php b/packages/admin/resources/lang/ro/global.php index 348827570a..7b156a0974 100644 --- a/packages/admin/resources/lang/ro/global.php +++ b/packages/admin/resources/lang/ro/global.php @@ -7,4 +7,8 @@ 'reports' => 'Rapoarte', 'settings' => 'Setări', ], + + 'yes' => 'Da', + 'no' => 'Nu', + ]; diff --git a/packages/admin/resources/lang/tr/global.php b/packages/admin/resources/lang/tr/global.php index 5f5af46c49..bf66b79430 100644 --- a/packages/admin/resources/lang/tr/global.php +++ b/packages/admin/resources/lang/tr/global.php @@ -7,4 +7,8 @@ 'reports' => 'Raporlar', 'settings' => 'Ayarlar', ], + + 'yes' => 'Evet', + 'no' => 'Hayır', + ]; diff --git a/packages/admin/resources/lang/vi/global.php b/packages/admin/resources/lang/vi/global.php index 73a76cc600..03c7441c4e 100644 --- a/packages/admin/resources/lang/vi/global.php +++ b/packages/admin/resources/lang/vi/global.php @@ -7,4 +7,8 @@ 'reports' => 'Báo cáo', 'settings' => 'Cài đặt', ], + + 'yes' => 'Có', + 'no' => 'Không', + ]; diff --git a/packages/admin/resources/views/pdf/order.blade.php b/packages/admin/resources/views/pdf/order.blade.php index 7cda19b783..68f2327c91 100644 --- a/packages/admin/resources/views/pdf/order.blade.php +++ b/packages/admin/resources/views/pdf/order.blade.php @@ -176,7 +176,7 @@ - @foreach($record->fulfillableLines as $line) + @foreach($record->productLines as $line) {{ $line->description }}
diff --git a/packages/admin/resources/views/resources/product-resource/widgets/product-options.blade.php b/packages/admin/resources/views/resources/product-resource/widgets/product-options.blade.php deleted file mode 100644 index 09855ca916..0000000000 --- a/packages/admin/resources/views/resources/product-resource/widgets/product-options.blade.php +++ /dev/null @@ -1,247 +0,0 @@ - - - @if(!$this->configuringOptions) -
-
-
-
-

- {{ __('lunarpanel::productoption.widgets.product-options.options-table.title') }} -

-
-
- - {{ __('lunarpanel::productoption.widgets.product-options.options-table.configure-options.label') }} - -
-
-
- @if(count($this->configuredOptions)) - - - - - - - - - @foreach($this->configuredOptions as $option) - - - - - @endforeach - -
- - {{ __('lunarpanel::productoption.widgets.product-options.options-table.table.option.label') }} - - - - {{ __('lunarpanel::productoption.widgets.product-options.options-table.table.values.label') }} - -
-
- - {{ $option['value'] }} - -
- 		-
- - {{ collect($option['option_values']) - ->filter( - fn ($value) => $value['enabled'] - )->map( - fn ($value) => $value['value'] - )->join(', ') }} - -
-
- @else -
-
-
- {{ \Filament\Support\generate_icon_html('lucide-shapes', size: \Filament\Support\Enums\IconSize::Large) }} -
-

No Product Options Configured

-
-
- @endif -
-
- -
-
-
-

- {{ __('lunarpanel::productoption.widgets.product-options.variants-table.title') }} -

-
-
-
- @if(count($this->variants)) - - - - @if($this->hasNewVariants) - - @endif - - - - - - - - - - @foreach($this->variants as $permutationIndex => $permutation) - - @if($this->hasNewVariants) - - @endif - - - - - - - - @endforeach - -
- - - {{ __('lunarpanel::productoption.widgets.product-options.variants-table.table.option.label') }} - - - - {{ __('lunarpanel::productoption.widgets.product-options.variants-table.table.sku.label') }} - - - - {{ __('lunarpanel::productoption.widgets.product-options.variants-table.table.price.label') }} - - - - {{ __('lunarpanel::productoption.widgets.product-options.variants-table.table.stock.label') }} - - -
-
- @if(!$permutation['variant_id']) - - {{ __('lunarpanel::productoption.widgets.product-options.variants-table.table.new.label') }} - - @endif -
- 		-
- - @foreach($permutation['values'] as $option => $value) - {{ $option }}: {{ $value }} - @endforeach - -
- 		-
- - - -
- 		-
- - - -
- 		-
- - - -
- 		-
- @if($permutation['variant_id']) - - {{ __('lunarpanel::productoption.widgets.product-options.variants-table.actions.edit.label') }} - - @endif - -
-
- @else -
-
-
- {{ \Filament\Support\generate_icon_html('lucide-shapes', size: \Filament\Support\Enums\IconSize::Large) }} -
-

{{ __('lunarpanel::productoption.widgets.product-options.variants-table.empty.heading') }}

-
-
- @endif -
-
-
- -
- {{ $this->saveVariantsAction }} -
- - @else -
-
-
- - {{ __('lunarpanel::productoption.widgets.product-options.actions.add-restricted-option.label') }} - - {{ $this->addSharedOptionAction }} -
-
- @if(!count($this->configuredOptions)) -
-
-
-
- {{ \Filament\Support\generate_icon_html('lucide-shapes', size: \Filament\Support\Enums\IconSize::Large) }} -
-

{{ __('lunarpanel::productoption.widgets.product-options.options-list.empty.heading') }}

-

{{ __('lunarpanel::productoption.widgets.product-options.options-list.empty.description') }}

-
-
-
- @else -
- -
- @endif - -
- - {{ __('lunarpanel::productoption.widgets.product-options.actions.save-options.label') }} - - - {{ __('lunarpanel::productoption.widgets.product-options.actions.cancel.label') }} - -
-
- - @endif - diff --git a/packages/admin/src/Filament/Resources/OrderResource/Pages/ManageOrder.php b/packages/admin/src/Filament/Resources/OrderResource/Pages/ManageOrder.php index 5e6f4d9e05..2a05c5f09b 100644 --- a/packages/admin/src/Filament/Resources/OrderResource/Pages/ManageOrder.php +++ b/packages/admin/src/Filament/Resources/OrderResource/Pages/ManageOrder.php @@ -201,7 +201,9 @@ public static function getDefaultAdditionalInfoSection(): Section } return TextEntry::make('meta_'.$key) - ->getStateUsing(fn () => $value) + ->getStateUsing(fn () => is_bool($value) + ? __($value ? 'lunarpanel::global.yes' : 'lunarpanel::global.no') + : $value) ->label($key) ->copyable() ->limit(50)->tooltip(function (TextEntry $component): ?string { diff --git a/packages/core/src/Pricing/DefaultPriceFormatter.php b/packages/core/src/Pricing/DefaultPriceFormatter.php index 277598fb6c..8088bc6490 100644 --- a/packages/core/src/Pricing/DefaultPriceFormatter.php +++ b/packages/core/src/Pricing/DefaultPriceFormatter.php @@ -13,6 +13,12 @@ public function __construct( public ?Currency $currency = null, public int $unitQty = 1 ) { + // unitDecimal() divides by this, so a zero or negative unit quantity + // would be a division by zero or a negative unit price. + if ($this->unitQty < 1) { + $this->unitQty = 1; + } + if (! $this->currency) { $this->currency = Currency::getDefault(); } diff --git a/packages/filament/resources/lang/ar/productoption.php b/packages/filament/resources/lang/ar/productoption.php index 6dc09a56eb..1d03fd92de 100644 --- a/packages/filament/resources/lang/ar/productoption.php +++ b/packages/filament/resources/lang/ar/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'حفظ الخيارات', ], + 'save-variants' => [ + 'label' => 'حفظ المتغيرات', + ], 'add-shared-option' => [ 'label' => 'إضافة خيار مشترك', 'form' => [ diff --git a/packages/filament/resources/lang/bg/productoption.php b/packages/filament/resources/lang/bg/productoption.php index 34d781e6aa..c6ef14d7f8 100644 --- a/packages/filament/resources/lang/bg/productoption.php +++ b/packages/filament/resources/lang/bg/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Запази опциите', ], + 'save-variants' => [ + 'label' => 'Запази вариантите', + ], 'add-shared-option' => [ 'label' => 'Добави споделена опция', 'form' => [ diff --git a/packages/filament/resources/lang/de/productoption.php b/packages/filament/resources/lang/de/productoption.php index be3001252c..7580f299a4 100644 --- a/packages/filament/resources/lang/de/productoption.php +++ b/packages/filament/resources/lang/de/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Optionen speichern', ], + 'save-variants' => [ + 'label' => 'Varianten speichern', + ], 'add-shared-option' => [ 'label' => 'Geteilte Option hinzufügen', 'form' => [ diff --git a/packages/filament/resources/lang/en/productoption.php b/packages/filament/resources/lang/en/productoption.php index a101853486..4956fd8fbd 100644 --- a/packages/filament/resources/lang/en/productoption.php +++ b/packages/filament/resources/lang/en/productoption.php @@ -49,6 +49,9 @@ 'save-options' => [ 'label' => 'Save Options', ], + 'save-variants' => [ + 'label' => 'Save Variants', + ], 'add-shared-option' => [ 'label' => 'Add Shared Option', 'form' => [ diff --git a/packages/filament/resources/lang/es/productoption.php b/packages/filament/resources/lang/es/productoption.php index 6c75dd3631..35d42c3349 100644 --- a/packages/filament/resources/lang/es/productoption.php +++ b/packages/filament/resources/lang/es/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Guardar Opciones', ], + 'save-variants' => [ + 'label' => 'Guardar Variantes', + ], 'add-shared-option' => [ 'label' => 'Agregar Opción Compartida', 'form' => [ diff --git a/packages/filament/resources/lang/fa/productoption.php b/packages/filament/resources/lang/fa/productoption.php index 98ac4eeb15..a323582446 100644 --- a/packages/filament/resources/lang/fa/productoption.php +++ b/packages/filament/resources/lang/fa/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Save Options', ], + 'save-variants' => [ + 'label' => 'ذخیره گونه‌ها', + ], 'add-shared-option' => [ 'label' => 'Add Shared Option', 'form' => [ diff --git a/packages/filament/resources/lang/fr/productoption.php b/packages/filament/resources/lang/fr/productoption.php index 66d523e4ec..ebb07b941e 100644 --- a/packages/filament/resources/lang/fr/productoption.php +++ b/packages/filament/resources/lang/fr/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Enregistrer les options', ], + 'save-variants' => [ + 'label' => 'Enregistrer les variantes', + ], 'add-shared-option' => [ 'label' => 'Ajouter une option partagée', 'form' => [ diff --git a/packages/filament/resources/lang/hr/productoption.php b/packages/filament/resources/lang/hr/productoption.php index 1c1ace85de..42048f3de4 100644 --- a/packages/filament/resources/lang/hr/productoption.php +++ b/packages/filament/resources/lang/hr/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Spremi opcije', ], + 'save-variants' => [ + 'label' => 'Spremi varijante', + ], 'add-shared-option' => [ 'label' => 'Dodaj dijeljenu opciju', 'form' => [ diff --git a/packages/filament/resources/lang/hu/productoption.php b/packages/filament/resources/lang/hu/productoption.php index 19274b2d74..aea5df7d15 100644 --- a/packages/filament/resources/lang/hu/productoption.php +++ b/packages/filament/resources/lang/hu/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Opciók mentése', ], + 'save-variants' => [ + 'label' => 'Termékváltozatok mentése', + ], 'add-shared-option' => [ 'label' => 'Megosztott opció hozzáadása', 'form' => [ diff --git a/packages/filament/resources/lang/mn/productoption.php b/packages/filament/resources/lang/mn/productoption.php index 70f24611d8..5ff9733c6c 100644 --- a/packages/filament/resources/lang/mn/productoption.php +++ b/packages/filament/resources/lang/mn/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Сонголтууд хадгалах', ], + 'save-variants' => [ + 'label' => 'Вариантууд хадгалах', + ], 'add-shared-option' => [ 'label' => 'Хуваалцсан сонголт нэмэх', 'form' => [ diff --git a/packages/filament/resources/lang/nl/productoption.php b/packages/filament/resources/lang/nl/productoption.php index ca38cdcb99..57688f545a 100644 --- a/packages/filament/resources/lang/nl/productoption.php +++ b/packages/filament/resources/lang/nl/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Opties opslaan', ], + 'save-variants' => [ + 'label' => 'Varianten opslaan', + ], 'add-shared-option' => [ 'label' => 'Gedeelde optie toevoegen', 'form' => [ diff --git a/packages/filament/resources/lang/pl/productoption.php b/packages/filament/resources/lang/pl/productoption.php index 311a5ac7b5..f93c63b347 100644 --- a/packages/filament/resources/lang/pl/productoption.php +++ b/packages/filament/resources/lang/pl/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Zapisz opcje', ], + 'save-variants' => [ + 'label' => 'Zapisz warianty', + ], 'add-shared-option' => [ 'label' => 'Dodaj opcję współdzieloną', 'form' => [ diff --git a/packages/filament/resources/lang/pt_BR/productoption.php b/packages/filament/resources/lang/pt_BR/productoption.php index e2fd1ba5b2..083a132e4e 100644 --- a/packages/filament/resources/lang/pt_BR/productoption.php +++ b/packages/filament/resources/lang/pt_BR/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Salvar opções', ], + 'save-variants' => [ + 'label' => 'Salvar variações', + ], 'add-shared-option' => [ 'label' => 'Adicionar opção compartilhada', 'form' => [ diff --git a/packages/filament/resources/lang/ro/productoption.php b/packages/filament/resources/lang/ro/productoption.php index ad916c513a..5c2528c520 100644 --- a/packages/filament/resources/lang/ro/productoption.php +++ b/packages/filament/resources/lang/ro/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Salvează opțiunile', ], + 'save-variants' => [ + 'label' => 'Salvează variantele', + ], 'add-shared-option' => [ 'label' => 'Adaugă opțiune partajată', 'form' => [ diff --git a/packages/filament/resources/lang/tr/productoption.php b/packages/filament/resources/lang/tr/productoption.php index db5c43ae1b..186f51373a 100644 --- a/packages/filament/resources/lang/tr/productoption.php +++ b/packages/filament/resources/lang/tr/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Seçenekleri Kaydet', ], + 'save-variants' => [ + 'label' => 'Varyantları Kaydet', + ], 'add-shared-option' => [ 'label' => 'Paylaşılan Seçenek Ekle', 'form' => [ diff --git a/packages/filament/resources/lang/vi/productoption.php b/packages/filament/resources/lang/vi/productoption.php index 253855c916..961d2f18e2 100644 --- a/packages/filament/resources/lang/vi/productoption.php +++ b/packages/filament/resources/lang/vi/productoption.php @@ -44,6 +44,9 @@ 'save-options' => [ 'label' => 'Lưu tùy chọn', ], + 'save-variants' => [ + 'label' => 'Lưu biến thể', + ], 'add-shared-option' => [ 'label' => 'Thêm tùy chọn dùng chung', 'form' => [ diff --git a/packages/filament/resources/views/widgets/products/product-options.blade.php b/packages/filament/resources/views/widgets/products/product-options.blade.php index 3189a28387..f5beb07c22 100644 --- a/packages/filament/resources/views/widgets/products/product-options.blade.php +++ b/packages/filament/resources/views/widgets/products/product-options.blade.php @@ -64,7 +64,7 @@
{{ \Filament\Support\generate_icon_html('lucide-shapes', size: \Filament\Support\Enums\IconSize::Large) }}
-

No Product Options Configured

+

{{ __('lunar-filament::productoption.widgets.product-options.options-list.empty.heading') }}

@endif diff --git a/packages/filament/src/Forms/Components/CollectionSelect.php b/packages/filament/src/Forms/Components/CollectionSelect.php index 9f562cf9d5..eea66b587c 100644 --- a/packages/filament/src/Forms/Components/CollectionSelect.php +++ b/packages/filament/src/Forms/Components/CollectionSelect.php @@ -27,11 +27,11 @@ protected function setUp(): void $this->label(__('lunar-filament::forms/selectors.collection.label')); $this->placeholder(__('lunar-filament::forms/selectors.collection.placeholder')); $this->searchable(); - $this->modifyOptionsQueryUsing(fn ($query) => method_exists($query, 'with') ? $query->with('ancestors') : $query); + $this->modifyOptionsQueryUsing(fn ($query) => method_exists($query, 'with') ? $query->with(['group', 'ancestors']) : $query); $this->getSearchResultsUsing(fn (string $search): array => $this->searchLunarRecords($search)); $this->getOptionLabelUsing(function ($value): ?string { $model = $this->lunarModel(); - $record = $model::with('ancestors')->find($value); + $record = $model::with(['group', 'ancestors'])->find($value); return $record ? $this->optionLabel($record) : null; }); @@ -84,11 +84,20 @@ public function withinGroup(CollectionGroup $group): static return $this; } + public function optionLabelRelations(): array + { + return ['group', 'ancestors']; + } + public function optionLabel(Model $record): string { $breadcrumb = $record->breadcrumb ?? collect(); - $trail = $breadcrumb->push($record->translate('name')) + // Group first: two collections can share a name and an ancestry in + // different groups, and the trail alone leaves them indistinguishable. + $trail = collect([$record->group?->name]) + ->concat($breadcrumb) + ->push($record->translate('name')) ->filter() ->implode(' > '); diff --git a/packages/filament/src/Forms/Components/Concerns/AppliesToExistingSelect.php b/packages/filament/src/Forms/Components/Concerns/AppliesToExistingSelect.php index a333cb20ac..fde80a59a5 100644 --- a/packages/filament/src/Forms/Components/Concerns/AppliesToExistingSelect.php +++ b/packages/filament/src/Forms/Components/Concerns/AppliesToExistingSelect.php @@ -20,6 +20,16 @@ */ trait AppliesToExistingSelect { + /** + * Relations optionLabel() reads, eager loaded before it is called. + * + * @return array + */ + public function optionLabelRelations(): array + { + return []; + } + public static function applyTo(Select $select): Select { $proxy = static::make($select->getName() ?: '__lunar_selector_proxy'); @@ -32,7 +42,7 @@ public static function applyTo(Select $select): Select $select->getSearchResultsUsing(static function (string $search) use ($proxy, $modelClass): array { $query = RecordSearch::for($modelClass, $search); - return $query->take(50)->get() + return $query->with($proxy->optionLabelRelations())->take(50)->get() ->mapWithKeys(static fn (Model $record): array => [ $record->getKey() => $proxy->optionLabel($record), ]) @@ -40,7 +50,9 @@ public static function applyTo(Select $select): Select }); $select->getOptionLabelUsing(static function ($value) use ($proxy, $modelClass): ?string { - $record = $modelClass::find($value); + // optionLabel() may reach for relations (a collection's group and + // ancestors), which lazy loading would refuse under strict mode. + $record = $modelClass::with($proxy->optionLabelRelations())->find($value); return $record ? $proxy->optionLabel($record) : null; }); diff --git a/packages/filament/src/RelationManagers/Discount/CollectionConditionRelationManager.php b/packages/filament/src/RelationManagers/Discount/CollectionConditionRelationManager.php index b8ac027cd5..78071c8b79 100644 --- a/packages/filament/src/RelationManagers/Discount/CollectionConditionRelationManager.php +++ b/packages/filament/src/RelationManagers/Discount/CollectionConditionRelationManager.php @@ -43,6 +43,9 @@ public function getDefaultTable(Table $table): Table fn ($query) => $query->where('type', 'condition') ->where('discountable_type', Collection::morphName()) ->whereHas('discountable') + ->with(['discountable' => fn ($morphTo) => $morphTo->morphWith([ + Collection::class => ['group', 'ancestors'], + ])]) ) ->headerActions([ CreateAction::make()->schema([ @@ -63,6 +66,12 @@ public function getDefaultTable(Table $table): Table ->label( __('lunar-filament::discount.relationmanagers.collection_conditions.table.name.label') ) + ->description( + fn (Model $record): string => collect([$record->discountable?->group?->name]) + ->concat($record->discountable?->breadcrumb ?? []) + ->filter() + ->implode(' > ') + ) ->formatStateUsing( fn (Model $record) => $record->discountable?->translate('name') ), diff --git a/packages/filament/src/RelationManagers/Discount/CollectionLimitationRelationManager.php b/packages/filament/src/RelationManagers/Discount/CollectionLimitationRelationManager.php index c770855bba..7f18285442 100644 --- a/packages/filament/src/RelationManagers/Discount/CollectionLimitationRelationManager.php +++ b/packages/filament/src/RelationManagers/Discount/CollectionLimitationRelationManager.php @@ -40,7 +40,7 @@ public function getDefaultTable(Table $table): Table ->modifyQueryUsing( fn ($query) => $query ->whereIn($prefix.'collection_discount.type', ['limitation', 'exclusion']) - ->with('ancestors') + ->with(['group', 'ancestors']) ) ->paginated(false) ->headerActions([ @@ -66,7 +66,10 @@ public function getDefaultTable(Table $table): Table ->label( __('lunar-filament::discount.relationmanagers.collections.table.name.label') ) - ->description(fn (Collection $record): string => $record->breadcrumb->implode(' > ')) + ->description(fn (Collection $record): string => collect([$record->group?->name]) + ->concat($record->breadcrumb) + ->filter() + ->implode(' > ')) ->formatStateUsing( fn (Model $record) => $record->translate('name') ), diff --git a/packages/filament/src/Schemas/ProductVariant/ProductVariantForm.php b/packages/filament/src/Schemas/ProductVariant/ProductVariantForm.php index bfbc3598e2..ce415512bc 100644 --- a/packages/filament/src/Schemas/ProductVariant/ProductVariantForm.php +++ b/packages/filament/src/Schemas/ProductVariant/ProductVariantForm.php @@ -95,7 +95,8 @@ public static function getUnitQtyComponent(): TextInput ->label(__('lunar-filament::productvariant.form.unit_quantity.label')) ->hintIcon('heroicon-m-question-mark-circle') ->hintIconTooltip(__('lunar-filament::productvariant.form.unit_quantity.tooltip')) - ->numeric(); + ->numeric() + ->minValue(1); } public static function getQuantityIncrementComponent(): TextInput diff --git a/packages/filament/src/Widgets/Products/ProductOptionsWidget.php b/packages/filament/src/Widgets/Products/ProductOptionsWidget.php index 37ea17329a..409d4c86fc 100644 --- a/packages/filament/src/Widgets/Products/ProductOptionsWidget.php +++ b/packages/filament/src/Widgets/Products/ProductOptionsWidget.php @@ -58,6 +58,9 @@ public function addSharedOptionAction(): Action ->get(); return Action::make('addSharedOption') + ->label( + __('lunar-filament::productoption.widgets.product-options.actions.add-shared-option.label') + ) ->schema([ Callout::make() ->status('info') @@ -354,6 +357,9 @@ protected function mapOptionValuesToIds(array $values): array public function saveVariantsAction(): Action { return Action::make('saveVariants') + ->label( + __('lunar-filament::productoption.widgets.product-options.actions.save-variants.label') + ) ->action(function () { try { $this->saveVariants(); diff --git a/tests/admin/Feature/Filament/Resources/OrderResource/Pages/ManageOrderTest.php b/tests/admin/Feature/Filament/Resources/OrderResource/Pages/ManageOrderTest.php index 0aad2102dd..689b925e6a 100644 --- a/tests/admin/Feature/Filament/Resources/OrderResource/Pages/ManageOrderTest.php +++ b/tests/admin/Feature/Filament/Resources/OrderResource/Pages/ManageOrderTest.php @@ -247,3 +247,37 @@ public function via(): array NotificationFacade::assertNothingSent(); }); + +it('displays boolean order meta values as localized yes/no', function () { + $this->order->update([ + 'meta' => [ + 'express_shipping' => true, + 'gift_message' => false, + ], + ]); + + Livewire::test(ManageOrder::class, [ + 'record' => $this->order->getRouteKey(), + ]) + ->assertSuccessful() + ->assertSchemaComponentStateSet('meta.meta_express_shipping', __('lunarpanel::global.yes')) + ->assertSchemaComponentStateSet('meta.meta_gift_message', __('lunarpanel::global.no')); +}); + +it('does not display numeric order meta values as yes/no', function () { + $this->order->update([ + 'meta' => [ + 'parcel_count' => 1, + 'failed_attempts' => 0, + 'floor' => '1', + ], + ]); + + Livewire::test(ManageOrder::class, [ + 'record' => $this->order->getRouteKey(), + ]) + ->assertSuccessful() + ->assertSchemaComponentStateSet('meta.meta_parcel_count', 1) + ->assertSchemaComponentStateSet('meta.meta_failed_attempts', 0) + ->assertSchemaComponentStateSet('meta.meta_floor', '1'); +}); diff --git a/tests/core/Unit/Models/OrderTest.php b/tests/core/Unit/Models/OrderTest.php index abcdcd048f..2d71d5e080 100644 --- a/tests/core/Unit/Models/OrderTest.php +++ b/tests/core/Unit/Models/OrderTest.php @@ -445,3 +445,49 @@ public function orderContactType(): string expect($order->load('lines')->lines->pluck('id')->all()) ->toBe($expectedOrder); }); + +test('product lines include both physical and digital lines', function () { + Currency::factory()->create([ + 'default' => true, + ]); + + $order = Order::factory()->create([ + 'user_id' => null, + ]); + + $shippableVariant = ProductVariant::factory()->create([ + 'shippable' => true, + ]); + + $nonShippableVariant = ProductVariant::factory()->create([ + 'shippable' => false, + ]); + + OrderLine::factory()->create([ + 'order_id' => $order->id, + 'purchasable_type' => $shippableVariant->getMorphClass(), + 'purchasable_id' => $shippableVariant->id, + 'type' => 'physical', + ]); + + OrderLine::factory()->create([ + 'order_id' => $order->id, + 'purchasable_type' => $nonShippableVariant->getMorphClass(), + 'purchasable_id' => $nonShippableVariant->id, + 'type' => 'digital', + ]); + + OrderLine::factory()->create([ + 'order_id' => $order->id, + 'type' => 'shipping', + ]); + + $order = $order->refresh(); + + // The PDF invoice iterates productLines: a non-shippable variant has + // requires_fulfilment false, so fulfillableLines dropped it from the body. + expect($order->physicalLines)->toHaveCount(1); + expect($order->digitalLines)->toHaveCount(1); + expect($order->productLines)->toHaveCount(2); + expect($order->fulfillableLines)->toHaveCount(1); +}); diff --git a/tests/core/Unit/Pricing/DefaultPriceFormatterTest.php b/tests/core/Unit/Pricing/DefaultPriceFormatterTest.php new file mode 100644 index 0000000000..047755d70b --- /dev/null +++ b/tests/core/Unit/Pricing/DefaultPriceFormatterTest.php @@ -0,0 +1,36 @@ +create([ + 'code' => 'GBP', + 'decimal_places' => 2, + ]); + + // unitDecimal() divides by the unit quantity, so zero would throw and a + // negative would invert the price. + $formatter = new DefaultPriceFormatter(1500, $currency, 0); + + expect($formatter->unitQty)->toBe(1) + ->and($formatter->unitDecimal())->toEqual(15.00) + ->and($formatter->unitFormatted())->toEqual('£15.00'); +}); + +test('divides by a unit quantity of more than one', function () { + $currency = Currency::factory()->create([ + 'code' => 'GBP', + 'decimal_places' => 2, + ]); + + $formatter = new DefaultPriceFormatter(1500, $currency, 3); + + expect($formatter->unitDecimal())->toEqual(5.00); +}); diff --git a/tests/filament/Unit/Schemas/ProductVariant/ProductVariantFormTest.php b/tests/filament/Unit/Schemas/ProductVariant/ProductVariantFormTest.php new file mode 100644 index 0000000000..87f9fadf53 --- /dev/null +++ b/tests/filament/Unit/Schemas/ProductVariant/ProductVariantFormTest.php @@ -0,0 +1,12 @@ +getValidationRules()) + ->toContain('min:1'); +}); diff --git a/tests/filament/Unit/Widgets/ProductOptionsWidgetLabelsTest.php b/tests/filament/Unit/Widgets/ProductOptionsWidgetLabelsTest.php new file mode 100644 index 0000000000..828d614ae3 --- /dev/null +++ b/tests/filament/Unit/Widgets/ProductOptionsWidgetLabelsTest.php @@ -0,0 +1,20 @@ +{$method}()->getLabel()) + ->toBe(__("lunar-filament::productoption.widgets.product-options.actions.{$key}.label")); +})->with([ + 'save variants' => ['saveVariantsAction', 'save-variants'], + 'add shared option' => ['addSharedOptionAction', 'add-shared-option'], +]); From 5f731c71df58cf3665ae772dfe66285e7de85302 Mon Sep 17 00:00:00 2001 From: Glenn Jacobs Date: Wed, 26 Aug 2026 18:26:43 +0100 Subject: [PATCH 09/11] fix(core): label the shipping:manage permission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/core/resources/lang/ar/auth.php | 2 ++ packages/core/resources/lang/bg/auth.php | 2 ++ packages/core/resources/lang/de/auth.php | 2 ++ packages/core/resources/lang/en/auth.php | 2 ++ packages/core/resources/lang/es/auth.php | 2 ++ packages/core/resources/lang/fa/auth.php | 2 ++ packages/core/resources/lang/fr/auth.php | 2 ++ packages/core/resources/lang/hr/auth.php | 2 ++ packages/core/resources/lang/hu/auth.php | 2 ++ packages/core/resources/lang/mn/auth.php | 2 ++ packages/core/resources/lang/nl/auth.php | 2 ++ packages/core/resources/lang/pl/auth.php | 2 ++ packages/core/resources/lang/pt_BR/auth.php | 2 ++ packages/core/resources/lang/ro/auth.php | 2 ++ packages/core/resources/lang/tr/auth.php | 2 ++ packages/core/resources/lang/vi/auth.php | 2 ++ specs/0065-port-v1.5-fixes.md | 2 +- .../Unit/ShippingPermissionLabelTest.php | 33 +++++++++++++++++++ 18 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 tests/shipping/Unit/ShippingPermissionLabelTest.php diff --git a/packages/core/resources/lang/ar/auth.php b/packages/core/resources/lang/ar/auth.php index 060410244e..7dafa3d5bd 100644 --- a/packages/core/resources/lang/ar/auth.php +++ b/packages/core/resources/lang/ar/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'السماح للموظف بإدارة العملاء', 'permissions.sales:manage-discounts.label' => 'إدارة الخصومات', 'permissions.sales:manage-discounts.description' => 'السماح للموظف بإدارة الخصومات', + 'permissions.shipping:manage.label' => 'إدارة الشحن', + 'permissions.shipping:manage.description' => 'السماح للموظف بإدارة الشحن', ]; diff --git a/packages/core/resources/lang/bg/auth.php b/packages/core/resources/lang/bg/auth.php index 109a64a8b7..71d4a7beb4 100644 --- a/packages/core/resources/lang/bg/auth.php +++ b/packages/core/resources/lang/bg/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Позволява на член на персонала да управлява клиенти', 'permissions.sales:manage-discounts.label' => 'Управление на отстъпки', 'permissions.sales:manage-discounts.description' => 'Позволява на член на персонала да управлява отстъпки', + 'permissions.shipping:manage.label' => 'Управление на доставки', + 'permissions.shipping:manage.description' => 'Позволява на член на персонала да управлява доставките', ]; diff --git a/packages/core/resources/lang/de/auth.php b/packages/core/resources/lang/de/auth.php index 38a036b29a..675e1e9f68 100644 --- a/packages/core/resources/lang/de/auth.php +++ b/packages/core/resources/lang/de/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Erlaubt dem Mitarbeiter, Kunden zu verwalten', 'permissions.sales:manage-discounts.label' => 'Rabatte verwalten', 'permissions.sales:manage-discounts.description' => 'Erlaubt dem Mitarbeiter, Rabatte zu verwalten', + 'permissions.shipping:manage.label' => 'Versand verwalten', + 'permissions.shipping:manage.description' => 'Erlaubt dem Mitarbeiter, den Versand zu verwalten', ]; diff --git a/packages/core/resources/lang/en/auth.php b/packages/core/resources/lang/en/auth.php index 2271ab451a..98341de58c 100644 --- a/packages/core/resources/lang/en/auth.php +++ b/packages/core/resources/lang/en/auth.php @@ -29,4 +29,6 @@ 'permissions.sales:manage-customers.description' => 'Allow the staff member to manage customers', 'permissions.sales:manage-discounts.label' => 'Manage Discounts', 'permissions.sales:manage-discounts.description' => 'Allow the staff member to manage discounts', + 'permissions.shipping:manage.label' => 'Manage Shipping', + 'permissions.shipping:manage.description' => 'Allow the staff member to manage shipping', ]; diff --git a/packages/core/resources/lang/es/auth.php b/packages/core/resources/lang/es/auth.php index 6b20a447b6..3c487b2a7d 100644 --- a/packages/core/resources/lang/es/auth.php +++ b/packages/core/resources/lang/es/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Permite al miembro del personal gestionar clientes', 'permissions.sales:manage-discounts.label' => 'Gestionar Descuentos', 'permissions.sales:manage-discounts.description' => 'Permite al miembro del personal gestionar descuentos', + 'permissions.shipping:manage.label' => 'Gestionar Envíos', + 'permissions.shipping:manage.description' => 'Permite al miembro del personal gestionar los envíos', ]; diff --git a/packages/core/resources/lang/fa/auth.php b/packages/core/resources/lang/fa/auth.php index 14a21c579f..27a6ada57b 100644 --- a/packages/core/resources/lang/fa/auth.php +++ b/packages/core/resources/lang/fa/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Allow the staff member to manage customers', 'permissions.sales:manage-discounts.label' => 'Manage Discounts', 'permissions.sales:manage-discounts.description' => 'Allow the staff member to manage discounts', + 'permissions.shipping:manage.label' => 'مدیریت حمل و نقل', + 'permissions.shipping:manage.description' => 'به کارمند اجازه می‌دهد حمل و نقل را مدیریت کند', ]; diff --git a/packages/core/resources/lang/fr/auth.php b/packages/core/resources/lang/fr/auth.php index 1c578b496f..a46b587f5e 100644 --- a/packages/core/resources/lang/fr/auth.php +++ b/packages/core/resources/lang/fr/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Permet au membre du personnel de gérer les clients', 'permissions.sales:manage-discounts.label' => 'Gérer les remises', 'permissions.sales:manage-discounts.description' => 'Permet au membre du personnel de gérer les remises', + 'permissions.shipping:manage.label' => 'Gérer l\'expédition', + 'permissions.shipping:manage.description' => 'Permet au membre du personnel de gérer l\'expédition', ]; diff --git a/packages/core/resources/lang/hr/auth.php b/packages/core/resources/lang/hr/auth.php index c8aeda7444..558d2a5f48 100644 --- a/packages/core/resources/lang/hr/auth.php +++ b/packages/core/resources/lang/hr/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Omogućuje osoblju upravljanje kupcima', 'permissions.sales:manage-discounts.label' => 'Upravljanje popustima', 'permissions.sales:manage-discounts.description' => 'Omogućuje osoblju upravljanje popustima', + 'permissions.shipping:manage.label' => 'Upravljanje dostavom', + 'permissions.shipping:manage.description' => 'Omogućuje osoblju upravljanje dostavom', ]; diff --git a/packages/core/resources/lang/hu/auth.php b/packages/core/resources/lang/hu/auth.php index e4cb5dc8c6..094b96b49b 100644 --- a/packages/core/resources/lang/hu/auth.php +++ b/packages/core/resources/lang/hu/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Engedélyezi a vásárlók kezelését', 'permissions.sales:manage-discounts.label' => 'Kedvezmények kezelése', 'permissions.sales:manage-discounts.description' => 'Engedélyezi a kedvezmények kezelését', + 'permissions.shipping:manage.label' => 'Szállítás kezelése', + 'permissions.shipping:manage.description' => 'Engedélyezi a szállítás kezelését', ]; diff --git a/packages/core/resources/lang/mn/auth.php b/packages/core/resources/lang/mn/auth.php index 7f9cc2c413..2dc577b66d 100644 --- a/packages/core/resources/lang/mn/auth.php +++ b/packages/core/resources/lang/mn/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Харилцагчдыг удирдах эрх олгоно', 'permissions.sales:manage-discounts.label' => 'Хөнгөлөлтүүдийн удирдлага', 'permissions.sales:manage-discounts.description' => 'Хөнгөлөлтүүдийг удирдах эрх олгоно', + 'permissions.shipping:manage.label' => 'Хүргэлтийн удирдлага', + 'permissions.shipping:manage.description' => 'Хүргэлтийг удирдах эрх олгоно', ]; diff --git a/packages/core/resources/lang/nl/auth.php b/packages/core/resources/lang/nl/auth.php index c46704ba64..48ccbe38a7 100644 --- a/packages/core/resources/lang/nl/auth.php +++ b/packages/core/resources/lang/nl/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Sta het personeelslid toe om klanten te beheren', 'permissions.sales:manage-discounts.label' => 'Beheer Kortingen', 'permissions.sales:manage-discounts.description' => 'Sta het personeelslid toe om kortingen te beheren', + 'permissions.shipping:manage.label' => 'Beheer Verzending', + 'permissions.shipping:manage.description' => 'Sta het personeelslid toe om verzending te beheren', ]; diff --git a/packages/core/resources/lang/pl/auth.php b/packages/core/resources/lang/pl/auth.php index 42c93958d5..1660cef359 100644 --- a/packages/core/resources/lang/pl/auth.php +++ b/packages/core/resources/lang/pl/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Pozwala na zarządzanie klientami', 'permissions.sales:manage-discounts.label' => 'Zarządzanie rabatami', 'permissions.sales:manage-discounts.description' => 'Pozwala na zarządzanie rabatami', + 'permissions.shipping:manage.label' => 'Zarządzanie dostawą', + 'permissions.shipping:manage.description' => 'Pozwala na zarządzanie dostawą', ]; diff --git a/packages/core/resources/lang/pt_BR/auth.php b/packages/core/resources/lang/pt_BR/auth.php index d56b710c1b..f370908259 100644 --- a/packages/core/resources/lang/pt_BR/auth.php +++ b/packages/core/resources/lang/pt_BR/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Permite que o membro da equipe gerencie clientes', 'permissions.sales:manage-discounts.label' => 'Gerenciar descontos', 'permissions.sales:manage-discounts.description' => 'Permite que o membro da equipe gerencie descontos', + 'permissions.shipping:manage.label' => 'Gerenciar envio', + 'permissions.shipping:manage.description' => 'Permite que o membro da equipe gerencie o envio', ]; diff --git a/packages/core/resources/lang/ro/auth.php b/packages/core/resources/lang/ro/auth.php index 58566805db..4c9b1c2036 100644 --- a/packages/core/resources/lang/ro/auth.php +++ b/packages/core/resources/lang/ro/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Permite personalului să gestioneze clienții', 'permissions.sales:manage-discounts.label' => 'Gestionare reduceri', 'permissions.sales:manage-discounts.description' => 'Permite personalului să gestioneze reducerile', + 'permissions.shipping:manage.label' => 'Gestionare livrare', + 'permissions.shipping:manage.description' => 'Permite personalului să gestioneze livrarea', ]; diff --git a/packages/core/resources/lang/tr/auth.php b/packages/core/resources/lang/tr/auth.php index 594e5cf409..483b71c721 100644 --- a/packages/core/resources/lang/tr/auth.php +++ b/packages/core/resources/lang/tr/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Personelin müşterileri yönetmesine izin verir', 'permissions.sales:manage-discounts.label' => 'İndirim Yönetimi', 'permissions.sales:manage-discounts.description' => 'Personelin indirimleri yönetmesine izin verir', + 'permissions.shipping:manage.label' => 'Kargo Yönetimi', + 'permissions.shipping:manage.description' => 'Personelin kargo işlemlerini yönetmesine izin verir', ]; diff --git a/packages/core/resources/lang/vi/auth.php b/packages/core/resources/lang/vi/auth.php index 506db755bb..1e0edc0055 100644 --- a/packages/core/resources/lang/vi/auth.php +++ b/packages/core/resources/lang/vi/auth.php @@ -23,4 +23,6 @@ 'permissions.sales:manage-customers.description' => 'Cho phép nhân viên quản lý khách hàng', 'permissions.sales:manage-discounts.label' => 'Quản lý giảm giá', 'permissions.sales:manage-discounts.description' => 'Cho phép nhân viên quản lý giảm giá', + 'permissions.shipping:manage.label' => 'Quản lý vận chuyển', + 'permissions.shipping:manage.description' => 'Cho phép nhân viên quản lý vận chuyển', ]; diff --git a/specs/0065-port-v1.5-fixes.md b/specs/0065-port-v1.5-fixes.md index 2a9f06eb2c..fb1ef6f6ae 100644 --- a/specs/0065-port-v1.5-fixes.md +++ b/specs/0065-port-v1.5-fixes.md @@ -58,7 +58,7 @@ The four `processAutomaticRewards()` fixes are one textual stack and must land i 21. **Localize boolean meta values** (#2545) — the `is_bool` ternary in `ManageOrder::getDefaultAdditionalInfoSection()` plus `yes`/`no` keys in `global.php` across all 16 `packages/admin` locales (this file still resolves through `lunarpanel::` — don't move namespaces as part of the port). Keep the stringification in `getStateUsing` so the `strlen()` tooltip closure downstream stays safe. 22. **Unit quantity minimum of 1** (#2547) — `->minValue(1)` on `ProductVariantForm::getUnitQtyComponent()` and the `max(1, $unitQty)` clamp in `DefaultPriceFormatter` (division-by-zero guard). The panel already enforces `min:1` in `VariantFields`. Consider the sibling `min_quantity`/`quantity_increment` components while there. 23. **Translate product-options widget labels** (#2555) — `->label()` on the `addSharedOption` and `saveVariants` actions, the empty-state heading in the widget blade (delete the stale duplicate blade under `packages/admin` rather than fixing both), and a new `save-variants.label` key across all 16 `packages/filament` locales. -24. **Shipping permission labels** (#2484) — do **not** port the 1.x `mergeTranslationsForPanel()` merge hack. v2 centralises label resolution in the `Permission` DTO on `lunar::auth.permissions.{handle}.*`, consumed by both the Filament staff form and the panel role/staff controllers — so either add the two `shipping:manage` keys to `packages/core` `auth.php` (16 locales), or have `ShippingServiceProvider` call `Lang::addLines()` into the `lunar` namespace. The latter doubles as the documented seam for third-party add-ons. Revisit the v1-era `lunarpanel.shipping` translation namespace in the same change. +24. **Shipping permission labels** (#2484) — the 1.x `mergeTranslationsForPanel()` merge hack is **not** ported. v2 centralises label resolution in the `Permission` DTO on the flat key `lunar::auth.permissions.{handle}.*`, consumed by both the Filament staff form and the panel role/staff controllers, so one fix covers both surfaces: the two `shipping:manage` keys go into `packages/core` `auth.php` across 16 locales. `shipping:manage` is a first-party permission registered by a first-party migration, so core is its home; no runtime merging, and no new machinery. A third-party add-on that wants its own permission labelled uses `Lang::addLines()` into the `lunar` namespace — the native Laravel API, which needs nothing from us because the DTO reads a flat dotted key. Renaming the v1-era `lunarpanel.shipping` translation namespace is deliberately **left alone**: it would break every `__('lunarpanel.shipping::…')` call site and any published overrides, so it needs its own spec and Rector rule. 25. **Collection breadcrumb gaps in discount selection** (#2557) — mostly covered by `CollectionSelect` already; close the deltas: prepend the collection group name to `optionLabel()` (+ eager-load `group`), give `CollectionConditionRelationManager`'s table column the breadcrumb `description()` + `morphWith` eager load for parity with the limitation manager, and eager-load `ancestors` in `AppliesToExistingSelect::applyTo()` (an N+1 that trips `preventLazyLoading()`). 26. **Digital lines on the PDF invoice** (#2599) — the bug survived the v2 refactor in a new form: `pdf/order.blade.php` iterates `fulfillableLines`, and non-shippable variants have `requires_fulfilment = false`, so they vanish from the invoice body. Switch to `productLines`. Audit other `fulfillableLines` call sites to confirm the invoice is the only mis-application. diff --git a/tests/shipping/Unit/ShippingPermissionLabelTest.php b/tests/shipping/Unit/ShippingPermissionLabelTest.php new file mode 100644 index 0000000000..d0f1af55f5 --- /dev/null +++ b/tests/shipping/Unit/ShippingPermissionLabelTest.php @@ -0,0 +1,33 @@ +group('shipping'); + +uses(RefreshDatabase::class); + +/** + * The permission is registered by the shipping package's own migration, but + * label resolution is centralised on `lunar::auth.permissions.{handle}.*` and + * falls back to the raw handle — so an unlabelled permission rendered as + * "shipping:manage" in the staff and roles UI of both panels. + */ +it('labels the shipping permission rather than falling back to the handle', function () { + $permission = Permission::make('shipping:manage', firstParty: true); + + expect($permission->transLabel())->toBe('Manage Shipping') + ->and($permission->transDescription())->not->toBe('shipping:manage'); +}); + +it('translates the shipping permission label', function (string $locale, string $label) { + app()->setLocale($locale); + + expect(Permission::make('shipping:manage', firstParty: true)->transLabel()) + ->toBe($label); +})->with([ + 'de' => ['de', 'Versand verwalten'], + 'fr' => ['fr', "Gérer l'expédition"], + 'nl' => ['nl', 'Beheer Verzending'], +]); From 335e165a705e43dc26170ec04553c22a0e4cb145 Mon Sep 17 00:00:00 2001 From: Glenn Jacobs Date: Wed, 26 Aug 2026 18:30:56 +0100 Subject: [PATCH 10/11] fix(stripe): convert amounts based on currency decimal places MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../stripe/resources/responses/charge.json | 2 +- .../stripe/resources/responses/refund.json | 9 ++ packages/stripe/src/Actions/StoreCharges.php | 3 +- .../stripe/src/Managers/StripeManager.php | 91 +++++++++++++++++-- packages/stripe/src/MockClient.php | 28 ++++++ packages/stripe/src/StripePaymentType.php | 12 ++- .../stripe/Unit/Actions/StoreChargesTest.php | 13 ++- .../Unit/Managers/StripeManagerTest.php | 83 +++++++++++++++++ tests/stripe/Unit/StripePaymentTypeTest.php | 90 ++++++++++++++++++ tests/stripe/Utils/CartBuilder.php | 6 +- tests/stripe/Utils/StripeFake.php | 6 +- 11 files changed, 321 insertions(+), 22 deletions(-) create mode 100644 packages/stripe/resources/responses/refund.json diff --git a/packages/stripe/resources/responses/charge.json b/packages/stripe/resources/responses/charge.json index 86291f26a7..3cd206be76 100644 --- a/packages/stripe/resources/responses/charge.json +++ b/packages/stripe/resources/responses/charge.json @@ -45,7 +45,7 @@ "type": "authorized" }, "paid": true, - "payment_intent": null, + "payment_intent": "{payment_intent}", "payment_method": "card_1MmlLrLkdIwHu7ixIJwEWSNR", "payment_method_details": { "card": { diff --git a/packages/stripe/resources/responses/refund.json b/packages/stripe/resources/responses/refund.json new file mode 100644 index 0000000000..e4e8471fb7 --- /dev/null +++ b/packages/stripe/resources/responses/refund.json @@ -0,0 +1,9 @@ +{ + "id": "re_TEST", + "object": "refund", + "amount": {refund_amount}, + "currency": "usd", + "payment_intent": "{payment_intent}", + "reason": null, + "status": "succeeded" +} diff --git a/packages/stripe/src/Actions/StoreCharges.php b/packages/stripe/src/Actions/StoreCharges.php index 65a64361e4..d66fd39f8b 100644 --- a/packages/stripe/src/Actions/StoreCharges.php +++ b/packages/stripe/src/Actions/StoreCharges.php @@ -5,6 +5,7 @@ use Illuminate\Support\Collection; use Lunar\Core\Models\Order; use Lunar\Core\Models\Transaction; +use Lunar\Stripe\Managers\StripeManager; class StoreCharges { @@ -64,7 +65,7 @@ public function store(Order $order, Collection $charges) 'success' => (bool) ! $charge->failure_code, 'type' => $charge->refunded ? 'refund' : $type, 'driver' => 'stripe', - 'amount' => $charge->amount, + 'amount' => StripeManager::fromStripeAmount($charge->amount, $order->currency), 'reference' => $charge->id, 'status' => $charge->status, 'notes' => $charge->failure_message ?: $charge->description, diff --git a/packages/stripe/src/Managers/StripeManager.php b/packages/stripe/src/Managers/StripeManager.php index cc837fc301..db57f8d163 100644 --- a/packages/stripe/src/Managers/StripeManager.php +++ b/packages/stripe/src/Managers/StripeManager.php @@ -216,26 +216,99 @@ public function getCharge(string $chargeId): Charge return $this->getClient()->charges->retrieve($chargeId); } + /** + * Zero-decimal currencies, per Stripe. The amount sent to Stripe is the + * major unit amount as-is. + * + * @see https://docs.stripe.com/currencies#zero-decimal + */ + protected const ZERO_DECIMAL_CURRENCIES = [ + 'bif', 'clp', 'djf', 'gnf', 'jpy', 'kmf', 'krw', 'mga', 'pyg', + 'rwf', 'ugx', 'vnd', 'vuv', 'xaf', 'xof', 'xpf', + ]; + + /** + * Three-decimal currencies, per Stripe. The amount sent to Stripe is the + * major unit amount multiplied by 1000. + * + * @see https://docs.stripe.com/currencies#three-decimal + */ + protected const THREE_DECIMAL_CURRENCIES = ['bhd', 'jod', 'kwd', 'omr', 'tnd']; + + /** + * HUF, TWD and UGX are ISO zero-decimal currencies, but Stripe still + * requires amounts to be sent as if they had two decimal places. + * + * @see https://docs.stripe.com/currencies#special-cases + */ + protected const SPECIAL_ZERO_DECIMAL_CURRENCIES = ['huf', 'twd', 'ugx']; + /** * Convert a Lunar price value to the amount expected by Stripe. * - * For most currencies Stripe expects the amount in the same sub-unit - * Lunar already stores it in (controlled by `Currency::decimal_places`). - * HUF, TWD and UGX are the exception: although they are ISO zero-decimal - * currencies, Stripe requires amounts to be sent as if they had two - * decimal places. + * Lunar stores prices as integers scaled by `Currency::decimal_places`, + * which merchants can set independently of what Stripe expects for a + * given currency. This converts back to the major unit amount first, + * then re-scales it to whatever sub-unit Stripe requires for the + * currency, so the result is correct regardless of how the merchant has + * configured `Currency::decimal_places`. * * @see https://docs.stripe.com/currencies */ public static function toStripeAmount(int $value, Currency $currency): int { - if (! in_array(strtolower($currency->code), ['huf', 'twd', 'ugx'], true)) { - return $value; + return self::rescale($value, max($currency->decimal_places, 0), self::stripeDecimalPlaces($currency)); + } + + /** + * Convert an amount received from Stripe back to a Lunar price value, + * scaled by `Currency::decimal_places`. Inverse of `toStripeAmount()`. + */ + public static function fromStripeAmount(int $amount, Currency $currency): int + { + return self::rescale($amount, self::stripeDecimalPlaces($currency), max($currency->decimal_places, 0)); + } + + /** + * The number of decimal places Stripe expects amounts in for a currency. + */ + protected static function stripeDecimalPlaces(Currency $currency): int + { + $code = strtolower($currency->code); + + // UGX is also in the zero-decimal list; the special case takes precedence. + if (in_array($code, self::SPECIAL_ZERO_DECIMAL_CURRENCIES, true)) { + return 2; + } + + if (in_array($code, self::ZERO_DECIMAL_CURRENCIES, true)) { + return 0; + } + + if (in_array($code, self::THREE_DECIMAL_CURRENCIES, true)) { + return 3; + } + + return 2; + } + + /** + * Rescale an integer amount between decimal-place precisions using integer + * arithmetic only — float division misrounds at half-unit boundaries + * (145 at 3dp: 0.145 stores as 0.1449…, so round() gives 14, not 15). + * Rounds half away from zero, matching round(). + */ + protected static function rescale(int $value, int $fromDecimalPlaces, int $toDecimalPlaces): int + { + $exponent = $toDecimalPlaces - $fromDecimalPlaces; + + if ($exponent >= 0) { + return $value * (10 ** $exponent); } - $majorAmount = $value / (10 ** $currency->decimal_places); + $divisor = 10 ** (-$exponent); - return (int) round($majorAmount * 100); + return intdiv(abs($value) + intdiv($divisor, 2), $divisor) * ($value < 0 ? -1 : 1); } /** diff --git a/packages/stripe/src/MockClient.php b/packages/stripe/src/MockClient.php index 1c29719cbf..4113258e86 100644 --- a/packages/stripe/src/MockClient.php +++ b/packages/stripe/src/MockClient.php @@ -19,6 +19,13 @@ class MockClient implements ClientInterface public string $url; + /** + * Every request made against the mock, for asserting outgoing payloads. + * + * @var array + */ + public array $requests = []; + private bool $failThenCaptureCalled = false; public function __construct() @@ -35,6 +42,8 @@ public function next(array $data): self public function request($method, $absUrl, $headers, $params, $hasFile, $apiMode = 'v1') { + $this->requests[] = ['method' => $method, 'url' => $absUrl, 'params' => $params]; + $id = array_slice(explode('/', $absUrl), -1)[0]; $policy = config('lunar.stripe.policy'); @@ -48,6 +57,16 @@ public function request($method, $absUrl, $headers, $params, $hasFile, $apiMode return [$this->rBody, $this->rcode, $this->rheaders]; } + if ($method == 'get' && str_contains($absUrl, 'charges/')) { + $this->rBody = $this->getResponse('charge', [ + 'id' => $id, + 'payment_intent' => 'PI_CAPTURE', + ...$this->nextData, + ]); + + return [$this->rBody, $this->rcode, $this->rheaders]; + } + if ($method == 'get' && str_contains($absUrl, 'charges')) { $status = 'succeeded'; @@ -160,6 +179,15 @@ public function request($method, $absUrl, $headers, $params, $hasFile, $apiMode } } + if ($method == 'post' && str_contains($absUrl, 'refunds')) { + $this->rBody = $this->getResponse('refund', [ + 'refund_amount' => $params['amount'] ?? 0, + 'payment_intent' => $params['payment_intent'] ?? 'PI_CAPTURE', + ]); + + return [$this->rBody, $this->rcode, $this->rheaders]; + } + if ($method == 'post' && str_contains($absUrl, 'payment_intents')) { $this->rBody = $this->getResponse('payment_intent_created'); diff --git a/packages/stripe/src/StripePaymentType.php b/packages/stripe/src/StripePaymentType.php index 8ecf3ce1d8..2de0e641d7 100644 --- a/packages/stripe/src/StripePaymentType.php +++ b/packages/stripe/src/StripePaymentType.php @@ -15,6 +15,7 @@ use Lunar\Stripe\Actions\UpdateOrderFromIntent; use Lunar\Stripe\Events\OrphanedPaymentIntentDetected; use Lunar\Stripe\Facades\Stripe; +use Lunar\Stripe\Managers\StripeManager; use Lunar\Stripe\Models\StripePaymentIntent; use Stripe\Exception\InvalidRequestException; use Stripe\PaymentIntent; @@ -196,13 +197,16 @@ protected function assertIntentMatchesTotal(): ?PaymentAuthorize if ($this->order) { $expectedAmount = $this->order->total; $expectedCurrency = $this->order->currency_code; + $currency = $this->order->currency; } else { $calculated = $this->cart->calculate(); $expectedAmount = $calculated->total->value; $expectedCurrency = $calculated->currency->code; + $currency = $calculated->currency; } - $amountMatches = $expectedAmount === (int) $this->paymentIntent->amount; + // The intent amount is in Stripe's sub-unit scale, not Lunar's. + $amountMatches = StripeManager::toStripeAmount($expectedAmount, $currency) === (int) $this->paymentIntent->amount; $currencyMatches = strtolower((string) $expectedCurrency) === strtolower((string) $this->paymentIntent->currency); if ($amountMatches && $currencyMatches) { @@ -232,7 +236,7 @@ public function capture(Transaction $transaction, $amount = 0): PaymentCapture $payload = []; if ($amount > 0) { - $payload['amount_to_capture'] = $amount; + $payload['amount_to_capture'] = StripeManager::toStripeAmount($amount, $transaction->order->currency); } $charge = Stripe::getCharge($transaction->reference); @@ -268,7 +272,7 @@ public function refund(Transaction $transaction, int $amount = 0, $notes = null) try { $refund = $this->stripe->refunds->create( - ['payment_intent' => $charge->payment_intent, 'amount' => $amount] + ['payment_intent' => $charge->payment_intent, 'amount' => StripeManager::toStripeAmount($amount, $transaction->order->currency)] ); } catch (InvalidRequestException $e) { return new PaymentRefund( @@ -281,7 +285,7 @@ public function refund(Transaction $transaction, int $amount = 0, $notes = null) 'success' => $refund->status != 'failed', 'type' => 'refund', 'driver' => 'stripe', - 'amount' => $refund->amount, + 'amount' => StripeManager::fromStripeAmount($refund->amount, $transaction->order->currency), 'reference' => $refund->payment_intent, 'status' => $refund->status, 'notes' => $notes, diff --git a/tests/stripe/Unit/Actions/StoreChargesTest.php b/tests/stripe/Unit/Actions/StoreChargesTest.php index e19e9f96f2..9f76fc715c 100644 --- a/tests/stripe/Unit/Actions/StoreChargesTest.php +++ b/tests/stripe/Unit/Actions/StoreChargesTest.php @@ -8,7 +8,13 @@ uses(TestCase::class); it('can store successful charge', function () { - $cart = CartBuilder::build(); + // Pin a two-decimal currency: the factory's random faker code can land on + // a currency Stripe scales differently (JPY, HUF, ...), which makes the + // stored amount diverge from the raw charge fixture amount. + $cart = CartBuilder::build(currencyParams: [ + 'code' => 'USD', + 'decimal_places' => 2, + ]); $order = $cart->createOrder(); @@ -31,7 +37,10 @@ })->group('lunar.stripe.actions'); it('updates existing transactions', function () { - $cart = CartBuilder::build(); + $cart = CartBuilder::build(currencyParams: [ + 'code' => 'USD', + 'decimal_places' => 2, + ]); $order = $cart->createOrder(); diff --git a/tests/stripe/Unit/Managers/StripeManagerTest.php b/tests/stripe/Unit/Managers/StripeManagerTest.php index 4bcc74dc73..280080cf48 100644 --- a/tests/stripe/Unit/Managers/StripeManagerTest.php +++ b/tests/stripe/Unit/Managers/StripeManagerTest.php @@ -99,3 +99,86 @@ expect(StripeManager::toStripeAmount(500, $currency))->toBe(50000); }); + +it('normalises amounts for currencies configured with more than 2 decimal places', function () { + // A $60.0000 cart stored with 4 decimal places is 600000, not 6000. + $currency = Currency::factory()->make([ + 'code' => 'USD', + 'decimal_places' => 4, + ]); + + expect(StripeManager::toStripeAmount(600000, $currency))->toBe(6000); +}); + +it('normalises amounts for zero-decimal currencies misconfigured with extra decimal places', function () { + $currency = Currency::factory()->make([ + 'code' => 'JPY', + 'decimal_places' => 2, + ]); + + expect(StripeManager::toStripeAmount(100000, $currency))->toBe(1000); +}); + +it('rounds half-unit boundaries exactly when rescaling', function () { + // 145 at 3dp is 0.145 — binary float division would misround this to 14. + $currency = Currency::factory()->make([ + 'code' => 'USD', + 'decimal_places' => 3, + ]); + + expect(StripeManager::toStripeAmount(145, $currency))->toBe(15); + + $currency = Currency::factory()->make([ + 'code' => 'USD', + 'decimal_places' => 4, + ]); + + expect(StripeManager::toStripeAmount(1450, $currency))->toBe(15); +}); + +it('sends three-decimal currency amounts in thousandths', function () { + $currency = Currency::factory()->make([ + 'code' => 'BHD', + 'decimal_places' => 3, + ]); + + expect(StripeManager::toStripeAmount(60123, $currency))->toBe(60123); + + $currency = Currency::factory()->make([ + 'code' => 'BHD', + 'decimal_places' => 2, + ]); + + expect(StripeManager::toStripeAmount(6012, $currency))->toBe(60120); +}); + +it('converts Stripe amounts back to the stored scale', function () { + $currency = Currency::factory()->make([ + 'code' => 'USD', + 'decimal_places' => 4, + ]); + + expect(StripeManager::fromStripeAmount(6000, $currency))->toBe(600000); + + $currency = Currency::factory()->make([ + 'code' => 'JPY', + 'decimal_places' => 2, + ]); + + expect(StripeManager::fromStripeAmount(1000, $currency))->toBe(100000); + + $currency = Currency::factory()->make([ + 'code' => 'HUF', + 'decimal_places' => 0, + ]); + + expect(StripeManager::fromStripeAmount(50000, $currency))->toBe(500); + + // Identity for a correctly-configured two-decimal currency. + $currency = Currency::factory()->make([ + 'code' => 'USD', + 'decimal_places' => 2, + ]); + + expect(StripeManager::fromStripeAmount(1999, $currency))->toBe(1999); +}); diff --git a/tests/stripe/Unit/StripePaymentTypeTest.php b/tests/stripe/Unit/StripePaymentTypeTest.php index b846f9eae6..bb44e65972 100644 --- a/tests/stripe/Unit/StripePaymentTypeTest.php +++ b/tests/stripe/Unit/StripePaymentTypeTest.php @@ -6,6 +6,7 @@ use Lunar\Core\Models\Transaction; use Lunar\Stripe\Events\OrphanedPaymentIntentDetected; use Lunar\Stripe\Facades\Stripe; +use Lunar\Stripe\Managers\StripeManager; use Lunar\Stripe\Models\StripePaymentIntent; use Lunar\Stripe\StripePaymentType; use Lunar\Tests\Stripe\Unit\TestCase; @@ -421,3 +422,92 @@ ->not ->toBe(true); }); + +it('authorizes a cart whose currency needs rescaling for stripe', function () { + $cart = CartBuilder::build(currencyParams: [ + 'code' => 'HUF', + 'decimal_places' => 0, + ]); + $payment = new StripePaymentType; + + $cart->calculate(); + + // Stripe holds HUF amounts multiplied by 100, as createIntent sends them. + StripeFake::forCart($cart, [ + 'amount' => StripeManager::toStripeAmount($cart->total->value, $cart->currency), + ]); + + $response = $payment->cart($cart)->withData([ + 'payment_intent' => 'PI_CAPTURE', + ])->authorize(); + + expect($response->success)->toBeTrue() + ->and($cart->refresh()->completedOrder->placed_at)->not()->toBeNull(); + + // The charges fixture reports 1099 Stripe sub-units; stored back as 11 HUF. + $transaction = $cart->completedOrder->transactions()->where('type', 'capture')->first(); + + expect($transaction->amount)->toBe(11); +}); + +it('rejects an intent holding the raw lunar value for a rescaled currency', function () { + $cart = CartBuilder::build(currencyParams: [ + 'code' => 'HUF', + 'decimal_places' => 0, + ]); + $payment = new StripePaymentType; + + $cart->calculate(); + + // An intent holding the raw stored value — 100x off Stripe's HUF scale. + StripeFake::forCart($cart, ['amount' => $cart->total->value]); + + $response = $payment->cart($cart)->withData([ + 'payment_intent' => 'PI_CAPTURE', + ])->authorize(); + + expect($response->success)->toBeFalse() + ->and($response->message)->toEqual('Payment intent amount does not match order total'); +}); + +it('converts capture and refund amounts to stripe scale and stores refunds in lunar scale', function () { + $cart = CartBuilder::build(currencyParams: [ + 'code' => 'HUF', + 'decimal_places' => 0, + ]); + $payment = new StripePaymentType; + + $cart->calculate(); + + $mock = StripeFake::forCart($cart, [ + 'amount' => StripeManager::toStripeAmount($cart->total->value, $cart->currency), + ]); + + $payment->cart($cart)->withData([ + 'payment_intent' => 'PI_CAPTURE', + ])->authorize(); + + $order = $cart->refresh()->completedOrder; + $transaction = $order->transactions()->where('type', 'capture')->first(); + + $payment->capture($transaction, 500); + + $captureRequest = collect($mock->requests)->last( + fn ($r) => $r['method'] == 'post' && str_contains($r['url'], 'capture') + ); + + expect($captureRequest['params']['amount_to_capture'])->toBe(50000); + + $payment->refund($transaction, 500, 'test refund'); + + $refundRequest = collect($mock->requests)->last( + fn ($r) => $r['method'] == 'post' && str_contains($r['url'], 'refunds') + ); + + expect($refundRequest['params']['amount'])->toBe(50000); + + // The refund transaction is stored back in Lunar's scale. + $refund = $order->transactions()->where('type', 'refund')->first(); + + expect($refund->amount)->toBe(500); +}); diff --git a/tests/stripe/Utils/CartBuilder.php b/tests/stripe/Utils/CartBuilder.php index 4832ccacc6..2e78eb982d 100644 --- a/tests/stripe/Utils/CartBuilder.php +++ b/tests/stripe/Utils/CartBuilder.php @@ -15,15 +15,15 @@ class CartBuilder { - public static function build(array $cartParams = []) + public static function build(array $cartParams = [], array $currencyParams = []) { Language::factory()->create([ 'default' => true, ]); - $currency = Currency::factory()->create([ + $currency = Currency::factory()->create(array_merge([ 'default' => true, - ]); + ], $currencyParams)); $taxClass = TaxClass::factory()->create(); diff --git a/tests/stripe/Utils/StripeFake.php b/tests/stripe/Utils/StripeFake.php index 12b2dd1b7c..d1df2e01ad 100644 --- a/tests/stripe/Utils/StripeFake.php +++ b/tests/stripe/Utils/StripeFake.php @@ -5,6 +5,7 @@ use Lunar\Core\Models\Cart; use Lunar\Core\Models\Order; use Lunar\Stripe\Facades\Stripe; +use Lunar\Stripe\Managers\StripeManager; use Lunar\Stripe\MockClient; class StripeFake @@ -18,7 +19,8 @@ public static function forCart(Cart $cart, array $extra = []): MockClient $cart->calculate(); return Stripe::fake([ - 'amount' => $cart->total->value, + // Mirror createIntent: Stripe holds amounts in its own sub-unit scale. + 'amount' => StripeManager::toStripeAmount($cart->total->value, $cart->currency), 'currency' => strtolower($cart->currency->code), ...$extra, ]); @@ -31,7 +33,7 @@ public static function forCart(Cart $cart, array $extra = []): MockClient public static function forOrder(Order $order, array $extra = []): MockClient { return Stripe::fake([ - 'amount' => $order->total, + 'amount' => StripeManager::toStripeAmount($order->total, $order->currency), 'currency' => strtolower($order->currency_code), ...$extra, ]); From 6cfd24b8294942b68e4080593558b95501f1c152 Mon Sep 17 00:00:00 2001 From: Glenn Jacobs Date: Wed, 26 Aug 2026 18:35:01 +0100 Subject: [PATCH 11/11] fix(core): type formatterStyle as int, not string 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) --- TODO.md | 2 +- .../src/Pricing/DefaultPriceFormatter.php | 6 +- .../src/Pricing/PriceFormatterInterface.php | 4 +- packages/upgrade/src/Rector/LunarSetList.php | 2 + .../RetypeFormatterStyleParamRector.php | 86 +++++++++++++++++++ specs/{ => completed}/0065-port-v1.5-fixes.md | 24 +++--- .../Pricing/DefaultPriceFormatterTest.php | 16 ++++ .../formatter_style_param.php.inc | 41 +++++++++ .../leaves_unrelated_alone.php.inc | 24 ++++++ .../RetypeFormatterStyleParamRectorTest.php | 28 ++++++ .../Rector/Pricing/config/formatter_style.php | 9 ++ 11 files changed, 224 insertions(+), 18 deletions(-) create mode 100644 packages/upgrade/src/Rector/Pricing/RetypeFormatterStyleParamRector.php rename specs/{ => completed}/0065-port-v1.5-fixes.md (95%) create mode 100644 tests/upgrade/Unit/Rector/Pricing/Fixture/RetypeFormatterStyleParamRector/formatter_style_param.php.inc create mode 100644 tests/upgrade/Unit/Rector/Pricing/Fixture/RetypeFormatterStyleParamRector/leaves_unrelated_alone.php.inc create mode 100644 tests/upgrade/Unit/Rector/Pricing/RetypeFormatterStyleParamRectorTest.php create mode 100644 tests/upgrade/Unit/Rector/Pricing/config/formatter_style.php diff --git a/TODO.md b/TODO.md index f2128c9e5f..d8cf10320e 100644 --- a/TODO.md +++ b/TODO.md @@ -22,7 +22,6 @@ Items tagged _(judgement)_ are genuine line-calls worth revisiting. - Order print templates — Print dropdown of selectable PDF templates, ships an Advice Note (spec 0027) - Cart/order line grouping — grouping key on the `*_lines` tables _(judgement)_ - Cart totals caching in the database — additive performance optimisation -- Port the v1.5.0 fixes into v2 (spec 0065) - Add Boost guidelines to packages ## Ideas @@ -85,3 +84,4 @@ Items tagged _(judgement)_ are genuine line-calls worth revisiting. - Per-attribute validation rules across both panels (spec 0062) - Standalone attributes surface in the Filament admin (spec 0063) - Long-lived worker safety — scoped lifetimes for per-visitor service state (spec 0064) +- Port the v1.5.0 fixes into v2 (spec 0065) diff --git a/packages/core/src/Pricing/DefaultPriceFormatter.php b/packages/core/src/Pricing/DefaultPriceFormatter.php index 8088bc6490..dc6f7b687d 100644 --- a/packages/core/src/Pricing/DefaultPriceFormatter.php +++ b/packages/core/src/Pricing/DefaultPriceFormatter.php @@ -38,17 +38,17 @@ public function unitDecimal(bool $rounding = true): float return $rounding ? round($convertedValue, $this->currency->decimal_places) : $convertedValue; } - public function formatted(?string $locale = null, string $formatterStyle = NumberFormatter::CURRENCY, ?int $decimalPlaces = null, bool $trimTrailingZeros = true): mixed + public function formatted(?string $locale = null, int $formatterStyle = NumberFormatter::CURRENCY, ?int $decimalPlaces = null, bool $trimTrailingZeros = true): mixed { return $this->formatValue($this->decimal(false), $locale, $formatterStyle, $decimalPlaces, $trimTrailingZeros); } - public function unitFormatted(?string $locale = null, string $formatterStyle = NumberFormatter::CURRENCY, ?int $decimalPlaces = null, bool $trimTrailingZeros = true): mixed + public function unitFormatted(?string $locale = null, int $formatterStyle = NumberFormatter::CURRENCY, ?int $decimalPlaces = null, bool $trimTrailingZeros = true): mixed { return $this->formatValue($this->unitDecimal(false), $locale, $formatterStyle, $decimalPlaces, $trimTrailingZeros); } - protected function formatValue(int|float $value, ?string $locale = null, string $formatterStyle = NumberFormatter::CURRENCY, ?int $decimalPlaces = null, bool $trimTrailingZeros = true): mixed + protected function formatValue(int|float $value, ?string $locale = null, int $formatterStyle = NumberFormatter::CURRENCY, ?int $decimalPlaces = null, bool $trimTrailingZeros = true): mixed { if (! $locale) { $locale = App::currentLocale(); diff --git a/packages/core/src/Pricing/PriceFormatterInterface.php b/packages/core/src/Pricing/PriceFormatterInterface.php index ff1a203c6f..55893da255 100644 --- a/packages/core/src/Pricing/PriceFormatterInterface.php +++ b/packages/core/src/Pricing/PriceFormatterInterface.php @@ -10,7 +10,7 @@ public function decimal(bool $rounding = true): float; public function unitDecimal(bool $rounding = true): float; - public function formatted(?string $locale = null, string $formatterStyle = NumberFormatter::CURRENCY, ?int $decimalPlaces = null, bool $trimTrailingZeros = true): mixed; + public function formatted(?string $locale = null, int $formatterStyle = NumberFormatter::CURRENCY, ?int $decimalPlaces = null, bool $trimTrailingZeros = true): mixed; - public function unitFormatted(?string $locale = null, string $formatterStyle = NumberFormatter::CURRENCY, ?int $decimalPlaces = null, bool $trimTrailingZeros = true): mixed; + public function unitFormatted(?string $locale = null, int $formatterStyle = NumberFormatter::CURRENCY, ?int $decimalPlaces = null, bool $trimTrailingZeros = true): mixed; } diff --git a/packages/upgrade/src/Rector/LunarSetList.php b/packages/upgrade/src/Rector/LunarSetList.php index 308dec4216..35bf2d35a0 100644 --- a/packages/upgrade/src/Rector/LunarSetList.php +++ b/packages/upgrade/src/Rector/LunarSetList.php @@ -5,6 +5,7 @@ namespace Lunar\Upgrade\Rector; use Lunar\Upgrade\Rector\Models\RewriteModelClassCallRector; +use Lunar\Upgrade\Rector\Pricing\RetypeFormatterStyleParamRector; /** * Catalog of Rector renames contributed by v2 breaking specs. @@ -528,6 +529,7 @@ final class LunarSetList */ public const V1_TO_V2 = [ RewriteModelClassCallRector::class, + RetypeFormatterStyleParamRector::class, ]; /** diff --git a/packages/upgrade/src/Rector/Pricing/RetypeFormatterStyleParamRector.php b/packages/upgrade/src/Rector/Pricing/RetypeFormatterStyleParamRector.php new file mode 100644 index 0000000000..e5a52fe300 --- /dev/null +++ b/packages/upgrade/src/Rector/Pricing/RetypeFormatterStyleParamRector.php @@ -0,0 +1,86 @@ +> + */ + public function getNodeTypes(): array + { + return [ClassMethod::class]; + } + + /** + * @param ClassMethod $node + */ + public function refactor(Node $node): ?Node + { + if (! $node->name instanceof Identifier) { + return null; + } + + if (! in_array($node->name->toString(), self::METHODS, true)) { + return null; + } + + $changed = false; + + foreach ($node->params as $param) { + if (! $param->var instanceof Node\Expr\Variable) { + continue; + } + + if ($param->var->name !== self::PARAM_NAME) { + continue; + } + + if (! $param->type instanceof Identifier || $param->type->toString() !== 'string') { + continue; + } + + $param->type = new Identifier('int'); + $changed = true; + } + + return $changed ? $node : null; + } +} diff --git a/specs/0065-port-v1.5-fixes.md b/specs/completed/0065-port-v1.5-fixes.md similarity index 95% rename from specs/0065-port-v1.5-fixes.md rename to specs/completed/0065-port-v1.5-fixes.md index fb1ef6f6ae..16a106e83b 100644 --- a/specs/0065-port-v1.5-fixes.md +++ b/specs/completed/0065-port-v1.5-fixes.md @@ -1,6 +1,6 @@ # 0065 — Port the v1.5.0 fixes into v2 -- Status: draft +- Status: implemented - Author: Glenn - Created: 2026-08-26 - TODO item: Port the v1.5.0 fixes into v2 @@ -76,7 +76,7 @@ The four `processAutomaticRewards()` fixes are one textual stack and must land i ## Migration impact - **Database**: none. Every schema-adjacent 1.x change (nullable purchasable morph, weight-unit column) is already in the v2 baseline. -- **Public contract surface**: additive only — `Discount::scopeUsable()` gains an optional `$exempt` parameter, `Cart` gains `consumedDiscountIds()`/`forgetConsumedDiscountIds()`, `GetTaxZoneState::execute()` gains an optional `$countryId`, new stock events in `Lunar\Core\Events\Products`. `PriceFormatterInterface`'s `formatterStyle` retype is a signature change on a published interface; downstream implementations that copied the buggy `string` type need the same one-word fix (worth a Rector note in the upgrade package). +- **Public contract surface**: additive only — `Discount::scopeUsable()` gains an optional `$exempt` parameter, `Cart` gains `consumedDiscountIds()`/`forgetConsumedDiscountIds()`, `GetTaxZoneState::execute()` gains an optional `$countryId`, new stock events in `Lunar\Core\Events\Products`. `PriceFormatterInterface`'s `formatterStyle` retype is a signature change on a published interface; downstream implementations that copied the buggy `string` type would become a signature mismatch, so `RetypeFormatterStyleParamRector` (registered in `LunarSetList::V1_TO_V2`) retypes them. - **Upgrade path**: no new v1 → v2 data transformations. - **Translations**: four locale batches — `:unit` placeholder in table-rate-shipping `relationmanagers.php` (14 locales), `yes`/`no` in admin `global.php` (16), `save-variants.label` in filament `productoption.php` (16), `shipping:manage` permission label/description (16, location per item 24). Translate values per locale; no English placeholders. - **Filament / admin**: items 17–26 above; no new screens, all fixes to existing surfaces. Panel is unaffected except where noted (already correct or section not yet built — the orders/discounts panel sections should pick up items 21 and 25 as acceptance criteria when they land). @@ -102,13 +102,13 @@ Still open: ## Implementation plan -- [ ] Slice 1 — BuyXGetY: conditions guard + automatic-rewards stack (items 1–5, in order) -- [ ] Slice 2 — Discount memoisation: cache key + consumed-discount exemption (items 6–7) -- [ ] Slice 3 — Cart/tax/shipping core: currency reprice, state tax zones, manifest re-entrancy, `getDefault()` (items 8–10, 12–13) -- [ ] Slice 4 — Weight-unit tiers incl. 14 locales (item 11) -- [ ] Slice 5 — Core model fixes: deterministic ordering, one address per type (items 14–15) -- [ ] Slice 6 — Filament admin batch 1: trim, option mapping, media properties, country/state selects (items 17–20) -- [ ] Slice 7 — Filament admin batch 2: boolean meta, unit qty, widget labels, breadcrumb gaps, PDF invoice, incl. locales (items 21–23, 25–26) -- [ ] Slice 8 — Shipping permission label seam incl. 16 locales (item 24) -- [ ] Slice 9 — Stripe amount conversion + test pin (item 27) -- [ ] Slice 10 — Pricing formatter: `formatterStyle` retype + unit-qty clamp (item 28 + the core half of item 22) +- [x] Slice 1 — BuyXGetY: conditions guard + automatic-rewards stack (items 1–5, in order) +- [x] Slice 2 — Discount memoisation: cache key + consumed-discount exemption (items 6–7) +- [x] Slice 3 — Cart/tax/shipping core: currency reprice, state tax zones, manifest re-entrancy, `getDefault()` (items 8–10, 12–13) +- [x] Slice 4 — Weight-unit tiers incl. 14 locales (item 11) +- [x] Slice 5 — Core model fixes: deterministic ordering, one address per type (items 14–15) +- [x] Slice 6 — Filament admin batch 1: trim, option mapping, media properties, country/state selects (items 17–20) +- [x] Slice 7 — Filament admin batch 2: boolean meta, unit qty, widget labels, breadcrumb gaps, PDF invoice, incl. locales (items 21–23, 25–26) +- [x] Slice 8 — Shipping permission label seam incl. 16 locales (item 24) +- [x] Slice 9 — Stripe amount conversion + test pin (item 27) +- [x] Slice 10 — Pricing formatter: `formatterStyle` retype + unit-qty clamp (item 28 + the core half of item 22) diff --git a/tests/core/Unit/Pricing/DefaultPriceFormatterTest.php b/tests/core/Unit/Pricing/DefaultPriceFormatterTest.php index 047755d70b..3fdcf7746b 100644 --- a/tests/core/Unit/Pricing/DefaultPriceFormatterTest.php +++ b/tests/core/Unit/Pricing/DefaultPriceFormatterTest.php @@ -34,3 +34,19 @@ expect($formatter->unitDecimal())->toEqual(5.00); }); + +test('accepts a NumberFormatter style constant', function () { + $currency = Currency::factory()->create([ + 'code' => 'GBP', + 'decimal_places' => 2, + ]); + + // NumberFormatter::CURRENCY and friends are int constants; the parameter + // used to be typed string, so passing one tripped a coercion deprecation. + $formatter = new DefaultPriceFormatter(1500, $currency); + + expect($formatter->formatted(locale: 'en_GB', formatterStyle: NumberFormatter::DECIMAL)) + ->toEqual('15.00') + ->and($formatter->formatted(locale: 'en_GB', formatterStyle: NumberFormatter::CURRENCY)) + ->toEqual('£15.00'); +}); diff --git a/tests/upgrade/Unit/Rector/Pricing/Fixture/RetypeFormatterStyleParamRector/formatter_style_param.php.inc b/tests/upgrade/Unit/Rector/Pricing/Fixture/RetypeFormatterStyleParamRector/formatter_style_param.php.inc new file mode 100644 index 0000000000..169b799147 --- /dev/null +++ b/tests/upgrade/Unit/Rector/Pricing/Fixture/RetypeFormatterStyleParamRector/formatter_style_param.php.inc @@ -0,0 +1,41 @@ + +----- + diff --git a/tests/upgrade/Unit/Rector/Pricing/Fixture/RetypeFormatterStyleParamRector/leaves_unrelated_alone.php.inc b/tests/upgrade/Unit/Rector/Pricing/Fixture/RetypeFormatterStyleParamRector/leaves_unrelated_alone.php.inc new file mode 100644 index 0000000000..43bc196495 --- /dev/null +++ b/tests/upgrade/Unit/Rector/Pricing/Fixture/RetypeFormatterStyleParamRector/leaves_unrelated_alone.php.inc @@ -0,0 +1,24 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__.'/Fixture/RetypeFormatterStyleParamRector'); + } + + public function provideConfigFilePath(): string + { + return __DIR__.'/config/formatter_style.php'; + } +} diff --git a/tests/upgrade/Unit/Rector/Pricing/config/formatter_style.php b/tests/upgrade/Unit/Rector/Pricing/config/formatter_style.php new file mode 100644 index 0000000000..21859dbbdc --- /dev/null +++ b/tests/upgrade/Unit/Rector/Pricing/config/formatter_style.php @@ -0,0 +1,9 @@ +withRules([RetypeFormatterStyleParamRector::class]);