diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModel.kt index c5b1f7b658..868ab6e1df 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModel.kt @@ -106,8 +106,8 @@ class AccountSettingsViewModel( AccountSettingsEvent.AuthenticationFailed -> { _actions.send(AccountSettingsAction.ShowAuthenticationFailed) } - AccountSettingsEvent.RemoveTotpAppConfirmed -> { - applyRemoveTotpAppConfirmed() + is AccountSettingsEvent.RemoveTotpAppConfirmed -> { + applyRemoveTotpAppConfirmed(event.appId) } AccountSettingsEvent.AdvancedPinSettingsClicked -> { _actions.send(AccountSettingsAction.NavigateToAdvancedPinSettings) @@ -214,11 +214,9 @@ class AccountSettingsViewModel( } } - private suspend fun applyRemoveTotpAppConfirmed() { - val dialog = _state.value.dialog as? Dialog.ConfirmRemoveTotpApp ?: return - + private suspend fun applyRemoveTotpAppConfirmed(appId: Long) { _state.update { it.copy(dialog = Dialog.None) } - removeTotpApp(dialog.appId) + removeTotpApp(appId) } private suspend fun removeTotpApp(appId: Long) { diff --git a/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModelTest.kt b/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModelTest.kt index ffa690af6e..05c119d490 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModelTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModelTest.kt @@ -480,25 +480,29 @@ class AccountSettingsViewModelTest { val actions = collectActions(viewModel.actions) viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(TOTP_APP)) - viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed) + viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed(TOTP_APP.id)) coVerify { repository.removeTotpApp(TOTP_APP.id) } assertThat(viewModel.state.value.dialog).isEqualTo(Dialog.None) assertThat(actions.last()).isEqualTo(AccountSettingsAction.ShowTotpAppRemoved) } - /** The open dialog is what says which app is being removed, so a confirmation without one has no app to act on. */ + /** The dialog dismisses itself before it confirms, so the removal has to survive the dismissal that lands first. */ @Test - fun `RemoveTotpAppConfirmed without the confirmation dialog removes nothing`() = runTest(testDispatcher) { + fun `RemoveTotpAppConfirmed removes the app even though the dialog dismissed itself first`() = runTest(testDispatcher) { every { repository.isPhoneNumberless() } returns true coEvery { repository.getTwoFactorMethods() } returns methods(TOTP_APP) val viewModel = createViewModel() + val actions = collectActions(viewModel.actions) - viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed) + viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(TOTP_APP)) + viewModel.onEvent(AccountSettingsEvent.DialogDismissed) + viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed(TOTP_APP.id)) - coVerify(exactly = 0) { repository.removeTotpApp(any()) } - assertThat(viewModel.state.value.signalLogin!!.twoFactorMethods).containsExactly(TOTP_APP) + coVerify { repository.removeTotpApp(TOTP_APP.id) } + assertThat(viewModel.state.value.dialog).isEqualTo(Dialog.None) + assertThat(actions.last()).isEqualTo(AccountSettingsAction.ShowTotpAppRemoved) } /** The list is what tells the user the app is gone, so it has to be read again rather than assumed. */ @@ -512,7 +516,7 @@ class AccountSettingsViewModelTest { viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(TOTP_APP)) coEvery { repository.getTwoFactorMethods() } returns methods() - viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed) + viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed(TOTP_APP.id)) assertThat(viewModel.state.value.signalLogin!!.twoFactorMethods).isEmpty() } @@ -527,7 +531,7 @@ class AccountSettingsViewModelTest { val actions = collectActions(viewModel.actions) viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(TOTP_APP)) - viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed) + viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed(TOTP_APP.id)) assertThat(actions.last()).isEqualTo(AccountSettingsAction.ShowTotpAppRemovalFailed) assertThat(viewModel.state.value.signalLogin!!.twoFactorMethods).containsExactly(TOTP_APP) diff --git a/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsEvent.kt b/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsEvent.kt index b88e9657dc..8b8d96d8a6 100644 --- a/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsEvent.kt +++ b/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsEvent.kt @@ -66,8 +66,8 @@ sealed interface AccountSettingsEvent { /** The screen lock turned the user away, so whatever asked for it goes no further. */ data object AuthenticationFailed : AccountSettingsEvent - /** The user confirmed removing the authenticator app named by the open dialog, which removes it. */ - data object RemoveTotpAppConfirmed : AccountSettingsEvent + /** The user confirmed removing the authenticator app with [appId], which removes it. */ + data class RemoveTotpAppConfirmed(val appId: Long) : AccountSettingsEvent /** The user tapped the advanced PIN settings row. */ data object AdvancedPinSettingsClicked : AccountSettingsEvent diff --git a/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsScreen.kt b/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsScreen.kt index 8af3e0dcba..65b9d7f55f 100644 --- a/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsScreen.kt +++ b/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsScreen.kt @@ -375,7 +375,7 @@ fun AccountSettingsScreen( RegistrationLockConfirmationDialog(dialog, onEvent) } } - is Dialog.ConfirmRemoveTotpApp -> ConfirmRemoveTotpAppDialog(onEvent = onEvent) + is Dialog.ConfirmRemoveTotpApp -> ConfirmRemoveTotpAppDialog(appId = dialog.appId, onEvent = onEvent) Dialog.MaxTotpAppsReached -> MaxTotpAppsReachedDialog(maxApps = state.signalLogin?.maxTotpApps ?: 0, onEvent = onEvent) } } @@ -617,13 +617,14 @@ private fun DeleteAllDataConfirmationDialog( @Composable private fun ConfirmRemoveTotpAppDialog( + appId: Long, onEvent: (AccountSettingsEvent) -> Unit ) { Dialogs.SimpleAlertDialog( title = stringResource(R.string.AccountSettingsFragment__remove_authenticator_app), body = stringResource(R.string.AccountSettingsFragment__you_wont_be_able_to_use_this_app), confirm = stringResource(R.string.AccountSettingsFragment__remove), - onConfirm = { onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed) }, + onConfirm = { onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed(appId)) }, onDismiss = { onEvent(AccountSettingsEvent.DialogDismissed) }, dismiss = stringResource(android.R.string.cancel), onDismissRequest = { onEvent(AccountSettingsEvent.DialogDismissed) }, @@ -885,7 +886,7 @@ private fun ConfirmPinToDisableRemindersDialogPreview() { @Composable private fun ConfirmRemoveTotpAppDialogPreview() { Previews.Preview { - ConfirmRemoveTotpAppDialog(onEvent = {}) + ConfirmRemoveTotpAppDialog(appId = 1, onEvent = {}) } } diff --git a/feature/app-settings/src/test/java/org/signal/appsettings/account/AccountSettingsScreenTest.kt b/feature/app-settings/src/test/java/org/signal/appsettings/account/AccountSettingsScreenTest.kt index 2eb44bdd82..56c9f20c2e 100644 --- a/feature/app-settings/src/test/java/org/signal/appsettings/account/AccountSettingsScreenTest.kt +++ b/feature/app-settings/src/test/java/org/signal/appsettings/account/AccountSettingsScreenTest.kt @@ -416,7 +416,7 @@ class AccountSettingsScreenTest { composeTestRule.onNodeWithTag(AccountSettingsTestTags.DIALOG_CONFIRM_REMOVE_TOTP_APP).assertIsDisplayed() composeTestRule.onNodeWithTag(Dialogs.TEST_TAG_ALERT_DIALOG_CONFIRM_BUTTON).performClick() - assertThat(events).contains(AccountSettingsEvent.RemoveTotpAppConfirmed) + assertThat(events).contains(AccountSettingsEvent.RemoveTotpAppConfirmed(METHODS[0].id)) } @Test