-
Notifications
You must be signed in to change notification settings - Fork 506
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 Save Payment Method #99
Conversation
<body> | ||
<div id="paypal-button-container"></div> | ||
<p id="result-message"></p> | ||
<script src="app.js"></script> |
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.
Would you be open to converting this file to .ejs? Wondering if that would make things easier with setting the data-user-id-token
.
For the standard integration we have kept it as plain html to keep it as simple as possible. But for other examples we have been leveraging ejs for server-side templating. That way you can embed values that the data-user-id from the server-side and still the load the JS SDK script in it too. Here's an example of how v2 card fields does this to set the clientID: https://github.com/paypal-examples/docs-examples/blob/main/advanced-integration/v2/server/views/checkout.ejs#L20
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.
ah great idea!! yes - I really like this approach!
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.
Converted in d581019
orderData?.purchase_units?.[0]?.payments?.authorizations?.[0]; | ||
resultMessage( | ||
`Transaction ${transaction.status}: ${transaction.id}<br><br>See console for all available details.<br> | ||
<a href='/?customerID=${orderData.payment_source.paypal.attributes.vault.customer.id}'>See the return buyer experience</a> |
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 thought this was a clever way to show the return buyer experience without a session or cookie dependency, but LMK if you have other ideas! I'm also curious if this implementation is not 100% correct because after the one click payment completes, a new customer id is generated (maybe it should be sent through during the order creation? <- need to investigate further)
<div id="paypal-button-container"></div> | ||
<p id="result-message"></p> | ||
<script | ||
src="https://www.paypal.com/sdk/js?client-id=<%= clientId %>" |
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'm also seeing this warning in the console:
though the docs do not include vault=true
in the code sample
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 added it in d4ea5c3 and the warning went away 🥳
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.
Amazing work @jshawl! 💯
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.
Good
This PR implements the Save Payment Method (during purchase) integration as described by the documentation.
I followed the standard-integration example as closely as possible with a few notable differences:
generateAccessToken
no longer calls the REST API directlyauthenticate
is used for obtaining an id token and an access token in the same request