bermudalamb
5ef97bef21
The cart showed "2h 15m left" and turned it red in the final hour. Neither updated. Both readings happened during render from the wall clock, and nothing scheduled a re-render — no setInterval anywhere in the file — so the number a customer read was whatever it was when the page loaded, and the warning colour could only appear by accident, because the component had already rendered before the final stretch began. That matters more here than in most shops: every item is one of a kind, so a lapsed reservation is not "buy it later", it is someone else buying the only one. New useNow hook returns the time as state rather than merely forcing a re-render, and that is the point. A component reading Date.now() while rendering produces output that depends on the clock, which React is entitled to assume it does not — react-hooks/purity says so, and this was the only instance in the codebase precisely because it was the only place doing it. Reading `now` from state makes render a function of its inputs again, so the rule is satisfied rather than suppressed. It ticks every 30s, which matches the display's one-minute resolution, and only while the cart holds something, so an empty cart is not waking React forever. A second defect the issue did not mention. The red warning was hardcoded to the final hour, but the hold became admin-configurable in #136 and accepts values as low as half an hour — so on any setting below an hour every item was red from the moment it was reserved, and a warning that is always on is not a warning. It now keys off the last tenth of the item's own added_at-to-expires_at span. Reading it from the item rather than from the setting also means an admin changing the value does not retroactively relabel a reservation granted under the old one. At zero the row keeps saying "expiring…" and the page refetches on each tick while anything is lapsed, so it clears within one interval of the server's sweep actually releasing it. The client cannot know when that lands — the sweep runs every few minutes — so the wording claims imminence, which is true, rather than completion, which is not ours to say. The header badge is refreshed alongside, since it counts held items and goes stale the same way. Verified against the unfixed component, not just the fixed one: two of the three new tests fail on the old code. The third documents the "expiring…" wording rather than the fix, and passes either way — worth having, but it is not evidence. The tests use Playwright's clock control rather than waiting in real time, which also makes them deterministic: without it, "the text changed" would depend on where in the minute the run happened to start. Refs #97
75 lines
3.1 KiB
TypeScript
75 lines
3.1 KiB
TypeScript
import { test, expect, createItem, uniqueSuffix } from './fixtures';
|
|
|
|
/**
|
|
* The countdown is the one part of this app whose output depends on the wall
|
|
* clock, so these use Playwright's clock control rather than waiting in real
|
|
* time. Installing it also makes the assertions deterministic: without it, "the
|
|
* text changed" would depend on where in the minute the test happened to start.
|
|
*/
|
|
test.describe('The cart reservation countdown', () => {
|
|
test('counts down on its own, with no interaction', async ({ page, customer, cart }) => {
|
|
const name = `Held c${uniqueSuffix()}`;
|
|
const item = await createItem(page.request, { name, price: '80' });
|
|
expect((await page.request.post(`/api/cart/items/${item.id}`)).status()).toBe(201);
|
|
|
|
// Installed before navigating: the interval is created on mount, and a clock
|
|
// swapped in afterwards would not be the one it is scheduled against.
|
|
await page.clock.install();
|
|
await cart.goto();
|
|
|
|
const before = await cart.remaining(name).textContent();
|
|
expect(before).toMatch(/left/);
|
|
|
|
// Two minutes, which is four ticks of the 30s interval and enough to move a
|
|
// display with one-minute resolution.
|
|
await page.clock.fastForward('02:00');
|
|
|
|
// The assertion the issue exists for: the text changes with nothing
|
|
// clicked, reloaded or navigated. Before this, it was a still photograph of
|
|
// the moment the page rendered.
|
|
await expect(cart.remaining(name)).not.toHaveText(before ?? '');
|
|
});
|
|
|
|
test('warns near the end of the hold rather than for the whole of it', async ({
|
|
page,
|
|
customer,
|
|
cart
|
|
}) => {
|
|
const name = `Waning c${uniqueSuffix()}`;
|
|
const item = await createItem(page.request, { name, price: '80' });
|
|
expect((await page.request.post(`/api/cart/items/${item.id}`)).status()).toBe(201);
|
|
|
|
await page.clock.install();
|
|
await cart.goto();
|
|
|
|
// Freshly reserved, so it is not urgent. antd renders `type="danger"` as a
|
|
// class rather than an attribute, which is why this asserts on the class.
|
|
await expect(cart.remaining(name)).not.toHaveClass(/ant-typography-danger/);
|
|
|
|
// Past 90% of the default 24-hour hold, so inside the final tenth. The old
|
|
// hardcoded "final hour" threshold would have been silent here.
|
|
await page.clock.fastForward('22:00:00');
|
|
await expect(cart.remaining(name)).toHaveClass(/ant-typography-danger/);
|
|
});
|
|
|
|
test('says the hold is expiring once it is past, rather than freezing', async ({
|
|
page,
|
|
customer,
|
|
cart
|
|
}) => {
|
|
const name = `Lapsing c${uniqueSuffix()}`;
|
|
const item = await createItem(page.request, { name, price: '80' });
|
|
expect((await page.request.post(`/api/cart/items/${item.id}`)).status()).toBe(201);
|
|
|
|
await page.clock.install();
|
|
await cart.goto();
|
|
|
|
// Past the deadline. The server releases held items on a sweep every few
|
|
// minutes, so between the deadline and that sweep the item is genuinely
|
|
// still held — "expiring…" claims imminence rather than completion.
|
|
await page.clock.fastForward('25:00:00');
|
|
|
|
await expect(cart.remaining(name)).toHaveText('expiring…');
|
|
});
|
|
});
|