From b8d43fa504ef1402025d5ed65f45a74451a533a3 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 23 Sep 2026 11:31:59 -0400 Subject: [PATCH] Allow using existing login while payment info is still loading. --- .../SignalLoginPaymentScreenEvents.kt | 9 ++ .../SignalLoginPaymentViewModel.kt | 106 ++++++++------ .../SignalLoginPaymentViewModelTest.kt | 136 ++++++++++++------ 3 files changed, 166 insertions(+), 85 deletions(-) diff --git a/feature/registration/src/main/java/org/signal/registration/screens/signalloginpayment/SignalLoginPaymentScreenEvents.kt b/feature/registration/src/main/java/org/signal/registration/screens/signalloginpayment/SignalLoginPaymentScreenEvents.kt index bf312e4c12..cde06954af 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/signalloginpayment/SignalLoginPaymentScreenEvents.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/signalloginpayment/SignalLoginPaymentScreenEvents.kt @@ -11,6 +11,15 @@ sealed class SignalLoginPaymentScreenEvents { /** Emitted once when the screen is created to load initial data (namely the purchase price) into the state. */ data object Initialize : SignalLoginPaymentScreenEvents() + /** The check for a paid-for but unredeemed Signal Login purchase finished. */ + data class UnredeemedPurchaseLoaded(val hasUnredeemedPurchase: Boolean) : SignalLoginPaymentScreenEvents() + + /** Google Play became able to take a payment, and the price lookup that follows has started. */ + data class PaymentInfoLoading(val paymentAvailability: PaymentAvailability) : SignalLoginPaymentScreenEvents() + + /** A lookup of whether Google Play can take a payment, and the resulting price, finished. */ + data class PaymentInfoLoaded(val paymentAvailability: PaymentAvailability, val price: SignalLoginPaymentState.Price) : SignalLoginPaymentScreenEvents() + /** The screen came back to the foreground, which is the cue to re-check anything the user went off to fix. */ data object Foregrounded : SignalLoginPaymentScreenEvents() diff --git a/feature/registration/src/main/java/org/signal/registration/screens/signalloginpayment/SignalLoginPaymentViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/signalloginpayment/SignalLoginPaymentViewModel.kt index 3f6a6055ed..0991ba75da 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/signalloginpayment/SignalLoginPaymentViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/signalloginpayment/SignalLoginPaymentViewModel.kt @@ -7,6 +7,7 @@ package org.signal.registration.screens.signalloginpayment import androidx.annotation.VisibleForTesting import androidx.lifecycle.viewModelScope +import kotlinx.coroutines.Job import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow @@ -15,6 +16,7 @@ import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.receiveAsFlow +import kotlinx.coroutines.launch import org.signal.core.ui.compose.EventDrivenViewModel import org.signal.core.util.logging.Log import org.signal.libsignal.net.RequestResult @@ -44,6 +46,8 @@ class SignalLoginPaymentViewModel( private val _actions = Channel(Channel.BUFFERED) val actions: Flow = _actions.receiveAsFlow() + private var paymentInfoJob: Job? = null + init { _state .onEach { Log.d(TAG, "[State] $it") } @@ -65,31 +69,35 @@ class SignalLoginPaymentViewModel( ) { when (event) { is SignalLoginPaymentScreenEvents.Initialize -> { - val hasUnredeemedPurchase = repository.hasUnredeemedSignalLoginPurchase() - - if (hasUnredeemedPurchase) { - Log.i(TAG, "[Initialize] The user already has a Signal Login purchase that was never redeemed.") - } - - val paymentAvailability = repository.getPaymentAvailability() - - val price = when { - paymentAvailability.isAvailable -> loadPrice() - paymentAvailability.isTerminal -> { - Log.w(TAG, "[Initialize] Google Play can never take a payment here ($paymentAvailability). Offering an existing login only.") - SignalLoginPaymentState.Price.Unavailable - } - else -> { - Log.w(TAG, "[Initialize] Google Play cannot take a payment ($paymentAvailability), so there is no price to show yet.") - SignalLoginPaymentState.Price.TransientError + paymentInfoJob = viewModelScope.launch { + val hasUnredeemedPurchase = repository.hasUnredeemedSignalLoginPurchase() + if (hasUnredeemedPurchase) { + Log.i(TAG, "[Initialize] The user already has a Signal Login purchase that was never redeemed.") } + onEvent(SignalLoginPaymentScreenEvents.UnredeemedPurchaseLoaded(hasUnredeemedPurchase)) + onEvent(loadPaymentInfo(repository.getPaymentAvailability())) } + } + is SignalLoginPaymentScreenEvents.UnredeemedPurchaseLoaded -> { + stateEmitter(state.copy(hasUnredeemedPurchase = event.hasUnredeemedPurchase)) + } + + is SignalLoginPaymentScreenEvents.PaymentInfoLoading -> { + stateEmitter( + state.copy( + price = SignalLoginPaymentState.Price.Loading, + paymentAvailability = event.paymentAvailability, + dialogs = state.dialogs.copy(paymentUnavailable = false) + ) + ) + } + + is SignalLoginPaymentScreenEvents.PaymentInfoLoaded -> { val updated = state.copy( - price = price, - hasUnredeemedPurchase = hasUnredeemedPurchase, - paymentAvailability = paymentAvailability, - dialogs = state.dialogs.copy(paymentUnavailable = !paymentAvailability.isAvailable) + price = event.price, + paymentAvailability = event.paymentAvailability, + dialogs = state.dialogs.copy(paymentUnavailable = !event.paymentAvailability.isAvailable) ) stateEmitter( @@ -102,37 +110,29 @@ class SignalLoginPaymentViewModel( } is SignalLoginPaymentScreenEvents.PriceRetryClicked -> { - val localState = state.copy(price = SignalLoginPaymentState.Price.Loading) - stateEmitter(localState) + stateEmitter(state.copy(price = SignalLoginPaymentState.Price.Loading)) - val availability = repository.getPaymentAvailability() - if (availability.isAvailable) { - stateEmitter(localState.copy(paymentAvailability = availability, price = loadPrice())) - } else { - Log.w(TAG, "[PriceRetryClicked] Google Play still cannot take a payment: $availability") - stateEmitter( - localState.copy( - paymentAvailability = availability, - price = if (availability.isTerminal) SignalLoginPaymentState.Price.Unavailable else SignalLoginPaymentState.Price.TransientError, - dialogs = localState.dialogs.copy(paymentUnavailable = true) - ) - ) + paymentInfoJob?.cancel() + paymentInfoJob = viewModelScope.launch { + onEvent(loadPaymentInfo(repository.getPaymentAvailability())) } } is SignalLoginPaymentScreenEvents.Foregrounded -> { - val availability = repository.getPaymentAvailability() + if (paymentInfoJob?.isActive == true) { + return + } - if (availability != state.paymentAvailability) { - Log.i(TAG, "[Foregrounded] Google Play availability changed from ${state.paymentAvailability} to $availability.") + val previousAvailability = state.paymentAvailability + paymentInfoJob = viewModelScope.launch { + val availability = repository.getPaymentAvailability() - val localState = state.copy(paymentAvailability = availability) - if (availability.isAvailable) { - stateEmitter(localState.copy(price = SignalLoginPaymentState.Price.Loading, dialogs = localState.dialogs.copy(paymentUnavailable = false))) - stateEmitter(localState.copy(price = loadPrice(), dialogs = localState.dialogs.copy(paymentUnavailable = false))) - } else { - val price = if (availability.isTerminal) SignalLoginPaymentState.Price.Unavailable else SignalLoginPaymentState.Price.TransientError - stateEmitter(localState.copy(price = price, dialogs = localState.dialogs.copy(paymentUnavailable = true))) + if (availability != previousAvailability) { + Log.i(TAG, "[Foregrounded] Google Play availability changed from $previousAvailability to $availability.") + if (availability.isAvailable) { + onEvent(SignalLoginPaymentScreenEvents.PaymentInfoLoading(availability)) + } + onEvent(loadPaymentInfo(availability)) } } } @@ -233,6 +233,22 @@ class SignalLoginPaymentViewModel( } } + private suspend fun loadPaymentInfo(availability: PaymentAvailability): SignalLoginPaymentScreenEvents.PaymentInfoLoaded { + val price = when { + availability.isAvailable -> loadPrice() + availability.isTerminal -> { + Log.w(TAG, "[loadPaymentInfo] Google Play can never take a payment here ($availability). Offering an existing login only.") + SignalLoginPaymentState.Price.Unavailable + } + else -> { + Log.w(TAG, "[loadPaymentInfo] Google Play cannot take a payment ($availability), so there is no price to show yet.") + SignalLoginPaymentState.Price.TransientError + } + } + + return SignalLoginPaymentScreenEvents.PaymentInfoLoaded(availability, price) + } + private suspend fun loadPrice(): SignalLoginPaymentState.Price { return when (val result = repository.getSignalLoginPrice()) { is SignalLoginPriceResult.Available -> SignalLoginPaymentState.Price.Available(result.formattedPrice) diff --git a/feature/registration/src/test/java/org/signal/registration/screens/signalloginpayment/SignalLoginPaymentViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/signalloginpayment/SignalLoginPaymentViewModelTest.kt index 6f2f7ae361..485b90ecf9 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/signalloginpayment/SignalLoginPaymentViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/signalloginpayment/SignalLoginPaymentViewModelTest.kt @@ -15,6 +15,7 @@ import io.mockk.clearMocks import io.mockk.coEvery import io.mockk.coVerify import io.mockk.mockk +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.launch @@ -76,6 +77,11 @@ class SignalLoginPaymentViewModelTest { return events to { event: RegistrationFlowEvent -> events.add(event) } } + private fun createViewModel(emitter: (RegistrationFlowEvent) -> Unit = parentEventEmitter): SignalLoginPaymentViewModel { + clearMocks(mockRepository, answers = false) + return SignalLoginPaymentViewModel(repository = mockRepository, parentEventEmitter = emitter) + } + private suspend fun applyEvent(state: SignalLoginPaymentState, event: SignalLoginPaymentScreenEvents, emitter: (RegistrationFlowEvent) -> Unit = parentEventEmitter): SignalLoginPaymentState { var result = state viewModel.applyEvent(state, event, emitter) { result = it } @@ -105,7 +111,7 @@ class SignalLoginPaymentViewModelTest { coEvery { mockRepository.getSignalLoginPrice() } returns SignalLoginPriceResult.Available("$1.99") coEvery { mockRepository.hasUnredeemedSignalLoginPurchase() } returns false - val state = applyEvent(SignalLoginPaymentState(), SignalLoginPaymentScreenEvents.Initialize) + val state = createViewModel().state.value assertThat(state.price).isEqualTo(SignalLoginPaymentState.Price.Available("$1.99")) assertThat(state.hasUnredeemedPurchase).isFalse() @@ -116,7 +122,7 @@ class SignalLoginPaymentViewModelTest { coEvery { mockRepository.getSignalLoginPrice() } returns SignalLoginPriceResult.Unavailable coEvery { mockRepository.hasUnredeemedSignalLoginPurchase() } returns false - val state = applyEvent(SignalLoginPaymentState(), SignalLoginPaymentScreenEvents.Initialize) + val state = createViewModel().state.value assertThat(state.price).isEqualTo(SignalLoginPaymentState.Price.Unavailable) } @@ -126,9 +132,7 @@ class SignalLoginPaymentViewModelTest { coEvery { mockRepository.getPaymentAvailability() } returns PaymentAvailability.PurchasesUnavailable coEvery { mockRepository.hasUnredeemedSignalLoginPurchase() } returns false coEvery { mockRepository.getSignalLoginPrice() } returns SignalLoginPriceResult.Available("$1.99") - clearMocks(mockRepository, answers = false) - - val state = applyEvent(SignalLoginPaymentState(), SignalLoginPaymentScreenEvents.Initialize) + val state = createViewModel().state.value assertThat(state.isPurchaseOptionEnabled).isFalse() assertThat(state.selectedOption).isEqualTo(SignalLoginPaymentState.Option.ExistingLogin) @@ -142,7 +146,7 @@ class SignalLoginPaymentViewModelTest { coEvery { mockRepository.getPaymentAvailability() } returns PaymentAvailability.PurchasesUnavailable coEvery { mockRepository.hasUnredeemedSignalLoginPurchase() } returns true - val state = applyEvent(SignalLoginPaymentState(), SignalLoginPaymentScreenEvents.Initialize) + val state = createViewModel().state.value assertThat(state.isPurchaseOptionEnabled).isTrue() assertThat(state.selectedOption).isEqualTo(SignalLoginPaymentState.Option.Purchase) @@ -152,9 +156,7 @@ class SignalLoginPaymentViewModelTest { fun `Initialize explains the problem and skips the price lookup when Google Play cannot take a payment`() = runTest(testDispatcher) { coEvery { mockRepository.getPaymentAvailability() } returns PaymentAvailability.ServiceMissing coEvery { mockRepository.hasUnredeemedSignalLoginPurchase() } returns false - clearMocks(mockRepository, answers = false) - - val state = applyEvent(SignalLoginPaymentState(), SignalLoginPaymentScreenEvents.Initialize) + val state = createViewModel().state.value assertThat(state.paymentAvailability).isEqualTo(PaymentAvailability.ServiceMissing) assertThat(state.dialogs.paymentUnavailable).isTrue() @@ -184,12 +186,12 @@ class SignalLoginPaymentViewModelTest { @Test fun `PriceRetryClicked explains the problem again when Google Play still cannot take a payment`() = runTest(testDispatcher) { coEvery { mockRepository.getPaymentAvailability() } returns PaymentAvailability.ServiceUpdating - clearMocks(mockRepository, answers = false) + val viewModel = createViewModel() + viewModel.onEvent(SignalLoginPaymentScreenEvents.PaymentUnavailableDialogDismissed) - val state = applyEvent( - SignalLoginPaymentState(price = SignalLoginPaymentState.Price.TransientError), - SignalLoginPaymentScreenEvents.PriceRetryClicked - ) + viewModel.onEvent(SignalLoginPaymentScreenEvents.PriceRetryClicked) + + val state = viewModel.state.value assertThat(state.paymentAvailability).isEqualTo(PaymentAvailability.ServiceUpdating) assertThat(state.dialogs.paymentUnavailable).isTrue() @@ -199,17 +201,15 @@ class SignalLoginPaymentViewModelTest { @Test fun `Foregrounded loads the price once the user has fixed Google Play`() = runTest(testDispatcher) { + coEvery { mockRepository.getPaymentAvailability() } returns PaymentAvailability.ServiceUpdating + val viewModel = createViewModel() + assertThat(viewModel.state.value.dialogs.paymentUnavailable).isTrue() + coEvery { mockRepository.getPaymentAvailability() } returns PaymentAvailability.Available coEvery { mockRepository.getSignalLoginPrice() } returns SignalLoginPriceResult.Available("$1.99") + viewModel.onEvent(SignalLoginPaymentScreenEvents.Foregrounded) - val state = applyEvent( - SignalLoginPaymentState( - price = SignalLoginPaymentState.Price.TransientError, - paymentAvailability = PaymentAvailability.ServiceUpdating, - dialogs = SignalLoginPaymentState.Dialogs(paymentUnavailable = true) - ), - SignalLoginPaymentScreenEvents.Foregrounded - ) + val state = viewModel.state.value assertThat(state.paymentAvailability).isEqualTo(PaymentAvailability.Available) assertThat(state.dialogs.paymentUnavailable).isFalse() @@ -219,15 +219,12 @@ class SignalLoginPaymentViewModelTest { @Test fun `Foregrounded leaves a dismissed dialog dismissed when nothing changed`() = runTest(testDispatcher) { coEvery { mockRepository.getPaymentAvailability() } returns PaymentAvailability.ServiceInvalid - clearMocks(mockRepository, answers = false) + val viewModel = createViewModel() + viewModel.onEvent(SignalLoginPaymentScreenEvents.PaymentUnavailableDialogDismissed) - val state = applyEvent( - SignalLoginPaymentState( - price = SignalLoginPaymentState.Price.TransientError, - paymentAvailability = PaymentAvailability.ServiceInvalid - ), - SignalLoginPaymentScreenEvents.Foregrounded - ) + viewModel.onEvent(SignalLoginPaymentScreenEvents.Foregrounded) + + val state = viewModel.state.value assertThat(state.dialogs.paymentUnavailable).isFalse() coVerify(exactly = 0) { mockRepository.getSignalLoginPrice() } @@ -283,7 +280,7 @@ class SignalLoginPaymentViewModelTest { coEvery { mockRepository.getSignalLoginPrice() } returns SignalLoginPriceResult.TransientError coEvery { mockRepository.hasUnredeemedSignalLoginPurchase() } returns false - val state = applyEvent(SignalLoginPaymentState(), SignalLoginPaymentScreenEvents.Initialize) + val state = createViewModel().state.value assertThat(state.price).isEqualTo(SignalLoginPaymentState.Price.TransientError) assertThat(state.isActionEnabled).isFalse() @@ -291,12 +288,13 @@ class SignalLoginPaymentViewModelTest { @Test fun `PriceRetryClicked re-fetches the price and recovers`() = runTest(testDispatcher) { - coEvery { mockRepository.getSignalLoginPrice() } returns SignalLoginPriceResult.Available("$1.99") + coEvery { mockRepository.getSignalLoginPrice() } returns SignalLoginPriceResult.TransientError + val viewModel = createViewModel() - val state = applyEvent( - SignalLoginPaymentState(price = SignalLoginPaymentState.Price.TransientError), - SignalLoginPaymentScreenEvents.PriceRetryClicked - ) + coEvery { mockRepository.getSignalLoginPrice() } returns SignalLoginPriceResult.Available("$1.99") + viewModel.onEvent(SignalLoginPaymentScreenEvents.PriceRetryClicked) + + val state = viewModel.state.value assertThat(state.price).isEqualTo(SignalLoginPaymentState.Price.Available("$1.99")) assertThat(state.isActionEnabled).isTrue() @@ -307,10 +305,68 @@ class SignalLoginPaymentViewModelTest { coEvery { mockRepository.getSignalLoginPrice() } returns SignalLoginPriceResult.TransientError coEvery { mockRepository.hasUnredeemedSignalLoginPurchase() } returns false - var state = applyEvent(SignalLoginPaymentState(), SignalLoginPaymentScreenEvents.Initialize) - state = applyEvent(state, SignalLoginPaymentScreenEvents.OptionSelected(SignalLoginPaymentState.Option.ExistingLogin)) + val viewModel = createViewModel() + viewModel.onEvent(SignalLoginPaymentScreenEvents.OptionSelected(SignalLoginPaymentState.Option.ExistingLogin)) - assertThat(state.isActionEnabled).isTrue() + assertThat(viewModel.state.value.isActionEnabled).isTrue() + } + + @Test + fun `a price lookup that is still loading does not block continuing with an existing login`() = runTest(testDispatcher) { + val price = CompletableDeferred() + coEvery { mockRepository.getSignalLoginPrice() } coAnswers { price.await() } + coEvery { mockRepository.hasUnredeemedSignalLoginPurchase() } returns false + val (events, emitter) = collectParentEvents() + val viewModel = createViewModel(emitter) + + viewModel.onEvent(SignalLoginPaymentScreenEvents.OptionSelected(SignalLoginPaymentState.Option.ExistingLogin)) + + assertThat(viewModel.state.value.price).isEqualTo(SignalLoginPaymentState.Price.Loading) + assertThat(viewModel.state.value.isActionEnabled).isTrue() + + viewModel.onEvent(SignalLoginPaymentScreenEvents.ContinueClicked) + + assertThat(events).containsExactly(RegistrationFlowEvent.NavigateToScreen(RegistrationRoute.SignalLoginCredentialEntry(), false)) + + price.complete(SignalLoginPriceResult.Available("$1.99")) + + assertThat(viewModel.state.value.price).isEqualTo(SignalLoginPaymentState.Price.Available("$1.99")) + assertThat(viewModel.state.value.selectedOption).isEqualTo(SignalLoginPaymentState.Option.ExistingLogin) + } + + @Test + fun `Foregrounded does not start a second lookup while one is in flight`() = runTest(testDispatcher) { + val price = CompletableDeferred() + coEvery { mockRepository.getSignalLoginPrice() } coAnswers { price.await() } + val viewModel = createViewModel() + + viewModel.onEvent(SignalLoginPaymentScreenEvents.Foregrounded) + + coVerify(exactly = 1) { mockRepository.getPaymentAvailability() } + + price.complete(SignalLoginPriceResult.Available("$1.99")) + + assertThat(viewModel.state.value.price).isEqualTo(SignalLoginPaymentState.Price.Available("$1.99")) + } + + @Test + fun `PriceRetryClicked replaces a lookup that is still in flight`() = runTest(testDispatcher) { + val stalePrice = CompletableDeferred() + var calls = 0 + coEvery { mockRepository.getSignalLoginPrice() } coAnswers { + calls++ + if (calls == 1) { + stalePrice.await() + } else { + SignalLoginPriceResult.Available("$2.99") + } + } + val viewModel = createViewModel() + + viewModel.onEvent(SignalLoginPaymentScreenEvents.PriceRetryClicked) + stalePrice.complete(SignalLoginPriceResult.Available("$1.99")) + + assertThat(viewModel.state.value.price).isEqualTo(SignalLoginPaymentState.Price.Available("$2.99")) } @Test @@ -318,7 +374,7 @@ class SignalLoginPaymentViewModelTest { coEvery { mockRepository.getSignalLoginPrice() } returns SignalLoginPriceResult.Available("$1.99") coEvery { mockRepository.hasUnredeemedSignalLoginPurchase() } returns true - val state = applyEvent(SignalLoginPaymentState(), SignalLoginPaymentScreenEvents.Initialize) + val state = createViewModel().state.value assertThat(state.hasUnredeemedPurchase).isTrue() }