Invert the useValidation hook behavior to return an error instead of a boolean value #37695

mdperez86 · 2023-04-13T00:22:53Z

Submission Review Guidelines:

I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have reviewed my code for security best practices.
Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

Closes #37663
Closes #37706
Closes #37707

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

Got to /wp-admin/tools.php?page=woocommerce-admin-test-helper
Under Features tab make sure to enable product-block-editor
Then visit /wp-admin/admin.php?page=wc-admin&path=/add-product
Validations should remains as usual

github-actions · 2023-04-13T00:23:09Z

Hi @joshuatf,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

github-actions · 2023-04-13T00:48:23Z

Test Results Summary

Commit SHA: c619b21

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	1m 10s
E2E Tests	187	0	0	10	0	197	13m 34s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

codecov · 2023-04-13T00:52:13Z

Codecov Report

Merging #37695 (4266c93) into trunk (c226fa1) will decrease coverage by 0.0%.
The diff coverage is n/a.

❗ Current head 4266c93 differs from pull request most recent head c619b21. Consider uploading reports for the commit c619b21 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             trunk   #37695     +/-   ##
==========================================
- Coverage     51.6%    51.6%   -0.0%     
  Complexity   17260    17260             
==========================================
  Files          429      429             
  Lines        79928    79928             
==========================================
- Hits         41213    41211      -2     
- Misses       38715    38717      +2

Impacted Files	Coverage Δ
...ludes/tracks/events/class-wc-products-tracking.php	`0.0% <ø> (ø)`

... and 1 file with indirect coverage changes

joshuatf

One optional change, but code tests well and looks great so pre-approving! 🎉

joshuatf · 2023-04-14T01:50:50Z

packages/js/product-editor/src/hooks/use-validation/README.md

+const validateTitle = useCallback( (): ValidationError => {
 	if ( product.title.length < 2 ) {
-		return false;
+		return 'Title must be greater than 1';


Probably worth including localization in here just in case people copy/paste this.

Suggested change

return 'Title must be greater than 1';

return __( 'Title must be greater than 1', 'text-domain' );

done -> 0378401

…a boolean value

joshuatf

A couple very minor semantic issues. Sorry I didn't catch those in my last review.

joshuatf · 2023-04-17T17:23:49Z

packages/js/product-editor/src/blocks/shipping-dimensions/edit.tsx

+		function dimensionsWidthValidator() {
 			if ( dimensions?.width && +dimensions.width <= 0 ) {
-				return false;
+				return __( 'Width must be higher than zero.', 'woocommerce' );


Suggested change

return __( 'Width must be higher than zero.', 'woocommerce' );

return __( 'Width must be greater than zero.', 'woocommerce' );

done -> c619b21

joshuatf · 2023-04-17T17:23:56Z

packages/js/product-editor/src/blocks/shipping-dimensions/edit.tsx

+		function dimensionsLengthValidator() {
 			if ( dimensions?.length && +dimensions.length <= 0 ) {
-				return false;
+				return __( 'Length must be higher than zero.', 'woocommerce' );


Suggested change

return __( 'Length must be higher than zero.', 'woocommerce' );

return __( 'Length must be greater than zero.', 'woocommerce' );

done -> c619b21

joshuatf · 2023-04-17T17:24:04Z

packages/js/product-editor/src/blocks/shipping-dimensions/edit.tsx

+		function dimensionsHeightValidator() {
 			if ( dimensions?.height && +dimensions.height <= 0 ) {
-				return false;
+				return __( 'Height must be higher than zero.', 'woocommerce' );


Suggested change

return __( 'Height must be higher than zero.', 'woocommerce' );

return __( 'Height must be greater than zero.', 'woocommerce' );

done -> c619b21

joshuatf · 2023-04-17T17:24:12Z

packages/js/product-editor/src/blocks/shipping-dimensions/edit.tsx

+		function weightValidator() {
 			if ( weight && +weight <= 0 ) {
-				return false;
+				return __( 'Weight must be higher than zero.', 'woocommerce' );


Suggested change

return __( 'Weight must be higher than zero.', 'woocommerce' );

return __( 'Weight must be greater than zero.', 'woocommerce' );

done -> c619b21

joshuatf · 2023-04-17T17:25:54Z

packages/js/product-editor/src/hooks/use-validation/README.md

+const validateTitle = useCallback( (): ValidationError => {
 	if ( product.title.length < 2 ) {
-		return false;
+		return __( 'Title must be greater than 1', 'text-domain' );


Suggested change

return __( 'Title must be greater than 1', 'text-domain' );

return __( 'Title must be more than 1 character', 'text-domain' );

done -> c619b21

joshuatf

Looks good! Thanks for those last changes 🎉

mdperez86 requested a review from a team April 13, 2023 00:22

mdperez86 self-assigned this Apr 13, 2023

mdperez86 force-pushed the add/37663 branch from d3554a4 to 4266c93 Compare April 14, 2023 15:53

joshuatf previously approved these changes Apr 14, 2023

View reviewed changes

mdperez86 added 5 commits April 17, 2023 10:48

Invert the useValidation hook behavior to return an error instead of …

448ea53

…a boolean value

Fix validation logic in schedule-sale and track-inventory blocks

31b020d

Add changelog file

a886893

Add validations to product name block

4917866

Fix shipping dimensions validations

ebb98d0

mdperez86 dismissed joshuatf’s stale review via e1b7daa April 17, 2023 14:58

mdperez86 force-pushed the add/37663 branch from 4266c93 to e1b7daa Compare April 17, 2023 14:58

Fix shipping fee validations

8a81f55

mdperez86 force-pushed the add/37663 branch from e1b7daa to 8a81f55 Compare April 17, 2023 15:00

Add i18n error message example to readme.md

0378401

mdperez86 requested a review from joshuatf April 17, 2023 15:06

joshuatf reviewed Apr 17, 2023

View reviewed changes

Cover semantic issues

c619b21

mdperez86 requested a review from joshuatf April 17, 2023 18:04

joshuatf approved these changes Apr 17, 2023

View reviewed changes

mdperez86 merged commit 41545ec into trunk Apr 18, 2023

mdperez86 deleted the add/37663 branch April 18, 2023 03:54

github-actions bot added this to the 7.8.0 milestone Apr 18, 2023

	return 'Title must be greater than 1';
	return __( 'Title must be greater than 1', 'text-domain' );

	return __( 'Width must be higher than zero.', 'woocommerce' );
	return __( 'Width must be greater than zero.', 'woocommerce' );

	return __( 'Length must be higher than zero.', 'woocommerce' );
	return __( 'Length must be greater than zero.', 'woocommerce' );

	return __( 'Height must be higher than zero.', 'woocommerce' );
	return __( 'Height must be greater than zero.', 'woocommerce' );

	return __( 'Weight must be higher than zero.', 'woocommerce' );
	return __( 'Weight must be greater than zero.', 'woocommerce' );

	return __( 'Title must be greater than 1', 'text-domain' );
	return __( 'Title must be more than 1 character', 'text-domain' );

Invert the useValidation hook behavior to return an error instead of a boolean value #37695

Invert the useValidation hook behavior to return an error instead of a boolean value #37695

Uh oh!

Conversation

mdperez86 commented Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Submission Review Guidelines:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Uh oh!

github-actions bot commented Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results Summary

Uh oh!

codecov bot commented Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

joshuatf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshuatf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshuatf left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mdperez86 commented Apr 13, 2023 •

edited

Loading

github-actions bot commented Apr 13, 2023 •

edited

Loading

github-actions bot commented Apr 13, 2023 •

edited

Loading

codecov bot commented Apr 13, 2023 •

edited

Loading