From d61075783128d2ca0e00434f5ff43ad0cc0e8aca Mon Sep 17 00:00:00 2001 From: Narasimha-sc <166327228+Narasimha-sc@users.noreply.github.com> Date: Thu, 6 Aug 2026 14:17:37 +0000 Subject: [PATCH] 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. --- .../2026-07-30-new-profile-for-invitation.md | 33 +++++++++++-------- tests/ChatTests/Profiles.hs | 5 +-- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/plans/2026-07-30-new-profile-for-invitation.md b/plans/2026-07-30-new-profile-for-invitation.md index d255e26816..e8cd1b76b8 100644 --- a/plans/2026-07-30-new-profile-for-invitation.md +++ b/plans/2026-07-30-new-profile-for-invitation.md @@ -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 diff --git a/tests/ChatTests/Profiles.hs b/tests/ChatTests/Profiles.hs index fdcf1d4ee8..ee2ff5dc93 100644 --- a/tests/ChatTests/Profiles.hs +++ b/tests/ChatTests/Profiles.hs @@ -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"