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.

Sign up for GitHub

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 analytics #1231

Merged
merged 12 commits into from
Dec 10, 2024
Merged

Paypal analytics #1231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7f7218c
Add constants
saperi22 Dec 4, 2024
f5a6b23
Add analytics around app switch return to app
saperi22 Dec 4, 2024
6177a2a
remove analytics captured in the wrong place
saperi22 Dec 4, 2024
1362366
update test
saperi22 Dec 4, 2024
0eec949
fix tests
saperi22 Dec 4, 2024
3b98d61
adding test for failure case
saperi22 Dec 5, 2024
94a370a
linter
saperi22 Dec 5, 2024
7385659
add test
saperi22 Dec 5, 2024
4ca4b66
Merge branch 'main' into paypal-analytics
saperi22 Dec 6, 2024
d1bf6e0
fix test
saperi22 Dec 6, 2024
32b963a
Merge branch 'paypal-analytics' of https://github.com/braintree/brain…
saperi22 Dec 6, 2024
50684dc
introduce code removed in merge conflict by mistake
saperi22 Dec 6, 2024
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
3 changes: 3 additions & 0 deletions PayPal/src/main/java/com/braintreepayments/api/paypal/PayPalAnalytics.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ internal object PayPalAnalytics {

// Additional Conversion events
const val HANDLE_RETURN_STARTED = "paypal:tokenize:handle-return:started"
const val HANDLE_RETURN_SUCCEEDED = "paypal:tokenize:handle-return:succeeded"
const val HANDLE_RETURN_FAILED = "paypal:tokenize:handle-return:failed"
const val HANDLE_RETURN_NO_RESULT = "paypal:tokenize:handle-return:no-result"

// App Switch events
const val APP_SWITCH_STARTED = "paypal:tokenize:app-switch:started"
Expand Down
4 changes: 0 additions & 4 deletions PayPal/src/main/java/com/braintreepayments/api/paypal/PayPalClient.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,6 @@ class PayPalClient internal constructor(

if (isAppSwitchFlow) {
Copy link
Contributor Author

@saperi22 saperi22 Dec 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

not sure why HANDLE_RETURN_STARTED was tied to app switch flow, but I'm NOT copying that logic into PayPalLauncher.

appSwitchUrlString = approvalUrl
braintreeClient.sendAnalyticsEvent(
PayPalAnalytics.HANDLE_RETURN_STARTED,
analyticsParams
)
}

approvalUrl?.let {
Expand Down
37 changes: 28 additions & 9 deletions PayPal/src/main/java/com/braintreepayments/api/paypal/PayPalLauncher.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,25 @@ import com.braintreepayments.api.BrowserSwitchClient
import com.braintreepayments.api.BrowserSwitchException
import com.braintreepayments.api.BrowserSwitchFinalResult
import com.braintreepayments.api.BrowserSwitchStartResult
import com.braintreepayments.api.core.AnalyticsClient
import com.braintreepayments.api.core.BraintreeException

/**
* Responsible for launching PayPal user authentication in a web browser
*/
class PayPalLauncher internal constructor(private val browserSwitchClient: BrowserSwitchClient) {
class PayPalLauncher internal constructor(
private val browserSwitchClient: BrowserSwitchClient,
lazyAnalyticsClient: Lazy<AnalyticsClient>
) {
/**
* Used to launch the PayPal flow in a web browser and deliver results to your Activity
*/
constructor() : this(BrowserSwitchClient())
constructor() : this(
browserSwitchClient = BrowserSwitchClient(),
lazyAnalyticsClient = AnalyticsClient.lazyInstance
)

private val analyticsClient: AnalyticsClient by lazyAnalyticsClient

/**
* Launches the PayPal flow by switching to a web browser for user authentication
Expand Down Expand Up @@ -73,17 +82,27 @@ class PayPalLauncher internal constructor(private val browserSwitchClient: Brows
pendingRequest: PayPalPendingRequest.Started,
intent: Intent
): PayPalPaymentAuthResult {
analyticsClient.sendEvent(PayPalAnalytics.HANDLE_RETURN_STARTED)
return when (val browserSwitchResult =
browserSwitchClient.completeRequest(intent, pendingRequest.pendingRequestString)) {
is BrowserSwitchFinalResult.Success -> PayPalPaymentAuthResult.Success(
browserSwitchResult
)
is BrowserSwitchFinalResult.Success -> {
analyticsClient.sendEvent(PayPalAnalytics.HANDLE_RETURN_SUCCEEDED)
PayPalPaymentAuthResult.Success(
browserSwitchResult
)
}

is BrowserSwitchFinalResult.Failure -> PayPalPaymentAuthResult.Failure(
browserSwitchResult.error
)
is BrowserSwitchFinalResult.Failure -> {
analyticsClient.sendEvent(PayPalAnalytics.HANDLE_RETURN_FAILED)
PayPalPaymentAuthResult.Failure(
browserSwitchResult.error
)
}

is BrowserSwitchFinalResult.NoResult -> PayPalPaymentAuthResult.NoResult
is BrowserSwitchFinalResult.NoResult -> {
analyticsClient.sendEvent(PayPalAnalytics.HANDLE_RETURN_NO_RESULT)
PayPalPaymentAuthResult.NoResult
}
}
}

Expand Down
1 change: 0 additions & 1 deletion PayPal/src/test/java/com/braintreepayments/api/paypal/PayPalClientUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

params.setPayPalContextId("EC-HERMES-SANDBOX-EC-TOKEN");
verify(braintreeClient).sendAnalyticsEvent(PayPalAnalytics.TOKENIZATION_SUCCEEDED, params);
AnalyticsEventParams appSwitchParams = new AnalyticsEventParams(
Expand Down
47 changes: 44 additions & 3 deletions PayPal/src/test/java/com/braintreepayments/api/paypal/PayPalLauncherUnitTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import com.braintreepayments.api.BrowserSwitchException
import com.braintreepayments.api.BrowserSwitchFinalResult
import com.braintreepayments.api.BrowserSwitchOptions
import com.braintreepayments.api.BrowserSwitchStartResult
import com.braintreepayments.api.core.AnalyticsClient
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import org.json.JSONException
import org.junit.Assert.assertEquals
import org.junit.Assert.assertSame
Expand All @@ -26,13 +28,14 @@ class PayPalLauncherUnitTest {
private val intent: Intent = mockk(relaxed = true)
private val options: BrowserSwitchOptions = mockk(relaxed = true)
private val pendingRequestString = "pending_request_string"
private val analyticsClient: AnalyticsClient = mockk(relaxed = true)

private lateinit var sut: PayPalLauncher

@Before
fun setup() {
every { paymentAuthRequestParams.browserSwitchOptions } returns options
sut = PayPalLauncher(browserSwitchClient)
sut = PayPalLauncher(browserSwitchClient, lazy { analyticsClient })
}

@Test
Expand Down Expand Up @@ -90,6 +93,16 @@ class PayPalLauncherUnitTest {
)
}

@Test
@Throws(JSONException::class)
fun `handleReturnToApp sends started event`() {
sut.handleReturnToApp(
PayPalPendingRequest.Started(pendingRequestString),
intent
)
verify { analyticsClient.sendEvent(PayPalAnalytics.HANDLE_RETURN_STARTED) }
}

@Test
@Throws(JSONException::class)
fun `handleReturnToApp when result exists returns result`() {
Expand All @@ -102,14 +115,40 @@ class PayPalLauncherUnitTest {
} returns browserSwitchFinalResult

val paymentAuthResult = sut.handleReturnToApp(
PayPalPendingRequest.Started(pendingRequestString), intent
PayPalPendingRequest.Started(pendingRequestString),
intent
)

assertTrue(paymentAuthResult is PayPalPaymentAuthResult.Success)
assertSame(
browserSwitchFinalResult,
(paymentAuthResult as PayPalPaymentAuthResult.Success).browserSwitchSuccess
)
verify { analyticsClient.sendEvent(PayPalAnalytics.HANDLE_RETURN_SUCCEEDED) }
}

@Test
@Throws(JSONException::class)
fun `handleReturnToApp when result fails returns failed result`() {
val browserSwitchFinalResult = mockk<BrowserSwitchFinalResult.Failure>()
every {
browserSwitchClient.completeRequest(intent, pendingRequestString)
} returns browserSwitchFinalResult

val exception = BrowserSwitchException("BrowserSwitchException")
every { browserSwitchFinalResult.error } returns exception

val paymentAuthResult = sut.handleReturnToApp(
PayPalPendingRequest.Started(pendingRequestString),
intent
)

assertTrue(paymentAuthResult is PayPalPaymentAuthResult.Failure)
assertSame(
exception,
(paymentAuthResult as PayPalPaymentAuthResult.Failure).error
)
verify { analyticsClient.sendEvent(PayPalAnalytics.HANDLE_RETURN_FAILED) }
}

@Test
Expand All @@ -120,9 +159,11 @@ class PayPalLauncherUnitTest {
} returns BrowserSwitchFinalResult.NoResult

val paymentAuthResult = sut.handleReturnToApp(
PayPalPendingRequest.Started(pendingRequestString), intent
PayPalPendingRequest.Started(pendingRequestString),
intent
)

assertTrue(paymentAuthResult is PayPalPaymentAuthResult.NoResult)
verify { analyticsClient.sendEvent(PayPalAnalytics.HANDLE_RETURN_NO_RESULT) }
}
}
Loading