diff --git a/feature/registration/src/main/java/org/signal/registration/RegistrationFlowState.kt b/feature/registration/src/main/java/org/signal/registration/RegistrationFlowState.kt index 0cb9fec9d7..b34c7c8df5 100644 --- a/feature/registration/src/main/java/org/signal/registration/RegistrationFlowState.kt +++ b/feature/registration/src/main/java/org/signal/registration/RegistrationFlowState.kt @@ -79,6 +79,14 @@ data class RegistrationFlowState( /** If true, the ViewModel is still deciding whether to restore a previous flow or start fresh. */ val isRestoringNavigationState: Boolean = true ) : Parcelable { + + /** + * Whether the user went through the archive restore selection process. This is separate from [pendingRestoreOption] because it's true even + * if they chose to skip. Basically just helps us make some strings nicer if we knew the path they took to get here. + */ + val sawArchiveRestoreSelectionScreen: Boolean + get() = backStack.any { it is RegistrationRoute.ArchiveRestoreSelection } + override fun toString(): String { return "RegistrationFlowState(backStack=${backStack.joinToString()}, sessionMetadata=$sessionMetadata, sessionE164=$sessionE164, submittedVerificationCode=${submittedVerificationCode?.censor()}, accountEntropyPool=${accountEntropyPool?.displayValue?.censor()}, aci=${aci?.logString()}, storageCapable=$storageCapable, isPhoneNumberlessAccount=$isPhoneNumberlessAccount, temporaryMasterKey=${temporaryMasterKey?.toString()?.censor()}, preExistingRegistrationData=$preExistingRegistrationData, doNotAttemptRecoveryPassword=$doNotAttemptRecoveryPassword, pendingRestoreOption=$pendingRestoreOption, unverifiedRestoredAep=${unverifiedRestoredAep?.displayValue?.censor()}, restoreMethodToken=${restoreMethodToken?.censor()}, lastSmsVerificationCodeRequest=$lastSmsVerificationCodeRequest, lastCallVerificationCodeRequest=$lastCallVerificationCodeRequest, isRestoringNavigation=$isRestoringNavigationState)" } diff --git a/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryScreen.kt b/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryScreen.kt index 01fda19de2..e458643b46 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryScreen.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryScreen.kt @@ -84,7 +84,6 @@ import org.signal.core.ui.compose.SignalIcons import org.signal.core.ui.compose.TextFields import org.signal.core.util.Util import org.signal.core.util.logging.Log -import org.signal.registration.PendingRestoreOption import org.signal.registration.R import org.signal.registration.RegistrationDependencies import org.signal.registration.screens.OnePaneRegistrationScaffold @@ -419,7 +418,7 @@ private fun NextButton( ) { Text( stringResource( - if (state.hasExistingAccount) { + if (state.sawArchiveRestoreSelectionScreen) { R.string.RegistrationActivity_use_account_id } else { R.string.RegistrationActivity_register_without_number @@ -667,7 +666,7 @@ private fun PhoneNumberScreenUseAccountIdPreview() { PhoneNumberScreen( state = PhoneNumberEntryState( isPhoneNumberlessRegistrationAvailable = true, - pendingRestoreOption = PendingRestoreOption.RemoteBackup + sawArchiveRestoreSelectionScreen = true ), onEvent = {} ) diff --git a/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryState.kt b/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryState.kt index 7d99495c09..d40c951ddb 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryState.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryState.kt @@ -34,6 +34,8 @@ data class PhoneNumberEntryState( val preExistingRegistrationData: PreExistingRegistrationData? = null, val restoredSvrCredentials: List = emptyList(), val pendingRestoreOption: PendingRestoreOption? = null, + /** Whether the user saw the archive restore selection screen. */ + val sawArchiveRestoreSelectionScreen: Boolean = false, val initialized: Boolean = false, /** Whether the entered number has a plausible length for the selected country code. */ val isNumberPossible: Boolean = false, @@ -52,10 +54,6 @@ data class PhoneNumberEntryState( null } - /** Whether the user already told us they have an account to restore, which lets us skip past the Signal Login purchase screen. */ - val hasExistingAccount: Boolean - get() = pendingRestoreOption != null - /** Whether what has been entered is complete enough to submit, be it a phone number or an account ID. */ val isNextEnabled: Boolean get() { @@ -68,7 +66,7 @@ data class PhoneNumberEntryState( } } - override fun toString(): String = "PhoneNumberEntryState(regionCode=$regionCode, countryCode=$countryCode, countryName=$countryName, countryEmoji=$countryEmoji, nationalNumber=${nationalNumber.censor()}, formattedNumber=${formattedNumber.censor()}, accountId=${accountId?.censor()}, accountIdError=$accountIdError, sessionE164=$sessionE164, sessionMetadata=$sessionMetadata, smsVerificationCodeRequest=$smsVerificationCodeRequest, showSpinner=$showSpinner, dialogs=$dialogs, preExistingRegistrationData=${preExistingRegistrationData?.let { "present" }}, restoredSvrCredentials=${restoredSvrCredentials.size} items, pendingRestoreOption=$pendingRestoreOption, initialized=$initialized, isNumberPossible=$isNumberPossible, isNumberInvalid=$isNumberInvalid, isLinkAndSyncAvailable=$isLinkAndSyncAvailable, isPhoneNumberlessRegistrationAvailable=$isPhoneNumberlessRegistrationAvailable)" + override fun toString(): String = "PhoneNumberEntryState(regionCode=$regionCode, countryCode=$countryCode, countryName=$countryName, countryEmoji=$countryEmoji, nationalNumber=${nationalNumber.censor()}, formattedNumber=${formattedNumber.censor()}, accountId=${accountId?.censor()}, accountIdError=$accountIdError, sessionE164=$sessionE164, sessionMetadata=$sessionMetadata, smsVerificationCodeRequest=$smsVerificationCodeRequest, showSpinner=$showSpinner, dialogs=$dialogs, preExistingRegistrationData=${preExistingRegistrationData?.let { "present" }}, restoredSvrCredentials=${restoredSvrCredentials.size} items, pendingRestoreOption=$pendingRestoreOption, sawArchiveRestoreSelectionScreen=$sawArchiveRestoreSelectionScreen, initialized=$initialized, isNumberPossible=$isNumberPossible, isNumberInvalid=$isNumberInvalid, isLinkAndSyncAvailable=$isLinkAndSyncAvailable, isPhoneNumberlessRegistrationAvailable=$isPhoneNumberlessRegistrationAvailable)" data class Dialogs( /** Asks the user to confirm the number they entered before submitting it. */ diff --git a/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModel.kt index 9cd2637fc1..aa814d5b11 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModel.kt @@ -151,7 +151,7 @@ class PhoneNumberEntryViewModel( parentEventEmitter.navigateTo(RegistrationRoute.LinkAccount()) } is PhoneNumberEntryScreenEvents.RegisterWithoutNumber -> { - if (state.hasExistingAccount) { + if (state.sawArchiveRestoreSelectionScreen) { parentEventEmitter.navigateTo(RegistrationRoute.SignalLoginCredentialEntry()) } else { parentEventEmitter.navigateTo(RegistrationRoute.SignalLoginPayment) @@ -220,7 +220,8 @@ class PhoneNumberEntryViewModel( smsVerificationCodeRequest = parentState.lastSmsVerificationCodeRequest, preExistingRegistrationData = parentState.preExistingRegistrationData, restoredSvrCredentials = state.restoredSvrCredentials.takeUnless { parentState.doNotAttemptRecoveryPassword } ?: emptyList(), - pendingRestoreOption = parentState.pendingRestoreOption + pendingRestoreOption = parentState.pendingRestoreOption, + sawArchiveRestoreSelectionScreen = parentState.sawArchiveRestoreSelectionScreen ) } 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 0bb1a667f6..23a4cd88cd 100644 --- a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt @@ -1872,6 +1872,40 @@ class RegistrationEndToEndTest { assert(networkController.lastRegisterAccountRequest?.aci == login.aci) { "Expected the entered login to be reclaimed but was ${networkController.lastRegisterAccountRequest}" } } + @Test + fun `skipping the restore still turns the numberless button into a direct jump to signal login entry`() { + enableSignalLoginRegistration() + val login = signalLoginFor(reregistration = true) + + var registrationComplete = false + launchRegistrationFlow(onRegistrationComplete = { registrationComplete = true }) + + // Declining the restore doesn't change the fact that the user told us they have an account to restore + startManualRestore() + chooseRestoreOption(TestTags.ARCHIVE_RESTORE_SELECTION_NONE) + waitForTag(Dialogs.TEST_TAG_ALERT_DIALOG_CONFIRM_BUTTON) + composeTestRule.onNodeWithTag(Dialogs.TEST_TAG_ALERT_DIALOG_CONFIRM_BUTTON).performClick() + + waitForTag(TestTags.PHONE_NUMBER_SCREEN) + composeTestRule.onNodeWithTag(TestTags.PHONE_NUMBER_REGISTER_WITHOUT_NUMBER_BUTTON).performClick() + waitForTag(TestTags.SIGNAL_LOGIN_CREDENTIAL_ENTRY_SCREEN) + assert(composeTestRule.onAllNodesWithTag(TestTags.SIGNAL_LOGIN_PAYMENT_SCREEN).fetchSemanticsNodes().isEmpty()) { + "Expected the Signal Login payment screen to be skipped for a user who came through the restore flow" + } + + enterSignalLogin(login) + + chooseRestoreOption(TestTags.ARCHIVE_RESTORE_SELECTION_NONE) + waitForTag(Dialogs.TEST_TAG_ALERT_DIALOG_CONFIRM_BUTTON) + composeTestRule.onNodeWithTag(Dialogs.TEST_TAG_ALERT_DIALOG_CONFIRM_BUTTON).performClick() + + waitFor("registration to complete") { registrationComplete } + + assert(purchaseApi.launchCount == 0) { "A user with an existing login should never hit Google Play" } + assert(networkController.lastCreateSessionE164 == null) { "Expected no verification session for a numberless login" } + assert(networkController.lastRegisterAccountRequest?.aci == login.aci) { "Expected the entered login to be reclaimed but was ${networkController.lastRegisterAccountRequest}" } + } + @Test fun `typing an account id into the phone number field goes straight to signal login entry with it filled in`() { enableSignalLoginRegistration() diff --git a/feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModelTest.kt index d24301088d..0d580edf12 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModelTest.kt @@ -46,7 +46,6 @@ import org.signal.network.api.RegistrationApiV2.SvrCredentials import org.signal.network.api.RegistrationApiV2.ThirdPartyServiceErrorResponse import org.signal.network.api.RegistrationApiV2.UpdateSessionError import org.signal.registration.KeyMaterial -import org.signal.registration.PendingRestoreOption import org.signal.registration.PreExistingRegistrationData import org.signal.registration.RegisteredAccountData import org.signal.registration.RegistrationFlowEvent @@ -467,9 +466,9 @@ class PhoneNumberEntryViewModelTest { } @Test - fun `RegisterWithoutNumber skips payment and goes straight to credential entry when a restore is pending`() = runTest { + fun `RegisterWithoutNumber skips payment and goes straight to credential entry when the user has an existing account`() = runTest { viewModel.applyEvent( - PhoneNumberEntryState(pendingRestoreOption = PendingRestoreOption.RemoteBackup), + PhoneNumberEntryState(sawArchiveRestoreSelectionScreen = true), PhoneNumberEntryScreenEvents.RegisterWithoutNumber, parentEventEmitter, stateEmitter diff --git a/feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberScreenTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberScreenTest.kt index 6f27691d65..3239677648 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberScreenTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberScreenTest.kt @@ -25,7 +25,6 @@ import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config import org.signal.core.ui.CoreUiDependenciesRule import org.signal.core.ui.compose.theme.SignalTheme -import org.signal.registration.PendingRestoreOption import org.signal.registration.R import org.signal.registration.screens.shared.AccountIdError import org.signal.registration.test.TestTags @@ -328,7 +327,7 @@ class PhoneNumberScreenTest { } @Test - fun `the numberless button offers to register without a number when there is no pending restore`() { + fun `the numberless button offers to register without a number when the user has no existing account`() { // Given composeTestRule.setContent { SignalTheme { @@ -344,14 +343,14 @@ class PhoneNumberScreenTest { } @Test - fun `the numberless button offers to use an account ID when a restore is pending`() { + fun `the numberless button offers to use an account ID when the user has an existing account`() { // Given composeTestRule.setContent { SignalTheme { PhoneNumberScreen( state = PhoneNumberEntryState( isPhoneNumberlessRegistrationAvailable = true, - pendingRestoreOption = PendingRestoreOption.RemoteBackup + sawArchiveRestoreSelectionScreen = true ), onEvent = {} )