plans, tests: correct claims the review pass disproved

The /users assertion added with the active_order fix does not test it: viewUsersList
sorts alphabetically by display name, so "bob" precedes "robert" whatever their order
is. What the assertion does prove is that the created user's active_user column is 0,
which /u does not cover - the comment now says that and nothing more. The ordering
itself has no regression coverage; the sort sites are all in the clients.

The plan claimed the picker's composition is disposed on Android while the form is on
top. That is true of the one-time link picker, which is itself a modal, and false of
the compose picker, which is not. It also recorded the pane-per-picker placement that
this round replaced with fullscreen.
This commit is contained in:
Narasimha-sc
2026-08-06 14:17:37 +00:00
parent 49a8cafa94
commit d610757831
2 changed files with 22 additions and 16 deletions
+19 -14
View File
@@ -121,16 +121,17 @@ Uses the existing `users_add` ("Add profile") string — **zero new translation
the row must be emitted **last** to render at the top. Surface 2's is not reversed, and
the row must sit **outside** the `activeProfile != null` branch or it disappears
whenever search text filters the active profile out.
- **The modal must open in the picker's own pane.** `ModalManager.center.showCustomModal`
sets `ChatModel.chatId = null` (`ModalView.kt`), and on desktop the chat view **is** the
centre pane — so opening the form from the compose picker closed the very invitation it
was for, taking the typed compose draft with it, and only the success path reopened it.
From surface 2, itself a start-pane modal, the form landed in another pane with the
picker still live beside it (the scrim at `App.kt` is suppressed while a centre modal is
open) — and `apiChangeConnectionUser` recreates the connection, so a row tapped
meanwhile invalidates the `pccConnId` the form is about to use. Pass the manager in:
`end` for surface 1, `start` for surface 2, matching the incognito modal each already
opens.
- **The modal must open in `ModalManager.fullscreen`.** Every other placement is wrong on
desktop, where the four managers are four panes: `center` sets `ChatModel.chatId = null`
(`ModalView.kt`) and the chat view **is** the centre pane, so the form closed the very
invitation it was for, taking the typed compose draft with it, and only the success path
reopened it. `end` leaves the compose picker live in the pane beside the form — and
`apiChangeConnectionUser` recreates the connection, so a row tapped meanwhile
invalidates the `pccConnId` the form is about to use — and its
`desktopExpandWindowToWidth` widens the window for good. `start` works for surface 2 but
disposes that picker, losing its search text. `fullscreen` is an opaque `Surface` over
every pane: no picker can be operated while the form is up, none is torn down, and
nothing else moves. On Android all four are the same manager regardless.
- **`keepingChatId` does not open the chat**, it only preserves its place in the reloaded
list. It does not need to: the earlier claim that you land on the chat list without an
explicit `chatModel.chatId.value = chat.id` was the `center` placement above, not
@@ -145,10 +146,14 @@ Uses the existing `users_add` ("Add profile") string — **zero new translation
skips the resetting `finally`) and a lazy item is disposed by scrolling — but so is the
whole picker: on Android every `ModalManager` placement shares one stack and
`showInView` renders only the top entry, so pushing the form disposes surface 2's
picker and it returns with every `remember` reset. Hence one top-level
`creatingProfileForInvitation`, and a **suspending** `onCreated` so the flag covers the
reassignment rather than just the creation — `changeProfile`/`selectProfile` only
*launch* it.
picker (a modal) and it returns with every `remember` reset. Surface 1's picker is not
a modal and survives on both platforms; the flag is shared anyway. Hence
`ChatModel.creatingProfileForInvitation` — in the model, not a top-level `val`, so it
is not first created inside a composition — and a **suspending** `onCreated` so the flag
covers the reassignment rather than just the creation (`changeProfile`/`selectProfile`
only *launch* it). Hand `onCreated` back to `Dispatchers.Main` before calling it: it
reassigns and switches, and every structure it touches is also written by the receiver
loop on Main.
- **Re-check the active user and the host before reassigning.** The reassignment resolves
the invitation under whatever is active when it runs, and nothing holds
`changingActiveUserMutex` across this flow: a notification tap
+3 -2
View File
@@ -4005,8 +4005,9 @@ testCreateUserKeepingActiveUser = testChat2 aliceProfile bobProfile test
-- ... and the active user is unchanged, with no switch back needed
bob ##> "/u"
showActiveUser bob "bob (Bob)"
-- the new user's own record is not active either - the clients branch on this
-- field of the created user, and it is listed last, having never been activated
-- the new user's record is not active either, which is the field both clients
-- branch on to detect a core that ignored the flag; /u only covers the TVar.
-- (/users sorts by display name, so the order here says nothing about active_order.)
bob ##> "/users"
bob <## "bob (Bob) (active)"
bob <## "robert"