Update Venmo with FPTI Analytics Events #846

sarahkoop · 2023-12-07T17:05:22Z

Summary of changes

Update analytics events throughout Venmo flow with new FPTI analytics events

Checklist

~~[ ] Added a changelog entry~~

Authors

@sarahkoop

sarahkoop · 2023-12-07T17:06:26Z

Venmo/src/main/java/com/braintreepayments/api/VenmoClient.java

        braintreeClient.getConfiguration((configuration, error) -> {
            if (configuration == null && error != null) {
                callback.onVenmoPaymentAuthRequest(new VenmoPaymentAuthRequest.Failure(error));
-                braintreeClient.sendAnalyticsEvent("pay-with-venmo.app-switch.failed");
+                braintreeClient.sendAnalyticsEvent(VenmoAnalytics.TOKENIZE_FAILED.event);


We used "app-switch" failed more previously, but to be more accurate, I updated this to only be "app switch failed" for errors returned from actual app switching, or if Venmo is not installed.

sshropshire · 2023-12-07T17:20:20Z

Venmo/src/main/java/com/braintreepayments/api/VenmoClient.java

@@ -110,7 +111,7 @@ public void createPaymentAuthRequest(@NonNull final FragmentActivity activity,
                callback.onVenmoPaymentAuthRequest(new VenmoPaymentAuthRequest.Failure(new BraintreeException(
                        "Cannot collect customer data when ECD is disabled. Enable this feature " +
                                "in the Control Panel to collect this data.")));
-                braintreeClient.sendAnalyticsEvent("pay-with-venmo.app-switch.failed");
+                braintreeClient.sendAnalyticsEvent(VenmoAnalytics.TOKENIZE_FAILED.event);


Suggested change

braintreeClient.sendAnalyticsEvent(VenmoAnalytics.TOKENIZE_FAILED.event);

braintreeClient.sendAnalyticsEvent(VenmoAnalytics.TOKENIZE_FAILED);

nit: Would it be better to convert VenmoAnalytics.TOKENIZE_FAILED to a String and remove the .event property access?

I could rename the "event" property in this enum, or what you're describing sounds like a class with a companion object filled with string constants rather than an enum - do you think that would be better?

Yeah I'd go companion const val to reduce some of the boilerplate. If we were performing a switch / when on it i'd keep it an enum, but for braintreeClient.sendAnalyticsEvent() a plain string value is equally sufficient.

Cool - updated 👍

scannillo · 2023-12-07T19:23:34Z

Venmo/src/main/java/com/braintreepayments/api/VenmoClient.java

@@ -110,7 +111,7 @@ public void createPaymentAuthRequest(@NonNull final FragmentActivity activity,
                callback.onVenmoPaymentAuthRequest(new VenmoPaymentAuthRequest.Failure(new BraintreeException(
                        "Cannot collect customer data when ECD is disabled. Enable this feature " +
                                "in the Control Panel to collect this data.")));
-                braintreeClient.sendAnalyticsEvent("pay-with-venmo.app-switch.failed");
+                braintreeClient.sendAnalyticsEvent(VenmoAnalytics.TOKENIZE_FAILED);


On BT iOS (and PPCP Android & iOS) we moved to this "notify pattern" (see BT iOS & PPCP Android examples) for firing analytics. The goal is that each analytics string constant should only be referenced in 1 place in source code.

The purpose was to reduce errors in our firing of the events, since we have previous SDK version with bugs for forgetting to send analytics in certain cases. This way we know that every time we return a callback result/cancel/error to the merchant, we're firing the right event (and only once).

What do you think of this?

I think that's a good idea - except for Android since we have two different callbacks for the two different parts of the flow, we'd need 5 analytics helper methods (one success, and then a cancel and failure for each callback) - do you still think that would be helpful?

On second thought, to make sure we always fire an analytics event with each completion callback, I think it makes sense even if it requires 5 methods

scannillo · 2023-12-08T04:37:45Z

Venmo/src/main/java/com/braintreepayments/api/VenmoClient.java

+    private void callbackPaymentAuthAppSwitchFailure(VenmoPaymentAuthRequestCallback callback, VenmoPaymentAuthRequest request) {
+        braintreeClient.sendAnalyticsEvent(VenmoAnalytics.APP_SWITCH_FAILED);
+        callback.onVenmoPaymentAuthRequest(request);
+    }
+
+    private void callbackPaymentAuthFailure(VenmoPaymentAuthRequestCallback callback, VenmoPaymentAuthRequest request) {
+        braintreeClient.sendAnalyticsEvent(VenmoAnalytics.TOKENIZE_FAILED);
+        callback.onVenmoPaymentAuthRequest(request);
+    }


If the app switch fails, I think we want that to count as a holistic "tokenize_failed", right? For that case on iOS we did this, and send both a "conversion" event (tokenize_failed) + the "extra details" event (app_switch_failed).

This way we can use the "conversion" event of tokenize_failed to capture all failures, to use to measure against the total # of "start" events.

Though we went back and forth a few times on this with iOS, so definitely down to talk through this and come up with a pattern that makes sense across both.

Ok agreed I think that makes sense - updated the app switch failure to send both the conversion analytic event and the app switch failure one. I agree with that approach for all the events 👍

MaxHastingsPP · 2023-12-11T17:06:58Z

Venmo/src/test/java/com/braintreepayments/api/VenmoClientUnitTest.java

@@ -77,19 +76,6 @@ public void showVenmoInGooglePlayStore_opensVenmoAppStoreURL() {
                "https://play.google.com/store/apps/details?id=com.venmo");
    }

-    @Test
-    public void showVenmoInGooglePlayStore_sendsAnalyticsEvent() {


Why are we removing this unit test ?

With the switch to FPTI analytics we trimmed down some of the extraneous analytics events not related to conversion metrics. This analytics event is no longer being sent, and we test the actual functionality of this method in a separate unit test, so removed this one.

I see. Should we consider removing this line of code in VenmoClient Line 60?

braintreeClient.sendAnalyticsEvent("android.pay-with-venmo.app-store.invoked");

Ah yes good catch! That was meant to be removed.

Wild - this same event slipped through the cracks on iOS too (see code snippet). So we actually are still sending it on iOS v6.

Do we think this is a useful event? Should we add it to our events doc?

cc: @jaxdesmarais

Discussed and confirmed we will leave this event out for now 👍

sarahkoop added 4 commits December 7, 2023 09:34

Replace Venmo analytics with FPTI events

1f37c59

Update unit tests

e13dc5d

Add new unit tests

c722c9d

Fix unit tests

5bb6ce5

sarahkoop requested a review from a team as a code owner December 7, 2023 17:05

sarahkoop commented Dec 7, 2023

View reviewed changes

sshropshire reviewed Dec 7, 2023

View reviewed changes

sshropshire approved these changes Dec 7, 2023

View reviewed changes

sarahkoop added 2 commits December 7, 2023 11:45

Convert VenmoAnalytics to class

7a9fec8

Remove companion object

2d5cfe0

scannillo reviewed Dec 7, 2023

View reviewed changes

Add helper analtyics methods

ef8b821

scannillo reviewed Dec 8, 2023

View reviewed changes

Update app switch failure analytics

8de3d07

MaxHastingsPP reviewed Dec 11, 2023

View reviewed changes

Remove extra analytics event

de18ecd

MaxHastingsPP approved these changes Dec 11, 2023

View reviewed changes

scannillo mentioned this pull request Dec 11, 2023

Remove missed Venmo app store analytics event braintree/braintree_ios#1155

Merged

sarahkoop merged commit 69b62cb into v5 Dec 11, 2023
2 checks passed

sarahkoop deleted the venmo_fpti branch December 11, 2023 20:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Venmo with FPTI Analytics Events #846

Update Venmo with FPTI Analytics Events #846

sarahkoop commented Dec 7, 2023

sarahkoop Dec 7, 2023

sshropshire Dec 7, 2023

sarahkoop Dec 7, 2023

sshropshire Dec 7, 2023

sarahkoop Dec 7, 2023

scannillo Dec 7, 2023

sarahkoop Dec 7, 2023

sarahkoop Dec 7, 2023

scannillo Dec 8, 2023

sarahkoop Dec 11, 2023

MaxHastingsPP Dec 11, 2023

sarahkoop Dec 11, 2023

MaxHastingsPP Dec 11, 2023

sarahkoop Dec 11, 2023

scannillo Dec 11, 2023

scannillo Dec 11, 2023

sarahkoop Dec 11, 2023

	braintreeClient.sendAnalyticsEvent(VenmoAnalytics.TOKENIZE_FAILED.event);
	braintreeClient.sendAnalyticsEvent(VenmoAnalytics.TOKENIZE_FAILED);

Update Venmo with FPTI Analytics Events #846

Update Venmo with FPTI Analytics Events #846

Conversation

sarahkoop commented Dec 7, 2023

Summary of changes

Checklist

Authors

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment