diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/SelectAdoptingGroupScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/SelectAdoptingGroupScreen.kt index 16313ef2..76fe9e43 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/SelectAdoptingGroupScreen.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/SelectAdoptingGroupScreen.kt @@ -26,7 +26,6 @@ import androidx.compose.ui.text.font.FontFamily import androidx.compose.ui.text.style.TextOverflow import androidx.lifecycle.viewmodel.compose.viewModel import com.vitorpamplona.quartz.nip01Core.core.HexKey -import kotlinx.coroutines.launch import mantra.composeapp.generated.resources.Res import mantra.composeapp.generated.resources.asked_that_group_to_adopt_this_one import mantra.composeapp.generated.resources.no_group_here_can_adopt_this_one @@ -40,6 +39,7 @@ import press.mantra.compose.ui.composable.widgets.ErrorState import press.mantra.compose.ui.composable.widgets.LoadingDataIndicator import press.mantra.compose.ui.composable.widgets.LocalSnackbarHostState import press.mantra.compose.ui.composable.widgets.NavigateBackButton +import press.mantra.compose.ui.composable.widgets.rememberNotifier import press.mantra.compose.ui.composable.widgets.ScreenStateTransition import press.mantra.compose.ui.theme.readableContent import press.mantra.compose.ui.theme.spacing @@ -52,8 +52,15 @@ import press.mantra.compose.ui.view.state.SelectAdoptingGroupUIState * Tapping a row asks that group's quorum to certify this room as its child. That * is an **offer** and nothing more: this room becomes their subgroup only once * its own quorum re-announces its key state naming them, which is a card back on - * the room's own screen. So this hands back rather than going anywhere new, and - * the snackbar says what was asked rather than what was done. + * the room's own screen. So the snackbar says what was *asked* rather than what + * was done. + * + * **It does not navigate away on success**, and that is the fix for two things at + * once. A message launched here while the screen was being popped was racing its + * own scope's cancellation; and the list is the better confirmation anyway -- the + * row that was just asked leaves it, because a certificate now stands for the + * pair and `refuseAdoption` says so. The way out is the back button, which a + * pushed screen has to have regardless. * * The list is the profile's own rooms -- see `SelectAdoptingGroupViewModel` for * why it can be nothing else -- so empty is an ordinary answer and the empty @@ -80,12 +87,10 @@ fun SelectAdoptingGroupScreen( LaunchedEffect(chatRoomId) { viewModel.initiate() } val snackbarHostState = LocalSnackbarHostState.current - val scope = rememberCoroutineScope() - // Read outside the handler: both of these are composable and an onClick - // lambda is not. The notifier takes the caller's scope because the message is - // shown as this screen navigates away, and one launched in the departing - // composable's scope would be cancelled with it. + // Both read outside the handler, because both are composable and an onClick + // lambda is not. + val notify = rememberNotifier(rememberCoroutineScope()) val asked = stringResource(Res.string.asked_that_group_to_adopt_this_one) ScreenStateTransition(viewModel.uiState) { uiState -> @@ -145,13 +150,11 @@ fun SelectAdoptingGroupScreen( Card( modifier = Modifier.fillMaxWidth(), onClick = { - scope.launch { - viewModel.offerTo(candidate.chatRoom.id) { refusal -> - scope.launch { - snackbarHostState.showSnackbar(refusal ?: asked) - } - if (refusal == null) onNavigateBack() - } + // `offerTo` is not suspending and calls back + // on the main thread, so there is nothing to + // launch around it. + viewModel.offerTo(candidate.chatRoom.id) { message -> + notify(message ?: asked) } } ) { diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomDetailViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomDetailViewModel.kt index 16fc5589..6e2d1f6f 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomDetailViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomDetailViewModel.kt @@ -22,6 +22,7 @@ import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.IO import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import press.mantra.compose.database.model.types.MarmotReindexReport class ChatRoomDetailViewModel( @@ -119,7 +120,13 @@ class ChatRoomDetailViewModel( if (!accepted) { logger.e("Failed to ask $chatRoomId to accept $parentChatRoomId") - onDone("Couldn't ask this group to accept that parent. Please try again.") + // On the main thread, because a caller's callback may touch + // things an IO coroutine must not -- navigation most of all. See + // `SelectAdoptingGroupViewModel.finish`, where calling back from + // IO crashed outright. + withContext(Dispatchers.Main) { + onDone("Couldn't ask this group to accept that parent. Please try again.") + } return@launch } @@ -127,7 +134,7 @@ class ChatRoomDetailViewModel( // which is a quorum away -- so this reloads rather than crossing it // off, and the card stays until the group has actually agreed. initiateChatRoomDetail() - onDone(null) + withContext(Dispatchers.Main) { onDone(null) } } } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SelectAdoptingGroupViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SelectAdoptingGroupViewModel.kt index a46ef417..d63d1fff 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SelectAdoptingGroupViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SelectAdoptingGroupViewModel.kt @@ -14,6 +14,7 @@ import com.vitorpamplona.quartz.nip01Core.core.HexKey import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.IO import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import press.mantra.compose.repository.ChatRepository import press.mantra.compose.ui.view.state.SelectAdoptingGroupUIState @@ -94,7 +95,7 @@ class SelectAdoptingGroupViewModel( if (refusal != null) { isActionPending.value = false - onDone(refusal) + finish(refusal, onDone) return@launch } @@ -108,14 +109,34 @@ class SelectAdoptingGroupViewModel( if (!proposed) { logger.e("Failed to ask $adoptingChatRoomId to adopt $chatRoomId") - onDone("Couldn't ask that group to adopt this one. Please try again.") + finish("Couldn't ask that group to adopt this one. Please try again.", onDone) return@launch } - onDone(null) + // Reload before handing back, so the row that was just asked leaves + // the list: a certificate now stands for the pair and + // `refuseAdoption` says so. That is the confirmation, and it is a + // better one than a message -- the list is the state. + initiate() + finish(null, onDone) } } + /** + * Hands [message] back **on the main thread**. + * + * The work runs on `Dispatchers.IO` and a caller's callback routinely touches + * things that may not be touched from there -- the first version of this + * screen called `popBackStack()` straight out of the IO coroutine and crashed + * on `Method setCurrentState must be called on the main thread`. The + * dispatcher is this function's business rather than every caller's, which is + * the shape `AddArtifactViewModel` and `SelectSubgroupAdminsViewModel` already + * use for the same handoff. + */ + private suspend fun finish(message: String?, onDone: (String?) -> Unit) { + withContext(Dispatchers.Main) { onDone(message) } + } + companion object { private const val TAG = "SelectAdoptingGroupViewModel" diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/CeremonyQueueRoutingJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/CeremonyQueueRoutingJvmTest.kt index f6c876cb..faaec773 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/CeremonyQueueRoutingJvmTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/CeremonyQueueRoutingJvmTest.kt @@ -13,6 +13,7 @@ import kotlinx.coroutines.withTimeout import kotlinx.coroutines.delay import press.mantra.compose.database.model.ChatRoom import press.mantra.compose.database.model.DkgSession +import press.mantra.compose.database.model.NegentropySynchronizeRequest import press.mantra.compose.database.model.Participant import press.mantra.compose.database.model.intermdiate.LocalChatRoom import press.mantra.compose.database.model.intermdiate.LocalParticipant @@ -151,7 +152,23 @@ class CeremonyQueueRoutingJvmTest { val viewModel = ChatMessageListViewModel( initialChatMessageListUIState = ChatMessageListUIState.Loading, localChatRoom = room(), - nostrRepository = NostrRepository.NO_OP_NOSTR_REPOSITORY, + // Not the bare no-op. `initiate` calls `scheduleSynchronization` on + // its second line, which launches on `Dispatchers.IO` and -- because + // `room()` gives every participant a null profile, so there is always + // somebody it cannot name -- reaches + // `queueNegentropySynchronizeRequest`. That member is one of the + // forty-one `TODO("Not yet implemented")` bodies on + // NO_OP_NOSTR_REPOSITORY, so it throws in a coroutine nobody awaits. + // + // Nothing catches it, so it reaches the default handler, and the next + // `runTest` in the same JVM reports it as `UncaughtExceptionsBeforeTest` + // -- against its own name. That is how this test failed + // `NavigationRoutingTest` four times a run while passing itself. + nostrRepository = object : NostrRepository by NostrRepository.NO_OP_NOSTR_REPOSITORY { + override suspend fun queueNegentropySynchronizeRequest( + negentropySynchronizeRequests: List + ) = Unit + }, chatRepository = ChatRepository.NO_OP_CHAT_REPOSITORY, frostSigningRepository = FrostSigningRepository.NO_OP_FROST_SIGNING_REPOSITORY, dkgRepository = object : DkgRepository by DkgRepository.NO_OP_DKG_REPOSITORY {