Skip to content

Fix issue with "Change email" behavior in Link #10706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import com.stripe.android.link.injection.NativeLinkComponent
import com.stripe.android.link.model.AccountStatus
import com.stripe.android.link.model.LinkAccount
import com.stripe.android.link.ui.LinkAppBarState
import com.stripe.android.link.ui.signup.SignUpViewModel
import com.stripe.android.link.ui.signup.SignUpViewModel.Companion.DID_SELECT_TO_CHANGE_EMAIL
import com.stripe.android.link.utils.LINK_DEFAULT_ANIMATION_DELAY_MILLIS
import com.stripe.android.paymentelement.confirmation.ConfirmationHandler
import com.stripe.android.paymentsheet.analytics.EventReporter
Expand Down Expand Up @@ -64,6 +64,9 @@ internal class LinkActivityViewModel @Inject constructor(
private val _linkScreenState = MutableStateFlow<ScreenState>(ScreenState.Loading)
val linkScreenState: StateFlow<ScreenState> = _linkScreenState

private val didChangeEmail: Boolean
get() = savedStateHandle.get<Boolean>(DID_SELECT_TO_CHANGE_EMAIL) == true

val linkAccount: LinkAccount?
get() = linkAccountManager.linkAccount.value

Expand Down Expand Up @@ -131,7 +134,11 @@ internal class LinkActivityViewModel @Inject constructor(
fun handleBackPressed() {
dismissWithResult?.invoke(
LinkActivityResult.Canceled(
linkAccountUpdate = linkAccountManager.linkAccountUpdate
linkAccountUpdate = if (didChangeEmail) {
LinkAccountUpdate.Value(null)
} else {
LinkAccountUpdate.Value(linkAccount)
},
)
)
}
Expand All @@ -157,7 +164,7 @@ internal class LinkActivityViewModel @Inject constructor(
}

fun changeEmail() {
savedStateHandle[SignUpViewModel.USE_LINK_CONFIGURATION_CUSTOMER_INFO] = false
savedStateHandle[DID_SELECT_TO_CHANGE_EMAIL] = true
navigate(LinkScreen.SignUp, clearStack = true)
}

Expand Down Expand Up @@ -248,7 +255,7 @@ internal class LinkActivityViewModel @Inject constructor(

DaggerNativeLinkComponent
.builder()
.configuration(args.configuration)
.configuration(args.configurationWithUpdatedCustomerInfo)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be weird that this happens here, but if we don't update the configuration, then this code will fetch the Link account associated with that email address.

.publishableKeyProvider { args.publishableKey }
.stripeAccountIdProvider { args.stripeAccountId }
.paymentElementCallbackIdentifier(args.paymentElementCallbackIdentifier)
Expand All @@ -271,3 +278,16 @@ internal sealed interface ScreenState {
}

internal class NoArgsException : IllegalArgumentException("NativeLinkArgs not found")

private val NativeLinkArgs.configurationWithUpdatedCustomerInfo: LinkConfiguration
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - I think this is more understandable as an extension function of LinkConfiguration, so that it reads:

args.configuration.withUpdatedCustomerInfo(args.LinkAccount) - might be a bit more verbose on the other hand so leave it as is if it makes sense!

// The user might have logged out on a previous Link session. We clear the customer information
// to avoid us defaulting back to the account that they previously logged out of.
get() {
val clearCustomerDetails = linkAccount == null
val effectiveCustomerInfo = if (clearCustomerDetails) {
LinkConfiguration.CustomerInfo(null, null, null, null)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of only setting the email address to null and keeping the other values as-is, but Web seems to fully clear the customer info for the same flow.

} else {
configuration.customerInfo
}
return configuration.copy(customerInfo = effectiveCustomerInfo)
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,12 @@ internal class SignUpViewModel @Inject constructor(
private val navigateAndClearStack: (LinkScreen) -> Unit,
private val moveToWeb: () -> Unit
) : ViewModel() {
private val useLinkConfigurationCustomerInfo =
savedStateHandle.get<Boolean>(USE_LINK_CONFIGURATION_CUSTOMER_INFO) ?: true
private val customerInfo = configuration.customerInfo.takeIf { useLinkConfigurationCustomerInfo }

private val didSelectToChangeEmail: Boolean
get() = savedStateHandle.get<Boolean>(DID_SELECT_TO_CHANGE_EMAIL) == true

private val customerInfo: LinkConfiguration.CustomerInfo?
get() = configuration.customerInfo.takeUnless { didSelectToChangeEmail }

val emailController = EmailConfig.createController(
initialValue = customerInfo?.email
Expand Down Expand Up @@ -240,7 +243,7 @@ internal class SignUpViewModel @Inject constructor(
companion object {
// How long to wait before triggering a call to lookup the email
internal val LOOKUP_DEBOUNCE = 1.seconds
internal const val USE_LINK_CONFIGURATION_CUSTOMER_INFO = "use_link_configuration_customer_info"
internal const val DID_SELECT_TO_CHANGE_EMAIL = "did_select_to_change_email"

fun factory(
parentComponent: NativeLinkComponent,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,33 @@ internal class LinkActivityViewModelTest {
}

@Test
fun `initializer creates ViewModel when args are valid`() {
val mockArgs = NativeLinkArgs(
configuration = mock(),
fun `initializer creates ViewModel when args are valid and a Link account is passed`() {
val configuration = TestFactory.LINK_CONFIGURATION
val args = NativeLinkArgs(
configuration = configuration,
publishableKey = "",
stripeAccountId = null,
startWithVerificationDialog = false,
linkAccount = TestFactory.LINK_ACCOUNT,
paymentElementCallbackIdentifier = "LinkNativeTestIdentifier",
)
val savedStateHandle = SavedStateHandle()
val factory = LinkActivityViewModel.factory(savedStateHandle)
savedStateHandle[LinkActivity.EXTRA_ARGS] = args

val viewModel = factory.create(LinkActivityViewModel::class.java, creationExtras())
assertThat(viewModel.activityRetainedComponent.configuration).isEqualTo(configuration)
}

@Test
fun `initializer creates ViewModel when args are valid and no Link account is passed`() {
val configuration = TestFactory.LINK_CONFIGURATION
val expectedConfiguration = configuration.copy(
customerInfo = LinkConfiguration.CustomerInfo(null, null, null, null)
)

val args = NativeLinkArgs(
configuration = configuration,
publishableKey = "",
stripeAccountId = null,
startWithVerificationDialog = false,
Expand All @@ -145,10 +169,10 @@ internal class LinkActivityViewModelTest {
)
val savedStateHandle = SavedStateHandle()
val factory = LinkActivityViewModel.factory(savedStateHandle)
savedStateHandle[LinkActivity.EXTRA_ARGS] = mockArgs
savedStateHandle[LinkActivity.EXTRA_ARGS] = args

val viewModel = factory.create(LinkActivityViewModel::class.java, creationExtras())
assertThat(viewModel.activityRetainedComponent.configuration).isEqualTo(mockArgs.configuration)
assertThat(viewModel.activityRetainedComponent.configuration).isEqualTo(expectedConfiguration)
}

@Test
Expand Down Expand Up @@ -530,8 +554,7 @@ internal class LinkActivityViewModelTest {

viewModel.changeEmail()

assertThat(savedStateHandle.get<Boolean>(SignUpViewModel.USE_LINK_CONFIGURATION_CUSTOMER_INFO)).isFalse()

assertThat(savedStateHandle.get<Boolean>(SignUpViewModel.DID_SELECT_TO_CHANGE_EMAIL)).isTrue()
navigationManager.assertNavigatedTo(
route = LinkScreen.SignUp.route,
popUpTo = PopUpToBehavior.Start
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ internal class SignUpViewModelTest {
}

@Test
fun `When USE_LINK_CONFIGURATION_CUSTOMER_INFO is false, controllers should not be prefilled`() =
fun `When DID_SELECT_TO_CHANGE_EMAIL is true, controllers should not be prefilled`() =
runTest(dispatcher) {
testUseLinkConfigurationCustomerInfo(
useLinkConfigurationCustomerInfo = false,
testDidChangeEmail(
didChangeEmail = true,
expectedSignUpState = SignUpState.InputtingPrimaryField,
expectedEmail = "",
expectedPhoneNumber = "",
Expand All @@ -104,9 +104,9 @@ internal class SignUpViewModelTest {
}

@Test
fun `When USE_LINK_CONFIGURATION_CUSTOMER_INFO is true, controllers should be prefilled`() = runTest(dispatcher) {
testUseLinkConfigurationCustomerInfo(
useLinkConfigurationCustomerInfo = true,
fun `When DID_SELECT_TO_CHANGE_EMAIL is false, controllers should be prefilled`() = runTest(dispatcher) {
testDidChangeEmail(
didChangeEmail = false,
expectedSignUpState = SignUpState.InputtingRemainingFields,
expectedEmail = CUSTOMER_EMAIL,
expectedPhoneNumber = TestFactory.CUSTOMER_PHONE,
Expand All @@ -115,10 +115,10 @@ internal class SignUpViewModelTest {
}

@Test
fun `When USE_LINK_CONFIGURATION_CUSTOMER_INFO is not set, controllers should be prefilled`() =
fun `When DID_SELECT_TO_CHANGE_EMAIL is not set, controllers should be prefilled`() =
runTest(dispatcher) {
testUseLinkConfigurationCustomerInfo(
useLinkConfigurationCustomerInfo = null,
testDidChangeEmail(
didChangeEmail = null,
expectedSignUpState = SignUpState.InputtingRemainingFields,
expectedEmail = CUSTOMER_EMAIL,
expectedPhoneNumber = TestFactory.CUSTOMER_PHONE,
Expand Down Expand Up @@ -561,17 +561,17 @@ internal class SignUpViewModelTest {
onSignUpClick()
}

private fun testUseLinkConfigurationCustomerInfo(
useLinkConfigurationCustomerInfo: Boolean?,
private fun testDidChangeEmail(
didChangeEmail: Boolean?,
expectedSignUpState: SignUpState = SignUpState.InputtingRemainingFields,
expectedEmail: String = CUSTOMER_EMAIL,
expectedPhoneNumber: String = TestFactory.CUSTOMER_PHONE,
expectedName: String = TestFactory.CUSTOMER_NAME
) {
val savedStateHandle = SavedStateHandle()
.apply {
useLinkConfigurationCustomerInfo?.let {
set(SignUpViewModel.USE_LINK_CONFIGURATION_CUSTOMER_INFO, it)
didChangeEmail?.let {
set(SignUpViewModel.DID_SELECT_TO_CHANGE_EMAIL, it)
}
}
val viewModel = createViewModel(
Expand Down
Loading