-
Notifications
You must be signed in to change notification settings - Fork 43
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
fix (ras-acc): prevent NYP, coupon submission from clearing credit card fields #1881
Conversation
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.
Nice work @laurelfulford! Looks good for the most part. Left a small non-blocking nit.
I also ran into a very edge case issue where when a product has both coupons AND nyp enabled, submitting a new NYP value will override the coupon entirely. However, this seems to be happening on epic/ras-acc
as well so will approve and create a new task for this.
src/modal-checkout/index.js
Outdated
/** | ||
* Get updated cart total to update the "Place Order" button. | ||
*/ | ||
function returnUpdatedCartTotal() { |
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.
non-blocking nit: might be clearer to name this something like getUpdatedCartTotal
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 call! This has been updated in 013161e.
Eeeeep good catch! 😱 I ran so many checkouts testing this and still missed it -- so glad you caught it! |
All Submissions:
Changes proposed in this Pull Request:
This PR moves setting and updating the 'Complete Transaction' button price from PHP to JS. This is to prevent an issue where submitting a coupon or NYP after filling in the credit card fields would cause the credit card fields to be cleared.
See: 1207817176293825-as-1208255803261156.
How to test the changes in this Pull Request:
npm run build
.Other information: