From f76db6c8fe80297fc474615b69bcdcb3fe7374e5 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Fri, 21 Aug 2026 18:21:11 -0500 Subject: [PATCH 1/2] fix(test): make the compose guard survive a CRLF checkout (#107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard added for #107 finds nothing on a checkout with CRLF line endings, which is every fresh clone on Windows. Splitting on a bare newline leaves a trailing carriage return, the end-of-line anchor in the entry pattern then cannot match, and all ten assertions in the file fail together. Worth being precise about why this shipped, because the process that was supposed to prevent it ran and did not. That guard was fired deliberately before committing: the UPLOADS_DIR line was removed, two tests failed, the line was restored, ten passed. What the exercise never varied was the file's line endings — and by then the working copy happened to be LF, because the backup-and-restore used to fire the guard had rewritten it that way. So the deliberate firing proved the guard catches a missing variable, on a file shaped exactly as the test run had shaped it, and proved nothing about the shape it meets in a clean clone. The failure mode is the one the file already worried about: parsing that matches nothing makes every other assertion vacuously true. Here it failed loudly instead only because the "parsed some entries at all" case exists — which is the case that turned a silent pass into a visible failure, and is the reason this was noticed at all rather than sitting green and checking nothing. Splitting on an optional carriage return fixes it. 172 unit tests pass on the CRLF checkout that was failing. Found while verifying #106, whose branch could not go green until this was fixed, which is why the fix lands there rather than on its own. Refs #107 Co-Authored-By: Claude Opus 5 --- backend/tests/unit/composeEnvironment.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/backend/tests/unit/composeEnvironment.test.ts b/backend/tests/unit/composeEnvironment.test.ts index add172a..b90926c 100644 --- a/backend/tests/unit/composeEnvironment.test.ts +++ b/backend/tests/unit/composeEnvironment.test.ts @@ -28,7 +28,13 @@ const compose = readFileSync(COMPOSE_PATH, 'utf8'); // A mention inside a comment cannot match, because a comment line starts with #. function environmentEntries(source: string): Map { const entries = new Map(); - for (const line of source.split('\n')) { + // The split pattern tolerates carriage returns. A checkout with CRLF line + // endings, which is every fresh clone on Windows, otherwise leaves a stray + // carriage return that the end-of-line anchor below cannot match, and every + // assertion in this file then silently finds nothing at all. That is exactly + // the failure the "parsed some entries" case exists to catch, and it is how + // this guard was found to be broken. + for (const line of source.split(/\r?\n/)) { const match = /^\s+- ([A-Z_0-9]+)=(.*)$/.exec(line); if (match) { entries.set(match[1], match[2].trim()); -- 2.54.0 From b287c077470f45567aca5478876b853fc398f853 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Fri, 21 Aug 2026 18:23:13 -0500 Subject: [PATCH 2/2] feat: capture first and last name so emails can greet informally (#106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registration collected one optional Name, so every greeting had only a whole name to use: "Hi Thom Lamb," rather than "Hi Thom,". Both parts are now captured, and the cart reminder greets by first name. Both are required of anyone new, refused individually rather than as a single "name is required" so a form that filled one and not the other is told which. The columns are nullable even so, and that is deliberate. Marking them NOT NULL would mean backfilling legacy rows with empty strings, which asserts that every customer has a name — untrue of anyone who registered while the field was optional. The table records what is actually the case; the rule that new registrations must supply both lives in the route, where a missing field can produce a message naming it. The backfill splits on the first space, and it is lossy in a way no version of this avoids. "Thom Lamb" becomes Thom and Lamb; "Mary Jane Smith" gets a last name of "Jane Smith"; names that are not two parts fare worse. It was chosen over leaving the columns empty because nothing currently lets a customer correct their own name — PUT /api/customers/me exists but no frontend calls it — so empty would have meant permanently unpersonalised for every existing customer. The migration says so, so nobody later reads backfilled values as data the customer supplied in that shape. Verified against a seeded database rather than reasoned about, because this is the part that cannot be covered by the suite: migrations run in globalSetup before any test, and the old column is gone afterwards. Six representative rows through the real migration gave Thom/Lamb, Mary/"Jane Smith", Cher/null, " Padded Name " trimmed to Padded/Name, and null and whitespace-only names left as null on both. The down migration rejoins the parts and returns all six to their original strings. The old column is dropped rather than kept alongside, so there is one source of truth instead of two that drift. The admin keeps receiving a single composed display name. It only ever shows one — the list cell and the drawer title — and never edits one, so giving it both parts plus the joining logic would be work for no reader. Churn was the bulk of this: 14 backend registrations and 10 end-to-end registration forms. A first attempt at the backend fixtures also added names to login and password-reset payloads, which would still have passed since the server ignores unknown fields, but a login test implying login takes a name is a small lie; that was reverted and redone against register calls only. Verified: 172 unit, 183 integration and 95 end-to-end passing, lint unchanged at 4 backend and 27 frontend warnings. Not covered: the cart reminder itself, which runs from a cron and had no test before this either. The greeting change is a one-line substitution in that query. Refs #106 Co-Authored-By: Claude Opus 5 --- .../1787400000000_split-customer-name.js | 62 +++++++++++++ backend/src/routes/adminCustomers.ts | 6 +- backend/src/routes/customers.ts | 36 ++++++-- backend/src/server.ts | 8 +- .../adminInventory.integration.test.ts | 2 +- .../integration/cart.integration.test.ts | 2 +- .../integration/customers.integration.test.ts | 92 +++++++++++++++++-- .../disableCustomer.integration.test.ts | 4 +- .../integration/favorites.integration.test.ts | 2 +- .../passwordReset.integration.test.ts | 2 +- frontend/src/customer/AuthForm.tsx | 17 +++- frontend/src/customer/customerApi.ts | 15 ++- frontend/tests/e2e/account-modal.spec.ts | 2 + .../tests/e2e/admin-disable-customer.spec.ts | 2 + .../tests/e2e/admin-reserved-items.spec.ts | 4 + frontend/tests/e2e/auth.spec.ts | 17 ++++ frontend/tests/e2e/favorites-filter.spec.ts | 4 + frontend/tests/e2e/favorites.spec.ts | 4 + frontend/tests/e2e/password-reset.spec.ts | 2 + 19 files changed, 246 insertions(+), 37 deletions(-) create mode 100644 backend/migrations/1787400000000_split-customer-name.js diff --git a/backend/migrations/1787400000000_split-customer-name.js b/backend/migrations/1787400000000_split-customer-name.js new file mode 100644 index 0000000..05e6f6e --- /dev/null +++ b/backend/migrations/1787400000000_split-customer-name.js @@ -0,0 +1,62 @@ +exports.up = (pgm) => { + pgm.sql(` + -- Emails greet customers, and a single 'name' column only allows the formal + -- whole name: "Hi Thom Lamb," rather than "Hi Thom,". Splitting it is what + -- makes an informal greeting possible. + -- + -- Both columns are nullable even though registration now requires them. The + -- requirement is enforced in the route, where a missing field can produce a + -- 400 naming it. Marking these NOT NULL would mean backfilling legacy rows + -- with empty strings, which asserts that every customer has a name — and + -- that is not true of anyone who registered while the field was optional. + -- The table should record what is actually the case. + ALTER TABLE customers ADD COLUMN IF NOT EXISTS first_name TEXT; + ALTER TABLE customers ADD COLUMN IF NOT EXISTS last_name TEXT; + + -- The lossy part, and there is no version of this that is not. + -- + -- Splitting on the first space is right for "Thom Lamb" and wrong for + -- "Mary Jane Smith", who ends up with a last name of "Jane Smith". Names do + -- not reliably divide into two parts at all. This was chosen over leaving + -- the columns empty because there is currently no way for a customer to + -- correct their own name — PUT /api/customers/me exists but nothing calls + -- it — so empty would mean permanently unpersonalised for everyone who + -- registered before this. + -- + -- Treat backfilled values as a best guess rather than as data the customer + -- gave you in this shape. + UPDATE customers + SET first_name = CASE + WHEN position(' ' in btrim(name)) > 0 THEN split_part(btrim(name), ' ', 1) + ELSE btrim(name) + END, + last_name = CASE + WHEN position(' ' in btrim(name)) > 0 + THEN btrim(substring(btrim(name) from position(' ' in btrim(name)) + 1)) + ELSE NULL + END + WHERE name IS NOT NULL AND btrim(name) <> ''; + + -- Dropped rather than kept alongside. Two columns describing the same fact + -- drift, and the new pair is now the only place a name lives. + ALTER TABLE customers DROP COLUMN IF EXISTS name; + `); +}; + +exports.down = (pgm) => { + pgm.sql(` + ALTER TABLE customers ADD COLUMN IF NOT EXISTS name TEXT; + + -- Rejoins the parts. Not a perfect inverse of the split above — a name that + -- was mangled on the way in stays mangled on the way out — but it restores + -- a usable whole name rather than leaving the column empty. + UPDATE customers + SET name = btrim(concat_ws(' ', first_name, last_name)) + WHERE first_name IS NOT NULL OR last_name IS NOT NULL; + + UPDATE customers SET name = NULL WHERE name = ''; + + ALTER TABLE customers DROP COLUMN IF EXISTS first_name; + ALTER TABLE customers DROP COLUMN IF EXISTS last_name; + `); +}; diff --git a/backend/src/routes/adminCustomers.ts b/backend/src/routes/adminCustomers.ts index 81753da..e2f19e3 100755 --- a/backend/src/routes/adminCustomers.ts +++ b/backend/src/routes/adminCustomers.ts @@ -7,7 +7,8 @@ const router = Router(); router.get('/', asyncRoute(async (_req: Request, res: Response) => { const { rows } = await pool.query(` SELECT - c.id, c.email, c.name, c.email_verified, c.marketing_consent, c.created_at, c.disabled_at, + c.id, c.email, nullif(btrim(concat_ws(' ', c.first_name, c.last_name)), '') AS name, + c.email_verified, c.marketing_consent, c.created_at, c.disabled_at, COUNT(o.id) FILTER (WHERE o.status = 'completed') AS order_count, COALESCE(SUM(o.amount_cents) FILTER (WHERE o.status = 'completed'), 0) AS total_spent_cents, MAX(o.created_at) AS last_order_at, @@ -141,7 +142,8 @@ router.post('/:id/reserved/:itemId/release', asyncRoute(async (req: Request, res router.get('/:id', asyncRoute(async (req: Request, res: Response) => { const { rows: customerRows } = await pool.query( - `SELECT id, email, name, email_verified, marketing_consent, marketing_consent_at, created_at + `SELECT id, email, nullif(btrim(concat_ws(' ', first_name, last_name)), '') AS name, + email_verified, marketing_consent, marketing_consent_at, created_at FROM customers WHERE id = $1`, [req.params.id] ); diff --git a/backend/src/routes/customers.ts b/backend/src/routes/customers.ts index eae43c9..444ddce 100755 --- a/backend/src/routes/customers.ts +++ b/backend/src/routes/customers.ts @@ -39,7 +39,11 @@ async function createSession(customerId: number): Promise { interface CustomerRow { id: number; email: string; - name: string | null; + // Nullable despite registration requiring both, because customers who + // registered while the field was optional genuinely have no name. The + // requirement is enforced at registration, not asserted by the schema. + first_name: string | null; + last_name: string | null; email_verified: boolean; marketing_consent: boolean; favorite_alerts: boolean; @@ -50,7 +54,8 @@ function publicCustomer(c: CustomerRow) { return { id: c.id, email: c.email, - name: c.name, + first_name: c.first_name, + last_name: c.last_name, email_verified: c.email_verified, marketing_consent: c.marketing_consent, favorite_alerts: c.favorite_alerts, @@ -59,10 +64,20 @@ function publicCustomer(c: CustomerRow) { } router.post('/register', asyncRoute(async (req: Request, res: Response) => { - const { email, password, name, marketingConsent } = req.body; + const { email, password, firstName, lastName, marketingConsent } = req.body; if (!email || !isValidEmail(String(email)) || !password || String(password).length < 8) { return res.status(400).json({ error: 'valid email and password (min 8 chars) required' }); } + // Named individually rather than as one "name is required", so a form that + // filled one field and not the other is told which. + const first = String(firstName ?? '').trim(); + const last = String(lastName ?? '').trim(); + if (!first) { + return res.status(400).json({ error: 'first name is required' }); + } + if (!last) { + return res.status(400).json({ error: 'last name is required' }); + } const normalizedEmail = String(email).toLowerCase().trim(); const { rows: existing } = await pool.query(`SELECT id FROM customers WHERE email = $1`, [normalizedEmail]); if (existing.length) return res.status(409).json({ error: 'an account with this email already exists' }); @@ -72,10 +87,10 @@ router.post('/register', asyncRoute(async (req: Request, res: Response) => { const consent = !!marketingConsent; const { rows } = await pool.query( - `INSERT INTO customers (email, password_hash, name, marketing_consent, marketing_consent_at, marketing_consent_text, unsubscribe_token) - VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING *`, + `INSERT INTO customers (email, password_hash, first_name, last_name, marketing_consent, marketing_consent_at, marketing_consent_text, unsubscribe_token) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING *`, [ - normalizedEmail, passwordHash, name || null, + normalizedEmail, passwordHash, first, last, consent, consent ? new Date() : null, consent ? MARKETING_CONSENT_TEXT : null, unsubscribeToken ] @@ -285,11 +300,14 @@ router.get('/me', requireCustomer, asyncRoute(async (req: Request, res: Response res.json(publicCustomer(rows[0])); })); +// Kept in step with registration for consistency. Note nothing in the frontend +// calls this today — the account page has no name editing — so this is API +// surface without a caller rather than a path in use. router.put('/me', requireCustomer, asyncRoute(async (req: Request, res: Response) => { - const { name } = req.body; + const { firstName, lastName } = req.body; const { rows } = await pool.query( - `UPDATE customers SET name = $1 WHERE id = $2 RETURNING *`, - [name || null, req.customerId] + `UPDATE customers SET first_name = $1, last_name = $2 WHERE id = $3 RETURNING *`, + [String(firstName ?? '').trim() || null, String(lastName ?? '').trim() || null, req.customerId] ); res.json(publicCustomer(rows[0])); })); diff --git a/backend/src/server.ts b/backend/src/server.ts index e8f8e9c..d04004e 100755 --- a/backend/src/server.ts +++ b/backend/src/server.ts @@ -23,7 +23,7 @@ async function sweepExpiredCarts(): Promise { async function sendCartReminders(): Promise { try { const { rows } = await pool.query(` - SELECT c.email, c.name, i.name AS item_name, ci.expires_at, ci.id AS cart_item_id + SELECT c.email, c.first_name, i.name AS item_name, ci.expires_at, ci.id AS cart_item_id FROM cart_items ci JOIN carts ca ON ca.id = ci.cart_id JOIN customers c ON c.id = ca.customer_id @@ -33,9 +33,9 @@ async function sendCartReminders(): Promise { AND ci.expires_at > now() `); - const byEmail = new Map(); + const byEmail = new Map(); for (const row of rows) { - if (!byEmail.has(row.email)) byEmail.set(row.email, { name: row.name, items: [] }); + if (!byEmail.has(row.email)) byEmail.set(row.email, { firstName: row.first_name, items: [] }); byEmail.get(row.email)!.items.push({ name: row.item_name, expiresAt: row.expires_at, cartItemId: row.cart_item_id }); } @@ -44,7 +44,7 @@ async function sendCartReminders(): Promise { await sendMail( email, 'Items waiting in your cart', - `

Hi${data.name ? ' ' + data.name : ''},

+ `

Hi${data.firstName ? ' ' + data.firstName : ''},

You still have items in your cart at Redefined Designs:

    ${itemList}

View your cart before your reservation expires.

` diff --git a/backend/tests/integration/adminInventory.integration.test.ts b/backend/tests/integration/adminInventory.integration.test.ts index 030a36b..a9238fa 100644 --- a/backend/tests/integration/adminInventory.integration.test.ts +++ b/backend/tests/integration/adminInventory.integration.test.ts @@ -42,7 +42,7 @@ async function createItem( async function registerCustomer(email: string) { const agent = request.agent(app); - await agent.post('/api/customers/register').send({ email, password: 'supersecret123' }); + await agent.post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email, password: 'supersecret123' }); const { rows } = await pool.query(`SELECT id FROM customers WHERE email = $1`, [email]); return { agent, id: rows[0].id as number }; } diff --git a/backend/tests/integration/cart.integration.test.ts b/backend/tests/integration/cart.integration.test.ts index d591be1..57ad95e 100644 --- a/backend/tests/integration/cart.integration.test.ts +++ b/backend/tests/integration/cart.integration.test.ts @@ -14,7 +14,7 @@ afterAll(async () => { async function registerAndGetAgent(email: string) { const agent = request.agent(app); - await agent.post('/api/customers/register').send({ email, password: 'supersecret123' }); + await agent.post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email, password: 'supersecret123' }); return agent; } diff --git a/backend/tests/integration/customers.integration.test.ts b/backend/tests/integration/customers.integration.test.ts index 5c4c13e..c0e2b5b 100755 --- a/backend/tests/integration/customers.integration.test.ts +++ b/backend/tests/integration/customers.integration.test.ts @@ -13,11 +13,83 @@ afterAll(async () => { }); describe('POST /api/customers/register', () => { - it('creates an account with marketing consent unchecked by default', async () => { + // Both names are required from anyone new, so the greeting in every email has + // something to use. Refused individually rather than as one "name required", + // so a form that filled one and not the other is told which. See #106. + it('refuses a registration with no first name', async () => { const res = await request(app).post('/api/customers/register').send({ - email: 'jane@example.com', + email: 'nofirst@example.com', password: 'supersecret123', - name: 'Jane' + lastName: 'Customer' + }); + + expect(res.status).toBe(400); + expect(res.body.error).toBe('first name is required'); + }); + + it('refuses a registration with no last name', async () => { + const res = await request(app).post('/api/customers/register').send({ + email: 'nolast@example.com', + password: 'supersecret123', + firstName: 'Test' + }); + + expect(res.status).toBe(400); + expect(res.body.error).toBe('last name is required'); + }); + + it('refuses names that are only whitespace', async () => { + const res = await request(app).post('/api/customers/register').send({ + email: 'blank@example.com', + password: 'supersecret123', + firstName: ' ', + lastName: 'Customer' + }); + + expect(res.status).toBe(400); + expect(res.body.error).toBe('first name is required'); + }); + + it('stores both names, trimmed, and returns them', async () => { + const res = await request(app).post('/api/customers/register').send({ + email: 'named@example.com', + password: 'supersecret123', + firstName: ' Thom ', + lastName: ' Lamb ' + }); + + expect(res.status).toBe(200); + expect(res.body.first_name).toBe('Thom'); + expect(res.body.last_name).toBe('Lamb'); + + const { rows } = await pool.query( + `SELECT first_name, last_name FROM customers WHERE email = $1`, + ['named@example.com'] + ); + expect(rows[0]).toEqual({ first_name: 'Thom', last_name: 'Lamb' }); + }); + + // The account is the customer's own, but the shape returned to them should + // not quietly grow — a password hash or a token appearing here is the kind of + // thing that goes unnoticed. + it('does not return anything beyond the public customer shape', async () => { + const res = await request(app).post('/api/customers/register').send({ + email: 'shape@example.com', + password: 'supersecret123', + firstName: 'Test', + lastName: 'Customer' + }); + + expect(Object.keys(res.body).sort()).toEqual([ + 'created_at', 'email', 'email_verified', 'favorite_alerts', + 'first_name', 'id', 'last_name', 'marketing_consent' + ]); + }); + + it('creates an account with marketing consent unchecked by default', async () => { + const res = await request(app).post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', + email: 'jane@example.com', + password: 'supersecret123' }); expect(res.status).toBe(200); expect(res.body.email).toBe('jane@example.com'); @@ -26,7 +98,7 @@ describe('POST /api/customers/register', () => { }); it('respects an explicit marketing opt-in', async () => { - const res = await request(app).post('/api/customers/register').send({ + const res = await request(app).post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email: 'opt-in@example.com', password: 'supersecret123', marketingConsent: true @@ -35,11 +107,11 @@ describe('POST /api/customers/register', () => { }); it('rejects a duplicate email', async () => { - await request(app).post('/api/customers/register').send({ + await request(app).post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email: 'dupe@example.com', password: 'supersecret123' }); - const res = await request(app).post('/api/customers/register').send({ + const res = await request(app).post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email: 'dupe@example.com', password: 'anotherpassword' }); @@ -47,7 +119,7 @@ describe('POST /api/customers/register', () => { }); it('rejects a password under 8 characters', async () => { - const res = await request(app).post('/api/customers/register').send({ + const res = await request(app).post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email: 'short@example.com', password: '123' }); @@ -55,7 +127,7 @@ describe('POST /api/customers/register', () => { }); it('rejects a malformed email', async () => { - const res = await request(app).post('/api/customers/register').send({ + const res = await request(app).post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email: 'not-an-email', password: 'supersecret123' }); @@ -66,7 +138,7 @@ describe('POST /api/customers/register', () => { describe('session-gated routes', () => { it('logs in and can access /me with the returned session cookie', async () => { const agent = request.agent(app); - await agent.post('/api/customers/register').send({ + await agent.post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email: 'login-test@example.com', password: 'supersecret123' }); @@ -81,7 +153,7 @@ describe('session-gated routes', () => { }); it('rejects login with the wrong password', async () => { - await request(app).post('/api/customers/register').send({ + await request(app).post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email: 'wrongpass@example.com', password: 'supersecret123' }); diff --git a/backend/tests/integration/disableCustomer.integration.test.ts b/backend/tests/integration/disableCustomer.integration.test.ts index e4582b3..1174944 100644 --- a/backend/tests/integration/disableCustomer.integration.test.ts +++ b/backend/tests/integration/disableCustomer.integration.test.ts @@ -16,7 +16,7 @@ const PASSWORD = 'supersecret123'; async function register(email: string) { const agent = request.agent(app); - const res = await agent.post('/api/customers/register').send({ email, password: PASSWORD }); + const res = await agent.post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email, password: PASSWORD }); expect(res.status).toBe(200); const { rows } = await pool.query(`SELECT id FROM customers WHERE email = $1`, [email]); return { agent, id: rows[0].id as number }; @@ -160,7 +160,7 @@ describe('disabling a customer', () => { // Otherwise disabling is trivially undone by signing up again. const res = await request(app) .post('/api/customers/register') - .send({ email: 'reregister@example.com', password: PASSWORD }); + .send({ firstName: 'Test', lastName: 'Customer', email: 'reregister@example.com', password: PASSWORD }); expect(res.status).toBe(409); }); }); diff --git a/backend/tests/integration/favorites.integration.test.ts b/backend/tests/integration/favorites.integration.test.ts index 3dab2d6..24e3b56 100644 --- a/backend/tests/integration/favorites.integration.test.ts +++ b/backend/tests/integration/favorites.integration.test.ts @@ -23,7 +23,7 @@ const PASSWORD = 'supersecret123'; async function register(email: string) { const agent = request.agent(app); - expect((await agent.post('/api/customers/register').send({ email, password: PASSWORD })).status).toBe(200); + expect((await agent.post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email, password: PASSWORD })).status).toBe(200); const { rows } = await pool.query(`SELECT id FROM customers WHERE email = $1`, [email]); return { agent, id: rows[0].id as number }; } diff --git a/backend/tests/integration/passwordReset.integration.test.ts b/backend/tests/integration/passwordReset.integration.test.ts index 9d6a1b4..20792ef 100644 --- a/backend/tests/integration/passwordReset.integration.test.ts +++ b/backend/tests/integration/passwordReset.integration.test.ts @@ -16,7 +16,7 @@ const PASSWORD = 'supersecret123'; async function register(email: string) { const agent = request.agent(app); - const res = await agent.post('/api/customers/register').send({ email, password: PASSWORD }); + const res = await agent.post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email, password: PASSWORD }); expect(res.status).toBe(200); return agent; } diff --git a/frontend/src/customer/AuthForm.tsx b/frontend/src/customer/AuthForm.tsx index 34bab4f..7eef1cc 100644 --- a/frontend/src/customer/AuthForm.tsx +++ b/frontend/src/customer/AuthForm.tsx @@ -76,12 +76,23 @@ export default function AuthForm({ mode, onModeChange, onForgotPassword, onSucce layout="vertical" onFinish={(values) => submit(() => - registerCustomer(values.email, values.password, values.name, !!values.marketingConsent) + registerCustomer(values.email, values.password, values.firstName, values.lastName, !!values.marketingConsent) ) } > - - + + + + + diff --git a/frontend/src/customer/customerApi.ts b/frontend/src/customer/customerApi.ts index db6d535..ab0a6d9 100755 --- a/frontend/src/customer/customerApi.ts +++ b/frontend/src/customer/customerApi.ts @@ -1,7 +1,10 @@ export interface Customer { id: number; email: string; - name: string | null; + // Nullable because customers who registered before these were required have + // neither. Registration demands both from anyone new. + first_name: string | null; + last_name: string | null; email_verified: boolean; marketing_consent: boolean; favorite_alerts: boolean; @@ -25,11 +28,17 @@ async function handle(res: Response): Promise { return res.json(); } -export function registerCustomer(email: string, password: string, name: string, marketingConsent: boolean): Promise { +export function registerCustomer( + email: string, + password: string, + firstName: string, + lastName: string, + marketingConsent: boolean +): Promise { return fetch('/api/customers/register', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ email, password, name, marketingConsent }) + body: JSON.stringify({ email, password, firstName, lastName, marketingConsent }) }).then(res => handle(res)); } diff --git a/frontend/tests/e2e/account-modal.spec.ts b/frontend/tests/e2e/account-modal.spec.ts index fbf35de..f490bb5 100644 --- a/frontend/tests/e2e/account-modal.spec.ts +++ b/frontend/tests/e2e/account-modal.spec.ts @@ -14,6 +14,8 @@ async function registerCustomer(page: Page): Promise { const email = uniqueEmail(); await page.goto('/register'); await page.getByRole('textbox', { name: 'Email' }).fill(email); + await page.getByRole('textbox', { name: 'First name' }).fill('Test'); + await page.getByRole('textbox', { name: 'Last name' }).fill('Customer'); await page.getByLabel('Password').fill(PASSWORD); await page.getByRole('button', { name: 'Create account' }).click(); await expect(page.getByRole('button', { name: 'My Account' })).toBeVisible({ timeout: 20000 }); diff --git a/frontend/tests/e2e/admin-disable-customer.spec.ts b/frontend/tests/e2e/admin-disable-customer.spec.ts index 09b2378..6d0748a 100644 --- a/frontend/tests/e2e/admin-disable-customer.spec.ts +++ b/frontend/tests/e2e/admin-disable-customer.spec.ts @@ -6,6 +6,8 @@ const uniqueEmail = () => `disable-${Date.now().toString(36)}${Math.random().toS async function register(page: Page, email: string) { await page.goto('/register'); await page.getByRole('textbox', { name: 'Email' }).fill(email); + await page.getByRole('textbox', { name: 'First name' }).fill('Test'); + await page.getByRole('textbox', { name: 'Last name' }).fill('Customer'); await page.getByLabel('Password').fill(PASSWORD); await page.getByRole('button', { name: 'Create account' }).click(); // Registering now closes the auth modal and returns to the page behind it, so diff --git a/frontend/tests/e2e/admin-reserved-items.spec.ts b/frontend/tests/e2e/admin-reserved-items.spec.ts index 0eee153..d5c6f77 100644 --- a/frontend/tests/e2e/admin-reserved-items.spec.ts +++ b/frontend/tests/e2e/admin-reserved-items.spec.ts @@ -17,6 +17,8 @@ async function reserveItem(page: import('@playwright/test').Page, itemName: stri await page.goto('/register'); await page.getByRole('textbox', { name: 'Email' }).fill(email); + await page.getByRole('textbox', { name: 'First name' }).fill('Test'); + await page.getByRole('textbox', { name: 'Last name' }).fill('Customer'); await page.getByLabel('Password').fill('supersecret123'); await page.getByRole('button', { name: 'Create account' }).click(); await expect(page.getByRole('button', { name: 'My Account' })).toBeVisible({ timeout: 20000 }); @@ -83,6 +85,8 @@ test.describe('Admin reserved items', () => { const email = `idle-${suffix()}@example.com`; await page.goto('/register'); await page.getByRole('textbox', { name: 'Email' }).fill(email); + await page.getByRole('textbox', { name: 'First name' }).fill('Test'); + await page.getByRole('textbox', { name: 'Last name' }).fill('Customer'); await page.getByLabel('Password').fill('supersecret123'); await page.getByRole('button', { name: 'Create account' }).click(); await expect(page.getByRole('button', { name: 'My Account' })).toBeVisible({ timeout: 20000 }); diff --git a/frontend/tests/e2e/auth.spec.ts b/frontend/tests/e2e/auth.spec.ts index 5f887ff..3beb5e4 100755 --- a/frontend/tests/e2e/auth.spec.ts +++ b/frontend/tests/e2e/auth.spec.ts @@ -15,6 +15,8 @@ function uniqueEmail(): string { async function register(page: Page, email: string) { await page.goto('/register'); await page.getByRole('textbox', { name: 'Email' }).fill(email); + await page.getByRole('textbox', { name: 'First name' }).fill('Test'); + await page.getByRole('textbox', { name: 'Last name' }).fill('Customer'); await page.getByLabel('Password').fill(PASSWORD); await page.getByRole('button', { name: 'Create account' }).click(); await expect(page.getByRole('button', { name: 'My Account' })).toBeVisible({ timeout: 20000 }); @@ -27,6 +29,21 @@ async function openAccount(page: Page) { } test.describe('Customer accounts', () => { + // Both names are required from anyone new so every email has a first name to + // greet with (#106). The server refuses without them; this is the form + // refusing first, so nobody gets a round trip to find out. + test('will not submit a registration without both names', async ({ page }) => { + await page.goto('/register'); + await page.getByRole('textbox', { name: 'Email' }).fill(uniqueEmail()); + await page.getByLabel('Password').fill(PASSWORD); + await page.getByRole('button', { name: 'Create account' }).click(); + + await expect(page.getByText('First name is required')).toBeVisible(); + await expect(page.getByText('Last name is required')).toBeVisible(); + // Still on the form rather than signed in. + await expect(page.getByRole('button', { name: 'My Account' })).toHaveCount(0); + }); + test('marketing consent checkbox is unchecked by default', async ({ page }) => { await page.goto('/register'); await expect(page.getByRole('checkbox')).not.toBeChecked(); diff --git a/frontend/tests/e2e/favorites-filter.spec.ts b/frontend/tests/e2e/favorites-filter.spec.ts index f4e1678..b85d7ec 100644 --- a/frontend/tests/e2e/favorites-filter.spec.ts +++ b/frontend/tests/e2e/favorites-filter.spec.ts @@ -30,6 +30,8 @@ test.beforeAll(async ({ playwright }) => { async function register(page: Page) { await page.goto('/register'); await page.getByRole('textbox', { name: 'Email' }).fill(uniqueEmail()); + await page.getByRole('textbox', { name: 'First name' }).fill('Test'); + await page.getByRole('textbox', { name: 'Last name' }).fill('Customer'); await page.getByLabel('Password').fill(PASSWORD); await page.getByRole('button', { name: 'Create account' }).click(); // Registering now closes the auth modal and returns to the page behind it, so @@ -134,6 +136,8 @@ test.describe('Filtering the storefront by favorites', () => { await expect(prompt).toBeVisible(); await prompt.getByRole('textbox', { name: 'Email' }).fill(uniqueEmail()); + await page.getByRole('textbox', { name: 'First name' }).fill('Test'); + await page.getByRole('textbox', { name: 'Last name' }).fill('Customer'); await prompt.getByLabel('Password').fill(PASSWORD); await prompt.getByRole('button', { name: 'Create account' }).click(); diff --git a/frontend/tests/e2e/favorites.spec.ts b/frontend/tests/e2e/favorites.spec.ts index c553def..8c5f429 100644 --- a/frontend/tests/e2e/favorites.spec.ts +++ b/frontend/tests/e2e/favorites.spec.ts @@ -20,6 +20,8 @@ test.beforeAll(async ({ playwright }) => { async function register(page: Page, email: string) { await page.goto('/register'); await page.getByRole('textbox', { name: 'Email' }).fill(email); + await page.getByRole('textbox', { name: 'First name' }).fill('Test'); + await page.getByRole('textbox', { name: 'Last name' }).fill('Customer'); await page.getByLabel('Password').fill(PASSWORD); await page.getByRole('button', { name: 'Create account' }).click(); // Registering now closes the auth modal and returns to the page behind it, so @@ -52,6 +54,8 @@ test.describe('Favoriting items', () => { const email = uniqueEmail(); await page.getByRole('dialog').getByRole('textbox', { name: 'Email' }).fill(email); + await page.getByRole('textbox', { name: 'First name' }).fill('Test'); + await page.getByRole('textbox', { name: 'Last name' }).fill('Customer'); await page.getByRole('dialog').getByLabel('Password').fill(PASSWORD); await page.getByRole('dialog').getByRole('button', { name: 'Create account' }).click(); diff --git a/frontend/tests/e2e/password-reset.spec.ts b/frontend/tests/e2e/password-reset.spec.ts index 4230411..ada1d2c 100644 --- a/frontend/tests/e2e/password-reset.spec.ts +++ b/frontend/tests/e2e/password-reset.spec.ts @@ -9,6 +9,8 @@ const uniqueEmail = () => `reset-${Date.now().toString(36)}${Math.random().toStr async function register(page: Page, email: string) { await page.goto('/register'); await page.getByRole('textbox', { name: 'Email' }).fill(email); + await page.getByRole('textbox', { name: 'First name' }).fill('Test'); + await page.getByRole('textbox', { name: 'Last name' }).fill('Customer'); await page.getByLabel('Password').fill(PASSWORD); await page.getByRole('button', { name: 'Create account' }).click(); // Registering now closes the auth modal and returns to the page behind it, so -- 2.54.0