Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FPTI Updates #208

Merged
merged 14 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@

# PayPal iOS SDK Release Notes

## unreleased
* CorePayments
* Analytics
* Update `component` from `ppcpmobilesdk` to `ppcpclientsdk`
* Send `correlation_id`, when possible, in PayPal Native Checkout analytic events

## 1.0.0 (2023-10-02)
* Breaking Changes
* Require Xcode 14.3+ and Swift 5.8+
Expand Down
11 changes: 9 additions & 2 deletions Sources/CardPayments/APIRequests/CheckoutOrdersAPI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@ import CorePayments
/// Details on this PayPal API can be found in PPaaS under Merchant > Checkout > Orders > v2.
class CheckoutOrdersAPI {

// MARK: - Private Propertires

// MARK: - Internal Properties

var correlationID: String?

// MARK: - Private Properties

private let coreConfig: CoreConfig
private let networkingClient: NetworkingClient

Expand Down Expand Up @@ -39,6 +43,9 @@ class CheckoutOrdersAPI {
)

let httpResponse = try await networkingClient.fetch(request: restRequest)
let httpResponseBodyJSON = try? JSONSerialization.jsonObject(with: httpResponse.body ?? Data()) as? [String: Any]
correlationID = httpResponseBodyJSON?["debug_id"] as? String

return try HTTPResponseParser().parseREST(httpResponse, as: ConfirmPaymentSourceResponse.self)
Copy link
Collaborator

@scannillo scannillo Oct 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Collaborator

@scannillo scannillo Oct 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🏈

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted: b031319

}
}
2 changes: 1 addition & 1 deletion Sources/CardPayments/CardClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public class CardClient: NSObject {
}

private func notifyFailure(with error: CoreSDKError) {
analyticsService?.sendEvent("card-payments:3ds:failed")
analyticsService?.sendEvent("card-payments:3ds:failed", correlationID: checkoutOrdersAPI.correlationID)
delegate?.card(self, didFinishWithError: error)
}

Expand Down
9 changes: 7 additions & 2 deletions Sources/CorePayments/AnalyticsEventData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ struct AnalyticsEventData: Encodable {
case clientSDKVersion = "c_sdk_ver"
case clientOS = "client_os"
case component = "comp"
case correlationID = "correlation_id"
case deviceManufacturer = "device_manufacturer"
case environment = "merchant_sdk_env"
case eventName = "event_name"
Expand All @@ -41,7 +42,9 @@ struct AnalyticsEventData: Encodable {

let clientOS: String = UIDevice.current.systemName + " " + UIDevice.current.systemVersion

let component = "ppcpmobilesdk"
let component = "ppcpclientsdk"
jaxdesmarais marked this conversation as resolved.
Show resolved Hide resolved

let correlationID: String?

let deviceManufacturer = "Apple"

Expand Down Expand Up @@ -90,11 +93,12 @@ struct AnalyticsEventData: Encodable {

let tenantName = "PayPal"

init(environment: String, eventName: String, clientID: String, orderID: String) {
init(environment: String, eventName: String, clientID: String, orderID: String, correlationID: String?) {
self.environment = environment
self.eventName = eventName
self.clientID = clientID
self.orderID = orderID
self.correlationID = correlationID
}

func encode(to encoder: Encoder) throws {
Expand All @@ -108,6 +112,7 @@ struct AnalyticsEventData: Encodable {
try eventParameters.encode(clientSDKVersion, forKey: .clientSDKVersion)
try eventParameters.encode(clientOS, forKey: .clientOS)
try eventParameters.encode(component, forKey: .component)
try eventParameters.encode(correlationID, forKey: .correlationID)
try eventParameters.encode(deviceManufacturer, forKey: .deviceManufacturer)
try eventParameters.encode(environment, forKey: .environment)
try eventParameters.encode(eventName, forKey: .eventName)
Expand Down
14 changes: 9 additions & 5 deletions Sources/CorePayments/Networking/AnalyticsService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,31 @@ public struct AnalyticsService {
// MARK: - Public Methods

/// This method is exposed for internal PayPal use only. Do not use. It is not covered by Semantic Versioning and may change or be removed at any time.
///
/// Sends analytics event to https://api.paypal.com/v1/tracking/events/ via a background task.
/// - Parameter name: Event name string used to identify this unique event in FPTI.
public func sendEvent(_ name: String) {
/// - Parameter correlationID: correlation ID associated with the request
public func sendEvent(_ name: String, correlationID: String? = nil) {
Task(priority: .background) {
await performEventRequest(name)
await performEventRequest(name, correlationID: correlationID)
}
}

// MARK: - Internal Methods

/// Exposed to be able to execute this function synchronously in unit tests
func performEventRequest(_ name: String) async {
/// - Parameters:
/// - name: Event name string used to identify this unique event in FPTI
/// - correlationID: correlation ID associated with the request
func performEventRequest(_ name: String, correlationID: String? = nil) async {
scannillo marked this conversation as resolved.
Show resolved Hide resolved
do {
let clientID = coreConfig.clientID

let eventData = AnalyticsEventData(
environment: coreConfig.environment.toString,
eventName: name,
clientID: clientID,
orderID: orderID
orderID: orderID,
correlationID: correlationID
)

let (_) = try await trackingEventsAPI.sendEvent(with: eventData)
Expand Down
4 changes: 2 additions & 2 deletions Sources/CorePayments/Networking/HTTPResponse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Foundation
public struct HTTPResponse {

let status: Int
let body: Data?
public let body: Data?

var isSuccessful: Bool { (200..<300).contains(status) }
}
5 changes: 5 additions & 0 deletions Sources/PayPalNativePayments/NativeCheckoutProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import CorePayments

class NativeCheckoutProvider: NativeCheckoutStartable {

/// Used in POST body for FPTI analytics.
var correlationID: String?

private let checkout: CheckoutProtocol.Type

init(_ mxo: CheckoutProtocol.Type = Checkout.self) {
Expand All @@ -32,6 +35,7 @@ class NativeCheckoutProvider: NativeCheckoutStartable {
createOrderAction.set(orderId: orderID)
},
onApprove: { approval in
self.correlationID = approval.data.correlationIDs.riskCorrelationID
onStartableApprove(approval.data.ecToken, approval.data.payerID)
},
onShippingChange: { shippingChangeData, shippingChangeActions in
Expand All @@ -46,6 +50,7 @@ class NativeCheckoutProvider: NativeCheckoutStartable {
},
onCancel: { onStartableCancel() },
onError: { error in
self.correlationID = error.correlationIDs.riskCorrelationID
onStartableError(error.reason)
}
)
Expand Down
6 changes: 4 additions & 2 deletions Sources/PayPalNativePayments/NativeCheckoutStartable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import CorePayments
#endif

protocol NativeCheckoutStartable {


/// Used in POST body for FPTI analytics.
var correlationID: String? { get set }

typealias StartableApproveCallback = (String, String) -> Void
typealias StartableShippingCallback = (
ShippingChangeType,
Expand All @@ -17,7 +20,6 @@ protocol NativeCheckoutStartable {
typealias StartableCancelCallback = () -> Void
typealias StartableErrorCallback = (String) -> Void


// swiftlint:disable:next function_parameter_count
func start(
presentingViewController: UIViewController?,
Expand Down
13 changes: 9 additions & 4 deletions Sources/PayPalNativePayments/PayPalNativeCheckoutClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ public class PayPalNativeCheckoutClient {

public weak var delegate: PayPalNativeCheckoutDelegate?
public weak var shippingDelegate: PayPalNativeShippingDelegate?

/// Used in POST body for FPTI analytics.
private var correlationID: String?
private let nativeCheckoutProvider: NativeCheckoutStartable
private let networkingClient: NetworkingClient
private let config: CoreConfig
Expand Down Expand Up @@ -40,6 +43,7 @@ public class PayPalNativeCheckoutClient {
request: PayPalNativeCheckoutRequest,
presentingViewController: UIViewController? = nil
) async {
correlationID = State.correlationIDs.riskCorrelationID
analyticsService = AnalyticsService(coreConfig: config, orderID: request.orderID)

let nxoConfig = CheckoutConfig(
Expand All @@ -62,6 +66,7 @@ public class PayPalNativeCheckoutClient {
orderID: ecToken,
payerID: payerID
)

self.notifySuccess(for: result)
},
onStartableShippingChange: { shippingType, shippingAction, shippingAddress, shippingMethod in
Expand Down Expand Up @@ -91,19 +96,19 @@ public class PayPalNativeCheckoutClient {
}

private func notifySuccess(for result: PayPalNativeCheckoutResult) {
analyticsService?.sendEvent("paypal-native-payments:succeeded")
analyticsService?.sendEvent("paypal-native-payments:succeeded", correlationID: nativeCheckoutProvider.correlationID)
delegate?.paypal(self, didFinishWithResult: result)
}

private func notifyFailure(with errorDescription: String) {
analyticsService?.sendEvent("paypal-native-payments:failed")
analyticsService?.sendEvent("paypal-native-payments:failed", correlationID: nativeCheckoutProvider.correlationID)

let error = PayPalNativePaymentsError.nativeCheckoutSDKError(errorDescription)
delegate?.paypal(self, didFinishWithError: error)
}

private func notifyCancellation() {
analyticsService?.sendEvent("paypal-native-payments:canceled")
analyticsService?.sendEvent("paypal-native-payments:canceled", correlationID: correlationID)
delegate?.paypalDidCancel(self)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ import PayPalCheckout
import XCTest

class MockNativeCheckoutProvider: NativeCheckoutStartable {

required init(nxoConfig: CheckoutConfig) {
}


var correlationID: String?
var onCancel: StartableCancelCallback?
var onError: StartableErrorCallback?
var onApprove: StartableApproveCallback?
var onShippingChange: StartableShippingCallback?

required init(nxoConfig: CheckoutConfig) { }

// todo: implemenet cases for other callbacks
func start(
presentingViewController: UIViewController?,
Expand Down
6 changes: 4 additions & 2 deletions UnitTests/PaymentsCoreTests/AnalyticsEventData_Tests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ class AnalyticsEventData_Tests: XCTestCase {
environment: "fake-env",
eventName: "fake-name",
clientID: "fake-client-id",
orderID: "fake-order"
orderID: "fake-order",
correlationID: "fake-correlation-id"
)
}

Expand All @@ -31,7 +32,8 @@ class AnalyticsEventData_Tests: XCTestCase {
XCTAssertEqual(eventParams["app_name"] as? String, "xctest")
XCTAssertTrue((eventParams["c_sdk_ver"] as! String).matches("^\\d+\\.\\d+\\.\\d+(-[0-9a-zA-Z-]+)?$"))
XCTAssertTrue((eventParams["client_os"] as! String).matches("iOS \\d+\\.\\d+|iPadOS \\d+\\.\\d+"))
XCTAssertEqual(eventParams["comp"] as? String, "ppcpmobilesdk")
XCTAssertEqual(eventParams["comp"] as? String, "ppcpclientsdk")
XCTAssertEqual(eventParams["correlation_id"] as! String, "fake-correlation-id")
XCTAssertEqual(eventParams["device_manufacturer"] as? String, "Apple")
XCTAssertEqual(eventParams["merchant_sdk_env"] as? String, "fake-env")
XCTAssertEqual(eventParams["event_name"] as? String, "fake-name")
Expand Down
5 changes: 3 additions & 2 deletions UnitTests/PaymentsCoreTests/AnalyticsService_Tests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ class AnalyticsService_Tests: XCTestCase {
// MARK: - sendEvent()

func testSendEvent_sendsAppropriateAnalyticsEventData() async {
await sut.performEventRequest("some-event")
await sut.performEventRequest("some-event", correlationID: "fake-correlation-id")

XCTAssertEqual(mockTrackingEventsAPI.capturedAnalyticsEventData?.eventName, "some-event")
XCTAssertEqual(mockTrackingEventsAPI.capturedAnalyticsEventData?.clientID, "some-client-id")
XCTAssertEqual(mockTrackingEventsAPI.capturedAnalyticsEventData?.orderID, "some-order-id")
XCTAssertEqual(mockTrackingEventsAPI.capturedAnalyticsEventData?.correlationID, "fake-correlation-id")
}

func testSendEvent_whenLive_sendsAppropriateEnvName() async {
Expand Down
7 changes: 5 additions & 2 deletions UnitTests/PaymentsCoreTests/TrackingEventsAPI_Tests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ class TrackingEventsAPI_Tests: XCTestCase {
environment: "my-env",
eventName: "my-event-name",
clientID: "my-id",
orderID: "my-order"
orderID: "my-order",
correlationID: nil
)

// MARK: - Test Lifecycle
Expand All @@ -41,7 +42,8 @@ class TrackingEventsAPI_Tests: XCTestCase {
environment: "my-env",
eventName: "my-event-name",
clientID: "my-id",
orderID: "my-order"
orderID: "my-order",
correlationID: "fake-correlation-id"
)
_ = try await sut.sendEvent(with: fakeAnalyticsEventData)

Expand All @@ -54,6 +56,7 @@ class TrackingEventsAPI_Tests: XCTestCase {
XCTAssertEqual(postData.eventName, "my-event-name")
XCTAssertEqual(postData.clientID, "my-id")
XCTAssertEqual(postData.orderID, "my-order")
XCTAssertEqual(postData.correlationID, "fake-correlation-id")
}

func testSendEvent_whenSuccess_bubblesHTTPResponse() async throws {
Expand Down
Loading