FPTI Updates

[jaxdesmarais]

Reason for changes

Update component name to match Braintree formatting (BT PR with this change) as will as send correlation ID where possible (BT PR with this change)

Feedback: currently the correlation ID was only added to the MXO flow. On BT PayPal Web we pull this value from the request (if present) or the Data Collector (source code). PPCP PayPal Web does not have the Data Collector as a dependency - we should consider if we want to add this dependency in a future PR or leave the correlation ID as is - only existing in NXO.

Note: We will make this same change on Android as well.

Summary of changes

• Update component from ppcpmobilesdk to ppcpclientsdk
• Update AnalyticsService to accept an optional correlationID parameter
• Collect correlationID & send in analytics for PayPalNativeCheckoutClient when present

Checklist

• Added a changelog entry

Authors

• @jwarmkessel @jaxdesmarais

[scannillo]

The API requests in the CardModule also return a riskCorrelationID as a response header (see Android).

Should we also log the correlationID in the card module where possible?

It also looks like we're mis-aligned with Android since they return riskCorrelationID in merchant-facing errors and iOS is not (slightly separate concern though).

[jaxdesmarais]

The API requests in the CardModule also return a riskCorrelationID as a response header (see Android).

Adding breakpoints in the request/response I am not seeing those headers. Additionally looking at confirm-payment-source in PPaaS here. It looks like the correlation ID is returned as the debug ID from this endpoint in the response body. I assume that header came from somewhere on the Android side, but not seeing it when triggering an error in the raw response.

Should we also log the correlationID in the card module where possible?

The networking layers and confirm payment source request/response parsing on Android and iOS are pretty divergent right now. I put up this commit which extracts the correlation/debug ID from the httpResponse to allow us to pass it in with errors. Curious to hear your thoughts - it's not the cleanest solution but works well without fully redoing how we make these requests/responses on iOS for cards.

It also looks like we're mis-aligned with Android since they return riskCorrelationID in merchant-facing errors and iOS is not (slightly separate concern though).

I think we can probably punt on this for now. We should align card as a whole more closely across the platforms as they are pretty different right now.

[scannillo]

I think we should 🏈 do this work in a separate PR/ workstream (for parsing out the riskCorrelationID from the body or response headers). If it's in the body, this should be included in the call on line 49, and added to ConfirmPaymentSourceResponse model.

[jaxdesmarais]

If it's in the body, this should be included in the call on like 49, and added to ConfirmPaymentSourceResponse model.

Yeah - that's why I mentioned the entire layer would likely need to be rewritten. It's only returned when there is an error, so we don't actually return a ConfirmPaymentSourceResponse when a debug_id is present since the SDK throws an error as part of the try at that point. Adding it to the response would mean it's always nil since we only get a ConfirmPaymentSourceResponse on success (and there is no debug_id on success according to PPaaS)

[scannillo]

Gotcha, so we only get that value back on errors.

We do have error specific parsing that uses a different underlying Decodable model (triggered here & defined here).

So we would update that model to parse out debug_id.

[scannillo]

Maybe in another ticket we can dig into when we expect that value to come in the error body, or in the response headers and which to prefer when 🏈

[jaxdesmarais]

Yeah - If we add it to those thrown errors (from here), we'd end up needing to parse the error again on the client to extract out the debug ID from the thrown error. It could make more sense to return a result object or closure as part of parseREST as well as update CheckoutOrdersAPI to do the same. Or we can take the same approach as web and just return it with all errors.

It's certainly a bit of an odd series of events and would be a good opportunity to align with Android on! Looking between the two codebases for this certainly illustrated how different this layer is in particular. I'll revert this for now in any case!

[jaxdesmarais]

Reverted: b031319

[scannillo]

Actual changes look good! Thanks for entertaining all of my ❓s!