From f4a036ed51aa20bb7f070167dc9aac66b59b2589 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Fri, 28 Aug 2026 11:51:02 -0500 Subject: [PATCH] fix(cart): say what the demo button does rather than what the shop does (#203) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #195 added a notice reading "Demonstration only — This shop is not taking payments at the moment", gated on `demoMode` alone. That claim is false in a configuration the ops runbook actively steers towards. `demoMode` and `paypalClientId` are independent. `checkDemoMode` and `checkPayPal` only make the PayPal secrets *required* when `DEMO_MODE=false`; nothing forbids them while it is `true`. And `production-stack-cutover.md:65` says flipping to `false` without all three crash-loops the container — so the only safe order is to populate the secrets while demo mode is still on, verify, then flip. In that window `Cart.tsx` renders live PayPal buttons directly beneath a banner telling the customer the shop takes no payments, and it is precisely the window in which someone is clicking around production checking their work. That is the same failure #195 fixed, pointed the other way: silent where a warning was needed, then confidently wrong where a customer can actually be charged. Telling someone nothing will be shipped above a live PayPal button is worse than saying nothing. The notice now describes the button instead of the shop, which is true in both configurations and stays visible in the one with two controls that do different things — where a customer most needs to be told they differ. Gating it on `!paypalClientId` would also have removed the false claim, by hiding the notice exactly there, which is the worse trade. The test for it was also not testing it. "Says so before the customer commits" seeded an address with `isDefault: true`, and the cart auto-selects the default on load — so an address was already selected and the button already rendered when it asserted. It would have passed with the notice moved inside the `selectedAddressId` guard, which is the regression it exists to catch. It now seeds no address and asserts the notice is up while the checkout button is absent, which states the property directly. Mutation-tested rather than assumed: moving the Alert inside that guard fails the new test, and would not have failed the old one. Verified: 3 end-to-end tests pass against a browser, frontend build clean, lint 0 errors (2 pre-existing warnings in `src/filters.ts`). Closes #203 Refs #195 Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/cart/Cart.tsx | 14 ++++++++++++-- frontend/tests/e2e/demo-checkout.spec.ts | 9 +++++++-- frontend/tests/e2e/pages/CartPage.ts | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/frontend/src/cart/Cart.tsx b/frontend/src/cart/Cart.tsx index ae85d12..f1be4f1 100644 --- a/frontend/src/cart/Cart.tsx +++ b/frontend/src/cart/Cart.tsx @@ -244,14 +244,24 @@ export default function Cart() { and whether or not PayPal is configured. The word on the button is the smaller half of this — it asks the customer to notice a parenthesis on the control they have already decided to press. + + It describes the button rather than the shop, and that is the + point rather than a phrasing preference. demoMode and + paypalClientId are independent — checkPayPal only *requires* + credentials when DEMO_MODE=false, it never forbids them when it + is true — and the documented cutover order is to populate the + PayPal secrets while demo mode is still on, then flip. In that + window live PayPal buttons render directly below this notice, so + anything claiming the shop is not taking payments would be false + exactly where a customer can be charged. See #203. */} {config?.demoMode && ( )} {!selectedAddressId && Select a shipping address to check out.} diff --git a/frontend/tests/e2e/demo-checkout.spec.ts b/frontend/tests/e2e/demo-checkout.spec.ts index 333d398..a638341 100644 --- a/frontend/tests/e2e/demo-checkout.spec.ts +++ b/frontend/tests/e2e/demo-checkout.spec.ts @@ -51,14 +51,19 @@ test.describe('Demo mode says so to the customer', () => { const name = `Demo n${uniqueSuffix()}`; const item = await createItem(page.request, { name, price: '80' }); expect((await page.request.post(`/api/cart/items/${item.id}`)).status()).toBe(201); - expect((await page.request.post('/api/customers/me/addresses', { data: ADDRESS })).ok()).toBe(true); + // Deliberately NO address. The cart auto-selects the default one on load, + // so seeding an address here would mean the button is already rendered and + // this would pass even with the notice moved inside the selectedAddressId + // guard — which is the regression it exists to catch. #203. await cart.goto(); // A label on the button alone asks the customer to notice a parenthesis on // the thing they are already clicking. The notice is the part that has to - // survive someone not reading carefully. + // survive someone not reading carefully, so it has to be up before there is + // anything to press. await expect(cart.demoNotice).toBeVisible({ timeout: 20000 }); + await expect(cart.checkoutButton).toHaveCount(0); }); test('the confirmation says nothing was charged', async ({ page, customer, cart }) => { diff --git a/frontend/tests/e2e/pages/CartPage.ts b/frontend/tests/e2e/pages/CartPage.ts index d4ceaa2..0bf77e6 100644 --- a/frontend/tests/e2e/pages/CartPage.ts +++ b/frontend/tests/e2e/pages/CartPage.ts @@ -21,7 +21,7 @@ export class CartPage { // That is the point of #195: the label was wrong, and a locator naming it in // full would have gone looking for the right button and found nothing. this.checkoutButton = page.getByRole('button', { name: /^Checkout/ }); - this.demoNotice = page.getByText('Demonstration only'); + this.demoNotice = page.getByText('Demo checkout'); } async goto(): Promise {