Refetch shipping on coupon application - #1441
pbennett1-godaddy wants to merge 35 commits into
Conversation
Task: task-2
Task: task-1
Task: task-4
Task: task-7
🦋 Changeset detectedLatest commit: 92a0191 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Preserve tipping, VAT, billing, and payment updates alongside coupon shipping reconciliation. Keep initial shipping taxes and avoid duplicate fulfillment refreshes, with regression coverage.
wcole1-godaddy
left a comment
wcole1-godaddy
left a comment
There was a problem hiding this comment.
Nice work splitting the core mutations from their workflow side effects. The single tax call is well covered, and the request-id guard in the express coupon sync is a good fix for the duplicate PriceAdjustments calls. CI-equivalent checks pass locally (818 tests, typecheck, biome).
I'm requesting changes for two shipping-selection regressions I reproduced against main, plus two express checkout issues. Details are inline. A few items didn't fit inline:
- Free-shipping filter removal: Is the free-shipping minimum order total enforced server-side in every environment? If not, removing the
experimental_rules.freeShippingfilter means free shipping is offered below the threshold. DroppingfreeShippingfrom the queries also removes it from the publicCheckoutSessiontype, which is derived from the query, in a patch release. - PR description: The description mentions express visibility changes ("without depending on temporary
PURCHASEfulfillment state", plus digital-only/pickup visibility coverage), but they aren't in the diff. Please update the description or add the missing changes. - Unrelated schema changes:
checkout-env.tsaddsorderIdand acheckoutSession(id:)arg. These look unrelated, so please split them out or confirm they're intended.
| const existingMethod = currentFormMethod || currentServiceCode; | ||
| const isInitialSelection = lastShippingMethodsKeyRef.current === null; | ||
| const { selectedMethod: methodToApply, methodsKey } = | ||
| selectShippingMethod({ |
There was a problem hiding this comment.
Regression: a persisted shipping selection is overwritten on load. On first render lastShippingMethodsKeyRef.current is null, so selectShippingMethod treats the rates as changed and picks the cheapest method. It then mutates the order if the saved line differs.
To reproduce, load a draft order with weight-based ($1) already selected and the default rates. main sends no ApplyCheckoutSessionShippingMethod; this branch applies free-shipping. This affects reloads, returning from redirect payment flows (MercadoPago/CCAvenue), and any remount of this form.
The free-order test was rewritten to expect this behavior, and the new "preserves an existing rate" test only uses the cheapest rate, so it can't catch it.
Suggested fix: on first load, seed the key from the current methods so an existing service code is kept, e.g. previousMethodsKey: lastShippingMethodsKeyRef.current ?? getShippingMethodsKey(shippingMethods) when existingMethod is set.
| const availableMethods = sortShippingMethods(shippingMethods); | ||
| const methodsKey = getShippingMethodsKey(availableMethods); | ||
| const methodsChanged = methodsKey !== previousMethodsKey; | ||
| const selectedMethod = methodsChanged |
There was a problem hiding this comment.
An explicit customer choice is downgraded whenever any rate in the list changes. The key includes cost, so any repricing counts as a change and falls back to availableMethods[0].
To reproduce, the customer picks Express ($20 vs Standard $5), then edits the postal code so the rates become $21 / $6. main keeps Express; this branch switches to Standard. Carrier rates reprice on almost every address edit, so this will be common. The same thing happens when a coupon makes a different method cheaper.
Is that intended? If not, I'd keep the current method whenever it's still available and the customer chose it. Only fall back to the cheapest when the current method is gone, was auto-defaulted, or free shipping newly appears. One way is to track user selection in handleValueChange.
| // Start with the base line items | ||
| const baseLineItems = [...poyntExpressRequest.lineItems]; | ||
|
|
||
| // Refetch shipping methods so rates reflect the coupon change (e.g. free-shipping discounts) |
There was a problem hiding this comment.
I don't think this refetch can reflect the coupon. DraftOrderShippingRatesQuery takes only destination, and a wallet-entered coupon is never written to the draft order. It only feeds the read-only calculatedAdjustments query. So the rates come back the same as before.
Meanwhile this adds a round trip on every coupon change. Combined with the block at ~L847, it replaces the wallet's shippingMethods while the line items and total still use godaddyTotals.shipping.value / shippingMethod. If the wallet resets the selection to the first (cheapest) option, the displayed method and the charged amount diverge. I'd remove this block and the one at ~L847.
| const defaultMethod = sortShippingMethods(shippingMethodsData || [])[0]; | ||
|
|
||
| if (defaultMethod) { | ||
| setSelectedShippingRate({ |
There was a problem hiding this comment.
This can mutate selectedShippingRate outside a wallet event. shippingAddress is reset in handleClick and handleCancel, so this branch only runs while the sheet is open. When it does run, it changes the selected rate without resolving anything to the Express Checkout Element. handleConfirm would then submit a shippingTotal and method different from what the customer saw. Since Stripe ECE has no coupon entry, I'd drop the shipping refetch from this effect entirely.
| syncPriceAdjustments(); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [draftOrder, draftOrderDiscountCodes]); | ||
| }, [hasDraftOrder, discountCodesKey, couponSyncRevision]); |
There was a problem hiding this comment.
Keying only on the discount codes removes the duplicate requests, but calculatedAdjustmentsRef depends on the subtotal and shipping too. If the cart or shipping changes while the codes stay the same, the cached adjustments are stale (e.g. a percentage discount computed on the old subtotal). Consider adding totals.subTotal / shippingTotal values to the key. The same applies to the Stripe effect.
| result?.checkoutSession?.draftOrder?.calculatedShippingRates?.rates | ||
| ) | ||
| ) { | ||
| throw new Error('Shipping rates are unavailable'); |
There was a problem hiding this comment.
Two questions:
- Can the API return
calculatedShippingRates: nullfor a legitimate "no rates" case, e.g. a store with no shipping profile? If so, this now shows the failure/retry UI and clears the applied shipping instead of "No shipping methods found." - Since this is an intentional throw, please set
retry: falsehere explicitly. Hosts can pass their ownQueryClient, and TanStack's default of 3 retries means ~7s of skeleton, plususeReconcileAfterDiscountawaits the refetch while the coupon button spins.
| ); | ||
| } | ||
|
|
||
| const allCodes = new Set<string>(); |
There was a problem hiding this comment.
Pre-existing, but easy to fix now: this re-applies only order and shipping-line codes. The discount mutation replaces the whole list (DiscountStandalone sends the full set, and removal sends []), so an order with both an order-level code and a line-item code loses the line-item code on any shipping change. getDraftOrderDiscountCodes(order) would fix it. DiscountStandalone could use the same helper.
| }, | ||
| }, | ||
| ], | ||
| shippingLines: [], |
There was a problem hiding this comment.
This hook isn't used anywhere. It's still edited here, and it calls useDiscountApply, which now runs useReconcileAfterDiscount. That could re-apply a shipping method right after it's removed. I'd delete it, or switch it to useApplyDiscountCore.
|
|
||
| Fix billing collection, shipping reconciliation, and discount/coupon sync across checkout flows. | ||
|
|
||
| - Align billing fields and validation for paid, free, pickup, shipping, purchase, and digital orders. |
| 'Geben Sie Ihre Adresse ein, um verfügbare Versandmethoden zu sehen.', | ||
| noShippingMethods: 'Keine Versandmethoden gefunden.', | ||
| failedToLoadMethods: | ||
| 'Versandarten konnten nicht geladen werden. Bitte versuche es erneut.', |
There was a problem hiding this comment.
Nit: the rest of deDe uses formal address ("Sie"). Suggest 'Versandarten konnten nicht geladen werden. Bitte versuchen Sie es erneut.'
Keep a saved or customer-chosen shipping method while it is still offered, including on load, and only fall back to the cheapest rate when it is gone, was picked automatically and rates changed, or free shipping newly appears. Re-apply line-item discount codes on shipping changes, stop the Stripe express sheet from refetching and reselecting shipping on coupon changes, use the highest-value code for express wallets, and recompute cached coupon adjustments when the subtotal changes. Keep the unused remove-shipping hook from triggering reconciliation, disable automatic retries on the shipping rates query, trim the changeset, and use formal German copy. Add tests for the selection rules, failed-reapply recovery, discount cache matching, and the GoDaddy and Stripe express coupon sync. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 participants
Summary
Ensure discount changes correctly reconcile shipping rates and taxes without duplicate requests or stale totals.
Discounts can change shipping eligibility, including enabling or disabling free shipping. The checkout now refetches shipping methods after a successful discount change when a valid shipping destination is available. It determines whether the returned rates require shipping reconciliation and assigns the final tax calculation to either the discount or shipping workflow, ensuring taxes run exactly once.
This PR also improves shipping-method selection and express checkout discount synchronization.
Key changes
experimental_rules.freeShippingclient-side filtering and query fields; the shipping API is authoritative for rate eligibility.PriceAdjustmentsrequests from unrelated draft-order updates.Changeset
Test Plan
Validation completed:
pnpm --filter @godaddy/react typecheckpnpm --filter @godaddy/react test(79 test files, 840 tests passed)biome checkon changed files🤖 Generated with Claude Code