Why on_sale and include did not work together in WooCommerce
WooCommerce has several REST endpoints that list products, such as /wc/v3/products and the Store API at /wc/store/v1/products that the product blocks use. They accept on_sale to show only products on sale or only products that are not, and include and exclude to limit the result to certain product IDs. Combined, those filters returned the wrong products. I fixed it in woocommerce/woocommerce#69432.
What went wrong
With products 10 to 12 not on sale and 13 to 18 on sale, on_sale=true&include=10,13 returned 10, 13, 15, 16, 17 and 18. Product 10 is not on sale, and 15 to 18 were never included. on_sale=false&include=10,13 returned both products, and on_sale=true&exclude=13 still returned 13.
Cause one, array union
include becomes post__in and exclude becomes post__not_in. The on_sale filter then added the IDs of every product on sale.
$on_sale_key = $request['on_sale'] ? 'post__in' : 'post__not_in';
$on_sale_ids = wc_get_product_ids_on_sale();
$args[ $on_sale_key ] += $on_sale_ids;In PHP, += on arrays is a union by key. Both arrays are lists with keys 0, 1, 2 and so on, so the on-sale IDs at keys that already existed were dropped and the rest were added. For include=10,13 that kept 10 and 13 and added the on-sale products from key 2 onwards.
Cause two, WP_Query
Fixing the += was not enough, which the reviewer found while testing. WP_Query only uses post__not_in when post__in is empty.
if ( $query_vars['post__in'] ) {
// WHERE ID IN (...)
} elseif ( $query_vars['post__not_in'] ) {
// WHERE ID NOT IN (...)
}So adding the on-sale IDs to post__not_in did nothing once include was set, and on_sale=true made exclude stop working.
The fix
When on_sale is true or include is set, the filter now works out the final list of IDs itself and puts it in post__in. Without include, on_sale=false still adds the on-sale IDs to post__not_in.
if ( $request['on_sale'] || ! empty( $args['post__in'] ) ) {
$post_in = empty( $args['post__in'] ) ? $on_sale_ids : $args['post__in'];
if ( $request['on_sale'] ) {
$post_in = array_intersect( $post_in, $on_sale_ids );
} else {
$post_in = array_diff( $post_in, $on_sale_ids );
}
$post_in = array_diff( $post_in, (array) $args['post__not_in'] );
$args['post__in'] = empty( $post_in ) ? array( 0 ) : array_values( $post_in );
} else {
$args['post__not_in'] = array_merge( (array) $args['post__not_in'], $on_sale_ids );
}The same code was in five places, the v2, v3 and v4 product endpoints, the variations endpoint and the Store API, so all five use this now. Tests cover on_sale with include and with exclude, and the reviewer added a few more before merging. The fix is scheduled for WooCommerce 11.3.
What to take from it
Use += on arrays only when you want to keep existing keys. For lists of IDs, use array_merge, array_intersect or array_diff. And if you build WP_Query arguments, do not rely on post__not_in while post__in is set.