-
Notifications
You must be signed in to change notification settings - Fork 678
Init Card Scan Migration with Feature Flag #11393
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
Init Card Scan Migration with Feature Flag #11393
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested manually, but let me know if there is any missing automatic test.
val request = PaymentCardRecognitionIntentRequest.getDefaultInstance() | ||
|
||
paymentsClient.getPaymentCardRecognitionIntent(request) | ||
.addOnSuccessListener { intentResponse -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These listeners, or say DefaultPaymentCardRecognitionClient
overall, are not testable because the response can not be produced with test context.
val cardScanGoogleLauncher = if (FeatureFlags.cardScanGooglePayMigration.isEnabled) { | ||
rememberCardScanGoogleLauncher(context, onGoogleCardScanResult) | ||
} else { | ||
null | ||
} | ||
val isCardScanGoogleAvailable by if (FeatureFlags.cardScanGooglePayMigration.isEnabled) { | ||
cardScanGoogleLauncher?.isAvailable?.collectAsState() ?: remember { mutableStateOf(false) } | ||
} else { | ||
remember { mutableStateOf(false) } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should prevent any of GPCR api being called accidentally.
Diffuse output:
APK
DEX
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a screen recording to the PR description to show what the new card scan impl looks like?
...ents-ui-core/src/test/java/com/stripe/android/ui/core/cardscan/CardScanGoogleLauncherTest.kt
Outdated
Show resolved
Hide resolved
...ents-ui-core/src/test/java/com/stripe/android/ui/core/cardscan/CardScanGoogleLauncherTest.kt
Outdated
Show resolved
Hide resolved
...i-core/src/test/java/com/stripe/android/ui/core/cardscan/FakePaymentCardRecognitionClient.kt
Outdated
Show resolved
Hide resolved
...ents-ui-core/src/test/java/com/stripe/android/ui/core/cardscan/CardScanGoogleLauncherTest.kt
Outdated
Show resolved
Hide resolved
...nts-ui-core/src/main/java/com/stripe/android/ui/core/elements/CardDetailsSectionElementUI.kt
Outdated
Show resolved
Hide resolved
...nts-ui-core/src/main/java/com/stripe/android/ui/core/elements/CardDetailsSectionElementUI.kt
Outdated
Show resolved
Hide resolved
...ts-ui-core/src/main/java/com/stripe/android/ui/core/cardscan/PaymentCardRecognitionClient.kt
Outdated
Show resolved
Hide resolved
payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/ScanCardButtonUI.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be good to have some tests for this UI -- e.g. we should test that the button is shown/hidden when expected and that onClick does what we expect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is tricky to fake the controller and card scan launchers to cover the test cases we want. Do you have any suggestion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have an idea for how we could do this -- let's discuss in our 1:1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed offline and these tests will be added as a follow up to this PR
451fc2a
to
057c426
Compare
057c426
to
06cf75e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed offline and these tests will be added as a follow up to this PR
Summary
Use feature flag to switch between legacy impl and GPCR impl in playground
Motivation
Testing
Screenshots
IMG_4632.MOV
IMG_4630.MOV
Changelog