Add Google Pay Implementation - #438
Conversation
| companion object { | ||
| fun createPaymentsClient(context: Context): PaymentsClient { | ||
| val walletOptions = Wallet.WalletOptions.Builder() | ||
| .setEnvironment(WalletConstants.ENVIRONMENT_TEST) |
There was a problem hiding this comment.
extend CoreConfig to contain environment for Google Pay, use it here dynamically
There was a problem hiding this comment.
Can you explain this more?
There was a problem hiding this comment.
we can change CoreConfig to include GooglePayEnvironment like below
data CoreConfig(
val clientId: String,
val environment: Environment = Environment.SANDBOX,
val googlePayEnvironment = (WalletConstants.ENVIRONMENT_TEST
)There was a problem hiding this comment.
But then core would have a dependency on Google Pay. So if someone is using CardPayments, they will need to pull in Google Pay into their project as well even though they aren't using that feature.
|
|
||
| @OptIn(InternalSerializationApi::class) | ||
| suspend fun start(request: GooglePayCheckoutRequest): GooglePayStartResult { | ||
| val merchantId = request.merchantId |
There was a problem hiding this comment.
should merchantId be coming from GooglePayCheckoutRequest, merchant passing everytime
this doesn't look like start, it's eligibility check for google pay
There was a problem hiding this comment.
Where should we put it?
There was a problem hiding this comment.
may be CoreConfig all details which will not change in every transaction stays in 1 place
There was a problem hiding this comment.
Google Pay is the only feature that requires a merchantId. I wouldn't feel comfortable doing that unless merchant ID was required for all payment methods.
| intent: Intent? | ||
| ): GooglePayLaunchResult? { | ||
| val googlePayResult = googlePayContract.parseResult(resultCode, intent) | ||
| val success = googlePayResult.status.statusCode == CommonStatusCodes.SUCCESS |
There was a problem hiding this comment.
val success = googlePayResult.status.statusCode in listOf(CommonStatusCodes.SUCCESS, CommonStatusCodes.SUCCESS_CACHE)
?
There was a problem hiding this comment.
What's SUCCESS_CACHE?
There was a problem hiding this comment.
SUCCESS_CACHE is when it already checked before and didn't performed realtime check and got values from cache, in our situation I think this is valid success case
https://developers.google.com/android/reference/com/google/android/gms/common/api/CommonStatusCodes#SUCCESS_CACHE()
There was a problem hiding this comment.
Looking at the docs it doesn't seem like SUCCESS_CACHE is a possibility. This is a good callout in general though, I'll look into the other error codes mentioned here.
|
|
||
| is GraphQLResult.Failure -> { | ||
| val error = | ||
| PayPalSDKError(123, "Unable to launch google pay: unable to fetch config.") |
There was a problem hiding this comment.
need to define error codes
There was a problem hiding this comment.
Yeah definitely. We also probably don't need error codes. That's a holdover from iOS. That's a separate item though.
| clientId = coreConfig.clientId, | ||
| merchantId = listOf(merchantId), | ||
| // TODO: see if we need this | ||
| merchantOrigin = "com.paypal.android.sdk", |
There was a problem hiding this comment.
this may be looking for host origin?
There was a problem hiding this comment.
You mean like the way JS does it?
There was a problem hiding this comment.
guessing by the name, not sure if it's expecting appId or host of merchant
There was a problem hiding this comment.
On Android the merchant isn't guaranteed to have a host. On JS I think they get this from Window.location–I'm not aware of an equivalent on Android.
fdc6a43 to
0d4e635
Compare
…ay services wallet dependency.
a73f63d to
ade31a9
Compare
Summary of changes
Checklist
Added a changelog entryAuthors