diff --git a/feature/registration/build.gradle.kts b/feature/registration/build.gradle.kts index e33afea987..7223d6211b 100644 --- a/feature/registration/build.gradle.kts +++ b/feature/registration/build.gradle.kts @@ -79,6 +79,9 @@ dependencies { // Phone number formatting implementation(libs.google.libphonenumber) + // Phone number hint + implementation(libs.google.play.services.auth) + // Testing testImplementation(testFixtures(project(":core:ui"))) testImplementation(testLibs.junit.junit) 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 7290a9bbf1..635b973b88 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 @@ -5,6 +5,13 @@ package org.signal.registration.screens.phonenumber +import android.Manifest +import android.annotation.SuppressLint +import android.content.Context +import android.content.pm.PackageManager +import androidx.activity.compose.rememberLauncherForActivityResult +import androidx.activity.result.IntentSenderRequest +import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement @@ -36,6 +43,8 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -44,6 +53,7 @@ import androidx.compose.ui.focus.FocusRequester import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.input.nestedscroll.nestedScroll +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalResources import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource @@ -54,6 +64,10 @@ import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.text.input.TextFieldValue import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp +import androidx.core.content.ContextCompat +import com.google.android.gms.auth.api.identity.GetPhoneNumberHintIntentRequest +import com.google.android.gms.auth.api.identity.Identity +import com.google.i18n.phonenumbers.PhoneNumberUtil import org.signal.core.ui.compose.AllDevicePreviews import org.signal.core.ui.compose.Buttons import org.signal.core.ui.compose.Dialogs @@ -61,6 +75,8 @@ import org.signal.core.ui.compose.DropdownMenus import org.signal.core.ui.compose.IconButtons.IconButton import org.signal.core.ui.compose.Previews import org.signal.core.ui.compose.Scaffolds +import org.signal.core.util.Util +import org.signal.core.util.logging.Log import org.signal.registration.R import org.signal.registration.screens.OnePaneRegistrationScaffold import org.signal.registration.screens.RegistrationScaffold @@ -70,6 +86,26 @@ import org.signal.registration.screens.phonenumber.PhoneNumberEntryState.OneTime import org.signal.registration.test.TestTags import org.signal.core.ui.R as CoreR +private const val TAG = "PhoneNumberScreen" + +/** + * Reads the device's own phone number from the SIM as an E164 string, but only if the relevant phone permission has + * already been granted. Returns null if the permission is missing or the number is unavailable. We never prompt for + * the permission solely to prefill the number. + */ +@SuppressLint("MissingPermission") +private fun readDeviceNumberE164(context: Context): String? { + val hasPhonePermission = ContextCompat.checkSelfPermission(context, Manifest.permission.READ_PHONE_STATE) == PackageManager.PERMISSION_GRANTED || + ContextCompat.checkSelfPermission(context, Manifest.permission.READ_PHONE_NUMBERS) == PackageManager.PERMISSION_GRANTED + + if (!hasPhonePermission) { + return null + } + + val deviceNumber = Util.getDeviceNumber(context).orElse(null) ?: return null + return PhoneNumberUtil.getInstance().format(deviceNumber, PhoneNumberUtil.PhoneNumberFormat.E164) +} + /** * Phone number entry screen */ @@ -80,7 +116,58 @@ fun PhoneNumberScreen( modifier: Modifier = Modifier ) { val resources = LocalResources.current + val context = LocalContext.current var simpleErrorMessage: String? by remember { mutableStateOf(null) } + var hasRequestedPhoneNumberHint by rememberSaveable { mutableStateOf(false) } + val currentNationalNumber by rememberUpdatedState(state.nationalNumber) + + val prefillFromDeviceNumberIfAllowed = { + if (currentNationalNumber.isEmpty()) { + readDeviceNumberE164(context)?.let { e164 -> + onEvent(PhoneNumberEntryScreenEvents.FullPhoneNumberEntered(e164)) + } + } + } + + val phoneNumberHintLauncher = rememberLauncherForActivityResult(ActivityResultContracts.StartIntentSenderForResult()) { result -> + val phoneNumber = try { + Identity.getSignInClient(context).getPhoneNumberFromIntent(result.data) + } catch (e: Exception) { + Log.w(TAG, "Failed to retrieve phone number from hint.", e) + null + } + + if (phoneNumber != null) { + onEvent(PhoneNumberEntryScreenEvents.FullPhoneNumberEntered(phoneNumber, autoConfirm = true)) + } + } + + LaunchedEffect(state.initialized) { + if (!state.initialized || hasRequestedPhoneNumberHint || state.nationalNumber.isNotEmpty() || state.preExistingRegistrationData != null) { + return@LaunchedEffect + } + hasRequestedPhoneNumberHint = true + + try { + Identity.getSignInClient(context) + .getPhoneNumberHintIntent(GetPhoneNumberHintIntentRequest.builder().build()) + .addOnSuccessListener { pendingIntent -> + try { + phoneNumberHintLauncher.launch(IntentSenderRequest.Builder(pendingIntent).build()) + } catch (e: Exception) { + Log.w(TAG, "Failed to launch phone number hint intent.", e) + prefillFromDeviceNumberIfAllowed() + } + } + .addOnFailureListener { e -> + Log.w(TAG, "Phone number hint unavailable. Falling back to device number.", e) + prefillFromDeviceNumberIfAllowed() + } + } catch (e: Exception) { + Log.w(TAG, "Unable to request phone number hint. Falling back to device number.", e) + prefillFromDeviceNumberIfAllowed() + } + } if (state.showDialog) { Dialogs.SimpleAlertDialog( @@ -88,7 +175,7 @@ fun PhoneNumberScreen( body = "+${state.countryCode} ${state.formattedNumber}\n\n${stringResource(R.string.RegistrationActivity_a_verification_code)}", confirm = stringResource(id = android.R.string.ok), dismiss = stringResource(R.string.RegistrationActivity_edit_number), - onConfirm = { onEvent(PhoneNumberEntryScreenEvents.PhoneNumberSubmitted) }, + onConfirm = { onEvent(PhoneNumberEntryScreenEvents.PhoneNumberConfirmed) }, onDismiss = { onEvent(PhoneNumberEntryScreenEvents.PhoneNumberCancelled) } ) } @@ -169,8 +256,8 @@ private fun OnePaneLayout( countryCode = state.countryCode, formattedNumber = state.formattedNumber, onCountryCodeChanged = { onEvent(PhoneNumberEntryScreenEvents.CountryCodeChanged(it)) }, - onPhoneNumberChanged = { onEvent(PhoneNumberEntryScreenEvents.PhoneNumberChanged(it)) }, - onPhoneNumberEntered = { onEvent(PhoneNumberEntryScreenEvents.PhoneNumberEntered) }, + onPhoneNumberChanged = { onEvent(PhoneNumberEntryScreenEvents.NationalNumberChanged(it)) }, + onPhoneNumberSubmitted = { onEvent(PhoneNumberEntryScreenEvents.NextClicked) }, modifier = Modifier.fillMaxWidth() ) } @@ -237,8 +324,8 @@ private fun TwoPaneLayout( countryCode = state.countryCode, formattedNumber = state.formattedNumber, onCountryCodeChanged = { onEvent(PhoneNumberEntryScreenEvents.CountryCodeChanged(it)) }, - onPhoneNumberChanged = { onEvent(PhoneNumberEntryScreenEvents.PhoneNumberChanged(it)) }, - onPhoneNumberEntered = { onEvent(PhoneNumberEntryScreenEvents.PhoneNumberEntered) }, + onPhoneNumberChanged = { onEvent(PhoneNumberEntryScreenEvents.NationalNumberChanged(it)) }, + onPhoneNumberSubmitted = { onEvent(PhoneNumberEntryScreenEvents.NextClicked) }, modifier = Modifier.fillMaxWidth() ) } @@ -332,7 +419,7 @@ private fun NextButton( verticalAlignment = Alignment.CenterVertically ) { Buttons.LargeTonal( - onClick = { onEvent(PhoneNumberEntryScreenEvents.PhoneNumberEntered) }, + onClick = { onEvent(PhoneNumberEntryScreenEvents.NextClicked) }, enabled = !state.showSpinner && state.isNumberPossible, modifier = Modifier.testTag(TestTags.PHONE_NUMBER_NEXT_BUTTON) ) { @@ -410,7 +497,7 @@ private fun PhoneNumberInputFields( formattedNumber: String, onCountryCodeChanged: (String) -> Unit, onPhoneNumberChanged: (String) -> Unit, - onPhoneNumberEntered: () -> Unit, + onPhoneNumberSubmitted: () -> Unit, modifier: Modifier = Modifier ) { var phoneNumberTextFieldValue by remember { mutableStateOf(TextFieldValue(formattedNumber)) } @@ -499,7 +586,7 @@ private fun PhoneNumberInputFields( imeAction = ImeAction.Done ), keyboardActions = KeyboardActions( - onDone = { onPhoneNumberEntered() } + onDone = { onPhoneNumberSubmitted() } ), singleLine = true, textStyle = MaterialTheme.typography.bodyLarge.copy( diff --git a/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryScreenEvents.kt b/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryScreenEvents.kt index 9f828db911..87fa8aea0d 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryScreenEvents.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryScreenEvents.kt @@ -9,12 +9,32 @@ import org.signal.core.util.censor import org.signal.registration.screens.localbackuprestore.LocalBackupRestoreResult sealed class PhoneNumberEntryScreenEvents { + /** The phone country code prefix (i.e. +1) was changed by the user. */ data class CountryCodeChanged(val value: String) : PhoneNumberEntryScreenEvents() - data class PhoneNumberChanged(val value: String) : PhoneNumberEntryScreenEvents() + + /** The national number (basically the number without the country code) was changed by the user. */ + data class NationalNumberChanged(val value: String) : PhoneNumberEntryScreenEvents() + + /** The user changed the country via the country picker. */ data class CountrySelected(val countryCode: Int, val regionCode: String, val countryName: String, val countryEmoji: String) : PhoneNumberEntryScreenEvents() - data object PhoneNumberEntered : PhoneNumberEntryScreenEvents() + + /** + * The user entered their full number all at once. This could be from the Google Play picker or autofilled using READ_PHONE_STATE permission. + * + * @param autoConfirm If true, we trust the entry enough to skip straight to the confirmation dialog (as if the user had clicked 'next'). + */ + data class FullPhoneNumberEntered(val e164: String, val autoConfirm: Boolean = false) : PhoneNumberEntryScreenEvents() + + /** The user clicked the 'next' button. */ + data object NextClicked : PhoneNumberEntryScreenEvents() + + /** The user dismissed or otherwise canceled the dialog that was shown to confirm their phone number. */ data object PhoneNumberCancelled : PhoneNumberEntryScreenEvents() - data object PhoneNumberSubmitted : PhoneNumberEntryScreenEvents() + + /** The user confirmed their phone number in the dialog. */ + data object PhoneNumberConfirmed : PhoneNumberEntryScreenEvents() + + /** The user requested to open the country picker. */ data object CountryPicker : PhoneNumberEntryScreenEvents() data class CaptchaCompleted(val token: String) : PhoneNumberEntryScreenEvents() { override fun toString(): String = "CaptchaCompleted(token=${token.censor()})" 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 7bbfc5c9ac..ab27065e6d 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 @@ -27,9 +27,10 @@ data class PhoneNumberEntryState( val oneTimeEvent: OneTimeEvent? = null, val preExistingRegistrationData: PreExistingRegistrationData? = null, val restoredSvrCredentials: List = emptyList(), - val pendingRestoreOption: PendingRestoreOption? = null + val pendingRestoreOption: PendingRestoreOption? = null, + val initialized: Boolean = false ) { - override fun toString(): String = "PhoneNumberEntryState(regionCode=$regionCode, countryCode=$countryCode, countryName=$countryName, countryEmoji=$countryEmoji, nationalNumber=$nationalNumber, formattedNumber=$formattedNumber, sessionE164=$sessionE164, sessionMetadata=${sessionMetadata?.let { "present" }}, showSpinner=$showSpinner, showDialog=$showDialog, oneTimeEvent=$oneTimeEvent, preExistingRegistrationData=${preExistingRegistrationData?.let { "present" }}, restoredSvrCredentials=${restoredSvrCredentials.size} items, pendingRestoreOption=$pendingRestoreOption)" + override fun toString(): String = "PhoneNumberEntryState(regionCode=$regionCode, countryCode=$countryCode, countryName=$countryName, countryEmoji=$countryEmoji, nationalNumber=$nationalNumber, formattedNumber=$formattedNumber, sessionE164=$sessionE164, sessionMetadata=${sessionMetadata?.let { "present" }}, showSpinner=$showSpinner, showDialog=$showDialog, oneTimeEvent=$oneTimeEvent, preExistingRegistrationData=${preExistingRegistrationData?.let { "present" }}, restoredSvrCredentials=${restoredSvrCredentials.size} items, pendingRestoreOption=$pendingRestoreOption, initialized=$initialized)" sealed interface OneTimeEvent { data object NetworkError : OneTimeEvent 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 fb3b598407..ed54b1fc53 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 @@ -10,11 +10,13 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.viewModelScope import com.google.i18n.phonenumbers.AsYouTypeFormatter +import com.google.i18n.phonenumbers.NumberParseException import com.google.i18n.phonenumbers.PhoneNumberUtil import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update @@ -63,6 +65,14 @@ class PhoneNumberEntryViewModel( restoredSvrCredentials = repository.getRestoredSvrCredentials() ) setDefaultCountry() + + parentState.firstOrNull()?.preExistingRegistrationData?.e164?.let { preExistingE164 -> + if (state.value.formattedNumber.isEmpty()) { + _state.value = applyFullPhoneNumberEntered(_state.value, preExistingE164) + } + } + + _state.update { it.copy(initialized = true) } } } @@ -94,16 +104,20 @@ class PhoneNumberEntryViewModel( is PhoneNumberEntryScreenEvents.CountrySelected -> { stateEmitter(applyCountrySelected(state, event.countryCode, event.regionCode, event.countryName, event.countryEmoji)) } - is PhoneNumberEntryScreenEvents.PhoneNumberChanged -> { + is PhoneNumberEntryScreenEvents.FullPhoneNumberEntered -> { + val populatedState = applyFullPhoneNumberEntered(state, event.e164) + stateEmitter(populatedState.copy(showDialog = event.autoConfirm && populatedState.isNumberPossible)) + } + is PhoneNumberEntryScreenEvents.NationalNumberChanged -> { stateEmitter(applyPhoneNumberChanged(state, event.value)) } - is PhoneNumberEntryScreenEvents.PhoneNumberEntered -> { + is PhoneNumberEntryScreenEvents.NextClicked -> { stateEmitter(state.copy(showDialog = true)) } is PhoneNumberEntryScreenEvents.PhoneNumberCancelled -> { stateEmitter(state.copy(showDialog = false)) } - is PhoneNumberEntryScreenEvents.PhoneNumberSubmitted -> { + is PhoneNumberEntryScreenEvents.PhoneNumberConfirmed -> { var localState = state.copy(showSpinner = true, showDialog = false) stateEmitter(localState) localState = applyPhoneNumberSubmitted(localState, parentEventEmitter) @@ -167,6 +181,32 @@ class PhoneNumberEntryViewModel( ) } + @VisibleForTesting + fun applyFullPhoneNumberEntered(state: PhoneNumberEntryState, e164: String): PhoneNumberEntryState { + val parsedNumber = try { + phoneNumberUtil.parse(e164, null) + } catch (e: NumberParseException) { + Log.w(TAG, "Failed to parse E164 used to populate phone number.", e) + return state + } + + val countryCode = parsedNumber.countryCode + val nationalNumber = parsedNumber.nationalNumber.toString() + val regionCode = phoneNumberUtil.getRegionCodeForNumber(parsedNumber) ?: phoneNumberUtil.getRegionCodeForCountryCode(countryCode) + + formatter = phoneNumberUtil.getAsYouTypeFormatter(regionCode) + val formattedNumber = formatNumber(nationalNumber) + + return state.copy( + countryCode = countryCode.toString(), + regionCode = regionCode, + countryName = E164Util.getRegionDisplayName(regionCode).orElse(""), + countryEmoji = CountryUtils.countryToEmoji(regionCode).takeIf { regionCode != "ZZ" } ?: "", + nationalNumber = nationalNumber, + formattedNumber = formattedNumber + ) + } + private fun applyCountryCodeChanged(state: PhoneNumberEntryState, countryCode: String): PhoneNumberEntryState { // Only allow digits, max 3 characters val sanitized = countryCode.filter { it.isDigit() }.take(3) 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 c0fc08c28c..a54ded0570 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 @@ -22,7 +22,9 @@ import io.mockk.mockk import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.launch import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain @@ -79,7 +81,7 @@ class PhoneNumberEntryViewModelTest { viewModel.applyEvent( initialState, - PhoneNumberEntryScreenEvents.PhoneNumberChanged("555-123-4567"), + PhoneNumberEntryScreenEvents.NationalNumberChanged("555-123-4567"), parentEventEmitter, stateEmitter ) @@ -95,7 +97,7 @@ class PhoneNumberEntryViewModelTest { viewModel.applyEvent( initialState, - PhoneNumberEntryScreenEvents.PhoneNumberChanged("5551234567"), + PhoneNumberEntryScreenEvents.NationalNumberChanged("5551234567"), parentEventEmitter, stateEmitter ) @@ -109,25 +111,25 @@ class PhoneNumberEntryViewModelTest { fun `PhoneNumberChanged formats progressively as digits are added`() = runTest { var state = PhoneNumberEntryState() - viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.PhoneNumberChanged("5"), parentEventEmitter, stateEmitter) + viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.NationalNumberChanged("5"), parentEventEmitter, stateEmitter) state = emittedStates.last() assertThat(state.nationalNumber).isEqualTo("5") - viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.PhoneNumberChanged("55"), parentEventEmitter, stateEmitter) + viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.NationalNumberChanged("55"), parentEventEmitter, stateEmitter) state = emittedStates.last() assertThat(state.nationalNumber).isEqualTo("55") - viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.PhoneNumberChanged("555"), parentEventEmitter, stateEmitter) + viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.NationalNumberChanged("555"), parentEventEmitter, stateEmitter) state = emittedStates.last() assertThat(state.nationalNumber).isEqualTo("555") - viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.PhoneNumberChanged("5551"), parentEventEmitter, stateEmitter) + viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.NationalNumberChanged("5551"), parentEventEmitter, stateEmitter) state = emittedStates.last() assertThat(state.nationalNumber).isEqualTo("5551") // libphonenumber formats progressively - at 4 digits it's still building the format assertThat(state.formattedNumber).isEqualTo("555-1") - viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.PhoneNumberChanged("55512"), parentEventEmitter, stateEmitter) + viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.NationalNumberChanged("55512"), parentEventEmitter, stateEmitter) state = emittedStates.last() assertThat(state.nationalNumber).isEqualTo("55512") assertThat(state.formattedNumber).isEqualTo("555-12") @@ -139,7 +141,7 @@ class PhoneNumberEntryViewModelTest { viewModel.applyEvent( initialState, - PhoneNumberEntryScreenEvents.PhoneNumberChanged("(555) abc 123-4567!"), + PhoneNumberEntryScreenEvents.NationalNumberChanged("(555) abc 123-4567!"), parentEventEmitter, stateEmitter ) @@ -154,7 +156,7 @@ class PhoneNumberEntryViewModelTest { viewModel.applyEvent( initialState, - PhoneNumberEntryScreenEvents.PhoneNumberChanged("555-123-4567"), + PhoneNumberEntryScreenEvents.NationalNumberChanged("555-123-4567"), parentEventEmitter, stateEmitter ) @@ -274,11 +276,143 @@ class PhoneNumberEntryViewModelTest { assertThat(state.regionCode).isEqualTo("DE") // Enter a German number - viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.PhoneNumberChanged("15123456789"), parentEventEmitter, stateEmitter) + viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.NationalNumberChanged("15123456789"), parentEventEmitter, stateEmitter) state = emittedStates.last() assertThat(state.nationalNumber).isEqualTo("15123456789") } + // ==================== FullPhoneNumberEntered Tests ==================== + + @Test + fun `PhoneNumberHintSelected populates country and number from US E164`() = runTest { + viewModel.applyEvent( + PhoneNumberEntryState(), + PhoneNumberEntryScreenEvents.FullPhoneNumberEntered("+15551234567"), + parentEventEmitter, + stateEmitter + ) + + assertThat(emittedStates).hasSize(1) + val result = emittedStates.last() + assertThat(result.countryCode).isEqualTo("1") + assertThat(result.regionCode).isEqualTo("US") + assertThat(result.nationalNumber).isEqualTo("5551234567") + assertThat(result.formattedNumber).isEqualTo("(555) 123-4567") + } + + @Test + fun `PhoneNumberHintSelected populates country and number from GB E164`() = runTest { + viewModel.applyEvent( + PhoneNumberEntryState(), + PhoneNumberEntryScreenEvents.FullPhoneNumberEntered("+442079460958"), + parentEventEmitter, + stateEmitter + ) + + assertThat(emittedStates).hasSize(1) + val result = emittedStates.last() + assertThat(result.countryCode).isEqualTo("44") + assertThat(result.regionCode).isEqualTo("GB") + assertThat(result.nationalNumber).isEqualTo("2079460958") + } + + @Test + fun `PhoneNumberHintSelected leaves state unchanged for unparseable number`() = runTest { + val initialState = PhoneNumberEntryState(countryCode = "1", regionCode = "US") + + viewModel.applyEvent( + initialState, + PhoneNumberEntryScreenEvents.FullPhoneNumberEntered("not-a-number"), + parentEventEmitter, + stateEmitter + ) + + assertThat(emittedStates).hasSize(1) + assertThat(emittedStates.last()).isEqualTo(initialState) + } + + @Test + fun `FullPhoneNumberEntered with autoConfirm populates and opens the confirmation dialog`() = runTest { + viewModel.applyEvent( + PhoneNumberEntryState(), + PhoneNumberEntryScreenEvents.FullPhoneNumberEntered("+15551234567", autoConfirm = true), + parentEventEmitter, + stateEmitter + ) + + assertThat(emittedStates).hasSize(1) + val result = emittedStates.last() + assertThat(result.nationalNumber).isEqualTo("5551234567") + assertThat(result.showDialog).isTrue() + + // We only open the dialog; we do not submit on our own. + assertThat(emittedEvents).isEmpty() + coVerify(exactly = 0) { mockRepository.createSession(any()) } + } + + @Test + fun `FullPhoneNumberEntered with autoConfirm does not open dialog when number is not possible`() = runTest { + viewModel.applyEvent( + PhoneNumberEntryState(), + PhoneNumberEntryScreenEvents.FullPhoneNumberEntered("not-a-number", autoConfirm = true), + parentEventEmitter, + stateEmitter + ) + + assertThat(emittedStates).hasSize(1) + assertThat(emittedStates.last().showDialog).isFalse() + assertThat(emittedEvents).isEmpty() + } + + @Test + fun `FullPhoneNumberEntered without autoConfirm only populates and does not open dialog`() = runTest { + viewModel.applyEvent( + PhoneNumberEntryState(), + PhoneNumberEntryScreenEvents.FullPhoneNumberEntered("+15551234567", autoConfirm = false), + parentEventEmitter, + stateEmitter + ) + + assertThat(emittedStates).hasSize(1) + assertThat(emittedStates.last().nationalNumber).isEqualTo("5551234567") + assertThat(emittedStates.last().showDialog).isFalse() + assertThat(emittedEvents).isEmpty() + } + + // ==================== Pre-existing Registration Data Prefill Tests ==================== + + @Test + fun `prefills phone number from preExistingRegistrationData when number is empty`() = runTest { + val preExisting = mockk(relaxed = true) + every { preExisting.e164 } returns "+15551234567" + + val populatedParentState = MutableStateFlow(RegistrationFlowState(preExistingRegistrationData = preExisting)) + val vm = PhoneNumberEntryViewModel(mockRepository, populatedParentState, parentEventEmitter) + + val states = mutableListOf() + val job = launch { vm.state.collect { states.add(it) } } + advanceUntilIdle() + job.cancel() + + val latest = states.last() + assertThat(latest.nationalNumber).isEqualTo("5551234567") + assertThat(latest.countryCode).isEqualTo("1") + assertThat(latest.regionCode).isEqualTo("US") + } + + @Test + fun `does not prefill phone number when there is no preExistingRegistrationData`() = runTest { + val emptyParentState = MutableStateFlow(RegistrationFlowState()) + val vm = PhoneNumberEntryViewModel(mockRepository, emptyParentState, parentEventEmitter) + + val states = mutableListOf() + val job = launch { vm.state.collect { states.add(it) } } + advanceUntilIdle() + job.cancel() + + assertThat(states.last().nationalNumber).isEmpty() + } + // ==================== PhoneNumberSubmitted Tests ==================== @Test @@ -295,7 +429,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -323,7 +457,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -350,7 +484,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -374,7 +508,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -393,7 +527,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -412,7 +546,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -433,7 +567,7 @@ class PhoneNumberEntryViewModelTest { coEvery { mockRepository.requestVerificationCode(any(), any(), any()) } returns RequestResult.Success(existingSession) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -465,7 +599,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -490,7 +624,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -516,7 +650,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -543,7 +677,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -572,7 +706,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -606,7 +740,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -644,7 +778,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -677,7 +811,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -710,7 +844,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -742,7 +876,7 @@ class PhoneNumberEntryViewModelTest { nationalNumber = "5551234567" ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Verify spinner states assertThat(emittedStates.first().showSpinner).isTrue() @@ -916,7 +1050,7 @@ class PhoneNumberEntryViewModelTest { preExistingRegistrationData = preExistingData ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) assertThat(emittedEvents.first()).isInstanceOf() assertThat(emittedEvents[1]) @@ -943,7 +1077,7 @@ class PhoneNumberEntryViewModelTest { preExistingRegistrationData = preExistingData ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) assertThat(emittedEvents.first()).isInstanceOf() assertThat(emittedEvents[1]) @@ -970,7 +1104,7 @@ class PhoneNumberEntryViewModelTest { preExistingRegistrationData = preExistingData ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) } @Test(expected = IllegalStateException::class) @@ -991,7 +1125,7 @@ class PhoneNumberEntryViewModelTest { preExistingRegistrationData = preExistingData ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) } @Test @@ -1017,7 +1151,7 @@ class PhoneNumberEntryViewModelTest { preExistingRegistrationData = preExistingData ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) assertThat(emittedEvents).hasSize(1) assertThat(emittedEvents.first()) @@ -1044,7 +1178,7 @@ class PhoneNumberEntryViewModelTest { preExistingRegistrationData = preExistingData ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) assertThat(emittedStates.last().oneTimeEvent).isNotNull() .isInstanceOf() @@ -1075,7 +1209,7 @@ class PhoneNumberEntryViewModelTest { preExistingRegistrationData = preExistingData ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Should emit RecoveryPasswordInvalid and then continue to session creation assertThat(emittedEvents.first()).isEqualTo(RegistrationFlowEvent.RecoveryPasswordInvalid) @@ -1106,7 +1240,7 @@ class PhoneNumberEntryViewModelTest { preExistingRegistrationData = preExistingData ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) assertThat(emittedEvents.first()).isEqualTo(RegistrationFlowEvent.RecoveryPasswordInvalid) assertThat(emittedStates.last().preExistingRegistrationData).isNull() @@ -1128,7 +1262,7 @@ class PhoneNumberEntryViewModelTest { preExistingRegistrationData = preExistingData ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) assertThat(emittedStates.last().oneTimeEvent).isEqualTo(PhoneNumberEntryState.OneTimeEvent.NetworkError) } @@ -1149,7 +1283,7 @@ class PhoneNumberEntryViewModelTest { preExistingRegistrationData = preExistingData ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) assertThat(emittedStates.last().oneTimeEvent).isEqualTo(PhoneNumberEntryState.OneTimeEvent.UnknownError) } @@ -1173,7 +1307,7 @@ class PhoneNumberEntryViewModelTest { preExistingRegistrationData = preExistingData ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Should skip RRP and go to session creation flow coVerify(exactly = 0) { mockRepository.registerAccountWithRecoveryPassword(any(), any(), any(), any(), any()) } @@ -1204,7 +1338,7 @@ class PhoneNumberEntryViewModelTest { restoredSvrCredentials = svrCredentials ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) assertThat(emittedEvents).hasSize(2) assertThat(emittedEvents[0]).isInstanceOf() @@ -1237,7 +1371,7 @@ class PhoneNumberEntryViewModelTest { restoredSvrCredentials = svrCredentials ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Should fall through to session creation assertThat(emittedEvents.last()) @@ -1266,7 +1400,7 @@ class PhoneNumberEntryViewModelTest { restoredSvrCredentials = svrCredentials ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) // Should ignore error and fall through assertThat(emittedEvents.last()) @@ -1295,7 +1429,7 @@ class PhoneNumberEntryViewModelTest { restoredSvrCredentials = svrCredentials ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) assertThat(emittedEvents.last()) .isInstanceOf() @@ -1325,7 +1459,7 @@ class PhoneNumberEntryViewModelTest { restoredSvrCredentials = svrCredentials ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) assertThat(emittedEvents.last()) .isInstanceOf() @@ -1355,7 +1489,7 @@ class PhoneNumberEntryViewModelTest { restoredSvrCredentials = svrCredentials ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) assertThat(emittedEvents.last()) .isInstanceOf() @@ -1378,7 +1512,7 @@ class PhoneNumberEntryViewModelTest { restoredSvrCredentials = emptyList() ) - viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberSubmitted, parentEventEmitter, stateEmitter) + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) coVerify(exactly = 0) { mockRepository.checkSvrCredentials(any(), any()) } assertThat(emittedEvents.last()) 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 cc589ed414..f5a317cb0d 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 @@ -94,7 +94,7 @@ class PhoneNumberScreenTest { composeTestRule.onNodeWithTag(TestTags.PHONE_NUMBER_NEXT_BUTTON).performClick() // Then - assert(emittedEvent is PhoneNumberEntryScreenEvents.PhoneNumberEntered) { + assert(emittedEvent is PhoneNumberEntryScreenEvents.NextClicked) { "Expected PhoneNumberEntered event but got $emittedEvent" } } diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index db573bfd9e..680ddf77de 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -20302,6 +20302,11 @@ https://docs.gradle.org/current/userguide/dependency_verification.html + + + + + @@ -20425,6 +20430,11 @@ https://docs.gradle.org/current/userguide/dependency_verification.html + + + + +