[QL] Conform BTPayPalClient to BTAppContextSwitchClient Protocol

[jaxdesmarais]

This work is into paypal-app-switch-feature

Summary of changes

• Conform BTPayPalClient to BTAppContextSwitchClient
• Add BTPayPalAppSwitchReturnURL struct
• Add Unit Tests

Checklist

• [ ] Added a changelog entry

Authors

• @jaxdesmarais

[jaxdesmarais]

Based on the current contracts I don't believe we will ever get an error path back. In the future if we do we can consider adding it here. We can also consider adding a check here that the host contains the universalLink set on BTAppContextSwitcher. What do folks think?

[scannillo]

There won't be an error specific scenario in the URL, no.

Though we will want to surface the cancel as an error to the merchant, as well as continue with the flow to call the paypal_accounts API in the case of success. Are you planning to add those paths in this PR? If not, can we add TODOs?

[jaxdesmarais]

Though we will want to surface the cancel as an error to the merchant

🤔 how would we both surface the cancel as an error and continue the flow? If we surfaced it in our completion I would expect the execution of that path would also end? Or do we want to diverge from that pattern and latter in the flow return both a nonce and an error?

Are you planning to add those paths in this PR? If not, can we add TODOs?

There is a TODO in handleOpen above where we will build out the logic in a future PR to handle the return URL. This is just the logic that determines if our SDK can process the URL we get back (though I did notice just now BTAppContextSwitcher wasn't registered properly and updated here along with deploying a success path so we can test that these methods are hit: 9efbff2)

[scannillo]

🤔 how would we both surface the cancel as an error and continue the flow? If we surfaced it in our completion I would expect the execution of that path would also end?

Yes! Like we do today when we get back cancel in the URL for the web-based flow, we end the flow for the merchant.

[jaxdesmarais]

That makes since, sorry I read that wrong at first as we want to continue the flow on cancel, but that portion was for success. 🙈

Do we think it'd be helpful to also validate that the host contains the value passed into BTAppContextSwitcher.shared.universalLink or do we think this is sufficient (checking for https and cancel or success)?

[scannillo]

Good question - maybe we can assume that if the switch successfully made it into the merchant app, that the host of the URL is fine. I vote for not doing that check, for now. But don't feel strongly if you have other thoughts

[jaxdesmarais]

I agree with that too, we can always add it in a future PR if we decide we need additional validation.

[scannillo]

Another thought - might be more readable to keep this similar to Venmo which has a separate class (BTVenmoAppSwitchReturnURL) for the isValid() logic & eventual param parsing? So adding something like a BTVeniceAppSwitchReturnURL class

[jaxdesmarais]

I'm down, updated here (I opted to call it BTPayPalAppSwitchReturnURL since I assume Venice is somewhat internal): 4b39541

[scannillo]

I know this is the naming we use in Venmo, but it's a little confusing to me since without looking through the code callsites you don't know if this method is for opening the Venmo app, or opening the merchant app.

Take it or leave it comment - but we could rename to handleReturnURL() or something similar

[jaxdesmarais]

That makes sense to me too: 2d53f54