From 1f948bcab4924b812485de73ca9cbe61ebe7452a Mon Sep 17 00:00:00 2001 From: Narasimha-sc <166327228+Narasimha-sc@users.noreply.github.com> Date: Fri, 7 Aug 2026 16:47:12 +0000 Subject: [PATCH] put the profile registration between the two early returns, not above both Above the stale-host branch it left two entries flagged activeUser when an old core activated the profile anyway, so a failed resync made firstOrNull { it.activeUser } resolve to whichever came first. It also only ever updated chatModel.users, never the pickers' own users and profiles state, which is otherwise filled in onAppear alone - so on the ownership-guard path the profile existed everywhere except the picker the user was looking at. Both lists now update at one point, which replaces the later fallbacks. --- .../ContextProfilePickerView.swift | 25 ++++++++++--------- .../Shared/Views/NewChat/NewChatView.swift | 19 ++++++++------ .../chat/simplex/common/views/WelcomeView.kt | 21 ++++++++-------- .../2026-07-30-new-profile-for-invitation.md | 14 ++++++++--- 4 files changed, 45 insertions(+), 34 deletions(-) diff --git a/apps/ios/Shared/Views/Chat/ComposeMessage/ContextProfilePickerView.swift b/apps/ios/Shared/Views/Chat/ComposeMessage/ContextProfilePickerView.swift index 1f709c44e7..a140d95769 100644 --- a/apps/ios/Shared/Views/Chat/ComposeMessage/ContextProfilePickerView.swift +++ b/apps/ios/Shared/Views/Chat/ComposeMessage/ContextProfilePickerView.swift @@ -259,14 +259,6 @@ struct ContextProfilePickerView: View { let ownerUserId = await MainActor.run { chatModel.currentUser?.userId } let profile = Profile(displayName: displayName, fullName: "", shortDescr: shortDescr, image: image) let newUser = try apiCreateActiveUser(profile, keepActiveUser: true) - // Before any early return below, as Kotlin does: nothing else on iOS refreshes - // chatModel.users, so a profile left out of it exists in the database but in no - // list the app shows - and creating it again fails on the duplicate name. - await MainActor.run { - if !chatModel.users.contains(where: { $0.user.userId == newUser.userId }) { - chatModel.users.append(UserInfo(user: newUser, unreadCount: 0)) - } - } // Checked before refreshing the lists below: on this path the core has already // activated the new profile, so they would disagree with chatModel.currentUser // until the resync lands - and changeActiveUserAsync_ refreshes them anyway. @@ -290,6 +282,19 @@ struct ContextProfilePickerView: View { alertAfterDismissal(NSLocalizedString("Error changing chat profile", comment: "alert title")) return } + // Below the branch above, which returns: adding it there would leave two entries + // flagged activeUser, and its own resync refreshes the list anyway. Above the guard + // below, which returns without refreshing either list - the profile exists by now, + // so it has to appear in both or the next attempt at the same name is a duplicate. + // users is otherwise only filled in onAppear. + await MainActor.run { + if !chatModel.users.contains(where: { $0.user.userId == newUser.userId }) { + chatModel.users.append(UserInfo(user: newUser, unreadCount: 0)) + } + if !users.contains(where: { $0.userId == newUser.userId }) { + users.append(newUser) + } + } // changeProfile resolves the prepared chat under whatever is active when it runs, // and a notification action can have switched it while we were creating. guard await MainActor.run({ chatModel.currentUser?.userId }) == ownerUserId else { @@ -303,10 +308,6 @@ struct ContextProfilePickerView: View { chatModel.users = updatedUsers // Only filled in onAppear otherwise, so the new profile is missing here users = updatedUsers.map { $0.user }.filter { u in u.activeUser || !u.hidden } - } else if !users.contains(where: { $0.userId == newUser.userId }) { - // changeProfile sets selectedUser to it, and otherUsers filters on that - - // absent from users, nothing is filtered out and a row is clipped. - users.append(newUser) } // changingProfile here too: the defer clears creatingProfile as soon as this returns showAddProfile = false diff --git a/apps/ios/Shared/Views/NewChat/NewChatView.swift b/apps/ios/Shared/Views/NewChat/NewChatView.swift index 613daf40f8..44c8a1f131 100644 --- a/apps/ios/Shared/Views/NewChat/NewChatView.swift +++ b/apps/ios/Shared/Views/NewChat/NewChatView.swift @@ -615,14 +615,6 @@ private struct ActiveProfilePicker: View { let ownerUserId = await MainActor.run { chatModel.currentUser?.userId } let profile = Profile(displayName: displayName, fullName: "", shortDescr: shortDescr, image: image) let newUser = try apiCreateActiveUser(profile, keepActiveUser: true) - // Before any early return below, as Kotlin does: nothing else on iOS refreshes - // chatModel.users, so a profile left out of it exists in the database but in no - // list the app shows - and creating it again fails on the duplicate name. - await MainActor.run { - if !chatModel.users.contains(where: { $0.user.userId == newUser.userId }) { - chatModel.users.append(UserInfo(user: newUser, unreadCount: 0)) - } - } // Checked before refreshing the lists below: on this path the core has already // activated the new profile, so they would disagree with chatModel.currentUser // until the resync lands - and changeActiveUserAsync_ refreshes them anyway. @@ -646,6 +638,17 @@ private struct ActiveProfilePicker: View { alertAfterDismissal(NSLocalizedString("Error changing chat profile", comment: "alert title")) return } + // Below the branch above, which returns: adding it there would leave two entries + // flagged activeUser, and its own resync refreshes the list anyway. Above the guard + // below, which returns without refreshing either list - the profile exists by now, + // so it has to appear in both or the next attempt at the same name is a duplicate. + // profiles is otherwise only filled in onAppear. + await MainActor.run { + if !chatModel.users.contains(where: { $0.user.userId == newUser.userId }) { + chatModel.users.append(UserInfo(user: newUser, unreadCount: 0)) + } + profiles = chatModel.users.map { $0.user } + } // apiChangeConnectionUser resolves pccConnId under whatever is active when the // selectedProfile handler runs, and a notification action can have switched it. guard await MainActor.run({ chatModel.currentUser?.userId }) == ownerUserId else { diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/WelcomeView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/WelcomeView.kt index edfd52a86e..34a81d2548 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/WelcomeView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/WelcomeView.kt @@ -390,12 +390,6 @@ fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) { val ownerUserId = chatModel.currentUser.value?.userId val profile = Profile(displayName.trim(), "", shortDescr.trim().ifEmpty { null }, image = image) val newUser = controller.apiCreateActiveUser(rhId, profile, keepActiveUser = true) ?: return@withApi - // Before any early return below, as on iOS: nothing else adds it, so a profile - // left out of chatModel.users exists in the database but in no list the app - // shows, and creating it again fails on the duplicate name. - if (chatModel.remoteHostId() == rhId && chatModel.users.none { it.user.userId == newUser.userId }) { - chatModel.users.add(UserInfo(newUser, 0)) - } if (newUser.activeUser) { // An older remote host ignored the flag and activated it, so the reassignment // would fail. Resync to what the host did and report it, with the form @@ -408,12 +402,19 @@ fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) { AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user)) return@withApi } - // Refresh so the new profile carries its real unread count and ordering rather - // than the placeholder added above. listUsers throws and withApi does not catch. + // Below the branch above, which returns: adding it there would leave two entries + // flagged activeUser, and its own resync refreshes the list anyway. Above the + // check below, which returns without listing it - the profile exists by now, so + // it has to be in the list or the next attempt at the same name is a duplicate. + // listUsers throws and withApi does not catch, so the refresh is guarded and the + // placeholder stands in for it, carrying the real counts once it succeeds. if (chatModel.remoteHostId() == rhId) { - runCatching { controller.listUsers(rhId) }.getOrNull()?.let { + val updatedUsers = runCatching { controller.listUsers(rhId) }.getOrNull() + if (updatedUsers != null) { chatModel.users.clear() - chatModel.users.addAll(it) + chatModel.users.addAll(updatedUsers) + } else if (chatModel.users.none { it.user.userId == newUser.userId }) { + chatModel.users.add(UserInfo(newUser, 0)) } } // onCreated resolves the invitation under whatever is active when it runs, and a diff --git a/plans/2026-07-30-new-profile-for-invitation.md b/plans/2026-07-30-new-profile-for-invitation.md index 95c4809147..1dba79748a 100644 --- a/plans/2026-07-30-new-profile-for-invitation.md +++ b/plans/2026-07-30-new-profile-for-invitation.md @@ -413,10 +413,16 @@ Three more, from a later round: `clickable(enabled = !busy)` on the profile and incognito rows also blocks their purely local `listExpanded` toggle, so the picker cannot be collapsed while a change is in flight; iOS gates only the work-starting branch (`if busy { return }` inside the tap). - Kept deliberately: master had *no* gate on those rows at all, so this is already - stricter than what it replaced, and holding the picker still during the change avoids - swapping `profilePicker()` for `currentSelection()` mid-flight — which on iOS is exactly - what disposed a presented sheet (see the "sheet's owner must outlive the sheet" note). + Master had *no* gate on those rows at all, so this is stricter than what it replaced. + The cost is real though, and worse than first written here: this picker has no + `progressByTimeout` spinner or alpha change, unlike `ActiveProfilePicker`, so during a + slow `changeProfileTo` it is simply inert with no feedback and cannot even be collapsed. + The justification originally given — that holding it still avoids swapping + `profilePicker()` for `currentSelection()` mid-flight — was borrowed from iOS, where that + swap disposes a presented sheet; Kotlin has no sheet to lose, so it does not apply here. + Left as is only because the change is cosmetic and touching these rows again is the kind + of edit that has repeatedly introduced defects in this branch. Gate the work-starting + branches instead, as iOS does, if this picker is revisited. - **`submitting:` and `.interactiveDismissDisabled(...)` are read inside the `.sheet` content closure.** If SwiftUI does not re-invoke that closure when the presenting view's