-
-
Notifications
You must be signed in to change notification settings - Fork 224
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
Revert "Revert "Webpack migration for pages that use Stripe"" #36123
Revert "Revert "Webpack migration for pages that use Stripe"" #36123
Conversation
This is ready for review now and is also going to have another pass of QA. |
@@ -204,6 +211,7 @@ hqDefine('accounting/js/payment_method_handler', [ | |||
if (self.savedCards().length > 0) { | |||
self.selectedCardType('saved'); | |||
} | |||
self.showOrHideStripeUI(self.mustCreateNewCard()); |
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.
If we already subscribe to this observable, is this line still necessary?
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.
nvm! I saw you add a comment in a later commit
Reverts #36121, restoring #36073
I didn't realize there's a different UI for users who don't have any cards saved. This PR has a few additional commits to add support for that UI. The js is a bit finicky, as there are several knockout vars controlling the existence of the credit card inputs, and Stripe has to mount the UI whenever that area is made visible.
Safety Assurance
Safety story
Moderate risk due to affecting payments. Risk is limited to the updated pages.
Automated test coverage
Accounting has tests, but I don't think it has javascript coverage.
QA Plan
Using the same ticket as for the previous PR: https://dimagi.atlassian.net/browse/QA-7597
Rollback instructions
Labels & Review