fix(cart): say what the demo button does rather than what the shop does (#203)
Linting / lint (pull_request) Successful in 2m4s
SonarQube Analysis / sonarqube (pull_request) Failing after 16m53s

#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) <noreply@anthropic.com>
This commit is contained in:
2026-08-28 11:51:02 -05:00
co-authored by Claude Opus 5
parent f0bdc590c9
commit f4a036ed51
3 changed files with 20 additions and 5 deletions
+12 -2
View File
@@ -244,14 +244,24 @@ export default function Cart() {
and whether or not PayPal is configured. The word on the button 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 is the smaller half of this — it asks the customer to notice a
parenthesis on the control they have already decided to press. 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 && ( {config?.demoMode && (
<Alert <Alert
type="warning" type="warning"
showIcon showIcon
style={{ marginBottom: 12 }} style={{ marginBottom: 12 }}
message="Demonstration only" message="Demo checkout"
description="This shop is not taking payments at the moment. Placing an order here costs nothing, and nothing will be shipped." description="The Checkout (Demo) button places a pretend order: nothing is charged, and nothing will be shipped."
/> />
)} )}
{!selectedAddressId && <Text type="warning">Select a shipping address to check out.</Text>} {!selectedAddressId && <Text type="warning">Select a shipping address to check out.</Text>}
+7 -2
View File
@@ -51,14 +51,19 @@ test.describe('Demo mode says so to the customer', () => {
const name = `Demo n${uniqueSuffix()}`; const name = `Demo n${uniqueSuffix()}`;
const item = await createItem(page.request, { name, price: '80' }); 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/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(); await cart.goto();
// A label on the button alone asks the customer to notice a parenthesis on // 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 // 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.demoNotice).toBeVisible({ timeout: 20000 });
await expect(cart.checkoutButton).toHaveCount(0);
}); });
test('the confirmation says nothing was charged', async ({ page, customer, cart }) => { test('the confirmation says nothing was charged', async ({ page, customer, cart }) => {
+1 -1
View File
@@ -21,7 +21,7 @@ export class CartPage {
// That is the point of #195: the label was wrong, and a locator naming it in // 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. // full would have gone looking for the right button and found nothing.
this.checkoutButton = page.getByRole('button', { name: /^Checkout/ }); this.checkoutButton = page.getByRole('button', { name: /^Checkout/ });
this.demoNotice = page.getByText('Demonstration only'); this.demoNotice = page.getByText('Demo checkout');
} }
async goto(): Promise<void> { async goto(): Promise<void> {