Card: Remove Approve Order Listener Pattern #305
Conversation
| * @param result [CardApproveOrderResult] result with details | ||
| */ | ||
| @MainThread | ||
| fun onCardApproveOrderResult(result: CardApproveOrderResult) |
There was a problem hiding this comment.
That's interesting. I don't think it's possible to do this in iOS with protocols.
Do you feel like this is something merchant would want us to handle?
There was a problem hiding this comment.
I'm not sure if we need to deliver results on the main thread, but it is convenient when updating the UI after executing a transaction.
It's up to y'all if you want to do the same. I feel like the closest equivalent to this annotation in Swift would be @MainActor.
| @@ -69,22 +68,27 @@ class CardClient internal constructor( | |||
| is ConfirmPaymentSourceResult.Success -> { | |||
| if (response.payerActionHref == null) { | |||
There was a problem hiding this comment.
this syntax looks really similar to completion handler. So callback in place of listener pattern is safer for memory management?
There was a problem hiding this comment.
It's similar. The main difference now is there's a callback for every async method. With the listener pattern, there is only one listener, and it needs to be set by the merchant before starting any of the payment flows.
|
|
||
| val url = Uri.parse(response.payerActionHref) | ||
| val authChallenge = CardAuthChallenge.ApproveOrder(url, cardRequest) | ||
| approveOrderListener?.onApproveOrderAuthorizationRequired(authChallenge) | ||
| val result = CardApproveOrderResult.AuthorizationRequired(authChallenge) |
There was a problem hiding this comment.
It's interesting that you have this intermediate state on the same result type.
So there is result type for API call and final result type for the merchant.
There was a problem hiding this comment.
This method does have three possible results. Alternatively, I thought about bundling the payer_action_required status into the Failure case but this felt more natural.
Technically it isn't a failure. PPaaS just needs more information before the buyer can continue.
| cardClient?.presentAuthChallenge(activity, authChallenge)?.let { presentAuthResult -> | ||
| when (presentAuthResult) { | ||
| is CardPresentAuthChallengeResult.Success -> { | ||
| authState = presentAuthResult.authState |
There was a problem hiding this comment.
I'm a bit lost on how this authState gets translated to success CardApproveOrderResult back to the merchant.
There was a problem hiding this comment.
So authState is a JSON string that allows us to continue the flow once the merchant app comes back into the foreground after browser switching. We created this pattern in BT v5 to allow Android merchants to recover from a process kill. In BT it's called a pendingRequest.
We can always come up with a new name, authState for me makes sense because it's similar to the state parameter used in OAuth2.
There was a problem hiding this comment.
Also this example should be a lot clearer in our revised sample merchant app we have planned.
There was a problem hiding this comment.
I see that viewModel's completeAuthChallenge gets called in ApproveOrderView.
Summary of changes
CardClientapprove order flow away from the listener pattern.CardClient.presentAuthChallenge()with calls toCardClient.finishApproveOrder()to complete an order flowChecklist
Authors