mirror of
https://github.com/simplex-chat/simplex-chat.git
synced 2026-08-16 13:20:19 +00:00
fix three gaps the targeted review found
The abort on "active user or host changed" left the create-profile form open over a profile that had already been created, so the next Create failed on the duplicate name for as long as the name was unchanged - a dead end with no way out but retyping. It closes the form now, like every other terminal path in that function. The compose picker's busy state only covered the create flow, so both the existing rows and the new one stayed live during an ordinary profile change. Tapping a profile and then "Add profile" reassigned the same prepared chat twice and issued two overlapping changeActiveUser_ calls, whose chat reloads are not serialised. Master had no way to trigger this - the new row is the third way in - so the flag comes back, but only around changeProfile, leaving changeProfileTo untouched. iOS never re-checked the active user between creating the profile and reassigning, which §4 requires and Kotlin does. A notification action switching profile in that window makes the reassignment resolve under the wrong user and strand the profile just created. Only the user is checked, not the host - iOS is never the controller. Records the two hazards deliberately left as they are on master: the picker's unguarded close(), which the ordinary row tap has had all along, and alertAfterDismissal's fixed delay.
This commit is contained in:
@@ -256,6 +256,7 @@ struct ContextProfilePickerView: View {
|
||||
}
|
||||
if alreadyCreating { return }
|
||||
defer { Task { @MainActor in creatingProfile = false } }
|
||||
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)
|
||||
// Checked before refreshing the lists below: on this path the core has already
|
||||
@@ -281,6 +282,13 @@ struct ContextProfilePickerView: View {
|
||||
alertAfterDismissal(NSLocalizedString("Error changing chat profile", comment: "alert title"))
|
||||
return
|
||||
}
|
||||
// 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 {
|
||||
await MainActor.run { showAddProfile = false }
|
||||
alertAfterDismissal(NSLocalizedString("Error changing chat profile", comment: "alert title"))
|
||||
return
|
||||
}
|
||||
let updatedUsers = try? await listUsersAsync()
|
||||
await MainActor.run {
|
||||
if let updatedUsers = updatedUsers {
|
||||
|
||||
@@ -592,6 +592,7 @@ private struct ActiveProfilePicker: View {
|
||||
}
|
||||
if alreadyCreating { return }
|
||||
defer { Task { @MainActor in creatingProfile = false } }
|
||||
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)
|
||||
// Checked before refreshing the lists below: on this path the core has already
|
||||
@@ -617,6 +618,13 @@ private struct ActiveProfilePicker: View {
|
||||
alertAfterDismissal(NSLocalizedString("Error changing chat profile", comment: "alert title"))
|
||||
return
|
||||
}
|
||||
// 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 {
|
||||
await MainActor.run { showAddProfile = false }
|
||||
alertAfterDismissal(NSLocalizedString("Error changing chat profile", comment: "alert title"))
|
||||
return
|
||||
}
|
||||
let updatedUsers = try? await listUsersAsync()
|
||||
await MainActor.run {
|
||||
if let updatedUsers = updatedUsers { chatModel.users = updatedUsers }
|
||||
|
||||
+3
@@ -401,6 +401,9 @@ fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) {
|
||||
// onCreated resolves the invitation under whatever is active when it runs, and a
|
||||
// notification tap or a host switch can have changed that while we were creating.
|
||||
if (chatModel.currentUser.value?.userId != ownerUserId || chatModel.remoteHostId() != rhId) {
|
||||
// Closed: the profile exists, so leaving the form up means the next Create
|
||||
// fails on the duplicate name for as long as the name is unchanged.
|
||||
if (modalManager.isLastModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) close()
|
||||
AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user))
|
||||
return@withApi
|
||||
}
|
||||
|
||||
+10
-4
@@ -41,9 +41,10 @@ fun ComposeContextProfilePickerView(
|
||||
val incognitoDefault = chatModel.controller.appPrefs.incognito.get()
|
||||
val users = chatModel.users.map { it.user }.filter { u -> u.activeUser || !u.hidden }
|
||||
val listExpanded = remember { mutableStateOf(false) }
|
||||
// Set until the invitation has moved onto the new profile, which is after the form has
|
||||
// closed and this picker is interactive again
|
||||
val busy = chatModel.creatingProfileForInvitation.value
|
||||
val changingProfile = remember { mutableStateOf(false) }
|
||||
// Creating stays set until the invitation has moved, which is after the form has closed
|
||||
// and this picker is interactive again
|
||||
val busy = changingProfile.value || chatModel.creatingProfileForInvitation.value
|
||||
|
||||
val maxHeightInPx = with(LocalDensity.current) { windowHeight().toPx() }
|
||||
val isVisible = remember { mutableStateOf(false) }
|
||||
@@ -121,7 +122,12 @@ fun ComposeContextProfilePickerView(
|
||||
}
|
||||
|
||||
fun changeProfile(newUser: User) {
|
||||
withApi { changeProfileTo(newUser) }
|
||||
// Set before withApi, which dispatches - the rows would be live until it runs.
|
||||
// The create path is covered by creatingProfileForInvitation instead.
|
||||
changingProfile.value = true
|
||||
withApi {
|
||||
try { changeProfileTo(newUser) } finally { changingProfile.value = false }
|
||||
}
|
||||
}
|
||||
|
||||
fun showCantChangeProfileAlert() {
|
||||
|
||||
@@ -180,6 +180,19 @@ Uses the existing `users_add` ("Add profile") string — **zero new translation
|
||||
new profile always has the SimpleX Team/Status cards. Core returns the updated contact;
|
||||
do not pre-check.
|
||||
|
||||
Two hazards left as they are on master, so review does not keep re-raising them:
|
||||
|
||||
- `selectProfileAsync`'s trailing `close()` pops whatever is on top rather than the
|
||||
picker, so backing out during the connection change dismisses the screen underneath.
|
||||
The ordinary row tap has had exactly that window since before this branch; creating a
|
||||
profile first is local and fast and barely widens it. Fixing it needs a `ModalViewId`
|
||||
on the picker at both call sites, including `ShareListView`, which does not offer this
|
||||
feature at all.
|
||||
- `alertAfterDismissal` waits a fixed 0.5s for a sheet transition rather than observing
|
||||
it. A slow device or a late-released interactive dismissal can still outlast it. The
|
||||
deterministic version needs the presenting controller's completion handler, which is
|
||||
not reachable from where these alerts are raised.
|
||||
|
||||
## 5. Generated and hand-synced artifacts — easy to miss
|
||||
|
||||
`NewUser` is a documented API type, and `apiDocsTest` generates **11 files** from those
|
||||
|
||||
Reference in New Issue
Block a user