Fix crash when switching between LPMs. (#4921)

stripe · May 2, 2022 · 98e3d70 · 98e3d70
1 parent a2b6760
commit 98e3d70
Show file tree

Hide file tree

Showing 5 changed files with 161 additions and 136 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,18 +9,18 @@
 
 ### PaymentSheet
 * [FIXED] [4918](https://github.com/stripe/stripe-android/pull/4918) Fix a problem introduced in 20.0.0 where save for future use was defaulted to true.
+* [FIXED] [4921](https://github.com/stripe/stripe-android/pull/4921) Fixed a crash that could happen when switching between LPMs.
 
 ## 20.2.0 - 2022-04-25
 This release adds card scanning to PaymentSheet.
 
 ### PaymentSheet
-* [FIXED] [4861](https://github.com/stripe/stripe-android/pull/4861) Remove font resource to save space and default to system default
 * [ADDED] [4804](https://github.com/stripe/stripe-android/pull/4804) Card-scanning in PaymentSheet
+* [FIXED] [4861](https://github.com/stripe/stripe-android/pull/4861) Remove font resource to save space and default to system default
+* [FIXED] [4909](https://github.com/stripe/stripe-android/pull/4909) In the multi-step flow when re-opening to a new card the form will pre-populate.  Also the default billing address will pre-populate in the form.
 
 ### Financial Connections
 * [CHANGED] [4887](https://github.com/stripe/stripe-android/pull/4887) Renamed Connections to Financial Connections.
-### PaymentSheet
-* [FIXED] [4909](https://github.com/stripe/stripe-android/pull/4909) In the multi-step flow when re-opening to a new card the form will pre-populate.  Also the default billing address will pre-populate in the form.
 
 ## 20.1.0 - 2022-04-18
 This release includes several Payments and PaymentSheet bug fixes.

diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/BaseAddPaymentMethodFragment.kt b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/BaseAddPaymentMethodFragment.kt
@@ -19,20 +19,15 @@ import androidx.lifecycle.asFlow
 import androidx.lifecycle.lifecycleScope
 import com.stripe.android.core.injection.InjectorKey
 import com.stripe.android.link.model.AccountStatus
-import com.stripe.android.model.CardBrand
-import com.stripe.android.model.PaymentMethod
 import com.stripe.android.model.StripeIntent
 import com.stripe.android.paymentsheet.databinding.FragmentPaymentsheetAddPaymentMethodBinding
-import com.stripe.android.paymentsheet.forms.FormFieldValues
 import com.stripe.android.paymentsheet.model.PaymentSelection
 import com.stripe.android.paymentsheet.model.SupportedPaymentMethod
 import com.stripe.android.paymentsheet.paymentdatacollection.ComposeFormDataCollectionFragment
 import com.stripe.android.paymentsheet.paymentdatacollection.FormFragmentArguments
-import com.stripe.android.paymentsheet.paymentdatacollection.TransformToPaymentMethodCreateParams
 import com.stripe.android.paymentsheet.ui.AnimationConstants
 import com.stripe.android.paymentsheet.viewmodels.BaseSheetViewModel
 import com.stripe.android.ui.core.Amount
-import com.stripe.android.ui.core.elements.IdentifierSpec
 import kotlinx.coroutines.launch
 
 internal abstract class BaseAddPaymentMethodFragment : Fragment() {
@@ -86,7 +81,6 @@ internal abstract class BaseAddPaymentMethodFragment : Fragment() {
 
         sheetViewModel.processing.observe(viewLifecycleOwner) { isProcessing ->
             viewBinding.linkInlineSignup.isEnabled = !isProcessing
-            (getFragment() as? ComposeFormDataCollectionFragment)?.setProcessing(isProcessing)
         }
 
         viewBinding.linkInlineSignup.apply {
@@ -105,39 +99,9 @@ internal abstract class BaseAddPaymentMethodFragment : Fragment() {
             }
         }
 
-        // If the activity was destroyed and recreated then we need to re-attach the fragment,
-        // as attach will not be called again.
-        childFragmentManager.fragments.forEach { fragment ->
-            attachComposeFragmentViewModel(fragment)
-        }
-
-        childFragmentManager.addFragmentOnAttachListener { _, fragment ->
-            attachComposeFragmentViewModel(fragment)
-        }
-
         sheetViewModel.eventReporter.onShowNewPaymentOptionForm()
     }
 
-    private fun attachComposeFragmentViewModel(fragment: Fragment) {
-        (fragment as? ComposeFormDataCollectionFragment)?.let { formFragment ->
-            // Need to access the formViewModel so it is constructed.
-            val formViewModel = formFragment.formViewModel
-            viewLifecycleOwner.lifecycleScope.launch {
-                formViewModel.completeFormValues.collect { formFieldValues ->
-                    // if the formFieldValues is a change either null or new values for the
-                    // newLpm then we should clear it out --- but what happens if we cancel -- selection should
-                    // have the correct value
-                    sheetViewModel.updateSelection(
-                        transformToPaymentSelection(
-                            formFieldValues,
-                            sheetViewModel.getAddFragmentSelectedLpmValue()
-                        )
-                    )
-                }
-            }
-        }
-    }
-
     private fun setupRecyclerView(
         viewBinding: FragmentPaymentsheetAddPaymentMethodBinding,
         paymentMethods: List<SupportedPaymentMethod>,
@@ -223,42 +187,6 @@ internal abstract class BaseAddPaymentMethodFragment : Fragment() {
         childFragmentManager.findFragmentById(R.id.payment_method_fragment_container)
 
     companion object {
-        private val transformToPaymentMethodCreateParams = TransformToPaymentMethodCreateParams()
-
-        @VisibleForTesting
-        internal fun transformToPaymentSelection(
-            formFieldValues: FormFieldValues?,
-            selectedPaymentMethodResources: SupportedPaymentMethod,
-        ) = formFieldValues?.let {
-            transformToPaymentMethodCreateParams.transform(
-                formFieldValues.copy(
-                    fieldValuePairs = it.fieldValuePairs
-                        .filterNot { entry ->
-                            entry.key == IdentifierSpec.SaveForFutureUse ||
-                                entry.key == IdentifierSpec.CardBrand
-                        }
-                ),
-                selectedPaymentMethodResources.type
-            ).run {
-                if (selectedPaymentMethodResources.type == PaymentMethod.Type.Card) {
-                    PaymentSelection.New.Card(
-                        paymentMethodCreateParams = this,
-                        brand = CardBrand.fromCode(
-                            formFieldValues.fieldValuePairs[IdentifierSpec.CardBrand]?.value
-                        ),
-                        customerRequestedSave = formFieldValues.userRequestedReuse
-
-                    )
-                } else {
-                    PaymentSelection.New.GenericPaymentMethod(
-                        selectedPaymentMethodResources.displayNameResource,
-                        selectedPaymentMethodResources.iconResource,
-                        this,
-                        customerRequestedSave = formFieldValues.userRequestedReuse
-                    )
-                }
-            }
-        }
 
         @VisibleForTesting
         fun getFormArguments(

diff --git a/...om/stripe/android/paymentsheet/paymentdatacollection/ComposeFormDataCollectionFragment.kt b/...om/stripe/android/paymentsheet/paymentdatacollection/ComposeFormDataCollectionFragment.kt
@@ -4,24 +4,41 @@ import android.os.Bundle
 import android.view.LayoutInflater
 import android.view.View
 import android.view.ViewGroup
+import androidx.annotation.VisibleForTesting
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.platform.ComposeView
 import androidx.fragment.app.Fragment
+import androidx.fragment.app.activityViewModels
 import androidx.fragment.app.viewModels
+import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.flowWithLifecycle
+import androidx.lifecycle.lifecycleScope
+import com.stripe.android.model.CardBrand
+import com.stripe.android.model.PaymentMethod
+import com.stripe.android.paymentsheet.PaymentOptionsActivity
+import com.stripe.android.paymentsheet.PaymentOptionsViewModel
+import com.stripe.android.paymentsheet.PaymentSheetActivity
+import com.stripe.android.paymentsheet.PaymentSheetViewModel
 import com.stripe.android.paymentsheet.forms.Form
+import com.stripe.android.paymentsheet.forms.FormFieldValues
 import com.stripe.android.paymentsheet.forms.FormViewModel
+import com.stripe.android.paymentsheet.model.PaymentSelection
 import com.stripe.android.paymentsheet.model.SupportedPaymentMethod
 import com.stripe.android.ui.core.PaymentsTheme
+import com.stripe.android.ui.core.elements.IdentifierSpec
 import kotlinx.coroutines.FlowPreview
+import kotlinx.coroutines.launch
 
 /**
  * Fragment that displays a form for payment data collection based on the [SupportedPaymentMethod]
  * received in the arguments bundle.
  */
 @OptIn(FlowPreview::class)
 internal class ComposeFormDataCollectionFragment : Fragment() {
+    private val transformToPaymentMethodCreateParams = TransformToPaymentMethodCreateParams()
+
     private val formLayout by lazy {
         requireNotNull(
             requireArguments().getParcelable<FormFragmentArguments>(EXTRA_CONFIG)
@@ -63,12 +80,81 @@ internal class ComposeFormDataCollectionFragment : Fragment() {
         }
     }
 
-    /**
-     * Informs the fragment whether PaymentSheet is in a processing state, so the fragment knows it
-     * should show its UI as enabled or disabled.
-     */
-    fun setProcessing(processing: Boolean) {
-        formViewModel.setEnabled(!processing)
+    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
+        super.onViewCreated(view, savedInstanceState)
+        val sheetViewModel by when (activity) {
+            is PaymentOptionsActivity -> {
+                activityViewModels<PaymentOptionsViewModel>()
+            }
+            is PaymentSheetActivity -> {
+                activityViewModels<PaymentSheetViewModel>()
+            }
+            else -> {
+                return
+            }
+        }
+
+        viewLifecycleOwner.lifecycleScope.launch {
+            // The block passed to repeatOnLifecycle is executed when the lifecycle
+            // is at least STARTED and is cancelled when the lifecycle is STOPPED.
+            // It automatically restarts the block when the lifecycle is STARTED again.
+            formViewModel.completeFormValues
+                .flowWithLifecycle(lifecycle, Lifecycle.State.STARTED)
+                .collect { formFieldValues ->
+                    // if the formFieldValues is a change either null or new values for the
+                    // newLpm then we should clear it out --- but what happens if we cancel -- selection should
+                    // have the correct value
+                    sheetViewModel.updateSelection(
+                        transformToPaymentSelection(
+                            formFieldValues,
+                            sheetViewModel.getAddFragmentSelectedLpmValue()
+                        )
+                    )
+                }
+        }
+
+        /**
+         * Informs the fragment whether PaymentSheet is in a processing state, so the fragment knows it
+         * should show its UI as enabled or disabled.
+         */
+        sheetViewModel.processing.observe(viewLifecycleOwner) { processing ->
+            formViewModel.setEnabled(!processing)
+        }
+    }
+
+    @VisibleForTesting
+    internal fun transformToPaymentSelection(
+        formFieldValues: FormFieldValues?,
+        selectedPaymentMethodResources: SupportedPaymentMethod,
+    ) = formFieldValues?.let {
+        transformToPaymentMethodCreateParams.transform(
+            formFieldValues.copy(
+                fieldValuePairs = it.fieldValuePairs
+                    .filterNot { entry ->
+                        entry.key == IdentifierSpec.SaveForFutureUse ||
+                            entry.key == IdentifierSpec.CardBrand
+                    }
+            ),
+            selectedPaymentMethodResources.type
+        ).run {
+            if (selectedPaymentMethodResources.type == PaymentMethod.Type.Card) {
+                PaymentSelection.New.Card(
+                    paymentMethodCreateParams = this,
+                    brand = CardBrand.fromCode(
+                        formFieldValues.fieldValuePairs[IdentifierSpec.CardBrand]?.value
+                    ),
+                    customerRequestedSave = formFieldValues.userRequestedReuse
+
+                )
+            } else {
+                PaymentSelection.New.GenericPaymentMethod(
+                    selectedPaymentMethodResources.displayNameResource,
+                    selectedPaymentMethodResources.iconResource,
+                    this,
+                    customerRequestedSave = formFieldValues.userRequestedReuse
+                )
+            }
+        }
     }
 
     internal companion object {

diff --git a/...src/test/java/com/stripe/android/paymentsheet/PaymentSheetAddPaymentMethodFragmentTest.kt b/...src/test/java/com/stripe/android/paymentsheet/PaymentSheetAddPaymentMethodFragmentTest.kt
@@ -26,16 +26,13 @@ import com.stripe.android.paymentsheet.PaymentSheetFixtures.COMPOSE_FRAGMENT_ARG
 import com.stripe.android.paymentsheet.PaymentSheetFixtures.CONFIG_MINIMUM
 import com.stripe.android.paymentsheet.PaymentSheetFixtures.MERCHANT_DISPLAY_NAME
 import com.stripe.android.paymentsheet.databinding.FragmentPaymentsheetAddPaymentMethodBinding
-import com.stripe.android.paymentsheet.forms.FormFieldValues
 import com.stripe.android.paymentsheet.model.FragmentConfig
 import com.stripe.android.paymentsheet.model.FragmentConfigFixtures
 import com.stripe.android.paymentsheet.model.PaymentSelection
 import com.stripe.android.paymentsheet.model.SupportedPaymentMethod
 import com.stripe.android.paymentsheet.paymentdatacollection.ComposeFormDataCollectionFragment
 import com.stripe.android.paymentsheet.paymentdatacollection.FormFragmentArguments
 import com.stripe.android.ui.core.Amount
-import com.stripe.android.ui.core.elements.IdentifierSpec
-import com.stripe.android.ui.core.forms.FormFieldEntry
 import com.stripe.android.utils.TestUtils.idleLooper
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.UnconfinedTestDispatcher
@@ -416,58 +413,6 @@ internal class PaymentSheetAddPaymentMethodFragmentTest : PaymentSheetViewModelT
         }
     }
 
-    @Test
-    fun `card payment method selection has the fields from formFieldValues`() {
-        val formFieldValues = FormFieldValues(
-            fieldValuePairs = mapOf(
-                IdentifierSpec.SaveForFutureUse to FormFieldEntry("true", true),
-                IdentifierSpec.CardNumber to FormFieldEntry("4242424242421234", true),
-                IdentifierSpec.CardBrand to FormFieldEntry(CardBrand.Visa.code, true)
-            ),
-            showsMandate = false,
-            userRequestedReuse = PaymentSelection.CustomerRequestedSave.RequestReuse
-        )
-        val selection =
-            BaseAddPaymentMethodFragment.transformToPaymentSelection(
-                formFieldValues,
-                SupportedPaymentMethod.Card
-            )
-        assertThat(selection?.customerRequestedSave).isEqualTo(
-            PaymentSelection.CustomerRequestedSave.RequestReuse
-        )
-        assertThat((selection as? PaymentSelection.New.Card)?.last4).isEqualTo(
-            "1234"
-        )
-        assertThat((selection as? PaymentSelection.New.Card)?.brand).isEqualTo(
-            CardBrand.Visa
-        )
-    }
-
-    @Test
-    fun `payment method selection has the fields from formFieldValues`() {
-        val formFieldValues = FormFieldValues(
-            fieldValuePairs = mapOf(
-                IdentifierSpec.SaveForFutureUse to FormFieldEntry("true", true)
-            ),
-            showsMandate = false,
-            userRequestedReuse = PaymentSelection.CustomerRequestedSave.RequestReuse
-        )
-        val selection =
-            BaseAddPaymentMethodFragment.transformToPaymentSelection(
-                formFieldValues,
-                SupportedPaymentMethod.Sofort
-            )
-        assertThat(selection?.customerRequestedSave).isEqualTo(
-            PaymentSelection.CustomerRequestedSave.RequestReuse
-        )
-        assertThat((selection as? PaymentSelection.New.GenericPaymentMethod)?.labelResource).isEqualTo(
-            R.string.stripe_paymentsheet_payment_method_sofort
-        )
-        assertThat((selection as? PaymentSelection.New.GenericPaymentMethod)?.iconResource).isEqualTo(
-            R.drawable.stripe_ic_paymentsheet_pm_klarna
-        )
-    }
-
     @Test
     fun `Factory gets initialized by Injector when Injector is available`() {
         createFragment(registerInjector = true) { fragment, _, viewModel ->