Add Payment Token functionality to Save Payment flow #43
Conversation
gregjopa
force-pushed
the
feature/vault-enhancements-v2
branch
from
1767bc3 to
ea1c939
Compare
|
|
||
| <div class="buttons-container"> | ||
| <paypal-button id="paypal-button" type="pay" hidden></paypal-button> | ||
| <paypal-button id="paypal-button" hidden></paypal-button> |
There was a problem hiding this comment.
I don't think the "Pay with" button label makes sense for saving a payment so removing it.
| const paymentTokenResponse = jsonResponse as PaymentTokenResponse; | ||
|
|
||
| if (paymentTokenResponse.id) { | ||
| await savePaymentTokenToDatabase(paymentTokenResponse); |
There was a problem hiding this comment.
I think its important that we teach developers NOT to return this paymentToken value to the front-end. This is a long-lived value that can be used for making payments where the buyer is not present or doing a one-shot transaction.
We want to teach developers to save this payment token in their database instead.
There was a problem hiding this comment.
We don't have a lot of comments in this repo, but I think this type of callout would be good to have in a comment here.
There was a problem hiding this comment.
Great point. I just added a code comment about not returning this payment token id back to the browser.
There was a problem hiding this comment.
Yeah I really like this comment!
| const createPaymentTokenResponse = await createPaymentToken( | ||
| data.vaultSetupToken, | ||
| ); | ||
| console.log(createPaymentTokenResponse); |
There was a problem hiding this comment.
Nit: maybe we can preface this log with a string if we want to keep it like:
console.log("Create payment token response: ", createPaymentTokenResponse);
There was a problem hiding this comment.
Great suggestion. I added this label.
| const paymentTokenResponse = jsonResponse as PaymentTokenResponse; | ||
|
|
||
| if (paymentTokenResponse.id) { | ||
| await savePaymentTokenToDatabase(paymentTokenResponse); |
There was a problem hiding this comment.
Yeah I really like this comment!
| echo "::group:: $directory" | ||
| cd "$directory" && npm install && cd - | ||
| cd "$directory" | ||
| npm install || exit 1 |
There was a problem hiding this comment.
Is the exit 1 needed? My assumption would be that npm install returns with a non-zero status code if it fails 🤔
There was a problem hiding this comment.
Best I can tell, I think this explicit "exit 1" is needed. I noticed on this PR that originally CI was passing even though the prettier check was erroring. You can see it in this GitHub Actions run: https://github.com/paypal-examples/v6-web-sdk-sample-integration/actions/runs/15213524563/job/42793351279
I am open to other ways of solving this.
5 participants
This PR updates the SavePayment flow to add logic to the onApprove() callback to upgrade the vaultSetupToken to a paymentToken that can be used for future transactions. Now our Save Payment demo aligns with this v5 doc: https://developer.paypal.com/docs/checkout/save-payment-methods/purchase-later/js-sdk/paypal/