From 012f04e8bab30cc18b13fd1cb7737c06aeefb0f3 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Tue, 11 Mar 2025 08:24:57 +0100 Subject: [PATCH 1/6] Add PaymentIntentStatus type explicitly --- .../android/cardreader/internal/payments/PaymentManager.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt b/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt index 8d79631a7c8..826e4db8510 100644 --- a/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt +++ b/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt @@ -2,7 +2,6 @@ package com.woocommerce.android.cardreader.internal.payments import com.stripe.stripeterminal.external.models.PaymentIntent import com.stripe.stripeterminal.external.models.PaymentIntentStatus -import com.stripe.stripeterminal.external.models.PaymentIntentStatus.CANCELED import com.woocommerce.android.cardreader.CardReaderStore import com.woocommerce.android.cardreader.CardReaderStore.CapturePaymentResponse import com.woocommerce.android.cardreader.config.CardReaderConfigFactory @@ -63,7 +62,7 @@ internal class PaymentManager( private fun processPaymentIntent(orderId: Long, data: PaymentIntent) = flow { var paymentIntent = data - if (paymentIntent.status == null || paymentIntent.status == CANCELED) { + if (paymentIntent.status == null || paymentIntent.status == PaymentIntentStatus.CANCELED) { emit(errorMapper.mapError(errorMessage = "Cannot retry paymentIntent with status ${paymentIntent.status}")) return@flow } From f2463b517a95f7f47206597195f4eb1ca4cc9c5d Mon Sep 17 00:00:00 2001 From: malinajirka Date: Tue, 11 Mar 2025 08:32:09 +0100 Subject: [PATCH 2/6] Enable cancelling all non-final PI status except of requires_capture --- .../cardreader/internal/payments/PaymentManager.kt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt b/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt index 826e4db8510..08be71c5dcc 100644 --- a/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt +++ b/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt @@ -51,10 +51,15 @@ internal class PaymentManager( fun cancelPayment(paymentData: PaymentData) { val paymentIntent = (paymentData as PaymentDataImpl).paymentIntent + cancelOngoingPayment(paymentIntent) + } + + private fun cancelOngoingPayment(paymentIntent: PaymentIntent) { /* If the paymentIntent is in REQUIRES_CAPTURE state the app should not cancel the payment intent as it doesn't know if it was already captured or not during one of the previous attempts to capture it. */ - if (paymentIntent.status == PaymentIntentStatus.REQUIRES_PAYMENT_METHOD || - paymentIntent.status == PaymentIntentStatus.REQUIRES_CONFIRMATION + if (paymentIntent.status != PaymentIntentStatus.SUCCEEDED && + paymentIntent.status != PaymentIntentStatus.CANCELED && + paymentIntent.status != PaymentIntentStatus.REQUIRES_CAPTURE ) { cancelPaymentAction.cancelPayment(paymentIntent) } From 8c66c5c419c0886a8416ff98a11ef42a4dc30805 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Tue, 11 Mar 2025 08:33:19 +0100 Subject: [PATCH 3/6] Automatically cancel PI for incomplete non-retryable statuses --- .../internal/payments/PaymentManager.kt | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt b/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt index 08be71c5dcc..e8476e6e53a 100644 --- a/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt +++ b/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt @@ -16,8 +16,15 @@ import com.woocommerce.android.cardreader.internal.payments.actions.ProcessPayme import com.woocommerce.android.cardreader.internal.payments.actions.ProcessPaymentAction.ProcessPaymentStatus import com.woocommerce.android.cardreader.internal.wrappers.TerminalWrapper import com.woocommerce.android.cardreader.payments.CardPaymentStatus -import com.woocommerce.android.cardreader.payments.CardPaymentStatus.* +import com.woocommerce.android.cardreader.payments.CardPaymentStatus.CapturingPayment import com.woocommerce.android.cardreader.payments.CardPaymentStatus.CardPaymentStatusErrorType.Generic +import com.woocommerce.android.cardreader.payments.CardPaymentStatus.CollectingPayment +import com.woocommerce.android.cardreader.payments.CardPaymentStatus.InitializingPayment +import com.woocommerce.android.cardreader.payments.CardPaymentStatus.PaymentCompleted +import com.woocommerce.android.cardreader.payments.CardPaymentStatus.PaymentFailed +import com.woocommerce.android.cardreader.payments.CardPaymentStatus.PaymentMethodType +import com.woocommerce.android.cardreader.payments.CardPaymentStatus.ProcessingPayment +import com.woocommerce.android.cardreader.payments.CardPaymentStatus.ProcessingPaymentCompleted import com.woocommerce.android.cardreader.payments.PaymentData import com.woocommerce.android.cardreader.payments.PaymentInfo import kotlinx.coroutines.flow.Flow @@ -43,7 +50,19 @@ internal class PaymentManager( if (paymentIntent?.status != PaymentIntentStatus.REQUIRES_PAYMENT_METHOD) { return@flow } - processPaymentIntent(paymentInfo.orderId, paymentIntent).collect { emit(it) } + var eligibleForRetry = false + try { + processPaymentIntent(paymentInfo.orderId, paymentIntent).collect { + emit(it) + if (it is PaymentFailed && it.paymentDataForRetry != null) { + eligibleForRetry = true + } + } + } finally { + if (!eligibleForRetry) { + cancelOngoingPayment(paymentIntent) + } + } } fun retryPayment(orderId: Long, paymentData: PaymentData) = From 1efe7c3c559ee168d9f557c8700e391996e8fe69 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Tue, 11 Mar 2025 08:34:16 +0100 Subject: [PATCH 4/6] Add tests for automatic PI cancellation --- .../internal/payments/PaymentManagerTest.kt | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/libs/cardreader/src/test/java/com/woocommerce/android/cardreader/internal/payments/PaymentManagerTest.kt b/libs/cardreader/src/test/java/com/woocommerce/android/cardreader/internal/payments/PaymentManagerTest.kt index 134ef3f64e7..13677a7b777 100644 --- a/libs/cardreader/src/test/java/com/woocommerce/android/cardreader/internal/payments/PaymentManagerTest.kt +++ b/libs/cardreader/src/test/java/com/woocommerce/android/cardreader/internal/payments/PaymentManagerTest.kt @@ -5,6 +5,8 @@ import com.stripe.stripeterminal.external.models.Charge import com.stripe.stripeterminal.external.models.PaymentIntent import com.stripe.stripeterminal.external.models.PaymentIntentStatus import com.stripe.stripeterminal.external.models.PaymentIntentStatus.CANCELED +import com.stripe.stripeterminal.external.models.PaymentIntentStatus.PROCESSING +import com.stripe.stripeterminal.external.models.PaymentIntentStatus.REQUIRES_ACTION import com.stripe.stripeterminal.external.models.PaymentIntentStatus.REQUIRES_CAPTURE import com.stripe.stripeterminal.external.models.PaymentIntentStatus.REQUIRES_CONFIRMATION import com.stripe.stripeterminal.external.models.PaymentIntentStatus.REQUIRES_PAYMENT_METHOD @@ -598,6 +600,28 @@ class PaymentManagerTest : CardReaderBaseUnitTest() { verify(cancelPaymentAction).cancelPayment(paymentIntent) } + @Test + fun `given PaymentStatus PROCESSING, when canceling payment, then payment intent canceled`() = + testBlocking { + val paymentIntent = createPaymentIntent(PROCESSING) + val paymentData = PaymentDataImpl(paymentIntent) + + manager.cancelPayment(paymentData) + + verify(cancelPaymentAction).cancelPayment(paymentIntent) + } + + @Test + fun `given PaymentStatus REQUIRES_ACTION, when canceling payment, then payment intent canceled`() = + testBlocking { + val paymentIntent = createPaymentIntent(REQUIRES_ACTION) + val paymentData = PaymentDataImpl(paymentIntent) + + manager.cancelPayment(paymentData) + + verify(cancelPaymentAction).cancelPayment(paymentIntent) + } + @Test fun `given PaymentStatus REQUIRES_CAPTURE, when canceling payment, then payment intent NOT canceled`() = testBlocking { @@ -608,6 +632,76 @@ class PaymentManagerTest : CardReaderBaseUnitTest() { verify(cancelPaymentAction, never()).cancelPayment(paymentIntent) } + + @Test + fun `given PaymentStatus SUCCEEDED, when canceling payment, then payment intent NOT canceled`() = + testBlocking { + val paymentIntent = createPaymentIntent(SUCCEEDED) + val paymentData = PaymentDataImpl(paymentIntent) + + manager.cancelPayment(paymentData) + + verify(cancelPaymentAction, never()).cancelPayment(paymentIntent) + } + + @Test + fun `given PaymentStatus CANCELED, when canceling payment, then payment intent NOT canceled`() = + testBlocking { + val paymentIntent = createPaymentIntent(CANCELED) + val paymentData = PaymentDataImpl(paymentIntent) + + manager.cancelPayment(paymentData) + + verify(cancelPaymentAction, never()).cancelPayment(paymentIntent) + } + + @Test + fun `given non-retryable error, when collecting payment intent fails, then payment automatically cancelled`() = + testBlocking { + whenever(collectPaymentAction.collectPayment(anyOrNull())) + .thenReturn(flow { emit(CollectPaymentStatus.Failure(mock())) }) + + manager.acceptPayment(createPaymentInfo()).toList() + + verify(cancelPaymentAction).cancelPayment(any()) + } + + @Test + fun `given retryable error, when collecting payment intent fails, then payment NOT cancelled`() = + testBlocking { + whenever(collectPaymentAction.collectPayment(anyOrNull())) + .thenReturn(flow { emit(CollectPaymentStatus.Failure(mock())) }) + val dataForRetry = mock() + whenever(paymentErrorMapper.mapTerminalError(anyOrNull(), anyOrNull())) + .thenReturn(PaymentFailed(CardPaymentStatusErrorType.Generic, dataForRetry, "")) + + manager.acceptPayment(createPaymentInfo()).toList() + + verify(cancelPaymentAction, never()).cancelPayment(any()) + } + + @Test + fun `given non-retryable error, when processing payment fails, then payment automatically cancelled`() = testBlocking { + whenever(processPaymentAction.processPayment(anyOrNull())) + .thenReturn(flow { emit(ProcessPaymentStatus.Failure(mock())) }) + + manager.acceptPayment(createPaymentInfo()).toList() + + verify(cancelPaymentAction).cancelPayment(any()) + } + + @Test + fun `given retryable error, when processing payment fails, then payment NOT cancelled`() = testBlocking { + whenever(processPaymentAction.processPayment(anyOrNull())) + .thenReturn(flow { emit(ProcessPaymentStatus.Failure(mock())) }) + val dataForRetry = mock() + whenever(paymentErrorMapper.mapTerminalError(anyOrNull(), anyOrNull())) + .thenReturn(PaymentFailed(CardPaymentStatusErrorType.Generic, dataForRetry, "")) + + manager.acceptPayment(createPaymentInfo()).toList() + + verify(cancelPaymentAction, never()).cancelPayment(any()) + } // END - Cancel private fun createPaymentIntent( From ef0bf4dd388cdeb2e9cafab202033a59ba3f218a Mon Sep 17 00:00:00 2001 From: malinajirka Date: Tue, 11 Mar 2025 11:17:10 +0100 Subject: [PATCH 5/6] Pass PaymentIntent as a reference --- .../internal/payments/PaymentManager.kt | 38 ++++++++++++------- .../internal/payments/PaymentManagerTest.kt | 23 +++++------ 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt b/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt index e8476e6e53a..e6fb160a672 100644 --- a/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt +++ b/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt @@ -50,9 +50,12 @@ internal class PaymentManager( if (paymentIntent?.status != PaymentIntentStatus.REQUIRES_PAYMENT_METHOD) { return@flow } + + val paymentIntentReference = PaymentIntentReference(paymentIntent) var eligibleForRetry = false try { - processPaymentIntent(paymentInfo.orderId, paymentIntent).collect { + /* Passing PI as reference is a hack so we can access the updated PI in the finally block */ + processPaymentIntent(paymentInfo.orderId, paymentIntentReference).collect { emit(it) if (it is PaymentFailed && it.paymentDataForRetry != null) { eligibleForRetry = true @@ -60,13 +63,14 @@ internal class PaymentManager( } } finally { if (!eligibleForRetry) { - cancelOngoingPayment(paymentIntent) + cancelOngoingPayment(paymentIntentReference.value) } } } fun retryPayment(orderId: Long, paymentData: PaymentData) = - processPaymentIntent(orderId, (paymentData as PaymentDataImpl).paymentIntent) + // TODO Make sure we are cancelling even in this code path + processPaymentIntent(orderId, PaymentIntentReference((paymentData as PaymentDataImpl).paymentIntent)) fun cancelPayment(paymentData: PaymentData) { val paymentIntent = (paymentData as PaymentDataImpl).paymentIntent @@ -84,18 +88,21 @@ internal class PaymentManager( } } - private fun processPaymentIntent(orderId: Long, data: PaymentIntent) = flow { - var paymentIntent = data - if (paymentIntent.status == null || paymentIntent.status == PaymentIntentStatus.CANCELED) { - emit(errorMapper.mapError(errorMessage = "Cannot retry paymentIntent with status ${paymentIntent.status}")) + private fun processPaymentIntent(orderId: Long, paymentIntentRef: PaymentIntentReference) = flow { + if (paymentIntentRef.value.status == null || paymentIntentRef.value.status == PaymentIntentStatus.CANCELED) { + emit( + errorMapper.mapError( + errorMessage = "Cannot retry paymentIntent with status ${paymentIntentRef.value.status}" + ) + ) return@flow } - if (paymentIntent.status == PaymentIntentStatus.REQUIRES_PAYMENT_METHOD) { - paymentIntent = collectPayment(paymentIntent) + if (paymentIntentRef.value.status == PaymentIntentStatus.REQUIRES_PAYMENT_METHOD) { + paymentIntentRef.value = collectPayment(paymentIntentRef.value) } - if (paymentIntent.status == PaymentIntentStatus.REQUIRES_CONFIRMATION) { - paymentIntent = processPayment(paymentIntent) + if (paymentIntentRef.value.status == PaymentIntentStatus.REQUIRES_CONFIRMATION) { + paymentIntentRef.value = processPayment(paymentIntentRef.value) } /* @@ -111,9 +118,11 @@ internal class PaymentManager( the success/failure of the actual payment. */ - if (paymentIntent.status == PaymentIntentStatus.REQUIRES_CAPTURE || isInteracPaymentSuccessful(paymentIntent)) { - retrieveReceiptUrl(paymentIntent)?.let { receiptUrl -> - capturePayment(receiptUrl, orderId, cardReaderStore, paymentIntent) + if (paymentIntentRef.value.status == PaymentIntentStatus.REQUIRES_CAPTURE || + isInteracPaymentSuccessful(paymentIntentRef.value) + ) { + retrieveReceiptUrl(paymentIntentRef.value)?.let { receiptUrl -> + capturePayment(receiptUrl, orderId, cardReaderStore, paymentIntentRef.value) } } } @@ -222,3 +231,4 @@ internal class PaymentManager( } internal data class PaymentDataImpl(val paymentIntent: PaymentIntent) : PaymentData +internal data class PaymentIntentReference(var value: PaymentIntent) diff --git a/libs/cardreader/src/test/java/com/woocommerce/android/cardreader/internal/payments/PaymentManagerTest.kt b/libs/cardreader/src/test/java/com/woocommerce/android/cardreader/internal/payments/PaymentManagerTest.kt index 13677a7b777..2b3d810c959 100644 --- a/libs/cardreader/src/test/java/com/woocommerce/android/cardreader/internal/payments/PaymentManagerTest.kt +++ b/libs/cardreader/src/test/java/com/woocommerce/android/cardreader/internal/payments/PaymentManagerTest.kt @@ -658,13 +658,13 @@ class PaymentManagerTest : CardReaderBaseUnitTest() { @Test fun `given non-retryable error, when collecting payment intent fails, then payment automatically cancelled`() = testBlocking { - whenever(collectPaymentAction.collectPayment(anyOrNull())) - .thenReturn(flow { emit(CollectPaymentStatus.Failure(mock())) }) + whenever(collectPaymentAction.collectPayment(anyOrNull())) + .thenReturn(flow { emit(CollectPaymentStatus.Failure(mock())) }) - manager.acceptPayment(createPaymentInfo()).toList() + manager.acceptPayment(createPaymentInfo()).toList() - verify(cancelPaymentAction).cancelPayment(any()) - } + verify(cancelPaymentAction).cancelPayment(any()) + } @Test fun `given retryable error, when collecting payment intent fails, then payment NOT cancelled`() = @@ -681,14 +681,15 @@ class PaymentManagerTest : CardReaderBaseUnitTest() { } @Test - fun `given non-retryable error, when processing payment fails, then payment automatically cancelled`() = testBlocking { - whenever(processPaymentAction.processPayment(anyOrNull())) - .thenReturn(flow { emit(ProcessPaymentStatus.Failure(mock())) }) + fun `given non-retryable error, when processing payment fails, then payment automatically cancelled`() = + testBlocking { + whenever(processPaymentAction.processPayment(anyOrNull())) + .thenReturn(flow { emit(ProcessPaymentStatus.Failure(mock())) }) - manager.acceptPayment(createPaymentInfo()).toList() + manager.acceptPayment(createPaymentInfo()).toList() - verify(cancelPaymentAction).cancelPayment(any()) - } + verify(cancelPaymentAction).cancelPayment(any()) + } @Test fun `given retryable error, when processing payment fails, then payment NOT cancelled`() = testBlocking { From 3cde3ca235ffc80bc1a2c3267e9ca72d31132706 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Tue, 11 Mar 2025 11:24:31 +0100 Subject: [PATCH 6/6] Cancel PaymentIntents in retry flow --- .../internal/payments/PaymentManager.kt | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt b/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt index e6fb160a672..f884ec53252 100644 --- a/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt +++ b/libs/cardreader/src/main/java/com/woocommerce/android/cardreader/internal/payments/PaymentManager.kt @@ -51,26 +51,11 @@ internal class PaymentManager( return@flow } - val paymentIntentReference = PaymentIntentReference(paymentIntent) - var eligibleForRetry = false - try { - /* Passing PI as reference is a hack so we can access the updated PI in the finally block */ - processPaymentIntent(paymentInfo.orderId, paymentIntentReference).collect { - emit(it) - if (it is PaymentFailed && it.paymentDataForRetry != null) { - eligibleForRetry = true - } - } - } finally { - if (!eligibleForRetry) { - cancelOngoingPayment(paymentIntentReference.value) - } - } + processPayment(paymentInfo.orderId, paymentIntent) } fun retryPayment(orderId: Long, paymentData: PaymentData) = - // TODO Make sure we are cancelling even in this code path - processPaymentIntent(orderId, PaymentIntentReference((paymentData as PaymentDataImpl).paymentIntent)) + processPayment(orderId, (paymentData as PaymentDataImpl).paymentIntent) fun cancelPayment(paymentData: PaymentData) { val paymentIntent = (paymentData as PaymentDataImpl).paymentIntent @@ -88,6 +73,24 @@ internal class PaymentManager( } } + private fun processPayment(orderId: Long, paymentIntent: PaymentIntent) = flow { + val paymentIntentReference = PaymentIntentReference(paymentIntent) + var eligibleForRetry = false + try { + /* Passing PI as reference is a hack so we can access the updated PI in the finally block */ + processPaymentIntent(orderId, paymentIntentReference).collect { + emit(it) + if (it is PaymentFailed && it.paymentDataForRetry != null) { + eligibleForRetry = true + } + } + } finally { + if (!eligibleForRetry) { + cancelOngoingPayment(paymentIntentReference.value) + } + } + } + private fun processPaymentIntent(orderId: Long, paymentIntentRef: PaymentIntentReference) = flow { if (paymentIntentRef.value.status == null || paymentIntentRef.value.status == PaymentIntentStatus.CANCELED) { emit(