-
Notifications
You must be signed in to change notification settings - Fork 219
Fix: Product Collection show products with correct stock statuses #11708
Conversation
The release ZIP for this PR is accessible via:
Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
Size Change: +2.15 kB (0%) Total Size: 1.59 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.
I tested the PR and found that it functions correctly on the post but not as expected in the Product Catalog template. To replicate the issue:
- Add a product collection block to the Product Catalog template.
- In settings, Check the following checkbox (as shown in screenshot). Despite this, out of stock items remain visible in the Editor, although they are correctly hidden on the Frontend.
I believe I've identified the cause of this issue. Here's my detailed assessment:
The problem seems to manifest exclusively in the Editor, and not in the Frontend. This discrepancy arises due to the use of the query_loop_block_query_vars filter in our frontend query construction, as seen here. The invocation of this filter is conditional, as outlined here, and it only executes when the 'Inherit query from template' option is turned off. Consequently, our custom query logic is not applied.
In contrast, within the Editor, the rest_product_query filter, referenced here, does not depend on such conditions. It runs regardless of whether the 'inherit query from template' is active or not.
This issue went unnoticed previously because the 'inherit query from template' feature, when enabled, triggers a reset of attribute values to their defaults, as implemented here. Therefore, default values were used in our custom query, making the conditional execution of rest_product_query redundant.
As for a solution, I propose we introduce a condition within the rest_product_query filter that bypasses our custom query logic when 'Inherit query from template' is enabled. I had hoped to use the inherit attribute we set here as a part of this condition, but it's currently not accessible in the arguments of the rest_product_query filter ($args, $request). More debugging is required to understand why.
Regarding the merge of this PR, it seems feasible since the frontend functionality is performing as expected. What do you think? CC: @kmanijak
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.
Apologies for long wait, I must've missed the notification 🙌
I tested the PR and found that it functions correctly on the post but not as expected in the Product Catalog template. To replicate the issue:
Add a product collection block to the Product Catalog template.
In settings, Check the following checkbox (as shown in screenshot). Despite this, out of stock items remain visible in the Editor, although they are correctly hidden on the Frontend.
I couldn't reproduce this problem. In both cases - with this option enabled and disabled - I could see the right outcome in Editor and frontend (
Regarding the merge of this PR, it seems feasible since the frontend functionality is performing as expected. What do you think? CC: @kmanijak
I'd say we merge it - it's a nice improvement both ways! 🚀
I am still able to reproduce this issue. Here are the steps that I followed:
@dinhtungdu Are you able to reproduce this issue? 🙂 |
@imanish003 The Product Collection block only checks for the |
Good catch! The code here is designed to determine the stock status based on the global setting for hiding out-of-stock items in Editor. This function is triggered under two specific conditions:
A potential solution might be to assess whether “Inherit query from template” is enabled. If it is, we could always set the “stock status” attribute to the value returned by getDefaultStockStatuses. |
@imanish003 I don't see it's a problem here. I think it's good to respect the block stock status settings after it's been added and we shouldn't take the global setting into account after that.
|
Based on my understanding, this applies to the Frontend but not to the Editor. Consequently, the Frontend operates smoothly, aligning with expectations, but the Editor encounters issues in the scenario I described #11708 (comment).
Does this approach seem logical? 🙂 |
That sounds good to me! Thanks so much for your explanation! |
PHP Unit tests are failing, going to push this onto 11.7.0 milestone |
Test is fixed. Merging. |
What
This PR fixes the issue that Product Collection shows out of stock products event if that status isn't selected:
outofstock
.Fixes #
Why
We're handling query related to stock status a little more complex than other:
outofstock
, we add out of stock term to product visibility tax query.We do it for some reasons:
Testing Instructions
Please consider any edge cases this change may have, and also other areas of the product this may impact.
outofstock
.WooCommerce Visibility
Required:
Checklist
Required:
[type]
label or a[skip-changelog]
label.Conditional:
[skip-changelog]
label is not present).Changelog