mirror of
https://github.com/simplex-chat/simplex-chat.git
synced 2026-08-26 05:10:07 +00:00
drop the listUsers refresh that raced changeActiveUser_, and un-delay row-tap errors
clear() + addAll() ran from Main while changeActiveUser_ does the same pair from withBGApi, so a notification action switching user mid-creation could interleave them and leave chatModel.users duplicated or short. The refresh had nothing to add over the row already appended - newUser is the API's own record and a new profile has no unread messages - so it is gone. changeProfile is reached from the create flow and from a plain row tap; routing both through alertAfterDismissal delayed row-tap errors half a second for no sheet, detaching them from the tap. Pass dismissingSheet instead. Also gate the remaining ungated incognito write, matching its sibling and both Kotlin ones.
This commit is contained in:
@@ -171,7 +171,9 @@ struct ContextProfilePickerView: View {
|
||||
if selectedUser == user {
|
||||
if !incognitoDefault {
|
||||
listExpanded.toggle()
|
||||
} else {
|
||||
} else if !busy {
|
||||
// Gated like the sibling write in incognitoOption, and like both of
|
||||
// the Kotlin ones: only expand/collapse stays live while busy.
|
||||
incognitoDefault = false
|
||||
listExpanded = false
|
||||
}
|
||||
@@ -315,10 +317,16 @@ struct ContextProfilePickerView: View {
|
||||
showAddProfile = false
|
||||
changingProfile = true
|
||||
}
|
||||
changeProfile(newUser)
|
||||
changeProfile(newUser, dismissingSheet: true)
|
||||
}
|
||||
|
||||
private func changeProfile(_ newUser: User) {
|
||||
/// [dismissingSheet] only on the create path, which closes the form first: the delay is
|
||||
/// there to outlast a sheet dismissal, and on the plain row tap there is no sheet, so it
|
||||
/// would just detach the error from the tap that caused it - master alerted at once.
|
||||
private func changeProfile(_ newUser: User, dismissingSheet: Bool = false) {
|
||||
func report(_ title: String, _ message: String? = nil) {
|
||||
if dismissingSheet { alertAfterDismissal(title, message) } else { showAlert(title, message: message) }
|
||||
}
|
||||
Task {
|
||||
defer { Task { @MainActor in changingProfile = false } }
|
||||
do {
|
||||
@@ -342,7 +350,7 @@ struct ContextProfilePickerView: View {
|
||||
do {
|
||||
try await changeActiveUserAsync_(newUser.userId, viewPwd: nil, keepingChatId: chat.id)
|
||||
} catch {
|
||||
alertAfterDismissal(
|
||||
report(
|
||||
NSLocalizedString("Error switching profile", comment: "alert title"),
|
||||
String.localizedStringWithFormat(NSLocalizedString("Your chat was moved to %@ but an unexpected error occurred while redirecting you to the profile.", comment: "alert message"), newUser.chatViewName)
|
||||
)
|
||||
@@ -353,7 +361,7 @@ struct ContextProfilePickerView: View {
|
||||
selectedUser = currentUser
|
||||
}
|
||||
}
|
||||
alertAfterDismissal(
|
||||
report(
|
||||
NSLocalizedString("Error changing chat profile", comment: "alert title"),
|
||||
responseError(error)
|
||||
)
|
||||
|
||||
+6
-10
@@ -406,16 +406,12 @@ fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) {
|
||||
// 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) {
|
||||
val updatedUsers = runCatching { controller.listUsers(rhId) }.getOrNull()
|
||||
if (updatedUsers != null) {
|
||||
chatModel.users.clear()
|
||||
chatModel.users.addAll(updatedUsers)
|
||||
} else if (chatModel.users.none { it.user.userId == newUser.userId }) {
|
||||
chatModel.users.add(UserInfo(newUser, 0))
|
||||
}
|
||||
// Just this row, not a listUsers refresh: that clears and refills the whole list
|
||||
// from Main while changeActiveUser_ can be doing the same from withBGApi, and it
|
||||
// has nothing to add - newUser is the API's own record and a profile created a
|
||||
// moment ago has no unread messages.
|
||||
if (chatModel.remoteHostId() == rhId && 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
|
||||
// notification tap or a host switch can have changed that while we were creating.
|
||||
|
||||
@@ -397,6 +397,13 @@ re-derive them.
|
||||
close that targets a specific id — a change to the subtlest code in the branch, for a
|
||||
window of about a second.
|
||||
|
||||
- **`alertAfterDismissal` on paths with no sheet — fixed.** `changeProfile` is reached both
|
||||
from the create flow (which dismisses the form first) and from an ordinary row tap (which
|
||||
dismisses nothing), so routing all of its errors through the delay detached them from the
|
||||
tap that caused them — master alerted at once. It now takes `dismissingSheet`, set only by
|
||||
the create path. The remaining note about the fixed delay still applies to the four callers
|
||||
that do dismiss a sheet:
|
||||
|
||||
- **`alertAfterDismissal` waits a fixed 0.5 s** (`ShareSheet.swift`). It exists because
|
||||
UIKit drops an alert presented on a controller that is still dismissing. The delay is a
|
||||
proxy for "the sheet has finished", so a slow enough dismissal still drops the alert —
|
||||
@@ -547,3 +554,11 @@ entries flagged `activeUser`, which makes `firstOrNull { it.activeUser }` resolv
|
||||
whichever comes first. Corrupting which profile reads as active is worse than a missing
|
||||
row, and the missing row needs an outdated remote host *and* a failed resync to appear at
|
||||
all; the alert on that path tells the user something went wrong either way.
|
||||
|
||||
The `listUsers` refresh in `createProfileForInvitation` was **removed** rather than kept:
|
||||
`clear()` + `addAll()` from `withApi` (Main) raced the identical pair `changeActiveUser_`
|
||||
runs from `withBGApi`'s single-thread dispatcher — reachable when a notification action
|
||||
switches user mid-creation — and it had nothing to contribute. `newUser` is the API's own
|
||||
record, and a profile created a moment ago has no unread messages, so appending the one row
|
||||
is both correct and strictly less to go wrong. The later re-check of the active user did not
|
||||
help: it runs after the list has already been clobbered.
|
||||
|
||||
Reference in New Issue
Block a user