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

Product Query: Unify data structure for tax_query and meta_query #7745

Closed
dinhtungdu opened this issue Nov 25, 2022 · 2 comments · Fixed by #7697
Closed

Product Query: Unify data structure for tax_query and meta_query #7745

dinhtungdu opened this issue Nov 25, 2022 · 2 comments · Fixed by #7697
Labels
block-type: product-query Issues related to/affecting all product-query variations. type: enhancement The issue is a request for an enhancement.

Comments

@dinhtungdu
Copy link
Member

Is your feature request related to a problem? Please describe.

In Product Query, we're using different data structures for partial tax_query and meta_query objects:

In get_stock_status_query() and get_filter_by_price_query(), the meta_query key holds an array containing query data (key, value, compare).

private function get_stock_status_query( $stock_statii ) {
return array(
'meta_query' => array(
'key' => '_stock_status',
'value' => (array) $stock_statii,
'compare' => 'IN',
),
);
}

return array(
'meta_query' => array(
'relation' => 'AND',
$max_price_query,
$min_price_query,
),
);

Same for tax_query in get_filter_by_attributes_query():

return array(
'tax_query' => array(
'relation' => 'AND',
$queries,
),
);


But in get_filter_by_stock_status_query(), the meta_query key holds an array containing a nested array which contains query data:

return array(
// Ignoring the warning of not using meta queries.
// phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_query
'meta_query' => array(
array(
'key' => '_stock_status',
'value' => $filtered_stock_status_values,
'operator' => 'IN',
),
),
);

Describe the solution you'd like

We should unify the way we return tax_query and meta_query data. We could go either way but should use only one.

Additional context

This will be solved by #7697. I raise this issue to explain the reason for returned data changes in #7697. cc @gigitux @sunyatasattva

@dinhtungdu dinhtungdu added type: enhancement The issue is a request for an enhancement. block-type: product-query Issues related to/affecting all product-query variations. labels Nov 25, 2022
@sunyatasattva
Copy link
Contributor

sunyatasattva commented Nov 25, 2022

Correct me if I'm wrong here, but I think the odd one out is get_stock_status_query(), right? Because get_filter_by_price_query() contains the relationship key at the top level, but then has nested arrays, as it should be. Or am I missing the point here? I think the wrong one is get_filter_by_price_query() (which, incidentally, I wrote 😬 ) and we should fix and change that. I agree that we should have a consistent shape.

EDIT: Ok I checked #7697 and I understand what you mean with get_filter_by_price_query(). I agree with the approach you took there. We should however think about a top-level relationship key at some point. But that's more a design discussion.

@dinhtungdu
Copy link
Member Author

We should however think about a top-level relationship key at some point. But that's more a design discussion.

I agree. For now, in #7697, we simply merge them, and each partial query (meta, date, tax) will be encapsulated inside an array.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
block-type: product-query Issues related to/affecting all product-query variations. type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants