Skip to content
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

Execute server-side WC validation when clicking button #942

Merged
merged 4 commits into from
Nov 2, 2022

Conversation

AlexP11223
Copy link
Contributor

Added execution of WC validate_checkout during order creation.

This should be better than the basic JS validation #513, but it is still done in a bit non-standard/hacky way, may break depending on other plugins using the hooks, WC version (the method is not the public API, though hopefully protected is close enough to that and unlikely to have breaking changes) etc. There is a filter for disabling it. Also we catch/log the (non-validation) errors during the validation execution, if this happens it will be skipped and continue like before #513. The JS validation also remains for now, but disabled by default.

Also fixed errorHandlers in JS, there were multiple instances of these objects creating their own error lists on the page, which was resulting in outdated messages still being present in some cases.

And it seems like after bb86f1b the order creation errors were replaced by the generic message (though probably not just this commit because after simply reverting it the messages were still broken), so fixed this too.

Copy link
Contributor

@Dinamiko Dinamiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, only a few imports that seems not needed.

onError: () => {
this.errorHandler.genericError();
onError: (err) => {
console.error(err);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this console error on purpose here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we don't show it on the page, so should show it at least in console if it comes from other sources.

@Dinamiko Dinamiko merged commit 77e18ea into trunk Nov 2, 2022
@Dinamiko Dinamiko deleted the pcp-817-validation branch November 2, 2022 15:31
@Dinamiko Dinamiko added the enhancement New feature or request label Nov 2, 2022
@Dinamiko Dinamiko added this to the 2.0.0 milestone Nov 2, 2022
@Dinamiko Dinamiko mentioned this pull request Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants