Skip to content

Point out fixable problems in a draft on its review page - #530

Closed
richshaw wants to merge 3 commits into
claude/issue-502-review-pagefrom
claude/issue-503-draft-notices
Closed

richshaw wants to merge 3 commits into
claude/issue-502-review-pagefrom
claude/issue-503-draft-notices

Conversation

@richshaw

Copy link
Copy Markdown
Owner

Part 1 of 3 for #503, in the #498 stack. Stacked on #528, which sits on #527 and #525.

  • This PR finds the problems and says what they are, with a link to the builder.
  • The next PR adds the one-tap fixes and Undo, plus review.notice_accepted.
  • The one after adds mergeSlots for rows that are copies.

The problem

Most drafts need nothing but a look and Publish. Some have problems the code can prove are there: placeholder rows that still say TBD, fields nobody fills in, blank slots, a row per person. Today nothing points them out before the signup goes live.

The fix

  • The checks (src/lib/draft-checks.ts) are pure functions over the signup's fields, slots and settings. Each returns { kind, slotIds | fieldId, … } with what a fix will need. The review page runs them over every slot, folded or not. In build order:
    1. placeholder: a value holding TBD, TBC or TBA as a whole word, in any case.
    2. empty_field: a field empty on every slot.
    3. blank_slot: a slot with no value in any field.
    4. duplicate_slots: rows identical in every field, with the kept slot's values and the total capacity (null if any copy had no limit). These are never offered when:
      • the rows hold a placeholder;
      • the rows are missing a value other slots have (unfinished slots, not copies);
      • numbered rows ("Helper 1", "Helper 2") don't all take one person, or aren't numbered 1 to n. "Room 101" and "Room 102" are different rooms.
    5. unnamed_field: a blank label, or the builder's default "Untitled". That default is now one constant, UNTITLED_FIELD_LABEL, shared with the builder.
    6. ungrouped_dates: a date field whose values repeat, with at least two dates, on a page that isn't grouped.
    7. time_in_text: no time field, and a text field holding a time of day ("9:00", "9am", "9:30 p.m."). Bare numbers and ranges like "Ages 8–12" don't count.
  • The words (src/lib/draft-notices.ts) name the organizer's own labels and values in quotes, so they read right in any language. Two examples:
    • "2 slots still say “TBD”, so people would see that on the page. Fill them in, or remove the ones you don’t need."
    • "3 slots are the same: Water station · Thu, Oct 3. One slot for 3 people does the same job."
  • On the page, the notices list under "N things to fix before you publish" on the slots card, each with "Fix in the builder" (the one-tap fixes come in the next PR). The card's mark turns from a check to "!". With no notices it says "Nothing to fix. This is the list people will see."
  • Telemetry: each kind shown writes review.notice_shown with { kind } only, after the page is sent. This is the event Record link copies, failed AI drafts and refused publishes #500 reserved, added to ACTIVITY_EVENTS now that something writes it, and documented in docs/telemetry.md.
  • Help: "Come back to a draft" now says the page points out anything to fix first, a slot that still says TBD being one example.

Tests

  • draft-checks.test.ts has 50 tests. Each check is tested on made-up data for what it must catch and what it must leave alone, including:
    • numbered placeholders ("TBD: game 1/2/3": a placeholder notice, no copies notice);
    • identical rows with a missing time while other slots have times (no copies notice);
    • numbered copies with capacity 2 (none) and with capacity 1 (copies, kept value without the number);
    • two copy groups;
    • a copy group with no limit;
    • "Game 1"/"Game 2" on different dates;
    • a single shared date;
    • a page already grouped;
    • "Ages 8–12";
    • a time field present.
  • draft-notices.test.ts covers each kind's words. view-tracker.db.test.ts checks recordNoticesShown writes one row per kind, holding only { kind }.
  • I ran pnpm lint && pnpm typecheck && pnpm test (1397 tests), pnpm knip (in a clean worktree), pnpm test:db (499 tests) and pnpm build && pnpm budgets. The page stays at 111.1 kB of 112, because the notices render on the server.

Generated by Claude Code

A pure module, src/lib/draft-checks.ts, that reads a draft's fields, slots
and settings and returns notices in build order: placeholder text (TBD,
TBC, TBA), fields no slot fills, blank slots, duplicate rows, unnamed
fields, dates that repeat on an ungrouped page, and times written into a
text field. Each notice carries what its one-tap fix needs, and each check
is its own exported function.

The checks stay silent when a case is ambiguous. Duplicate rows are never
rows holding a placeholder, blank rows, or rows missing a value other
slots have. Numbered copies ("Helper 1", "Helper 2") need capacity 1 and
numbers counting from 1, so "Room 101" and "Room 102" stay separate rooms.

The builder's default field name moves into UNTITLED_FIELD_LABEL so the
unnamed-field check and the field form read the same constant.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTvMvHZ4i4FWQFAsxcr5Zm
The review page now runs the draft checks over every slot and lists what
they find under "N things to fix before you publish", each in plain
words with a link to fix it in the builder: slots that still say TBD,
TBC or TBA; a field empty on every slot; blank slots; copies; a field
with no name; dates repeating on a page that isn't grouped by them; and
times typed into a text field. With none, the card says "Nothing to
fix. This is the list people will see." and keeps its check; with some,
its mark turns to "!".

The checks (src/lib/draft-checks.ts) are pure and exact: copies are
never offered for rows holding a placeholder or missing a value other
slots have, and numbered rows count only when each takes one person.
The builder's default field name is now one constant both use. Their
words are in src/lib/draft-notices.ts. Each kind shown writes
review.notice_shown { kind }.

Part of #503.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTvMvHZ4i4FWQFAsxcr5Zm
From review:

- Numbered rows ("Lane 1", "Lane 2") may be different things, so the
  notice says they differ only by a number and quotes two, leaving the
  call to the organizer, instead of calling them the same.
- Closed slots are never copies. Repeated dates are pointed out only
  when at least half the dates repeat, as one-slot groups help nobody.
  A bare clock time needs its minutes on a five, so "John 3:16" and a
  21:14 score aren't times. Whitespace-only values count as empty on
  the page's filled-slot check too.
- No empty quotes for a field with no name, and "One slot has" when it
  is one. The placeholder word comes from the check, and the capacity
  wording from signup-summary's people().
- "Nothing to fix" hides when Magic Compose's warnings show just above
  it, and the notices use the warnings' colors.
- review.notice_shown rows go in one insert (recordActivitiesBestEffort),
  docs/telemetry.md lists them with the other organizer telemetry and
  says to compare kinds by distinct signup, and the privacy page says
  the log records which kinds of problem were pointed out.
- The help screenshot is re-taken with "Nothing to fix".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTvMvHZ4i4FWQFAsxcr5Zm

Copy link
Copy Markdown
Owner Author

Tophat: passed

I ran it locally with pnpm dev, Postgres 16, and Chromium at 1280×900 and 390×844. I used made-up drafts created through the API. One has a problem of every kind: three identical "Water station" rows, two "TBD: marshal" rows, a blank slot, an empty "Volunteer name" field, "Morning 8am setup" in a text field, and repeated dates on an ungrouped page. A second has "Lane 1/2/3" and "John 3:16 reading". I ran the help walkthrough against pnpm build with HELP_SCREENSHOTS=1.

Check Result What I saw
A draft with every problem ✅ "6 things to fix before you publish", each in plain words with "Fix in the builder", and the card's mark turns to "!". The notices run over all 8 slots, though only 6 show before "Show all" (530-notices-desktop, 530-notices-phone)
review.notice_shown ✅ One row per kind per view, payload { kind } only, beside signup.editor_opened { section: 'review' }
Numbered rows ✅ "3 slots differ only by a number, like “Lane 1” and “Lane 2”. If they’re the same job, one slot for 3 people does it."
"John 3:16 reading" ✅ No time notice
A draft with nothing to fix ✅ "Nothing to fix. This is the list people will see." With Magic Compose's warnings showing (?aiDraft=1), that line is hidden (display: none)
Help walkthrough ✅ 3 passed. check-your-signup.png was re-taken and now shows "Nothing to fix"
Console ✅ No errors

For the owner (UX): a draft's review page now lists problems the code can prove, before anyone sees them, each with a link to the builder. The one-tap fixes come in the next PR. The privacy page's activity-log bullet now says the log records which kinds of problem were pointed out, never the values.

Code review

I ran /code-review (high) on b5ee55e. It found 15 things, and 18abbe7 fixes these:

  • The checks:
    • "Lane 1/2/3", "Day 1/2" and "Table 1/2" were called copies. Numbered rows now say they differ only by a number and quote two of them. They're no longer called "the same", and the call is left to the organizer, before any merge is offered.
    • "John 3:16" or a 21:14 score read as a time. A bare clock time now needs its minutes on a five.
    • A closed slot could be counted as a copy of an open one. It never is now.
    • Grouping by date was suggested when a single date repeated. Now at least half the dates must repeat.
    • Whitespace counted as empty in the checks but as a value on the page. Both now treat it as empty.
  • The words:
    • A field with a blank name showed as empty quotes. It now reads "a field with no name".
    • "Some slots" was used for one slot. It now says "One slot has".
    • "Nothing to fix" showed under Magic Compose's own warnings. It's now hidden with CSS :has(), so no JS is added.
  • Telemetry and trust docs:
    • The notice rows took one write per kind. They now go in a single insert (recordActivitiesBestEffort).
    • docs/telemetry.md lists the event with the other organizer telemetry, and says to compare kinds by distinct signup_id, since a refresh writes them again.
    • The privacy page names what's recorded, and its date is 21 October.
  • Reuse:
    • The placeholder regex was duplicated. The check now returns the word.
    • The capacity wording came from a second helper. It now comes from signup-summary's people().
    • The notices now use the warning list's colours.
  • Help: the screenshot is re-taken.

Kept as is: writing review.notice_shown on every render. That matches every other page view in the log, and the doc now says how to count it.

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


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Closing without merging. The draft → publish → share flow is being redesigned (#498) rather than shipped as this stack. The branch stays: the checks in src/lib/draft-checks.ts and their tests are meant to be reused by the redesign.


Generated by Claude Code

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.

2 participants