From ecf609134e9b7ad1c890a0407d2369f69a6e7840 Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Wed, 14 Aug 2024 10:28:05 -0400 Subject: [PATCH] Address code review feedback Consolidate layers in `LinkSignupHandler`. --- .../FinancialConnectionsSheetNativeModule.kt | 13 ++- .../networkinglinksignup/LinkSignupHandler.kt | 73 ++++++++------- .../networkinglinksignup/PerformLinkSignup.kt | 63 ------------- .../NetworkingLinkSignupViewModelTest.kt | 90 ++++++++++++++++--- 4 files changed, 128 insertions(+), 111 deletions(-) delete mode 100644 financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkinglinksignup/PerformLinkSignup.kt diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/di/FinancialConnectionsSheetNativeModule.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/di/FinancialConnectionsSheetNativeModule.kt index db53eb34a4d..32805271f92 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/di/FinancialConnectionsSheetNativeModule.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/di/FinancialConnectionsSheetNativeModule.kt @@ -13,7 +13,8 @@ import com.stripe.android.financialconnections.domain.IsLinkWithStripe import com.stripe.android.financialconnections.domain.RealAttachConsumerToLinkAccountSession import com.stripe.android.financialconnections.domain.RealHandleError import com.stripe.android.financialconnections.features.networkinglinksignup.LinkSignupHandler -import com.stripe.android.financialconnections.features.networkinglinksignup.LinkSignupHandlerFactory +import com.stripe.android.financialconnections.features.networkinglinksignup.LinkSignupHandlerForInstantDebits +import com.stripe.android.financialconnections.features.networkinglinksignup.LinkSignupHandlerForNetworking import com.stripe.android.financialconnections.features.notice.PresentSheet import com.stripe.android.financialconnections.features.notice.RealPresentSheet import com.stripe.android.financialconnections.model.SynchronizeSessionResponse @@ -36,6 +37,7 @@ import dagger.Module import dagger.Provides import java.util.Locale import javax.inject.Named +import javax.inject.Provider import javax.inject.Singleton @Module @@ -164,9 +166,14 @@ internal interface FinancialConnectionsSheetNativeModule { @Provides internal fun provideLinkSignupHandler( isLinkWithStripe: IsLinkWithStripe, - linkSignupHandlerFactory: LinkSignupHandlerFactory, + linkSignupHandlerForInstantDebits: Provider, + linkSignupHandlerForNetworking: Provider, ): LinkSignupHandler { - return linkSignupHandlerFactory.create(isLinkWithStripe()) + return if (isLinkWithStripe()) { + linkSignupHandlerForInstantDebits.get() + } else { + linkSignupHandlerForNetworking.get() + } } } } diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkinglinksignup/LinkSignupHandler.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkinglinksignup/LinkSignupHandler.kt index 726f62ea123..e25d9d0268d 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkinglinksignup/LinkSignupHandler.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkinglinksignup/LinkSignupHandler.kt @@ -1,8 +1,15 @@ package com.stripe.android.financialconnections.features.networkinglinksignup import com.stripe.android.core.Logger +import com.stripe.android.financialconnections.analytics.FinancialConnectionsAnalyticsEvent.Click import com.stripe.android.financialconnections.analytics.FinancialConnectionsAnalyticsTracker import com.stripe.android.financialconnections.analytics.logError +import com.stripe.android.financialconnections.domain.AttachConsumerToLinkAccountSession +import com.stripe.android.financialconnections.domain.GetCachedAccounts +import com.stripe.android.financialconnections.domain.GetOrFetchSync +import com.stripe.android.financialconnections.domain.GetOrFetchSync.RefetchCondition.Always +import com.stripe.android.financialconnections.domain.SaveAccountToLink +import com.stripe.android.financialconnections.features.common.isDataFlow import com.stripe.android.financialconnections.model.FinancialConnectionsSessionManifest.Pane import com.stripe.android.financialconnections.model.FinancialConnectionsSessionManifest.Pane.LINK_ACCOUNT_PICKER import com.stripe.android.financialconnections.model.FinancialConnectionsSessionManifest.Pane.LINK_LOGIN @@ -11,35 +18,8 @@ import com.stripe.android.financialconnections.navigation.Destination.Networking import com.stripe.android.financialconnections.navigation.Destination.NetworkingSaveToLinkVerification import com.stripe.android.financialconnections.navigation.Destination.Success import com.stripe.android.financialconnections.navigation.NavigationManager +import com.stripe.android.financialconnections.repository.FinancialConnectionsConsumerSessionRepository import javax.inject.Inject -import javax.inject.Provider - -internal class LinkSignupHandlerFactory @Inject constructor( - private val performLinkSignupForNetworking: Provider, - private val performLinkSignupForInstantDebits: Provider, - private val navigationManager: NavigationManager, - private val eventTracker: FinancialConnectionsAnalyticsTracker, - private val logger: Logger, -) { - - fun create(isInstantDebits: Boolean): LinkSignupHandler { - return if (isInstantDebits) { - LinkSignupHandlerForInstantDebits( - performLinkSignup = performLinkSignupForInstantDebits.get(), - navigationManager = navigationManager, - eventTracker = eventTracker, - logger = logger, - ) - } else { - LinkSignupHandlerForNetworking( - performLinkSignup = performLinkSignupForNetworking.get(), - navigationManager = navigationManager, - eventTracker = eventTracker, - logger = logger, - ) - } - } -} internal interface LinkSignupHandler { @@ -56,7 +36,9 @@ internal interface LinkSignupHandler { } internal class LinkSignupHandlerForInstantDebits @Inject constructor( - private val performLinkSignup: PerformLinkSignup, + private val consumerRepository: FinancialConnectionsConsumerSessionRepository, + private val attachConsumerToLinkAccountSession: AttachConsumerToLinkAccountSession, + private val getOrFetchSync: GetOrFetchSync, private val navigationManager: NavigationManager, private val eventTracker: FinancialConnectionsAnalyticsTracker, private val logger: Logger, @@ -65,7 +47,20 @@ internal class LinkSignupHandlerForInstantDebits @Inject constructor( override suspend fun performSignup( state: NetworkingLinkSignupState, ): Pane { - performLinkSignup(state) + val phoneController = state.payload()!!.phoneController + + val signup = consumerRepository.signUp( + email = state.validEmail!!, + phoneNumber = phoneController.getE164PhoneNumber(state.validPhone!!), + country = phoneController.getCountryCode(), + ) + + attachConsumerToLinkAccountSession( + consumerSessionClientSecret = signup.consumerSession.clientSecret, + ) + + getOrFetchSync(refetchCondition = Always) + return LINK_ACCOUNT_PICKER } @@ -91,7 +86,9 @@ internal class LinkSignupHandlerForInstantDebits @Inject constructor( } internal class LinkSignupHandlerForNetworking @Inject constructor( - private val performLinkSignup: PerformLinkSignup, + private val getOrFetchSync: GetOrFetchSync, + private val getCachedAccounts: GetCachedAccounts, + private val saveAccountToLink: SaveAccountToLink, private val eventTracker: FinancialConnectionsAnalyticsTracker, private val navigationManager: NavigationManager, private val logger: Logger, @@ -100,7 +97,19 @@ internal class LinkSignupHandlerForNetworking @Inject constructor( override suspend fun performSignup( state: NetworkingLinkSignupState, ): Pane { - performLinkSignup(state) + eventTracker.track(Click(eventName = "click.save_to_link", pane = NETWORKING_LINK_SIGNUP_PANE)) + val selectedAccounts = getCachedAccounts() + val manifest = getOrFetchSync().manifest + val phoneController = state.payload()!!.phoneController + require(state.valid) { "Form invalid! ${state.validEmail} ${state.validPhone}" } + saveAccountToLink.new( + country = phoneController.getCountryCode(), + email = state.validEmail!!, + phoneNumber = phoneController.getE164PhoneNumber(state.validPhone!!), + selectedAccounts = selectedAccounts, + shouldPollAccountNumbers = manifest.isDataFlow, + ) + return Pane.SUCCESS } diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkinglinksignup/PerformLinkSignup.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkinglinksignup/PerformLinkSignup.kt deleted file mode 100644 index fd5de11ac89..00000000000 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkinglinksignup/PerformLinkSignup.kt +++ /dev/null @@ -1,63 +0,0 @@ -package com.stripe.android.financialconnections.features.networkinglinksignup - -import com.stripe.android.financialconnections.analytics.FinancialConnectionsAnalyticsEvent.Click -import com.stripe.android.financialconnections.analytics.FinancialConnectionsAnalyticsTracker -import com.stripe.android.financialconnections.domain.AttachConsumerToLinkAccountSession -import com.stripe.android.financialconnections.domain.GetCachedAccounts -import com.stripe.android.financialconnections.domain.GetOrFetchSync -import com.stripe.android.financialconnections.domain.GetOrFetchSync.RefetchCondition.Always -import com.stripe.android.financialconnections.domain.SaveAccountToLink -import com.stripe.android.financialconnections.features.common.isDataFlow -import com.stripe.android.financialconnections.model.FinancialConnectionsSessionManifest.Pane.NETWORKING_LINK_SIGNUP_PANE -import com.stripe.android.financialconnections.repository.FinancialConnectionsConsumerSessionRepository -import javax.inject.Inject - -internal fun interface PerformLinkSignup { - suspend operator fun invoke(state: NetworkingLinkSignupState) -} - -internal class PerformLinkSignupForInstantDebits @Inject constructor( - private val consumerRepository: FinancialConnectionsConsumerSessionRepository, - private val attachConsumerToLinkAccountSession: AttachConsumerToLinkAccountSession, - private val getOrFetchSync: GetOrFetchSync, -) : PerformLinkSignup { - - override suspend fun invoke(state: NetworkingLinkSignupState) { - val phoneController = state.payload()!!.phoneController - - val signup = consumerRepository.signUp( - email = state.validEmail!!, - phoneNumber = phoneController.getE164PhoneNumber(state.validPhone!!), - country = phoneController.getCountryCode(), - ) - - attachConsumerToLinkAccountSession( - consumerSessionClientSecret = signup.consumerSession.clientSecret, - ) - - getOrFetchSync(refetchCondition = Always) - } -} - -internal class PerformLinkSignupForNetworking @Inject constructor( - private val eventTracker: FinancialConnectionsAnalyticsTracker, - private val getOrFetchSync: GetOrFetchSync, - private val getCachedAccounts: GetCachedAccounts, - private val saveAccountToLink: SaveAccountToLink, -) : PerformLinkSignup { - - override suspend fun invoke(state: NetworkingLinkSignupState) { - eventTracker.track(Click(eventName = "click.save_to_link", pane = NETWORKING_LINK_SIGNUP_PANE)) - val selectedAccounts = getCachedAccounts() - val manifest = getOrFetchSync().manifest - val phoneController = state.payload()!!.phoneController - require(state.valid) { "Form invalid! ${state.validEmail} ${state.validPhone}" } - saveAccountToLink.new( - country = phoneController.getCountryCode(), - email = state.validEmail!!, - phoneNumber = phoneController.getE164PhoneNumber(state.validPhone!!), - selectedAccounts = selectedAccounts, - shouldPollAccountNumbers = manifest.isDataFlow, - ) - } -} diff --git a/financial-connections/src/test/java/com/stripe/android/financialconnections/features/networkinglinksignup/NetworkingLinkSignupViewModelTest.kt b/financial-connections/src/test/java/com/stripe/android/financialconnections/features/networkinglinksignup/NetworkingLinkSignupViewModelTest.kt index 4e83b681186..8738c4cb1c8 100644 --- a/financial-connections/src/test/java/com/stripe/android/financialconnections/features/networkinglinksignup/NetworkingLinkSignupViewModelTest.kt +++ b/financial-connections/src/test/java/com/stripe/android/financialconnections/features/networkinglinksignup/NetworkingLinkSignupViewModelTest.kt @@ -4,15 +4,18 @@ import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import com.stripe.android.core.Logger import com.stripe.android.core.exception.APIConnectionException +import com.stripe.android.financialconnections.ApiKeyFixtures.cachedPartnerAccounts +import com.stripe.android.financialconnections.ApiKeyFixtures.consumerSessionSignup import com.stripe.android.financialconnections.ApiKeyFixtures.sessionManifest import com.stripe.android.financialconnections.ApiKeyFixtures.syncResponse import com.stripe.android.financialconnections.CoroutineTestRule import com.stripe.android.financialconnections.TestFinancialConnectionsAnalyticsTracker import com.stripe.android.financialconnections.analytics.FinancialConnectionsAnalyticsEvent.ConsentAgree.analyticsValue +import com.stripe.android.financialconnections.domain.GetCachedAccounts import com.stripe.android.financialconnections.domain.GetOrFetchSync import com.stripe.android.financialconnections.domain.LookupAccount import com.stripe.android.financialconnections.domain.NativeAuthFlowCoordinator -import com.stripe.android.financialconnections.model.FinancialConnectionsSessionManifest.Pane +import com.stripe.android.financialconnections.domain.SaveAccountToLink import com.stripe.android.financialconnections.model.FinancialConnectionsSessionManifest.Pane.LINK_LOGIN import com.stripe.android.financialconnections.model.FinancialConnectionsSessionManifest.Pane.NETWORKING_LINK_SIGNUP_PANE import com.stripe.android.financialconnections.model.LinkLoginPane @@ -24,6 +27,7 @@ import com.stripe.android.financialconnections.navigation.Destination.Networking import com.stripe.android.financialconnections.navigation.Destination.NetworkingSaveToLinkVerification import com.stripe.android.financialconnections.navigation.NavigationIntent import com.stripe.android.financialconnections.navigation.NavigationManagerImpl +import com.stripe.android.financialconnections.repository.FinancialConnectionsConsumerSessionRepository import com.stripe.android.financialconnections.utils.UriUtils import com.stripe.android.model.ConsumerSessionLookup import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -31,6 +35,8 @@ import kotlinx.coroutines.test.runTest import org.junit.Rule import org.junit.Test import org.mockito.kotlin.any +import org.mockito.kotlin.doAnswer +import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock import org.mockito.kotlin.whenever @@ -185,7 +191,8 @@ class NetworkingLinkSignupViewModelTest { val viewModel = buildViewModel( state = NetworkingLinkSignupState(isInstantDebits = true), signupHandler = mockLinkSignupHandlerForInstantDebits( - performLinkSignup = { error("Not expected to call sign-up") }, + // We don't expect a signup to be performed + failOnSignup = true, ), ) @@ -222,7 +229,8 @@ class NetworkingLinkSignupViewModelTest { val viewModel = buildViewModel( state = NetworkingLinkSignupState(), signupHandler = mockLinkSignupHandlerForNetworking( - performLinkSignup = { error("Not expected to call sign-up") }, + // We don't expect a signup to be performed + failOnSignup = true, ) ) @@ -382,9 +390,7 @@ class NetworkingLinkSignupViewModelTest { val viewModel = buildViewModel( state = NetworkingLinkSignupState(isInstantDebits = false), - signupHandler = mockLinkSignupHandlerForNetworking( - performLinkSignup = { throw APIConnectionException() }, - ), + signupHandler = mockLinkSignupHandlerForNetworking(failOnSignup = true), ) navigationManager.navigationFlow.test { @@ -418,9 +424,7 @@ class NetworkingLinkSignupViewModelTest { val viewModel = buildViewModel( state = NetworkingLinkSignupState(isInstantDebits = true), - signupHandler = mockLinkSignupHandlerForInstantDebits( - performLinkSignup = { throw APIConnectionException() }, - ) + signupHandler = mockLinkSignupHandlerForInstantDebits(failOnSignup = true), ) navigationManager.navigationFlow.test { @@ -436,10 +440,41 @@ class NetworkingLinkSignupViewModelTest { } private fun mockLinkSignupHandlerForNetworking( - performLinkSignup: PerformLinkSignup = PerformLinkSignup { Pane.SUCCESS }, + failOnSignup: Boolean = false, ): LinkSignupHandler { + val manifest = sessionManifest().copy( + businessName = "Business", + accountholderCustomerEmailAddress = "test@test.com" + ) + + val getOrFetchSync = mock { + onBlocking { invoke(any()) } doReturn syncResponse().copy( + manifest = manifest, + text = TextUpdate( + consent = null, + networkingLinkSignupPane = networkingLinkSignupPane(), + ) + ) + } + + val getCachedAccounts = mock { + onBlocking { invoke() } doReturn cachedPartnerAccounts() + } + + val saveAccountToLink = mock { + if (failOnSignup) { + onBlocking { new(any(), any(), any(), any(), any()) } doAnswer { + throw APIConnectionException() + } + } else { + onBlocking { new(any(), any(), any(), any(), any()) } doReturn manifest + } + } + return LinkSignupHandlerForNetworking( - performLinkSignup = performLinkSignup, + getOrFetchSync = getOrFetchSync, + getCachedAccounts = getCachedAccounts, + saveAccountToLink = saveAccountToLink, eventTracker = eventTracker, navigationManager = navigationManager, logger = Logger.noop(), @@ -447,10 +482,39 @@ class NetworkingLinkSignupViewModelTest { } private fun mockLinkSignupHandlerForInstantDebits( - performLinkSignup: PerformLinkSignup = PerformLinkSignup { Pane.SUCCESS }, + failOnSignup: Boolean = false, ): LinkSignupHandler { + val manifest = sessionManifest().copy( + businessName = "Business", + accountholderCustomerEmailAddress = "test@test.com" + ) + + val getOrFetchSync = mock { + onBlocking { invoke(any()) } doReturn syncResponse().copy( + manifest = manifest, + text = TextUpdate( + consent = null, + linkLoginPane = linkLoginPane(), + ) + ) + } + + val consumerRepository = mock { + if (failOnSignup) { + onBlocking { signUp(any(), any(), any()) } doAnswer { + throw APIConnectionException() + } + } else { + onBlocking { signUp(any(), any(), any()) } doReturn consumerSessionSignup() + } + } + return LinkSignupHandlerForInstantDebits( - performLinkSignup = performLinkSignup, + getOrFetchSync = getOrFetchSync, + consumerRepository = consumerRepository, + attachConsumerToLinkAccountSession = { + // Mock a successful attach + }, eventTracker = eventTracker, navigationManager = navigationManager, logger = Logger.noop(),