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 82390f8622..4526bab64f 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 @@ -6,9 +6,16 @@ package org.signal.registration.screens.phonenumber import org.signal.core.util.censor +import org.signal.registration.RegistrationFlowState import org.signal.registration.screens.localbackuprestore.LocalBackupRestoreResult sealed class PhoneNumberEntryScreenEvents { + /** Emitted once when the screen is created to load initial data into the state. */ + data object Initialize : PhoneNumberEntryScreenEvents() + + /** The parent registration flow state changed and needs to be merged into this screen's state. */ + data class ParentStateChanged(val parentState: RegistrationFlowState) : PhoneNumberEntryScreenEvents() + /** The phone country code prefix (i.e. +1) was changed by the user. */ data class CountryCodeChanged(val value: String) : PhoneNumberEntryScreenEvents() 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 a2b9c4cd51..4241b62c0a 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 @@ -13,14 +13,11 @@ 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.asStateFlow +import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update -import kotlinx.coroutines.launch import kotlinx.coroutines.withTimeoutOrNull import org.signal.core.models.AccountEntropyPool import org.signal.core.util.E164Util @@ -39,27 +36,6 @@ import org.signal.registration.screens.localbackuprestore.LocalBackupRestoreResu import org.signal.registration.screens.phonenumber.PhoneNumberEntryState.OneTimeEvent import org.signal.registration.screens.util.navigateTo -/** - * Returns how many characters a single edit inserted when going from [old] to [new], by diffing off the common prefix - * and suffix. Used to distinguish a typed character (1) from a paste or autofill (more than 1). Returns 0 for - * deletions or replacements that don't grow the differing region. - */ -private fun insertedCharCount(old: String, new: String): Int { - val max = minOf(old.length, new.length) - - var prefix = 0 - while (prefix < max && old[prefix] == new[prefix]) { - prefix++ - } - - var suffix = 0 - while (suffix < max - prefix && old[old.length - 1 - suffix] == new[new.length - 1 - suffix]) { - suffix++ - } - - return (new.length - prefix - suffix).coerceAtLeast(0) -} - class PhoneNumberEntryViewModel( val repository: RegistrationRepository, private val parentState: StateFlow, @@ -75,26 +51,20 @@ class PhoneNumberEntryViewModel( private var formatter: AsYouTypeFormatter = phoneNumberUtil.getAsYouTypeFormatter("US") private val _state = MutableStateFlow(PhoneNumberEntryState()) - val state = _state - .combine(parentState) { state, parentState -> applyParentState(state, parentState) } - .onEach { Log.d(TAG, "[State] $it") } - .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), PhoneNumberEntryState()) + val state: StateFlow = _state.asStateFlow() init { - viewModelScope.launch { - _state.value = state.value.copy( - restoredSvrCredentials = repository.getRestoredSvrCredentials() - ) - setDefaultCountry() + setDefaultCountry() - parentState.firstOrNull()?.preExistingRegistrationData?.e164?.let { preExistingE164 -> - if (state.value.formattedNumber.isEmpty()) { - _state.value = applyFullPhoneNumberEntered(_state.value, preExistingE164) - } - } + _state + .onEach { Log.d(TAG, "[State] $it") } + .launchIn(viewModelScope) - _state.update { it.copy(initialized = true) } - } + onEvent(PhoneNumberEntryScreenEvents.Initialize) + + parentState + .onEach { onEvent(PhoneNumberEntryScreenEvents.ParentStateChanged(it)) } + .launchIn(viewModelScope) } fun setDefaultCountry() { @@ -111,7 +81,7 @@ class PhoneNumberEntryViewModel( } override suspend fun processEvent(event: PhoneNumberEntryScreenEvents) { - applyEvent(state.value, event, parentEventEmitter) { + applyEvent(_state.value, event, parentEventEmitter) { _state.value = it } } @@ -119,6 +89,12 @@ class PhoneNumberEntryViewModel( @VisibleForTesting suspend fun applyEvent(state: PhoneNumberEntryState, event: PhoneNumberEntryScreenEvents, parentEventEmitter: (RegistrationFlowEvent) -> Unit, stateEmitter: (PhoneNumberEntryState) -> Unit) { when (event) { + is PhoneNumberEntryScreenEvents.Initialize -> { + stateEmitter(applyInitialize(state)) + } + is PhoneNumberEntryScreenEvents.ParentStateChanged -> { + stateEmitter(applyParentState(state, event.parentState)) + } is PhoneNumberEntryScreenEvents.CountryCodeChanged -> { stateEmitter(applyCountryCodeChanged(state, event.value)) } @@ -178,8 +154,19 @@ class PhoneNumberEntryViewModel( } } - @VisibleForTesting - fun applyParentState(state: PhoneNumberEntryState, parentState: RegistrationFlowState): PhoneNumberEntryState { + private suspend fun applyInitialize(inputState: PhoneNumberEntryState): PhoneNumberEntryState { + var state = inputState.copy(restoredSvrCredentials = repository.getRestoredSvrCredentials()) + + parentState.value.preExistingRegistrationData?.e164?.let { preExistingE164 -> + if (state.formattedNumber.isEmpty()) { + state = applyFullPhoneNumberEntered(state, preExistingE164) + } + } + + return state.copy(initialized = true) + } + + private fun applyParentState(state: PhoneNumberEntryState, parentState: RegistrationFlowState): PhoneNumberEntryState { return state.copy( sessionE164 = parentState.sessionE164, sessionMetadata = parentState.sessionMetadata, @@ -791,6 +778,22 @@ class PhoneNumberEntryViewModel( } } + private fun insertedCharCount(old: String, new: String): Int { + val max = minOf(old.length, new.length) + + var prefix = 0 + while (prefix < max && old[prefix] == new[prefix]) { + prefix++ + } + + var suffix = 0 + while (suffix < max - prefix && old[old.length - 1 - suffix] == new[new.length - 1 - suffix]) { + suffix++ + } + + return (new.length - prefix - suffix).coerceAtLeast(0) + } + /** * Recomputes [PhoneNumberEntryState.isNumberPossible] and [PhoneNumberEntryState.isNumberInvalid] from the current * country code and national number. Should be applied to any state that changes either of those fields. 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 ec05af089d..fb9ae58dc6 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 @@ -560,6 +560,55 @@ class PhoneNumberEntryViewModelTest { assertThat(emittedEvents).isEmpty() } + // ==================== Initialize Tests ==================== + + @Test + fun `state is populated with the default country and initialized once construction settles`() { + // The view model was constructed and its event loop advanced to idle in setup(). + val state = viewModel.state.value + assertThat(state.regionCode).isEqualTo("US") + assertThat(state.countryCode).isEqualTo("1") + assertThat(state.countryName).isEqualTo("United States") + assertThat(state.initialized).isTrue() + } + + @Test + fun `Initialize loads restored SVR credentials into state and marks it initialized`() = runTest { + val credentials = listOf(NetworkController.SvrCredentials(username = "user", password = "pass")) + coEvery { mockRepository.getRestoredSvrCredentials() } returns credentials + + viewModel.applyEvent(PhoneNumberEntryState(), PhoneNumberEntryScreenEvents.Initialize, parentEventEmitter, stateEmitter) + + assertThat(emittedStates).hasSize(1) + assertThat(emittedStates.last().restoredSvrCredentials).isEqualTo(credentials) + assertThat(emittedStates.last().initialized).isTrue() + } + + @Test + fun `Initialize does not prefill when the number field is already populated`() = runTest { + val preExisting = mockk(relaxed = true) + every { preExisting.e164 } returns "+15551234567" + parentState.value = RegistrationFlowState(preExistingRegistrationData = preExisting) + + val alreadyPopulated = PhoneNumberEntryState(countryCode = "44", regionCode = "GB", nationalNumber = "2079460958", formattedNumber = "2079460958") + viewModel.applyEvent(alreadyPopulated, PhoneNumberEntryScreenEvents.Initialize, parentEventEmitter, stateEmitter) + + val result = emittedStates.last() + assertThat(result.nationalNumber).isEqualTo("2079460958") + assertThat(result.countryCode).isEqualTo("44") + } + + // ==================== Parent State Tests ==================== + + @Test + fun `parent state changes are merged into state through the event stream`() = runTest { + parentState.value = RegistrationFlowState(sessionE164 = "+15551234567") + testDispatcher.scheduler.advanceUntilIdle() + + assertThat(viewModel.state.value.sessionE164).isEqualTo("+15551234567") + assertThat(viewModel.state.value.regionCode).isEqualTo("US") + } + // ==================== Pre-existing Registration Data Prefill Tests ==================== @Test @@ -1172,43 +1221,43 @@ class PhoneNumberEntryViewModelTest { assertThat(emittedStates.last().oneTimeEvent).isEqualTo(PhoneNumberEntryState.OneTimeEvent.NetworkError) } - // ==================== applyParentState Tests ==================== + // ==================== ParentStateChanged Tests ==================== @Test - fun `applyParentState copies preExistingRegistrationData from parent`() { + fun `ParentStateChanged copies preExistingRegistrationData from parent`() = runTest { val preExistingData = mockk(relaxed = true) - val state = PhoneNumberEntryState() val parentFlowState = RegistrationFlowState(preExistingRegistrationData = preExistingData) - val result = viewModel.applyParentState(state, parentFlowState) + viewModel.applyEvent(PhoneNumberEntryState(), PhoneNumberEntryScreenEvents.ParentStateChanged(parentFlowState), parentEventEmitter, stateEmitter) - assertThat(result.preExistingRegistrationData).isEqualTo(preExistingData) + assertThat(emittedStates).hasSize(1) + assertThat(emittedStates.last().preExistingRegistrationData).isEqualTo(preExistingData) } @Test - fun `applyParentState clears restoredSvrCredentials when doNotAttemptRecoveryPassword is true`() { + fun `ParentStateChanged clears restoredSvrCredentials when doNotAttemptRecoveryPassword is true`() = runTest { val credentials = listOf( NetworkController.SvrCredentials(username = "user", password = "pass") ) val state = PhoneNumberEntryState(restoredSvrCredentials = credentials) val parentFlowState = RegistrationFlowState(doNotAttemptRecoveryPassword = true) - val result = viewModel.applyParentState(state, parentFlowState) + viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.ParentStateChanged(parentFlowState), parentEventEmitter, stateEmitter) - assertThat(result.restoredSvrCredentials).isEmpty() + assertThat(emittedStates.last().restoredSvrCredentials).isEmpty() } @Test - fun `applyParentState keeps restoredSvrCredentials when doNotAttemptRecoveryPassword is false`() { + fun `ParentStateChanged keeps restoredSvrCredentials when doNotAttemptRecoveryPassword is false`() = runTest { val credentials = listOf( NetworkController.SvrCredentials(username = "user", password = "pass") ) val state = PhoneNumberEntryState(restoredSvrCredentials = credentials) val parentFlowState = RegistrationFlowState(doNotAttemptRecoveryPassword = false) - val result = viewModel.applyParentState(state, parentFlowState) + viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.ParentStateChanged(parentFlowState), parentEventEmitter, stateEmitter) - assertThat(result.restoredSvrCredentials).isEqualTo(credentials) + assertThat(emittedStates.last().restoredSvrCredentials).isEqualTo(credentials) } // ==================== Pre-existing Registration Data (RRP) Tests ====================