Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Fix Product categories, Product Tags & Keyword filter not working in Products block #8377

Merged
merged 12 commits into from
Feb 21, 2023
Merged

Fix Product categories, Product Tags & Keyword filter not working in Products block #8377

Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8beb36b
Fix product categories & keyword filter not working
imanish003 Feb 3, 2023
37f0ba0
Fix Product tags filter not working
imanish003 Feb 3, 2023
1954437
Merge branch 'trunk' into fix/8249-filters-not-working
imanish003 Feb 3, 2023
4095396
Merge branch 'trunk' into fix/8249-filters-not-working
imanish003 Feb 14, 2023
db26bdb
Make code readable by creating functions for the different parts of t…
imanish003 Feb 14, 2023
be0610f
Merge branch 'trunk' into fix/8249-filters-not-working
imanish003 Feb 14, 2023
6f871e2
Merge branch 'trunk' into fix/8249-filters-not-working
imanish003 Feb 17, 2023
e6bd862
Make function generic by including all taxonomies
imanish003 Feb 17, 2023
a5b2482
Minor improvements based on PR feedback
imanish003 Feb 17, 2023
4ad1ae9
Move null check for tax_query to function
imanish003 Feb 17, 2023
40b78a4
Merge branch 'trunk' into fix/8249-filters-not-working
imanish003 Feb 20, 2023
0039f49
Merge branch 'trunk' into fix/8249-filters-not-working
imanish003 Feb 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 55 additions & 3 deletions src/BlockTypes/ProductQuery.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ public function build_query( $query ) {
$this->get_global_query( $parsed_block ),
$this->get_custom_orderby_query( $query['orderby'] ),
$this->get_queries_by_attributes( $parsed_block ),
$this->get_queries_by_applied_filters()
$this->get_queries_by_applied_filters( $query )
);
}

Expand Down Expand Up @@ -474,15 +474,29 @@ function( $acc, $attribute ) {
/**
* Return queries that are generated by query args.
*
* @param array $query Query args.
*
* @return array
*/
private function get_queries_by_applied_filters() {
return array(
private function get_queries_by_applied_filters( $query ) {
$result = array(
'price_filter' => $this->get_filter_by_price_query(),
'attributes_filter' => $this->get_filter_by_attributes_query(),
'stock_status_filter' => $this->get_filter_by_stock_status_query(),
'rating_filter' => $this->get_filter_by_rating_query(),
);

// "Product Categories" & "Product Tags" could be provided using "Filters" ToolsPanel available in Inspector Controls.
if ( isset( $query['tax_query'] ) ) {
$result['tax_query'] = $this->get_filter_by_product_categories_or_tags_query( $query['tax_query'] );
}
Copy link
Contributor Author

@imanish003 imanish003 Feb 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sunyatasattva @gigitux Can you please take a look at this PR? I have a query regarding these lines of code.

As I am not sure what else could be there in $query['tax_query'], therefore I have created get_filter_by_product_categories_or_tags_query function to extract queries related to only Product categories & Product tags.

I am wondering if we could use $query['tax_query'] as it is 🤔 Is there any possibility of extra items in $query['tax_query'] which we won't need & therefore we need to extract only what we need((For example, only Product categories & tags))?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I can see that there is get_global_query function. But code of this function only runs if is_custom_inherit_global_query_implementation_enabled is true.

Only this function extracts Keyword which could be provided using "Filters" ToolsPanel available in Inspector Controls. Don't we need Keyword when is_custom_inherit_global_query_implementation_enabled is false?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if we could use $query['tax_query'] as it is 🤔 Is there any possibility of extra items in $query['tax_query'] which we won't need & therefore we need to extract only what we need((For example, only Product categories & tags))?

I don't recall exactly the reasoning behind it, as @gigitux has worked on that part of the code, but I believe it is intentional. Perhaps something to do with the interaction with filters blocks if we take tax_query as it is?

Or maybe it was a prophylactic measure to enable the custom_inherit_global_query, which we eventually want to switch to.

Don't we need Keyword when is_custom_inherit_global_query_implementation_enabled is false?

Yes, we definitely do. It must have been an oversight we didn't catch, as we were focused on implementing the custom inherit, before the scope was changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember very well, tbh 😭.

In any case, I think that we should not use the $query['tax_query'] as it is to be ready when we will switch to the custom inherit query logic.

We already have the logic to merge the queries.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gigitux 👋

In any case, I think that we should not use the $query['tax_query'] as it is to be ready when we will switch to the custom inherit query logic.

Got it, then the changes I made in this PR looks good to me. In short, I created get_filter_by_product_categories_or_tags_query function to extract product_tag & product_cat from the tax_query as you can see here. Also, I extracted the keyword here. Please let me know if you notice any problem with this implementation.

We already have the logic to merge the queries.

Right, but while calling merge_queries function, we were not passing tax_query for product categories, product tags & keyword. I made the changes in this PR to pass these to merge_queries function.

Please let me know if I misunderstood something & if you see any problem with changes I made in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gigitux That makes sense. There are two sections of filters in Editor i.e. Advance Filters & Filters (see screenshot)
image

As per my understanding, we are creating queries for Advance filters in get_queries_by_attributes (which isn't obvious from the name of function).

I am thinking to do one of the following:

  1. Maybe we can rename this function to get_filter_queries & return queries for Advance Filters & Filters from this function.
  2. Rename this function to get_advance_filter_queries to make it more readable and clear. And, create new function called get_filter_queries which will return queries for product categories, product tags & keyword i.e. all the filters. Then pass get_filter_queries() as argument here.

Which of these you think would be better approach?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per my understanding, we are creating queries for Advance filters in get_queries_by_attributes (which isn't obvious from the name of function).

That function generates queries by the custom attributes that the merchant sets via the editor. Furthermore, I think that there should not be a relationship between the UI and the name of the function of the backend.

So, I would create another function to create a valid query object starting from the $tax_query object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That function generates queries by the custom attributes that the merchant sets via the editor.

Got it, maybe we can rename the function to get_queries_by_custom_attributes just to make it more readable & clear. What do you think? 🤔

That function generates queries by the custom attributes that the merchant sets via the editor. Furthermore, I think that there should not be a relationship between the UI and the name of the function of the backend.
So, I would create another function to create a valid query object starting from the $tax_query object.

That makes sense. I will create a new function. Thanks for the feedback 🙌🏻

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think?

It makes sense! 👍

That makes sense. I will create a new function. Thanks for the feedback 🙌🏻

💪

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gigitux I made the changes in this commit. Would you like to have a quick look?


// "Keyword" could be provided using "Filters" ToolsPanel available in Inspector Controls.
if ( isset( $query['s'] ) ) {
$result['s'] = $query['s'];
}

return $result;
}

/**
Expand Down Expand Up @@ -814,4 +828,42 @@ function( $rating ) use ( $product_visibility_terms ) {
);
}


/**
* User could provide "Product Categories" using "Filters" ToolsPanel available in Inspector Controls.
* We use this function to extract it's query from $tax_query.
*
* For example, this is how the query for product categories will look like in $tax_query array:
* Array
* (
* [taxonomy] => product_cat
* [terms] => Array
* (
* [0] => 36
* )
* )
*
* For product categories, taxonomy would be "product_tag"
*
* @param array $tax_query Tax query.
* @return array
*/
private function get_filter_by_product_categories_or_tags_query( $tax_query ) {
if ( ! is_array( $tax_query ) ) {
return array();
}

$result = array();
foreach ( $tax_query as $item ) {
if ( isset( $item['taxonomy'] ) && (
'product_cat' === $item['taxonomy'] ||
'product_tag' === $item['taxonomy']
) ) {
$result[] = $item;
}
}

return $result;
}

}