-
Notifications
You must be signed in to change notification settings - Fork 220
Handle out of stock product visibility setting in All Products block. #3859
Conversation
Size Change: +106 B (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to see this being fixed. I needed to make a change to the code to get this working, but after that it was testing fine. I commented below the change I needed to do.
I would like @mikejolley to take a second look to this PR since I'm never 100% confident when reviewing code that queries the database, but to me it's looking good.
Also wondering if this is a breaking change to the store API. Should we keep supporting stock_status
being a string in addition to an array?
src/StoreApi/Routes/Products.php
Outdated
'items' => array( | ||
'type' => 'string', | ||
), | ||
'default' => [], | ||
'sanitize_callback' => 'sanitize_text_field', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think sanitize_callback
needs to be inside items
. Otherwise, it was not working for me. 😕
@@ -309,10 +309,14 @@ public function get_collection_params() { | |||
|
|||
$params['stock_status'] = array( | |||
'description' => __( 'Limit result set to products with specified stock status.', 'woo-gutenberg-products-block' ), | |||
'type' => 'string', | |||
'enum' => array_keys( wc_get_product_stock_status_options() ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry if I wasn't clear yesterday. We should keep enum
if possible, I just wasn't sure if it needs to be defined within or outside of items. Ideally we can keep this in place so invalid values are rejected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes pushed, please check again. I also moved sanitize_callback
and validate_callback
under items.
@Aljullu Supporting both a single string or an array would be interesting. I know you can define an array of types, but I am unsure who this would behave in conjunction with @ralucaStan Is any other code inside blocks using this query? There are some docs too in the StoreAPI folder which need updating if this changes from string to array. |
@@ -316,7 +316,7 @@ public function add_query_clauses( $args, $wp_query ) { | |||
|
|||
if ( $wp_query->get( 'stock_status' ) ) { | |||
$args['join'] = $this->append_product_sorting_table_join( $args['join'] ); | |||
$args['where'] .= $wpdb->prepare( ' AND wc_product_meta_lookup.stock_status = %s ', $wp_query->get( 'stock_status' ) ); | |||
$args['where'] .= ' AND wc_product_meta_lookup.stock_status IN ("' . implode( '","', array_map( 'esc_sql', $wp_query->get( 'stock_status' ) ) ) . '")'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mikejolley I did not find another block using this query.
Thanks for the heads-up, I've updated the docs file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job 👍🏻 Also magically still works with strings and comma separated value, so must be some magic behind the scenes to convert those values automatically :)
A new `hideOutOfStockItems` setting flag has been added and it is used to hide the out of stock products from the block. Products that are not out of stock can have 'instock', 'onbackorder' as stock status.
99c01b0
to
fe93c51
Compare
My product blocks are still displaying out of stock products in WooCommerce 5.1.0. Am I missing something? |
Is this issue the same as when you don't put a price on a variation? Because they show up aswell. |
Hi there @fishblizz, |
Hi @ralucaStan, my reference is this issue wich kinda got forwarded to Github: https://wordpress.org/support/topic/product-with-variations-shows-product-without-a-price/page/2/#post-14787001 |
HI @fishblizz, |
Hi @ralucaStan, thank you for reaction and confirmation that the other ticket is still actual. I will leave this one and move to the one you've pointed out. |
This issue is still there. Wrong products are still displayed. Not sensitive to the "display out of stock" setting in WooCommerce settings. So the block still shows out of stock products. Is the issue not properly understood here? |
@Magganpice I wasn't able to reproduce the issue with the latest version of WooCommerce and WooCommerce Blocks. Could you please provide us with detailed steps on how to reliably reproduce the issue? We'd also appreciate it if you could share which versions of WordPress, WooCommerce, and WooCommerce Blocks you are using. Thank you! |
A new
hideOutOfStockItems
setting flag has been added to reflect the user settingHide out of stock items from the catalog
. When true, the products call will include a field forstock_status: ['instock', 'onbackorder']
.Fixes #2824
How to test the changes in this Pull Request:
Out of stock
, and one asOn back order
Out of stock
product is missing, but theOn back order
is visiblePHP Changes
Not sure if the code here is the best, feel free to suggest improvements. Also make sure the code is not missing any edge cases.
Changelog