Paypal analytics #1231
Paypal analytics #1231
Conversation
| @@ -212,13 +212,6 @@ class PayPalClient internal constructor( | |||
| val switchInitiatedTime = Uri.parse(approvalUrl).getQueryParameter("switch_initiated_time") | |||
| val isAppSwitchFlow = !switchInitiatedTime.isNullOrEmpty() | |||
|
|
|||
| if (isAppSwitchFlow) { | |||
There was a problem hiding this comment.
not sure why HANDLE_RETURN_STARTED was tied to app switch flow, but I'm NOT copying that logic into PayPalLauncher.
| @@ -625,7 +625,6 @@ public void tokenize_whenPayPalInternalClientTokenizeResult_sendsAppSwitchSuccee | |||
| assertEquals(payPalAccountNonce, ((PayPalResult.Success) result).getNonce()); | |||
|
|
|||
| AnalyticsEventParams params = new AnalyticsEventParams(); | |||
| verify(braintreeClient).sendAnalyticsEvent(PayPalAnalytics.HANDLE_RETURN_STARTED, params); | |||
There was a problem hiding this comment.
Curious why we needed to remove this check from this tests. It looks like this test returns APP_SWITCH_SUCCEEDED which I would expect also to have a HANDLE_RETURN_STARTED event, but maybe am misunderstanding the updates.
There was a problem hiding this comment.
We were previously sending the events on tokenize() method in the PayPalClient, due to some limitations around code structuring.
See: https://github.com/braintree/braintree_android/pull/1231/files#diff-51d311cd1ffbe27893d893c2eaed2c00c8ff26cb497f6d5f2cd1b7eb2e42e00aL222
I removed that logic.
After some refactor by @tdchow, we are now able to send the events from PayPalLauncher. I've moved the logic there.
https://github.com/braintree/braintree_android/pull/1231/files#diff-c4a51a1d2f652fee6eef372ea3a5d1e232de64156bc91c97cf5b4f45f7420e2bR98
There was a problem hiding this comment.
Gotcha, so there is no way in this test to mock calling PayPalLauncher to verify this event is sent with the app switch success events? If not that's fine, moreso for my understanding since on iOS we don't have an internal vs external client.
There was a problem hiding this comment.
I may not have understood your question correctly.
Since these are 'unit' tests, PayPalClient wouldn't have any knowledge of what PayPalLauncher is doing.
I've moved the event logging into PayPalLauncher and I've a test in its tests. See: https://github.com/braintree/braintree_android/pull/1231/files/50684dcbba1170da5419d61df0a7d882f0e82959#diff-c4a51a1d2f652fee6eef372ea3a5d1e232de64156bc91c97cf5b4f45f7420e2bR103
Does that answer your question? If not, we can sync off-comment; online.
|
@saperi22 to add the app switch url to the analytics params. |
Will add it as part of a separate PR to get reviews. |
Summary of changes
Checklist
Authors