Use the validatorId to get access to the field instead of the context (#50035)

* Remove context

* Add method to get closest block

* Make methods async

* Add changelog

* Add await to getProductErrorMessageAndProps

* Fix getClientIdByField

* Fix tests

* Undo send id to DateTimePickerControl

* Add check before calling getClientIdByField
This commit is contained in:
Fernando Marichal 2024-08-02 08:31:18 -03:00 committed by GitHub
parent 66daa66aa3
commit 14751afeac
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
21 changed files with 92 additions and 87 deletions

View File

@ -0,0 +1,4 @@
Significance: minor
Type: dev
Use the validatorId to get access to the field instead of the context #50035

View File

@ -17,7 +17,6 @@ import { useProductEdits } from '../../../hooks/use-product-edits';
export function Edit( { export function Edit( {
attributes, attributes,
clientId,
context: { postType }, context: { postType },
}: ProductEditorBlockEditProps< NumberBlockAttributes > ) { }: ProductEditorBlockEditProps< NumberBlockAttributes > ) {
const blockProps = useWooBlockProps( attributes ); const blockProps = useWooBlockProps( attributes );
@ -57,7 +56,6 @@ export function Edit( {
), ),
min min
), ),
context: clientId,
}; };
} }
if ( if (
@ -74,13 +72,11 @@ export function Edit( {
), ),
min min
), ),
context: clientId,
}; };
} }
if ( required && ! value ) { if ( required && ! value ) {
return { return {
message: __( 'This field is required.', 'woocommerce' ), message: __( 'This field is required.', 'woocommerce' ),
context: clientId,
}; };
} }
}, },

View File

@ -21,7 +21,6 @@ import { TextBlockAttributes } from './types';
export function Edit( { export function Edit( {
attributes, attributes,
clientId,
context: { postType }, context: { postType },
}: ProductEditorBlockEditProps< TextBlockAttributes > ) { }: ProductEditorBlockEditProps< TextBlockAttributes > ) {
const blockProps = useWooBlockProps( attributes ); const blockProps = useWooBlockProps( attributes );
@ -135,7 +134,6 @@ export function Edit( {
if ( ! input.validity.valid ) { if ( ! input.validity.valid ) {
return { return {
message: customErrorMessage, message: customErrorMessage,
context: clientId,
}; };
} }
}, },

View File

@ -57,7 +57,6 @@ export function Edit( {
'This field must be a positive number.', 'This field must be a positive number.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
}, },

View File

@ -57,7 +57,6 @@ export function Edit( {
'Stock quantity must be a positive number.', 'Stock quantity must be a positive number.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
}, },

View File

@ -85,7 +85,6 @@ export function NameBlockEdit( {
if ( ! name || name === AUTO_DRAFT_NAME ) { if ( ! name || name === AUTO_DRAFT_NAME ) {
return { return {
message: __( 'Product name is required.', 'woocommerce' ), message: __( 'Product name is required.', 'woocommerce' ),
context: clientId,
}; };
} }
@ -95,7 +94,6 @@ export function NameBlockEdit( {
'Please enter a product name shorter than 120 characters.', 'Please enter a product name shorter than 120 characters.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
}, },

View File

@ -187,8 +187,12 @@ export function ProductDetailsSectionDescriptionBlockEdit( {
template: productTemplate.id, template: productTemplate.id,
} ); } );
} catch ( error ) { } catch ( error ) {
const { message, errorProps } = getProductErrorMessageAndProps( const { message, errorProps } =
errorHandler( error as WPError, productStatus ) as WPError, await getProductErrorMessageAndProps(
errorHandler(
error as WPError,
productStatus
) as WPError,
selectedTab selectedTab
); );
createErrorNotice( message, errorProps ); createErrorNotice( message, errorProps );
@ -305,7 +309,8 @@ export function ProductDetailsSectionDescriptionBlockEdit( {
// by the product editor. // by the product editor.
window.location.href = getNewPath( {}, `/product/${ productId }` ); window.location.href = getNewPath( {}, `/product/${ productId }` );
} catch ( error ) { } catch ( error ) {
const { message, errorProps } = getProductErrorMessageAndProps( const { message, errorProps } =
await getProductErrorMessageAndProps(
errorHandler( error as WPError, productStatus ) as WPError, errorHandler( error as WPError, productStatus ) as WPError,
selectedTab selectedTab
); );

View File

@ -72,7 +72,6 @@ export function Edit( {
'Regular price must be greater than or equals to zero.', 'Regular price must be greater than or equals to zero.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
if ( if (
@ -84,7 +83,6 @@ export function Edit( {
'Regular price must be greater than the sale price.', 'Regular price must be greater than the sale price.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
} else if ( isRequired ) { } else if ( isRequired ) {
@ -94,7 +92,6 @@ export function Edit( {
__( '%s is required.', 'woocommerce' ), __( '%s is required.', 'woocommerce' ),
label label
), ),
context: clientId,
}; };
} }
}, },

View File

@ -64,7 +64,6 @@ export function Edit( {
'Sale price must be greater than or equals to zero.', 'Sale price must be greater than or equals to zero.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
const listPrice = Number.parseFloat( regularPrice ); const listPrice = Number.parseFloat( regularPrice );
@ -77,7 +76,6 @@ export function Edit( {
'Sale price must be lower than the regular price.', 'Sale price must be lower than the regular price.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
} }

View File

@ -105,7 +105,6 @@ export function Edit( {
'Please enter a valid date.', 'Please enter a valid date.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
@ -115,7 +114,6 @@ export function Edit( {
'The start date of the sale must be before the end date.', 'The start date of the sale must be before the end date.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
} }
@ -137,7 +135,6 @@ export function Edit( {
'Please enter a valid date.', 'Please enter a valid date.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
@ -147,7 +144,6 @@ export function Edit( {
'The end date of the sale must be after the start date.', 'The end date of the sale must be after the start date.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
} }

View File

@ -103,7 +103,6 @@ export function Edit( {
'Width must be greater than zero.', 'Width must be greater than zero.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
}, },
@ -125,7 +124,6 @@ export function Edit( {
'Length must be greater than zero.', 'Length must be greater than zero.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
}, },
@ -147,7 +145,6 @@ export function Edit( {
'Height must be greater than zero.', 'Height must be greater than zero.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
}, },
@ -169,7 +166,6 @@ export function Edit( {
'Weight must be greater than zero.', 'Weight must be greater than zero.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
}, },

View File

@ -32,7 +32,6 @@ import { EmptyState } from '../../../components/empty-state';
export function Edit( { export function Edit( {
attributes, attributes,
clientId,
context: { isInSelectedTab }, context: { isInSelectedTab },
}: ProductEditorBlockEditProps< VariationOptionsBlockAttributes > ) { }: ProductEditorBlockEditProps< VariationOptionsBlockAttributes > ) {
const noticeDimissed = useRef( false ); const noticeDimissed = useRef( false );
@ -126,7 +125,6 @@ export function Edit( {
'Set variation prices before adding this product.', 'Set variation prices before adding this product.',
'woocommerce' 'woocommerce'
), ),
context: clientId,
}; };
} }
}, },

View File

@ -36,11 +36,9 @@ export function PreviewButton( {
navigateTo( { url } ); navigateTo( { url } );
} }
}, },
onSaveError( error ) { async onSaveError( error ) {
const { message, errorProps } = getProductErrorMessageAndProps( const { message, errorProps } =
error, await getProductErrorMessageAndProps( error, visibleTab );
visibleTab
);
createErrorNotice( message, errorProps ); createErrorNotice( message, errorProps );
}, },
} ); } );

View File

@ -51,11 +51,9 @@ export function PublishButtonMenu( {
showSuccessNotice( scheduledProduct ); showSuccessNotice( scheduledProduct );
} ) } )
.catch( ( error ) => { .catch( async ( error ) => {
const { message, errorProps } = getProductErrorMessageAndProps( const { message, errorProps } =
error, await getProductErrorMessageAndProps( error, visibleTab );
visibleTab
);
createErrorNotice( message, errorProps ); createErrorNotice( message, errorProps );
} ) } )
.finally( () => { .finally( () => {
@ -138,9 +136,9 @@ export function PublishButtonMenu( {
); );
navigateTo( { url } ); navigateTo( { url } );
} ) } )
.catch( ( error ) => { .catch( async ( error ) => {
const { message, errorProps } = const { message, errorProps } =
getProductErrorMessageAndProps( await getProductErrorMessageAndProps(
error, error,
visibleTab visibleTab
); );
@ -176,9 +174,9 @@ export function PublishButtonMenu( {
url: productListUrl, url: productListUrl,
} ); } );
} ) } )
.catch( ( error ) => { .catch( async ( error ) => {
const { message, errorProps } = const { message, errorProps } =
getProductErrorMessageAndProps( await getProductErrorMessageAndProps(
error, error,
visibleTab visibleTab
); );

View File

@ -62,11 +62,9 @@ export function PublishButton( {
navigateTo( { url } ); navigateTo( { url } );
} }
}, },
onPublishError( error ) { async onPublishError( error ) {
const { message, errorProps } = getProductErrorMessageAndProps( const { message, errorProps } =
error, await getProductErrorMessageAndProps( error, visibleTab );
visibleTab
);
createErrorNotice( message, errorProps ); createErrorNotice( message, errorProps );
}, },
} ); } );

View File

@ -48,11 +48,9 @@ export function SaveDraftButton( {
navigateTo( { url } ); navigateTo( { url } );
} }
}, },
onSaveError( error ) { async onSaveError( error ) {
const { message, errorProps } = getProductErrorMessageAndProps( const { message, errorProps } =
error, await getProductErrorMessageAndProps( error, visibleTab );
visibleTab
);
createErrorNotice( message, errorProps ); createErrorNotice( message, errorProps );
}, },
} ); } );

View File

@ -26,7 +26,7 @@ export type ValidationProviderProps = {
}; };
export type ValidationError = export type ValidationError =
| { message?: string; context?: string; validatorId?: string } | { message?: string; validatorId?: string }
| undefined; | undefined;
export type ValidationErrors = Record< string, ValidationError >; export type ValidationErrors = Record< string, ValidationError >;

View File

@ -62,5 +62,6 @@ export function useValidations< T = unknown >() {
} ); } );
}, },
focusByValidatorId, focusByValidatorId,
getFieldByValidatorId: context.getFieldByValidatorId,
}; };
} }

View File

@ -11,6 +11,7 @@ import { useBlocksHelper } from '../use-blocks-helper';
const mockNavigateTo = jest.fn(); const mockNavigateTo = jest.fn();
const mockFocusByValidatorId = jest.fn(); const mockFocusByValidatorId = jest.fn();
const mockGetFieldByValidatorId = jest.fn();
jest.mock( '@woocommerce/navigation', () => ( { jest.mock( '@woocommerce/navigation', () => ( {
getNewPath: jest.fn().mockReturnValue( '/new-path' ), getNewPath: jest.fn().mockReturnValue( '/new-path' ),
@ -26,6 +27,9 @@ jest.mock( '../../contexts/validation-context', () => ( {
focusByValidatorId: jest.fn( ( args ) => focusByValidatorId: jest.fn( ( args ) =>
mockFocusByValidatorId( args ) mockFocusByValidatorId( args )
), ),
getFieldByValidatorId: jest.fn( ( args ) =>
mockGetFieldByValidatorId( args )
),
} ), } ),
} ) ); } ) );
@ -39,6 +43,7 @@ jest.mock( '../use-blocks-helper', () => ( {
useBlocksHelper: jest.fn().mockReturnValue( { useBlocksHelper: jest.fn().mockReturnValue( {
getParentTabId: jest.fn( () => 'inventory' ), getParentTabId: jest.fn( () => 'inventory' ),
getParentTabIdByBlockName: jest.fn( () => 'inventory' ), getParentTabIdByBlockName: jest.fn( () => 'inventory' ),
getClientIdByField: jest.fn( ( arg ) => arg ),
} ), } ),
} ) ); } ) );
@ -47,7 +52,7 @@ describe( 'useErrorHandler', () => {
jest.clearAllMocks(); jest.clearAllMocks();
} ); } );
it( 'should return the correct error message and props when exists and the field is visible', () => { it( 'should return the correct error message and props when exists and the field is visible', async () => {
const error = { const error = {
code: 'product_invalid_sku', code: 'product_invalid_sku',
message: 'Invalid or duplicated SKU.', message: 'Invalid or duplicated SKU.',
@ -57,7 +62,7 @@ describe( 'useErrorHandler', () => {
const { result } = renderHook( () => useErrorHandler() ); const { result } = renderHook( () => useErrorHandler() );
const { getProductErrorMessageAndProps } = result.current; const { getProductErrorMessageAndProps } = result.current;
const { message, errorProps } = getProductErrorMessageAndProps( const { message, errorProps } = await getProductErrorMessageAndProps(
error, error,
visibleTab visibleTab
); );
@ -66,7 +71,7 @@ describe( 'useErrorHandler', () => {
expect( errorProps ).toEqual( {} ); expect( errorProps ).toEqual( {} );
} ); } );
it( 'should return the correct error message and props when exists and the field is not visible', () => { it( 'should return the correct error message and props when exists and the field is not visible', async () => {
const error = { const error = {
code: 'product_invalid_sku', code: 'product_invalid_sku',
} as WPError; } as WPError;
@ -75,7 +80,7 @@ describe( 'useErrorHandler', () => {
const { result } = renderHook( () => useErrorHandler() ); const { result } = renderHook( () => useErrorHandler() );
const { getProductErrorMessageAndProps } = result.current; const { getProductErrorMessageAndProps } = result.current;
const { message, errorProps } = getProductErrorMessageAndProps( const { message, errorProps } = await getProductErrorMessageAndProps(
error, error,
visibleTab visibleTab
); );
@ -84,7 +89,7 @@ describe( 'useErrorHandler', () => {
expect( errorProps.explicitDismiss ).toBeTruthy(); expect( errorProps.explicitDismiss ).toBeTruthy();
} ); } );
it( 'should call focusByValidatorId for form field errors when errorProps action is triggered', () => { it( 'should call focusByValidatorId for form field errors when errorProps action is triggered', async () => {
const error = { const error = {
code: 'product_form_field_error', code: 'product_form_field_error',
validatorId: 'test-validator', validatorId: 'test-validator',
@ -94,7 +99,7 @@ describe( 'useErrorHandler', () => {
const { result } = renderHook( () => useErrorHandler() ); const { result } = renderHook( () => useErrorHandler() );
const { getProductErrorMessageAndProps } = result.current; const { getProductErrorMessageAndProps } = result.current;
const { errorProps } = getProductErrorMessageAndProps( const { errorProps } = await getProductErrorMessageAndProps(
error, error,
visibleTab visibleTab
); );
@ -111,8 +116,11 @@ describe( 'useErrorHandler', () => {
expect( mockFocusByValidatorId ).toHaveBeenCalledWith( expect( mockFocusByValidatorId ).toHaveBeenCalledWith(
'test-validator' 'test-validator'
); );
expect( mockGetFieldByValidatorId ).toHaveBeenCalledWith(
'test-validator'
);
} ); } );
it( 'should call getParentTabIdByBlockName and focusByValidatorId for invalid sku errors when errorProps action is triggered', () => { it( 'should call getParentTabIdByBlockName and focusByValidatorId for invalid sku errors when errorProps action is triggered', async () => {
const error = { const error = {
code: 'product_invalid_sku', code: 'product_invalid_sku',
} as WPError; } as WPError;
@ -121,10 +129,8 @@ describe( 'useErrorHandler', () => {
const { result } = renderHook( () => useErrorHandler() ); const { result } = renderHook( () => useErrorHandler() );
const { getProductErrorMessageAndProps } = result.current; const { getProductErrorMessageAndProps } = result.current;
const { errorProps: fieldsErrorProps } = getProductErrorMessageAndProps( const { errorProps: fieldsErrorProps } =
error, await getProductErrorMessageAndProps( error, visibleTab );
visibleTab
);
expect( fieldsErrorProps ).toBeDefined(); expect( fieldsErrorProps ).toBeDefined();
expect( fieldsErrorProps.actions ).toBeDefined(); expect( fieldsErrorProps.actions ).toBeDefined();
@ -137,7 +143,7 @@ describe( 'useErrorHandler', () => {
expect( mockFocusByValidatorId ).toHaveBeenCalledWith( 'sku' ); expect( mockFocusByValidatorId ).toHaveBeenCalledWith( 'sku' );
} ); } );
it( 'should not call getErrorPropsWithActions for invalid sku errors when getParentTabIdByBlockName returns null', () => { it( 'should not call getErrorPropsWithActions for invalid sku errors when getParentTabIdByBlockName returns null', async () => {
const error = { const error = {
code: 'product_invalid_sku', code: 'product_invalid_sku',
} as WPError; } as WPError;
@ -146,12 +152,13 @@ describe( 'useErrorHandler', () => {
( useBlocksHelper as jest.Mock ).mockReturnValue( { ( useBlocksHelper as jest.Mock ).mockReturnValue( {
getParentTabId: jest.fn( () => null ), // Mock returns null getParentTabId: jest.fn( () => null ), // Mock returns null
getParentTabIdByBlockName: jest.fn( () => null ), // Mock returns null getParentTabIdByBlockName: jest.fn( () => null ), // Mock returns null
getClientIdByField: jest.fn( () => null ), // Mock returns null
} ); } );
const { result } = renderHook( () => useErrorHandler() ); const { result } = renderHook( () => useErrorHandler() );
const { getProductErrorMessageAndProps } = result.current; const { getProductErrorMessageAndProps } = result.current;
const { message, errorProps } = getProductErrorMessageAndProps( const { message, errorProps } = await getProductErrorMessageAndProps(
error, error,
visibleTab visibleTab
); );
@ -161,7 +168,7 @@ describe( 'useErrorHandler', () => {
expect( mockFocusByValidatorId ).not.toHaveBeenCalled(); expect( mockFocusByValidatorId ).not.toHaveBeenCalled();
expect( message ).toBe( 'Invalid or duplicated SKU.' ); expect( message ).toBe( 'Invalid or duplicated SKU.' );
} ); } );
it( 'should not call getErrorPropsWithActions for form field errors when getParentTabId returns null', () => { it( 'should not call getErrorPropsWithActions for form field errors when getParentTabId returns null', async () => {
const error = { const error = {
code: 'product_form_field_error', code: 'product_form_field_error',
validatorId: 'test-validator', validatorId: 'test-validator',
@ -172,12 +179,13 @@ describe( 'useErrorHandler', () => {
( useBlocksHelper as jest.Mock ).mockReturnValue( { ( useBlocksHelper as jest.Mock ).mockReturnValue( {
getParentTabId: jest.fn( () => null ), // Mock returns null getParentTabId: jest.fn( () => null ), // Mock returns null
getParentTabIdByBlockName: jest.fn( () => null ), // Mock returns null getParentTabIdByBlockName: jest.fn( () => null ), // Mock returns null
getClientIdByField: jest.fn( () => null ), // Mock returns null
} ); } );
const { result } = renderHook( () => useErrorHandler() ); const { result } = renderHook( () => useErrorHandler() );
const { getProductErrorMessageAndProps } = result.current; const { getProductErrorMessageAndProps } = result.current;
const { message, errorProps } = getProductErrorMessageAndProps( const { message, errorProps } = await getProductErrorMessageAndProps(
error, error,
visibleTab visibleTab
); );
@ -185,6 +193,9 @@ describe( 'useErrorHandler', () => {
expect( errorProps ).toBeDefined(); expect( errorProps ).toBeDefined();
expect( errorProps.actions ).not.toBeDefined(); expect( errorProps.actions ).not.toBeDefined();
expect( mockFocusByValidatorId ).not.toHaveBeenCalled(); expect( mockFocusByValidatorId ).not.toHaveBeenCalled();
expect( mockGetFieldByValidatorId ).toHaveBeenCalledWith(
'test-validator'
);
expect( message ).toBe( 'Test error message' ); expect( message ).toBe( 'Test error message' );
} ); } );
} ); } );

View File

@ -22,7 +22,14 @@ export function useBlocksHelper() {
return attributes?.id; return attributes?.id;
} }
function getParentTabId( clientId?: string ) { function getClientIdByField( field: HTMLElement ) {
const parentBlockElement = field.closest(
'[data-block]'
) as HTMLElement;
return parentBlockElement?.dataset.block;
}
function getParentTabId( clientId?: string | null ) {
if ( clientId ) { if ( clientId ) {
return getClosestParentTabId( clientId ); return getClosestParentTabId( clientId );
} }
@ -41,6 +48,7 @@ export function useBlocksHelper() {
} }
return { return {
getClientIdByField,
getParentTabId, getParentTabId,
getParentTabIdByBlockName, getParentTabIdByBlockName,
}; };

View File

@ -24,7 +24,6 @@ export type WPError = {
code: WPErrorCode; code: WPErrorCode;
message: string; message: string;
validatorId?: string; validatorId?: string;
context?: string;
}; };
type ErrorProps = { type ErrorProps = {
@ -41,10 +40,10 @@ type UseErrorHandlerTypes = {
getProductErrorMessageAndProps: ( getProductErrorMessageAndProps: (
error: WPError, error: WPError,
visibleTab: string | null visibleTab: string | null
) => { ) => Promise< {
message: string; message: string;
errorProps: ErrorProps; errorProps: ErrorProps;
}; } >;
}; };
function getUrl( tab: string ): string { function getUrl( tab: string ): string {
@ -74,22 +73,32 @@ function getErrorPropsWithActions(
} }
export const useErrorHandler = (): UseErrorHandlerTypes => { export const useErrorHandler = (): UseErrorHandlerTypes => {
const { focusByValidatorId } = useValidations(); const { focusByValidatorId, getFieldByValidatorId } = useValidations();
const { getParentTabId, getParentTabIdByBlockName } = useBlocksHelper(); const { getClientIdByField, getParentTabId, getParentTabIdByBlockName } =
useBlocksHelper();
async function getClientIdByValidatorId( validatorId: string ) {
if ( ! validatorId ) {
return null;
}
const field = await getFieldByValidatorId( validatorId );
if ( ! field ) {
return null;
}
return getClientIdByField( field );
}
const getProductErrorMessageAndProps = useCallback( const getProductErrorMessageAndProps = useCallback(
( error: WPError, visibleTab: string | null ) => { async ( error: WPError, visibleTab: string | null ) => {
const response = { const response = {
message: '', message: '',
errorProps: {} as ErrorProps, errorProps: {} as ErrorProps,
}; };
const { const { code, message: errorMessage, validatorId = '' } = error;
code,
context = '', const clientId = await getClientIdByValidatorId( validatorId );
message: errorMessage, const errorContext = getParentTabId( clientId );
validatorId = '',
} = error;
const errorContext = getParentTabId( context );
switch ( code ) { switch ( code ) {
case 'variable_product_no_variation_prices': case 'variable_product_no_variation_prices':
response.message = errorMessage; response.message = errorMessage;