From 9df8b1845d1c487b9079f08d9e19939b493e203b Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 8 Sep 2026 21:33:45 +0000 Subject: [PATCH] Clear the save-not-confirmed dialog when leaving to save a login manually. --- .../org/signal/core/ui/compose/Dialogs.kt | 9 ++-- .../SignalLoginInfoViewModel.kt | 1 + .../registration/RegistrationEndToEndTest.kt | 33 +++++++++++++ .../SignalLoginInfoScreenTest.kt | 46 ++++++++++++++++++- .../SignalLoginInfoViewModelTest.kt | 26 +++++++++++ 5 files changed, 110 insertions(+), 5 deletions(-) diff --git a/core/ui/src/main/java/org/signal/core/ui/compose/Dialogs.kt b/core/ui/src/main/java/org/signal/core/ui/compose/Dialogs.kt index c2f27c9439..5d288e6291 100644 --- a/core/ui/src/main/java/org/signal/core/ui/compose/Dialogs.kt +++ b/core/ui/src/main/java/org/signal/core/ui/compose/Dialogs.kt @@ -77,6 +77,9 @@ object Dialogs { const val TEST_TAG_ALERT_DIALOG_CONFIRM_BUTTON = "dialog-confirm-button" const val TEST_TAG_ALERT_DIALOG_DISMISS_BUTTON = "dialog-dismiss-button" const val TEST_TAG_MESSAGE_DIALOG_DISMISS_BUTTON = "dialog-message-dismiss-button" + const val TEST_TAG_ADVANCED_ALERT_DIALOG_POSITIVE_BUTTON = "dialog-advanced-positive-button" + const val TEST_TAG_ADVANCED_ALERT_DIALOG_NEUTRAL_BUTTON = "dialog-advanced-neutral-button" + const val TEST_TAG_ADVANCED_ALERT_DIALOG_NEGATIVE_BUTTON = "dialog-advanced-negative-button" object Defaults { val shape: Shape @Composable get() = RoundedCornerShape(28.dp) @@ -802,13 +805,13 @@ object Dialogs { horizontalAlignment = Alignment.End, modifier = Modifier.fillMaxWidth() ) { - TextButton(onClick = onPositive) { + TextButton(onClick = onPositive, modifier = Modifier.testTag(TEST_TAG_ADVANCED_ALERT_DIALOG_POSITIVE_BUTTON)) { Text(text = positive) } - TextButton(onClick = onNeutral) { + TextButton(onClick = onNeutral, modifier = Modifier.testTag(TEST_TAG_ADVANCED_ALERT_DIALOG_NEUTRAL_BUTTON)) { Text(text = neutral) } - TextButton(onClick = onNegative) { + TextButton(onClick = onNegative, modifier = Modifier.testTag(TEST_TAG_ADVANCED_ALERT_DIALOG_NEGATIVE_BUTTON)) { Text(text = negative) } } diff --git a/feature/registration/src/main/java/org/signal/registration/screens/signallogininfo/SignalLoginInfoViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/signallogininfo/SignalLoginInfoViewModel.kt index e178f9b4fc..b5b8e6bbd4 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/signallogininfo/SignalLoginInfoViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/signallogininfo/SignalLoginInfoViewModel.kt @@ -104,6 +104,7 @@ class SignalLoginInfoViewModel( } is SignalLoginInfoScreenEvents.SaveManuallyClicked -> { + stateEmitter(state.copy(dialogs = SignalLoginInfoState.Dialogs())) parentEventEmitter.navigateTo(RegistrationRoute.SignalLoginViewDetailsForManualSave) } diff --git a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt index abbd103512..c3bd46119a 100644 --- a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt @@ -1643,6 +1643,39 @@ class RegistrationEndToEndTest { waitForTag(TestTags.SIGNAL_LOGIN_MANUAL_SAVE_SCREEN) } + @Test + fun `the error confirming a login does not come back when the user returns from recording the login by hand`() { + enableSignalLoginRegistration() + stubPasswordManager() + coEvery { SignalCredentialManager.getCredential(any(), any()) } returns null + + launchRegistrationFlow() + + startSignalLoginRegistration() + buySignalLogin() + + waitForTag(TestTags.SIGNAL_LOGIN_INFO_SCREEN) + composeTestRule.onNodeWithTag(TestTags.SIGNAL_LOGIN_INFO_SAVE_TO_PASSWORD_MANAGER_BUTTON).performClick() + waitForTag(TestTags.CONFIRM_LOGIN_SAVED_TO_PASSWORD_MANAGER_CONFIRM_BUTTON) + composeTestRule.onNodeWithTag(TestTags.CONFIRM_LOGIN_SAVED_TO_PASSWORD_MANAGER_CONFIRM_BUTTON).performClick() + + val context = ApplicationProvider.getApplicationContext() + val warning = context.getString(R.string.SignalLoginInfoScreen__your_signal_login_could_not_be_confirmed) + waitForText(warning) + + // Recording it by hand is taken straight from the warning rather than by dismissing it first + composeTestRule.onNodeWithTag(Dialogs.TEST_TAG_ADVANCED_ALERT_DIALOG_NEUTRAL_BUTTON).performClick() + waitForTag(TestTags.SIGNAL_LOGIN_MANUAL_SAVE_SCREEN) + + pressSystemBack() + + // Nothing failed on the way back, so the user is not warned all over again + waitForTag(TestTags.SIGNAL_LOGIN_INFO_SCREEN) + assert(composeTestRule.onAllNodesWithText(warning).fetchSemanticsNodes().isEmpty()) { + "Expected the warning to be gone after coming back from recording the login by hand" + } + } + @Test fun `typing back a login that is not the one the user was shown is rejected until the real one is entered`() { enableSignalLoginRegistration() diff --git a/feature/registration/src/test/java/org/signal/registration/screens/signallogininfo/SignalLoginInfoScreenTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/signallogininfo/SignalLoginInfoScreenTest.kt index c84a0e8156..2fc3870e2e 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/signallogininfo/SignalLoginInfoScreenTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/signallogininfo/SignalLoginInfoScreenTest.kt @@ -6,8 +6,14 @@ package org.signal.registration.screens.signallogininfo import android.app.Application +import android.content.Context +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.junit4.createComposeRule import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.test.core.app.ApplicationProvider import assertk.assertThat @@ -20,7 +26,9 @@ import org.robolectric.annotation.Config import org.signal.core.models.AccountEntropyPool import org.signal.core.models.ServiceId.ACI import org.signal.core.ui.CoreUiDependenciesRule +import org.signal.core.ui.compose.Dialogs import org.signal.core.ui.compose.theme.SignalTheme +import org.signal.registration.R import org.signal.registration.test.TestTags import java.util.UUID @@ -39,6 +47,8 @@ class SignalLoginInfoScreenTest { @get:Rule val coreUiDependenciesRule = CoreUiDependenciesRule(ApplicationProvider.getApplicationContext()) + private val context: Context = ApplicationProvider.getApplicationContext() + private val events = mutableListOf() @Test @@ -77,7 +87,38 @@ class SignalLoginInfoScreenTest { assertThat(events).contains(SignalLoginInfoScreenEvents.SeeLoginInfoAgainClicked) } - private fun setContent(showConfirmSavedSheet: Boolean = false) { + @Test + fun `the not-confirmed dialog is up for as long as the state says the save could not be confirmed`() { + var dialogs by mutableStateOf(SignalLoginInfoState.Dialogs(saveNotConfirmed = true)) + composeTestRule.setContent { + SignalTheme { + SignalLoginInfoScreen( + state = SignalLoginInfoState(aci = ACI_VALUE, aep = AEP, isPasswordManagerAvailable = true, dialogs = dialogs), + onEvent = { events += it } + ) + } + } + + composeTestRule.onNodeWithText(context.getString(R.string.SignalLoginInfoScreen__error_confirming_login_info)).assertIsDisplayed() + + dialogs = SignalLoginInfoState.Dialogs() + + composeTestRule.onNodeWithText(context.getString(R.string.SignalLoginInfoScreen__error_confirming_login_info)).assertDoesNotExist() + } + + @Test + fun `when save manually on the not-confirmed dialog is clicked, SaveManuallyClicked is emitted`() { + setContent(dialogs = SignalLoginInfoState.Dialogs(saveNotConfirmed = true)) + + composeTestRule.onNodeWithTag(Dialogs.TEST_TAG_ADVANCED_ALERT_DIALOG_NEUTRAL_BUTTON).performClick() + + assertThat(events).contains(SignalLoginInfoScreenEvents.SaveManuallyClicked) + } + + private fun setContent( + showConfirmSavedSheet: Boolean = false, + dialogs: SignalLoginInfoState.Dialogs = SignalLoginInfoState.Dialogs() + ) { composeTestRule.setContent { SignalTheme { SignalLoginInfoScreen( @@ -85,7 +126,8 @@ class SignalLoginInfoScreenTest { aci = ACI_VALUE, aep = AEP, isPasswordManagerAvailable = true, - showConfirmSavedSheet = showConfirmSavedSheet + showConfirmSavedSheet = showConfirmSavedSheet, + dialogs = dialogs ), onEvent = { events += it } ) diff --git a/feature/registration/src/test/java/org/signal/registration/screens/signallogininfo/SignalLoginInfoViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/signallogininfo/SignalLoginInfoViewModelTest.kt index 673476dee5..cea783e1e0 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/signallogininfo/SignalLoginInfoViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/signallogininfo/SignalLoginInfoViewModelTest.kt @@ -271,6 +271,32 @@ class SignalLoginInfoViewModelTest { assertThat(parentEvents).containsExactly(RegistrationFlowEvent.NavigateToScreen(RegistrationRoute.SignalLoginViewDetailsForManualSave)) } + @Test + fun `SaveManuallyClicked from the not-confirmed dialog clears it on the way out`() = runTest(testDispatcher) { + val parentEvents = mutableListOf() + var emittedState: SignalLoginInfoState? = null + + viewModel.applyEvent( + SignalLoginInfoState(dialogs = SignalLoginInfoState.Dialogs(saveNotConfirmed = true)), + SignalLoginInfoScreenEvents.SaveManuallyClicked, + { parentEvents.add(it) } + ) { emittedState = it } + + assertThat(emittedState?.dialogs).isEqualTo(SignalLoginInfoState.Dialogs()) + assertThat(parentEvents).containsExactly(RegistrationFlowEvent.NavigateToScreen(RegistrationRoute.SignalLoginViewDetailsForManualSave)) + } + + @Test + fun `SaveManuallyClicked leaves nothing behind in the view model that outlives the navigation`() = runTest(testDispatcher) { + viewModel.onEvent(SignalLoginInfoScreenEvents.SavedCredentialRetrieved(null)) + + assertThat(viewModel.state.value.dialogs.saveNotConfirmed).isEqualTo(true) + + viewModel.onEvent(SignalLoginInfoScreenEvents.SaveManuallyClicked) + + assertThat(viewModel.state.value.dialogs.saveNotConfirmed).isEqualTo(false) + } + @Test fun `SaveNotConfirmedDialogDismissed clears the not-confirmed dialog`() = runTest(testDispatcher) { var emittedState: SignalLoginInfoState? = null