-
Notifications
You must be signed in to change notification settings - Fork 221
Best selling products block: show only products with sales and most sold first #10448
Conversation
The release ZIP for this PR is accessible via:
Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
Size Change: 0 B Total Size: 1.34 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 have tested this, and it works correctly!
I used https://github.com/75nineteen/order-simulator-woocommerce to generate dummy orders - pretty useful :)
One question, shouldn't we also update the block.json
, which currently is set to:
"orderby": {
"type": "string",
"enum": [
"date",
"popularity",
"price_asc",
"price_desc",
"rating",
"title",
"menu_order"
],
"default": "popularity"
}
@danieldudzic 🤔 Actually, I think it's not really being used anywhere, I might be missing something but I think the |
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.
@albarin Thanks for the PR! In addition to the review comments, I have a concern about this.
As people are already using this block (which is a replacement to the shortcode), I'm afraid hiding the products that have no sale may affects the existing users as it changes the current behavior of the blocks. The impact may be small but it's definitely there.
What do you think about adding an option enabling merchant to choose whether to display products without sale?
For example, if I'm just starting a new store, given that I don't have much sales, I still want to use the best selling block as a potential way to drive more traffic from my home page into those products. Hiding all products that don't have any sale will make that block empty and prevent me to use it.
From developer perspective, I can setup the Best Selling Products block that display products that don't have any sales to make store looks good from the start and continue working as expect after that. If the block only displays products with sale, I need to use another block first (newest for example), and then comeback to replace with the Best Selling Products block when the store get more sales.
cc @nielslange as you originally opened the issue.
$query_args['meta_key'] = 'total_sales'; // phpcs:ignore WordPress.DB.SlowDBQuery | ||
$query_args['orderby'] = 'meta_value_num'; | ||
$query_args['order'] = 'DESC'; |
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.
The order query arguments will be later overridden by AbstractProductGrid::set_ordering_query_args()
, which explains the Danny comment.
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.
If we want to remove the orderby
attribute in the block.json
file, we will need to reorder the call of set_ordering_query_args()
in AbstractProductGrid
so the query variables set by child classes won't be override by other methods in AbstractProductGrid
.
@@ -19,6 +19,14 @@ class ProductBestSellers extends AbstractProductGrid { | |||
* @param array $query_args Query args. | |||
*/ | |||
protected function set_block_query_args( &$query_args ) { | |||
$query_args['orderby'] = 'popularity'; |
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.
This is the correct query for best selling product. I wonder why do you need to change it 🤔
@dinhtungdu thanks for your feedback, I think you are right, we need to take into account that users are already using the block. The same happens with #10434 which I merged last week, I think I'm going to revert since it has not been released yet. |
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
Apologies for my delayed response, @albarin. I haven't seen your message earlier. I do like @dinhtungdu's suggestion, but I would create adjust the PR in a way that only best-selling products are visible by default. If a merchant wants to show all products, meaning also products without sales, then the merchant needs to manually adjust the visibility. To me, it's rather confusing that the Best Selling Products block shows all products by default, independent of if they've been purchased ion the past or not. To me personally, only products that have been sold can become best-selling products. |
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
This PR updates the query in the
Best Selling Product
blocks to show only products with sales and show the most sold before.Fixes woocommerce/woocommerce#42596
Screenshots
Testing
User Facing Testing
Best Selling Product
block.WooCommerce Visibility
Changelog