This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Refactor existing error notices from the API #7728
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Cart errors need to be watched for and created as notices...Cart errors need to be watched for and created as notices elsewhere. 🚀 This comment was generated by the automations bot based on a
|
mikejolley
commented
Nov 22, 2022
| ); | ||
| __internalProcessCheckoutResponse( response ); | ||
| } ); | ||
| processErrorResponse( response ); |
There was a problem hiding this comment.
processErrorResponse is the new shared util which translates error codes to notices.
|
The release ZIP for this PR is accessible via: |
mikejolley
commented
Nov 22, 2022
| /** | ||
| * Creates a notice only if the Store Notice Container is visible. | ||
| */ | ||
| export const createNoticeIfVisible = ( |
There was a problem hiding this comment.
I created this so we could dispatch notices only if shipping and/or billing is visible. This is because the API returns errors for both the shiping and billing address params if you use the same address for both. Only one block would be visible in that case. It prevents duplication.
mikejolley
commented
Nov 22, 2022
| @@ -24,7 +24,7 @@ const FrontendBlock = ( { | |||
| const { hasDarkControls } = useCartBlockContext(); | |||
| const { createErrorNotice } = useDispatch( 'core/notices' ); | |||
|
|
|||
| // Ensures any cart errors listed in the API response get shown. | |||
| // @todo Cart errors need to be watched for and created as notices elsewhere. | |||
There was a problem hiding this comment.
This requires a follow up so we can dispatch cart errors on both cart and checkout using the same logic.
mikejolley
commented
Nov 22, 2022
| * - Supports 1 level of nesting. | ||
| * - Decodes HTML entities in error messages. | ||
| */ | ||
| const getErrorDetails = ( response: ApiErrorResponse ): ApiParamError[] => { |
There was a problem hiding this comment.
This formats and flattens errors from our API schema into something more usable.
mikejolley
commented
Nov 22, 2022
|
|
||
| errorDetails.forEach( ( { code, message, id, param } ) => { | ||
| switch ( code ) { | ||
| case 'invalid_phone': |
There was a problem hiding this comment.
Future parameters we want to handle should be added here.
There was a problem hiding this comment.
Could we eventually allow extensions to add their own handlers here?
There was a problem hiding this comment.
Yes, I think so. We'll work this out in a follow up.
mikejolley
commented
Nov 22, 2022
| @@ -35,13 +35,30 @@ const StoreNotices = ( { | |||
|
|
|||
| useEffect( () => { | |||
| // Scroll to container when an error is added here. | |||
| const containerRef = ref.current; | |||
There was a problem hiding this comment.
I added this to avoid scrolling to errors as you type in a field.
|
Size Change: -2.41 kB (0%) Total Size: 966 kB
ℹ️ View Unchanged
|
TypeScript Errors ReportFiles with errors: 434
assets/js/base/components/cart-checkout/address-form/address-form.tsx
assets/js/base/context/providers/cart-checkout/checkout-events/index.tsx assets/js/base/utils/create-notice.ts assets/js/data/cart/resolvers.ts assets/js/data/cart/test/resolvers.js assets/js/data/checkout/types.ts assets/js/data/checkout/utils.ts assets/js/data/index.ts assets/js/data/payment/types.ts assets/js/data/shared-controls.ts assets/js/data/validation/actions.ts assets/js/data/validation/reducers.ts assets/js/data/validation/test/reducers.ts assets/js/data/validation/test/selectors.ts packages/checkout/components/store-notices-container/snackbar-notices.tsx packages/checkout/components/store-notices-container/store-notices.tsx packages/checkout/components/store-notices-container/test/index.tsx |
Script Dependencies ReportThe
This comment was automatically generated by the |
opr
approved these changes
Nov 22, 2022
There was a problem hiding this comment.
Thanks Mike, this is testing well. Just to note I see the phone error appearing twice in the contact information context, this should only appear in the address field where the phone is invalid. Note: this only happens following pushChanges, so entering only an invalid phone number won't trigger this, you have to have an invalid phone number and then update the address.
This may be better off being addressed in the other PR.
I have made a few suggestions about TS and some general improvements, none of my comments are blocking though so approving.
assets/js/base/context/providers/cart-checkout/checkout-processor.ts
Outdated
Show resolved
Hide resolved
assets/js/base/context/providers/cart-checkout/checkout-processor.ts
Outdated
Show resolved
Hide resolved
|
|
||
| errorDetails.forEach( ( { code, message, id, param } ) => { | ||
| switch ( code ) { | ||
| case 'invalid_phone': |
There was a problem hiding this comment.
Could we eventually allow extensions to add their own handlers here?
Co-authored-by: Thomas Roberts <[email protected]>
Remove this when supported in Gutenberg.Remove this when supported in Gutenberg. github.com/WordPress/gutenberg/pull/44059
woocommerce-blocks/assets/js/base/utils/create-notice.ts Lines 88 to 100 in 25282fb 🚀 This comment was generated by the automations bot based on a
|
|
@opr merging this to the other branch now. Thanks for the feedback; that's all resolved. |
1 task
mikejolley
added a commit
that referenced
this pull request
Dec 5, 2022
* Update API type defs * Move create notice utils * Replace useCheckoutNotices with new contexts * processCheckoutResponseHeaders should check headers are defined * Scroll to error notices only if we're not editing a field * Error handling utils * processErrorResponse when pushing changes * processErrorResponse when processing checkout * remove formatStoreApiErrorMessage * Add todo for cart errors * Remove unused deps * unused imports * Fix linting warnings * Unused dep * Update assets/js/types/type-defs/api-response.ts Co-authored-by: Thomas Roberts <[email protected]> * Add todo * Use generic * remove const * Update array types * Phone should be in address blocks Co-authored-by: Thomas Roberts <[email protected]>
mikejolley
added a commit
that referenced
this pull request
Dec 14, 2022
* Update API type defs * Move create notice utils * Replace useCheckoutNotices with new contexts * processCheckoutResponseHeaders should check headers are defined * Scroll to error notices only if we're not editing a field * Error handling utils * processErrorResponse when pushing changes * processErrorResponse when processing checkout * remove formatStoreApiErrorMessage * Add todo for cart errors * Remove unused deps * unused imports * Fix linting warnings * Unused dep * Update assets/js/types/type-defs/api-response.ts Co-authored-by: Thomas Roberts <[email protected]> * Add todo * Use generic * remove const * Update array types * Phone should be in address blocks Co-authored-by: Thomas Roberts <[email protected]>
opr
added a commit
that referenced
this pull request
Dec 16, 2022
* Update API type defs * Move create notice utils * Replace useCheckoutNotices with new contexts * processCheckoutResponseHeaders should check headers are defined * Scroll to error notices only if we're not editing a field * Error handling utils * processErrorResponse when pushing changes * processErrorResponse when processing checkout * remove formatStoreApiErrorMessage * Add todo for cart errors * Remove unused deps * unused imports * Fix linting warnings * Unused dep * Update assets/js/types/type-defs/api-response.ts Co-authored-by: Thomas Roberts <[email protected]> * Add todo * Use generic * remove const * Update array types * Phone should be in address blocks Co-authored-by: Thomas Roberts <[email protected]>
mikejolley
added a commit
that referenced
this pull request
Dec 16, 2022
* Update API type defs * Move create notice utils * Replace useCheckoutNotices with new contexts * processCheckoutResponseHeaders should check headers are defined * Scroll to error notices only if we're not editing a field * Error handling utils * processErrorResponse when pushing changes * processErrorResponse when processing checkout * remove formatStoreApiErrorMessage * Add todo for cart errors * Remove unused deps * unused imports * Fix linting warnings * Unused dep * Update assets/js/types/type-defs/api-response.ts Co-authored-by: Thomas Roberts <[email protected]> * Add todo * Use generic * remove const * Update array types * Phone should be in address blocks Co-authored-by: Thomas Roberts <[email protected]>
mikejolley
added a commit
that referenced
this pull request
Dec 19, 2022
* Update API type defs * Move create notice utils * Replace useCheckoutNotices with new contexts * processCheckoutResponseHeaders should check headers are defined * Scroll to error notices only if we're not editing a field * Error handling utils * processErrorResponse when pushing changes * processErrorResponse when processing checkout * remove formatStoreApiErrorMessage * Add todo for cart errors * Remove unused deps * unused imports * Fix linting warnings * Unused dep * Update assets/js/types/type-defs/api-response.ts Co-authored-by: Thomas Roberts <[email protected]> * Add todo * Use generic * remove const * Update array types * Phone should be in address blocks Co-authored-by: Thomas Roberts <[email protected]>
mikejolley
added a commit
that referenced
this pull request
Dec 19, 2022
* Refactor Store Notices Move snackbar hiding filter before notice creation Implements showApplyCouponNotice Refactor context providers Use STORE_NOTICE_CONTEXTS use refs to track notice containers Refactor ref usage Use existing noticeContexts * Move new notice code to checkout package * Combine store and snackbars * Update noticeContexts imports * Remove context provider * Update data store * Fix 502 * Add new error contexts * Force types * Unnecessary reorder of imports * Fix global handling * Document forceType * Optional props are undefined * Remove function name * Missing condition * Remove context prop * Define ACTION_TYPES * Remove controls * Update assets/js/base/context/event-emit/utils.ts Co-authored-by: Thomas Roberts <[email protected]> * CONTACT_INFORMATION * Remove ref from registerContainer * Abstract container locating methods * pass context correctly when displaying notices * Remove debugging buttons * Update filter usage - remove useMemo so filter can work inline * Refactor existing error notices from the API (#7728) * Update API type defs * Move create notice utils * Replace useCheckoutNotices with new contexts * processCheckoutResponseHeaders should check headers are defined * Scroll to error notices only if we're not editing a field * Error handling utils * processErrorResponse when pushing changes * processErrorResponse when processing checkout * remove formatStoreApiErrorMessage * Add todo for cart errors * Remove unused deps * unused imports * Fix linting warnings * Unused dep * Update assets/js/types/type-defs/api-response.ts Co-authored-by: Thomas Roberts <[email protected]> * Add todo * Use generic * remove const * Update array types * Phone should be in address blocks Co-authored-by: Thomas Roberts <[email protected]> * Update store name to wc/store/store-notices * Fix assertResponseIsValid * Funnel woocommerce_rest_invalid_email_address to the correct place * woocommerce_rest_missing_email_address * Move comments around * Move data back into const * Spacing * Remove spacing * Remove forced snack bar and styling * Move notices within wrapper * Remove type * hasStoreNoticesContainer rename * Group by status/context * Remove global context * Remove white space * remove changes to simplify diff * white space * Move comment to typescript * List style * showApplyCouponNotice docs * See if scrollIntoView exists * fix notice tests Co-authored-by: Thomas Roberts <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a followup to #7711 which consolodates error handling logic for the checkout. After this change, the Checkout Processor and the push-changes code both share error handling utilities.
API Errors are converted to notices using the param name and error code, and mapped to the correct Store Notice Context on checkout (contexts were added in #7711).
This also implements typescript for API related things, and the Checkout Processor.
One change that was needed for #7711 was the group all notice contexts to determine if errors are present (see assets/js/base/context/providers/cart-checkout/checkout-events/index.tsx). I replaced
useCheckoutNoticeswith this new logic.Testing
User Facing Testing
123as your postcode.