fix(cart): the demo notice claims the shop is not taking payments, which is false when PayPal is configured #203

Closed
opened 2026-08-28 11:39:39 -05:00 by bermudalamb · 0 comments
Owner

#195 added a notice to the cart's Checkout card reading "Demonstration only — This shop is not taking payments at the moment. Placing an order here costs nothing, and nothing will be shipped." It is gated on config?.demoMode alone (frontend/src/cart/Cart.tsx:248-256).

demoMode and paypalClientId are independent. demoMode is DEMO_MODE !== 'false' (backend/src/app.ts:50), and checkPayPal (backend/src/envValidation.ts:98-107) only requires PayPal credentials when DEMO_MODE=false — it never forbids them when it is true. docker-compose.prod.yml interpolates both independently.

So there is a reachable configuration where demo mode is on and PayPal is configured. In it, Cart.tsx:258 renders live PayPal buttons directly beneath a banner telling the customer the shop is not taking payments.

It is not hypothetical — it is the documented cutover sequence

docs/ops/production-stack-cutover.md:65 states that flipping DEMO_MODE to false without all three PayPal secrets present crash-loops the container. The only safe order is therefore to populate the PayPal secrets while DEMO_MODE is still true, confirm the stack is healthy, and then flip. That window is exactly the broken configuration, and it is the window in which someone is most likely to be clicking around production checking their work.

Why this is worse than the bug it came from

#195 fixed a case where the UI was silent when it needed to warn. This is the same failure pointed the other way: the UI is confidently wrong in the one configuration where a customer can actually be charged. Telling someone "nothing will be shipped" above a live PayPal button is worse than saying nothing at all.

Fix

Make the notice describe the button rather than the shop, so it stays true in both configurations and remains visible where PayPal is present — which is where a customer most needs to know the two controls do different things. Something along the lines of: "The Checkout (Demo) button below places a pretend order: nothing is charged and nothing will be shipped."

Gating the notice on demoMode && !paypalClientId would also remove the false claim, but it hides the notice in the configuration with two buttons that do different things, which is the worse trade.

Also: the test for this notice does not test what it claims

frontend/tests/e2e/demo-checkout.spec.ts:46-62 is titled "says so before the customer commits, not only on the button", and the requirement is that the notice appears before an address is selected. But line 54 seeds the address with isDefault: true, and Cart.tsx:63-64 auto-selects the default on load — so an address is selected by the time it asserts.

The test would pass unchanged if someone moved the Alert inside the selectedAddressId guard, which is the regression it exists to catch. Dropping the address creation from that test makes the claim real: with no address, the button is absent and the notice must still be visible. Asserting toHaveCount(0) on the checkout button alongside it states the property directly.

Found by

Code review of 456e1aa (#195), raised as Important 1 and 2. The reviewer endorsed the deviation on the type prop and added a reason worth recording: unconditional primary would give the demo button equal visual weight to the real PayPal buttons in this same window.

#195 added a notice to the cart's Checkout card reading **"Demonstration only — This shop is not taking payments at the moment. Placing an order here costs nothing, and nothing will be shipped."** It is gated on `config?.demoMode` alone (`frontend/src/cart/Cart.tsx:248-256`). `demoMode` and `paypalClientId` are independent. `demoMode` is `DEMO_MODE !== 'false'` (`backend/src/app.ts:50`), and `checkPayPal` (`backend/src/envValidation.ts:98-107`) only *requires* PayPal credentials when `DEMO_MODE=false` — it never forbids them when it is `true`. `docker-compose.prod.yml` interpolates both independently. So there is a reachable configuration where demo mode is on **and** PayPal is configured. In it, `Cart.tsx:258` renders live PayPal buttons directly beneath a banner telling the customer the shop is not taking payments. ## It is not hypothetical — it is the documented cutover sequence `docs/ops/production-stack-cutover.md:65` states that flipping `DEMO_MODE` to `false` without all three PayPal secrets present crash-loops the container. The only safe order is therefore to populate the PayPal secrets **while `DEMO_MODE` is still `true`**, confirm the stack is healthy, and then flip. That window is exactly the broken configuration, and it is the window in which someone is most likely to be clicking around production checking their work. ## Why this is worse than the bug it came from #195 fixed a case where the UI was silent when it needed to warn. This is the same failure pointed the other way: the UI is confidently wrong in the one configuration where a customer can actually be charged. Telling someone "nothing will be shipped" above a live PayPal button is worse than saying nothing at all. ## Fix Make the notice describe the **button** rather than the shop, so it stays true in both configurations and remains visible where PayPal is present — which is where a customer most needs to know the two controls do different things. Something along the lines of: *"The Checkout (Demo) button below places a pretend order: nothing is charged and nothing will be shipped."* Gating the notice on `demoMode && !paypalClientId` would also remove the false claim, but it hides the notice in the configuration with two buttons that do different things, which is the worse trade. ## Also: the test for this notice does not test what it claims `frontend/tests/e2e/demo-checkout.spec.ts:46-62` is titled *"says so before the customer commits, not only on the button"*, and the requirement is that the notice appears before an address is selected. But line 54 seeds the address with `isDefault: true`, and `Cart.tsx:63-64` auto-selects the default on load — so an address **is** selected by the time it asserts. The test would pass unchanged if someone moved the `Alert` inside the `selectedAddressId` guard, which is the regression it exists to catch. Dropping the address creation from that test makes the claim real: with no address, the button is absent and the notice must still be visible. Asserting `toHaveCount(0)` on the checkout button alongside it states the property directly. ## Found by Code review of `456e1aa` (#195), raised as Important 1 and 2. The reviewer endorsed the deviation on the `type` prop and added a reason worth recording: unconditional `primary` would give the demo button equal visual weight to the real PayPal buttons in this same window.
Sign in to join this conversation.
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bermudalamb/redefined-designs#203