Skip to content

Centralize participant input limits in one module - #706

Merged
richshaw merged 10 commits into
richshaw:mainfrom
bfs2021:fix/705-participant-limits-single-source
Oct 31, 2026
Merged

richshaw merged 10 commits into
richshaw:mainfrom
bfs2021:fix/705-participant-limits-single-source

Conversation

@bfs2021

@bfs2021 bfs2021 commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Problem

Fixes #705. The participant-facing limits were literals copied across schemas,
copy, and form attributes: name 100 in NameSchema and NAME_INVALID, notes
500 in both commitment schemas and the Notes field, quantity 999 twice, and
title/description 2/120/2000 in the New signup form while
src/schemas/signups.ts already held the constants.

Root cause

Each page restated a number the schema owned, so changing a limit leaves the
message or the maxLength saying the old value.

Fix

  • New import-free src/lib/participant-limits.ts with NAME_MAX_LENGTH,
    NOTES_MAX_LENGTH and QUANTITY_MAX, so the client pages under /s/[slug]
    can read them without pulling zod into their bundle.
  • NameSchema, both commitment schemas, NAME_INVALID, the Notes field and
    the name boundary test use those constants.
  • Export TITLE_MIN_LENGTH and use it, TITLE_MAX_LENGTH and
    DESCRIPTION_MAX_LENGTH on the New signup form.

No behaviour change: no maxLength was added to the name inputs.

Testing

npx tsc --noEmit            # clean
npx eslint <changed files>  # clean
npx vitest run              # 1778 passed; the one failure is worker-render.test.ts
                            # spawning `pnpm` (ENOENT in this shell), not this change

The name boundary test now derives its 100/101 inputs from
NAME_MAX_LENGTH and passes.

Name, notes and quantity limits were literals in two places each: the zod
schemas, the participant copy, the textarea's maxLength and the schema boundary
test. The New signup form also restated the title and description constants
that src/schemas/signups.ts already held, because TITLE_MIN_LENGTH was not
exported.

Add src/lib/participant-limits.ts (no imports, so the client pages under
/s/[slug] stay zod-free) and point every copy at it; export TITLE_MIN_LENGTH
and use the existing signups constants on the form. No behaviour change.
@richshaw
richshaw self-requested a review October 29, 2026 22:14

@richshaw richshaw left a comment •

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for this. It's a safe change: every limit keeps its old value, it merges cleanly with current main, and a security review found nothing. A few things are left before it fully closes #705. The inline comments say which points are older bugs this PR doesn't cause (not blocking).

Still typed in more than one place. These files aren't in the diff, so they can't take inline comments:

  • src/components/build-shared/useBuildState.ts: the Build tab still has its own trimmed.length < 2, trimmed.slice(0, 120), trimmed.slice(0, 2000) and the "Title must be at least 2 characters." message in TITLE_TOO_SHORT. If TITLE_MAX_LENGTH were raised, the Build tab would quietly cut titles back to 120 on save. The module already pulls in zod through @/schemas/slot-fields, so importing TITLE_MIN_LENGTH, TITLE_MAX_LENGTH and DESCRIPTION_MAX_LENGTH from @/schemas/signups adds nothing to the bundle.
  • src/lib/magic-compose/prompt.ts: the system prompt still says 2-120 chars and max 2000 chars. SYSTEM_PROMPT is a template literal, so interpolating the constants keeps the prompt byte-for-byte the same and __golden__/system-prompt.txt won't change.

*/
export const NAME_MAX_LENGTH = 100;
export const NOTES_MAX_LENGTH = 500;
export const QUANTITY_MAX = 999;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The doc comment says this file exists so the client pages can read these limits, but no client reads QUANTITY_MAX. The Spots input (max={spots.max} in participant-fields.tsx) gets no upper bound on an unlimited slot (commit-dialog.tsx passes undefined when left === null, and the edit form passes spotsMax ?? undefined), or when more than 999 spots are left. A participant can type 1000 and the server answers with its developer message about quantity. Using QUANTITY_MAX as the fallback, or Math.min(..., QUANTITY_MAX), would close that. The bug is older than this PR, but it's the same drift #705 is about.

* Participant-facing input limits, kept import-free so the client pages
* under /s/[slug] can read them without pulling zod into their bundle.
*/
export const NAME_MAX_LENGTH = 100;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

NameSchema also validates the organizer's organizerDisplayName (src/schemas/signups.ts:97), so this isn't only a participant limit. Lowering it here would also tighten organizer display names through the API and Magic Compose. Worth saying so in the doc comment, or naming the file for what it holds (say input-limits.ts).

name="description"
rows={4}
maxLength={2000}
maxLength={DESCRIPTION_MAX_LENGTH}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Not blocking, older than this PR: this only matches the schema for single-line text. Browsers count a line break as 1 character for maxLength, but the form sends it as CRLF (2 characters). So a description close to 2000 characters with a few paragraph breaks passes in the browser, then DESCRIPTION_MAX_LENGTH refuses it on the server, and the error redirect clears the form. Better as its own issue than in this PR.

required
minLength={2}
maxLength={120}
minLength={TITLE_MIN_LENGTH}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Not blocking, older than this PR: minLength checks the raw text, but TitleSchema trims before its minimum check. A title like a passes in the browser, then the server refuses it, and the redirect clears the form.

*/
export const NAME_INVALID = {
message: 'Enter your name, up to 100 characters.',
message: `Enter your name, up to ${NAME_MAX_LENGTH} characters.`,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Not blocking, older than this PR: NameSchema applies max(NAME_MAX_LENGTH) to the raw string before trimming, and the edit page sends the name untrimmed (the sign-up dialog trims first). So on the edit page, trailing spaces count toward the limit this message quotes: a 99-character name with 2 trailing spaces is refused with "up to 100 characters".

Comment thread src/schemas/common.test.ts Outdated
expect(NameSchema.safeParse('a'.repeat(101)).success).toBe(false);
expect(NameSchema.safeParse('a'.repeat(100)).success).toBe(true);
it('refuses a name over the limit', () => {
expect(NameSchema.safeParse('a'.repeat(NAME_MAX_LENGTH + 1)).success).toBe(false);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Optional: now that the limit comes from the constant, this test can't catch an accidental change to the public limit (a typo like NAME_MAX_LENGTH = 10 would still pass). Keeping the literal 100/101 here, or adding expect(NAME_MAX_LENGTH).toBe(100), keeps that guard.

Comment thread src/app/s/[slug]/participant-fields.tsx Outdated
return (
<div className={spots ? 'grid grid-cols-[1fr_auto] gap-3' : undefined}>
<Field label="Notes (optional)" type="text" name="notes" maxLength={500} {...notes} />
<Field label="Notes (optional)" type="text" name="notes" maxLength={NOTES_MAX_LENGTH} {...notes} />

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nit: this line is 105 characters, over the repo's Prettier printWidth of 100, so pnpm format:check flags the file. pnpm format will split it.

@richshaw richshaw left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Requesting changes for the points in my review above: the Build tab and Magic Compose still restate the title and description limits, and the Spots input doesn't use QUANTITY_MAX. The comments marked not blocking are older bugs and can wait for their own issues if you can't get to them.

…ywhere

The Build tab and the Magic Compose system prompt still restated the title and
description limits, so raising a limit would silently diverge; the Spots input
ignored QUANTITY_MAX. Read the constants in all three places.
@bfs2021

bfs2021 commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — addressed the blocking points: the Build tab now reads TITLE_MIN_LENGTH, TITLE_MAX_LENGTH and DESCRIPTION_MAX_LENGTH (message included), the Magic Compose system prompt interpolates the same constants (golden text is byte-for-byte unchanged), and the Spots input caps max at QUANTITY_MAX. Also fixed the Prettier line in participant-fields.tsx and made the name-limit test assert the literal 100. The older inline notes are left for their own issues as you suggested.

bfs2021 and others added 2 commits October 30, 2026 10:38
No content change; keeps the PR diff free of Windows line-ending noise.

@richshaw richshaw left a comment •

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Merging, thanks for this! Every participant limit now lives in one place. Thanks too for the quick turnaround on the review.

@richshaw
richshaw enabled auto-merge (squash) October 30, 2026 03:03
auto-merge was automatically disabled October 30, 2026 03:09

Head branch was pushed to by a user without write access

@bfs2021

bfs2021 commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 2851ee6 for the lint-type-test failure on efe7271: the two tests that still expected no max on an unlimited slot now expect QUANTITY_MAX (999), which is the cap the Spots input now applies there (commit-dialog.test.tsx and edit-form.test.tsx). Both files pass locally, with tsc --noEmit, eslint and prettier --check clean.

@richshaw
richshaw enabled auto-merge (squash) October 30, 2026 03:10
useBuildState is a client hook; importing TITLE_MIN_LENGTH and friends
from @/schemas/signups pulled that module (and zod) into the build page,
taking it to 161.0 kB against its 161 kB budget. The constants now live in
src/lib/signup-limits.ts, which schemas/signups.ts re-exports, the same
shape participant-limits.ts already uses for participant-facing limits.
auto-merge was automatically disabled October 30, 2026 03:46

Head branch was pushed to by a user without write access

@bfs2021

bfs2021 commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Good catch from CI: importing the constants from @/schemas/signups into the client hook pulled that module into the build page and took it to 161.0 kB against its 161 kB budget. The constants now live in an import-free src/lib/signup-limits.ts (same shape as participant-limits.ts), schemas/signups.ts re-exports them, and the build page is back to 160.3 kB — pnpm build + pnpm budgets pass locally.

@richshaw

Copy link
Copy Markdown
Owner

Thanks, the import-free signup-limits.ts is great.

One more thing before this can merge: 15748f3 saved useBuildState.ts, signups.ts and signup-limits.ts with CRLF line endings, so a 12-line change shows as about 2,400 changed lines. Could you convert those three files back to LF, as you did in cacab71?

Restore LF endings lost when the files were edited on Windows so the diff stays reviewable.
@bfs2021

bfs2021 commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

Converted useBuildState.ts, signups.ts and signup-limits.ts back to LF in bfc580d; the commit is line endings only, so the 12-line change is reviewable again.

Resolve the new-signup page conflict: keep the shared TITLE_MIN_LENGTH while adopting main's carried title/description defaults.
@richshaw
richshaw merged commit 8c1db15 into richshaw:main Oct 31, 2026
2 checks passed
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.

Participant name and notes limits are typed twice, in the schemas and on the participant pages

2 participants