Skip to content

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

Merged
richshaw merged 1 commit into
mainfrom
claude/issue-515-blank-name
Oct 21, 2026
Merged

richshaw merged 1 commit into
mainfrom
claude/issue-515-blank-name

Conversation

@richshaw

Copy link
Copy Markdown
Owner

Fixes #515.

The problem

NameSchema checked min(1) before trimming, so a name of only spaces passed and came out as "". Signing up stored the participant with a blank name (blank on the Responses tab and in the CSV, "Thanks , you signed up…" in the email), and the edit page skipped the blank name and said "Saved." Only a move refused it. When the server did refuse a name, the sign-up dialog showed the validator's text ("String must contain at least 1 character(s)", 'Check the value of "name".').

The fix

  • src/schemas/common.ts: NameSchema now trims first and checks after, the way EmailSchema does (max(100) on the raw string, then .pipe(z.string().min(1))). Sign-up and edit both refuse a blank name with invalid_input on name.
  • src/services/commitments.ts: MoveNameSchema and the move's extra check are removed. The edit schema now applies the same rule, so the check was redundant. The existing "refuses a blank name on a move" DB test still covers it.
  • src/app/s/[slug]/name-message.ts (new), used by participantErrorCopy: an invalid_input on name reads "Enter your name, up to 100 characters." in the sign-up dialog and on the edit page. One message covers both reasons a name can be refused (blank, or over 100), so it never says the wrong one. I chose to word the server's refusal rather than add a client-side check, because both forms already go through participantErrorCopy.
  • As the issue notes, organizerDisplayName uses NameSchema too, so a display name of only spaces is now refused as well.
  • Names already stored blank stay as they are. A move that sends no name still works for them.
  • CHANGELOG line under Fixed.

Tests

  • common.test.ts: NameSchema trims, refuses empty, spaces-only and whitespace-only names, and refuses names over 100 characters. commitments.test.ts: the create and update schemas both refuse " " on name. These failed before the schema change.
  • participant-error.test.ts: a refused name gets the new copy on both pages, and another invalid_input doesn't.
  • commitments.db.test.ts: the test that pinned the blank name ("a name of spaces is stored blank") is replaced. A new test checks that sign-up and edit refuse " " and keep the old name. The move test for a stored blank name now creates that row directly, as an old row would be.
  • Ran and passed: pnpm lint && pnpm typecheck && pnpm knip && pnpm test (1302), pnpm test:db (493), pnpm build && pnpm budgets.

Generated by Claude Code

NameSchema checked min(1) before trimming, so "   " passed and was
stored blank on sign-up, and the edit page skipped it and said "Saved."
It now trims first, the way EmailSchema does, so both refuse it with
invalid_input on name, as a move already did. The move's own re-check
is now redundant and goes.

The sign-up dialog and the edit page show "Enter your name, up to 100
characters." for that error instead of the validator's text.

Fixes #515.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W25bvopySXe58SR1eCv6Qh
@richshaw richshaw added pipeline:ux Changes what a person using OpenSignup sees or does pipeline:working A session is working on it right now labels Oct 21, 2026 — with Claude
@richshaw
richshaw requested a balanced review from Copilot October 21, 2026 16:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@richshaw richshaw added pipeline:tophat pipeline:working A session is working on it right now and removed pipeline:working A session is working on it right now labels Oct 21, 2026 — with Claude

Copy link
Copy Markdown
Owner Author

Tophat: passed

I ran 86ab04f locally with pnpm dev, Postgres 16, and Chromium at 1280×900 and 390×844. I didn't run the worker, because nothing here touches jobs. Before checking the head, I reproduced each symptom on the base (1d3a73c). The test signup was "Snack duty — Spring season", with a "Fruit and water" slot for 3 people. I typed the spaces-only names as real key presses, so the form sent " ".

Check Result What I saw
Sign-up dialog, spaces-only name (phone) ✅ Base: the dialog showed "String must contain at least 1 character(s) / Check the value of "name"." (545-base-blank-signup-phone). Head: it shows "Enter your name, up to 100 characters." Nothing was saved and no email was sent (545-head-blank-signup-phone).
Commit API, name: " " ✅ Base: the API returned 200 and stored the participant with the name "". Head: it returns 400 invalid_input on name ("name is required"), and no participant row is created.
Edit page, spaces-only name (phone) ✅ Base: the page said "Saved." and the name stayed "Sam Example" (545-base-edit-blank-phone). Head: it shows "Enter your name, up to 100 characters." under the form, and the stored name is unchanged (545-head-edit-blank-phone).
101-character name in the dialog (desktop) ✅ It shows the same "Enter your name, up to 100 characters." (545-head-long-name-desktop). The input has no maxLength, so this is the path someone can reach. The API's own text stays the validator's ("String must contain at most 100 character(s)"), which is for API callers.
Neighbours: valid sign-up and edit ✅ A sign-up as " Alex Example " was stored trimmed as "Alex Example". The confirmation email went out, and the page said "saved your spot" (545-head-signup-valid-desktop). Renaming Sam to "Sam Q Example" on the edit page said "Saved." The Responses tab lists both names (545-head-responses-desktop).
A row stored blank before this change ✅ I opened the edit page for the commitment the base had stored with a blank name. Saving with the name left empty is stopped by the browser's own "Please fill out this field." (the input's required, same as on main) (545-head-old-blank-row-desktop). After I typed "Robin Example", the name and the new notes saved.
Moves ✅ PATCH /api/commitments/<id> with swapToSlotId and name: " " returns 400 invalid_input on name, and the commitment doesn't move. The same move with no name succeeds, so the removed move-only check isn't missed.
organizerDisplayName ✅ POST /api/signups with organizerDisplayName: " " returns 400 invalid_input on that field. With " Pat Organizer ", the signup is created.
Console and server logs ✅ The only browser console errors were the 400s from the refusals I triggered on purpose. The only server errors were the health check's pgboss query, because the worker wasn't running.

UX: participants see one new message. When the sign-up dialog or the edit page refuses a name (only spaces, or longer than 100 characters), they see "Enter your name, up to 100 characters." This replaces the validator text in the dialog, and the false "Saved." on the edit page. API callers and MCP clients now get a 400 for a spaces-only name, where before it was stored blank. The base-and-head screenshot pairs show the change: 545-base-blank-signup-phone / 545-head-blank-signup-phone and 545-base-edit-blank-phone / 545-head-edit-blank-phone.

Screenshots are in https://claude.ai/code/session_01PHChS19UH8onuBt22xk69S.


Generated by Claude Code

@richshaw richshaw added pipeline:review pipeline:working A session is working on it right now and removed pipeline:tophat pipeline:working A session is working on it right now labels Oct 21, 2026 — with Claude

@richshaw richshaw left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: passed (round 1)

The PR fixes #515 the way the issue outlines. NameSchema (src/schemas/common.ts:23) now trims and then checks min(1) through a .pipe, as EmailSchema does, so " " is refused on sign-up and on edit. The issue path stays ['name'], so fromZodError sets field: 'name', api-response.ts passes it to the client, and both commit-dialog.tsx and edit-form.tsx already send the error through participantErrorCopy, which now maps it to NAME_INVALID. Removing MoveNameSchema is safe: CommitmentUpdateInputSchema uses the same NameSchema.optional(), so a move that sends a blank name is refused by the edit schema before anything is written, and a move that sends no name still skips the check (the DB test for a stored blank name now creates that row directly). The length limit is still on the raw string, as before, so no name that passed before is refused for length now. organizerDisplayName follows the new rule, and no column stores it (the MCP tool omits it for that reason), so no existing row can start failing a settings save. Every "Done when" item is covered.

Blocking (0)
Optional (1):

  • 🟡 The comment above errorText in src/app/s/[slug]/commit-dialog.tsx:339-342 lists which errors get their own copy and doesn't mention a refused name yet. If another push comes, add it there.

Earlier threads: none. Copilot couldn't review (quota).
Checks: CI is green on 86ab04f (lint-type-test, e2e, CodeQL). I read the diff, the callers of NameSchema and participantErrorCopy, and the error path through api-response.ts. The tophat passed on this exact SHA (ux=yes), so no runtime code has changed since.

Why the owner: this is UX. Participants now see "Enter your name, up to 100 characters." when the sign-up dialog or the edit page refuses a name, in place of the validator's text in the dialog and the false "Saved." on the edit page. API callers get a 400 for a spaces-only name where it used to be stored blank. Worth a look: the tophat's before-and-after pairs 545-base-blank-signup-phone / 545-head-blank-signup-phone and 545-base-edit-blank-phone / 545-head-edit-blank-phone.

https://claude.ai/code/session_01Ge3RPBKd3XN7tyWzLhpSP7


Generated by Claude Code

@richshaw richshaw added pipeline:owner Passed; waits for the owner (UX or high-risk) and removed pipeline:review pipeline:working A session is working on it right now labels Oct 21, 2026 — with Claude
@richshaw richshaw added pipeline:merge and removed pipeline:owner Passed; waits for the owner (UX or high-risk) labels Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Approved by the owner at 86ab04f.

https://claude.ai/code/session_01DXydxGNyVt8sXcDp4XzG84


Generated by Claude Code

@richshaw
richshaw merged commit eaeeda2 into main Oct 21, 2026
5 checks passed
@richshaw
richshaw deleted the claude/issue-515-blank-name branch October 21, 2026 22:24
@richshaw richshaw removed pipeline:merge pipeline:ux Changes what a person using OpenSignup sees or does labels Oct 21, 2026 — with Claude
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A participant name of only spaces is stored blank on sign-up, ignored on edit, and shows raw validator text in the sign-up dialog

3 participants