Compare commits

..
2 Commits
Author SHA1 Message Date
bermudalamb 8ad11c6dd6 docs: record branch naming, the modal-route pattern, and two e2e timing traps
SonarQube Analysis / sonarqube (pull_request) Successful in 2m48s
Tests / backend-unit (pull_request) Successful in 33s
Tests / frontend-e2e (pull_request) Failing after 6m27s
Branches follow Conventional Branch — `<type>/<description>` with types feature, bugfix, hotfix, release, chore — carrying the issue number so the work is identifiable from `git branch`. Commits stay Conventional Commits with the number appended to the subject and a `Closes #N` line in the body. Spells out which half does what, because it is easy to assume the branch name links the work: Gitea builds the reference from a `#N` in a commit message or PR and never from the branch name.

Records the backdrop-location arrangement in `AppRoutes` as the pattern the sibling navigational dead-end issues should copy rather than each inventing their own.

Adds two testing lessons that cost real time here: the 5s default expect timeout is too tight for anything waiting on a bcrypt round-trip, and the local database's accumulated junk eventually stops being harmless clutter and starts producing flake that rotates between unrelated specs on every run.
2026-08-18 16:36:20 -05:00
bermudalamb 91485b6ac1 feat(account): open My Account as a modal over the page behind it (#51)
/account had no header and no links of any kind, so once a customer opened it the only way back to the storefront was the browser's back button or editing the URL.

It is now a modal rendered over whatever the customer was looking at, while staying a real route. Opening it from the header pushes /account and names the current page as the backdrop, so closing returns there with filters intact, and the browser's Back button does the same thing as the close control. Entering /account directly — a bookmark, the link in a verification email, or the redirect after registering — has no page behind it and falls back to the storefront, so closing always lands somewhere real. Keeping it a route means the URL still works: bookmarkable, shareable, and refreshable with the view still open, which the header link and the four post-authentication redirects already depend on.

Deleting an account now clears the session as well. Previously it removed the account server-side and navigated home without touching the auth context, so the header went on offering "My Account" for an account that no longer existed until the next reload. That was always wrong, but the modal makes it visible rather than merely stale, because the storefront is rendered behind and the wrong header is on screen throughout.

The modal body is capped and scrolls, and the order history table scrolls within itself, so the view survives a phone without pushing its own title and close control off-screen.

Also scopes the account switch locator in the favorites spec to the modal, since the storefront now renders behind the account view and has a theme switch of its own, and gives the post-registration wait a realistic timeout — it waits on a bcrypt round-trip rather than a render, and the 5s default was surfacing as a flake on whichever test lost the race under parallel load.

Closes #51
2026-08-18 16:36:20 -05:00
3 changed files with 55 additions and 3 deletions
+3 -1
View File
@@ -257,6 +257,8 @@ Integration tests truncate between cases (`resetDb()` in `tests/integration/setu
- **Tables paginate.** With dozens of accumulated rows a freshly created record often isn't on page 1; `admin-taxonomy.spec.ts` confirms tag creation through the API rather than hunting for the row.
- **Clicking a submit button only dispatches the request.** Wait for the resulting confirmation (`'Tag added'`) before querying the API, or the read races the write. This produced a one-in-four flake until fixed.
- **`isVisible()` does not wait.** It answers about *this instant*, so guarding an optional dialog with `if (await x.isVisible())` loses the race whenever the dialog is still on its way — and an antd modal left open then intercepts every later click, which surfaces as an unrelated element "not found" thirty seconds later. If the dialog is deterministic, click it unconditionally and let the locator auto-wait; only use `isVisible()` when it genuinely may never appear, and even then give it something to wait on first.
- **The 5s default `expect` timeout is too tight for anything waiting on a round-trip.** Registration is a bcrypt hash — about half a second unloaded, and well past 5s when the suite's workers all register at once. The failure surfaces on whichever test lost the race, so it looks like an unrelated flake that moves between runs. Give such assertions an explicit generous timeout; they are asserting *that* the server answered, not how fast.
- **The local database's accumulated junk eventually shows up as flake, not just clutter.** At ~450 items and ~250 customers the storefront render and the parallel bcrypt load together push these round-trips past their timeouts, and the repeated runs also trip the password-reset rate limiter. When failures start rotating between unrelated specs on each run, reset the database before debugging any of them.
- **`fullyParallel: true` means a test that mutates a shared fixture races every other test in the file.** A spec that marked an item sold broke the sibling tests reading that same item. Give any test that changes an item's state its own fixture.
## Known gaps / natural next steps
@@ -280,7 +282,7 @@ Integration tests truncate between cases (`resetDb()` in `tests/integration/setu
- Change admin-configurable settings → `admin_settings` table + `backend/src/routes/adminSettings.ts` + `frontend/src/admin/Settings.tsx`
- Add a new async route → wrap the handler in `asyncRoute()` from `backend/src/asyncRoute.ts`, or a failure will hang the request instead of returning 500
- Change what an item row returns → `backend/src/itemSelect.ts` (one place, used by both the public and admin routes)
- Change how a customer route is framed (modal vs page) → `frontend/src/main.tsx`. `AppRoutes` renders the route table against a *backdrop* location rather than the real one: `/account` is a modal over the page named in `location.state.background`, falling back to the storefront when there is none (a bookmark, an email link, a post-registration redirect). This is the pattern to copy for the sibling dead-end issues (#49, #50) — it keeps the URL real and linkable while making sure closing always lands somewhere. The link that opens it must pass `state={{ background: location }}`, or closing goes to the fallback instead of where the customer was.
- Change how a customer route is framed (modal vs page) → `frontend/src/main.tsx`. `AppRoutes` renders the route table against a *backdrop* location rather than the real one: `/account` is a modal over the page named in `location.state.background`, falling back to the storefront when there is none (a bookmark, an email link, a post-registration redirect). This is the pattern to copy for the sibling dead-end issues (#49, #50) — it came out of #51 it keeps the URL real and linkable while making sure closing always lands somewhere. The link that opens it must pass `state={{ background: location }}`, or closing goes to the fallback instead of where the customer was.
- Change storefront filtering → `backend/src/itemFilters.ts` (parsing + SQL), `backend/src/routes/filters.ts` (`/api/filters`, the drawer's single fetch), `frontend/src/filters.ts` (state, URL round-trip, tree building), `frontend/src/components/FilterDrawer.tsx`, `frontend/src/components/ActiveFilterChips.tsx`
- Add a filter dimension that depends on who is asking → follow the favorites filter (#35). The identity comes from `req.customerId` (`attachCustomer` runs globally, so it is available on the public `/api/items` too) and is passed into `buildItemFilterSql` as an explicit argument — never parsed from the query string, or a hand-edited URL could name another customer. Each route decides what to do when it cannot satisfy the filter: the storefront answers 401, the admin inventory 400, and the builder throws rather than silently dropping the clause and returning everything.
- Change category/tag management → `backend/src/routes/adminCategories.ts`, `backend/src/routes/adminTags.ts`, `frontend/src/admin/Categories.tsx`, `frontend/src/admin/Tags.tsx`
+16 -1
View File
@@ -65,8 +65,14 @@ export default function Account({ onClose }: Props) {
okButtonProps: { danger: true },
onOk: async () => {
await deleteMyAccount();
// The storefront is rendered behind this modal, so without clearing the
// session it goes on showing "My Account" and hiding Sign up for an
// account that no longer exists — visibly stale, not merely stale in
// state. replace, so Back cannot return to /account and bounce to
// /login.
refresh();
message.success('Account deleted');
navigate('/');
navigate('/', { replace: true });
}
});
}
@@ -81,6 +87,12 @@ export default function Account({ onClose }: Props) {
onCancel={onClose}
footer={null}
width={700}
// The view holds profile, two consents, order history, and the account
// controls, which is taller than a phone. Capping the body and letting it
// scroll keeps the title and close control in reach instead of pushing
// them off-screen.
style={{ maxWidth: 'calc(100vw - 32px)', top: 24 }}
styles={{ body: { maxHeight: '70vh', overflowY: 'auto' } }}
destroyOnHidden
>
<div>
@@ -113,6 +125,9 @@ export default function Account({ onClose }: Props) {
size="small"
dataSource={orders}
pagination={false}
// Scrolls within itself rather than widening the modal past the
// viewport on a phone.
scroll={{ x: 'max-content' }}
columns={[
{ title: 'Item', dataIndex: 'item_name' },
{ title: 'Amount', dataIndex: 'amount_cents', render: (v: number) => `$${(v / 100).toFixed(2)}` },
+36 -1
View File
@@ -12,7 +12,11 @@ async function registerAndCloseAccount(page: Page): Promise<string> {
await page.getByRole('textbox', { name: 'Email' }).fill(email);
await page.getByLabel('Password').fill(PASSWORD);
await page.getByRole('button', { name: 'Create account' }).click();
await expect(page).toHaveURL(/\/account/);
// Registration is a bcrypt round-trip, not a render. Unloaded it takes about
// half a second; with the suite's workers all registering at once it can pass
// Playwright's 5s default, which shows up as a failure on whichever test lost
// the race rather than as the load problem it is.
await expect(page).toHaveURL(/\/account/, { timeout: 20000 });
await closeAccount(page);
return email;
}
@@ -95,4 +99,35 @@ test.describe('My Account opens as a modal', () => {
// own, so an unscoped switch locator would be ambiguous.
await expect(modal.getByRole('switch')).toHaveCount(2);
});
test('deleting the account does not leave the page behind it looking signed in', async ({ page }) => {
await registerAndCloseAccount(page);
await page.goto('/account');
await accountModal(page).getByRole('button', { name: 'Delete my account' }).click();
await page.getByRole('dialog', { name: 'Delete your account?' })
.getByRole('button', { name: 'Delete my account' }).click();
// The storefront is rendered behind the modal, so a session left in place
// would visibly go on offering My Account for an account that is gone.
await expect(page.getByRole('button', { name: 'Sign up' })).toBeVisible();
await expect(page.getByRole('button', { name: 'My Account' })).toBeHidden();
await expect(page).toHaveURL(/\/$/);
});
test('stays usable on a phone, with the close control in reach', async ({ page }) => {
await registerAndCloseAccount(page);
await page.setViewportSize({ width: 390, height: 664 });
await page.goto('/account');
const modal = accountModal(page);
await expect(modal).toBeVisible();
// The view is taller than the viewport, so the body scrolls rather than
// pushing the title and close control off-screen.
await expect(modal.getByRole('button', { name: 'Close' })).toBeInViewport();
await expect(modal.getByText('Order History')).toBeVisible();
await closeAccount(page);
await expect(page).toHaveURL(/\/$/);
});
});