Fix Products by Category JSON response error

[Aljullu]

I'm not sure what are the exact steps to reproduce this issue, I found it in a random test site I have but everything was working fine in all others. After adding the Products by Category block, the list of categories doesn't appear and instead it shows an error (The response is not a valid JSON response.):

Inspecting the JSON response, it's like this:

Warning: intval() expects at most 2 parameters, 3 given in .../wp-includes/rest-api/class-wp-rest-request.php on line 804
[{"id":21,"name":"Accessories","slug":"accessories","description":"","parent":18,"count":5,"image":null,"review_count":0, ...}, ...]

After this PR, everything works fine again:

Tagging this as blocker because I was unable to reproduce it in 2.5.15, so it must be a regression in 2.6.

How to test the changes in this Pull Request:

1. Add a Products by Category block and verify there are no errors and you can select a category.

[github-actions]

Size Change: 0 B

Total Size: 1.99 MB

ℹ️ View Unchanged

Filename	Size	Change
build/active-filters-frontend.js	7.23 kB	0 B
build/active-filters.js	7.89 kB	0 B
build/all-products-frontend.js	71.8 kB	0 B
build/all-products.js	19.2 kB	0 B
build/all-reviews-legacy.js	10.3 kB	0 B
build/all-reviews.js	10.6 kB	0 B
build/attribute-filter-frontend.js	16.7 kB	0 B
build/attribute-filter.js	11.5 kB	0 B
build/block-error-boundary-legacy.js	774 B	0 B
build/block-error-boundary.js	775 B	0 B
build/blocks-legacy.js	2.92 kB	0 B
build/blocks.js	2.93 kB	0 B
build/cart-frontend.js	113 kB	0 B
build/cart.js	32.2 kB	0 B
build/checkout-frontend.js	129 kB	0 B
build/checkout.js	37.8 kB	0 B
build/custom-select-control-style-legacy.js	782 B	0 B
build/custom-select-control-style.js	782 B	0 B
build/editor-legacy-rtl.css	12.5 kB	0 B
build/editor-legacy.css	12.6 kB	0 B
build/editor-rtl.css	13.5 kB	0 B
build/editor.css	13.5 kB	0 B
build/featured-category-legacy.js	146 kB	0 B
build/featured-category.js	146 kB	0 B
build/featured-product-legacy.js	9.49 kB	0 B
build/featured-product.js	9.8 kB	0 B
build/handpicked-products-legacy.js	6.88 kB	0 B
build/handpicked-products.js	7.16 kB	0 B
build/panel-style-legacy.js	773 B	0 B
build/panel-style.js	774 B	0 B
build/price-filter-frontend.js	13.8 kB	0 B
build/price-filter.js	9.97 kB	0 B
build/product-best-sellers-legacy.js	6.96 kB	0 B
build/product-best-sellers.js	7.23 kB	0 B
build/product-categories-legacy.js	3.22 kB	0 B
build/product-categories.js	3.21 kB	0 B
build/product-category-legacy.js	7.88 kB	0 B
build/product-category.js	8.14 kB	0 B
build/product-list-style-legacy.js	775 B	0 B
build/product-new-legacy.js	7.12 kB	0 B
build/product-new.js	7.4 kB	0 B
build/product-on-sale-legacy.js	7.49 kB	0 B
build/product-on-sale.js	7.8 kB	0 B
build/product-search-legacy.js	3.12 kB	0 B
build/product-search.js	3.36 kB	0 B
build/product-tag-legacy.js	6.07 kB	0 B
build/product-tag.js	6.33 kB	0 B
build/product-top-rated-legacy.js	7.09 kB	0 B
build/product-top-rated.js	7.37 kB	0 B
build/products-by-attribute-legacy.js	7.85 kB	0 B
build/products-by-attribute.js	8.12 kB	0 B
build/reviews-by-category-legacy.js	12.3 kB	0 B
build/reviews-by-category.js	12.7 kB	0 B
build/reviews-by-product-legacy.js	13.8 kB	0 B
build/reviews-by-product.js	14.2 kB	0 B
build/reviews-frontend-legacy.js	7.99 kB	0 B
build/reviews-frontend.js	8.81 kB	0 B
build/snackbar-notice-style-legacy.js	778 B	0 B
build/snackbar-notice-style.js	779 B	0 B
build/spinner-style-legacy.js	775 B	0 B
build/spinner-style.js	771 B	0 B
build/style-legacy-rtl.css	5.62 kB	0 B
build/style-legacy.css	5.63 kB	0 B
build/style-rtl.css	16.5 kB	0 B
build/style.css	16.5 kB	0 B
build/vendors-legacy.js	376 kB	0 B
build/vendors-style-legacy-rtl.css	1.65 kB	0 B
build/vendors-style-legacy.css	1.65 kB	0 B
build/vendors-style-legacy.js	105 B	0 B
build/vendors-style-rtl.css	1.65 kB	0 B
build/vendors-style.css	1.65 kB	0 B
build/vendors-style.js	105 B	0 B
build/vendors.js	473 kB	0 B
build/wc-blocks-data.js	7.08 kB	0 B
build/wc-blocks-middleware.js	931 B	0 B
build/wc-blocks-registry.js	1.79 kB	0 B
build/wc-payment-method-cheque.js	794 B	0 B
build/wc-payment-method-paypal.js	830 B	0 B
build/wc-payment-method-stripe.js	11.9 kB	0 B
build/wc-settings.js	2.14 kB	0 B

_{compressed-size-action}

[nerrad]

Looks good! In this case passing 10 is unneeded because unlike the equivalent JavaScript function (parseInt) php does default to 10 for the base here.

Also there's a php linting fail that is being reported by Travis.

Pre-approving so you can merge after the fix!

[mikejolley]

@Aljullu If this is never set to -1 for any reason we could/should use absint to avoid rolling our own methods https://developer.wordpress.org/reference/functions/absint/

[Aljullu]

@mikejolley the reason that we opted for our own wrapping method instead of absint was that we weren't sure if -1 was ever used so we wanted to play it save to avoid breaking any current usage.

Looking at the code, I can't spot any obvious place were per_page: -1 is set, but might be worth double-checking by somebody else more used to the PHP-side of the codebase. 🙂

But even if -1 is not currently used in our codebase, does it make sense to restrict it? What are the downsides of using our own methods for validation?

[mikejolley]

@Aljullu

even if -1 is not currently used in our codebase, does it make sense to restrict it?

yes it most certainly does make sense to restrict to valid values. The terms API uses 0 for unlimited results rather than -1, and I recall making all endpoints follow this rather than allowing for -1 values. The min is also set to 0 in the validation schema.

[nerrad]

Sorry Albert, I should have thought to look at the schema, I agree with Mike here, the schema for the API has 0 as the minimum value so it looks like absint is the way to go here (WP uses it internally on the number value too for the Term Query).

[Aljullu]

Makes sense, thanks for the input @mikejolley @nerrad. I will push a PR fixing this today. 👍