Skip to content

Merge copies of a slot in one tap on the review page, with Undo - #534

Closed
richshaw wants to merge 4 commits into
claude/issue-503-one-tap-fixesfrom
claude/issue-503-merge-slots
Closed

richshaw wants to merge 4 commits into
claude/issue-503-one-tap-fixesfrom
claude/issue-503-merge-slots

Conversation

@richshaw

@richshaw richshaw commented Oct 21, 2026 •

Copy link
Copy Markdown
Owner

Fixes #503 (part 3 of 3; parts 1 and 2 are #530 and #533). Part of #498. Stacked on #533; the diff here is only the merge.

What changes

When a draft has slots that are copies of each other, the review page's notice ("3 slots are the same: Setup · Thu, May 2. One slot for 3 people does the same job.") now has Merge into one slot:

  • One slot keeps the copies' total spots (no limit if any of them had none). The others are deleted.
  • For numbered rows ("Helper 1", "Helper 2", "Helper 3", each for one person), the kept slot takes the shared value ("Helper"). The notice already says "If they're the same job, one slot for 3 people does it."
  • The line above the list says "Merged 3 slots into one for 3 people." (or "… with no limit.") with Undo. Undo puts every copy back where it was, with its values and spots.

This was the issue's check 4: "one slot whose capacity is the sum of theirs, with the others deleted … in one transaction with one activity row, through a small mergeSlots service."

How it works

  • New services in src/services/slot-merge.ts:
    • mergeSlots: one transaction and one slot.merged row. It locks the signup, then its slots, in the documented order. Under the lock it refuses:

      • anything but a draft;
      • a slot from another signup;
      • a closed slot;
      • any sign-up on them;
      • slots that are no longer copies of each other. findDuplicateSlots runs again on the locked rows, so a copy renamed since the notice was shown isn't deleted.

      Capacity is worked out from the locked rows, never taken from the caller.

    • unmergeSlots: the exact inverse, with one slot.unmerged row. The undo carries the kept slot's updatedAt from the merge. It is refused when that has changed, which covers:

      • a later rename or date change;
      • a capacity change;
      • a reorder that moved it;
      • an Undo that already ran.
  • One insert path. insertSlot in slots.ts is the insert path for addSlot, addSlotsBulk and the undo, so restored rows get their ref and slot_at the same way.
  • Wiring. The page's fix machinery from Fix four kinds of draft problem in one tap on the review page, with Undo #533 gains the duplicate_slots kind. The request must match a fresh notice by its exact set of ids. The undo is exactly what mergeSlots returned, checked against the strict unmerge schema on the way back.
  • Refusals in plain words:
    • a copy deleted, closed, booked or changed since: "That changed since you opened this page, so nothing was done."
    • the kept slot changed: "That changed again since, so it can't be undone."
    • not a draft any more: "This signup isn't a draft any more. Change it in the builder."
    • anything no change could cause: "Couldn't fix that. Try again."
  • Telemetry: slot.merged ({ slotId, mergedSlotIds, capacity }) and slot.unmerged ({ slotId, restoredSlotIds, capacity }), plus review.notice_accepted / review.fix_undone with kind duplicate_slots. docs/telemetry.md lists them.

Help

"Create and publish your first signup": "For slots that are copies of each other, choose Merge into one slot: one slot takes all their spots." A new walkthrough test merges copies on the review page and undoes it.

Checks

  • pnpm lint && pnpm typecheck && pnpm knip && pnpm test: pass (1486)
  • pnpm test:db: pass (592)
  • pnpm build && pnpm budgets: pass (/app/signups/[id] 112.7 / 113 kB, unchanged)
  • Full e2e (chromium): 62 passed

Tophat and the code-review fixes are in the comments.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BTvMvHZ4i4FWQFAsxcr5Zm

…ndoes it

The review page's fix for identical rows (issue #503) needs one call that
turns copies of a slot into one slot holding all their places, and one that
puts them back for Undo.

mergeSlots keeps one slot, gives it the copies' total capacity (worked out
from the rows as locked, no limit when any had none) and, when sent, new
values and the slot_at they resolve to, then deletes the rest. unmergeSlots
gives the kept slot back its values and capacity and adds the others again
at their old place in the order. Each runs in one transaction under the
signup lock and then the slot locks, writes one activity row (slot.merged,
slot.unmerged), and works on drafts only: on a published signup a merge
would move people who have signed up. Both refuse any slot with a
commitment rather than let a delete take it away, and an undo is refused
once the kept slot has been deleted or its capacity changed.

The slot insert addSlot and addSlotsBulk each wrote out is now one helper,
insertSlot, which the undo uses too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTvMvHZ4i4FWQFAsxcr5Zm
The review page's notice for slots that are copies of each other now has
"Merge into one slot" in place of "Fix in the builder" (#503). It runs as
the other one-tap fixes do: only on a draft, and only while the draft still
has exactly those copies. The checks run again on fresh data and the
request has to name the same set of slots. It then calls mergeSlots with
the notice's kept slot and copies. Numbered rows ("Helper 1", "Helper 2")
send the values without the number; identical rows keep the kept slot's
own values. The page says "Merged 3 slots into one for 3 people." ("with
no limit" when any copy had none).

Undo sends back exactly what mergeSlots returned, checked by its own
schema, to unmergeSlots. Before that, the page checks that the kept slot
is still there with the merged total. The service refuses the same thing
under its locks. When the kept slot had no limit, its capacity can't tell
a second undo from the first, so the page also refuses while its copies
are back, rather than putting them back twice. The merge services'
refusals read as the other fixes' do: a changed draft is "That changed
since you opened this page, so nothing was done.", and a kept slot gone
or changed is "That changed again since, so it can't be undone."

The one-tap merge is offered for up to MAX_MERGE_SLOTS copies, now
exported. Unit tests cover matching, the merge request, the undo and its
checks, and the wording. The actions' DB tests run the merge through
fix, undo, a stale request, a viewer and a published signup. They also
cover an undo after the kept slot's places changed, and a second undo of
copies with no limit. The telemetry doc names slot.merged and
slot.unmerged among what the fixes write. The help page "Create and
publish your first signup" names the button, and its walkthrough merges
two copies and undoes it. The changelog describes the service, the
button and Undo.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTvMvHZ4i4FWQFAsxcr5Zm
A kept slot with no limit reads the same before a merge and after its
undo, so the capacity check couldn't tell a repeated undo from the first,
and the copies came back twice. unmergeSlots now reads the latest
slot.merged / slot.unmerged row for the kept slot under the signup lock,
as reminder.sent stops a second reminder, and refuses once it's undone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTvMvHZ4i4FWQFAsxcr5Zm
A merge now runs the review page's copy check again under its locks, on
the slots and fields as locked. The slots sent must be exactly one set of
copies, and for numbered rows the values sent must be the ones the copies
now share. A copy changed or added since, a set that is only part of the
copies, or values the page worked out from an older version are refused,
and nothing is deleted.

Undo is guarded by one version check instead of three. mergeSlots puts
the kept slot's updatedAt, as the merge wrote it, into the undo as
keptUpdatedAt. unmergeSlots refuses once the kept slot's updatedAt has
moved. Editing the slot, reordering it and the undo itself all move it.
So Undo no longer puts old values back over a rename or a new date made
since, and it can't put the copies back twice, whether the kept slot had
a limit or not. The undo writes an updatedAt past the merge's, so two
writes in one millisecond can't fool it. This replaces the capacity-sum
check, the activity-log lookup and the review page's copies heuristic,
which wrongly refused a first undo once a new copy of the kept slot had
been added. The review page now checks only that the kept slot exists.

A merge's refusals read as a change since ("stale" / "changed") only when
a change since causes them. Anything else reads as "Couldn't fix that.
Try again." or "Couldn't undo that. Change it in the builder.", rather
than offering the same button again. The service's field never passes
on: a slot field called "label" read as the rename box's refusal. The
rename refusal now applies to a fix only. The mapping is a pure
mergeRefusal in src/lib/draft-fixes.ts, unit-tested.

mergeRequestFor sends values only for numbered rows. The undo schema is
strict and has no per-row status (a merge only takes open slots), and
insertSlot no longer takes one.

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: pass, in the running app with made-up data (pat@example.com).

  • Exact copies: three "Setup · Thu, May 2" rows for one person each. Merge into one slot shows "Merged 3 slots into one for 3 people." and focus moves to Undo. The list shows one "Setup · 3 people", and the "dates repeat" notice goes with the copies. Undo puts the three rows back in place.
  • Numbered rows: "Helper 1/2/3" become "Helper · 3 people". Undo brings back Helper 1, 2, 3 in order.
  • Undo after a later edit (on 4baff66): merge, then another tab renames the merged slot to "Setup crew" with its spots unchanged. Undo now says "That changed again since, so it can't be undone." and the rename stays. Before this push, Undo would have quietly put "Setup" back.
  • Activity: slot.merged, then review.notice_accepted {kind: duplicate_slots}, then slot.unmerged, then review.fix_undone. No console errors.

Code review on this PR. Fixed in aeaa678 and 4baff66:

  • One version check for Undo. The undo carries the kept slot's post-merge updatedAt, and unmergeSlots refuses once it differs. This covers:

    • a later rename or date change;
    • a capacity change;
    • a reorder that moved the slot;
    • a repeated or concurrent Undo, including slots with no limit.

    It replaces the capacity-sum check, the activity-log lookup (which also lacked a workspace condition), and the page-side copies heuristic (which wrongly refused a first Undo once a new copy had been added).

  • Merge re-checks under its lock that the slots are still copies: findDuplicateSlots runs on the locked rows. A copy renamed since the notice was shown is refused, and nothing is deleted.

  • Honest refusals. Only refusals a concurrent change can cause read as "changed since". A values or schema refusal says "Couldn't fix that. Try again." A merge error's field no longer reaches the rename branch.

  • Simplifications: mergeRequestFor sends values only for numbered rows. The unmerge schema is .strict() with no per-row status, and insertSlot lost its unused status parameter.

Left as is: restore order for copies that share a sortOrder. Copies made by createSignup or addSlotsBulk have distinct ones, and a reorder that touched the kept slot now refuses Undo.

Checks on 4baff66: lint, typecheck, knip, unit (1486), test:db (592), build + budgets (/app/signups/[id] 112.7 / 113 kB), e2e 62/62.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Closing without merging, as part of the redesign in #498. The branch stays: mergeSlots and unmergeSlots in src/services/slot-merge.ts and their tests are meant to be reused.


Generated by Claude Code

@richshaw richshaw closed this Oct 22, 2026
@richshaw
richshaw removed this pull request from stack #549 October 23, 2026 22:38
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.

Find fixable problems in a draft and offer one-tap fixes

2 participants