-
Notifications
You must be signed in to change notification settings - Fork 62
Make CCO Call for all PayPal Web Checkout Funding Sources #332
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
base: bcdc-paypal-payments
Are you sure you want to change the base?
Make CCO Call for all PayPal Web Checkout Funding Sources #332
Conversation
private object Defaults { | ||
const val INTEGRATION_ARTIFACT = "MOBILE_SDK" | ||
const val USER_EXPERIENCE_FLOW = "INCONTEXT" | ||
const val PRODUCT_FLOW = "MOBILE_SDK" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We landed on HERMES for product flow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also added channel to be explicit.
We had relied on defaults in XOR before but I wanted to be as explicit as possible
because they are respecting our CCO values sent in from us now. (I think on version soon to be released)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok is Nirvan aware of this change? The ticket has MOBILE_SDK
for the PRODUCT_FLOW
field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think the original ticket was made a while ago. And Nirvan was involved in all of the discussions with those teams. Apologies, I didn't realize that the ticket was not updated.
We are going to do some sandbox testing once XOR changes are released (It'd be on experiment in Sandbox until we ok it) and I will see if we need to send in channel explicitly then.
Steven, I'm a bit confused, are you going to replace #314 |
This PR is going to merge into that one. We will still need a complete overview of the BCDC feature. |
Are you including the paypal-context-id in the header? |
Yes we're also setting it here in UpdateClientConfigAPI.kt |
@KunJeongPark that's a good catch though, the header name is incorrect. I'll update. |
@KunJeongPark actually should it be |
you are right, I misspoke |
private val coreConfig: CoreConfig, | ||
private val applicationContext: Context, | ||
private val graphQLClient: GraphQLClient, | ||
private val resourceLoader: ResourceLoader | ||
) { | ||
|
||
private object Defaults { | ||
const val INTEGRATION_ARTIFACT = "MOBILE_SDK" | ||
const val USER_EXPERIENCE_FLOW = "INCONTEXT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
USER_EXPERIENCE_FLOW is this value not NATIVE
? because flow is initiated by native app, even though it's completed in web
Q: HERMES
what does this mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe HERMES
is an internal name I'm not 100% sure of its origin. All of the values in Defaults
come from the JIRA ticket I was assigned. Nirvan may have further background on the actual meaning of each.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we took JS SDK's INCONTEXT value as web view and meant to distinguish native experiences like Apple Pay and Venmo as NATIVE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HERMES is name for old checkout experience, I think but here it is just default value most flows have for product_flow.
From what I understand, product_flow refers to a specific flow within your SDK and I don't think it applies to our SDK
so we were advised to just go with the default.
Summary of changes
UpdateClientConfigAPI
toCorePayments
so that it is accessible from all modules.Checklist
Added a changelog entryAuthors