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.
This commit is contained in:
Narasimha-sc
2026-08-07 16:47:12 +00:00
parent d2adea5481
commit 1f948bcab4
4 changed files with 45 additions and 34 deletions
@@ -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
@@ -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 {
@@ -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
+10 -4
View File
@@ -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