48 my account modal #57

Merged
bermudalamb merged 2 commits from 48-my-account-modal into main 2026-08-18 16:39:08 -05:00
6 changed files with 244 additions and 28 deletions
+9 -2
View File
@@ -213,8 +213,12 @@ sudo docker exec -it redefined-designs-db-syn psql -U redefined -d redefined -c
## Conventions
- **Every commit message follows [Conventional Commits](https://www.conventionalcommits.org/)**: `feat:`, `fix:`, `chore:`, `docs:`, `test:`, `ci:`, `refactor:`, etc.
- **Never commit directly to `main`.** Always branch: `feature/<short-description>` or `fix/<short-description>`. Open a PR, merge, branch auto-deletes (repo setting is on).
- **Never commit directly to `main`.** Always branch, open a PR, merge; the branch auto-deletes (repo setting is on).
- **Branches follow [Conventional Branch](https://conventional-branch.github.io/), with the issue number carried for Gitea:** `<type>/<issue-number>-<short-slug>`, e.g. `feature/48-my-account-modal`, `bugfix/57-cart-total-wrong`. Types are `feature`, `bugfix`, `hotfix`, `release`, `chore` — the same `feature/` prefix this repo has always used, so nothing in the existing history is wrong. Drop the number when there is no issue behind the work (`chore/tidy-dead-routes`). The type should agree with the Conventional Commit type of the work it carries.
- **Commits follow [Conventional Commits](https://www.conventionalcommits.org/)** — `feat:`, `fix:`, `chore:`, `docs:`, `test:`, `ci:`, `refactor:` — with the issue number appended to the subject: `feat(account): open My Account as a modal (#48)`.
- **Put `Closes #48` in the commit body**, on its own line, for the commit that completes the issue (`Refs #48` when it only contributes). This is what actually closes the issue on merge, independently of whether the PR description repeats it.
- **Be clear about which part does the linking.** Gitea creates the reference from a `#48` appearing in a *commit message or PR* — never from the branch name. The branch name is for humans reading `git branch`; the reference is what ties the work to the issue. Both are wanted, but only one of them links.
- **Commit bodies are unwrapped paragraphs** — no hard line breaks inside a paragraph.
- Local dev/editing happens in **VS Code**, pushed via **PowerShell** `git` — the NAS-side `gitc` workflow is *only* for pulling already-merged code down to deploy, never for authoring changes.
- **Thom does the pushing.** Commit locally and stop; don't `git push` on his behalf.
- **When work comes from a Gitea issue, post every clarifying question and its answer back to that issue as a comment** — including the options considered and why the rejected ones were rejected. The issue is the durable record; decisions made in a chat session are invisible to anyone reading it later. Post each round as the answers come in rather than batching everything to the end.
@@ -253,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
@@ -276,6 +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 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`
+8 -2
View File
@@ -1,7 +1,7 @@
import { useEffect, useState, useCallback, useMemo } from 'react';
import { Layout, Typography, Switch, Row, Col, Spin, Button, theme, Badge, Empty, Alert } from 'antd';
import { ShoppingCartOutlined, FilterOutlined } from '@ant-design/icons';
import { Link, useSearchParams } from 'react-router-dom';
import { Link, useLocation, useSearchParams } from 'react-router-dom';
import { Item, FilterOptions, fetchItems, fetchFilterOptions } from './api';
import ItemCard from './components/ItemCard';
import FilterDrawer from './components/FilterDrawer';
@@ -33,6 +33,7 @@ export default function App() {
const [drawerOpen, setDrawerOpen] = useState(false);
const [authModalOpen, setAuthModalOpen] = useState(false);
const [searchParams, setSearchParams] = useSearchParams();
const location = useLocation();
const { mode, toggle } = useThemeMode();
const { customer, loading: authLoading } = useCustomerAuth();
const { items: cartItems } = useCart();
@@ -127,8 +128,13 @@ export default function App() {
<Button icon={<ShoppingCartOutlined />} />
</Badge>
</Link>
{/* The account link names the page to render behind the modal, so
closing it comes back here — filters and all — rather than to a
default. */}
{customer ? (
<Link to="/account"><Button>My Account</Button></Link>
<Link to="/account" state={{ background: location }}>
<Button>My Account</Button>
</Link>
) : (
<>
<Link to="/login"><Button>Log in</Button></Link>
+37 -7
View File
@@ -1,5 +1,5 @@
import { useEffect, useState } from 'react';
import { Card, Typography, Switch, Button, Table, Modal, message, Space, Divider } from 'antd';
import { Typography, Switch, Button, Table, Modal, message, Space, Divider } from 'antd';
import { useNavigate } from 'react-router-dom';
import { fetchMyOrders, OrderHistoryItem, updateConsent, exportMyData, deleteMyAccount } from './customerApi';
import { setFavoriteAlerts } from './favoritesApi';
@@ -7,7 +7,13 @@ import { useCustomerAuth } from './CustomerAuthContext';
const { Title, Text } = Typography;
export default function Account() {
interface Props {
// Supplied by the route, which decides where closing lands: back to the page
// the customer came from, or to the storefront when they arrived directly.
onClose: () => void;
}
export default function Account({ onClose }: Props) {
const { customer, loading, refresh, logout } = useCustomerAuth();
const [orders, setOrders] = useState<OrderHistoryItem[]>([]);
const navigate = useNavigate();
@@ -59,16 +65,37 @@ export default function Account() {
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 });
}
});
}
return (
<div style={{ maxWidth: 700, margin: '48px auto', padding: '0 16px' }}>
<Card>
<Title level={3}>My Account</Title>
<Modal
open
// The account view is a place a customer can be sent by an email link or
// a bookmark, so it is titled and closable rather than relying on the
// page behind it to say where they are.
title="My Account"
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>
<Text>{customer.email}</Text>
{!customer.email_verified && (
<div style={{ marginTop: 8 }}>
@@ -98,6 +125,9 @@ export default function Account() {
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)}` },
@@ -112,7 +142,7 @@ export default function Account() {
<Button onClick={handleLogout}>Log out</Button>
<Button danger onClick={handleDelete}>Delete my account</Button>
</Space>
</Card>
</div>
</Modal>
);
}
+50 -13
View File
@@ -1,6 +1,7 @@
import React, { useEffect, useState } from 'react';
import ReactDOM from 'react-dom/client';
import { BrowserRouter, Routes, Route } from 'react-router-dom';
import { BrowserRouter, Routes, Route, useLocation, useNavigate } from 'react-router-dom';
import type { Location } from 'react-router-dom';
import { ConfigProvider, theme as antdTheme } from 'antd';
import 'antd/dist/reset.css';
import App from './App';
@@ -26,6 +27,11 @@ const DARK_ACCENT = '#f0f0f0';
const REDUCED_MOTION_QUERY = '(prefers-reduced-motion: reduce)';
// What /account renders over when it was entered directly — a bookmark, a link
// in an email, or a redirect after signing in. There is no page behind in that
// case, and a modal floating on nothing has nowhere to close back to.
const STOREFRONT_BACKDROP: Partial<Location> = { pathname: '/', search: '', hash: '' };
// Respects the OS-level "reduce motion" accessibility setting by turning off
// antd's transitions. Beyond the accessibility win, animated popups are a
// standing source of flake in end-to-end tests, which drive the app with this
@@ -45,6 +51,48 @@ function usePrefersReducedMotion(): boolean {
return prefers;
}
// /account is a route that renders as a modal over whatever the customer was
// looking at, rather than a page of its own. It stays a real, linkable URL —
// bookmarkable, refreshable, and closed by the browser's Back button — while
// never being a place with no way out of it.
function AppRoutes() {
const location = useLocation();
const navigate = useNavigate();
const state = location.state as { background?: Location } | null;
const isAccount = location.pathname === '/account';
// In-app navigation names the page to render behind. Anything else — a
// bookmark, an email link, the redirect after registering — falls back to the
// storefront, so closing always lands somewhere real.
const background = state?.background;
const backdrop = isAccount ? background ?? { ...location, ...STOREFRONT_BACKDROP } : location;
function closeAccount() {
// Back, when there is somewhere to go back to, so closing the modal and
// pressing Back do the same thing and neither leaves a dead entry behind.
if (background) navigate(-1);
else navigate('/', { replace: true });
}
return (
<>
<Routes location={backdrop as Location}>
<Route path="/" element={<App />} />
<Route path="/admin" element={<Admin />} />
<Route path="/login" element={<Login />} />
<Route path="/register" element={<Register />} />
<Route path="/cart" element={<Cart />} />
<Route path="/privacy" element={<PrivacyPolicy />} />
<Route path="/verify-email" element={<VerifyEmail />} />
<Route path="/forgot-password" element={<ForgotPassword />} />
<Route path="/reset-password" element={<ResetPassword />} />
</Routes>
{/* Rendered outside the Routes above, which are showing the backdrop. */}
{isAccount && <Account onClose={closeAccount} />}
</>
);
}
function Root() {
const { mode } = useThemeMode();
const prefersReducedMotion = usePrefersReducedMotion();
@@ -66,18 +114,7 @@ function Root() {
}}
>
<BrowserRouter>
<Routes>
<Route path="/" element={<App />} />
<Route path="/admin" element={<Admin />} />
<Route path="/login" element={<Login />} />
<Route path="/register" element={<Register />} />
<Route path="/account" element={<Account />} />
<Route path="/cart" element={<Cart />} />
<Route path="/privacy" element={<PrivacyPolicy />} />
<Route path="/verify-email" element={<VerifyEmail />} />
<Route path="/forgot-password" element={<ForgotPassword />} />
<Route path="/reset-password" element={<ResetPassword />} />
</Routes>
<AppRoutes />
</BrowserRouter>
</ConfigProvider>
);
+133
View File
@@ -0,0 +1,133 @@
import { test, expect, Page } from '@playwright/test';
const PASSWORD = 'supersecret123';
const uniqueEmail = () => `account-${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}@example.com`;
// Registering lands on /account, which is now the modal over the storefront.
// Returns the address so a test can assert the right account is shown.
async function registerAndCloseAccount(page: Page): Promise<string> {
const email = uniqueEmail();
await page.goto('/register');
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 closeAccount(page);
return email;
}
const accountModal = (page: Page) => page.getByRole('dialog', { name: 'My Account' });
async function closeAccount(page: Page) {
await accountModal(page).getByRole('button', { name: 'Close' }).click();
await expect(accountModal(page)).toBeHidden();
}
test.describe('My Account opens as a modal', () => {
test('opens over the storefront and closes back to it, filters and all', async ({ page }) => {
await registerAndCloseAccount(page);
// A filtered view, to prove closing restores where the customer actually
// was rather than a bare storefront.
await page.goto('/?max_price=50000');
await expect(page.getByRole('button', { name: 'My Account' })).toBeVisible();
await page.getByRole('button', { name: 'My Account' }).click();
await expect(accountModal(page)).toBeVisible();
// Still a real, linkable URL rather than hidden view state.
await expect(page).toHaveURL(/\/account/);
await closeAccount(page);
await expect(page).toHaveURL(/max_price=50000/);
});
test('the browser back button closes it, the same as the close control', async ({ page }) => {
await registerAndCloseAccount(page);
await page.goto('/?max_price=50000');
await page.getByRole('button', { name: 'My Account' }).click();
await expect(accountModal(page)).toBeVisible();
await page.goBack();
await expect(accountModal(page)).toBeHidden();
await expect(page).toHaveURL(/max_price=50000/);
});
test('a direct visit renders the storefront behind it, so closing lands somewhere real', async ({ page }) => {
await registerAndCloseAccount(page);
// A bookmark, or the link in a verification email. There is no page behind
// in this case, which is what used to make /account a dead end.
await page.goto('/account');
await expect(accountModal(page)).toBeVisible();
await expect(page.getByRole('heading', { name: 'Redefined Designs' })).toBeVisible();
await closeAccount(page);
await expect(page).toHaveURL(/\/$/);
await expect(page.getByRole('button', { name: 'My Account' })).toBeVisible();
});
test('survives a reload, since it is a route rather than view state', async ({ page }) => {
const email = await registerAndCloseAccount(page);
await page.goto('/?max_price=50000');
await page.getByRole('button', { name: 'My Account' }).click();
await expect(accountModal(page)).toBeVisible();
await page.reload();
await expect(accountModal(page)).toBeVisible();
await expect(accountModal(page)).toContainText(email);
});
test('shows the signed-in account and its settings', async ({ page }) => {
const email = await registerAndCloseAccount(page);
await page.goto('/account');
const modal = accountModal(page);
await expect(modal).toContainText(email);
await expect(modal).toContainText('Order History');
// Scoped to the modal: the storefront behind it has a theme switch of its
// 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(/\/$/);
});
});
+6 -3
View File
@@ -130,10 +130,13 @@ test.describe('Favoriting items', () => {
await page.request.put('/api/customers/me/favorite-alerts', { data: { enabled: true } });
await page.goto('/account');
const toggle = page.getByText('Email me when an item I favorited is sold');
await expect(toggle).toBeVisible();
// Scoped to the account modal. The storefront renders behind it and has a
// theme switch of its own, so an unscoped switch locator only picks the
// right control by DOM accident.
const account = page.getByRole('dialog', { name: 'My Account' });
await expect(account.getByText('Email me when an item I favorited is sold')).toBeVisible();
await page.getByRole('switch').last().click();
await account.getByRole('switch').last().click();
await expect(page.getByText('Turned off')).toBeVisible();
const me = await (await page.request.get('/api/customers/me')).json();