Move navigation back on card brand success logic into update PM interactor

paymentsheet/src/main/java/com/stripe/android/customersheet/CustomerSheetViewModel.kt

@@ -512,7 +512,6 @@ internal class CustomerSheetViewModel(
                 productUsageTokens = setOf("CustomerSheet"),
             )
         ).onSuccess { updatedMethod ->
-            onBackPressed()
             updatePaymentMethodInState(updatedMethod)
 
             eventReporter.onUpdatePaymentMethodSucceeded(
@@ -556,6 +555,7 @@ internal class CustomerSheetViewModel(
                             selectedBrand = it
                         )
                     },
+                    onUpdateSuccess = ::onBackPressed,
                     updateCardBrandExecutor = ::updateCardBrandExecutor,
                     workContext = workContext,
                     // This checkbox is never displayed in CustomerSheet.

paymentsheet/src/main/java/com/stripe/android/paymentelement/embedded/content/EmbeddedContentHelper.kt

@@ -212,8 +212,6 @@ internal class DefaultEmbeddedContentHelper @Inject constructor(
                     selection = selectionHolder.selection.value,
                 )
             },
-            navigationPop = {
-            },
             isLinkEnabled = stateFlowOf(paymentMethodMetadata.linkState != null),
             isNotPaymentFlow = false,
         )

paymentsheet/src/main/java/com/stripe/android/paymentelement/embedded/manage/EmbeddedUpdateScreenInteractorFactory.kt

@@ -24,6 +24,7 @@ internal class DefaultEmbeddedUpdateScreenInteractorFactory @Inject constructor(
     private val customerStateHolder: CustomerStateHolder,
     private val selectionHolder: EmbeddedSelectionHolder,
     private val eventReporter: EventReporter,
+    private val manageNavigatorProvider: Provider<ManageNavigator>,
 ) : EmbeddedUpdateScreenInteractorFactory {
     override fun createUpdateScreenInteractor(
         displayableSavedPaymentMethod: DisplayableSavedPaymentMethod
@@ -73,6 +74,9 @@ internal class DefaultEmbeddedUpdateScreenInteractorFactory @Inject constructor(
                         defaultPaymentMethodId = customerStateHolder.customer.value?.defaultPaymentMethodId
                     )
                 ),
+            onUpdateSuccess = {
+                manageNavigatorProvider.get().performAction(ManageNavigator.Action.Back)
+            },
         )
     }
 }

paymentsheet/src/main/java/com/stripe/android/paymentelement/embedded/manage/ManageSavedPaymentMethodMutatorFactory.kt

@@ -57,9 +57,6 @@ internal class ManageSavedPaymentMethodMutatorFactory @Inject constructor(
             onUpdatePaymentMethod = { displayableSavedPaymentMethod, _, _, _ ->
                 onUpdatePaymentMethod(displayableSavedPaymentMethod)
             },
-            navigationPop = {
-                manageNavigatorProvider.get().performAction(ManageNavigator.Action.Back)
-            },
             isLinkEnabled = stateFlowOf(false), // Link is never enabled in the manage screen.
             isNotPaymentFlow = false,
         )

paymentsheet/src/main/java/com/stripe/android/paymentsheet/SavedPaymentMethodMutator.kt

@@ -55,7 +55,6 @@ internal class SavedPaymentMethodMutator(
         performRemove: suspend () -> Throwable?,
         updateExecutor: suspend (brand: CardBrand) -> Result<PaymentMethod>,
     ) -> Unit,
-    private val navigationPop: () -> Unit,
     isLinkEnabled: StateFlow<Boolean?>,
     isNotPaymentFlow: Boolean,
 ) {
@@ -265,8 +264,6 @@ internal class SavedPaymentMethodMutator(
                 )
 
                 onSuccess(updatedMethod)
-
-                navigationPop()
             }
 
             eventReporter.onUpdatePaymentMethodSucceeded(
@@ -363,6 +360,7 @@ internal class SavedPaymentMethodMutator(
                                         viewModel.customerStateHolder.customer.value?.defaultPaymentMethodId
                                     )
                                 ),
+                            onUpdateSuccess = viewModel.navigationHandler::pop,
                         )
                     )
                 )
@@ -396,7 +394,6 @@ internal class SavedPaymentMethodMutator(
                         updateCardBrandExecutor = updateCardBrandExecutor,
                     )
                 },
-                navigationPop = viewModel.navigationHandler::pop,
                 isLinkEnabled = viewModel.linkHandler.isLinkEnabled,
                 isNotPaymentFlow = !viewModel.isCompleteFlow,
             ).apply {

paymentsheet/src/main/java/com/stripe/android/paymentsheet/ui/UpdatePaymentMethodInteractor.kt

@@ -87,6 +87,7 @@ internal class DefaultUpdatePaymentMethodInteractor(
     private val updateCardBrandExecutor: UpdateCardBrandOperation,
     private val onBrandChoiceOptionsShown: (CardBrand) -> Unit,
     private val onBrandChoiceOptionsDismissed: (CardBrand) -> Unit,
+    private val onUpdateSuccess: () -> Unit,
     workContext: CoroutineContext = Dispatchers.Default,
 ) : UpdatePaymentMethodInteractor {
     private val coroutineScope = CoroutineScope(workContext + SupervisorJob())
@@ -171,12 +172,14 @@ internal class DefaultUpdatePaymentMethodInteractor(
 
             val updateCardBrandResult = maybeUpdateCardBrand()
 
-            val errorMessage = getErrorMessageForUpdates(
+            val updateResult = getUpdateResult(
                 updateCardBrandResult = updateCardBrandResult,
             )
 
-            if (errorMessage != null) {
-                error.emit(errorMessage)
+            when (updateResult) {
+                is UpdateResult.Error -> error.emit(updateResult.errorMessage)
+                UpdateResult.Success -> onUpdateSuccess()
+                UpdateResult.NoUpdatesMade -> {}
             }
 
             status.emit(UpdatePaymentMethodInteractor.Status.Idle)
@@ -198,12 +201,16 @@ internal class DefaultUpdatePaymentMethodInteractor(
         }
     }
 
-    private fun getErrorMessageForUpdates(
+    private fun getUpdateResult(
         updateCardBrandResult: Result<PaymentMethod>?,
-    ): ResolvableString? {
+    ): UpdateResult {
         return when {
-            updateCardBrandResult?.isFailure == true -> updateCardBrandResult.exceptionOrNull()?.stripeErrorMessage()
-            else -> null
+            updateCardBrandResult == null -> UpdateResult.NoUpdatesMade
+            updateCardBrandResult.isFailure -> UpdateResult.Error(
+                updateCardBrandResult.exceptionOrNull()?.stripeErrorMessage()
+            )
+            updateCardBrandResult.isSuccess -> UpdateResult.Success
+            else -> UpdateResult.NoUpdatesMade
         }
     }
 
@@ -243,6 +250,12 @@ internal class DefaultUpdatePaymentMethodInteractor(
         }?.filter { cardBrandFilter.isAccepted(it) }
         return (filteredCardBrands?.size ?: 0) > 1
     }
+
+    sealed class UpdateResult {
+        data class Error(val errorMessage: ResolvableString?) : UpdateResult()
+        data object Success : UpdateResult()
+        data object NoUpdatesMade : UpdateResult()
+    }
 }
 
 internal const val PaymentMethodRemovalDelayMillis = 600L

paymentsheet/src/main/java/com/stripe/android/paymentsheet/ui/UpdatePaymentMethodUI.kt

@@ -534,6 +534,7 @@ private fun PreviewUpdatePaymentMethodUI() {
             onBrandChoiceOptionsShown = {},
             onBrandChoiceOptionsDismissed = {},
             shouldShowSetAsDefaultCheckbox = true,
+            onUpdateSuccess = {},
         ),
         modifier = Modifier
     )

paymentsheet/src/test/java/com/stripe/android/customersheet/CustomerSheetScreenshotTest.kt

@@ -357,6 +357,7 @@ internal class CustomerSheetScreenshotTest {
                 onBrandChoiceOptionsShown = {},
                 // This checkbox is never displayed in CustomerSheet.
                 shouldShowSetAsDefaultCheckbox = false,
+                onUpdateSuccess = {},
             ),
             isLiveMode = true,
         )

paymentsheet/src/test/java/com/stripe/android/paymentsheet/SavedPaymentMethodMutatorTest.kt

@@ -408,7 +408,6 @@ class SavedPaymentMethodMutatorTest {
             updatePaymentMethodTurbine.awaitItem().updateExecutor(CardBrand.CartesBancaires)
 
             assertThat(calledUpdate.awaitItem()).isTrue()
-            assertThat(navigationPopTurbine.awaitItem()).isNotNull()
 
             val paymentMethods = customerStateHolder.paymentMethods.value
             assertThat(paymentMethods).hasSize(1)
@@ -450,7 +449,6 @@ class SavedPaymentMethodMutatorTest {
             )
 
             assertThat(calledUpdate.awaitItem()).isTrue()
-            assertThat(navigationPopTurbine.awaitItem()).isNotNull()
 
             val paymentMethods = customerStateHolder.paymentMethods.value
             assertThat(paymentMethods).hasSize(1)
@@ -552,7 +550,6 @@ class SavedPaymentMethodMutatorTest {
             val postPaymentMethodRemovedTurbine = Turbine<Unit>()
             val prePaymentMethodRemovedTurbine = Turbine<Unit>()
             val updatePaymentMethodTurbine = Turbine<UpdateCall>()
-            val navigationPopTurbine = Turbine<Unit>()
 
             val savedPaymentMethodMutator = SavedPaymentMethodMutator(
                 paymentMethodMetadataFlow = stateFlowOf(
@@ -579,7 +576,6 @@ class SavedPaymentMethodMutatorTest {
                         UpdateCall(displayableSavedPaymentMethod, canRemove, performRemove, updateExecutor)
                     )
                 },
-                navigationPop = { navigationPopTurbine.add(Unit) },
                 isLinkEnabled = stateFlowOf(false),
                 isNotPaymentFlow = true,
             )
@@ -591,7 +587,6 @@ class SavedPaymentMethodMutatorTest {
                 prePaymentMethodRemovedTurbine = prePaymentMethodRemovedTurbine,
                 postPaymentMethodRemovedTurbine = postPaymentMethodRemovedTurbine,
                 updatePaymentMethodTurbine = updatePaymentMethodTurbine,
-                navigationPopTurbine = navigationPopTurbine,
                 testScope = this,
             ).apply {
                 block()
@@ -601,7 +596,6 @@ class SavedPaymentMethodMutatorTest {
 
             postPaymentMethodRemovedTurbine.ensureAllEventsConsumed()
             updatePaymentMethodTurbine.ensureAllEventsConsumed()
-            navigationPopTurbine.ensureAllEventsConsumed()
         }
     }
 
@@ -613,7 +607,6 @@ class SavedPaymentMethodMutatorTest {
         val prePaymentMethodRemovedTurbine: ReceiveTurbine<Unit>,
         val postPaymentMethodRemovedTurbine: ReceiveTurbine<Unit>,
         val updatePaymentMethodTurbine: ReceiveTurbine<UpdateCall>,
-        val navigationPopTurbine: ReceiveTurbine<Unit>,
         val testScope: TestScope,
     )
 

paymentsheet/src/test/java/com/stripe/android/paymentsheet/ui/DefaultUpdatePaymentMethodInteractorTest.kt

@@ -159,6 +159,38 @@ class DefaultUpdatePaymentMethodInteractorTest {
         }
     }
 
+    @Test
+    fun updatingCardBrand_callsOnUpdateSuccess() {
+        var updateSuccessCalled = false
+        fun onUpdateSuccess() {
+            updateSuccessCalled = true
+        }
+
+        runScenario(
+            displayableSavedPaymentMethod =
+            PaymentMethodFixtures.CARD_WITH_NETWORKS_PAYMENT_METHOD.toDisplayableSavedPaymentMethod(),
+            onUpdateCardBrand = { paymentMethod, _ -> Result.success(paymentMethod) },
+            onUpdateSuccess = ::onUpdateSuccess,
+        ) {
+            interactor.state.test {
+                assertThat(awaitItem().cardBrandChoice.brand).isEqualTo(CardBrand.CartesBancaires)
+            }
+
+            val expectedNewCardBrand = CardBrand.Visa
+            interactor.handleViewAction(
+                UpdatePaymentMethodInteractor.ViewAction.BrandChoiceChanged(
+                    cardBrandChoice = CardBrandChoice(brand = expectedNewCardBrand, enabled = true)
+                )
+            )
+            interactor.state.test {
+                assertThat(awaitItem().cardBrandHasBeenChanged).isTrue()
+            }
+
+            interactor.handleViewAction(UpdatePaymentMethodInteractor.ViewAction.SaveButtonPressed)
+            assertThat(updateSuccessCalled).isTrue()
+        }
+    }
+
     @Test
     fun updatingCardBrand_updatesStateAsExpected() {
         var updatedPaymentMethod: PaymentMethod? = null
@@ -175,6 +207,7 @@ class DefaultUpdatePaymentMethodInteractorTest {
         runScenario(
             displayableSavedPaymentMethod = initialPaymentMethod.toDisplayableSavedPaymentMethod(),
             onUpdateCardBrand = ::updateCardBrandExecutor,
+            onUpdateSuccess = {},
         ) {
             interactor.state.test {
                 assertThat(awaitItem().cardBrandChoice.brand).isEqualTo(CardBrand.CartesBancaires)
@@ -308,6 +341,7 @@ class DefaultUpdatePaymentMethodInteractorTest {
         displayableSavedPaymentMethod: DisplayableSavedPaymentMethod = PaymentMethodFixtures.displayableCard(),
         onRemovePaymentMethod: (PaymentMethod) -> Throwable? = { notImplemented() },
         onUpdateCardBrand: (PaymentMethod, CardBrand) -> Result<PaymentMethod> = { _, _ -> notImplemented() },
+        onUpdateSuccess: () -> Unit = { notImplemented() },
         shouldShowSetAsDefaultCheckbox: Boolean = false,
         testBlock: suspend TestParams.() -> Unit
     ) {
@@ -322,6 +356,7 @@ class DefaultUpdatePaymentMethodInteractorTest {
             onBrandChoiceOptionsShown = {},
             onBrandChoiceOptionsDismissed = {},
             shouldShowSetAsDefaultCheckbox = shouldShowSetAsDefaultCheckbox,
+            onUpdateSuccess = onUpdateSuccess,
         )
 
         TestParams(interactor).apply { runTest { testBlock() } }