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

Shopper insights rp1 feature include session #1235

Merged
merged 18 commits into from
Dec 9, 2024
Merged

Shopper insights rp1 feature include session #1235

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
7dedb0a
Add shopper insights session ID to paypalRequest
warmkesselj Dec 3, 2024
dedaf01
Update factory for paypal request
warmkesselj Dec 3, 2024
b1f1767
remove extra setting of email address
warmkesselj Dec 3, 2024
ebb92f1
Remove optional null and check for null in PayPalRequestFactory
warmkesselj Dec 3, 2024
7b06a23
Use String type and fix check for null to prevent run time crashes
warmkesselj Dec 3, 2024
a51d13b
Merge branch 'shopper-insights-rp1-feature' of https://github.com/bra…
warmkesselj Dec 6, 2024
737f16f
Remove DEMO UI updates
warmkesselj Dec 6, 2024
c4437cd
Update comment for shopperSessionId
warmkesselj Dec 6, 2024
c6b72d5
Add experimentalAPI for shopper session Id
warmkesselj Dec 6, 2024
e548542
Remove unused shopperInsightsSessionIdNullSwitch
warmkesselj Dec 6, 2024
c9f419a
Check for null and empty string
warmkesselj Dec 6, 2024
179e35b
Update to use putOpt method for shopperId
warmkesselj Dec 6, 2024
152b11e
Update changelog
warmkesselj Dec 6, 2024
073938b
Check for null and empty strings for request properties.
warmkesselj Dec 6, 2024
fe9def0
Update unit tests
warmkesselj Dec 6, 2024
41a47b0
Update test
warmkesselj Dec 6, 2024
c5c0c21
Update unit tests
warmkesselj Dec 7, 2024
41c4f17
Update changelog
warmkesselj Dec 9, 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
6 changes: 4 additions & 2 deletions Demo/src/main/java/com/braintreepayments/demo/PayPalFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,17 @@ private void launchPayPal(
activity,
buyerEmailAddress,
buyerPhoneCountryCode,
buyerPhoneNationalNumber
buyerPhoneNationalNumber,
null
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it looks like there are some extra spaces for the 2 null properties here

);
} else {
payPalRequest = createPayPalCheckoutRequest(
activity,
amount,
buyerEmailAddress,
buyerPhoneCountryCode,
buyerPhoneNationalNumber
buyerPhoneNationalNumber,
null
);
}
payPalClient.createPaymentAuthRequest(requireContext(), payPalRequest,
Expand Down
23 changes: 17 additions & 6 deletions Demo/src/main/java/com/braintreepayments/demo/PayPalRequestFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,32 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;

public class PayPalRequestFactory {

public static PayPalVaultRequest createPayPalVaultRequest(
Context context,
String buyerEmailAddress,
String buyerPhoneCountryCode,
String buyerPhoneNationalNumber
String buyerPhoneNationalNumber,
String shopperInsightsSessionId
) {

PayPalVaultRequest request = new PayPalVaultRequest(true);

if (!buyerEmailAddress.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to change all these other params? It seems like it should be unaffected by this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is that the original code is potentially a run time crash if the value is null. In the case of the demo app and how its setup, setting the toggles to null didn't have an impact on this flow because the values would always be true.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok in that case do we need to check that it's not null and it's not empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, great catch!

if (buyerEmailAddress != null) {
request.setUserAuthenticationEmail(buyerEmailAddress);
}

if (!buyerPhoneCountryCode.isEmpty() && !buyerPhoneNationalNumber.isEmpty()) {
if (buyerPhoneCountryCode != null && buyerPhoneNationalNumber != null) {
request.setUserPhoneNumber(new PayPalPhoneNumber(buyerPhoneCountryCode, buyerPhoneNationalNumber));
}

if (shopperInsightsSessionId != null) {
request.setShopperSessionId(shopperInsightsSessionId);
}

if (Settings.isPayPalAppSwithEnabled(context)) {
request.setEnablePayPalAppSwitch(true);
}
Expand Down Expand Up @@ -111,18 +117,23 @@ public static PayPalCheckoutRequest createPayPalCheckoutRequest(
String amount,
String buyerEmailAddress,
String buyerPhoneCountryCode,
String buyerPhoneNationalNumber
String buyerPhoneNationalNumber,
String shopperInsightsSessionId
) {
PayPalCheckoutRequest request = new PayPalCheckoutRequest(amount, true);

if (!buyerEmailAddress.isEmpty()) {
if (buyerEmailAddress != null) {
request.setUserAuthenticationEmail(buyerEmailAddress);
}

if (!buyerPhoneCountryCode.isEmpty() && !buyerPhoneNationalNumber.isEmpty()) {
if (buyerPhoneCountryCode != null && buyerPhoneNationalNumber != null) {
request.setUserPhoneNumber(new PayPalPhoneNumber(buyerPhoneCountryCode, buyerPhoneNationalNumber));
}

if (shopperInsightsSessionId != null) {
request.setShopperSessionId(shopperInsightsSessionId);
}

request.setDisplayName(Settings.getPayPalDisplayName(context));

String landingPageType = Settings.getPayPalLandingPageType(context);
Expand Down
9 changes: 7 additions & 2 deletions Demo/src/main/java/com/braintreepayments/demo/ShopperInsightsFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class ShopperInsightsFragment : BaseFragment() {
private lateinit var nationalNumberInput: TextInputLayout
private lateinit var emailNullSwitch: SwitchMaterial
private lateinit var phoneNullSwitch: SwitchMaterial
private lateinit var shopperInsightsSessionIdNullSwitch: SwitchMaterial

private lateinit var shopperInsightsClient: ShopperInsightsClient
private lateinit var payPalClient: PayPalClient
Expand All @@ -60,12 +61,14 @@ class ShopperInsightsFragment : BaseFragment() {
private lateinit var venmoStartedPendingRequest: VenmoPendingRequest.Started
private lateinit var paypalStartedPendingRequest: PayPalPendingRequest.Started

private var sessionId: String = "test-shopper-session-id"

override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?
): View? {
shopperInsightsClient = ShopperInsightsClient(requireContext(), authStringArg, "test-shopper-session-id")
shopperInsightsClient = ShopperInsightsClient(requireContext(), authStringArg, sessionId)

venmoClient = VenmoClient(requireContext(), super.getAuthStringArg(), null)
payPalClient = PayPalClient(
Expand Down Expand Up @@ -229,7 +232,9 @@ class ShopperInsightsFragment : BaseFragment() {
activity,
emailInput.editText?.text.toString(),
countryCodeInput.editText?.text.toString(),
nationalNumberInput.editText?.text.toString()
nationalNumberInput.editText?.text.toString(),
sessionId

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

)
) { authRequest ->
when (authRequest) {
Expand Down
4 changes: 4 additions & 0 deletions PayPal/src/main/java/com/braintreepayments/api/paypal/PayPalCheckoutRequest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ class PayPalCheckoutRequest @JvmOverloads constructor(

userPhoneNumber?.let { parameters.put(PHONE_NUMBER_KEY, it.toJson()) }

shopperSessionId?.let {
if (it.isNotEmpty()) parameters.put(SHOPPER_SESSION_ID, it)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to use the same simplified putOpt logic from the PayPalVaultRequest here


if (currencyCode == null) {
currencyCode = configuration?.payPalCurrencyIsoCode
}
Expand Down
4 changes: 4 additions & 0 deletions PayPal/src/main/java/com/braintreepayments/api/paypal/PayPalRequest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ import org.json.JSONException
* where the user has a PayPal Account with the same email.
* @property userPhoneNumber User phone number used to initiate a quicker authentication flow in
* cases where the user has a PayPal Account with the phone number.
* @property shopperSessionId value should be the shopper session ID returned from your server SDK
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this doc string was included in the ticket but I'm wondering if it's clear enough for when we are outside of the shopper insights module. Should we include something like the shopper session ID returned from your shopper insights server SDK integration?. Also should this param be beta/experimental API @jaxdesmarais?

Copy link
Contributor Author

@warmkesselj warmkesselj Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I like it.
Current:
shopperSessionId value should be the shopper session ID returned from your server SDK request.
To:
shopperSessionId the shopper session ID returned from your shopper insights server SDK integration

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'll make the changes in iOS as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep we can for sure give this the beta/experimental API tag

* request.
* @property lineItems The line items for this transaction. It can include up to 249 line items.
*/
abstract class PayPalRequest internal constructor(
Expand All @@ -87,6 +89,7 @@ abstract class PayPalRequest internal constructor(
open var riskCorrelationId: String? = null,
open var userAuthenticationEmail: String? = null,
open var userPhoneNumber: PayPalPhoneNumber? = null,
open var shopperSessionId: String? = null,
open var lineItems: List<PayPalLineItem> = emptyList()
) : Parcelable {

Expand Down Expand Up @@ -132,5 +135,6 @@ abstract class PayPalRequest internal constructor(
internal const val PLAN_TYPE_KEY: String = "plan_type"
internal const val PLAN_METADATA_KEY: String = "plan_metadata"
internal const val PHONE_NUMBER_KEY: String = "phone_number"
internal const val SHOPPER_SESSION_ID: String = "shopper_session_id"
}
}
2 changes: 2 additions & 0 deletions PayPal/src/main/java/com/braintreepayments/api/paypal/PayPalVaultRequest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ class PayPalVaultRequest

parameters.putOpt(PAYER_EMAIL_KEY, userAuthenticationEmail)

parameters.putOpt(SHOPPER_SESSION_ID, shopperSessionId)

userPhoneNumber?.let { parameters.put(PHONE_NUMBER_KEY, it.toJson()) }

if (enablePayPalAppSwitch && !appLink.isNullOrEmpty() && !userAuthenticationEmail.isNullOrEmpty()) {
Expand Down
Loading