-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Fix 'Invalid payment method' error upon double click 'Delete' #30862 #30884
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 'Invalid payment method' error upon double click 'Delete' #30862 #30884
Conversation
ef4d229 to
5c53516
Compare
kalessil
left a comment
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.
The approach looks good ! Leaving a simplification proposal.
…erce#30862 An 'Invalid payment method' error shows when a user double clicks the 'Delete' button in the 'Add Payment method screen'. Every time the user clicks the button, there is a GET request to delete the payment method. First request is handled correctly, but subsequent requests throw an error, because the payment method cannot be found since it was already deleted. This adds an event handler to disable the button on the first click.
5c53516 to
dea2aa9
Compare
|
Pulling @nerrad and @ObliviousHarmony for a review, on my end, the changes are looking good ! |
|
Ping @ObliviousHarmony |
vedanshujain
left a comment
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.
LGTM! Looks like CI is failing, but it's probably unrelated. I have restarted it, and will merge when it passes.
…double-click-delete
|
Hi @vedanshujain, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:
|
All Submissions:
Changes proposed in this Pull Request:
An 'Invalid payment method' error shows when a user double clicks the'Delete' button in the 'Add Payment method screen'. Every time the user clicks the button, there is a GET request to delete the payment method. First request is handled correctly, but subsequent requests throw an error, because the payment method cannot be found since it was already deleted.
This is a video of the bug, taken from the issue.
To avoid firing the multiple requests, this PR adds an event handler to the buttons in the Payment method table. The event is attached to the table
tbodyand delegated to.payment-method-actions .button. When the event handler is executed, it will check if the button has.disabledclass. If the button is not disabled, it will add this class and the GET request will fire. If the button is disabled, it callspreventDefaultin the event, to avoid firing the request.I'd like to merge this PR to avoid confusion for the user and I also think there is a little performance gain since we are sparing some requests that will end up throwing an error.
Closes #30862
How to test the changes in this Pull Request:
Prerequisites
You'll need to upload and activate the WooCommerce Payments plugin, to be able to add and remove payment methods in the user's account
This the behavior with the fix applied:
Other information:
Changelog entry
FOR PR REVIEWER ONLY: