Add VAT Included line item for inclusive taxes - #1445
Conversation
🦋 Changeset detectedLatest commit: 6b0368c The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
wcole1-godaddy
left a comment
There was a problem hiding this comment.
Thanks for this — the feature is small and well-scoped, typecheck and the full @godaddy/react vitest suite pass on the branch, and all 21 locales have the new key. Requesting changes for the first two items below; the rest are non-blocking but worth addressing.
Should fix before merge
1. The additional exclusion is dead code on the checkout path, so cart and checkout can disagree.
getIncludedTaxTotal filters out additional === true, but the checkout schema's LineItemTax type has no additional field and DraftOrderQuery cannot select it — only the storefront OrderTax has it. Because TaxAmount.additional is optional, tsc never surfaces the gap. For an order with a tax that is both included and additional, the cart drawer excludes it and the checkout summary counts it. The unit test asserts a shape checkout can never produce.
Suggest either dropping the filter, or typing the helper against the real query result types (CartTax plus a DraftOrder['taxes'][number] alias) so the mismatch is visible, and documenting that the exclusion is storefront-only.
→ packages/react/src/components/checkout/totals/utils/get-included-tax-total.ts:16
2. The 502-line checkout-env.ts regen is unrelated to this PR.
It is insertion-only and adds tips, CheckoutSessionFee, calculateCheckoutSessionFees, FundingSourceType, etc. DraftOrder.taxes and LineItemTax already existed at the merge base, so the new taxes { ... } selection type-checks without it. The introspection is generated from a gitignored schema file, so reviewers can't reproduce it or confirm which endpoint it came from. Please revert that file here and land the schema sync as its own PR.
Should fix
3. The VAT row bypasses the enableTaxes switch.
"Estimated taxes" is gated by enableTaxes (derived in checkout-form from session.enableTaxCollection || taxTotal > 0); the new row is gated only on vatIncluded > 0. Two concrete effects: the library's own storefront Cart passes enableTaxes={false} to CartTotals while vatIncluded arrives via the spread, so the cart drawer now shows "VAT included" with no taxes row above it; and a merchant with tax collection disabled but tax-inclusive pricing gets a row they can't turn off except by passing vatIncluded={0}. If showing it in the cart is intentional, please say so in the description and add a dedicated enable prop.
→ packages/react/src/components/checkout/totals/totals.tsx:160
4. Stale VAT amount after tax/discount/shipping mutations.
isTaxLoading only tracks the pending updateDraftOrderTaxes mutation. Its onSuccess in use-update-taxes.ts patches totals only (and writes it under the key taxesTotal — pre-existing bug), and the discount/shipping mutations don't select taxes. So after an address change from a VAT region to a non-VAT one, the skeleton clears and the old VAT amount renders next to already-updated totals until the onSettled refetch lands. The skeleton also never shows on first appearance because vatIncluded is still 0 when loading starts.
5. Missing extension anchor.
Every sibling row has a <Target id='checkout.summary.totals.<row>.before' />; the new row doesn't, so UI extensions can't position relative to it.
Minor
- Exempted constituents are counted. An included tax with
exempted: truestill adds to the VAT row while "Estimated taxes" shows zero. Worth confirming the API never emits that shape, or filtering it. - Public prop name is jurisdiction-specific.
vatIncludedlands on the exportedDraftOrderTotalsProps/CartTotals, yet enAu labels it "GST included" and the API returns per-constituentname. ConsiderincludedTaxesfor the prop and keep the VAT wording only in the locale key — renaming later is a breaking change. - Tests are weaker than the description claims. The description lists four integration cases; the diff adds one test plus one negative assertion. That negative assertion runs against the fixture default
taxes: [], so it would pass if the row rendered whenever any tax exists.getAllByText('$1.23').length > 0can't fail on length (getAllByTextthrows on zero matches) and doesn't tie the amount to the VAT row. - Over-fetching. The new order-level
taxesselection inDraftOrderQueryrequestscurrencyCode,exempted,id,name,ratePercentage, none of which the client reads, and this query refetches on every window focus and draft-order mutation. - Nit. The
?? nullinuseDraftOrderIncludedTaxTotalis redundant (the helper acceptsundefined), and the cart call site omits it.
Summary
Adds a separate VAT Included line item to checkout and storefront order totals.
Inclusive taxes are already part of the displayed product price and are not included in
order.totals.taxTotal. As a result, they were not visible in the existing Estimated taxes row. Thischange derives the VAT amount from the order-level tax constituents and displays it separately without changing the amount due.
The VAT total:
amount.valuereturned by the tax calculation pipeline.included === true.additional === true.The checkout draft-order query now retrieves order-level tax constituent amounts. The storefront order query also retrieves the
additionalflag so additive taxes can be explicitly excluded.Localized labels were added for all supported locales, including GST Included for Australia.
Changeset
Test Plan
Added unit tests for inclusive tax total calculation:
Added checkout integration coverage verifying:
Verified the VAT row uses the existing tax-loading state.
Ran the following validation:
pnpm --filter @godaddy/localizations typecheck
pnpm --filter @godaddy/react typecheck
pnpm --filter @godaddy/react lint
pnpm --filter @godaddy/react exec vitest run src/components/checkout/totals/utils/get-included-tax-total.test.ts src/components/checkout/tests/checkout-totals-summary.test.tsx
All checks and 12 relevant tests passed.