Skip to content

Commit

Permalink
Address code review feedback
Browse files Browse the repository at this point in the history
Consolidate layers in `LinkSignupHandler`.
  • Loading branch information
tillh-stripe committed Aug 14, 2024
1 parent 067813a commit ecf6091
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -164,9 +166,14 @@ internal interface FinancialConnectionsSheetNativeModule {
@Provides
internal fun provideLinkSignupHandler(
isLinkWithStripe: IsLinkWithStripe,
linkSignupHandlerFactory: LinkSignupHandlerFactory,
linkSignupHandlerForInstantDebits: Provider<LinkSignupHandlerForInstantDebits>,
linkSignupHandlerForNetworking: Provider<LinkSignupHandlerForNetworking>,
): LinkSignupHandler {
return linkSignupHandlerFactory.create(isLinkWithStripe())
return if (isLinkWithStripe()) {
linkSignupHandlerForInstantDebits.get()
} else {
linkSignupHandlerForNetworking.get()
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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<PerformLinkSignupForNetworking>,
private val performLinkSignupForInstantDebits: Provider<PerformLinkSignupForInstantDebits>,
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 {

Expand All @@ -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,
Expand All @@ -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
}

Expand All @@ -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,
Expand All @@ -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
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -24,13 +27,16 @@ 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
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

Expand Down Expand Up @@ -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,
),
)

Expand Down Expand Up @@ -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,
)
)

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -436,21 +440,81 @@ class NetworkingLinkSignupViewModelTest {
}

private fun mockLinkSignupHandlerForNetworking(
performLinkSignup: PerformLinkSignup = PerformLinkSignup { Pane.SUCCESS },
failOnSignup: Boolean = false,
): LinkSignupHandler {
val manifest = sessionManifest().copy(
businessName = "Business",
accountholderCustomerEmailAddress = "[email protected]"
)

val getOrFetchSync = mock<GetOrFetchSync> {
onBlocking { invoke(any()) } doReturn syncResponse().copy(
manifest = manifest,
text = TextUpdate(
consent = null,
networkingLinkSignupPane = networkingLinkSignupPane(),
)
)
}

val getCachedAccounts = mock<GetCachedAccounts> {
onBlocking { invoke() } doReturn cachedPartnerAccounts()
}

val saveAccountToLink = mock<SaveAccountToLink> {
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(),
)
}

private fun mockLinkSignupHandlerForInstantDebits(
performLinkSignup: PerformLinkSignup = PerformLinkSignup { Pane.SUCCESS },
failOnSignup: Boolean = false,
): LinkSignupHandler {
val manifest = sessionManifest().copy(
businessName = "Business",
accountholderCustomerEmailAddress = "[email protected]"
)

val getOrFetchSync = mock<GetOrFetchSync> {
onBlocking { invoke(any()) } doReturn syncResponse().copy(
manifest = manifest,
text = TextUpdate(
consent = null,
linkLoginPane = linkLoginPane(),
)
)
}

val consumerRepository = mock<FinancialConnectionsConsumerSessionRepository> {
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(),
Expand Down

0 comments on commit ecf6091

Please sign in to comment.