Skip to content

Commit 06cf75e

Browse files
cttsai-stripecttsai-stripe
committed
Code Review
1 parent b42a3b5 commit 06cf75e

File tree

5 files changed

+37
-40
lines changed

5 files changed

+37
-40
lines changed

‎payments-ui-core/src/main/java/com/stripe/android/ui/core/cardscan/CardScanGoogleLauncher.kt

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import androidx.activity.result.contract.ActivityResultContracts
1010
import androidx.annotation.VisibleForTesting
1111
import androidx.compose.runtime.Composable
1212
import androidx.compose.runtime.remember
13-
import com.google.android.gms.wallet.CreditCardExpirationDate
1413
import com.google.android.gms.wallet.PaymentCardRecognitionResult
1514
import kotlinx.coroutines.flow.MutableStateFlow
1615
import kotlinx.coroutines.flow.StateFlow
@@ -53,15 +52,16 @@ internal class CardScanGoogleLauncher @VisibleForTesting constructor(
5352
val data = result.data ?: return CardScanResult.Canceled
5453
val paymentCardRecognitionResult = PaymentCardRecognitionResult.getFromIntent(data)
5554
val pan = paymentCardRecognitionResult?.pan
56-
val expirationDate = paymentCardRecognitionResult?.creditCardExpirationDate
5755
return if (pan != null) {
58-
CardScanResult.Completed(ScannedCard(pan, expirationDate))
56+
CardScanResult.Completed(ScannedCard(pan))
5957
} else {
6058
val error = Throwable("Failed to parse card data")
6159
CardScanResult.Failed(error)
6260
}
61+
} else if (result.resultCode == Activity.RESULT_CANCELED) {
62+
return CardScanResult.Canceled
6363
}
64-
return CardScanResult.Canceled
64+
return CardScanResult.Failed(Throwable("Null data or unexpected result code: ${result.resultCode}"))
6565
}
6666

6767
companion object {
@@ -99,5 +99,4 @@ internal sealed interface CardScanResult {
9999
*/
100100
internal data class ScannedCard(
101101
val pan: String,
102-
val expirationDate: CreditCardExpirationDate?
103102
)

‎payments-ui-core/src/main/java/com/stripe/android/ui/core/cardscan/PaymentCardRecognitionClient.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,18 @@ import com.google.android.gms.wallet.WalletConstants
1010
internal interface PaymentCardRecognitionClient {
1111
fun fetchIntent(
1212
context: Context,
13-
onFailure: (Exception) -> Unit = {},
13+
onFailure: (Throwable) -> Unit = {},
1414
onSuccess: (IntentSenderRequest) -> Unit
1515
)
1616
}
1717

1818
internal class DefaultPaymentCardRecognitionClient : PaymentCardRecognitionClient {
1919
override fun fetchIntent(
2020
context: Context,
21-
onFailure: (Exception) -> Unit,
21+
onFailure: (Throwable) -> Unit,
2222
onSuccess: (IntentSenderRequest) -> Unit
2323
) {
24-
try {
24+
runCatching {
2525
val paymentsClient = createPaymentsClient(context)
2626
val request = PaymentCardRecognitionIntentRequest.getDefaultInstance()
2727

@@ -34,7 +34,7 @@ internal class DefaultPaymentCardRecognitionClient : PaymentCardRecognitionClien
3434
.addOnFailureListener { e ->
3535
onFailure(e)
3636
}
37-
} catch (@Suppress("TooGenericExceptionCaught") e: Exception) {
37+
}.onFailure { e ->
3838
onFailure(e)
3939
}
4040
}

‎payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/CardDetailsSectionElementUI.kt

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,23 +48,17 @@ fun CardDetailsSectionElementUI(
4848
if (controller.isCardScanEnabled &&
4949
(controller.isStripeCardScanAvailable() || FeatureFlags.cardScanGooglePayMigration.isEnabled)
5050
) {
51+
val onGoogleCardScanResult: (CardScanResult) -> Unit = { result ->
52+
(result as? CardScanResult.Completed)?.scannedCard?.let { scannedCard ->
53+
controller.cardDetailsElement.controller.numberElement.controller.onRawValueChange(
54+
scannedCard.pan
55+
)
56+
}
57+
}
5158
ScanCardButtonUI(
5259
enabled = enabled,
5360
elementsSessionId = controller.elementsSessionId,
54-
onGoogleCardScanResult = { result ->
55-
(result as? CardScanResult.Completed)?.scannedCard?.let { scannedCard ->
56-
controller.cardDetailsElement.controller.numberElement.controller.onRawValueChange(
57-
scannedCard.pan
58-
)
59-
scannedCard.expirationDate?.let { expirationDate ->
60-
controller.cardDetailsElement.controller.expirationDateElement.controller
61-
.onRawValueChange(
62-
@Suppress("MagicNumber")
63-
"${expirationDate.month}/${expirationDate.year % 100}"
64-
)
65-
}
66-
}
67-
}
61+
onGoogleCardScanResult = onGoogleCardScanResult
6862
) {
6963
controller.cardDetailsElement.controller.numberElement.controller.onCardScanResult(it)
7064
}

‎payments-ui-core/src/test/java/com/stripe/android/ui/core/cardscan/CardScanGoogleLauncherTest.kt

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import androidx.core.app.ActivityOptionsCompat
1010
import androidx.test.core.app.ApplicationProvider
1111
import app.cash.turbine.ReceiveTurbine
1212
import app.cash.turbine.Turbine
13-
import com.google.common.truth.Truth
13+
import com.google.common.truth.Truth.assertThat
1414
import kotlinx.coroutines.test.runTest
1515
import org.junit.Test
1616
import org.junit.runner.RunWith
@@ -25,16 +25,18 @@ class CardScanGoogleLauncherTest {
2525

2626
val scanResult = launcher.parseActivityResult(result)
2727

28-
Truth.assertThat(scanResult).isInstanceOf(CardScanResult.Canceled::class.java)
28+
assertThat(scanResult).isInstanceOf(CardScanResult.Canceled::class.java)
2929
}
3030

3131
@Test
32-
fun `parseActivityResult with RESULT_OK but null data returns Canceled`() = runScenario {
32+
fun `parseActivityResult with RESULT_OK but null data returns Failed`() = runScenario {
3333
val result = ActivityResult(Activity.RESULT_OK, null)
3434

3535
val scanResult = launcher.parseActivityResult(result)
3636

37-
Truth.assertThat(scanResult).isInstanceOf(CardScanResult.Canceled::class.java)
37+
assertThat(scanResult).isInstanceOf(CardScanResult.Failed::class.java)
38+
val failedResult = scanResult as CardScanResult.Failed
39+
assertThat(failedResult.error.message).isEqualTo("Null data or unexpected result code: -1")
3840
}
3941

4042
@Test
@@ -47,33 +49,35 @@ class CardScanGoogleLauncherTest {
4749

4850
val scanResult = launcher.parseActivityResult(result)
4951

50-
Truth.assertThat(scanResult).isInstanceOf(CardScanResult.Failed::class.java)
52+
assertThat(scanResult).isInstanceOf(CardScanResult.Failed::class.java)
5153
val failedResult = scanResult as CardScanResult.Failed
52-
Truth.assertThat(failedResult.error.message).isEqualTo("Failed to parse card data")
54+
assertThat(failedResult.error.message).isEqualTo("Failed to parse card data")
5355
}
5456

5557
@Test
56-
fun `parseActivityResult with custom result code returns Canceled`() = runScenario {
58+
fun `parseActivityResult with custom result code returns Failed`() = runScenario {
5759
val result = ActivityResult(123, null) // Some other result code
5860

5961
val scanResult = launcher.parseActivityResult(result)
6062

61-
Truth.assertThat(scanResult).isInstanceOf(CardScanResult.Canceled::class.java)
63+
assertThat(scanResult).isInstanceOf(CardScanResult.Failed::class.java)
64+
val failedResult = scanResult as CardScanResult.Failed
65+
assertThat(failedResult.error.message).isEqualTo("Null data or unexpected result code: 123")
6266
}
6367

6468
@Test
6569
fun `card scan launcher should be able to launch card scan activity`() = runScenario {
66-
Truth.assertThat(launcher.isAvailable.value).isTrue()
70+
assertThat(launcher.isAvailable.value).isTrue()
6771

6872
launcher.launch(ApplicationProvider.getApplicationContext())
69-
Truth.assertThat(activityLauncher.launchCall.awaitItem()).isEqualTo(Unit)
73+
assertThat(activityLauncher.launchCall.awaitItem()).isEqualTo(Unit)
7074
}
7175

7276
@Test
7377
fun `card scan launcher should not be available when fetchIntent fails`() = runScenario(
74-
isFetchClientSuccess = false
78+
isFetchClientSucceed = false
7579
) {
76-
Truth.assertThat(launcher.isAvailable.value).isFalse()
80+
assertThat(launcher.isAvailable.value).isFalse()
7781
launcher.launch(ApplicationProvider.getApplicationContext())
7882
// No launch call should be made since fetchIntent failed
7983
}
@@ -103,13 +107,13 @@ class CardScanGoogleLauncherTest {
103107
)
104108

105109
private fun runScenario(
106-
isFetchClientSuccess: Boolean = true,
110+
isFetchClientSucceed: Boolean = true,
107111
block: suspend Scenario.() -> Unit
108112
) = runTest {
109113
val activityLauncher = FakeActivityLauncher<IntentSenderRequest>()
110114
val launcher = CardScanGoogleLauncher(
111115
context = ApplicationProvider.getApplicationContext(),
112-
paymentCardRecognitionClient = FakePaymentCardRecognitionClient(isFetchClientSuccess)
116+
paymentCardRecognitionClient = FakePaymentCardRecognitionClient(isFetchClientSucceed)
113117
).apply {
114118
this.activityLauncher = activityLauncher
115119
}

‎payments-ui-core/src/test/java/com/stripe/android/ui/core/cardscan/FakePaymentCardRecognitionClient.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@ import android.content.Context
44
import android.content.Intent
55
import androidx.activity.result.IntentSenderRequest
66

7-
class FakePaymentCardRecognitionClient(private val shouldSuccess: Boolean) : PaymentCardRecognitionClient {
7+
class FakePaymentCardRecognitionClient(private val shouldSucceed: Boolean) : PaymentCardRecognitionClient {
88
override fun fetchIntent(
99
context: Context,
10-
onFailure: (Exception) -> Unit,
10+
onFailure: (Throwable) -> Unit,
1111
onSuccess: (IntentSenderRequest) -> Unit
1212
) {
13-
if (shouldSuccess) {
13+
if (shouldSucceed) {
1414
val mockPendingIntent = android.app.PendingIntent.getActivity(
1515
context,
1616
0,

0 commit comments

Comments
 (0)