-
Notifications
You must be signed in to change notification settings - Fork 221
Store API: Refactored validation handling and introduced woocommerce_store_api_cart_errors
hook
#5904
Conversation
Size Change: 0 B Total Size: 863 kB ℹ️ View Unchanged
|
578fcae
to
8115fa4
Compare
This pretty much supersedes #5874 (it is a bit broader in scope as it is meant to add some extra clarity around ongoing discussions on extensibility, consistency, and code re-use). |
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.
Added some initial feedback. I think the main thing we need to do is clean up the approach to WP_Error/Exception because there is a bunch of to-and-fro between the two types.
I tried to merge with Update: This branch is in line with |
682e19b
to
6381251
Compare
2a4ac9d
to
492ca88
Compare
This action can be used by third party developers to add extra 'errors' in cart route responses. This is meant to function as a replacement of the legacy 'woocommerce_check_cart_items' hook.
The new action can be used to modify the finalized draft order object. Additionally, developers can use it to throw a RouteException in order to prevent access to the checkout block. The PR also shuffles some existing code that checks the order instance and sets the draft order ID before the 'woocommerce_blocks_checkout_update_order_meta' gets fired.
…er_meta' docblock on the effect of exceptions thrown from callbacks
c2095ce
to
83bbf2d
Compare
__experimental_woocommerce_store_api_cart_errors
hook
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.
Changes look good now, up to date with trunk, smoke tested the new (and old) hooks, 🚢
__experimental_woocommerce_store_api_cart_errors
hookwoocommerce_store_api_cart_errors
hook
This refactor is based on how the system handles the
InvalidStockLevelsInCartException
and aims to create a more DRY error validation architecture.Currently, when there are multiple _(stock-related)_errors raised from the cart contents, the system will group them in a single
WP_Error
instance with a "409 Conflict" code. During the REST response, these errors are deconstructed into multiple error keys. One error/key.This PR replaces the
InvalidStockLevelsInCartException
with a more genericInvalidCartException
that should handle every validation-related error from the core, stock, or 3PD.In addition, the
InvalidCartException
is designed to handle multiple errors in its ownWP_Error
.When a
validate_xxxx
method throws anInvalidCartException
containing multiple errors:WP_Error
objects; the final REST response will be the same.InvalidStockLevelsInCartException
is that the system will have the original error codes instead of "409 Conflict".In a nutshell:
InvalidCartException
class to be used in all errors related to the cart validity.validate_xxxx
methods.__experimental_woocommerce_store_api_validate_cart
and thewoocommerce_check_cart_items
are connected. It seems like a good path for deprecating the latter in the API. See Next Steps # 2__experimental_woocommerce_store_api_validate_cart
hook is called in the cart and checkout context to create consistency.What's changed
RouteException
is now handled in both the cart and checkout flows, as previously, were only held in the cart flow.Example 3PD cart validation usage:
The snippet above adds multiple errors, and the result is handled accordingly: https://d.pr/i/vPircs (checkout) and https://d.pr/i/alWAW9 (cart).
A present use-case:
Conditional Shipping and Payments extension is a condition-based restriction system that could potentially produce complex business restrictions. Throwing a
WP_Error
object containing multiple errors is crucial for correctly handling all situations.Next steps:
context
argument invalidate_cart()
and control whether the__experimental_woocommerce_store_api_validate_cart
or thewoocommerce_check_cart_items
should fire. Perhaps, for performance reasons, we need to only run thewoocommerce_check_cart_items
legacy hook in the checkout context to avoid handling WC notices to exceptions all the time.context
argument by moving thewoocommerce_check_cart_items
directly inside theCheckout::get_route_post_response
; it will be deprecated, so it shouldn't be a clean-code issue.Accessibility
prefers-reduced-motion
Other Checks
Testing
Automated Tests
User Facing Testing
None.
Changelog