-
Notifications
You must be signed in to change notification settings - Fork 295
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
Paypal Messaging Analytics #1161
Paypal Messaging Analytics #1161
Conversation
Co-authored-by: scannillo <[email protected]>
@@ -141,10 +142,12 @@ extension BTPayPalMessagingClient: PayPalMessageViewEventDelegate, PayPalMessage | |||
} | |||
|
|||
public func onSuccess(_ paypalMessageView: PayPalMessages.PayPalMessageView) { | |||
apiClient.sendAnalyticsEvent(BTPayPalMessagingAnalytics.succeeded) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically we have used the notify
pattern in other PRs. Open to using that pattern here if folks prefer but to me it seemed odd to extract out the delegate call below and the analytics event for just success and error while keeping the rest of the delegate calls above when we aren't sending analytics.
…tree/braintree_ios into paypal-messaging-analytics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, let's keep it simple and build out from there.
Do we have test coverage for events being sent?
There is a follow up ticket as a whole for testing this feature. Since this is going into a feature branch we wanted to keep the PRs small/digestible. But there will be tests - that will be the next PR once the current 3 are merged in. 😄 |
@jaxdesmarais ahhhh I see. Carry on! |
# Conflicts: # Braintree.xcodeproj/project.pbxproj # CHANGELOG.md # Demo/Application/Features/PayPalMessagingViewController.swift # Sources/BraintreePayPalMessaging/BTPayPalMessagingDelegate.swift # Sources/BraintreePayPalMessaging/BTPayPalMessagingError.swift # Sources/BraintreePayPalMessaging/BTPayPalMessagingLogoType.swift # Sources/BraintreePayPalMessaging/BTPayPalMessagingRequest.swift # Sources/BraintreePayPalMessaging/BTPayPalMessagingView.swift
# Conflicts: # Braintree.xcodeproj/project.pbxproj # CHANGELOG.md # Demo/Application/Features/PayPalMessagingViewController.swift # Sources/BraintreePayPalMessaging/BTPayPalMessagingDelegate.swift # Sources/BraintreePayPalMessaging/BTPayPalMessagingError.swift # Sources/BraintreePayPalMessaging/BTPayPalMessagingLogoType.swift # Sources/BraintreePayPalMessaging/BTPayPalMessagingRequest.swift # Sources/BraintreePayPalMessaging/BTPayPalMessagingView.swift
This work is going into the
paypal-messaging-feature
branchSummary of changes
started
,failed
, andsucceeded
events for PayPal Messaging featureChecklist
[ ] Added a changelog entryAuthors