Refuse a participant name of only spaces on sign-up and edit

CHANGELOG.md

@@ -54,6 +54,7 @@ All notable changes to OpenSignup are documented here. Format follows [Keep a Ch
 - SMTP sends reuse their connections (#333). nodemailer used to connect, set up TLS and log in for every email, so a busy reminder run opened one connection per reminder, which was slower and could hit a mail provider's connections-per-minute limit (a `421` reply, then a retried job). The SMTP transport now keeps a pool of up to 3 connections open between emails, each replaced after 100 messages; the web server and the worker each keep their own. A connection the mail server closes while idle is dropped, and the next email opens a new one. An email whose connection closes while it is being sent is tried once more on another connection, then fails as before. The connect timeouts, the STARTTLS requirement and the refusal of file paths and URLs as content apply to every pooled connection as they did to each single one. Console and Resend are unchanged.
 
 ### Fixed
+- A participant name of only spaces is now refused, on signing up and on the edit page, with `invalid_input` on `name` (#515). Signing up used to store it blank, so the Responses tab, the CSV export and the confirmation email ("Thanks , you signed up…") showed no name, and the edit page said "Saved." while keeping the old name; only a move refused it. The sign-up dialog and the edit page now say "Enter your name, up to 100 characters." instead of the validator's own text. The organizer's display name in signup settings follows the same rule. Names already stored blank stay as they are.
 - After **Sign in →** on the page a sign-in email opens, the button now says "Signing in…" and ignores further clicks (#240). It gave no sign that anything was happening, and the first request can take a few seconds, so people clicked again; the second click reused the single-use token and ended on "That sign-in link is no longer valid" (already signed in, they were then sent to their dashboard rather than where they were going). It is still a plain link, and a click that opens a new tab, or coming back with the Back button, works as before. The server also logs a used, expired or unknown sign-in link as one warning instead of Auth.js's error with a full stack trace, and sends every other Auth.js error through the app's logger, so its redaction applies.
 - On the Build tab, a settings save that names a field the tab hasn't loaded now fetches the field list first (#398). When another tab or an AI assistant added a date field and moved reminders to it, the next "Group slots by" change made that field the reminder field without the tab knowing it: no field showed the reminder bell, and every date field's reminder box was blocked by a note with a blank where the field's name should be, until a reload. Now the new field appears and shows the bell.
 - In the builder, a day's heading on a signup grouped by a Date field is now set with a date picker, the same one a slot's Date uses, and a heading on a signup grouped by a List field is set from the list's choices, with **Add to list** as in a slot (#473). Both used to be free text, and the server only takes a date as YYYY-MM-DD or one of the list's choices, so typing a date the way the heading shows it ("Sat, Nov 14") looked saved but wasn't, and the slots lost their date on reload. A heading's change is saved once the organizer has finished choosing, not as they type, and a half-typed date leaves the heading as it was. If the change still doesn't save, the heading goes back to its saved value with "That change didn't save, so this heading is back to what it was. Try again." under it. Short text headings are typed as before.

src/app/s/[slug]/name-message.ts

@@ -0,0 +1,16 @@
+/**
+ * Participant-facing copy for an `invalid_input` on `name`, from signing up or
+ * from the edit page: the name was blank once trimmed, or longer than
+ * `NameSchema` allows. The server's text ("name is required", "Check the value
+ * of \"name\".") is written for API callers, not participants.
+ */
+export const NAME_INVALID = {
+  message: 'Enter your name, up to 100 characters.',
+} as const;
+
+/** `NAME_INVALID` for an `invalid_input` on `name`, null for anything else. */
+export function nameMessage(
+  error: { code?: string; field?: string } | null | undefined,
+): typeof NAME_INVALID | null {
+  return error?.code === 'invalid_input' && error.field === 'name' ? NAME_INVALID : null;
+}

src/app/s/[slug]/participant-error.test.ts

@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest';
 import { ALREADY_SIGNED_UP } from './conflict-message';
 import { CLOSED } from './closed-message';
 import { GONE } from './gone-message';
+import { NAME_INVALID } from './name-message';
 import { errorCopyText, participantErrorCopy } from './participant-error';
 
 describe('participantErrorCopy', () => {
@@ -22,6 +23,13 @@ describe('participantErrorCopy', () => {
     expect(participantErrorCopy({ code: 'not_found' }, 'change')).toBe(GONE);
   });
 
+  it('words a refused name on both pages', () => {
+    const error = { code: 'invalid_input', field: 'name', message: 'name is required' };
+    expect(participantErrorCopy(error, 'join')).toBe(NAME_INVALID);
+    expect(participantErrorCopy(error, 'change')).toBe(NAME_INVALID);
+    expect(participantErrorCopy({ code: 'invalid_input', field: 'notes' }, 'join')).toBeNull();
+  });
+
   it('rewords a repeat sign-up and the rate limit only when signing up', () => {
     expect(participantErrorCopy({ code: 'conflict' }, 'join')).toBe(ALREADY_SIGNED_UP);
     expect(participantErrorCopy({ code: 'rate_limited' }, 'join')?.message).toBe(

src/app/s/[slug]/participant-error.ts

@@ -2,6 +2,7 @@ import { capacityMessage, type CapacityContext } from './capacity-message';
 import { closedMessage } from './closed-message';
 import { conflictMessage } from './conflict-message';
 import { goneMessage } from './gone-message';
+import { nameMessage } from './name-message';
 import { rateLimitMessage } from './rate-limit-message';
 
 export interface ParticipantErrorCopy {
@@ -11,6 +12,7 @@ export interface ParticipantErrorCopy {
 
 interface ApiErrorLike {
   code?: string;
+  field?: string;
   details?: {
     remaining?: unknown;
     requested?: unknown;
@@ -23,7 +25,8 @@ interface ApiErrorLike {
  * Participant-facing copy for an error from signing up (`'join'`) or from
  * changing or cancelling a sign-up (`'change'`), or null when there is none and
  * the caller shows the server's own text. A capacity error is built from its
- * numbers, and a slot or signup closed or removed meanwhile gets its own copy.
+ * numbers, and a slot or signup closed or removed meanwhile, and a blank or
+ * overlong name, each get their own copy.
  * Signing up also rephrases a second sign-up by the same email and the rate
  * limit, which the edit page has never reworded.
  */
@@ -32,7 +35,10 @@ export function participantErrorCopy(
   context: CapacityContext,
 ): ParticipantErrorCopy | null {
   const shared =
-    capacityMessage(error, context) ?? closedMessage(error, context) ?? goneMessage(error);
+    capacityMessage(error, context) ??
+    closedMessage(error, context) ??
+    goneMessage(error) ??
+    nameMessage(error);
   if (shared || context === 'change') return shared;
   return conflictMessage(error) ?? rateLimitMessage(error);
 }

src/schemas/commitments.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from 'vitest';
-import { CommitmentCreateInputSchema } from './commitments';
+import { CommitmentCreateInputSchema, CommitmentUpdateInputSchema } from './commitments';
 
 describe('CommitmentCreateInputSchema email', () => {
   const base = { name: 'Pat Example' };
@@ -63,3 +63,15 @@ describe('CommitmentCreateInputSchema idempotencyKey', () => {
     expect(r.error.issues[0]?.path).toEqual(['idempotencyKey']);
   });
 });
+
+describe('Commitment names', () => {
+  it('refuses a name of only spaces on sign-up and on edit, on the name field', () => {
+    const created = CommitmentCreateInputSchema.safeParse({ name: '   ' });
+    const updated = CommitmentUpdateInputSchema.safeParse({ name: '   ' });
+    for (const r of [created, updated]) {
+      expect(r.success).toBe(false);
+      if (r.success) continue;
+      expect(r.error.issues[0]?.path).toEqual(['name']);
+    }
+  });
+});

src/schemas/common.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from 'vitest';
-import { EmailSchema, normalizeEmail } from './common';
+import { EmailSchema, NameSchema, normalizeEmail } from './common';
 
 describe('EmailSchema', () => {
   it('trims whitespace and lowercases', () => {
@@ -22,3 +22,22 @@ describe('normalizeEmail', () => {
     expect(normalizeEmail(input)).toBe(EmailSchema.parse(input));
   });
 });
+
+describe('NameSchema', () => {
+  it('trims a name', () => {
+    expect(NameSchema.parse('  Pat Example ')).toBe('Pat Example');
+  });
+
+  it.each([
+    ['empty', ''],
+    ['only spaces', '   '],
+    ['only whitespace', ' \t\n '],
+  ])('refuses an %s name', (_label, name) => {
+    expect(NameSchema.safeParse(name).success).toBe(false);
+  });
+
+  it('refuses a name over 100 characters', () => {
+    expect(NameSchema.safeParse('a'.repeat(101)).success).toBe(false);
+    expect(NameSchema.safeParse('a'.repeat(100)).success).toBe(true);
+  });
+});

src/schemas/common.ts

@@ -18,7 +18,13 @@ export const EmailSchema = z
   .transform(normalizeEmail)
   .pipe(z.string().email());
 
-export const NameSchema = z.string().min(1).max(100).transform((v) => v.trim());
+// Trimmed before the min(1) check, like EmailSchema, so a name of only spaces
+// is refused rather than stored blank. max(100) is on the raw string.
+export const NameSchema = z
+  .string()
+  .max(100)
+  .transform((v) => v.trim())
+  .pipe(z.string().min(1, 'name is required'));
 
 /** The top of the int `sort_order` column on slots and slot fields. */
 export const MAX_SORT_ORDER = 2_147_483_647;

src/services/commitments.db.test.ts

@@ -498,20 +498,52 @@ describe('updateOwnCommitment swap (db)', () => {
     expect(onSecond).toEqual([]);
   });
 
-  // Signing up trims the name and keeps it, so a name of spaces is stored
-  // blank. A move that sends no name must not check that stored one again.
+  it('refuses a name of only spaces on sign-up and on edit', async () => {
+    const a = await makeOpenSignupWithSlot(fx, 'Signup Blank');
+    const blank = await commitToSlot(fx.db, a.slotId, {
+      name: '   ',
+      email: 'mo@example.test',
+      quantity: 1,
+    });
+    expect(blank.ok).toBe(false);
+    if (blank.ok) return;
+    expect(blank.error).toMatchObject({ code: 'invalid_input', field: 'name' });
+
+    const mine = await commitToSlot(fx.db, a.slotId, {
+      name: 'Mo',
+      email: 'mo@example.test',
+      quantity: 1,
+    });
+    if (!mine.ok) throw new Error(`commitToSlot failed: ${mine.error.message}`);
+    const edit = await updateOwnCommitment(fx.db, mine.value.commitment.id, mine.value.editToken, {
+      name: '   ',
+    });
+    expect(edit.ok).toBe(false);
+    if (edit.ok) return;
+    expect(edit.error).toMatchObject({ code: 'invalid_input', field: 'name' });
+
+    const [p] = await fx.db
+      .select({ name: participants.name })
+      .from(participants)
+      .where(eq(participants.id, mine.value.commitment.participantId));
+    expect(p?.name).toBe('Mo');
+  });
+
+  // Signing up used to store a name of spaces blank, so such rows exist. A
+  // move that sends no name must not check that stored one again.
   it('moves someone whose stored name is blank when the move sends no name', async () => {
     const a = await makeOpenSignupWithSlot(fx, 'Signup L');
     const second = await addSlot(fx.db, fx.actor, a.signupId, { values: {}, capacity: 5 });
     if (!second.ok) throw new Error(`addSlot failed: ${second.error.message}`);
 
     const mine = await commitToSlot(fx.db, a.slotId, {
-      name: '   ',
+      name: 'Lou',
       email: 'lou@example.test',
       quantity: 1,
     });
     if (!mine.ok) throw new Error(`commitToSlot failed: ${mine.error.message}`);
     const { participantId } = mine.value.commitment;
+    await fx.db.update(participants).set({ name: '' }).where(eq(participants.id, participantId));
 
     const r = await updateOwnCommitment(fx.db, mine.value.commitment.id, mine.value.editToken, {
       swapToSlotId: second.value.id,

src/services/commitments.ts

@@ -588,10 +588,6 @@ export async function getOwnCommitmentsForSignup(
   return out;
 }
 
-// A name a move sends, checked by the rule signing up uses on the trimmed
-// name, which the edit schema alone does not apply: blank is refused.
-const MoveNameSchema = CommitmentCreateInputSchema.pick({ name: true });
-
 /**
  * The commitment as an edit left it. A move to another slot gives the new
  * commitment, whose id and edit token are new, with `moved` holding what its
@@ -617,14 +613,6 @@ export async function updateOwnCommitment(
 
   if (data.swapToSlotId && data.swapToSlotId !== current.slotId) {
     const swapToSlotId = data.swapToSlotId;
-    // Checked before anything is written, and only a name the move sends: one
-    // it leaves out stays as it is, whatever it is. Quantity and notes need no
-    // more checking. Each either passed the edit schema, which bounds it as
-    // signing up does, or comes from the commitment itself (below).
-    if (data.name !== undefined) {
-      const name = parseInputSafe(MoveNameSchema, { name: data.name });
-      if (!name.ok) return name;
-    }
     // A refusal as closed or full rolls the whole move back, so its
     // attempt_failed row is written after, by the participant who tried.
     let refusal: Record<string, unknown> | undefined;