Show who else is editing a lineup group - #243
Conversation
Presence now relays which lineup groups each person is editing, so a
teammate about to change the same lineups sees it first: "Sam is editing
these lineups right now." in the lineup panel and edit dialog, and over
the media view. A group is one cloud row, so edits to it at the same
time conflict; this is the soft hint we chose over an edit lock.
- Room: a new `editing` message ({page, groups}), kept per peer, carried
in welcome and join, broadcast when it changes. Old clients and an old
room ignore it, so either can deploy first.
- Client: what this user edits is the groups of the lineup spots held,
the spot a placement is pinned to, and the lineups an open dialog
shows. It goes out throttled (the room ignores faster sends), at most
eight groups, and again after a rejoin. The lineup group memory now
says when it learns a group, so both sides recompute.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
From review: - The room always relays a clear, however soon it follows a change: two messages the network delivered together could otherwise leave the notice on everyone's screen. A clear only broadcasts after an accepted change, so it cannot flood the room. - Over the media view the notice shares one row with Delete, Edit and Close, so it gives way to them in a narrow window. - The notice uses DESIGN.md's 8px step and the 12px/600 label role. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ) { | ||
| return; | ||
| } | ||
| ws.serializeAttachment({ ...self, editing, lastEditingAt: now }); |
There was a problem hiding this comment.
If a pass renewal is awaiting verification when an editor clears a lineup group, the clear reaches existing peers, but renewal restores the earlier socket attachment. Newcomers then receive the cleared group in their welcome message and see an incorrect editing notice. Preserve intervening editing changes when applying the renewed pass; this needs fixing before merging.
Artifacts
Executable room renewal and editing race test
- The authored script executes both room source revisions with deferred pass verification and an interleaved editing clear, showing the exact tested sequence.
Parent room behavior before the change
- Running the parent source shows the clear was throttled, so the existing group remained in the newcomer welcome.
Current room behavior after the change
- Running the candidate shows the clear was broadcast but renewal restored the old group in the newcomer welcome, confirming the race.
There was a problem hiding this comment.
Fixed in #244. After the pass check, the renewal re-reads the socket's attachment and renews that state. It returns without renewing if the socket closed while it waited.
| for (final ids in open.values) ...ids, | ||
| }) | ||
| if (liveSync.lineupGroupOf(pageId, id) case final group?) group, |
There was a problem hiding this comment.
An edit dialog can remain open after its page changes, but its registered lineup ID is looked up on the newly active page. If that page has a lineup with the same ID, collaborators see an editing notice for the new page’s group while the dialog still contains the old page’s draft. This misleading notice is a non-blocking concern; tie the registration to the page where the dialog opened.
Artifacts
Flutter test for the lineup edit dialog across a page switch
- The authored test opens the edit dialog on p1, switches provider state and the lineup graph to p2, and checks registration, presence, draft fields, and saving, exercising the requested behavior.
Edit dialog state before switching pages
- The focused widget command captured the open p1 dialog, its registered lineup ID, `{group-p1}` presence, and unsaved draft before the switch.
Edit dialog state after switching pages
- The focused widget command captured the dialog remaining open in both p2 cases and, for a reused ID, `{group-p2}` presence and the p1 draft saved into p2’s lineup.
Edit dialog initialization and save paths
- A captured source search located the one-time draft initialization, dialog registration, and save lookup, supporting the observed switch and save behavior.
There was a problem hiding this comment.
Fixed in #244. Each open dialog records the page it opened on, and only dialogs from the page on screen count as editing.
| 'page': editing.pageId, | ||
| // The room refuses more than eight; in practice it is one or | ||
| // two. | ||
| 'groups': ([...editing.groupIds]..sort()).take(8).toList(), |
There was a problem hiding this comment.
When open dialogs or held items cover more than eight distinct lineup groups, this truncation silently omits the remaining groups from presence. Collaborators receive no editing notice for an omitted group, making the notice incomplete in this non-blocking edge case.
Artifacts
Flutter test source for open-dialog group presence
- The authored test creates distinct dialog groups, captures the real client outbound payload, and applies it to the remote presence reader.
Eight-group control test output
- The Flutter test ran with eight open-dialog groups and captured all eight outbound and remotely visible, with none missing.
- The Flutter test ran with nine open-dialog groups and captured an eight-group payload, missing `group-09` and leaving it without a remote editor.
There was a problem hiding this comment.
Intended. The room refuses more than 8 groups, so the client sends at most 8 rather than having the whole message dropped. In practice it's one or two: a held spot, a pinned placement, an open dialog. Editing nine lineup groups at once doesn't happen.
There was a problem hiding this comment.
That’s fair. Since the room accepts at most eight groups, truncating the client payload to eight is the correct compatibility behavior, and the expected editing flows normally involve only one or two groups. I’m withdrawing this as a finding; no change is needed.
1 participant
Stacked on #234. It needs #236's lineup group ids, so merge it into #234's branch, or after #234.
A lineup group (lineups that share a spot) is one cloud row, so two people changing it at the same time conflict. Rather than lock groups, presence now shows who is working on one. The lineup panel, the edit dialog and the media view say "Sam is editing these lineups right now.", with a dot in Sam's presence color. Two editors read "Sam and Alex are…"; three or more read "Sam and 2 others are…".
What counts as editing: the groups of the lineup spots you're dragging, the spot a new lineup is being placed from, and the lineups an open panel or edit dialog shows.
Room (
presence/):{t: "editing", page, groups}.welcomeandjoin, broadcast only when it changes.Client:
Verified:
presence/:npm run typecheckis clean;npm testpasses 20 of 20, including 7 new tests.flutter test: 1576 passed, 4 skipped. That includes tests for:Not verified: a live run against the deployed Worker. The tests use fakes.
🤖 Generated with Claude Code
Do not merge until renewal preserves editing changes made while pass verification is pending. The dialog attribution and eight-group limit issues are non-blocking.
Findings
Summary
The PR adds lineup-group editing notices to the presence room and lineup views. A pass renewal can restore a cleared editing state, so newcomers see an incorrect notice; this must be fixed before merging. An edit dialog can also attribute an old-page draft to a group on a newly active page, and groups beyond the eight-group message limit lose their notices. Those two notice issues are non-blocking.
Reviews (1) · Last reviewed commit: "Never drop a cleared editing; fit the no..."