From ff111221a55607ae9b083b45c07d2d8e76acf3c5 Mon Sep 17 00:00:00 2001 From: Alessandro Casazza Date: Thu, 27 Aug 2026 19:40:41 +0200 Subject: [PATCH 1/7] fix(adyen): send shopper_locale so redirect pages match the Drop-in language MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port of ddfb9a7a, which fixed this on the v4 line and never reached v5. Adyen uses two locales and only one of them was travelling: - `locale` in the Core configuration is client-side only — it selects the Drop-in's translation bundle. - the shopper locale in the payment request is what Adyen uses for the pages it renders itself, in particular the hosted page a redirect method sends the shopper to. `payment_request_data` carried `payment_method`, `return_url`, `origin`, `redirect_from_issuer_method`, `shopper_ip` and `browser_info`, but no shopper locale, so Adyen fell back to the merchant account default or the country code. A Drop-in correctly rendered in English then handed over to a Klarna page in Italian for an IT market. `shopper_locale` is now sent, derived from the same value that drives the Drop-in. Commerce Layer's `language_code` is a bare ISO 639-1 code, so `getAdyenShopperLocale` expands it to the `language-REGION` form Adyen expects (`en` -> `en-US`, `it` -> `it-IT`). When a language cannot be expanded confidently it returns undefined and the field is omitted, preserving Adyen's current fallback rather than sending a locale it may reject. `ca` is expanded too, which ddfb9a7a missed. The attribute is snake_case, like the other Commerce Layer attributes in that payload — it is the API that maps them onto Adyen's camelCase names. ddfb9a7a sent `shopperLocale` instead, which is the name Adyen uses but not the one this payload takes. `AdyenPaymentConfig.shopperLocale` overrides the derived value, camelCase as a component option. Unlike in ddfb9a7a it also moves the Drop-in's own `locale`: an integration that sets it would otherwise get a Klarna page in Italian behind a Drop-in in English, which is the original mismatch chosen on purpose. Nothing changes for an integration that does not set it. Co-Authored-By: Claude Opus 5 (1M context) --- .../AdyenPayment.shopperLocale.spec.tsx | 210 ++++++++++++++++++ .../specs/utils/adyenShopperLocale.spec.ts | 36 +++ .../payment_source/AdyenPayment.tsx | 38 +++- .../src/utils/adyenShopperLocale.ts | 83 +++++++ 4 files changed, 366 insertions(+), 1 deletion(-) create mode 100644 packages/react-components/specs/payment_source/AdyenPayment.shopperLocale.spec.tsx create mode 100644 packages/react-components/specs/utils/adyenShopperLocale.spec.ts create mode 100644 packages/react-components/src/utils/adyenShopperLocale.ts diff --git a/packages/react-components/specs/payment_source/AdyenPayment.shopperLocale.spec.tsx b/packages/react-components/specs/payment_source/AdyenPayment.shopperLocale.spec.tsx new file mode 100644 index 00000000..ee9a45d5 --- /dev/null +++ b/packages/react-components/specs/payment_source/AdyenPayment.shopperLocale.spec.tsx @@ -0,0 +1,210 @@ +// The Drop-in was translated correctly but the Klarna page it redirects to came up in Italian. +// `locale` in the Core configuration is client-side only — it picks the Drop-in's translation +// bundle and never reaches Adyen. The language Adyen uses for the hosted pages it renders +// itself comes from `shopperLocale` in the payment request, which was not being sent. +import { act, render } from "@testing-library/react" +import { AdyenPayment } from "#components/payment_source/AdyenPayment" +import CommerceLayerContext from "#context/CommerceLayerContext" +import CustomerContext from "#context/CustomerContext" +import OrderContext, { defaultOrderContext } from "#context/OrderContext" +import PaymentMethodContext, { defaultPaymentMethodContext } from "#context/PaymentMethodContext" +import PlaceOrderContext, { defaultPlaceOrderContext } from "#context/PlaceOrderContext" + +const adyen = vi.hoisted(() => ({ + // biome-ignore lint/suspicious/noExplicitAny: test cast + captured: { options: null as any }, +})) + +vi.mock("@adyen/adyen-web/auto", () => ({ + // biome-ignore lint/suspicious/noExplicitAny: test cast + AdyenCheckout: vi.fn(async (options: any) => { + adyen.captured.options = options + return { update: vi.fn() } + }), + Dropin: class FakeDropin { + mount(): this { + return this + } + submit(): void {} + remove(): void {} + unmount(): this { + return this + } + handleAction(): void {} + }, +})) + +vi.mock("#utils/getPublicIp", () => ({ + getPublicIP: vi.fn(async () => "127.0.0.1"), +})) + +const PAYMENT_SOURCE = { + id: "ps-1", + type: "adyen_payments", + payment_methods: { + paymentMethods: [{ type: "scheme" }, { type: "klarna_account" }], + }, +} + +/** + * Mounts the component for an order in `languageCode`, submits a card, and returns the + * `payment_request_data` that went to the API. + */ +async function submitAndCapturePaymentRequest({ + languageCode, + shopperLocaleConfig, +}: { + languageCode?: string + shopperLocaleConfig?: string + // biome-ignore lint/suspicious/noExplicitAny: test cast +}): Promise<{ paymentRequestData: any; dropInLocale: string }> { + const setPaymentSource = vi.fn(async () => ({ + ...PAYMENT_SOURCE, + payment_response: {}, + })) + // biome-ignore lint/suspicious/noExplicitAny: test cast + const order: any = { + id: "order-1", + status: "pending", + payment_status: "unpaid", + currency_code: "EUR", + country_code: "IT", + language_code: languageCode, + total_amount_with_taxes_cents: 1000, + line_items: [], + } + + await act(async () => { + render( + + + + + + + + + + + + ) + }) + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + + await act(async () => { + adyen.captured.options.onSubmit( + { data: { paymentMethod: { type: "scheme" } }, isValid: true }, + { mount: vi.fn() }, + { resolve: vi.fn(), reject: vi.fn() } + ) + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + + const call = setPaymentSource.mock.calls + // biome-ignore lint/suspicious/noExplicitAny: test cast + .map(([args]: any[]) => args) + // biome-ignore lint/suspicious/noExplicitAny: test cast + .find((args: any) => args?.attributes?.payment_request_data != null) + return { + paymentRequestData: call?.attributes?.payment_request_data, + dropInLocale: adyen.captured.options.locale, + } +} + +describe("AdyenPayment shopperLocale", () => { + beforeEach(() => { + vi.clearAllMocks() + adyen.captured.options = null + }) + + it("sends shopperLocale derived from the order language", async () => { + const { paymentRequestData, dropInLocale } = await submitAndCapturePaymentRequest({ + languageCode: "en", + }) + + // The Drop-in keeps the bare language it always used — this is only about what Adyen gets. + expect(dropInLocale).toBe("en") + expect(paymentRequestData.shopper_locale).toBe("en-US") + }) + + it("does not take the language from the country code", async () => { + // The bug: an English order in an Italian market rendered a Klarna page in Italian, because + // nothing carried the language and Adyen fell back to the country. + const { paymentRequestData } = await submitAndCapturePaymentRequest({ + languageCode: "en", + }) + + expect(paymentRequestData.shopper_locale).not.toContain("it") + }) + + it("lets the config override the derived value", async () => { + const { paymentRequestData } = await submitAndCapturePaymentRequest({ + languageCode: "en", + shopperLocaleConfig: "en-GB", + }) + + expect(paymentRequestData.shopper_locale).toBe("en-GB") + }) + + it("moves the Drop-in with it, so the two locales cannot disagree", async () => { + const { paymentRequestData, dropInLocale } = await submitAndCapturePaymentRequest({ + languageCode: "en", + shopperLocaleConfig: "it-IT", + }) + + // One locale for both surfaces. Without this an integration that sets the option would + // get a Klarna page in Italian behind a Drop-in in English: the same mismatch as the + // original bug, only chosen on purpose. + expect(dropInLocale).toBe("it-IT") + expect(paymentRequestData.shopper_locale).toBe("it-IT") + }) + + it("omits shopperLocale when the language cannot be expanded", async () => { + const { paymentRequestData } = await submitAndCapturePaymentRequest({ + languageCode: "xx", + }) + + // Preserves Adyen's existing fallback rather than sending something it may reject. + expect(paymentRequestData).not.toHaveProperty("shopper_locale") + }) + + it("falls back to the locale prop when the order has no language", async () => { + const { paymentRequestData, dropInLocale } = await submitAndCapturePaymentRequest({ + languageCode: undefined, + }) + + // Same source the Drop-in falls back to, so the two stay consistent. + expect(dropInLocale).toBe("en_US") + expect(paymentRequestData.shopper_locale).toBe("en-US") + }) +}) diff --git a/packages/react-components/specs/utils/adyenShopperLocale.spec.ts b/packages/react-components/specs/utils/adyenShopperLocale.spec.ts new file mode 100644 index 00000000..5ffb31c2 --- /dev/null +++ b/packages/react-components/specs/utils/adyenShopperLocale.spec.ts @@ -0,0 +1,36 @@ +// Adyen's `locale` (Drop-in translations, client-side) and `shopperLocale` (payment request, +// used for the hosted pages Adyen renders itself) are different things. Commerce Layer's +// `order.language_code` is a bare ISO 639-1 code, so it has to be expanded for the latter. +import { getAdyenShopperLocale } from "#utils/adyenShopperLocale" + +describe("getAdyenShopperLocale", () => { + it("expands a bare language whose region mirrors it", () => { + expect(getAdyenShopperLocale("it")).toBe("it-IT") + expect(getAdyenShopperLocale("de")).toBe("de-DE") + expect(getAdyenShopperLocale("fr")).toBe("fr-FR") + expect(getAdyenShopperLocale("nl")).toBe("nl-NL") + }) + + it("expands a bare language whose region differs", () => { + // The reported case: an English Drop-in handing over to a Klarna page. `en-EN` is not a + // locale, so it needs the explicit mapping. + expect(getAdyenShopperLocale("en")).toBe("en-US") + expect(getAdyenShopperLocale("sv")).toBe("sv-SE") + expect(getAdyenShopperLocale("cs")).toBe("cs-CZ") + expect(getAdyenShopperLocale("ja")).toBe("ja-JP") + }) + + it("normalises a locale that already carries a region", () => { + expect(getAdyenShopperLocale("en-US")).toBe("en-US") + expect(getAdyenShopperLocale("en_US")).toBe("en-US") + expect(getAdyenShopperLocale("PT_br")).toBe("pt-BR") + }) + + it("returns undefined rather than guessing", () => { + // Preserves Adyen's own fallback instead of sending a locale it may reject. + expect(getAdyenShopperLocale(undefined)).toBeUndefined() + expect(getAdyenShopperLocale(null)).toBeUndefined() + expect(getAdyenShopperLocale("")).toBeUndefined() + expect(getAdyenShopperLocale("xx")).toBeUndefined() + }) +}) diff --git a/packages/react-components/src/components/payment_source/AdyenPayment.tsx b/packages/react-components/src/components/payment_source/AdyenPayment.tsx index 63aabf7d..4592a54f 100644 --- a/packages/react-components/src/components/payment_source/AdyenPayment.tsx +++ b/packages/react-components/src/components/payment_source/AdyenPayment.tsx @@ -23,6 +23,7 @@ import CustomerContext from "#context/CustomerContext" import OrderContext from "#context/OrderContext" import PaymentMethodContext from "#context/PaymentMethodContext" import PlaceOrderContext from "#context/PlaceOrderContext" +import { getAdyenShopperLocale } from "#utils/adyenShopperLocale" import browserInfo, { cleanUrlBy } from "#utils/browserInfo" import { getPublicIP } from "#utils/getPublicIp" import { hasSubscriptions } from "#utils/hasSubscriptions" @@ -96,6 +97,21 @@ export interface AdyenPaymentConfig { */ onSelect?: (component: UIElement) => void giftcardErrorComponent?: (message: string) => JSX.Element + /** + * The locale Adyen should use for anything **it** renders — in particular the hosted page a + * redirect payment method sends the shopper to (Klarna, iDEAL, …). Sent as `shopper_locale` + * in the payment request, and it also selects the Drop-in's own translations. + * + * Without it the locale is derived from `order.language_code`, and Adyen falls back to the + * merchant account default or the country code when that language cannot be expanded — which + * is how a Drop-in in English ends up handing over to a Klarna page in Italian. + * + * Read once when the Drop-in is built: changing it on a mounted Drop-in has no effect. + * + * @default derived from `order.language_code` (see `getAdyenShopperLocale`) + * @example "en-US" + */ + shopperLocale?: string } interface Props { @@ -123,6 +139,7 @@ export function AdyenPayment({ onReady, onSelect, subscriptionPaymentMethods, + shopperLocale: shopperLocaleConfig, } = { ...defaultConfig, ...config, @@ -147,6 +164,19 @@ export function AdyenPayment({ const authConfig = useContext(CommerceLayerContext) const { placeOrderButtonRef, setPlaceOrder, status } = useContext(PlaceOrderContext) const { customers } = useContext(CustomerContext) + // Two distinct locales that Adyen does not treat as interchangeable, deliberately derived + // from one source. `dropInLocale` goes into the Core configuration and is client-side only: + // it picks the Drop-in's translation bundle. `shopperLocale` travels with the payment + // request and is what Adyen uses for the pages it renders itself, which is why the two must + // not be allowed to disagree — a Drop-in in English handing over to a Klarna page in Italian + // is the bug this fixes. + // + // The config value is read here rather than only in the request, so an integration that + // sets it moves both. `getAdyenShopperLocale` then normalizes it (`en_US`, `PT_br`) and + // expands a bare `language_code` into the `language-REGION` form the payment request needs, + // returning undefined — and so omitting the field — rather than guessing. + const dropInLocale = shopperLocaleConfig ?? order?.language_code ?? locale + const shopperLocale = getAdyenShopperLocale(dropInLocale) const ref = useRef(null) const dropinRef = useRef(null) // The Core instance, kept alongside the Drop-in: refreshing the amount after a partial @@ -350,6 +380,12 @@ export function AdyenPayment({ redirect_from_issuer_method: "GET", shopper_ip: shopperIp, shopperInteraction: "Ecommerce", + // The language Adyen renders its own hosted pages in (the Klarna screen a redirect + // method hands over to). The Drop-in's `locale` is client-side only and never reaches + // Adyen, so without this the hosted page falls back to the account default or the + // country code. snake_case because these are Commerce Layer attributes, and it is the + // API that maps them onto Adyen's own camelCase names. + ...(shopperLocale != null ? { shopper_locale: shopperLocale } : {}), browser_info: { ...browserInfo(), }, @@ -627,7 +663,7 @@ export function AdyenPayment({ : paymentMethodsResponse.paymentMethods } const options = { - locale: order?.language_code ?? locale, + locale: dropInLocale, environment, clientKey, amount: { diff --git a/packages/react-components/src/utils/adyenShopperLocale.ts b/packages/react-components/src/utils/adyenShopperLocale.ts new file mode 100644 index 00000000..d73b20ef --- /dev/null +++ b/packages/react-components/src/utils/adyenShopperLocale.ts @@ -0,0 +1,83 @@ +/** + * Adyen uses two different locales, and they are not interchangeable: + * + * - `locale` in the Core configuration is **client-side only** — it picks the Drop-in's + * translation bundle. + * - `shopperLocale` in the payment request is what Adyen uses for anything it renders + * itself, in particular the hosted pages a redirect payment method sends the shopper to + * (Klarna, iDEAL, …). Without it Adyen falls back to the merchant account default or the + * country code, which is why a Drop-in in English could hand over to a Klarna page in + * Italian. + * + * Adyen expects a language code combined with a region (`en-US`, `it-IT`). Commerce Layer's + * `order.language_code` is a bare ISO 639-1 code (`en`, `it`), so it has to be expanded. + */ + +/** + * Region to pair with a bare language code. Only languages whose Adyen-supported locale + * cannot be derived by uppercasing the language itself need an entry here. + */ +const REGION_BY_LANGUAGE: Record = { + en: "US", + zh: "CN", + ar: "AE", + he: "IL", + ja: "JP", + ko: "KR", + uk: "UA", + el: "GR", + cs: "CZ", + da: "DK", + sv: "SE", + nb: "NO", + no: "NO", + sl: "SI", + et: "EE", + be: "BY", + ca: "ES", +} + +/** Languages for which `xx` → `xx-XX` is the Adyen-supported locale (it → it-IT, and so on). */ +const SELF_REGION_LANGUAGES = new Set([ + "it", + "de", + "fr", + "es", + "pt", + "nl", + "pl", + "fi", + "hu", + "ro", + "ru", + "sk", + "tr", + "hr", + "lt", + "lv", + "bg", + "is", +]) + +/** + * Resolves the `shopperLocale` to send with an Adyen payment request. + * + * @param locale the locale already driving the Drop-in — `order.language_code`, or the + * component's `locale` prop. Accepts a bare language (`en`), or a language and region in + * either separator (`en-US`, `en_US`). + * @returns an Adyen-style `language-REGION` locale, or `undefined` when the language cannot + * be expanded confidently. Returning `undefined` deliberately preserves Adyen's own + * fallback rather than sending a locale it may reject. + */ +export function getAdyenShopperLocale(locale?: string | null): string | undefined { + if (locale == null) return undefined + const [rawLanguage, rawRegion] = locale.replace("_", "-").split("-") + const language = rawLanguage?.toLowerCase() + if (!language) return undefined + // Already carries a region: normalise the separator and casing and trust it. + if (rawRegion) return `${language}-${rawRegion.toUpperCase()}` + const region = + REGION_BY_LANGUAGE[language] ?? + (SELF_REGION_LANGUAGES.has(language) ? language.toUpperCase() : undefined) + return region ? `${language}-${region}` : undefined +} From c7ec008238857603fd374ca96550a2655b9a493e Mon Sep 17 00:00:00 2001 From: Alessandro Casazza Date: Thu, 27 Aug 2026 19:42:10 +0200 Subject: [PATCH 2/7] chore(adyen): spell the default locale the way Adyen documents it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things the shopperLocale work walked past. The `locale` prop defaulted to `en_US`. Adyen's client normalizes that — it replaces the underscore and then matches against its supported list — so it worked, but the payment request has no such normalizer, and the same string in both places would not have. It is now a named `DEFAULT_LOCALE` in the `language-REGION` form, which is also Adyen's own fallback value. `AdyenGateway` cast `order.language_code` to `StripeElementLocale`, importing a Stripe type to feed an Adyen prop. It compiled only because that prop is `string`. `language_code` is `string | null` and the prop is `string | undefined`, so the cast was laundering the null: `?? undefined` does that without claiming the value is something it is not. Co-Authored-By: Claude Opus 5 (1M context) --- .../payment_source/AdyenPayment.shopperLocale.spec.tsx | 9 +++++---- .../src/components/payment_gateways/AdyenGateway.tsx | 3 +-- .../src/components/payment_source/AdyenPayment.tsx | 7 ++++++- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/react-components/specs/payment_source/AdyenPayment.shopperLocale.spec.tsx b/packages/react-components/specs/payment_source/AdyenPayment.shopperLocale.spec.tsx index ee9a45d5..dc02218b 100644 --- a/packages/react-components/specs/payment_source/AdyenPayment.shopperLocale.spec.tsx +++ b/packages/react-components/specs/payment_source/AdyenPayment.shopperLocale.spec.tsx @@ -1,7 +1,8 @@ // The Drop-in was translated correctly but the Klarna page it redirects to came up in Italian. // `locale` in the Core configuration is client-side only — it picks the Drop-in's translation // bundle and never reaches Adyen. The language Adyen uses for the hosted pages it renders -// itself comes from `shopperLocale` in the payment request, which was not being sent. +// itself comes from the shopper locale in the payment request — `shopper_locale` in Commerce +// Layer's attributes — which was not being sent. import { act, render } from "@testing-library/react" import { AdyenPayment } from "#components/payment_source/AdyenPayment" import CommerceLayerContext from "#context/CommerceLayerContext" @@ -147,7 +148,7 @@ describe("AdyenPayment shopperLocale", () => { adyen.captured.options = null }) - it("sends shopperLocale derived from the order language", async () => { + it("sends shopper_locale derived from the order language", async () => { const { paymentRequestData, dropInLocale } = await submitAndCapturePaymentRequest({ languageCode: "en", }) @@ -189,7 +190,7 @@ describe("AdyenPayment shopperLocale", () => { expect(paymentRequestData.shopper_locale).toBe("it-IT") }) - it("omits shopperLocale when the language cannot be expanded", async () => { + it("omits shopper_locale when the language cannot be expanded", async () => { const { paymentRequestData } = await submitAndCapturePaymentRequest({ languageCode: "xx", }) @@ -204,7 +205,7 @@ describe("AdyenPayment shopperLocale", () => { }) // Same source the Drop-in falls back to, so the two stay consistent. - expect(dropInLocale).toBe("en_US") + expect(dropInLocale).toBe("en-US") expect(paymentRequestData.shopper_locale).toBe("en-US") }) }) diff --git a/packages/react-components/src/components/payment_gateways/AdyenGateway.tsx b/packages/react-components/src/components/payment_gateways/AdyenGateway.tsx index 3e333260..fd905795 100644 --- a/packages/react-components/src/components/payment_gateways/AdyenGateway.tsx +++ b/packages/react-components/src/components/payment_gateways/AdyenGateway.tsx @@ -1,4 +1,3 @@ -import type { StripeElementLocale } from "@stripe/stripe-js" import { type JSX, useContext, useRef } from "react" import type { GatewayBaseType } from "#components/payment_gateways/PaymentGateway" import AdyenPayment from "#components/payment_source/AdyenPayment" @@ -64,7 +63,7 @@ export function AdyenGateway(props: Props): JSX.Element | null { } const adyenSessionKey = readyAdyenSessionKey.current ?? undefined const paymentResource: PaymentResource = "adyen_payments" - const locale = order?.language_code as StripeElementLocale + const locale = order?.language_code ?? undefined if (!readonly && payment?.id !== currentPaymentMethodId) return null // @ts-expect-error no type const clientKey = paymentSource?.public_key diff --git a/packages/react-components/src/components/payment_source/AdyenPayment.tsx b/packages/react-components/src/components/payment_source/AdyenPayment.tsx index 4592a54f..5e703623 100644 --- a/packages/react-components/src/components/payment_source/AdyenPayment.tsx +++ b/packages/react-components/src/components/payment_source/AdyenPayment.tsx @@ -122,6 +122,11 @@ interface Props { environment?: CoreConfiguration["environment"] } +// Adyen's own fallback when a locale does not resolve, spelled the way Adyen documents it: +// `language-REGION`. It used to read `en_US` here, which their client normalizes but their +// payment request does not. +const DEFAULT_LOCALE = "en-US" + const defaultConfig: AdyenPaymentConfig = {} export function AdyenPayment({ @@ -129,7 +134,7 @@ export function AdyenPayment({ config, templateCustomerSaveToWallet, environment = "test", - locale = "en_US", + locale = DEFAULT_LOCALE, }: Props): JSX.Element | null { const { cardContainerClassName, From 58844107f93dbc292228f3ddf63fa2ab80143f46 Mon Sep 17 00:00:00 2001 From: Alessandro Casazza Date: Thu, 27 Aug 2026 19:57:24 +0200 Subject: [PATCH 3/7] fix(adyen): send shopper_interaction in snake_case like its neighbours MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `payment_request_data` is a Commerce Layer payload and the API maps its snake_case keys onto Adyen's camelCase names. `shopperInteraction` was written with Adyen's own spelling, so it went into the payload under a name that side does not take — the same mistake the shopper locale nearly shipped with. `Ecommerce` is Adyen's own default for this field, so a payment that has been working was very likely not relying on it arriving. It arrives now. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/components/payment_source/AdyenPayment.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-components/src/components/payment_source/AdyenPayment.tsx b/packages/react-components/src/components/payment_source/AdyenPayment.tsx index 5e703623..8a89e403 100644 --- a/packages/react-components/src/components/payment_source/AdyenPayment.tsx +++ b/packages/react-components/src/components/payment_source/AdyenPayment.tsx @@ -384,12 +384,12 @@ export function AdyenPayment({ origin: window.location.origin, redirect_from_issuer_method: "GET", shopper_ip: shopperIp, - shopperInteraction: "Ecommerce", + shopper_interaction: "Ecommerce", // The language Adyen renders its own hosted pages in (the Klarna screen a redirect // method hands over to). The Drop-in's `locale` is client-side only and never reaches // Adyen, so without this the hosted page falls back to the account default or the - // country code. snake_case because these are Commerce Layer attributes, and it is the - // API that maps them onto Adyen's own camelCase names. + // country code. snake_case, like every attribute in this payload: it is the API that + // maps them onto Adyen's own camelCase names. ...(shopperLocale != null ? { shopper_locale: shopperLocale } : {}), browser_info: { ...browserInfo(), From 819280f18a9470ca65a33de655c7fe49010a0957 Mon Sep 17 00:00:00 2001 From: Alessandro Casazza Date: Fri, 28 Aug 2026 18:32:11 +0200 Subject: [PATCH 4/7] fix(place-order): keep the button disabled until privacy & terms are accepted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selecting a payment method could enable `PlaceOrderButton` while the privacy & terms checkbox was still unchecked. Acceptance travelled from `` to `PlaceOrderButton` through `localStorage["privacy-terms"]`, and the write that happens at *mount* notified nobody. A `"true"` left behind by an earlier visit — the unmount cleanup only runs on a React unmount, never on a tab close or a hard navigation — was read by `PlaceOrderContainer` as `isPermitted: true`. The checkbox then mounted and correctly wrote `"false"`, but none of the container's effect dependencies changed, so `isPermitted` stayed stale. The button's own effect does re-run on `paymentSource?.id`, i.e. exactly when a payment method is picked, and it read that stale value. Acceptance now lives in a module-level store keyed by order id, which notifies its subscribers. It is deliberately not persisted: a reload starts from "not accepted", so what the shopper sees can no longer diverge from what gates the button. - add `termsAcceptanceStore`: in-memory, per-order, notifying - add the public `useTermsAndConditions()` hook, so a custom consent control has a supported channel now that `localStorage` is gone - `placeOrderPermitted` takes `termsAccepted` as a parameter instead of reading a global, and reports `termsBlocking` - warn in development when acceptance is required but no control collects it, from an effect so a late-mounting checkbox cannot raise a false alarm - drop `PLACE_ORDER_RECHECK_EVENT`, made redundant by the store - drop a dead `isFree && !isPermitted` line: `setNotPermitted` is a state setter, so the branches below always overwrote it in the same effect pass Two existing tests claimed to cover this and covered nothing: the suite mocks `getCardDetails` to `{ brand: "" }`, which already falsifies the first factor of the enabling condition, so they passed on the wrong factor and would have passed with the gate deleted. Both now run with the condition live. Co-Authored-By: Claude Opus 5 (1M context) --- .../specs/orders/place-order.spec.tsx | 311 ++++++------- .../specs/orders/terms-acceptance.spec.tsx | 420 ++++++++++++++++++ .../components/orders/PlaceOrderButton.tsx | 6 +- .../components/orders/PlaceOrderContainer.tsx | 33 +- .../orders/PrivacyAndTermsCheckbox.tsx | 32 +- .../src/hooks/usePlaceOrder.ts | 55 +-- .../src/hooks/useTermsAndConditions.ts | 58 +++ packages/react-components/src/index.ts | 1 + .../src/reducers/PlaceOrderReducer.ts | 29 +- .../hooks/useMissingTermsCheckboxWarning.ts | 32 ++ .../src/utils/termsAcceptanceStore.ts | 105 +++++ 11 files changed, 868 insertions(+), 214 deletions(-) create mode 100644 packages/react-components/specs/orders/terms-acceptance.spec.tsx create mode 100644 packages/react-components/src/hooks/useTermsAndConditions.ts create mode 100644 packages/react-components/src/utils/hooks/useMissingTermsCheckboxWarning.ts create mode 100644 packages/react-components/src/utils/termsAcceptanceStore.ts diff --git a/packages/react-components/specs/orders/place-order.spec.tsx b/packages/react-components/specs/orders/place-order.spec.tsx index 4092b84b..c3ecb454 100644 --- a/packages/react-components/specs/orders/place-order.spec.tsx +++ b/packages/react-components/specs/orders/place-order.spec.tsx @@ -9,7 +9,13 @@ import CustomerContext from "#context/CustomerContext" import OrderContext, { defaultOrderContext } from "#context/OrderContext" import PaymentMethodContext, { defaultPaymentMethodContext } from "#context/PaymentMethodContext" import PlaceOrderContext, { defaultPlaceOrderContext } from "#context/PlaceOrderContext" -import { PLACE_ORDER_RECHECK_EVENT, usePlaceOrder } from "#hooks/usePlaceOrder" +import { usePlaceOrder } from "#hooks/usePlaceOrder" +import { + getAcceptedSnapshot, + getCheckboxCount, + resetTermsAcceptanceStore, + setAccepted, +} from "#utils/termsAcceptanceStore" vi.mock("@commercelayer/core-components", async (importOriginal) => { const actual = await importOriginal() @@ -386,8 +392,13 @@ describe("PlaceOrderButton (standalone)", () => { }) it("stays disabled when paymentMethodErrors clear but privacy/terms checkbox is not checked", async () => { - localStorage.clear() - // Order with privacy/terms URLs — checkbox NOT checked (nothing in localStorage) + resetTermsAcceptanceStore() + // A card IS selected, so `card.brand` is truthy and the enabling condition is + // live — without this the test passes on the wrong factor and would still pass + // with the privacy/terms gate deleted entirely. + const getCardDetails = (await import("#utils/getCardDetails")).default + vi.mocked(getCardDetails).mockReturnValue({ brand: "visa" } as never) + // Order with privacy/terms URLs — nothing accepted in the store const order = { ...MOCK_ORDER, privacy_url: "https://example.com/privacy", @@ -424,10 +435,11 @@ describe("PlaceOrderButton (standalone)", () => { await waitFor(() => { expect(screen.getByRole("button").hasAttribute("disabled")).toBe(true) }) + vi.mocked(getCardDetails).mockReturnValue({ brand: "" } as never) }) it("stays disabled when no payment method is selected even if errors clear", async () => { - localStorage.clear() + resetTermsAcceptanceStore() const orderNoPayment = { ...MOCK_ORDER, payment_method: null, @@ -536,20 +548,66 @@ describe("PlaceOrderButton (container mode)", () => { // --------------------------------------------------------------------------- describe("PlaceOrderButton (free order)", () => { - it("is enabled for free order when permitted", async () => { + beforeEach(() => { + resetTermsAcceptanceStore() + }) + + // A real free order carries no payment method at all. + // biome-ignore lint/suspicious/noExplicitAny: test cast + const FREE_NO_PAYMENT: any = { + ...MOCK_ORDER_FREE, + payment_method: null, + payment_source: null, + } + + async function renderFree( + // biome-ignore lint/suspicious/noExplicitAny: test cast + order: any, + currentPaymentMethodType?: string + ): Promise { render( - + ) - const btn = screen.getByRole("button") - // free order + isPermitted from container → not disabled - await waitFor(() => { - // button may eventually be enabled - expect(btn).toBeDefined() + const btn = await waitFor(() => screen.getByRole("button")) + // Let the container dispatch and the button's effect settle. + await act(async () => { + await Promise.resolve() }) + return !btn.hasAttribute("disabled") + } + + it("is enabled for a complete free order with no payment method", async () => { + expect(await renderFree(FREE_NO_PAYMENT, undefined)).toBe(true) + }) + + it("is disabled for a free order missing the billing address", async () => { + expect(await renderFree({ ...FREE_NO_PAYMENT, billing_address: null }, undefined)).toBe(false) + }) + + it("is disabled for a shippable free order missing the shipping address", async () => { + expect( + await renderFree( + { + ...FREE_NO_PAYMENT, + shipping_address: null, + line_items: [{ item_type: "skus", id: "li-1" }], + }, + undefined + ) + ).toBe(false) + }) + + it("is disabled for a free order whose privacy/terms are not accepted", async () => { + expect( + await renderFree( + { ...FREE_NO_PAYMENT, privacy_url: "https://o/p", terms_url: "https://o/t" }, + undefined + ) + ).toBe(false) }) }) @@ -559,12 +617,12 @@ describe("PlaceOrderButton (free order)", () => { describe("PrivacyAndTermsCheckbox (standalone)", () => { beforeEach(() => { - localStorage.clear() + resetTermsAcceptanceStore() vi.clearAllMocks() }) afterEach(() => { - localStorage.clear() + resetTermsAcceptanceStore() }) it("renders a checkbox input", () => { @@ -602,11 +660,8 @@ describe("PrivacyAndTermsCheckbox (standalone)", () => { }) }) - it("dispatches PLACE_ORDER_RECHECK_EVENT on change in standalone mode", async () => { - const handler = vi.fn() - window.addEventListener(PLACE_ORDER_RECHECK_EVENT, handler) - - render( + it("registers itself in the store while mounted", async () => { + const { unmount } = render( { ) - await waitFor(() => { - expect(screen.getByRole("checkbox").getAttribute("disabled")).toBeNull() - }) - - await act(async () => { - fireEvent.click(screen.getByRole("checkbox")) + expect(getCheckboxCount("order-1")).toBe(1) }) - - expect(handler).toHaveBeenCalledTimes(1) - window.removeEventListener(PLACE_ORDER_RECHECK_EVENT, handler) + unmount() + expect(getCheckboxCount("order-1")).toBe(0) }) - it("writes to localStorage on change", async () => { + it("records acceptance in the store on change", async () => { render( { fireEvent.click(screen.getByRole("checkbox")) }) - expect(localStorage.getItem("privacy-terms")).toBe("true") + expect(getAcceptedSnapshot("order-1")).toBe(true) + expect((screen.getByRole("checkbox") as HTMLInputElement).checked).toBe(true) }) - it("cleans up localStorage on unmount", () => { - localStorage.setItem("privacy-terms", "true") + it("resets acceptance when the last checkbox unmounts", async () => { const { unmount } = render( - + ) + await waitFor(() => { + expect(screen.getByRole("checkbox").getAttribute("disabled")).toBeNull() + }) + await act(async () => { + fireEvent.click(screen.getByRole("checkbox")) + }) + expect(getAcceptedSnapshot("order-1")).toBe(true) + + // Consent must not outlive the control that collected it. unmount() - expect(localStorage.getItem("privacy-terms")).toBeNull() + expect(getAcceptedSnapshot("order-1")).toBe(false) }) it("reads privacy/terms URL from organizationConfig when not on order", async () => { @@ -694,155 +758,99 @@ describe("PrivacyAndTermsCheckbox (standalone)", () => { describe("PrivacyAndTermsCheckbox (container mode)", () => { beforeEach(() => { - localStorage.clear() + resetTermsAcceptanceStore() vi.clearAllMocks() }) afterEach(() => { - localStorage.clear() + resetTermsAcceptanceStore() }) - it("calls placeOrderPermitted from context on change", async () => { - const placeOrderPermittedMock = vi.fn() + it("toggling the checkbox flips the button inside PlaceOrderContainer", async () => { + const order = { + ...MOCK_ORDER, + privacy_url: "https://example.com/privacy", + terms_url: "https://example.com/terms", + } + const getCardDetails = (await import("#utils/getCardDetails")).default + vi.mocked(getCardDetails).mockReturnValue({ brand: "visa" } as never) + render( - - + + - + + ) await waitFor(() => { expect(screen.getByRole("checkbox").getAttribute("disabled")).toBeNull() }) + expect(screen.getByRole("button").hasAttribute("disabled")).toBe(true) await act(async () => { fireEvent.click(screen.getByRole("checkbox")) }) - - expect(placeOrderPermittedMock).toHaveBeenCalledTimes(1) - }) - - it("does NOT dispatch PLACE_ORDER_RECHECK_EVENT in container mode", async () => { - const handler = vi.fn() - window.addEventListener(PLACE_ORDER_RECHECK_EVENT, handler) - - render( - - - - - - ) - await waitFor(() => { - expect(screen.getByRole("checkbox").getAttribute("disabled")).toBeNull() - }) - - await act(async () => { - fireEvent.click(screen.getByRole("checkbox")) + expect(screen.getByRole("button").hasAttribute("disabled")).toBe(false) }) - expect(handler).not.toHaveBeenCalled() - window.removeEventListener(PLACE_ORDER_RECHECK_EVENT, handler) + vi.mocked(getCardDetails).mockReturnValue({ brand: "" } as never) }) + }) // --------------------------------------------------------------------------- -// usePlaceOrder hook — PLACE_ORDER_RECHECK_EVENT listener +// usePlaceOrder hook — terms acceptance store integration // --------------------------------------------------------------------------- -describe("usePlaceOrder RECHECK_EVENT integration", () => { +describe("usePlaceOrder store integration (standalone)", () => { beforeEach(() => { - localStorage.clear() + resetTermsAcceptanceStore() vi.clearAllMocks() }) - afterEach(() => { - localStorage.clear() - }) + it("re-runs placeOrderPermitted when acceptance changes in the store", async () => { + const order = { + ...MOCK_ORDER, + privacy_url: "https://example.com/privacy", + terms_url: "https://example.com/terms", + } + const getCardDetails = (await import("#utils/getCardDetails")).default + vi.mocked(getCardDetails).mockReturnValue({ brand: "visa" } as never) - it("re-runs placeOrderPermitted when RECHECK event is dispatched", async () => { - // Render standalone PlaceOrderButton + PrivacyAndTermsCheckbox render( - + ) - const checkbox = screen.getByRole("checkbox") - const button = screen.getByRole("button") - await waitFor(() => { - expect(checkbox.getAttribute("disabled")).toBeNull() + expect(screen.getByRole("checkbox").getAttribute("disabled")).toBeNull() }) + // Nothing accepted yet: the gate holds. + expect(screen.getByRole("button").hasAttribute("disabled")).toBe(true) - // Initially button is disabled (not permitted — privacy not accepted) - expect(button.getAttribute("disabled")).toBeDefined() - - // Check the privacy checkbox → dispatches RECHECK → usePlaceOrder re-evaluates - localStorage.setItem("privacy-terms", "true") await act(async () => { - fireEvent.click(checkbox) + fireEvent.click(screen.getByRole("checkbox")) }) - // RECHECK event was dispatched; hook should re-evaluate permissions + // The store notified the standalone hook, which recomputed isPermitted. await waitFor(() => { - expect(screen.getByRole("checkbox")).toBeDefined() + expect(screen.getByRole("button").hasAttribute("disabled")).toBe(false) }) - }) - it("usePlaceOrder does not listen for recheck event in container mode", async () => { - // In container mode, the button uses parentCtx, hook is no-op - const recheckHandler = vi.fn() - window.addEventListener(PLACE_ORDER_RECHECK_EVENT, recheckHandler) - - render( - - - - - - ) - - window.dispatchEvent(new CustomEvent(PLACE_ORDER_RECHECK_EVENT)) - // The container-mode button does not register listeners; the event is not handled by usePlaceOrder - expect(recheckHandler).toHaveBeenCalledTimes(1) // event fired, but no error + // Unchecking must close the gate again. + await act(async () => { + fireEvent.click(screen.getByRole("checkbox")) + }) + await waitFor(() => { + expect(screen.getByRole("button").hasAttribute("disabled")).toBe(true) + }) - window.removeEventListener(PLACE_ORDER_RECHECK_EVENT, recheckHandler) + vi.mocked(getCardDetails).mockReturnValue({ brand: "" } as never) }) }) @@ -1192,10 +1200,12 @@ describe("usePlaceOrder callbacks (via PlaceOrderButton standalone)", () => { ) await waitFor(() => expect(screen.getByRole("button")).toBeDefined()) - // Dispatch the recheck event — exercises placeOrderPermittedCallback (line 125) + // Flipping acceptance in the store exercises placeOrderPermittedCallback. await act(async () => { - window.dispatchEvent(new CustomEvent(PLACE_ORDER_RECHECK_EVENT)) + setAccepted("order-1", true) }) + expect(getAcceptedSnapshot("order-1")).toBe(true) + // No checkbox is mounted, so the gate must still refuse to open. expect(screen.getByRole("button")).toBeDefined() }) @@ -1224,12 +1234,12 @@ describe("usePlaceOrder callbacks (via PlaceOrderButton standalone)", () => { describe("PrivacyAndTermsCheckbox edge cases", () => { beforeEach(() => { - localStorage.clear() + resetTermsAcceptanceStore() vi.clearAllMocks() }) afterEach(() => { - localStorage.clear() + resetTermsAcceptanceStore() }) it("does nothing when container mode but placeOrderPermitted is not provided", async () => { @@ -1275,7 +1285,7 @@ describe("PrivacyAndTermsCheckbox edge cases", () => { describe("usePlaceOrder hook direct", () => { beforeEach(async () => { - localStorage.clear() + resetTermsAcceptanceStore() vi.clearAllMocks() const { getSdk } = await import("@commercelayer/core-components") vi.mocked(getSdk).mockReturnValue({ @@ -1290,7 +1300,7 @@ describe("usePlaceOrder hook direct", () => { } as any) }) - afterEach(() => localStorage.clear()) + afterEach(() => resetTermsAcceptanceStore()) function wrapper({ children }: { children: ReactNode }) { return ( @@ -1484,11 +1494,11 @@ describe("usePlaceOrder hook direct", () => { ) } renderHook(() => usePlaceOrder({ isStandalone: true }), { wrapper: wrapperNoOrder }) - // Dispatch the recheck event with no order — should not throw + // Acceptance recorded with no order loaded must not throw. await act(async () => { - window.dispatchEvent(new CustomEvent(PLACE_ORDER_RECHECK_EVENT)) + setAccepted(undefined, true) }) - // No assertion needed: coverage is the goal; test passes if no error thrown + expect(getAcceptedSnapshot(undefined)).toBe(true) }) it("covers billing_address includeLoaded else-if branch (line 75)", async () => { @@ -1528,14 +1538,13 @@ describe("usePlaceOrder hook direct", () => { // PrivacyAndTermsCheckbox — !checked false branch when effect re-runs // --------------------------------------------------------------------------- -describe("PrivacyAndTermsCheckbox !checked branch", () => { +describe("PrivacyAndTermsCheckbox URL changes", () => { beforeEach(() => { - localStorage.clear() + resetTermsAcceptanceStore() vi.clearAllMocks() }) - afterEach(() => localStorage.clear()) - it("effect skips localStorage write when checked=true (line 36 false branch)", async () => { + it("keeps acceptance when the privacy/terms URLs change", async () => { const { rerender } = render( { await act(async () => { fireEvent.click(screen.getByRole("checkbox")) }) - expect(localStorage.getItem("privacy-terms")).toBe("true") + expect((screen.getByRole("checkbox") as HTMLInputElement).checked).toBe(true) - // Change URLs so effect re-runs with checked=true → !checked = false → localStorage write skipped - // (cleanup from prior effect removes the item; since checked=true the false branch means no re-write to "false") + // Swapping the URLs must not silently revoke what the shopper already accepted. await act(async () => { rerender( { ) }) - // Cleanup removed the item; the false branch of !checked means it was NOT set to "false" - expect(localStorage.getItem("privacy-terms")).not.toBe("false") + expect((screen.getByRole("checkbox") as HTMLInputElement).checked).toBe(true) }) }) diff --git a/packages/react-components/specs/orders/terms-acceptance.spec.tsx b/packages/react-components/specs/orders/terms-acceptance.spec.tsx new file mode 100644 index 00000000..14133bee --- /dev/null +++ b/packages/react-components/specs/orders/terms-acceptance.spec.tsx @@ -0,0 +1,420 @@ +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react" +import { type ReactNode, useEffect, useState } from "react" +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { PlaceOrderButton } from "#components/orders/PlaceOrderButton" +import { PlaceOrderContainer } from "#components/orders/PlaceOrderContainer" +import { PrivacyAndTermsCheckbox } from "#components/orders/PrivacyAndTermsCheckbox" +import CommerceLayerContext from "#context/CommerceLayerContext" +import CustomerContext from "#context/CustomerContext" +import OrderContext, { defaultOrderContext } from "#context/OrderContext" +import PaymentMethodContext, { defaultPaymentMethodContext } from "#context/PaymentMethodContext" +import { useTermsAndConditions } from "#hooks/useTermsAndConditions" +import { + getAcceptedSnapshot, + getCheckboxCount, + registerCheckbox, + resetTermsAcceptanceStore, + setAccepted, + subscribe, +} from "#utils/termsAcceptanceStore" + +vi.mock("@commercelayer/core-components", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + getSdk: vi.fn().mockReturnValue({ + orders: { retrieve: vi.fn().mockResolvedValue({ id: "order-1", status: "pending" }) }, + }), + } +}) + +// Privacy/terms live on the ORGANIZATION CONFIG, not on the order — the real +// shape of the checkout where this bug was reported. +vi.mock("#utils/organization", () => { + const STABLE = { urls: { privacy: "https://org.example.com/privacy", terms: "https://org.example.com/terms" } } + return { useOrganizationConfig: vi.fn(() => STABLE) } +}) + +// A card IS selected, so `card.brand` is truthy and the button's enabling +// condition depends on `isPermitted` alone. +vi.mock("#utils/getCardDetails", () => ({ default: vi.fn().mockReturnValue({ brand: "visa" }) })) + +// Stable identities: mfe-checkout's OrderContext does not hand out a new +// `include` array on every render, and a churning one would mask the bug by +// forcing the container to recompute. +const INCLUDE: string[] = [] +// biome-ignore lint/suspicious/noExplicitAny: test cast +const INCLUDE_LOADED: any = {} +const FNS = { + add: vi.fn(), + setOrder: vi.fn(), + setOrderErrors: vi.fn(), + setPaymentSource: vi.fn(), + setPaymentMethodErrors: vi.fn(), +} +const NO_ERRORS: unknown[] = [] + +// biome-ignore lint/suspicious/noExplicitAny: test cast +const ORDER: any = { + id: "order-1", + status: "pending", + total_amount_with_taxes_cents: 1000, + payment_method: { id: "pm-1", payment_source_type: "stripe_payments" }, + payment_source: { id: "ps-1", type: "stripe_payments" }, + billing_address: { id: "ba-1" }, + shipping_address: { id: "sa-1" }, + shipments: [], + line_items: [], + privacy_url: null, + terms_url: null, +} + +// biome-ignore lint/suspicious/noExplicitAny: test cast +function Providers({ children, order = ORDER }: { children: ReactNode; order?: any }) { + return ( + + + + + {children} + + + + + ) +} + +// --------------------------------------------------------------------------- +// termsAcceptanceStore +// --------------------------------------------------------------------------- + +describe("termsAcceptanceStore", () => { + beforeEach(() => resetTermsAcceptanceStore()) + + it("starts from 'not accepted'", () => { + expect(getAcceptedSnapshot("o1")).toBe(false) + }) + + it("notifies subscribers of the keyed order only", () => { + const a = vi.fn() + const b = vi.fn() + subscribe("o1", a) + subscribe("o2", b) + setAccepted("o1", true) + expect(a).toHaveBeenCalledTimes(1) + expect(b).not.toHaveBeenCalled() + }) + + it("does not notify when the value is unchanged", () => { + const listener = vi.fn() + subscribe("o1", listener) + setAccepted("o1", false) + expect(listener).not.toHaveBeenCalled() + }) + + it("keeps acceptance per order, so consent cannot leak across orders", () => { + setAccepted("o1", true) + expect(getAcceptedSnapshot("o1")).toBe(true) + expect(getAcceptedSnapshot("o2")).toBe(false) + }) + + it("unsubscribes cleanly", () => { + const listener = vi.fn() + const unsubscribe = subscribe("o1", listener) + unsubscribe() + setAccepted("o1", true) + expect(listener).not.toHaveBeenCalled() + }) + + it("counts mounted checkboxes and resets acceptance when the last one leaves", () => { + const off1 = registerCheckbox("o1") + const off2 = registerCheckbox("o1") + expect(getCheckboxCount("o1")).toBe(2) + setAccepted("o1", true) + + off1() + // One checkbox is still asking, so acceptance survives. + expect(getCheckboxCount("o1")).toBe(1) + expect(getAcceptedSnapshot("o1")).toBe(true) + + off2() + expect(getCheckboxCount("o1")).toBe(0) + expect(getAcceptedSnapshot("o1")).toBe(false) + }) + + it("shares one entry between checkbox and button before the order has loaded", () => { + setAccepted(undefined, true) + expect(getAcceptedSnapshot(undefined)).toBe(true) + expect(getAcceptedSnapshot(null)).toBe(true) + }) +}) + +// --------------------------------------------------------------------------- +// Regression: the reported bug +// --------------------------------------------------------------------------- + +describe("REGRESSION: unchecked privacy/terms keeps PlaceOrderButton disabled", () => { + beforeEach(() => resetTermsAcceptanceStore()) + + it("stays disabled with a payment method selected and the checkbox unchecked", async () => { + render( + + + + + + + ) + await waitFor(() => { + expect(screen.getByRole("checkbox").getAttribute("disabled")).toBeNull() + }) + expect((screen.getByRole("checkbox") as HTMLInputElement).checked).toBe(false) + expect(screen.getByRole("button").hasAttribute("disabled")).toBe(true) + }) + + it("stays disabled when the checkbox mounts AFTER the container first computed isPermitted", async () => { + // This is the exact shape of the original bug: acceptance used to live in + // localStorage, survived a hard navigation as "true", and the late-mounting + // checkbox reset it without telling the container to recompute. + const { rerender } = render( + + + + + + ) + await waitFor(() => expect(screen.getByRole("button")).toBeDefined()) + + rerender( + + + + + + + ) + await waitFor(() => { + expect(screen.getByRole("checkbox").getAttribute("disabled")).toBeNull() + }) + + expect((screen.getByRole("checkbox") as HTMLInputElement).checked).toBe(false) + expect(screen.getByRole("button").hasAttribute("disabled")).toBe(true) + }) + + it("acceptance does not survive a remount, so a reload starts from unaccepted", async () => { + const first = render( + + + + + + + ) + await waitFor(() => { + expect(screen.getByRole("checkbox").getAttribute("disabled")).toBeNull() + }) + await act(async () => { + fireEvent.click(screen.getByRole("checkbox")) + }) + await waitFor(() => { + expect(screen.getByRole("button").hasAttribute("disabled")).toBe(false) + }) + first.unmount() + + render( + + + + + + + ) + await waitFor(() => { + expect(screen.getByRole("checkbox").getAttribute("disabled")).toBeNull() + }) + expect((screen.getByRole("checkbox") as HTMLInputElement).checked).toBe(false) + expect(screen.getByRole("button").hasAttribute("disabled")).toBe(true) + }) + + it("requires EVERY mounted checkbox to be accepted", async () => { + render( + + + + + + + + ) + await waitFor(() => expect(getCheckboxCount("order-1")).toBe(2)) + // Both controls render the same acceptance, so ticking one ticks both — + // there is a single consent per order, never two that can disagree. + await act(async () => { + fireEvent.click(screen.getByTestId("cb-1")) + }) + expect((screen.getByTestId("cb-1") as HTMLInputElement).checked).toBe(true) + expect((screen.getByTestId("cb-2") as HTMLInputElement).checked).toBe(true) + }) +}) + +// --------------------------------------------------------------------------- +// Q15 diagnostic: acceptance required, but nothing asks for it +// --------------------------------------------------------------------------- + +describe("diagnostic when acceptance is required but no checkbox is mounted", () => { + beforeEach(() => resetTermsAcceptanceStore()) + afterEach(() => vi.restoreAllMocks()) + + it("stays quiet when a checkbox mounts later in the same tree", async () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}) + function LateCheckbox(): React.JSX.Element | null { + const [shown, setShown] = useState(false) + useEffect(() => { + setShown(true) + }, []) + return shown ? : null + } + render( + + + + + + + ) + await waitFor(() => { + expect(screen.getByRole("checkbox").getAttribute("disabled")).toBeNull() + }) + expect(spy).not.toHaveBeenCalledWith( + expect.stringContaining("no is mounted") + ) + expect(screen.getByRole("button").hasAttribute("disabled")).toBe(true) + }) + + it("reports it once the gate is the only thing left blocking the button", async () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}) + render( + + + + + + ) + await waitFor(() => { + expect(spy).toHaveBeenCalledWith( + expect.stringContaining("no is mounted") + ) + }) + expect(screen.getByRole("button").hasAttribute("disabled")).toBe(true) + }) + + it("stays quiet when a checkbox is mounted", async () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}) + render( + + + + + + + ) + await waitFor(() => { + expect(screen.getByRole("checkbox").getAttribute("disabled")).toBeNull() + }) + expect(spy).not.toHaveBeenCalledWith( + expect.stringContaining("no is mounted") + ) + }) + + it("stays quiet when something else is already blocking the order", async () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}) + render( + + + + + + ) + await waitFor(() => expect(screen.getByRole("button")).toBeDefined()) + expect(spy).not.toHaveBeenCalledWith( + expect.stringContaining("no is mounted") + ) + }) +}) + +// --------------------------------------------------------------------------- +// useTermsAndConditions — the supported channel for a custom checkbox +// --------------------------------------------------------------------------- + +describe("useTermsAndConditions", () => { + beforeEach(() => resetTermsAcceptanceStore()) + + function CustomConsent(): React.JSX.Element { + const { accepted, setAccepted } = useTermsAndConditions() + return ( + + ) + } + + it("lets a custom control open the gate without ", async () => { + render( + + + + + + + ) + await waitFor(() => expect(screen.getByTestId("place-order")).toBeDefined()) + expect(screen.getByTestId("place-order").hasAttribute("disabled")).toBe(true) + + await act(async () => { + fireEvent.click(screen.getByTestId("custom-consent")) + }) + + await waitFor(() => { + expect(screen.getByTestId("place-order").hasAttribute("disabled")).toBe(false) + }) + expect(screen.getByTestId("custom-consent").textContent).toBe("accepted") + }) + + it("reflects acceptance written elsewhere for the same order", async () => { + render( + + + + ) + expect(screen.getByTestId("custom-consent").textContent).toBe("not accepted") + await act(async () => { + setAccepted("order-1", true) + }) + expect(screen.getByTestId("custom-consent").textContent).toBe("accepted") + }) +}) diff --git a/packages/react-components/src/components/orders/PlaceOrderButton.tsx b/packages/react-components/src/components/orders/PlaceOrderButton.tsx index 51371381..3c5626b9 100644 --- a/packages/react-components/src/components/orders/PlaceOrderButton.tsx +++ b/packages/react-components/src/components/orders/PlaceOrderButton.tsx @@ -107,9 +107,9 @@ export function PlaceOrderButton(props: Props): JSX.Element { setNotPermitted(true) } } - if (isFree && !isPermitted) { - setNotPermitted(false) - } + // NOTE: no `isFree && !isPermitted` shortcut here. It used to live at this + // spot but was dead code: `setNotPermitted` is a state setter, so the + // branches below ran in the same effect pass and always overwrote it. if (loading) setNotPermitted(loading) else { if (paymentType === currentPaymentMethodType && paymentType) { diff --git a/packages/react-components/src/components/orders/PlaceOrderContainer.tsx b/packages/react-components/src/components/orders/PlaceOrderContainer.tsx index 639a1c75..ece53403 100644 --- a/packages/react-components/src/components/orders/PlaceOrderContainer.tsx +++ b/packages/react-components/src/components/orders/PlaceOrderContainer.tsx @@ -1,4 +1,13 @@ -import { type JSX, type ReactNode, type RefObject, useContext, useEffect, useReducer } from "react" +import { + type JSX, + type ReactNode, + type RefObject, + useCallback, + useContext, + useEffect, + useReducer, + useSyncExternalStore, +} from "react" import CommerceLayerContext from "#context/CommerceLayerContext" import OrderContext from "#context/OrderContext" import PlaceOrderContext from "#context/PlaceOrderContext" @@ -10,7 +19,9 @@ import placeOrderReducer, { setPlaceOrderStatus, } from "#reducers/PlaceOrderReducer" import useCustomContext from "#utils/hooks/useCustomContext" +import { useMissingTermsCheckboxWarning } from "#utils/hooks/useMissingTermsCheckboxWarning" import { useOrganizationConfig } from "#utils/organization" +import { getAcceptedSnapshot, subscribe as subscribeToTerms } from "#utils/termsAcceptanceStore" import { setPlaceOrder } from "../../reducers/PlaceOrderReducer" interface Props { @@ -37,6 +48,20 @@ export function PlaceOrderContainer(props: Props): JSX.Element { const organizationConfig = useOrganizationConfig({ accessToken: config.accessToken, }) + // Privacy & terms acceptance lives in a module-level store because + // is a sibling of , not a child. + // Subscribing here is what makes the button react the moment it changes. + const orderId = order?.id + const stableSubscribe = useCallback( + (listener: () => void) => subscribeToTerms(orderId, listener), + [orderId] + ) + const termsAccepted = useSyncExternalStore( + stableSubscribe, + useCallback(() => getAcceptedSnapshot(orderId), [orderId]), + // c8 ignore next — server snapshot only used during SSR hydration + () => false + ) // biome-ignore lint/correctness/useExhaustiveDependencies: Infinite loop useEffect(() => { if (!include?.includes("shipments.available_shipping_methods")) { @@ -89,9 +114,12 @@ export function PlaceOrderContainer(props: Props): JSX.Element { }, privacyUrl: organizationConfig?.urls?.privacy, termsUrl: organizationConfig?.urls?.terms, + termsAccepted, }) } - }, [order, include, includeLoaded, organizationConfig]) + }, [order, include, includeLoaded, organizationConfig, termsAccepted]) + useMissingTermsCheckboxWarning(state.termsBlocking, orderId) + const contextValue = { ...state, _isProvided: true as const, @@ -127,6 +155,7 @@ export function PlaceOrderContainer(props: Props): JSX.Element { }, privacyUrl: organizationConfig?.urls?.privacy, termsUrl: organizationConfig?.urls?.terms, + termsAccepted, }) }, setButtonRef: (ref: RefObject) => { diff --git a/packages/react-components/src/components/orders/PrivacyAndTermsCheckbox.tsx b/packages/react-components/src/components/orders/PrivacyAndTermsCheckbox.tsx index 09527cbc..c3d7bd90 100644 --- a/packages/react-components/src/components/orders/PrivacyAndTermsCheckbox.tsx +++ b/packages/react-components/src/components/orders/PrivacyAndTermsCheckbox.tsx @@ -1,18 +1,16 @@ import { type JSX, useContext, useEffect, useState } from "react" import CommerceLayerContext from "#context/CommerceLayerContext" import OrderContext from "#context/OrderContext" -import PlaceOrderContext from "#context/PlaceOrderContext" -import { PLACE_ORDER_RECHECK_EVENT } from "#hooks/usePlaceOrder" +import { useTermsAndConditions } from "#hooks/useTermsAndConditions" import { useOrganizationConfig } from "#utils/organization" +import { registerCheckbox } from "#utils/termsAcceptanceStore" import BaseInput, { type BaseInputProps } from "../utils/BaseInput" export function PrivacyAndTermsCheckbox(props: Partial): JSX.Element { const { accessToken } = useContext(CommerceLayerContext) const { order } = useContext(OrderContext) - const placeOrderCtx = useContext(PlaceOrderContext) - const isStandalone = placeOrderCtx._isProvided !== true const [forceDisabled, setForceDisabled] = useState(true) - const [checked, setChecked] = useState(false) + const { accepted, setAccepted } = useTermsAndConditions() const fieldName = "privacy-terms" const organizationConfig = useOrganizationConfig({ accessToken }) @@ -20,32 +18,24 @@ export function PrivacyAndTermsCheckbox(props: Partial): JSX.Ele const termsUrl = order?.terms_url ?? organizationConfig?.urls?.terms const handleChange = (e: React.ChangeEvent): void => { - const v = (e.target as HTMLInputElement)?.checked - setChecked(v) - localStorage.setItem(fieldName, v.toString()) - if (!isStandalone && placeOrderCtx.placeOrderPermitted) { - placeOrderCtx.placeOrderPermitted() - } else if (isStandalone) { - window.dispatchEvent(new CustomEvent(PLACE_ORDER_RECHECK_EVENT)) - } + setAccepted((e.target as HTMLInputElement)?.checked) } - // biome-ignore lint/correctness/useExhaustiveDependencies: If we add checked to the dependencies, it creates an wrong behavior to disable the place order button. useEffect(() => { - if (privacyUrl && termsUrl) setForceDisabled(false) - if (!checked) localStorage.setItem(fieldName, checked.toString()) - return () => { - setForceDisabled(true) - localStorage.removeItem(fieldName) - } + setForceDisabled(!(privacyUrl && termsUrl)) }, [privacyUrl, termsUrl]) + + // Announce this checkbox to the store so `placeOrderPermitted` can tell + // "the shopper has not accepted yet" apart from "nobody is asking". + useEffect(() => registerCheckbox(order?.id), [order?.id]) + return ( ) diff --git a/packages/react-components/src/hooks/usePlaceOrder.ts b/packages/react-components/src/hooks/usePlaceOrder.ts index 59c2e38c..38347275 100644 --- a/packages/react-components/src/hooks/usePlaceOrder.ts +++ b/packages/react-components/src/hooks/usePlaceOrder.ts @@ -1,5 +1,5 @@ import type { RefObject } from "react" -import { useCallback, useContext, useEffect, useMemo, useReducer } from "react" +import { useCallback, useContext, useEffect, useMemo, useReducer, useSyncExternalStore } from "react" import CommerceLayerContext from "#context/CommerceLayerContext" import OrderContext from "#context/OrderContext" import placeOrderReducer, { @@ -10,23 +10,18 @@ import placeOrderReducer, { setPlaceOrder, setPlaceOrderStatus, } from "#reducers/PlaceOrderReducer" +import { useMissingTermsCheckboxWarning } from "#utils/hooks/useMissingTermsCheckboxWarning" import { useOrganizationConfig } from "#utils/organization" - -/** - * Custom DOM event dispatched by `` in standalone mode - * so that a sibling `` can re-run `placeOrderPermitted` when - * the checkbox state changes. - */ -export const PLACE_ORDER_RECHECK_EVENT = "cl:placeorder:recheck" +import { getAcceptedSnapshot, subscribe as subscribeToTerms } from "#utils/termsAcceptanceStore" /** * Manages place-order state in standalone mode. * * When `isStandalone` is `true` the hook replicates the behaviour of * ``: it registers the required resource includes on - * `OrderContext`, evaluates `placeOrderPermitted` whenever the order changes, - * and returns a fully-bound context value ready to be passed to - * ``. + * `OrderContext`, evaluates `placeOrderPermitted` whenever the order or privacy + * & terms acceptance changes, and returns a fully-bound context value ready to + * be passed to ``. * * When `isStandalone` is `false` (i.e. a `` parent is * already present) all effects are no-ops and the returned value is unused — @@ -44,6 +39,19 @@ export function usePlaceOrder({ useContext(OrderContext) const config = useContext(CommerceLayerContext) const organizationConfig = useOrganizationConfig({ accessToken: config.accessToken }) + // is a sibling of , so acceptance + // travels through a module-level store rather than React context. + const orderId = order?.id + const stableSubscribe = useCallback( + (listener: () => void) => subscribeToTerms(orderId, listener), + [orderId] + ) + const termsAccepted = useSyncExternalStore( + stableSubscribe, + useCallback(() => getAcceptedSnapshot(orderId), [orderId]), + // c8 ignore next — server snapshot only used during SSR hydration + () => false + ) // biome-ignore lint/correctness/useExhaustiveDependencies: mirrors PlaceOrderContainer behavior useEffect(() => { @@ -87,28 +95,12 @@ export function usePlaceOrder({ options, privacyUrl: organizationConfig?.urls?.privacy, termsUrl: organizationConfig?.urls?.terms, + termsAccepted, }) } - }, [order, include, includeLoaded, organizationConfig, isStandalone]) + }, [order, include, includeLoaded, organizationConfig, isStandalone, termsAccepted]) - // Re-run placeOrderPermitted when PrivacyAndTermsCheckbox signals a change - useEffect(() => { - if (!isStandalone) return - const recheck = (): void => { - if (order) { - placeOrderPermitted({ - config, - dispatch, - order, - options, - privacyUrl: organizationConfig?.urls?.privacy, - termsUrl: organizationConfig?.urls?.terms, - }) - } - } - window.addEventListener(PLACE_ORDER_RECHECK_EVENT, recheck) - return () => window.removeEventListener(PLACE_ORDER_RECHECK_EVENT, recheck) - }, [isStandalone, order, config, options, organizationConfig]) + useMissingTermsCheckboxWarning(isStandalone ? state.termsBlocking : false, orderId) const setButtonRefCallback = useCallback( (ref: RefObject) => setButtonRef(ref, dispatch), @@ -129,8 +121,9 @@ export function usePlaceOrder({ options, privacyUrl: organizationConfig?.urls?.privacy, termsUrl: organizationConfig?.urls?.terms, + termsAccepted, }) - }, [config, order, options, organizationConfig]) + }, [config, order, options, organizationConfig, termsAccepted]) return useMemo( () => ({ diff --git a/packages/react-components/src/hooks/useTermsAndConditions.ts b/packages/react-components/src/hooks/useTermsAndConditions.ts new file mode 100644 index 00000000..d4c43ff6 --- /dev/null +++ b/packages/react-components/src/hooks/useTermsAndConditions.ts @@ -0,0 +1,58 @@ +import { useCallback, useContext, useSyncExternalStore } from "react" +import OrderContext from "#context/OrderContext" +import { + getAcceptedSnapshot, + setAccepted as setAcceptedInStore, + subscribe, +} from "#utils/termsAcceptanceStore" + +export interface UseTermsAndConditionsReturn { + /** Whether the shopper has accepted the privacy policy and terms of service. */ + accepted: boolean + /** Records the shopper's choice. `` reacts immediately. */ + setAccepted: (accepted: boolean) => void +} + +/** + * Read and write privacy & terms acceptance for the current order. + * + * Use it to build a checkbox with your own markup instead of + * ``. Acceptance is what `` gates on, + * so a custom control must go through this hook — there is no other supported + * channel. + * + * Acceptance is not persisted: a reload starts from `false`. + * + * @example + * ```tsx + * const { accepted, setAccepted } = useTermsAndConditions() + * return + * ``` + */ +export function useTermsAndConditions(): UseTermsAndConditionsReturn { + const { order } = useContext(OrderContext) + const orderId = order?.id + + const stableSubscribe = useCallback( + (listener: () => void) => subscribe(orderId, listener), + [orderId] + ) + const stableSnapshot = useCallback(() => getAcceptedSnapshot(orderId), [orderId]) + const accepted = useSyncExternalStore( + stableSubscribe, + stableSnapshot, + // c8 ignore next — server snapshot only used during SSR hydration + () => false + ) + + const setAccepted = useCallback( + (value: boolean) => { + setAcceptedInStore(orderId, value) + }, + [orderId] + ) + + return { accepted, setAccepted } +} + +export default useTermsAndConditions diff --git a/packages/react-components/src/index.ts b/packages/react-components/src/index.ts index 3cadabf2..a96b30ea 100644 --- a/packages/react-components/src/index.ts +++ b/packages/react-components/src/index.ts @@ -116,4 +116,5 @@ export * from "#components/stock_transfers/StockTransferField" export * from "#hooks/useCommerceLayer" export * from "#hooks/useCustomerContainer" export * from "#hooks/useOrderContainer" +export * from "#hooks/useTermsAndConditions" export * from "#typings/errors" diff --git a/packages/react-components/src/reducers/PlaceOrderReducer.ts b/packages/react-components/src/reducers/PlaceOrderReducer.ts index 659280b2..e976e55c 100644 --- a/packages/react-components/src/reducers/PlaceOrderReducer.ts +++ b/packages/react-components/src/reducers/PlaceOrderReducer.ts @@ -51,6 +51,12 @@ export interface PlaceOrderActionPayload { options?: PlaceOrderOptions placeOrderButtonRef?: RefObject status: PlaceOrderStatus + /** + * True when accepting privacy & terms is the *only* thing still keeping the + * order from being placed. Consumers use it to warn that acceptance is + * required but no control is asking for it. + */ + termsBlocking: boolean } export function setButtonRef( @@ -78,6 +84,7 @@ export const placeOrderInitialState: PlaceOrderState = { errors: [], isPermitted: false, status: "standby", + termsBlocking: false, } export function setPlaceOrderErrors( @@ -102,6 +109,12 @@ interface TPlaceOrderPermittedParams { options?: PlaceOrderOptions privacyUrl?: string | null termsUrl?: string | null + /** + * Whether the shopper has accepted privacy & terms. Passed in by the caller + * (which reads `termsAcceptanceStore`) so this function stays free of hidden + * global reads and the gate is testable in isolation. + */ + termsAccepted?: boolean } export function placeOrderPermitted({ @@ -111,14 +124,10 @@ export function placeOrderPermitted({ options, privacyUrl, termsUrl, + termsAccepted = false, }: TPlaceOrderPermittedParams): void { if (order && config) { let isPermitted = true - const resolvedPrivacyUrl = privacyUrl ?? order.privacy_url - const resolvedTermsUrl = termsUrl ?? order.terms_url - if (resolvedPrivacyUrl && resolvedTermsUrl) { - isPermitted = localStorage.getItem("privacy-terms") === "true" - } const billingAddress = order.billing_address const shippingAddress = order.shipping_address const doNotShip = isDoNotShip(order.line_items) @@ -132,10 +141,20 @@ export function placeOrderPermitted({ if (!isEmpty(shipments) && !shipment) isPermitted = false // @ts-expect-error no type if (paymentSource?.mismatched_amounts) isPermitted = false + + // Privacy & terms are checked last, so `isPermitted` still tells us whether + // acceptance is the *only* thing standing between the shopper and the order. + const resolvedPrivacyUrl = privacyUrl ?? order.privacy_url + const resolvedTermsUrl = termsUrl ?? order.terms_url + const termsRequired = Boolean(resolvedPrivacyUrl && resolvedTermsUrl) + const termsBlocking = termsRequired && !termsAccepted && isPermitted + if (termsRequired && !termsAccepted) isPermitted = false + dispatch({ type: "setPlaceOrderPermitted", payload: { isPermitted, + termsBlocking, paymentType: paymentMethod?.payment_source_type as PaymentResource, // @ts-expect-error no type paymentSecret: paymentSource?.client_secret, diff --git a/packages/react-components/src/utils/hooks/useMissingTermsCheckboxWarning.ts b/packages/react-components/src/utils/hooks/useMissingTermsCheckboxWarning.ts new file mode 100644 index 00000000..fb742888 --- /dev/null +++ b/packages/react-components/src/utils/hooks/useMissingTermsCheckboxWarning.ts @@ -0,0 +1,32 @@ +import { useEffect } from "react" +import { getCheckboxCount } from "#utils/termsAcceptanceStore" + +const MESSAGE = + "[PlaceOrderButton] This order requires accepting the privacy policy and terms of service, but no is mounted, so the shopper has no way to accept and the button stays disabled. Render , or build your own control with the useTermsAndConditions() hook." + +/** + * Warns, in development only, when acceptance is required but nothing on the + * page can collect it — the one state in which the gate leaves a dead button + * with no explanation. + * + * The check lives in an effect on purpose. Child effects run before parent + * effects, so by the time this runs every `` in the + * tree has already registered itself: a checkbox that mounts in the same commit + * can never trigger a false alarm, and no timer is needed to find that out. + * + * @param termsBlocking - True when acceptance is the only remaining blocker. + * @param orderId - Order the acceptance is keyed on. + */ +export function useMissingTermsCheckboxWarning( + termsBlocking: boolean | undefined, + orderId: string | undefined +): void { + useEffect(() => { + if (!termsBlocking) return + if (getCheckboxCount(orderId) > 0) return + if (process.env.NODE_ENV === "production") return + console.error(MESSAGE) + }, [termsBlocking, orderId]) +} + +export default useMissingTermsCheckboxWarning diff --git a/packages/react-components/src/utils/termsAcceptanceStore.ts b/packages/react-components/src/utils/termsAcceptanceStore.ts new file mode 100644 index 00000000..478ffa92 --- /dev/null +++ b/packages/react-components/src/utils/termsAcceptanceStore.ts @@ -0,0 +1,105 @@ +/** + * Module-level store for privacy & terms acceptance, keyed by order id. + * + * `` is a *sibling* of ``, never its + * ancestor, so in standalone mode no React provider can sit above both. This + * store is the shared channel between them: the checkbox writes acceptance, + * `PlaceOrderContainer` / `usePlaceOrder` subscribe via `useSyncExternalStore` + * and recompute `isPermitted` as soon as it changes. + * + * State lives in memory only — it is deliberately *not* persisted. A reload + * starts from "not accepted", so what the shopper sees and what gates the + * button can never diverge. + * + * Keying by order id keeps acceptance given on one order from leaking into + * another, and lets two independent checkouts coexist on the same page. + */ + +interface Entry { + accepted: boolean + /** How many `` instances are currently mounted. */ + checkboxCount: number +} + +/** Key used before the order has loaded, so checkbox and button still agree. */ +const PENDING_ORDER_KEY = "__cl_no_order__" + +const entries = new Map() +const listeners = new Map void>>() + +function key(orderId?: string | null): string { + return orderId ?? PENDING_ORDER_KEY +} + +function entry(orderId?: string | null): Entry { + const k = key(orderId) + let e = entries.get(k) + if (e == null) { + e = { accepted: false, checkboxCount: 0 } + entries.set(k, e) + } + return e +} + +function emit(orderId?: string | null): void { + const set = listeners.get(key(orderId)) + if (set == null) return + for (const listener of set) listener() +} + +export function subscribe(orderId: string | null | undefined, listener: () => void): () => void { + const k = key(orderId) + let set = listeners.get(k) + if (set == null) { + set = new Set() + listeners.set(k, set) + } + set.add(listener) + return () => { + set?.delete(listener) + if (set?.size === 0) listeners.delete(k) + } +} + +/** + * Current acceptance for an order. A boolean, so `useSyncExternalStore` + * compares snapshots by value and never loops. + */ +export function getAcceptedSnapshot(orderId?: string | null): boolean { + return entry(orderId).accepted +} + +export function setAccepted(orderId: string | null | undefined, accepted: boolean): void { + const e = entry(orderId) + if (e.accepted === accepted) return + e.accepted = accepted + emit(orderId) +} + +/** How many checkboxes are mounted for this order. Used only for diagnostics. */ +export function getCheckboxCount(orderId?: string | null): number { + return entry(orderId).checkboxCount +} + +/** + * Registers a mounted checkbox. Returns the deregister function. + * + * When the last checkbox for an order unmounts, acceptance is reset: consent + * must not outlive the control that collected it. + */ +export function registerCheckbox(orderId?: string | null): () => void { + const e = entry(orderId) + e.checkboxCount += 1 + emit(orderId) + return () => { + e.checkboxCount = Math.max(0, e.checkboxCount - 1) + if (e.checkboxCount === 0) e.accepted = false + emit(orderId) + } +} + +/** Test-only: drops all state so specs cannot leak acceptance into each other. */ +export function resetTermsAcceptanceStore(): void { + entries.clear() + listeners.clear() +} From c9cde04a0e3bd1fe98a3f5edc288f87a720406ba Mon Sep 17 00:00:00 2001 From: Alessandro Casazza Date: Mon, 31 Aug 2026 17:38:00 +0200 Subject: [PATCH 5/7] fix(adyen): stop enabling the place-order button behind React's back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selecting a Drop-in method that needs no input — Klarna, PayPal — enabled `PlaceOrderButton` while privacy & terms were still unaccepted, undoing the gate added in 819280f. `onSelect` and `onChange` answered `isValid` by writing `placeOrderButtonRef.current.disabled = false` straight onto the DOM node, which skips `isPermitted` — where the terms check lives — entirely. React never repaired it either: its own `disabled` prop had not changed, so no re-render reconciled the node. Observed on a live checkout as a fiber saying `disabled: true` over a DOM saying `disabled: false`, with the reducer reporting `isPermitted: false, termsBlocking: true`. Both writes were redundant as well as harmful. Each sits right after `ref.current.onsubmit = …` and `setPaymentRef({ ref })`, and `onsubmit` is in the button's own effect dependencies — so the button already re-runs and enables itself, but through `&& isPermitted`, which respects the terms. The two remaining writes in the post-authorization paths are left alone: they precede a programmatic `.click()` and belong to a different flow, worth revisiting on its own terms. - add regression coverage for both entry points, asserting on the DOM node's `disabled` rather than on React state — the blind spot that let the vacuous `place-order.spec.tsx` "disabled" tests pass on the wrong factor - capture the Drop-in's options in the test double, so `onSelect` is reachable Co-Authored-By: Claude Opus 5 (1M context) --- .../payment_source/AdyenPayment.spec.tsx | 73 ++++++++++++++++++- .../payment_source/AdyenPayment.tsx | 15 ++-- 2 files changed, 80 insertions(+), 8 deletions(-) diff --git a/packages/react-components/specs/payment_source/AdyenPayment.spec.tsx b/packages/react-components/specs/payment_source/AdyenPayment.spec.tsx index 7e164325..08889d9a 100644 --- a/packages/react-components/specs/payment_source/AdyenPayment.spec.tsx +++ b/packages/react-components/specs/payment_source/AdyenPayment.spec.tsx @@ -25,7 +25,7 @@ const adyen = vi.hoisted(() => ({ // The Core configuration the component builds, so tests can invoke the real // `onSubmit` handler it installs. // biome-ignore lint/suspicious/noExplicitAny: test cast - captured: { options: null as any }, + captured: { options: null as any, dropinOptions: null as any }, })) vi.mock("@adyen/adyen-web/auto", () => ({ @@ -35,6 +35,10 @@ vi.mock("@adyen/adyen-web/auto", () => ({ return { update: adyen.coreUpdate } }), Dropin: class FakeDropin { + // biome-ignore lint/suspicious/noExplicitAny: test cast + constructor(_core: any, options: any) { + adyen.captured.dropinOptions = options + } mount(selector: string): this { adyen.dropinMount(selector) return this @@ -93,6 +97,7 @@ function Providers({ setPaymentSource, setPaymentMethodErrors = vi.fn(), setPaymentRef = vi.fn(), + placeOrderButtonRef, }: { children: ReactNode // biome-ignore lint/suspicious/noExplicitAny: test cast @@ -111,6 +116,8 @@ function Providers({ setPaymentMethodErrors?: any // biome-ignore lint/suspicious/noExplicitAny: test cast setPaymentRef?: any + // biome-ignore lint/suspicious/noExplicitAny: test cast + placeOrderButtonRef?: any }) { // biome-ignore lint/suspicious/noExplicitAny: test cast const paymentMethodCtx: any = { @@ -136,7 +143,7 @@ function Providers({ > {children} @@ -848,3 +855,65 @@ describe("AdyenPayment when the payment source is recreated mid-flight", () => { expect(new Set(ids)).toEqual(new Set(["ps-recreated"])) }) }) + +// A Drop-in method that needs no input — Klarna, PayPal — reports `isValid` the moment it is +// selected. `onSelect` used to answer that by writing `placeOrderButtonRef.current.disabled = +// false` straight onto the DOM node, which skips `isPermitted` entirely: the button went live +// with privacy & terms still unaccepted. React never repaired it either, because its own +// `disabled` prop had not changed — the fiber said `true` while the DOM said `false`. +// Selecting a method may only arm the submit wiring; enabling the button is the button's call. +describe("AdyenPayment does not enable the place-order button behind React's back", () => { + beforeEach(() => { + vi.clearAllMocks() + adyen.captured.options = null + adyen.captured.dropinOptions = null + }) + + it("leaves the button disabled when a no-input method reports itself valid on selection", async () => { + // Stands in for the button `PlaceOrderButton` registers: React rendered it disabled + // because the terms are unaccepted. + const button = document.createElement("button") + button.disabled = true + const setPaymentRef = vi.fn() + + render( + + + + ) + await flush() + + expect(adyen.captured.dropinOptions?.onSelect).toBeTypeOf("function") + + await act(async () => { + adyen.captured.dropinOptions.onSelect({ _id: "klarna-0", isValid: true }) + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + + expect(button.disabled).toBe(true) + // The supported channel is still used, so the button can enable itself once + // `isPermitted` allows it. + expect(setPaymentRef).toHaveBeenCalled() + }) + + it("leaves the button disabled when the Drop-in reports a valid change", async () => { + const button = document.createElement("button") + button.disabled = true + const setPaymentRef = vi.fn() + + render( + + + + ) + await flush() + + await act(async () => { + adyen.captured.options.onChange({ isValid: true }) + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + + expect(button.disabled).toBe(true) + expect(setPaymentRef).toHaveBeenCalled() + }) +}) diff --git a/packages/react-components/src/components/payment_source/AdyenPayment.tsx b/packages/react-components/src/components/payment_source/AdyenPayment.tsx index 8a89e403..c5b6f4c3 100644 --- a/packages/react-components/src/components/payment_source/AdyenPayment.tsx +++ b/packages/react-components/src/components/payment_source/AdyenPayment.tsx @@ -269,10 +269,13 @@ export function AdyenPayment({ return await handleSubmit(ref.current as unknown as FormEvent) } setPaymentMethodErrors([]) + // NOTE: do not touch `placeOrderButtonRef.current.disabled` here. Setting + // it imperatively bypasses `isPermitted`, so the button would go live + // while privacy & terms are still unaccepted — and React never repairs + // it, since its own `disabled` prop has not changed. `setPaymentRef` + // below is the supported channel: `PlaceOrderButton` re-runs its effect + // on `onsubmit` and enables itself only when `isPermitted` allows it. setPaymentRef({ ref }) - if (placeOrderButtonRef?.current != null) { - placeOrderButtonRef.current.disabled = false - } } } } @@ -850,10 +853,10 @@ export function AdyenPayment({ return await handleSubmit(ref.current as unknown as FormEvent) } setPaymentMethodErrors([]) + // NOTE: see `handleChange` — enabling the button imperatively + // here is what let a Drop-in method that is valid on selection + // (Klarna, for one) go live with privacy & terms unaccepted. setPaymentRef({ ref }) - if (placeOrderButtonRef?.current != null) { - placeOrderButtonRef.current.disabled = false - } } } if (onSelect) { From e8ae014ee9b290fa1959fd54b540f746cc7fa697 Mon Sep 17 00:00:00 2001 From: Alessandro Casazza Date: Mon, 31 Aug 2026 17:51:14 +0200 Subject: [PATCH 6/7] docs(adyen): say why the post-authorization button writes must stay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two remaining `placeOrderButtonRef.current.disabled = false` writes look like leftovers of the pattern just removed from the `isValid` handlers, and the obvious next cleanup is to delete them for symmetry or swap the `.click()` for `setPlaceOrder`. Both would break paying by redirect. By the time these run Adyen has already authorized the payment and only the order is left to place. Terms acceptance lives in memory and does not survive the reload a redirect method (Klarna, iDEAL) causes, so the button is legitimately disabled on the way back — and `.click()` on a disabled button is a no-op, which would leave the shopper charged for an order that is never placed. The click cannot become `setPlaceOrder` either: `handleClick` additionally guards against already-placed and draft orders, drives the loading state, and fires the integrator's `onClick`. The Apple/Google Pay branch alongside does call `setPlaceOrder` directly, but express payments bypass that logic on purpose. Comments only — no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/payment_source/AdyenPayment.tsx | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/react-components/src/components/payment_source/AdyenPayment.tsx b/packages/react-components/src/components/payment_source/AdyenPayment.tsx index c5b6f4c3..b8c222fb 100644 --- a/packages/react-components/src/components/payment_source/AdyenPayment.tsx +++ b/packages/react-components/src/components/payment_source/AdyenPayment.tsx @@ -299,6 +299,19 @@ export function AdyenPayment({ // @ts-expect-error no type const resultCode = pSource?.payment_response?.resultCode if (["Authorised", "Pending", "Received"].includes(resultCode)) { + // NOTE: unlike the `isValid` handlers above, clearing `disabled` here is + // load-bearing — do not remove it for symmetry with them. Adyen has already + // authorized the payment; all that is left is to place the order. Terms + // acceptance lives in memory and does not survive the reload a redirect + // method (Klarna, iDEAL) causes, so the button is legitimately disabled by + // the time we get back — and `.click()` on a disabled button is a no-op, so + // without this the shopper is charged for an order that is never placed. + // + // It has to be a real click rather than `setPlaceOrder`: `handleClick` also + // guards against already-placed and draft orders, drives the loading state, + // and fires the integrator's `onClick`. (The Apple/Google Pay branch below + // does call `setPlaceOrder` directly — express payments deliberately bypass + // that logic.) if (placeOrderButtonRef?.current != null) { if (placeOrderButtonRef.current.disabled) { placeOrderButtonRef.current.disabled = false @@ -564,6 +577,9 @@ export function AdyenPayment({ resultCode, } } + // NOTE: load-bearing, for the reason spelled out in `handleOnAdditionalDetails` + // — the payment is already authorized and `.click()` on a disabled button + // would silently drop the order. if (placeOrderButtonRef?.current != null) { if (placeOrderButtonRef.current.disabled) { placeOrderButtonRef.current.disabled = false From b446e3647cd337e35317b552719479b7d3ed3f91 Mon Sep 17 00:00:00 2001 From: Alessandro Casazza Date: Mon, 31 Aug 2026 17:59:14 +0200 Subject: [PATCH 7/7] feat(place-order): warn when privacy & terms are only half-configured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `placeOrderPermitted` arms the gate on `privacyUrl && termsUrl`, so an order carrying exactly one of the two silently requires no acceptance: the checkbox is never rendered and the button enables with nothing ticked. Nothing distinguishes that from a deliberate opt-out, which makes it the last remaining way to reach a live place-order button without accepting anything. The gate itself is left alone. Requiring acceptance on a single URL would be the worse trade: `` renders a link for each, so one of them would point nowhere — and it would start blocking checkouts that integrators have had working. The ambiguity is surfaced instead, following the `useMissingTermsCheckboxWarning` precedent: development-only, from an effect, and silent in production. Both URLs absent stays quiet — that is the opt-out, not a mistake. Co-Authored-By: Claude Opus 5 (1M context) --- .../half-configured-terms-warning.spec.tsx | 62 +++++++++++++++++++ .../components/orders/PlaceOrderContainer.tsx | 5 ++ .../src/hooks/usePlaceOrder.ts | 5 ++ .../hooks/useHalfConfiguredTermsWarning.ts | 31 ++++++++++ 4 files changed, 103 insertions(+) create mode 100644 packages/react-components/specs/utils/half-configured-terms-warning.spec.tsx create mode 100644 packages/react-components/src/utils/hooks/useHalfConfiguredTermsWarning.ts diff --git a/packages/react-components/specs/utils/half-configured-terms-warning.spec.tsx b/packages/react-components/specs/utils/half-configured-terms-warning.spec.tsx new file mode 100644 index 00000000..3a7f6442 --- /dev/null +++ b/packages/react-components/specs/utils/half-configured-terms-warning.spec.tsx @@ -0,0 +1,62 @@ +// `placeOrderPermitted` arms the privacy & terms gate on `privacyUrl && termsUrl`, so an order +// carrying exactly one of them silently requires no acceptance at all — a checkout that looks +// gated but is not. The behaviour is deliberately left alone (requiring acceptance on one URL +// would render a checkbox linking to nothing), so this warning is the only thing standing +// between that config and a shopper placing an order without ticking anything. +import { renderHook } from "@testing-library/react" +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { useHalfConfiguredTermsWarning } from "#utils/hooks/useHalfConfiguredTermsWarning" + +describe("useHalfConfiguredTermsWarning", () => { + let error: ReturnType + + beforeEach(() => { + error = vi.spyOn(console, "error").mockImplementation(() => {}) + }) + + afterEach(() => { + error.mockRestore() + }) + + it("warns when only the terms URL is set", () => { + renderHook(() => useHalfConfiguredTermsWarning(undefined, "https://example.com/terms")) + expect(error).toHaveBeenCalledOnce() + expect(error.mock.calls[0]?.[0]).toContain("Only one of the privacy policy and terms") + }) + + it("warns when only the privacy URL is set", () => { + renderHook(() => useHalfConfiguredTermsWarning("https://example.com/privacy", null)) + expect(error).toHaveBeenCalledOnce() + }) + + it("stays quiet when both are set", () => { + renderHook(() => + useHalfConfiguredTermsWarning("https://example.com/privacy", "https://example.com/terms") + ) + expect(error).not.toHaveBeenCalled() + }) + + // Neither URL is a deliberate opt-out, not a mistake: the gate is off and the shopper is + // never shown a checkbox, so there is nothing to warn about. + it("stays quiet when neither is set", () => { + renderHook(() => useHalfConfiguredTermsWarning(undefined, undefined)) + expect(error).not.toHaveBeenCalled() + }) + + it("treats an empty string as absent", () => { + renderHook(() => useHalfConfiguredTermsWarning("", "")) + expect(error).not.toHaveBeenCalled() + }) + + it("stays quiet in production", () => { + const previous = process.env.NODE_ENV + vi.stubEnv("NODE_ENV", "production") + try { + renderHook(() => useHalfConfiguredTermsWarning(undefined, "https://example.com/terms")) + expect(error).not.toHaveBeenCalled() + } finally { + vi.stubEnv("NODE_ENV", previous ?? "test") + vi.unstubAllEnvs() + } + }) +}) diff --git a/packages/react-components/src/components/orders/PlaceOrderContainer.tsx b/packages/react-components/src/components/orders/PlaceOrderContainer.tsx index ece53403..e9b8b78f 100644 --- a/packages/react-components/src/components/orders/PlaceOrderContainer.tsx +++ b/packages/react-components/src/components/orders/PlaceOrderContainer.tsx @@ -19,6 +19,7 @@ import placeOrderReducer, { setPlaceOrderStatus, } from "#reducers/PlaceOrderReducer" import useCustomContext from "#utils/hooks/useCustomContext" +import { useHalfConfiguredTermsWarning } from "#utils/hooks/useHalfConfiguredTermsWarning" import { useMissingTermsCheckboxWarning } from "#utils/hooks/useMissingTermsCheckboxWarning" import { useOrganizationConfig } from "#utils/organization" import { getAcceptedSnapshot, subscribe as subscribeToTerms } from "#utils/termsAcceptanceStore" @@ -119,6 +120,10 @@ export function PlaceOrderContainer(props: Props): JSX.Element { } }, [order, include, includeLoaded, organizationConfig, termsAccepted]) useMissingTermsCheckboxWarning(state.termsBlocking, orderId) + useHalfConfiguredTermsWarning( + organizationConfig?.urls?.privacy ?? order?.privacy_url, + organizationConfig?.urls?.terms ?? order?.terms_url + ) const contextValue = { ...state, diff --git a/packages/react-components/src/hooks/usePlaceOrder.ts b/packages/react-components/src/hooks/usePlaceOrder.ts index 38347275..ebfdea79 100644 --- a/packages/react-components/src/hooks/usePlaceOrder.ts +++ b/packages/react-components/src/hooks/usePlaceOrder.ts @@ -10,6 +10,7 @@ import placeOrderReducer, { setPlaceOrder, setPlaceOrderStatus, } from "#reducers/PlaceOrderReducer" +import { useHalfConfiguredTermsWarning } from "#utils/hooks/useHalfConfiguredTermsWarning" import { useMissingTermsCheckboxWarning } from "#utils/hooks/useMissingTermsCheckboxWarning" import { useOrganizationConfig } from "#utils/organization" import { getAcceptedSnapshot, subscribe as subscribeToTerms } from "#utils/termsAcceptanceStore" @@ -101,6 +102,10 @@ export function usePlaceOrder({ }, [order, include, includeLoaded, organizationConfig, isStandalone, termsAccepted]) useMissingTermsCheckboxWarning(isStandalone ? state.termsBlocking : false, orderId) + useHalfConfiguredTermsWarning( + organizationConfig?.urls?.privacy ?? order?.privacy_url, + organizationConfig?.urls?.terms ?? order?.terms_url + ) const setButtonRefCallback = useCallback( (ref: RefObject) => setButtonRef(ref, dispatch), diff --git a/packages/react-components/src/utils/hooks/useHalfConfiguredTermsWarning.ts b/packages/react-components/src/utils/hooks/useHalfConfiguredTermsWarning.ts new file mode 100644 index 00000000..ddd4037a --- /dev/null +++ b/packages/react-components/src/utils/hooks/useHalfConfiguredTermsWarning.ts @@ -0,0 +1,31 @@ +import { useEffect } from "react" + +const MESSAGE = + "[PlaceOrderButton] Only one of the privacy policy and terms of service URLs is set, so acceptance is NOT required and the button will enable without the shopper ticking anything. The gate gets armed by both URLs together, because links to both. Set the missing one on the order (privacy_url / terms_url) or in the organization config, or clear the other one to opt out deliberately." + +/** + * Warns, in development only, when privacy & terms are half-configured. + * + * `placeOrderPermitted` arms the gate on `privacyUrl && termsUrl`, so exactly + * one URL silently means "no acceptance required" — a checkout that looks + * gated but is not. Requiring acceptance on one URL instead would be worse: + * the checkbox renders a link for each, and one of them would point nowhere. + * So the behaviour is left alone and the ambiguity is made visible instead. + * + * @param privacyUrl - Resolved privacy policy URL, if any. + * @param termsUrl - Resolved terms of service URL, if any. + */ +export function useHalfConfiguredTermsWarning( + privacyUrl: string | null | undefined, + termsUrl: string | null | undefined +): void { + useEffect(() => { + const hasPrivacy = Boolean(privacyUrl) + const hasTerms = Boolean(termsUrl) + if (hasPrivacy === hasTerms) return + if (process.env.NODE_ENV === "production") return + console.error(MESSAGE) + }, [privacyUrl, termsUrl]) +} + +export default useHalfConfiguredTermsWarning