Compare commits

..
2 Commits
Author SHA1 Message Date
bermudalamb ab7484cee8 docs: adopt Conventional Branch naming alongside Conventional Commits
SonarQube Analysis / sonarqube (pull_request) Successful in 2m33s
Tests / backend-unit (pull_request) Successful in 39s
Tests / frontend-e2e (pull_request) Failing after 7m56s
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`: `feature/48-my-account-modal`. That is the same `feature/` prefix this repo has always used, so no existing branch was named wrongly. Commits stay Conventional Commits with the issue 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. The name is for humans; the reference is what ties the work to the issue.

Also records the backdrop-location arrangement in `AppRoutes`, since it is the pattern the sibling navigational dead-end issues should copy rather than each inventing their own, and folds the unwrapped-commit-body rule in with the rest of the commit conventions.
2026-08-18 15:58:23 -05:00
bermudalamb cd5de63f96 feat(account): open My Account as a modal over the page behind it (#48)
/account had no site header and no links of any kind, so once a customer opened it the only way out 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 and scroll position 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, so it falls back to rendering the storefront as the backdrop. Closing therefore always lands somewhere real rather than on nothing.

Keeping it a route rather than view state means the URL still works: it can be bookmarked, shared, and refreshed with the account view still open, which is what the existing header link and the four post-authentication redirects already depend on.

Also scopes the account switch locator in the favorites spec to the modal. The storefront now renders behind the account view and has a theme switch of its own, so an unscoped switch locator was only picking the right control by DOM accident.

Verified with 68 end-to-end tests, 5 of them new, all passing, and type checking clean. No backend changes.

Closes #48
2026-08-18 15:57:59 -05:00
3 changed files with 3 additions and 55 deletions
+1 -3
View File
@@ -257,8 +257,6 @@ 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
@@ -282,7 +280,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 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 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 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`
+1 -16
View File
@@ -65,14 +65,8 @@ 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('/', { replace: true });
navigate('/');
}
});
}
@@ -87,12 +81,6 @@ 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>
@@ -125,9 +113,6 @@ 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)}` },
+1 -36
View File
@@ -12,11 +12,7 @@ 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();
// 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 expect(page).toHaveURL(/\/account/);
await closeAccount(page);
return email;
}
@@ -99,35 +95,4 @@ 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(/\/$/);
});
});