fixUniversalLink #398
fixUniversalLink #398
Conversation
* introduce appLinkUrl in PayPalWebVaultRequest and PayPalWebCheckoutRequest
* deprecate vault function without app switch feature
* deprecate vault function without app switch feature * introduces startAsync, vaultAsync functions to provide suspend support
* fix unit tests * update unit tests
| private val payPalDataCollector = PayPalDataCollector(coreConfig) | ||
| private val paypalClient = | ||
| PayPalWebCheckoutClient(applicationContext, coreConfig, "com.paypal.android.demo") | ||
| PayPalWebCheckoutClient(applicationContext, coreConfig) |
There was a problem hiding this comment.
Do we still need the fallback? Some users may have app links disabled. I'm thinking now this is why BT has both.
There was a problem hiding this comment.
Oh wait I see now. I forgot we discussed adding this as a parameter to the request.
There was a problem hiding this comment.
We should test with app links disabled though.
There was a problem hiding this comment.
when app links disabled, app settings -> open default -> uncheck urls
then it goes to web page, applink url can be http url or it can be $customScheme://$host/$path format it works, browser switch accepts it too. $path is optional
There was a problem hiding this comment.
Do we need to update the demo app so that we can still use the fallback URL when app links are disabled?
| * | ||
| * @param request [PayPalWebCheckoutRequest] for requesting an order approval | ||
| */ | ||
| suspend fun startAsync( |
There was a problem hiding this comment.
| suspend fun startAsync( | |
| fun startAsync( |
We should wait for v3 to introduce coroutine APIs. Or even if we have them in v2, we should create coroutine APIs for all of our modules.
There was a problem hiding this comment.
we still support deprecated fun start, so added this as new functionality, we can make this private and just keep fun start(activity, orderRequest, callback) as the only way to get app switch functionality and add suspend fun start in v3.
Also what other places we can add coroutines support
There was a problem hiding this comment.
Theoretically CardClient and FraudDetection might be two modules where we can add coroutine support. It's better if we have a plan for coroutine support first.
| ) { | ||
| applicationScope.launch { | ||
| val result = start(activity, request) | ||
| val result = startAsync(activity, request) |
There was a problem hiding this comment.
Should this be wrapped in an async{} block as well? I'd prefer using either applicationScope.launch{} or async{} throughout.
There was a problem hiding this comment.
async{} is used to run updateCCO and patchCCOAppSwitch in parallel
applicationScope.launch{} is used to ensure this keeps running with supervisor job
still thread is going on, added comment to clarify it
* remove unwanted logs * removed unnecessary code
* add fallbackUrlScheme to PayPalWebVaultRequest
* update public API
Summary of changes
startAsync,vaultAsyncsuspend functionsappLinkUrlinPayPalWebVaultRequestandPayPalWebCheckoutRequestto support universal linksurlSchemeinPayPalWebCheckoutClientappLinkUrlis not null then used for navigation, if not usedurlScheme, if both are null then result will be failure, this logic is handled in Browser SwitchChecklist
Authors