From f2f5ab3892076496161d87c9f3b82dfce46e0dac Mon Sep 17 00:00:00 2001 From: Narasimha-sc <166327228+Narasimha-sc@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:45:11 +0000 Subject: [PATCH] android, desktop: fix the create-profile-for-invitation flow Six defects, all in the window between creating the profile and moving the invitation onto it. Pane. The form was shown in ModalManager.center, which on desktop sets ChatModel.chatId to null - and the chat view *is* the centre pane. Tapping "Add profile" in a prepared invitation therefore closed that chat, discarding the typed compose draft with it, and only the success path reopened it: cancelling the form, or any failure, left "No selected chat" with the invitation nowhere on screen. From the one-time link picker, which is itself a start-pane modal, the form appeared in a different pane with the picker still live beside it and no scrim, so a profile could be selected while the form was open - and apiChangeConnectionUser recreates the connection, invalidating the pccConnId the form was about to use. The manager is now passed in: end for the compose picker, matching the incognito modal it already opens there, and start for the one-time link picker, matching its own. In-flight guard. The flag was per-picker and released when the profile existed, not when the invitation had moved. Both are wrong: onCreated only launches the reassignment, and on Android the picker's composition is disposed while the form is on top of it, so the flag it comes back with is a fresh false. There was no spinner, no dimming and every row was live during the switch, so a second tap created a second profile or started a competing reassignment. onCreated is now suspending so the flag covers the whole flow, and the flag is a single top-level state both pickers read. The compose picker had no in-progress state at all and now has one, and its new row was also the only one missing the profileChangeProhibited guard. Active user and host. The reassignment resolves the invitation under whatever profile is active when it runs. A notification tap switches the active user from another dispatcher, and a remote host connect/disconnect switches the host, so both are checked before handing over rather than reassigning under the wrong profile. Torn-down form. isLastModalOpen was used only to decide whether to close the form, and the flow continued when it was false - reassigning and switching under a screen the user had already left. It now stops. Connection not moved. The one-time link picker dismissed itself and turned the app-wide incognito default off whether or not apiChangeConnectionUser succeeded. Unlike the prepared-chat reassignment, that call provisions a new queue, so it is what fails offline - leaving a profile just created for the invitation stranded with nothing pointing at it and the picker gone. It now keeps the picker open and only clears incognito once the connection has actually moved. Reopening the chat. chatModel.chatId was set unconditionally after the switch, before the check that the switch actually happened, pointing the chat view at a chat that had just moved to another profile and defeating the guard in updateChats that clears it. Also applies the users refresh on the main thread, where the receiver loop updates the same list, and skips it when the host has changed underneath. --- .../chat/simplex/common/views/WelcomeView.kt | 58 +++++++-- .../chat/ComposeContextProfilePickerView.kt | 48 ++++++-- .../common/views/newchat/NewChatView.kt | 116 ++++++++++-------- 3 files changed, 145 insertions(+), 77 deletions(-) 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 f281306587..324057f2b7 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 @@ -46,9 +46,11 @@ import chat.simplex.common.views.usersettings.DeleteImageButton import chat.simplex.common.views.usersettings.EditImageButton import chat.simplex.common.views.usersettings.SettingsActionItem import chat.simplex.res.MR +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import java.net.URI const val MAX_BIO_LENGTH_BYTES = 160 @@ -347,20 +349,35 @@ private fun CreateFirstProfileDesktop(chatModel: ChatModel, close: () -> Unit) { } } +/** True while a profile is being created for an invitation and handed over to the picker + * that asked for it. Deliberately not remembered in either picker: on Android the picker's + * composition is disposed while the create-profile modal is on top of it, and comes back + * with every remembered flag reset - a per-picker flag would be released the moment the + * form opens, leaving the rows live during the reassignment. */ +val creatingProfileForInvitation = mutableStateOf(false) + // Creates a profile for an invitation and hands it to onCreated, which moves the // invitation onto it. The profile is created *without* becoming active: the reassignment // APIs resolve the prepared chat or connection under the active user, so the profile that -// owns the invitation has to stay active until onCreated has run. -fun createProfileForInvitation(rhId: Long?, creating: MutableState, onCreated: (User) -> Unit) { +// owns the invitation has to stay active until onCreated has run - which is also why +// onCreated is suspending, so the in-flight flag covers the reassignment and not just the +// creation. +fun createProfileForInvitation(rhId: Long?, modalManager: ModalManager, onCreated: suspend (User) -> Unit) { + // Shown in the picker's own pane: ModalManager.center nulls chatId on desktop, which + // closes the chat the prepared invitation is in - and for the picker that is itself a + // start-pane modal, leaves the picker live beside the form. // Two taps before the modal renders would otherwise stack two modals sharing one id, // after which close() could dismiss the wrong one. - if (ModalManager.center.hasModalOpen(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) return - ModalManager.center.showModalCloseable(id = ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE) { close -> + if (modalManager.hasModalOpen(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) return + modalManager.showModalCloseable(id = ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE) { close -> CreateProfile { displayName, shortDescr, image -> - if (creating.value) return@CreateProfile - creating.value = true + if (creatingProfileForInvitation.value) return@CreateProfile + creatingProfileForInvitation.value = true withBGApi { try { + // The reassignment in onCreated resolves the invitation under whatever is active + // then, so remember what owns it now and check nothing moved underneath us. + val ownerUserId = chatModel.currentUser.value?.userId val profile = Profile(displayName.trim(), "", shortDescr.trim().ifEmpty { null }, image = image) val newUser = controller.apiCreateProfileKeepingActive(rhId, profile) ?: return@withBGApi if (newUser.activeUser) { @@ -374,17 +391,32 @@ fun createProfileForInvitation(rhId: Long?, creating: MutableState, onC } // Keep chatModel.users current even if onCreated's reassignment fails - it only // refreshes when it actually switches. listUsers throws and withBGApi does not - // catch, so this cosmetic refresh is guarded. - runCatching { controller.listUsers(rhId) }.getOrNull()?.let { updatedUsers -> - chatModel.users.clear() - chatModel.users.addAll(updatedUsers) + // catch, so this cosmetic refresh is guarded; and it is applied on the main + // thread, where the receiver loop also updates this list. + if (chatModel.remoteHostId() == rhId) { + runCatching { controller.listUsers(rhId) }.getOrNull()?.let { updatedUsers -> + withContext(Dispatchers.Main) { + chatModel.users.clear() + chatModel.users.addAll(updatedUsers) + } + } } - if (ModalManager.center.isLastModalOpen(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) { - close() + // A notification tap or a remote host switch can change the active user or tear + // the form down while the profile is being created. Reassigning after either + // would resolve the invitation under the wrong profile, or move it under a + // screen the user has already left, so stop with the profile created. + if ( + chatModel.currentUser.value?.userId != ownerUserId || + chatModel.remoteHostId() != rhId || + !modalManager.isLastModalOpen(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE) + ) { + AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user)) + return@withBGApi } + close() onCreated(newUser) } finally { - creating.value = false + creatingProfileForInvitation.value = false } } } diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeContextProfilePickerView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeContextProfilePickerView.kt index 5230e7d600..a8dfa6e1e6 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeContextProfilePickerView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeContextProfilePickerView.kt @@ -19,6 +19,7 @@ import chat.simplex.common.model.* import chat.simplex.common.platform.* import chat.simplex.common.ui.theme.* import chat.simplex.common.views.createProfileForInvitation +import chat.simplex.common.views.creatingProfileForInvitation import chat.simplex.common.views.helpers.* import chat.simplex.common.views.newchat.IncognitoOptionImage import chat.simplex.common.views.usersettings.IncognitoView @@ -41,8 +42,11 @@ 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) } - // Not rememberSaveable, and hoisted out of the lazy item: either strands it true. - val creatingProfile = remember { mutableStateOf(false) } + // Not rememberSaveable: process death would skip the resetting finally and strand it true. + val changingProfile = remember { mutableStateOf(false) } + // Creating a profile keeps the rows disabled too - the reassignment runs after the form + // closes, and until it has, picking anything else moves the invitation twice. + val busy = changingProfile.value || creatingProfileForInvitation.value val maxHeightInPx = with(LocalDensity.current) { windowHeight().toPx() } val isVisible = remember { mutableStateOf(false) } @@ -80,8 +84,9 @@ fun ComposeContextProfilePickerView( } } - fun changeProfile(newUser: User) { - withApi { + suspend fun changeProfileTo(newUser: User) { + changingProfile.value = true + try { var chatMoved = false if (chat.chatInfo is ChatInfo.Direct) { val updatedContact = chatModel.controller.apiChangePreparedContactUser(rhId, chat.chatInfo.contact.contactId, newUser.userId) @@ -104,6 +109,7 @@ fun ComposeContextProfilePickerView( } // Only switch profile if the chat was actually moved to it, otherwise the user // would end up in another profile with the invitation left behind in this one. + // apiChangePreparedContactUser/apiChangePreparedGroupUser report the failure. if (chatMoved) { chatModel.controller.changeActiveUser_( rhId = newUser.remoteHostId, @@ -111,21 +117,31 @@ fun ComposeContextProfilePickerView( viewPwd = null, keepingChatId = chat.id ) - // Reopen the chat under the new profile. keepingChatId only preserves its - // place in the reloaded list, so without this the switch lands on the chat - // list of the new profile rather than the invitation it was chosen for. - // The id is unchanged by the reassignment - it is the contact/group id. - chatModel.chatId.value = chat.id if (chatModel.currentUser.value?.userId != newUser.userId) { AlertManager.shared.showAlertMsg( generalGetString(MR.strings.switching_profile_error_title), String.format(generalGetString(MR.strings.switching_profile_error_message), newUser.chatViewName) ) + } else { + // Reopen the chat under the new profile - only once the switch is known to + // have happened, or this would point the chat view at a chat that now belongs + // to a different profile, defeating the guard in updateChats that clears it. + // The id is unchanged by the reassignment - it is the contact/group id. + chatModel.chatId.value = chat.id } } + } finally { + changingProfile.value = false } } + fun changeProfile(newUser: User) { + // Also set here, not only in changeProfileTo: withApi dispatches, so between the tap + // and the coroutine starting the row would still be enabled. + changingProfile.value = true + withApi { changeProfileTo(newUser) } + } + fun showCantChangeProfileAlert() { AlertManager.shared.showAlertMsg( generalGetString(MR.strings.context_user_picker_cant_change_profile_alert_title), @@ -139,7 +155,7 @@ fun ComposeContextProfilePickerView( Modifier .fillMaxWidth() .sizeIn(minHeight = DEFAULT_MIN_SECTION_ITEM_HEIGHT + 8.dp) - .clickable(onClick = { + .clickable(enabled = !busy, onClick = { if (!chat.chatInfo.profileChangeProhibited) { if (selectedUser.value.userId == user.userId) { if (!incognitoDefault) { @@ -182,7 +198,7 @@ fun ComposeContextProfilePickerView( Modifier .fillMaxWidth() .sizeIn(minHeight = DEFAULT_MIN_SECTION_ITEM_HEIGHT + 8.dp) - .clickable(onClick = { + .clickable(enabled = !busy, onClick = { if (!chat.chatInfo.profileChangeProhibited) { if (incognitoDefault) { listExpanded.value = !listExpanded.value @@ -242,7 +258,15 @@ fun ComposeContextProfilePickerView( Modifier .fillMaxWidth() .sizeIn(minHeight = DEFAULT_MIN_SECTION_ITEM_HEIGHT + 8.dp) - .clickable(onClick = { createProfileForInvitation(rhId, creatingProfile) { changeProfile(it) } }) + // ModalManager.end, like the incognito info modal above: the center manager nulls + // chatId on desktop, which closes the very chat this picker belongs to. + .clickable(enabled = !busy, onClick = { + if (!chat.chatInfo.profileChangeProhibited) { + createProfileForInvitation(rhId, ModalManager.end) { changeProfileTo(it) } + } else { + showCantChangeProfileAlert() + } + }) .padding(horizontal = DEFAULT_PADDING_HALF, vertical = 4.dp), horizontalArrangement = Arrangement.SpaceBetween, verticalAlignment = Alignment.CenterVertically diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/NewChatView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/NewChatView.kt index 4bf48c85da..1e8bdd3268 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/NewChatView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/NewChatView.kt @@ -40,6 +40,7 @@ import chat.simplex.common.ui.theme.* import chat.simplex.common.views.chat.item.CIFileViewScope import chat.simplex.common.views.chat.topPaddingToContent import chat.simplex.common.views.createProfileForInvitation +import chat.simplex.common.views.creatingProfileForInvitation import chat.simplex.common.views.helpers.* import chat.simplex.common.views.usersettings.* import chat.simplex.common.BuildConfigCommon @@ -294,8 +295,6 @@ fun ActiveProfilePicker( showIncognito: Boolean = true ) { val switchingProfile = remember { mutableStateOf(false) } - // Not rememberSaveable, and hoisted out of the lazy item: either strands it true. - val creatingProfile = remember { mutableStateOf(false) } val incognito = remember { chatModel.showingInvitation.value?.conn?.incognito ?: controller.appPrefs.incognito.get() } @@ -308,67 +307,80 @@ fun ActiveProfilePicker( var progressByTimeout by rememberSaveable { mutableStateOf(false) } - LaunchedEffect(switchingProfile.value) { - progressByTimeout = if (switchingProfile.value) { + // Creating a profile for this invitation keeps the picker busy until the connection has + // been moved onto it, not just until the profile exists. + val busy = switchingProfile.value || creatingProfileForInvitation.value + + LaunchedEffect(busy) { + progressByTimeout = if (busy) { delay(500) - switchingProfile.value + busy } else { false } } + suspend fun selectProfileAsync(user: User) { + switchingProfile.value = true + try { + var updatedConn: PendingContactConnection? = null + + if (contactConnection != null) { + updatedConn = controller.apiChangeConnectionUser(rhId, contactConnection.pccConnId, user.userId) + // The connection was not moved - apiChangeConnectionUser reports it. Leave the + // picker open instead of switching or dismissing: a profile just created for this + // invitation would otherwise be stranded with nothing pointing at it, and the + // connection needs the network here, so this is what happens when offline. + if (updatedConn == null) return + withContext(Dispatchers.Main) { + chatModel.chatsContext.updateContactConnection(rhId, updatedConn) + updateShownConnection(updatedConn) + } + } + // Set only once the connection is known to have moved, so a failed reassignment + // does not silently turn the app-wide incognito default off. + appPreferences.incognito.set(false) + + controller.changeActiveUser_( + rhId = user.remoteHostId, + toUserId = user.userId, + viewPwd = if (user.hidden) searchTextOrPassword.value else null + ) + + if (chatModel.currentUser.value?.userId != user.userId) { + AlertManager.shared.showAlertMsg(generalGetString( + MR.strings.switching_profile_error_title), + String.format(generalGetString(MR.strings.switching_profile_error_message), user.chatViewName) + ) + } + + if (updatedConn != null) { + withContext(Dispatchers.Main) { + chatModel.chatsContext.updateContactConnection(user.remoteHostId, updatedConn) + } + } + + close() + } finally { + switchingProfile.value = false + } + } + fun selectProfile(user: User) { switchingProfile.value = true - withApi { - try { - appPreferences.incognito.set(false) - var updatedConn: PendingContactConnection? = null; - - if (contactConnection != null) { - updatedConn = controller.apiChangeConnectionUser(rhId, contactConnection.pccConnId, user.userId) - if (updatedConn != null) { - withContext(Dispatchers.Main) { - chatModel.chatsContext.updateContactConnection(rhId, updatedConn) - updateShownConnection(updatedConn) - } - } - } - - if ((contactConnection != null && updatedConn != null) || contactConnection == null) { - controller.changeActiveUser_( - rhId = user.remoteHostId, - toUserId = user.userId, - viewPwd = if (user.hidden) searchTextOrPassword.value else null - ) - - if (chatModel.currentUser.value?.userId != user.userId) { - AlertManager.shared.showAlertMsg(generalGetString( - MR.strings.switching_profile_error_title), - String.format(generalGetString(MR.strings.switching_profile_error_message), user.chatViewName) - ) - } - } - - if (updatedConn != null) { - withContext(Dispatchers.Main) { - chatModel.chatsContext.updateContactConnection(user.remoteHostId, updatedConn) - } - } - - close() - } finally { - switchingProfile.value = false - } - } + withApi { selectProfileAsync(user) } } @Composable fun NewProfileOption() { ProfilePickerOption( title = stringResource(MR.strings.users_add), - disabled = switchingProfile.value || creatingProfile.value, + disabled = busy, selected = false, - onSelected = { createProfileForInvitation(rhId, creatingProfile) { selectProfile(it) } }, + // ModalManager.start, the pane this picker is itself shown in, like the incognito + // info modal below: the center manager renders the form in another pane on desktop, + // leaving this picker live beside it, and nulls chatId, closing any open chat. + onSelected = { createProfileForInvitation(rhId, ModalManager.start) { selectProfileAsync(it) } }, image = { Box(Modifier.size(42.dp), contentAlignment = Alignment.Center) { Icon( @@ -388,7 +400,7 @@ fun ActiveProfilePicker( ProfilePickerOption( title = user.chatViewName, - disabled = switchingProfile.value || selected, + disabled = busy || selected, selected = selected, onSelected = { selectProfile(user) }, image = { ProfileImage(size = 42.dp, image = user.image) }, @@ -399,11 +411,11 @@ fun ActiveProfilePicker( @Composable fun IncognitoUserOption() { ProfilePickerOption( - disabled = switchingProfile.value, + disabled = busy, title = stringResource(MR.strings.incognito), selected = incognito, onSelected = { - if (incognito || switchingProfile.value || contactConnection == null) return@ProfilePickerOption + if (incognito || busy || contactConnection == null) return@ProfilePickerOption switchingProfile.value = true withApi { @@ -435,7 +447,7 @@ fun ActiveProfilePicker( .fillMaxSize() .alpha(if (progressByTimeout) 0.6f else 1f) ) { - LazyColumnWithScrollBar(Modifier.padding(top = topPaddingToContent(false)), userScrollEnabled = !switchingProfile.value) { + LazyColumnWithScrollBar(Modifier.padding(top = topPaddingToContent(false)), userScrollEnabled = !busy) { item { val oneHandUI = remember { appPrefs.oneHandUI.state } if (oneHandUI.value) {