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.

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

Add Shopper Session ID param #1241

Merged
merged 7 commits into from
Dec 16, 2024
Merged

Conversation

sarahkoop
Copy link
Contributor

@sarahkoop sarahkoop commented Dec 16, 2024

Summary of changes

  • Remove previous updates to pass existing sessionId for shopper insights
  • Add new shopper_session_id FPTI tag to be used specifically for shopper insights and related payment flows
  • Tested manually and verified shopper_session_id is getting to FPTI as expected
  • Aligns with iOS PR: ShopperInsights - send shopper_session_id to FPTI braintree_ios#1485

Checklist

  • Added a changelog entry
  • Relevant test coverage
  • Tested and confirmed payment flows affected by this change are functioning as expected

Authors

@sarahkoop

@sarahkoop sarahkoop requested a review from a team as a code owner December 16, 2024 16:45
@sarahkoop sarahkoop changed the title Session id fixup Add Shopper Session ID param Dec 16, 2024
/**
* Used for sending Shopper Insights session ID provided by merchant to FPTI
*/
private var shopperSessionId: String? = null
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we save this in a repository singleton outside of the Client class? Maybe a PayPalRepository that holds all PayPal specific values.

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 considered adding it to the AnalyticsParamRepository but since it is passed in by the merchant at each step there didn't seem much value there and it seemed a little confusing that we don't do that for any other general analytics event params. Open to a new param repository or adding it to the existing one if folks think that would be more helpful!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah gotcha. That makes sense 👍

@@ -17,6 +17,7 @@ import androidx.annotation.RestrictTo
* the experiment, as a JSON string, that the merchant sent to the us.
* @property paymentMethodsDisplayed A ShopperInsights module specific event that indicates the
* order of payment methods displayed to the shopper by the merchant.
* @property shopperSessionId A ShopperInsights session ID provided by the merchant
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: do we want to match iOS?
/// The Shopper Insights customer session ID created by a merchant's server SDK or graphQL 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.

Updated 👍

@sarahkoop sarahkoop merged commit 026b2f5 into shopper-insights-rp1-feature Dec 16, 2024
3 checks passed
@sarahkoop sarahkoop deleted the session-id-fixup branch December 16, 2024 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants