PayPal Launcher API #807
PayPal Launcher API #807
Conversation
| * @throws BrowserSwitchException if there is an error launching the PayPal web flow | ||
| */ | ||
| public void launch(@NonNull FragmentActivity activity, @NonNull PayPalResponse payPalResponse) | ||
| throws BrowserSwitchException { |
There was a problem hiding this comment.
Should this throw or how should this exception be handled? The Activity result based launch methods (Google Pay, Venmo) don't throw or callback any exception, so I opted to throw to keep the launch parameters cleaner. This does mean a try/catch block in the merchant code (see migration guide example). Other alternative would be to callback an error in this method, or give the PayPalLauncher a callback in the constructor to handle results (this would align with the Activity based launchers)
There was a problem hiding this comment.
Yeah we should design it to have the same API as the Activity launchers if possible. It'd be awesome if we're able to offer a familiar DX for onboarding each Activity and Browser-based payment flow.
There was a problem hiding this comment.
Ok updated this so that the launcher constructor takes a callback as a parameter (to align with Activity Result flows) and if there's an error in the launch method, that error will be delivered to the callback from the constructor.
| */ | ||
| public BrowserSwitchResult deliverResult(@NonNull Context context, @NonNull Intent intent) { | ||
| BrowserSwitchResult result = browserSwitchClient.parseResult(context, PAYPAL, intent); | ||
| browserSwitchClient.clearActiveRequests(context); |
There was a problem hiding this comment.
@sshropshire Can clearActiveRequests be called under the hood here or do we need to give the merchants an accessor to call this themselves after handling the result?
There was a problem hiding this comment.
For this one I'm 50/50. We could call it for merchants, but run the risk of using too much abstraction. I'm in favor of having merchants call launcher.clearActiveRequests() (doesn't have to be the same name). A better solution may be to revisit the acceptance criteria for browser switching to make sure we have it right.
There was a problem hiding this comment.
Ok agreed for now I am going to keep it under the hood because I think it should always be called immediately after a successful parse, but created a ticket to revisit this pattern before GA
| BrowserSwitchResult getBrowserSwitchResult() { | ||
| return browserSwitchResult; | ||
| } | ||
|
|
||
| Exception getError() { | ||
| return error; | ||
| } |
There was a problem hiding this comment.
Do these need to be public w/ docstrings?
There was a problem hiding this comment.
No the idea with this result object is that the merchant doesn't have to do anything with it / be aware of any params and just passes it directly to the PayPalClient#onBrowserSwitchResult method, which will then handle any errors and call them back to the merchant. We could break it up and allow merchants to handle errors at each step, but this seems cleaner for integrations
Co-authored-by: scannillo <[email protected]>
Co-authored-by: scannillo <[email protected]>
Summary of changes
Checklist
Authors