From 32cce6032d9d9db6e88b28b234ad9aeca4650023 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Thu, 13 May 2021 12:29:23 +0200 Subject: [PATCH 1/8] Add Filterer class and use it when filtering by attributes lookup table usage is enabled. --- includes/class-wc-query.php | 63 ++++++++-- ...ProductAttributesLookupServiceProvider.php | 4 + .../ProductAttributesLookup/Filterer.php | 119 ++++++++++++++++++ .../LookupDataStore.php | 9 ++ 4 files changed, 183 insertions(+), 12 deletions(-) create mode 100644 src/Internal/ProductAttributesLookup/Filterer.php diff --git a/includes/class-wc-query.php b/includes/class-wc-query.php index 3575e228de0..5e87e910d60 100644 --- a/includes/class-wc-query.php +++ b/includes/class-wc-query.php @@ -6,6 +6,8 @@ * @package WooCommerce\Classes */ +use Automattic\WooCommerce\Internal\ProductAttributesLookup\Filterer; + defined( 'ABSPATH' ) || exit; /** @@ -34,10 +36,19 @@ class WC_Query { */ private static $chosen_attributes; + /** + * The instance of the class that helps filtering with the product attributes lookup table. + * + * @var Filterer + */ + private $filterer; + /** * Constructor for the query class. Hooks in methods. */ public function __construct() { + $this->filterer = wc_get_container()->get( Filterer::class ); + add_action( 'init', array( $this, 'add_endpoints' ) ); if ( ! is_admin() ) { add_action( 'wp_loaded', array( $this, 'get_errors' ), 20 ); @@ -134,7 +145,7 @@ class WC_Query { $title = __( 'Add payment method', 'woocommerce' ); break; case 'lost-password': - if ( in_array( $action, array( 'rp', 'resetpass', 'newaccount' ) ) ) { + if ( in_array( $action, array( 'rp', 'resetpass', 'newaccount' ), true ) ) { $title = __( 'Set password', 'woocommerce' ); } else { $title = __( 'Lost password', 'woocommerce' ); @@ -487,12 +498,38 @@ class WC_Query { self::$product_query = $q; // Additonal hooks to change WP Query. - add_filter( 'posts_clauses', array( $this, 'price_filter_post_clauses' ), 10, 2 ); + add_filter( + 'posts_clauses', + function( $args, $wp_query ) { + return $this->product_query_post_clauses( $args, $wp_query ); + }, + 10, + 2 + ); add_filter( 'the_posts', array( $this, 'handle_get_posts' ), 10, 2 ); do_action( 'woocommerce_product_query', $q, $this ); } + /** + * Add extra clauses to the product query. + * + * @param array $args Product query clauses. + * @param WP_Query $wp_query The current product query. + * @return array The updated product query clauses array. + */ + private function product_query_post_clauses( $args, $wp_query ) { + $args = $this->price_filter_post_clauses( $args, $wp_query ); + $args = $this->filterer->filter_by_attribute_post_clauses( $args, $wp_query, $this->get_layered_nav_chosen_attributes() ); + + $search = $this->get_main_search_query_sql(); + if ( $search ) { + $args['where'] .= ' AND ' . $search; + } + + return $args; + } + /** * Remove the query. */ @@ -722,16 +759,18 @@ class WC_Query { ); } - // Layered nav filters on terms. - if ( $main_query ) { - foreach ( $this->get_layered_nav_chosen_attributes() as $taxonomy => $data ) { - $tax_query[] = array( - 'taxonomy' => $taxonomy, - 'field' => 'slug', - 'terms' => $data['terms'], - 'operator' => 'and' === $data['query_type'] ? 'AND' : 'IN', - 'include_children' => false, - ); + if ( ! $this->filterer->filtering_via_lookup_table_is_active() ) { + // Layered nav filters on terms. + if ( $main_query ) { + foreach ( $this->get_layered_nav_chosen_attributes() as $taxonomy => $data ) { + $tax_query[] = array( + 'taxonomy' => $taxonomy, + 'field' => 'slug', + 'terms' => $data['terms'], + 'operator' => 'and' === $data['query_type'] ? 'AND' : 'IN', + 'include_children' => false, + ); + } } } diff --git a/src/Internal/DependencyManagement/ServiceProviders/ProductAttributesLookupServiceProvider.php b/src/Internal/DependencyManagement/ServiceProviders/ProductAttributesLookupServiceProvider.php index f1a818c25f0..173621078d6 100644 --- a/src/Internal/DependencyManagement/ServiceProviders/ProductAttributesLookupServiceProvider.php +++ b/src/Internal/DependencyManagement/ServiceProviders/ProductAttributesLookupServiceProvider.php @@ -7,6 +7,7 @@ namespace Automattic\WooCommerce\Internal\DependencyManagement\ServiceProviders; use Automattic\WooCommerce\Internal\DependencyManagement\AbstractServiceProvider; use Automattic\WooCommerce\Internal\ProductAttributesLookup\DataRegenerator; +use Automattic\WooCommerce\Internal\ProductAttributesLookup\Filterer; use Automattic\WooCommerce\Internal\ProductAttributesLookup\LookupDataStore; /** @@ -21,6 +22,8 @@ class ProductAttributesLookupServiceProvider extends AbstractServiceProvider { */ protected $provides = array( DataRegenerator::class, + Filterer::class, + LookupDataStore::class, ); /** @@ -28,6 +31,7 @@ class ProductAttributesLookupServiceProvider extends AbstractServiceProvider { */ public function register() { $this->share( DataRegenerator::class )->addArgument( LookupDataStore::class ); + $this->share( Filterer::class )->addArgument( LookupDataStore::class ); $this->share( LookupDataStore::class ); } } diff --git a/src/Internal/ProductAttributesLookup/Filterer.php b/src/Internal/ProductAttributesLookup/Filterer.php new file mode 100644 index 00000000000..8b4600cc0cc --- /dev/null +++ b/src/Internal/ProductAttributesLookup/Filterer.php @@ -0,0 +1,119 @@ +data_store = $data_store; + $this->lookup_table_name = $data_store->get_lookup_table_name(); + } + + /** + * Checks if the product attribute filtering via lookup table feature is enabled. + * + * @return bool + */ + public function filtering_via_lookup_table_is_active() { + return 'yes' === get_option( 'woocommerce_attribute_lookup__enabled' ); + } + + /** + * Adds post clauses for filtering via lookup table. + * This method should be invoked within a 'posts_clauses' filter. + * + * @param array $args Product query clauses as supplied to the 'posts_clauses' filter. + * @param \WP_Query $wp_query Current product query as supplied to the 'posts_clauses' filter. + * @param array $attributes_to_filter_by Attribute filtering data as generated by WC_Query::get_layered_nav_chosen_attributes. + * @return array The updated product query clauses. + */ + public function filter_by_attribute_post_clauses( array $args, \WP_Query $wp_query, array $attributes_to_filter_by ) { + global $wpdb; + + if ( ! $wp_query->is_main_query() || ! $this->filtering_via_lookup_table_is_active() ) { + return $args; + } + + $clause_root = " {$wpdb->prefix}posts.ID IN ("; + if ( 'yes' === get_option( 'woocommerce_hide_out_of_stock_items' ) ) { + $in_stock_clause = ' AND in_stock = 1'; + } else { + $in_stock_clause = ''; + } + + foreach ( $attributes_to_filter_by as $taxonomy => $data ) { + $all_terms = get_terms( $taxonomy, array( 'hide_empty' => false ) ); + $term_ids_by_slug = wp_list_pluck( $all_terms, 'term_id', 'slug' ); + $term_ids_to_filter_by = array_values( array_intersect_key( $term_ids_by_slug, array_flip( $data['terms'] ) ) ); + $term_ids_to_filter_by_list = '(' . join( ',', $term_ids_to_filter_by ) . ')'; + $is_and_query = 'and' === $data['query_type']; + + $count = count( $term_ids_to_filter_by ); + if ( 0 !== $count ) { + if ( $is_and_query ) { + $clauses[] = " + {$clause_root} + SELECT product_or_parent_id + FROM {$this->lookup_table_name} lt + WHERE is_variation_attribute=0 + {$in_stock_clause} + AND term_id in {$term_ids_to_filter_by_list} + GROUP BY product_id + HAVING COUNT(product_id)={$count} + UNION + SELECT product_or_parent_id + FROM {$this->lookup_table_name} lt + WHERE is_variation_attribute=1 + {$in_stock_clause} + AND term_id in {$term_ids_to_filter_by_list} + )"; + } else { + $clauses[] = " + {$clause_root} + SELECT product_or_parent_id + FROM {$this->lookup_table_name} lt + WHERE term_id in {$term_ids_to_filter_by_list} + {$in_stock_clause} + )"; + } + } + } + + if ( ! empty( $clauses ) ) { + $args['where'] .= ' AND (' . join( ' AND ', $clauses ) . ')'; + } elseif ( ! empty( $attributes_to_filter_by ) ) { + $args['where'] .= ' AND 1=0'; + } + + return $args; + } +} diff --git a/src/Internal/ProductAttributesLookup/LookupDataStore.php b/src/Internal/ProductAttributesLookup/LookupDataStore.php index 77cc2845238..22d6d432850 100644 --- a/src/Internal/ProductAttributesLookup/LookupDataStore.php +++ b/src/Internal/ProductAttributesLookup/LookupDataStore.php @@ -61,6 +61,15 @@ class LookupDataStore { $this->is_feature_visible = false; } + /** + * Get the name of the lookup table. + * + * @return string + */ + public function get_lookup_table_name() { + return $this->lookup_table_name; + } + /** * Insert or update the lookup data for a given product or variation. * If a variable product is passed the information is updated for all of its variations. From e085898fc45a8f6fa798d1aa30e543fd02b69095 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Fri, 14 May 2021 17:04:25 +0200 Subject: [PATCH 2/8] Use the product attributes lookup table for the filter by attribute widget when enabled. --- .../widgets/class-wc-widget-layered-nav.php | 69 +----- .../ProductAttributesLookup/Filterer.php | 198 ++++++++++++++++++ 2 files changed, 201 insertions(+), 66 deletions(-) diff --git a/includes/widgets/class-wc-widget-layered-nav.php b/includes/widgets/class-wc-widget-layered-nav.php index e72711f175d..c4a4bb141a6 100644 --- a/includes/widgets/class-wc-widget-layered-nav.php +++ b/includes/widgets/class-wc-widget-layered-nav.php @@ -6,6 +6,8 @@ * @version 2.6.0 */ +use Automattic\WooCommerce\Internal\ProductAttributesLookup\Filterer; + defined( 'ABSPATH' ) || exit; /** @@ -342,72 +344,7 @@ class WC_Widget_Layered_Nav extends WC_Widget { * @return array */ protected function get_filtered_term_product_counts( $term_ids, $taxonomy, $query_type ) { - global $wpdb; - - $tax_query = $this->get_main_tax_query(); - $meta_query = $this->get_main_meta_query(); - - if ( 'or' === $query_type ) { - foreach ( $tax_query as $key => $query ) { - if ( is_array( $query ) && $taxonomy === $query['taxonomy'] ) { - unset( $tax_query[ $key ] ); - } - } - } - - $meta_query = new WP_Meta_Query( $meta_query ); - $tax_query = new WP_Tax_Query( $tax_query ); - $meta_query_sql = $meta_query->get_sql( 'post', $wpdb->posts, 'ID' ); - $tax_query_sql = $tax_query->get_sql( $wpdb->posts, 'ID' ); - $term_ids_sql = '(' . implode( ',', array_map( 'absint', $term_ids ) ) . ')'; - - // Generate query. - $query = array(); - $query['select'] = "SELECT COUNT( DISTINCT {$wpdb->posts}.ID ) AS term_count, terms.term_id AS term_count_id"; - $query['from'] = "FROM {$wpdb->posts}"; - $query['join'] = " - INNER JOIN {$wpdb->term_relationships} AS term_relationships ON {$wpdb->posts}.ID = term_relationships.object_id - INNER JOIN {$wpdb->term_taxonomy} AS term_taxonomy USING( term_taxonomy_id ) - INNER JOIN {$wpdb->terms} AS terms USING( term_id ) - " . $tax_query_sql['join'] . $meta_query_sql['join']; - - $query['where'] = " - WHERE {$wpdb->posts}.post_type IN ( 'product' ) - AND {$wpdb->posts}.post_status = 'publish' - {$tax_query_sql['where']} {$meta_query_sql['where']} - AND terms.term_id IN $term_ids_sql"; - - $search = $this->get_main_search_query_sql(); - if ( $search ) { - $query['where'] .= ' AND ' . $search; - } - - $query['group_by'] = 'GROUP BY terms.term_id'; - $query = apply_filters( 'woocommerce_get_filtered_term_product_counts_query', $query ); - $query_sql = implode( ' ', $query ); - - // We have a query - let's see if cached results of this query already exist. - $query_hash = md5( $query_sql ); - - // Maybe store a transient of the count values. - $cache = apply_filters( 'woocommerce_layered_nav_count_maybe_cache', true ); - if ( true === $cache ) { - $cached_counts = (array) get_transient( 'wc_layered_nav_counts_' . sanitize_title( $taxonomy ) ); - } else { - $cached_counts = array(); - } - - if ( ! isset( $cached_counts[ $query_hash ] ) ) { - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared - $results = $wpdb->get_results( $query_sql, ARRAY_A ); - $counts = array_map( 'absint', wp_list_pluck( $results, 'term_count', 'term_count_id' ) ); - $cached_counts[ $query_hash ] = $counts; - if ( true === $cache ) { - set_transient( 'wc_layered_nav_counts_' . sanitize_title( $taxonomy ), $cached_counts, DAY_IN_SECONDS ); - } - } - - return array_map( 'absint', (array) $cached_counts[ $query_hash ] ); + return wc_get_container()->get( Filterer::class )->get_filtered_term_product_counts( $term_ids, $taxonomy, $query_type ); } /** diff --git a/src/Internal/ProductAttributesLookup/Filterer.php b/src/Internal/ProductAttributesLookup/Filterer.php index 8b4600cc0cc..881b56f7f5b 100644 --- a/src/Internal/ProductAttributesLookup/Filterer.php +++ b/src/Internal/ProductAttributesLookup/Filterer.php @@ -116,4 +116,202 @@ class Filterer { return $args; } + + /** + * Count products within certain terms, taking the main WP query into consideration, + * for the WC_Widget_Layered_Nav widget. + * + * This query allows counts to be generated based on the viewed products, not all products. + * + * @param array $term_ids Term IDs. + * @param string $taxonomy Taxonomy. + * @param string $query_type Query Type. + * @return array + */ + public function get_filtered_term_product_counts( $term_ids, $taxonomy, $query_type ) { + global $wpdb; + + $use_lookup_table = $this->filtering_via_lookup_table_is_active(); + + $tax_query = \WC_Query::get_main_tax_query(); + $meta_query = \WC_Query::get_main_meta_query(); + if ( 'or' === $query_type ) { + foreach ( $tax_query as $key => $query ) { + if ( is_array( $query ) && $taxonomy === $query['taxonomy'] ) { + unset( $tax_query[ $key ] ); + } + } + } + + $meta_query = new \WP_Meta_Query( $meta_query ); + $tax_query = new \WP_Tax_Query( $tax_query ); + + if ( $use_lookup_table ) { + $query = $this->get_product_counts_query_using_lookup_table( $tax_query, $meta_query, $taxonomy, $term_ids ); + } else { + $query = $this->get_product_counts_query_not_using_lookup_table( $tax_query, $meta_query, $term_ids ); + } + + $query = apply_filters( 'woocommerce_get_filtered_term_product_counts_query', $query ); + $query_sql = implode( ' ', $query ); + + // We have a query - let's see if cached results of this query already exist. + $query_hash = md5( $query_sql ); + // Maybe store a transient of the count values. + $cache = apply_filters( 'woocommerce_layered_nav_count_maybe_cache', true ); + if ( true === $cache ) { + $cached_counts = (array) get_transient( 'wc_layered_nav_counts_' . sanitize_title( $taxonomy ) ); + } else { + $cached_counts = array(); + } + if ( ! isset( $cached_counts[ $query_hash ] ) ) { + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + $results = $wpdb->get_results( $query_sql, ARRAY_A ); + $counts = array_map( 'absint', wp_list_pluck( $results, 'term_count', 'term_count_id' ) ); + $cached_counts[ $query_hash ] = $counts; + if ( true === $cache ) { + set_transient( 'wc_layered_nav_counts_' . sanitize_title( $taxonomy ), $cached_counts, DAY_IN_SECONDS ); + } + } + return array_map( 'absint', (array) $cached_counts[ $query_hash ] ); + } + + /** + * Get the query for counting products by terms using the product attributes lookup table. + * + * @param \WP_Tax_Query $tax_query The current main tax query. + * @param \WP_Meta_Query $meta_query The current main meta query. + * @param string $taxonomy The attribute name to get the term counts for. + * @param string $term_ids The term ids to include in the search. + * @return array An array of SQL query parts. + */ + private function get_product_counts_query_using_lookup_table( $tax_query, $meta_query, $taxonomy, $term_ids ) { + global $wpdb; + + $meta_query_sql = $meta_query->get_sql( 'post', $this->lookup_table_name, 'product_or_parent_id' ); + $tax_query_sql = $tax_query->get_sql( $this->lookup_table_name, 'product_or_parent_id' ); + $hide_out_of_stock = 'yes' === get_option( 'woocommerce_hide_out_of_stock_items' ); + $in_stock_clause = $hide_out_of_stock ? ' AND in_stock = 1' : ''; + + $query['select'] = 'SELECT COUNT(DISTINCT product_or_parent_id) as term_count, term_id as term_count_id'; + $query['from'] = "FROM {$this->lookup_table_name}"; + $query['join'] = "INNER JOIN {$wpdb->posts} ON {$wpdb->posts}.ID = {$this->lookup_table_name}.product_or_parent_id"; + + $term_ids_sql = $this->get_term_ids_sql( $term_ids ); + $query['where'] = " + WHERE {$wpdb->posts}.post_type IN ( 'product' ) + AND {$wpdb->posts}.post_status = 'publish' + {$tax_query_sql['where']} {$meta_query_sql['where']} + AND {$this->lookup_table_name}.taxonomy='{$taxonomy}' + AND {$this->lookup_table_name}.term_id IN $term_ids_sql + {$in_stock_clause}"; + + if ( ! empty( $term_ids ) ) { + $attributes_to_filter_by = \WC_Query::get_layered_nav_chosen_attributes(); + + if ( ! empty( $attributes_to_filter_by ) ) { + $all_terms_to_filter_by = array(); + foreach ( $attributes_to_filter_by as $taxonomy => $data ) { + $all_terms = get_terms( $taxonomy, array( 'hide_empty' => false ) ); + $term_ids_by_slug = wp_list_pluck( $all_terms, 'term_id', 'slug' ); + $term_ids_to_filter_by = array_values( array_intersect_key( $term_ids_by_slug, array_flip( $data['terms'] ) ) ); + $all_terms_to_filter_by = array_merge( $all_terms_to_filter_by, $term_ids_to_filter_by ); + $term_ids_to_filter_by_list = '(' . join( ',', $term_ids_to_filter_by ) . ')'; + + $count = count( $term_ids_to_filter_by ); + if ( 0 !== $count ) { + $query['where'] .= ' AND product_or_parent_id IN ('; + if ( 'and' === $attributes_to_filter_by[ $taxonomy ]['query_type'] ) { + $query['where'] .= " + SELECT product_or_parent_id + FROM {$this->lookup_table_name} lt + WHERE is_variation_attribute=0 + {$in_stock_clause} + AND term_id in {$term_ids_to_filter_by_list} + GROUP BY product_id + HAVING COUNT(product_id)={$count} + UNION + SELECT product_or_parent_id + FROM {$this->lookup_table_name} lt + WHERE is_variation_attribute=1 + {$in_stock_clause} + AND term_id in {$term_ids_to_filter_by_list} + )"; + } else { + $query['where'] .= " + SELECT product_or_parent_id FROM {$this->lookup_table_name} + WHERE term_id in {$term_ids_to_filter_by_list} + {$in_stock_clause} + )"; + } + } + } + } else { + $query['where'] .= $in_stock_clause; + } + } elseif ( $hide_out_of_stock ) { + $query['where'] .= " AND {$this->lookup_table_name}.in_stock=1"; + } + + $search_query_sql = \WC_Query::get_main_search_query_sql(); + if ( $search_query_sql ) { + $query['where'] .= ' AND ' . $search_query_sql; + } + + $query['group_by'] = 'GROUP BY terms.term_id'; + $query['group_by'] = "GROUP BY {$this->lookup_table_name}.term_id"; + + return $query; + } + + /** + * Get the query for counting products by terms NOT using the product attributes lookup table. + * + * @param \WP_Tax_Query $tax_query The current main tax query. + * @param \WP_Meta_Query $meta_query The current main meta query. + * @param string $term_ids The term ids to include in the search. + * @return array An array of SQL query parts. + */ + private function get_product_counts_query_not_using_lookup_table( $tax_query, $meta_query, $term_ids ) { + global $wpdb; + + $meta_query_sql = $meta_query->get_sql( 'post', $wpdb->posts, 'ID' ); + $tax_query_sql = $tax_query->get_sql( $wpdb->posts, 'ID' ); + + // Generate query. + $query = array(); + $query['select'] = "SELECT COUNT( DISTINCT {$wpdb->posts}.ID ) AS term_count, terms.term_id AS term_count_id"; + $query['from'] = "FROM {$wpdb->posts}"; + $query['join'] = " + INNER JOIN {$wpdb->term_relationships} AS term_relationships ON {$wpdb->posts}.ID = term_relationships.object_id + INNER JOIN {$wpdb->term_taxonomy} AS term_taxonomy USING( term_taxonomy_id ) + INNER JOIN {$wpdb->terms} AS terms USING( term_id ) + " . $tax_query_sql['join'] . $meta_query_sql['join']; + + $term_ids_sql = $this->get_term_ids_sql( $term_ids ); + $query['where'] = " + WHERE {$wpdb->posts}.post_type IN ( 'product' ) + AND {$wpdb->posts}.post_status = 'publish' + {$tax_query_sql['where']} {$meta_query_sql['where']} + AND terms.term_id IN $term_ids_sql"; + + $search_query_sql = \WC_Query::get_main_search_query_sql(); + if ( $search_query_sql ) { + $query['where'] .= ' AND ' . $search_query_sql; + } + + $query['group_by'] = 'GROUP BY terms.term_id'; + + return $query; + } + + /** + * Formats a list of term ids as "(id,id,id)". + * + * @param array $term_ids The list of terms to format. + * @return string The formatted list. + */ + private function get_term_ids_sql( $term_ids ) { + return '(' . implode( ',', array_map( 'absint', $term_ids ) ) . ')'; + } } From 9878aa37aa8a0a799321857a90bd42b7318cf9a1 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Fri, 28 May 2021 17:27:03 +0200 Subject: [PATCH 3/8] Add unit tests for the Filterer class (simple products only for now) Also fix a small issue in the product counters when using "or" filter. --- includes/class-wc-query.php | 7 + ...ProductAttributesLookupServiceProvider.php | 3 +- .../ProductAttributesLookup/Filterer.php | 9 +- .../ProductAttributesLookup/FiltererTest.php | 551 ++++++++++++++++++ 4 files changed, 562 insertions(+), 8 deletions(-) create mode 100644 tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php diff --git a/includes/class-wc-query.php b/includes/class-wc-query.php index 5e87e910d60..0f2375c6aa5 100644 --- a/includes/class-wc-query.php +++ b/includes/class-wc-query.php @@ -60,6 +60,13 @@ class WC_Query { $this->init_query_vars(); } + /** + * Reset the chosen attributes so that get_layered_nav_chosen_attributes will get them from the query again. + */ + public static function reset_chosen_attributes() { + self::$chosen_attributes = null; + } + /** * Get any errors from querystring. */ diff --git a/src/Internal/DependencyManagement/ServiceProviders/ProductAttributesLookupServiceProvider.php b/src/Internal/DependencyManagement/ServiceProviders/ProductAttributesLookupServiceProvider.php index 173621078d6..31fba73fe63 100644 --- a/src/Internal/DependencyManagement/ServiceProviders/ProductAttributesLookupServiceProvider.php +++ b/src/Internal/DependencyManagement/ServiceProviders/ProductAttributesLookupServiceProvider.php @@ -9,6 +9,7 @@ use Automattic\WooCommerce\Internal\DependencyManagement\AbstractServiceProvider use Automattic\WooCommerce\Internal\ProductAttributesLookup\DataRegenerator; use Automattic\WooCommerce\Internal\ProductAttributesLookup\Filterer; use Automattic\WooCommerce\Internal\ProductAttributesLookup\LookupDataStore; +use Automattic\WooCommerce\Proxies\LegacyProxy; /** * Service provider for the ProductAttributesLookupServiceProvider namespace. @@ -31,7 +32,7 @@ class ProductAttributesLookupServiceProvider extends AbstractServiceProvider { */ public function register() { $this->share( DataRegenerator::class )->addArgument( LookupDataStore::class ); - $this->share( Filterer::class )->addArgument( LookupDataStore::class ); + $this->share( Filterer::class )->addArgument( LookupDataStore::class )->addArgument( LegacyProxy::class ); $this->share( LookupDataStore::class ); } } diff --git a/src/Internal/ProductAttributesLookup/Filterer.php b/src/Internal/ProductAttributesLookup/Filterer.php index 881b56f7f5b..4146b314469 100644 --- a/src/Internal/ProductAttributesLookup/Filterer.php +++ b/src/Internal/ProductAttributesLookup/Filterer.php @@ -220,9 +220,8 @@ class Filterer { $count = count( $term_ids_to_filter_by ); if ( 0 !== $count ) { - $query['where'] .= ' AND product_or_parent_id IN ('; if ( 'and' === $attributes_to_filter_by[ $taxonomy ]['query_type'] ) { - $query['where'] .= " + $query['where'] .= " AND product_or_parent_id IN ( SELECT product_or_parent_id FROM {$this->lookup_table_name} lt WHERE is_variation_attribute=0 @@ -238,11 +237,7 @@ class Filterer { AND term_id in {$term_ids_to_filter_by_list} )"; } else { - $query['where'] .= " - SELECT product_or_parent_id FROM {$this->lookup_table_name} - WHERE term_id in {$term_ids_to_filter_by_list} - {$in_stock_clause} - )"; + $query['where'] .= $in_stock_clause; } } } diff --git a/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php b/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php new file mode 100644 index 00000000000..94c6bcf9e24 --- /dev/null +++ b/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php @@ -0,0 +1,551 @@ +query( + " + CREATE TABLE IF NOT EXISTS {$wpdb->prefix}wc_product_attributes_lookup ( + product_id bigint(20) NOT NULL, + product_or_parent_id bigint(20) NOT NULL, + taxonomy varchar(32) NOT NULL, + term_id bigint(20) NOT NULL, + is_variation_attribute tinyint(1) NOT NULL, + in_stock tinyint(1) NOT NULL + ); + " + ); + + $wpdb->query( "TRUNCATE TABLE {$wpdb->prefix}wc_product_attributes_lookup" ); + + // This is required too for WC_Query to act on the main query. + $wp_post_types['product']->has_archive = true; + } + + /** + * Runs after each test. + */ + public function tearDown() { + global $wpdb; + + parent::tearDown(); + + // Unregister all product attributes. + + $attribute_ids_by_name = wc_get_attribute_taxonomy_ids(); + foreach ( $attribute_ids_by_name as $attribute_name => $attribute_id ) { + $attribute_name = wc_sanitize_taxonomy_name( $attribute_name ); + $taxonomy_name = wc_attribute_taxonomy_name( $attribute_name ); + unregister_taxonomy( $taxonomy_name ); + + wc_delete_attribute( $attribute_id ); + } + + // Remove all products. + + $product_ids = wc_get_products( array( 'return' => 'ids' ) ); + foreach ( $product_ids as $product_id ) { + $product = wc_get_product( $product_id ); + $is_variable = $product->is_type( 'variable' ); + + foreach ( $product->get_children() as $child_id ) { + $child = wc_get_product( $child_id ); + if ( empty( $child ) ) { + continue; + } + + if ( $is_variable ) { + $child->delete( true ); + } else { + $child->set_parent_id( 0 ); + $child->save(); + } + } + + $product->delete( true ); + } + + $wpdb->query( "TRUNCATE TABLE {$wpdb->prefix}wc_product_attributes_lookup" ); + + \WC_Query::reset_chosen_attributes(); + } + + /** + * Core function to create a product. + * + * @param string $class_name The name of the product class that will be instantiated to create the product. + * @param array $attribute_terms_by_name An array of product attributes, keys are attribute names, values are arrays of attribute term names. + * @param bool $attributes_define_variations True if the attributes are used to define variations. + * @return mixed An instance of the class passed in $class_name representing the created product. + */ + private function create_product_core( $class_name, $attribute_terms_by_name, $attributes_define_variations ) { + $attributes = array(); + $attribute_ids_by_name = wc_get_attribute_taxonomy_ids(); + + foreach ( $attribute_terms_by_name as $attribute_name => $attribute_terms ) { + $sanitized_attribute_name = wc_sanitize_taxonomy_name( $attribute_name ); + + $term_ids = array(); + foreach ( $attribute_terms as $term ) { + $term_ids[] = (int) term_exists( $term, 'pa_' . $sanitized_attribute_name )['term_id']; + } + + $attribute = new \WC_Product_Attribute(); + $attribute->set_id( $attribute_ids_by_name ); + $attribute->set_name( 'pa_' . $sanitized_attribute_name ); + $attribute->set_options( $term_ids ); + $attribute->set_visible( true ); + $attribute->set_variation( $attributes_define_variations ); + $attributes[] = $attribute; + } + + $product = new $class_name(); + $product->set_props( + array( + 'name' => 'Product', + 'regular_price' => 1, + 'price' => 1, + 'sku' => 'DUMMY SKU', + 'manage_stock' => false, + 'tax_status' => 'taxable', + 'downloadable' => false, + 'virtual' => false, + ) + ); + + $product->set_attributes( $attributes ); + + return $product; + } + + /** + * Creates a simple product. + * + * @param array $attribute_terms_by_name An array of product attributes, keys are attribute names, values are arrays of attribute term names. + * @param bool $in_stock True if the product is in stock, false otherwise. + * @return array The product data, as generated by the REST API product creation entry point. + */ + private function create_simple_product( $attribute_terms_by_name, $in_stock ) { + $product = $this->create_product_core( \WC_Product_Simple::class, $attribute_terms_by_name, false ); + + $product->set_stock_status( $in_stock ? 'instock' : 'outofstock' ); + + $product->save(); + + if ( empty( $attribute_terms_by_name ) ) { + return $product; + } + + $lookup_insert_clauses = array(); + $lookup_insert_values = array(); + + foreach ( $attribute_terms_by_name as $name => $terms ) { + $id = $product->get_id(); + $this->compose_lookup_table_insert( $id, $id, $name, $terms, $lookup_insert_clauses, $lookup_insert_values, $in_stock, false ); + } + + $this->run_lookup_table_insert( $lookup_insert_clauses, $lookup_insert_values ); + + return $product; + } + + + /** + * Compose the values part of a query to insert data in the lookup table. + * + * @param int $product_id Value for the "product_id" column. + * @param int $product_or_parent_id Value for the "product_or_parent_id" column. + * @param string $attribute_name Taxonomy name of the attribute. + * @param array $terms Term names to insert for the attribute. + * @param array $insert_query_parts Array of strings to add the new query parts to. + * @param array $insert_query_values Array of values to add the new query values to. + * @param bool $in_stock True if the product/variation is in stock, false otherwise. + * @param bool $is_variation True if it's an attribute that defines a variation, false otherwise. + */ + private function compose_lookup_table_insert( $product_id, $product_or_parent_id, $attribute_name, $terms, &$insert_query_parts, &$insert_query_values, $in_stock, $is_variation ) { + $taxonomy_name = wc_attribute_taxonomy_name( $attribute_name ); + $term_objects = get_terms( $taxonomy_name ); + $term_ids_by_names = wp_list_pluck( $term_objects, 'term_id', 'name' ); + + foreach ( $terms as $term ) { + $insert_query_parts[] = '(%d, %d, %s, %d, %d, %d )'; + $insert_query_values[] = $product_id; + $insert_query_values[] = $product_or_parent_id; + $insert_query_values[] = wc_attribute_taxonomy_name( $attribute_name ); + $insert_query_values[] = $term_ids_by_names[ $term ]; + $insert_query_values[] = $is_variation ? 1 : 0; + $insert_query_values[] = $in_stock ? 1 : 0; + } + } + + /** + * Runs an insert clause in the lookup table. + * The clauses and values are to be generated with compose_lookup_table_insert. + * + * @param array $insert_query_parts Array of strings with query parts. + * @param array $insert_values Array of values for the query. + */ + private function run_lookup_table_insert( $insert_query_parts, $insert_values ) { + global $wpdb; + + // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared + + $insert_query = + "INSERT INTO {$wpdb->prefix}wc_product_attributes_lookup ( product_id, product_or_parent_id, taxonomy, term_id, is_variation_attribute, in_stock ) VALUES " + . join( ',', $insert_query_parts ); + + $prepared_insert = $wpdb->prepare( $insert_query, $insert_values ); + + $wpdb->query( $prepared_insert ); + + // phpcs:enable WordPress.DB.PreparedSQL.NotPrepared + } + + /** + * Create a product attribute. + * + * @param string $name The attribute name. + * @param array $terms The terms that will be created for the attribute. + */ + private function create_product_attribute( $name, $terms ) { + return ProductHelper::create_attribute( $name, $terms ); + } + + /** + * Set the "hide out of stock products" option. + * + * @param bool $hide The value to set the option to. + */ + private function set_hide_out_of_stock_items( $hide ) { + update_option( 'woocommerce_hide_out_of_stock_items', $hide ? 'yes' : 'no' ); + } + + /** + * Set the "hide out of stock products" option. + * + * @param bool $use The value to set the option to. + */ + private function set_use_lookup_table( $use ) { + update_option( 'woocommerce_attribute_lookup__enabled', $use ? 'yes' : 'no' ); + } + + /** + * Simulate a product query. + * + * @param array $filters The attribute filters as an array of attribute name => attribute terms. + * @param array $query_types The query types for each attribute as an array of attribute name => "or"/"and". + * @return mixed + */ + private function do_product_request( $filters, $query_types = array() ) { + global $wp_the_query; + + foreach ( $filters as $name => $values ) { + $_GET[ 'filter_' . wc_sanitize_taxonomy_name( $name ) ] = join( ',', array_map( 'wc_sanitize_taxonomy_name', $values ) ); + } + + foreach ( $query_types as $name => $value ) { + $_GET[ 'query_type_' . wc_sanitize_taxonomy_name( $name ) ] = $value; + } + + return $wp_the_query->query( + array( + 'post_type' => 'product', + 'fields' => 'ids', + ) + ); + } + + /** + * Assert that the filter by attribute widget lists a given set of terms for an attribute + * (with a count of 1 each) + * + * @param string $attribute_name The attribute name the terms belong to. + * @param array $expected_terms The labelss of the terms that are expected to be listed. + * @param string $filter_type The filter type in use, "and" or "or". + */ + private function assert_counters( $attribute_name, $expected_terms, $filter_type = 'and' ) { + $widget = new class() extends \WC_Widget_Layered_Nav { + // phpcs:disable Generic.CodeAnalysis.UselessOverridingMethod, Squiz.Commenting.FunctionComment + public function get_filtered_term_product_counts( $term_ids, $taxonomy, $query_type ) { + return parent::get_filtered_term_product_counts( $term_ids, $taxonomy, $query_type ); + } + // phpcs:enable Generic.CodeAnalysis.UselessOverridingMethod, Squiz.Commenting.FunctionComment + }; + + $taxonomy = wc_attribute_taxonomy_name( $attribute_name ); + $term_ids_by_name = wp_list_pluck( get_terms( $taxonomy, array( 'hide_empty' => '1' ) ), 'term_id', 'name' ); + + $expected = array(); + foreach ( $expected_terms as $term ) { + $expected[ $term_ids_by_name[ $term ] ] = 1; + } + + $term_counts = $widget->get_filtered_term_product_counts( $term_ids_by_name, $taxonomy, $filter_type ); + $this->assertEquals( $expected, $term_counts ); + } + + /** + * Data provider for the test_filtering_simple_product_in_stock tests + * + * @return array[] + */ + public function data_provider_for_test_filtering_simple_product_in_stock() { + return array( + array( array(), 'and', true ), + array( array(), 'or', true ), + array( array( 'Blue' ), 'and', true ), + array( array( 'Blue' ), 'or', true ), + array( array( 'Blue', 'Red' ), 'and', true ), + array( array( 'Blue', 'Red' ), 'or', true ), + array( array( 'Green' ), 'and', false ), + array( array( 'Green' ), 'or', false ), + array( array( 'Blue', 'Green' ), 'and', false ), + array( array( 'Blue', 'Green' ), 'or', true ), + ); + } + + /** + * @testdox The product query shows a simple product only if it's not filtered out by the specified attribute filters (using lookup table). + * + * @dataProvider data_provider_for_test_filtering_simple_product_in_stock + * + * @param array $attributes The color attribute names that will be included in the query. + * @param string $filter_type The filtering type, "or" or "and". + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_simple_product_in_stock_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { + $this->set_use_lookup_table( true ); + $this->test_filtering_simple_product_in_stock_core( $attributes, $filter_type, $expected_to_be_visible ); + } + + /** + * @testdox The product query shows a simple product only if it's not filtered out by the specified attribute filters (not using lookup table). + * + * @dataProvider data_provider_for_test_filtering_simple_product_in_stock + * + * @param array $attributes The color attribute names that will be included in the query. + * @param string $filter_type The filtering type, "or" or "and". + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_simple_product_in_stock_not_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { + $this->set_use_lookup_table( false ); + $this->test_filtering_simple_product_in_stock_core( $attributes, $filter_type, $expected_to_be_visible ); + } + + /** + * Main code for the test_filtering_simple_product_in_stock tests. + * + * @param array $attributes The color attribute names that will be included in the query. + * @param string $filter_type The filtering type, "or" or "and". + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + private function test_filtering_simple_product_in_stock_core( $attributes, $filter_type, $expected_to_be_visible ) { + $this->create_product_attribute( 'Color', array( 'Blue', 'Red', 'Green' ) ); + + $product = $this->create_simple_product( + array( + 'Color' => array( + 'Blue', + 'Red', + ), + ), + true + ); + + $filtered_product_ids = $this->do_product_request( array( 'Color' => $attributes ), array( 'Color' => $filter_type ) ); + + if ( $expected_to_be_visible ) { + $this->assertEquals( array( $product->get_id() ), $filtered_product_ids ); + } else { + $this->assertEmpty( $filtered_product_ids ); + } + + $expected_to_be_included_in_count = 'or' === $filter_type || $expected_to_be_visible; + $this->assert_counters( 'Color', $expected_to_be_included_in_count ? array( 'Blue', 'Red' ) : array(), $filter_type ); + } + + /** + * Data provider for the test_filtering_simple_product_out_of_stock tests. + * + * @return array + */ + public function data_provider_for_test_filtering_simple_product_out_of_stock() { + return array( + array( false, true, true ), + array( false, false, true ), + array( true, true, true ), + array( true, false, false ), + ); + } + + /** + * @testdox The product query shows a simple product only if it's in stock OR we don't have "hide out of stock items" set. + * + * @xtestWith [false, true, true] + * [false, false, true] + * [true, true, true] + * [true, false, false] + * + * @dataProvider data_provider_for_test_filtering_simple_product_out_of_stock + * + * @param bool $hide_out_of_stock The value of the "hide out of stock products" option. + * @param bool $is_in_stock True if the product is in stock, false otherwise. + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_simple_product_out_of_stock_using_lookup_table( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ) { + $this->set_use_lookup_table( true ); + $this->test_filtering_simple_product_out_of_stock_core( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ); + } + + /** + * @testdox The product query shows a simple product only if it's in stock OR we don't have "hide out of stock items" set. + * + * @xtestWith [false, true, true] + * [false, false, true] + * [true, true, true] + * [true, false, false] + * + * @dataProvider data_provider_for_test_filtering_simple_product_out_of_stock + * + * @param bool $hide_out_of_stock The value of the "hide out of stock products" option. + * @param bool $is_in_stock True if the product is in stock, false otherwise. + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_simple_product_out_of_stock_not_using_lookup_table( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ) { + $this->set_use_lookup_table( false ); + $this->test_filtering_simple_product_out_of_stock_core( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ); + } + + /** + * Main code for the test_filtering_simple_product_out_of_stock tests. + * + * @param bool $hide_out_of_stock The value of the "hide out of stock products" option. + * @param bool $is_in_stock True if the product is in stock, false otherwise. + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + private function test_filtering_simple_product_out_of_stock_core( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ) { + $this->create_product_attribute( 'Features', array( 'Washable', 'Ironable', 'Elastic' ) ); + + $product = $this->create_simple_product( + array( 'Features' => array( 'Washable', 'Ironable' ) ), + $is_in_stock + ); + + $this->set_hide_out_of_stock_items( $hide_out_of_stock ); + + $filtered_product_ids = $this->do_product_request( array() ); + + if ( $expected_to_be_visible ) { + $this->assertEquals( array( $product->get_id() ), $filtered_product_ids ); + } else { + $this->assertEmpty( $filtered_product_ids ); + } + + $this->assert_counters( 'Features', $expected_to_be_visible ? array( 'Washable', 'Ironable' ) : array() ); + } + + /** + * Data provider for the test_filtering_simple_product_by_multiple_attributes tests. + * + * @return array[] + */ + public function data_provider_for_test_filtering_simple_product_by_multiple_attributes() { + return array( + array( array(), array(), true ), + array( array( 'Blue' ), array(), true ), + array( array(), array( 'Ironable' ), true ), + array( array( 'Blue' ), array( 'Ironable' ), true ), + array( array( 'Red' ), array(), false ), + array( array(), array( 'Washable' ), false ), + array( array( 'Blue' ), array( 'Washable' ), false ), + array( array( 'Red' ), array( 'Ironable' ), false ), + ); + } + + /** + * @testdox The product query shows a simple product only if it's not filtered out by the specified attribute filters (when filtering by multiple attributes, using the lookup table). + * + * Worth noting that multiple attributes are always combined in an AND fashion for filtering. + * + * @dataProvider data_provider_for_test_filtering_simple_product_by_multiple_attributes + * + * @param array $attributes_1 The color attribute names that will be included in the query. + * @param array $attributes_2 The features attribute names that will be included in the query. + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_simple_product_by_multiple_attributes_using_lookup_table( $attributes_1, $attributes_2, $expected_to_be_visible ) { + $this->set_use_lookup_table( true ); + $this->test_filtering_simple_product_by_multiple_attributes_core( $attributes_1, $attributes_2, $expected_to_be_visible ); + } + + /** + * @testdox The product query shows a simple product only if it's not filtered out by the specified attribute filters (when filtering by multiple attributes, not using the lookup table). + * + * Worth noting that multiple attributes are always combined in an AND fashion for filtering. + * + * @dataProvider data_provider_for_test_filtering_simple_product_by_multiple_attributes + * + * @param array $attributes_1 The color attribute names that will be included in the query. + * @param array $attributes_2 The features attribute names that will be included in the query. + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_simple_product_by_multiple_attributes_not_using_using_lookup_table( $attributes_1, $attributes_2, $expected_to_be_visible ) { + $this->set_use_lookup_table( false ); + $this->test_filtering_simple_product_by_multiple_attributes_core( $attributes_1, $attributes_2, $expected_to_be_visible ); + } + + /** + * Main code for the test_filtering_simple_product_by_multiple_attributes tests. + * + * @param array $attributes_1 The color attribute names that will be included in the query. + * @param array $attributes_2 The features attribute names that will be included in the query. + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + private function test_filtering_simple_product_by_multiple_attributes_core( $attributes_1, $attributes_2, $expected_to_be_visible ) { + $this->create_product_attribute( 'Color', array( 'Blue', 'Red' ) ); + $this->create_product_attribute( 'Features', array( 'Ironable', 'Washable' ) ); + + $product = $this->create_simple_product( + array( + 'Color' => array( + 'Blue', + ), + 'Features' => array( + 'Ironable', + ), + ), + true + ); + + $filtered_product_ids = $this->do_product_request( + array( + 'Color' => $attributes_1, + 'Features' => $attributes_2, + ) + ); + + if ( $expected_to_be_visible ) { + $this->assertEquals( array( $product->get_id() ), $filtered_product_ids ); + } else { + $this->assertEmpty( $filtered_product_ids ); + } + + $this->assert_counters( 'Color', $expected_to_be_visible ? array( 'Blue' ) : array() ); + $this->assert_counters( 'Features', $expected_to_be_visible ? array( 'Ironable' ) : array() ); + } +} From c78627e6ee5ebfdc271bf055361d39adef90ca2d Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Mon, 31 May 2021 16:54:55 +0200 Subject: [PATCH 4/8] Add unit tests for the Filterer class (variable products) --- .../ProductAttributesLookup/FiltererTest.php | 665 +++++++++++++++++- 1 file changed, 637 insertions(+), 28 deletions(-) diff --git a/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php b/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php index 94c6bcf9e24..0d5f0dd90c3 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php @@ -4,6 +4,7 @@ namespace Automattic\WooCommerce\Tests\Internal\ProductAttributesLookup; use Automattic\WooCommerce\Internal\AttributesHelper; use Automattic\WooCommerce\RestApi\UnitTests\Helpers\ProductHelper; +use Automattic\WooCommerce\Utilities\ArrayUtil; /** * Tests related to filtering for WC_Query. @@ -88,30 +89,49 @@ class FiltererTest extends \WC_Unit_Test_Case { /** * Core function to create a product. * + * Format of $product_attributes is: + * + * [ + * 'non_variation_defining' => [ + * 'Name' => ['Value','Value'],... + * ] + * 'variation_defining' => [ + * 'Name' => ['Value','Value'],... + * ] + * ] + * * @param string $class_name The name of the product class that will be instantiated to create the product. - * @param array $attribute_terms_by_name An array of product attributes, keys are attribute names, values are arrays of attribute term names. - * @param bool $attributes_define_variations True if the attributes are used to define variations. + * @param array $product_attributes The product attributes. * @return mixed An instance of the class passed in $class_name representing the created product. */ - private function create_product_core( $class_name, $attribute_terms_by_name, $attributes_define_variations ) { + private function create_product_core( $class_name, $product_attributes ) { $attributes = array(); $attribute_ids_by_name = wc_get_attribute_taxonomy_ids(); - foreach ( $attribute_terms_by_name as $attribute_name => $attribute_terms ) { - $sanitized_attribute_name = wc_sanitize_taxonomy_name( $attribute_name ); + $product_attributes = array( + false => ArrayUtil::get_value_or_default( $product_attributes, 'non_variation_defining', array() ), + true => ArrayUtil::get_value_or_default( $product_attributes, 'variation_defining', array() ), + ); + foreach ( $product_attributes as $defines_variation => $attribute_terms_by_name ) { + foreach ( $attribute_terms_by_name as $attribute_name => $attribute_terms ) { + if ( ! is_array( $attribute_terms ) ) { + $attribute_terms = array( $attribute_terms ); + } + $sanitized_attribute_name = wc_sanitize_taxonomy_name( $attribute_name ); - $term_ids = array(); - foreach ( $attribute_terms as $term ) { - $term_ids[] = (int) term_exists( $term, 'pa_' . $sanitized_attribute_name )['term_id']; + $term_ids = array(); + foreach ( $attribute_terms as $term ) { + $term_ids[] = (int) term_exists( $term, 'pa_' . $sanitized_attribute_name )['term_id']; + } + + $attribute = new \WC_Product_Attribute(); + $attribute->set_id( $attribute_ids_by_name ); + $attribute->set_name( 'pa_' . $sanitized_attribute_name ); + $attribute->set_options( $term_ids ); + $attribute->set_visible( true ); + $attribute->set_variation( $defines_variation ); + $attributes[] = $attribute; } - - $attribute = new \WC_Product_Attribute(); - $attribute->set_id( $attribute_ids_by_name ); - $attribute->set_name( 'pa_' . $sanitized_attribute_name ); - $attribute->set_options( $term_ids ); - $attribute->set_visible( true ); - $attribute->set_variation( $attributes_define_variations ); - $attributes[] = $attribute; } $product = new $class_name(); @@ -141,7 +161,7 @@ class FiltererTest extends \WC_Unit_Test_Case { * @return array The product data, as generated by the REST API product creation entry point. */ private function create_simple_product( $attribute_terms_by_name, $in_stock ) { - $product = $this->create_product_core( \WC_Product_Simple::class, $attribute_terms_by_name, false ); + $product = $this->create_product_core( \WC_Product_Simple::class, array( 'non_variation_defining' => $attribute_terms_by_name ) ); $product->set_stock_status( $in_stock ? 'instock' : 'outofstock' ); @@ -164,6 +184,132 @@ class FiltererTest extends \WC_Unit_Test_Case { return $product; } + /** + * Creates a variable product. + * Format for the supplied data: + * + * variation_attributes => [ + * Color => [Red, Blue, Green], + * Size => [Big, Medium, Small] + * ], + * non_variation_attributes => [ + * Features => [Washable, Ironable] + * ], + * variations => [ + * [ + * defining_attributes => [ + * Color => Red, + * Size => Small + * ], + * in_stock => true + * ], + * [ + * defining_attributes => [ + * Color => Red, + * Size => null //Means "Any" + * ], + * in_stock => false + * ], + * ] + * + * Format for the returned data: + * + * [ + * id => 1, + * variation_ids => [2,3] + * ] + * + * @param array $data The data for creating the product. + * @returns array The product and variation ids. + */ + private function create_variable_product( $data ) { + + // * First create the main product. + + $product = $this->create_product_core( + \WC_Product_Variable::class, + array( + 'non_variation_defining' => $data['non_variation_attributes'], + 'variation_defining' => $data['variation_attributes'], + ) + ); + + $product->save(); + + $product_id = $product->get_id(); + + // * Now create the variations. + + $variation_ids = array(); + + foreach ( $data['variations'] as $variation_data ) { + $variation = new \WC_Product_Variation(); + $variation->set_props( + array( + 'parent_id' => $product->get_id(), + 'regular_price' => 10, + ) + ); + $attributes = array(); + foreach ( $variation_data['defining_attributes'] as $attribute_name => $attribute_value ) { + $attribute_name = wc_attribute_taxonomy_name( $attribute_name ); + $attribute_value = wc_sanitize_taxonomy_name( $attribute_value ); + $attributes[ $attribute_name ] = $attribute_value; + + } + $variation->set_attributes( $attributes ); + $variation->set_stock_status( $variation_data['in_stock'] ? 'instock' : 'outofstock' ); + $variation->save(); + + $variation_ids[] = $variation->get_id(); + } + + // This is needed because it's not done by the REST API. + \WC_Product_Variable::sync_stock_status( $product_id ); + + // * And finally, insert the data in the lookup table. + + $lookup_insert_clauses = array(); + $lookup_insert_values = array(); + + if ( ! empty( $data['non_variation_attributes'] ) ) { + $main_product_in_stock = ! empty( + array_filter( + $data['variations'], + function( $variation ) { + return $variation['in_stock']; + } + ) + ); + + foreach ( $data['non_variation_attributes'] as $name => $terms ) { + $this->compose_lookup_table_insert( $product->get_id(), $product->get_id(), $name, $terms, $lookup_insert_clauses, $lookup_insert_values, $main_product_in_stock, false ); + } + } + + reset( $variation_ids ); + foreach ( $data['variations'] as $variation_data ) { + $variation_id = current( $variation_ids ); + + foreach ( $variation_data['defining_attributes'] as $attribute_name => $attribute_value ) { + if ( is_null( $attribute_value ) ) { + $attribute_values = $data['variation_attributes'][ $attribute_name ]; + } else { + $attribute_values = array( $attribute_value ); + } + $this->compose_lookup_table_insert( $variation_id, $product->get_id(), $attribute_name, $attribute_values, $lookup_insert_clauses, $lookup_insert_values, $variation_data['in_stock'], true ); + } + + next( $variation_ids ); + } + + $this->run_lookup_table_insert( $lookup_insert_clauses, $lookup_insert_values ); + + return array( + 'id' => $product_id, + 'variation_ids' => $variation_ids, + ); + } /** * Compose the values part of a query to insert data in the lookup table. @@ -179,7 +325,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ private function compose_lookup_table_insert( $product_id, $product_or_parent_id, $attribute_name, $terms, &$insert_query_parts, &$insert_query_values, $in_stock, $is_variation ) { $taxonomy_name = wc_attribute_taxonomy_name( $attribute_name ); - $term_objects = get_terms( $taxonomy_name ); + $term_objects = get_terms( $taxonomy_name, array( 'hide_empty' => false ) ); $term_ids_by_names = wp_list_pluck( $term_objects, 'term_id', 'name' ); foreach ( $terms as $term ) { @@ -255,7 +401,9 @@ class FiltererTest extends \WC_Unit_Test_Case { global $wp_the_query; foreach ( $filters as $name => $values ) { - $_GET[ 'filter_' . wc_sanitize_taxonomy_name( $name ) ] = join( ',', array_map( 'wc_sanitize_taxonomy_name', $values ) ); + if ( ! empty( $values ) ) { + $_GET[ 'filter_' . wc_sanitize_taxonomy_name( $name ) ] = join( ',', array_map( 'wc_sanitize_taxonomy_name', $values ) ); + } } foreach ( $query_types as $name => $value ) { @@ -330,7 +478,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_simple_product_in_stock_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { $this->set_use_lookup_table( true ); - $this->test_filtering_simple_product_in_stock_core( $attributes, $filter_type, $expected_to_be_visible ); + $this->base_test_filtering_simple_product_in_stock( $attributes, $filter_type, $expected_to_be_visible ); } /** @@ -344,7 +492,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_simple_product_in_stock_not_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { $this->set_use_lookup_table( false ); - $this->test_filtering_simple_product_in_stock_core( $attributes, $filter_type, $expected_to_be_visible ); + $this->base_test_filtering_simple_product_in_stock( $attributes, $filter_type, $expected_to_be_visible ); } /** @@ -354,7 +502,7 @@ class FiltererTest extends \WC_Unit_Test_Case { * @param string $filter_type The filtering type, "or" or "and". * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. */ - private function test_filtering_simple_product_in_stock_core( $attributes, $filter_type, $expected_to_be_visible ) { + private function base_test_filtering_simple_product_in_stock( $attributes, $filter_type, $expected_to_be_visible ) { $this->create_product_attribute( 'Color', array( 'Blue', 'Red', 'Green' ) ); $product = $this->create_simple_product( @@ -409,7 +557,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_simple_product_out_of_stock_using_lookup_table( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ) { $this->set_use_lookup_table( true ); - $this->test_filtering_simple_product_out_of_stock_core( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ); + $this->base_test_filtering_simple_product_out_of_stock( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ); } /** @@ -428,7 +576,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_simple_product_out_of_stock_not_using_lookup_table( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ) { $this->set_use_lookup_table( false ); - $this->test_filtering_simple_product_out_of_stock_core( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ); + $this->base_test_filtering_simple_product_out_of_stock( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ); } /** @@ -438,7 +586,7 @@ class FiltererTest extends \WC_Unit_Test_Case { * @param bool $is_in_stock True if the product is in stock, false otherwise. * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. */ - private function test_filtering_simple_product_out_of_stock_core( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ) { + private function base_test_filtering_simple_product_out_of_stock( $hide_out_of_stock, $is_in_stock, $expected_to_be_visible ) { $this->create_product_attribute( 'Features', array( 'Washable', 'Ironable', 'Elastic' ) ); $product = $this->create_simple_product( @@ -490,7 +638,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_simple_product_by_multiple_attributes_using_lookup_table( $attributes_1, $attributes_2, $expected_to_be_visible ) { $this->set_use_lookup_table( true ); - $this->test_filtering_simple_product_by_multiple_attributes_core( $attributes_1, $attributes_2, $expected_to_be_visible ); + $this->base_test_filtering_simple_product_by_multiple_attributes( $attributes_1, $attributes_2, $expected_to_be_visible ); } /** @@ -506,7 +654,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_simple_product_by_multiple_attributes_not_using_using_lookup_table( $attributes_1, $attributes_2, $expected_to_be_visible ) { $this->set_use_lookup_table( false ); - $this->test_filtering_simple_product_by_multiple_attributes_core( $attributes_1, $attributes_2, $expected_to_be_visible ); + $this->base_test_filtering_simple_product_by_multiple_attributes( $attributes_1, $attributes_2, $expected_to_be_visible ); } /** @@ -516,7 +664,7 @@ class FiltererTest extends \WC_Unit_Test_Case { * @param array $attributes_2 The features attribute names that will be included in the query. * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. */ - private function test_filtering_simple_product_by_multiple_attributes_core( $attributes_1, $attributes_2, $expected_to_be_visible ) { + private function base_test_filtering_simple_product_by_multiple_attributes( $attributes_1, $attributes_2, $expected_to_be_visible ) { $this->create_product_attribute( 'Color', array( 'Blue', 'Red' ) ); $this->create_product_attribute( 'Features', array( 'Ironable', 'Washable' ) ); @@ -548,4 +696,465 @@ class FiltererTest extends \WC_Unit_Test_Case { $this->assert_counters( 'Color', $expected_to_be_visible ? array( 'Blue' ) : array() ); $this->assert_counters( 'Features', $expected_to_be_visible ? array( 'Ironable' ) : array() ); } + + /** + * Data provider for the test_filtering_variable_product_in_stock_for_non_variation_defining_attributes tests. + * + * @return array[] + */ + public function data_provider_for_test_filtering_variable_product_in_stock_for_non_variation_defining_attributes() { + return array( + array( array(), 'and', true ), + array( array(), 'or', true ), + array( array( 'Washable' ), 'and', true ), + array( array( 'Washable' ), 'or', true ), + array( array( 'Washable', 'Ironable' ), 'and', true ), + array( array( 'Washable', 'Ironable' ), 'or', true ), + array( array( 'Elastic' ), 'and', false ), + array( array( 'Elastic' ), 'or', false ), + array( array( 'Washable', 'Elastic' ), 'and', false ), + array( array( 'Washable', 'Elastic' ), 'or', true ), + ); + } + + /** + * @testdox The product query shows a variable product only if it's not filtered out by the specified attribute filters (for non-variation-defining attributes), using the lookup table. + * + * @dataProvider data_provider_for_test_filtering_variable_product_in_stock_for_non_variation_defining_attributes + * + * @param array $attributes The feature attribute names that will be included in the query. + * @param string $filter_type The filtering type, "or" or "and". + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_variable_product_in_stock_for_non_variation_defining_attributes_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { + $this->set_use_lookup_table( true ); + $this->base_test_filtering_variable_product_in_stock_for_non_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible ); + } + + /** + * @testdox The product query shows a variable product only if it's not filtered out by the specified attribute filters (for non-variation-defining attributes), not using the lookup table. + * + * @dataProvider data_provider_for_test_filtering_variable_product_in_stock_for_non_variation_defining_attributes + * + * @param array $attributes The feature attribute names that will be included in the query. + * @param string $filter_type The filtering type, "or" or "and". + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_variable_product_in_stock_for_non_variation_defining_attributes_not_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { + $this->set_use_lookup_table( false ); + $this->base_test_filtering_variable_product_in_stock_for_non_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible ); + } + + /** + * Main code for the test_filtering_variable_product_in_stock_for_non_variation_defining_attributes tests. + * + * @param array $attributes The feature attribute names that will be included in the query. + * @param string $filter_type The filtering type, "or" or "and". + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + private function base_test_filtering_variable_product_in_stock_for_non_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible ) { + $this->create_product_attribute( 'Color', array( 'Blue', 'Red' ) ); + $this->create_product_attribute( 'Features', array( 'Washable', 'Ironable', 'Elastic' ) ); + + $product = $this->create_variable_product( + array( + 'variation_attributes' => array( + 'Color' => array( 'Blue', 'Red' ), + ), + 'non_variation_attributes' => array( + 'Features' => array( 'Washable', 'Ironable' ), + ), + 'variations' => array( + array( + 'in_stock' => true, + 'defining_attributes' => array( + 'Color' => 'Blue', + ), + ), + array( + 'in_stock' => true, + 'defining_attributes' => array( + 'Color' => 'Red', + ), + ), + ), + ) + ); + + $filtered_product_ids = $this->do_product_request( array( 'Features' => $attributes ), array( 'Features' => $filter_type ) ); + + if ( $expected_to_be_visible ) { + $this->assertEquals( array( $product['id'] ), $filtered_product_ids ); + } else { + $this->assertEmpty( $filtered_product_ids ); + } + + $expected_to_be_included_in_count = 'or' === $filter_type || $expected_to_be_visible; + $this->assert_counters( 'Features', $expected_to_be_included_in_count ? array( 'Washable', 'Ironable' ) : array(), $filter_type ); + } + + /** + * Data provider for the test_filtering_variable_product_out_of_stock tests. + * + * @return array[] + */ + public function data_provider_for_test_filtering_variable_product_out_of_stock() { + return array( + array( false, true, true, true ), + array( false, true, false, true ), + array( false, false, true, true ), + array( false, false, false, true ), + array( true, true, true, true ), + array( true, true, false, true ), + array( true, false, true, true ), + array( true, false, false, false ), + ); + } + + /** + * @testdox The product query shows a variable product only if at least one of the variations is in stock OR we don't have "hide out of stock items" set (using the lookup table). + * + * @dataProvider data_provider_for_test_filtering_variable_product_out_of_stock + * + * @param bool $hide_out_of_stock The value of the "hide out of stock products" option. + * @param bool $variation_1_is_in_stock True if the first variation is in stock, false otherwise. + * @param bool $variation_2_is_in_stock True if the second variation is in stock, false otherwise. + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_variable_product_out_of_stock_using_lookup_table( $hide_out_of_stock, $variation_1_is_in_stock, $variation_2_is_in_stock, $expected_to_be_visible ) { + $this->set_use_lookup_table( true ); + $this->base_test_filtering_variable_product_out_of_stock( $hide_out_of_stock, $variation_1_is_in_stock, $variation_2_is_in_stock, $expected_to_be_visible, true ); + } + + /** + * @testdox The product query shows a variable product only if at least one of the variations is in stock OR we don't have "hide out of stock items" set (using the lookup table). + * + * @dataProvider data_provider_for_test_filtering_variable_product_out_of_stock + * + * @param bool $hide_out_of_stock The value of the "hide out of stock products" option. + * @param bool $variation_1_is_in_stock True if the first variation is in stock, false otherwise. + * @param bool $variation_2_is_in_stock True if the second variation is in stock, false otherwise. + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_variable_product_out_of_stock_not_using_lookup_table( $hide_out_of_stock, $variation_1_is_in_stock, $variation_2_is_in_stock, $expected_to_be_visible ) { + $this->set_use_lookup_table( false ); + $this->base_test_filtering_variable_product_out_of_stock( $hide_out_of_stock, $variation_1_is_in_stock, $variation_2_is_in_stock, $expected_to_be_visible, false ); + } + + /** + * Main code for the test_filtering_variable_product_out_of_stock tests. + * + * @param bool $hide_out_of_stock The value of the "hide out of stock products" option. + * @param bool $variation_1_is_in_stock True if the first variation is in stock, false otherwise. + * @param bool $variation_2_is_in_stock True if the second variation is in stock, false otherwise. + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + * @param bool $using_lookup_table Are we using the lookup table?. + */ + private function base_test_filtering_variable_product_out_of_stock( $hide_out_of_stock, $variation_1_is_in_stock, $variation_2_is_in_stock, $expected_to_be_visible, $using_lookup_table ) { + $this->create_product_attribute( 'Color', array( 'Blue', 'Red' ) ); + + $product = $this->create_variable_product( + array( + 'variation_attributes' => array( + 'Color' => array( 'Blue', 'Red' ), + ), + 'non_variation_attributes' => array(), + 'variations' => array( + array( + 'in_stock' => $variation_1_is_in_stock, + 'defining_attributes' => array( + 'Color' => 'Blue', + ), + ), + array( + 'in_stock' => $variation_2_is_in_stock, + 'defining_attributes' => array( + 'Color' => 'Red', + ), + ), + ), + ) + ); + + $this->set_hide_out_of_stock_items( $hide_out_of_stock ); + + $filtered_product_ids = $this->do_product_request( array() ); + + if ( $expected_to_be_visible ) { + $this->assertEquals( array( $product['id'] ), $filtered_product_ids ); + } else { + $this->assertEmpty( $filtered_product_ids ); + } + + /** + * When using the lookup table, attribute counters only take in account in stock variations. + * When not using it, all variations are accounted if at least one of them has stock. + */ + + $expected_visible_attributes = array(); + if ( $using_lookup_table ) { + if ( ! $hide_out_of_stock || $variation_1_is_in_stock ) { + $expected_visible_attributes[] = 'Blue'; + } + if ( ! $hide_out_of_stock || $variation_2_is_in_stock ) { + $expected_visible_attributes[] = 'Red'; + } + } elseif ( ! $hide_out_of_stock || $variation_1_is_in_stock || $variation_2_is_in_stock ) { + $expected_visible_attributes = array( 'Blue', 'Red' ); + } + + $this->assert_counters( 'Color', $expected_visible_attributes ); + } + + /** + * Base data provider for the test_filtering_variable_product_in_stock_for_variation_defining_attributes tests. + * + * @return array[] + */ + private function data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_core() { + return array( + array( array(), 'and', true ), + array( array(), 'or', true ), + array( array( 'Blue' ), 'and', true ), + array( array( 'Blue' ), 'or', true ), + array( array( 'Blue', 'Red' ), 'and', true ), + array( array( 'Blue', 'Red' ), 'or', true ), + array( array( 'Green' ), 'and', false ), + array( array( 'Green' ), 'or', false ), + + ); + } + + /** + * Data provider for test_filtering_variable_product_in_stock_for_variation_defining_attributes_using_lookup_table. + * + * @return array[] + */ + public function data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_using_lookup_table() { + $data = $this->data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_core(); + + /** + * When filtering by an attribute having a variation AND another one not having it: + * The product shows, since when dealing with variation attributes we're effectively doing OR. + */ + + $data[] = array( array( 'Blue', 'Green' ), 'and', true ); + return $data; + } + + /** + * @testdox The product query shows a variable product only if it's not filtered out by the specified attribute filters (for variation-defining attributes), using the lookup table. + * + * Note that the difference with the simple product or the non-variation attributes case is that "and" is equivalent to "or". + * + * @dataProvider data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_using_lookup_table + * + * @param array $attributes The color attribute names that will be included in the query. + * @param string $filter_type The filtering type, "or" or "and". + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_variable_product_in_stock_for_variation_defining_attributes_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { + $this->set_use_lookup_table( true ); + $this->base_test_filtering_variable_product_in_stock_for_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible ); + } + + /** + * Data provider for test_filtering_variable_product_in_stock_for_variation_defining_attributes_not_using_lookup_table. + * + * @return array[] + */ + public function data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_not_using_lookup_table() { + $data = $this->data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_core(); + + /** + * When filtering by an attribute having a variation AND another one not having it: + * The product doesn't show because variation attributes are treated as non-variation ones. + */ + + $data[] = array( array( 'Blue', 'Green' ), 'and', false ); + return $data; + } + + /** + * @testdox The product query shows a variable product only if it's not filtered out by the specified attribute filters (for variation-defining attributes), not using the lookup table. + * + * Note that the difference with the simple product or the non-variation attributes case is that "and" is equivalent to "or". + * + * @dataProvider data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_not_using_lookup_table + * + * @param array $attributes The color attribute names that will be included in the query. + * @param string $filter_type The filtering type, "or" or "and". + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_variable_product_in_stock_for_variation_defining_attributes_not_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { + $this->set_use_lookup_table( false ); + $this->base_test_filtering_variable_product_in_stock_for_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible ); + } + + /** + * Main code for the test_filtering_variable_product_in_stock_for_variation_defining_attributes tests. + * + * @param array $attributes The color attribute names that will be included in the query. + * @param string $filter_type The filtering type, "or" or "and". + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + private function base_test_filtering_variable_product_in_stock_for_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible ) { + $this->create_product_attribute( 'Color', array( 'Blue', 'Red', 'Green' ) ); + + $product = $this->create_variable_product( + array( + 'variation_attributes' => array( + 'Color' => array( 'Blue', 'Red' ), + ), + 'non_variation_attributes' => array(), + 'variations' => array( + array( + 'in_stock' => true, + 'defining_attributes' => array( + 'Color' => 'Blue', + ), + ), + array( + 'in_stock' => true, + 'defining_attributes' => array( + 'Color' => 'Red', + ), + ), + ), + ) + ); + + $filtered_product_ids = $this->do_product_request( array( 'Color' => $attributes ), array( 'Color' => $filter_type ) ); + + if ( $expected_to_be_visible ) { + $this->assertEquals( array( $product['id'] ), $filtered_product_ids ); + } else { + $this->assertEmpty( $filtered_product_ids ); + } + + $expected_counted_attributes = 'or' === $filter_type || $expected_to_be_visible ? array( 'Blue', 'Red' ) : array(); + $this->assert_counters( 'Color', $expected_counted_attributes, $filter_type ); + } + + + /** + * Base data provider for the test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value tests. + * + * @return array[] + */ + private function data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_core() { + return array( + array( array(), 'and', true ), + array( array(), 'or', true ), + array( array( 'Blue' ), 'and', true ), + array( array( 'Blue' ), 'or', true ), + array( array( 'Red' ), 'and', true ), + array( array( 'Red' ), 'or', true ), + array( array( 'Green' ), 'and', true ), + array( array( 'Green' ), 'or', true ), + array( array( 'White' ), 'and', false ), + array( array( 'White' ), 'or', false ), + ); + } + + /** + * Data provider for test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_using_lookup_table. + * + * @return array[] + */ + public function data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_using_lookup_table() { + $data = $this->data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_core(); + + /** + * When filtering by attributes having a variation AND others not having it: + * The product shows, since when dealing with variation attributes we're effectively doing OR. + */ + $data[] = array( array( 'Blue', 'Red', 'Green', 'White' ), 'and', true ); + + return $data; + } + + /** + * @testdox The product query shows a variable product only if it's not filtered out by the specified attribute filters (for variation-defining attributes, with "Any" values), using the lookup table. + * + * @dataProvider data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_using_lookup_table + * + * @param array $attributes The color attribute names that will be included in the query. + * @param string $filter_type The filtering type, "or" or "and". + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { + $this->set_use_lookup_table( true ); + $this->base_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value( $attributes, $filter_type, $expected_to_be_visible ); + } + + /** + * Data provider for test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_not_using_lookup_table. + * + * @return array[] + */ + public function data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_not_using_lookup_table() { + $data = $this->data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_core(); + + /** + * When filtering by attributes having a variation AND others not having it: + * The product doesn't show because variation attributes are treated as non-variation ones. + */ + $data[] = array( array( 'Blue', 'Red', 'Green', 'White' ), 'and', false ); + + return $data; + } + + /** + * @testdox The product query shows a variable product only if it's not filtered out by the specified attribute filters (for variation-defining attributes, with "Any" values), not using the lookup table. + * + * @dataProvider data_provider_for_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_not_using_lookup_table + * + * @param array $attributes The color attribute names that will be included in the query. + * @param string $filter_type The filtering type, "or" or "and". + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_not_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { + $this->set_use_lookup_table( false ); + $this->base_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value( $attributes, $filter_type, $expected_to_be_visible ); + } + + /** + * Main code for the test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value tests. + * + * @param array $attributes The color attribute names that will be included in the query. + * @param string $filter_type The filtering type, "or" or "and". + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + private function base_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value( $attributes, $filter_type, $expected_to_be_visible ) { + $this->create_product_attribute( 'Color', array( 'Blue', 'Red', 'Green', 'White' ) ); + + $product = $this->create_variable_product( + array( + 'variation_attributes' => array( + 'Color' => array( 'Blue', 'Red', 'Green' ), + ), + 'non_variation_attributes' => array(), + 'variations' => array( + array( + 'in_stock' => true, + 'defining_attributes' => array( + 'Color' => null, + ), + ), + ), + ) + ); + + $filtered_product_ids = $this->do_product_request( array( 'Color' => $attributes ), array( 'Color' => $filter_type ) ); + + if ( $expected_to_be_visible ) { + $this->assertEquals( array( $product['id'] ), $filtered_product_ids ); + } else { + $this->assertEmpty( $filtered_product_ids ); + } + + $expected_to_be_included_in_count = 'or' === $filter_type || $expected_to_be_visible; + $this->assert_counters( 'Color', $expected_to_be_included_in_count ? array( 'Blue', 'Red', 'Green' ) : array(), $filter_type ); + } } From 72442f20bb56f241fe47bab3703cf31779fce5fc Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Tue, 1 Jun 2021 10:06:25 +0200 Subject: [PATCH 5/8] Add the remaining tests for the Filterer class. --- .../ProductAttributesLookup/FiltererTest.php | 240 ++++++++++++++++++ 1 file changed, 240 insertions(+) diff --git a/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php b/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php index 0d5f0dd90c3..61e5730f7d9 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php @@ -1157,4 +1157,244 @@ class FiltererTest extends \WC_Unit_Test_Case { $expected_to_be_included_in_count = 'or' === $filter_type || $expected_to_be_visible; $this->assert_counters( 'Color', $expected_to_be_included_in_count ? array( 'Blue', 'Red', 'Green' ) : array(), $filter_type ); } + + /** + * @testdox Products not in "publish" state aren't shown. + * + * @testWith [true, ["Red"]] + * [false, ["Blue", "Red"]] + * + * @param bool $using_lookup_table Use the lookup table?. + * @param array $expected_colors_included_in_counters Expected colors to be included in the widget counters. + */ + public function test_filtering_excludes_non_published_products( $using_lookup_table, $expected_colors_included_in_counters ) { + $this->set_use_lookup_table( $using_lookup_table ); + $this->create_product_attribute( 'Color', array( 'Blue', 'Red' ) ); + $this->create_product_attribute( 'Features', array( 'Washable', 'Ironable' ) ); + + $product_simple_1 = $this->create_simple_product( + array( 'Features' => array( 'Washable' ) ), + true + ); + + $product_simple_2 = $this->create_simple_product( + array( 'Features' => array( 'Ironable' ) ), + true + ); + + $product_variable_1 = $this->create_variable_product( + array( + 'variation_attributes' => array( + 'Color' => array( 'Blue', 'Red' ), + ), + 'non_variation_attributes' => array(), + 'variations' => array( + array( + 'in_stock' => true, + 'defining_attributes' => array( + 'Color' => 'Blue', + ), + ), + ), + ) + ); + + $product_variable_2 = $this->create_variable_product( + array( + 'variation_attributes' => array( + 'Color' => array( 'Blue', 'Red' ), + ), + 'non_variation_attributes' => array(), + 'variations' => array( + array( + 'in_stock' => true, + 'defining_attributes' => array( + 'Color' => 'Red', + ), + ), + ), + ) + ); + + $post_data = array( 'post_status' => 'draft' ); + $post_data['ID'] = $product_simple_1->get_id(); + wp_update_post( $post_data ); + $post_data['ID'] = $product_variable_1['id']; + wp_update_post( $post_data ); + + $filtered_product_ids = $this->do_product_request( array() ); + + $this->assertEquals( array( $product_simple_2->get_id(), $product_variable_2['id'] ), $filtered_product_ids ); + + $this->assert_counters( 'Color', $expected_colors_included_in_counters ); + $this->assert_counters( 'Features', array( 'Ironable' ) ); + } + + /** + * @testdox Hidden products aren't shown. + * + * @testWith [true, ["Red"]] + * [false, ["Blue", "Red"]] + * + * @param bool $using_lookup_table Use the lookup table?. + * @param array $expected_colors_included_in_counters Expected colors to be included in the widget counters. + */ + public function test_filtering_excludes_hidden_products( $using_lookup_table, $expected_colors_included_in_counters ) { + $this->set_use_lookup_table( $using_lookup_table ); + $this->create_product_attribute( 'Color', array( 'Blue', 'Red' ) ); + $this->create_product_attribute( 'Features', array( 'Washable', 'Ironable' ) ); + + $product_simple_1 = $this->create_simple_product( + array( 'Features' => array( 'Washable' ) ), + true + ); + + $product_simple_2 = $this->create_simple_product( + array( 'Features' => array( 'Ironable' ) ), + true + ); + + $product_variable_1 = $this->create_variable_product( + array( + 'variation_attributes' => array( + 'Color' => array( 'Blue', 'Red' ), + ), + 'non_variation_attributes' => array(), + 'variations' => array( + array( + 'in_stock' => true, + 'defining_attributes' => array( + 'Color' => 'Blue', + ), + ), + ), + ) + ); + + $product_variable_2 = $this->create_variable_product( + array( + 'variation_attributes' => array( + 'Color' => array( 'Blue', 'Red' ), + ), + 'non_variation_attributes' => array(), + 'variations' => array( + array( + 'in_stock' => true, + 'defining_attributes' => array( + 'Color' => 'Red', + ), + ), + ), + ) + ); + + $terms = array( 'exclude-from-catalog' ); + wp_set_object_terms( $product_simple_1->get_id(), $terms, 'product_visibility' ); + wp_set_object_terms( $product_variable_1['id'], $terms, 'product_visibility' ); + + $filtered_product_ids = $this->do_product_request( array() ); + + $this->assertEquals( array( $product_simple_2->get_id(), $product_variable_2['id'] ), $filtered_product_ids ); + + $this->assert_counters( 'Color', $expected_colors_included_in_counters ); + $this->assert_counters( 'Features', array( 'Ironable' ) ); + } + + /** + * Data provider for the test_filtering_variable_product_for_variation_defining_attributes_by_multiple_attributes tests. + * + * @return array[] + */ + public function data_provider_for_test_filtering_variable_product_for_variation_defining_attributes_by_multiple_attributes() { + return array( + array( array(), array(), true ), + array( array( 'Blue' ), array(), true ), + array( array(), array( 'Medium' ), true ), + array( array( 'Blue' ), array( 'Medium' ), true ), + array( array( 'Red' ), array(), false ), + array( array(), array( 'Large' ), false ), + array( array( 'Blue' ), array( 'Large' ), false ), + array( array( 'Red' ), array( 'Medium' ), false ), + ); + } + + /** + * @testdox The product query shows a variable product only if it's not filtered out by the specified attribute filters (when filtering by multiple attributes), using the lookup table. + * + * Worth noting that multiple attributes are always combined in an AND fashion for filtering. + * + * @dataProvider data_provider_for_test_filtering_variable_product_for_variation_defining_attributes_by_multiple_attributes + * + * @param array $attributes_1 The color attribute names that will be included in the query. + * @param array $attributes_2 The size attribute names that will be included in the query. + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_variable_product_for_variation_defining_attributes_by_multiple_attributes_using_lookup_table( $attributes_1, $attributes_2, $expected_to_be_visible ) { + $this->set_use_lookup_table( true ); + $this->base_test_filtering_variable_product_for_variation_defining_attributes_by_multiple_attributes( $attributes_1, $attributes_2, $expected_to_be_visible ); + } + + /** + * @testdox The product query shows a variable product only if it's not filtered out by the specified attribute filters (when filtering by multiple attributes), not using the lookup table. + * + * Worth noting that multiple attributes are always combined in an AND fashion for filtering. + * + * @dataProvider data_provider_for_test_filtering_variable_product_for_variation_defining_attributes_by_multiple_attributes + * + * @param array $attributes_1 The color attribute names that will be included in the query. + * @param array $attributes_2 The size attribute names that will be included in the query. + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + public function test_filtering_variable_product_for_variation_defining_attributes_by_multiple_attributes_not_using_lookup_table( $attributes_1, $attributes_2, $expected_to_be_visible ) { + $this->set_use_lookup_table( false ); + $this->base_test_filtering_variable_product_for_variation_defining_attributes_by_multiple_attributes( $attributes_1, $attributes_2, $expected_to_be_visible ); + } + + /** + * Main code for the test_filtering_variable_product_for_variation_defining_attributes_by_multiple_attributes tests. + * + * @param array $attributes_1 The color attribute names that will be included in the query. + * @param array $attributes_2 The size attribute names that will be included in the query. + * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + */ + private function base_test_filtering_variable_product_for_variation_defining_attributes_by_multiple_attributes( $attributes_1, $attributes_2, $expected_to_be_visible ) { + $this->set_use_lookup_table( false ); + $this->create_product_attribute( 'Color', array( 'Blue', 'Red' ) ); + $this->create_product_attribute( 'Size', array( 'Large', 'Medium' ) ); + + $product = $this->create_variable_product( + array( + 'variation_attributes' => array( + 'Color' => array( 'Blue' ), + 'Size' => array( 'Medium' ), + ), + 'non_variation_attributes' => array(), + 'variations' => array( + array( + 'in_stock' => true, + 'defining_attributes' => array( + 'Color' => 'Blue', + 'Size' => 'Medium', + ), + ), + ), + ) + ); + + $filtered_product_ids = $this->do_product_request( + array( + 'Color' => $attributes_1, + 'Size' => $attributes_2, + ) + ); + + if ( $expected_to_be_visible ) { + $this->assertEquals( array( $product['id'] ), $filtered_product_ids ); + } else { + $this->assertEmpty( $filtered_product_ids ); + } + + $this->assert_counters( 'Color', $expected_to_be_visible ? array( 'Blue' ) : array() ); + $this->assert_counters( 'Size', $expected_to_be_visible ? array( 'Medium' ) : array() ); + } } From c6dff96c0f298baafd2f9155fe331cece95e2340 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Tue, 1 Jun 2021 11:25:57 +0200 Subject: [PATCH 6/8] Undo a wrong change in filtering logic. --- .../ProductAttributesLookup/Filterer.php | 9 ++- .../ProductAttributesLookup/FiltererTest.php | 75 +++++++++++++++---- 2 files changed, 66 insertions(+), 18 deletions(-) diff --git a/src/Internal/ProductAttributesLookup/Filterer.php b/src/Internal/ProductAttributesLookup/Filterer.php index 4146b314469..881b56f7f5b 100644 --- a/src/Internal/ProductAttributesLookup/Filterer.php +++ b/src/Internal/ProductAttributesLookup/Filterer.php @@ -220,8 +220,9 @@ class Filterer { $count = count( $term_ids_to_filter_by ); if ( 0 !== $count ) { + $query['where'] .= ' AND product_or_parent_id IN ('; if ( 'and' === $attributes_to_filter_by[ $taxonomy ]['query_type'] ) { - $query['where'] .= " AND product_or_parent_id IN ( + $query['where'] .= " SELECT product_or_parent_id FROM {$this->lookup_table_name} lt WHERE is_variation_attribute=0 @@ -237,7 +238,11 @@ class Filterer { AND term_id in {$term_ids_to_filter_by_list} )"; } else { - $query['where'] .= $in_stock_clause; + $query['where'] .= " + SELECT product_or_parent_id FROM {$this->lookup_table_name} + WHERE term_id in {$term_ids_to_filter_by_list} + {$in_stock_clause} + )"; } } } diff --git a/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php b/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php index 61e5730f7d9..b1d509897db 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php @@ -478,7 +478,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_simple_product_in_stock_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { $this->set_use_lookup_table( true ); - $this->base_test_filtering_simple_product_in_stock( $attributes, $filter_type, $expected_to_be_visible ); + $this->base_test_filtering_simple_product_in_stock( $attributes, $filter_type, $expected_to_be_visible, true ); } /** @@ -492,7 +492,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_simple_product_in_stock_not_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { $this->set_use_lookup_table( false ); - $this->base_test_filtering_simple_product_in_stock( $attributes, $filter_type, $expected_to_be_visible ); + $this->base_test_filtering_simple_product_in_stock( $attributes, $filter_type, $expected_to_be_visible, false ); } /** @@ -501,8 +501,9 @@ class FiltererTest extends \WC_Unit_Test_Case { * @param array $attributes The color attribute names that will be included in the query. * @param string $filter_type The filtering type, "or" or "and". * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + * @param bool $using_lookup_table Are we using the lookup table?. */ - private function base_test_filtering_simple_product_in_stock( $attributes, $filter_type, $expected_to_be_visible ) { + private function base_test_filtering_simple_product_in_stock( $attributes, $filter_type, $expected_to_be_visible, $using_lookup_table ) { $this->create_product_attribute( 'Color', array( 'Blue', 'Red', 'Green' ) ); $product = $this->create_simple_product( @@ -523,7 +524,17 @@ class FiltererTest extends \WC_Unit_Test_Case { $this->assertEmpty( $filtered_product_ids ); } - $expected_to_be_included_in_count = 'or' === $filter_type || $expected_to_be_visible; + /* + * If a variable product defines an attribute value that isn't used by any variation: + * When using the lookup table: that value is not included in the count. + * When not using the lookup table: the value is included in the count since it is part of the parent product. + */ + if ( $using_lookup_table && 'or' === $filter_type && array( 'Green' ) === $attributes ) { + $expected_to_be_included_in_count = false; + } else { + $expected_to_be_included_in_count = 'or' === $filter_type || $expected_to_be_visible; + } + $this->assert_counters( 'Color', $expected_to_be_included_in_count ? array( 'Blue', 'Red' ) : array(), $filter_type ); } @@ -728,7 +739,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_variable_product_in_stock_for_non_variation_defining_attributes_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { $this->set_use_lookup_table( true ); - $this->base_test_filtering_variable_product_in_stock_for_non_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible ); + $this->base_test_filtering_variable_product_in_stock_for_non_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible, true ); } /** @@ -742,7 +753,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_variable_product_in_stock_for_non_variation_defining_attributes_not_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { $this->set_use_lookup_table( false ); - $this->base_test_filtering_variable_product_in_stock_for_non_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible ); + $this->base_test_filtering_variable_product_in_stock_for_non_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible, false ); } /** @@ -751,8 +762,9 @@ class FiltererTest extends \WC_Unit_Test_Case { * @param array $attributes The feature attribute names that will be included in the query. * @param string $filter_type The filtering type, "or" or "and". * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + * @param bool $using_lookup_table Are we using the lookup table?. */ - private function base_test_filtering_variable_product_in_stock_for_non_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible ) { + private function base_test_filtering_variable_product_in_stock_for_non_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible, $using_lookup_table ) { $this->create_product_attribute( 'Color', array( 'Blue', 'Red' ) ); $this->create_product_attribute( 'Features', array( 'Washable', 'Ironable', 'Elastic' ) ); @@ -789,7 +801,16 @@ class FiltererTest extends \WC_Unit_Test_Case { $this->assertEmpty( $filtered_product_ids ); } - $expected_to_be_included_in_count = 'or' === $filter_type || $expected_to_be_visible; + /* + * If a variable product defines an attribute value that isn't used by any variation: + * When using the lookup table: that value is not included in the count. + * When not using the lookup table: the value is included in the count since it is part of the parent product. + */ + if ( $using_lookup_table && 'or' === $filter_type && array( 'Elastic' ) === $attributes ) { + $expected_to_be_included_in_count = false; + } else { + $expected_to_be_included_in_count = 'or' === $filter_type || $expected_to_be_visible; + } $this->assert_counters( 'Features', $expected_to_be_included_in_count ? array( 'Washable', 'Ironable' ) : array(), $filter_type ); } @@ -955,7 +976,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_variable_product_in_stock_for_variation_defining_attributes_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { $this->set_use_lookup_table( true ); - $this->base_test_filtering_variable_product_in_stock_for_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible ); + $this->base_test_filtering_variable_product_in_stock_for_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible, true ); } /** @@ -988,7 +1009,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_variable_product_in_stock_for_variation_defining_attributes_not_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { $this->set_use_lookup_table( false ); - $this->base_test_filtering_variable_product_in_stock_for_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible ); + $this->base_test_filtering_variable_product_in_stock_for_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible, false ); } /** @@ -997,8 +1018,9 @@ class FiltererTest extends \WC_Unit_Test_Case { * @param array $attributes The color attribute names that will be included in the query. * @param string $filter_type The filtering type, "or" or "and". * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + * @param bool $using_lookup_table Are we using the lookup table?. */ - private function base_test_filtering_variable_product_in_stock_for_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible ) { + private function base_test_filtering_variable_product_in_stock_for_variation_defining_attributes( $attributes, $filter_type, $expected_to_be_visible, $using_lookup_table ) { $this->create_product_attribute( 'Color', array( 'Blue', 'Red', 'Green' ) ); $product = $this->create_variable_product( @@ -1032,7 +1054,17 @@ class FiltererTest extends \WC_Unit_Test_Case { $this->assertEmpty( $filtered_product_ids ); } - $expected_counted_attributes = 'or' === $filter_type || $expected_to_be_visible ? array( 'Blue', 'Red' ) : array(); + /* + * If a variable product defines an attribute value that isn't used by any variation: + * When using the lookup table: that value is not included in the count. + * When not using the lookup table: the value is included in the count since it is part of the parent product. + */ + if ( $using_lookup_table && 'or' === $filter_type && array( 'Green' ) === $attributes ) { + $expected_counted_attributes = array(); + } else { + $expected_counted_attributes = 'or' === $filter_type || $expected_to_be_visible ? array( 'Blue', 'Red' ) : array(); + } + $this->assert_counters( 'Color', $expected_counted_attributes, $filter_type ); } @@ -1085,7 +1117,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { $this->set_use_lookup_table( true ); - $this->base_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value( $attributes, $filter_type, $expected_to_be_visible ); + $this->base_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value( $attributes, $filter_type, $expected_to_be_visible, true ); } /** @@ -1116,7 +1148,7 @@ class FiltererTest extends \WC_Unit_Test_Case { */ public function test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value_not_using_lookup_table( $attributes, $filter_type, $expected_to_be_visible ) { $this->set_use_lookup_table( false ); - $this->base_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value( $attributes, $filter_type, $expected_to_be_visible ); + $this->base_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value( $attributes, $filter_type, $expected_to_be_visible, false ); } /** @@ -1125,8 +1157,9 @@ class FiltererTest extends \WC_Unit_Test_Case { * @param array $attributes The color attribute names that will be included in the query. * @param string $filter_type The filtering type, "or" or "and". * @param bool $expected_to_be_visible True if the product is expected to be returned by the query, false otherwise. + * @param bool $using_lookup_table Are we using the lookup table?. */ - private function base_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value( $attributes, $filter_type, $expected_to_be_visible ) { + private function base_test_filtering_variable_product_in_stock_for_variation_defining_attributes_with_any_value( $attributes, $filter_type, $expected_to_be_visible, $using_lookup_table ) { $this->create_product_attribute( 'Color', array( 'Blue', 'Red', 'Green', 'White' ) ); $product = $this->create_variable_product( @@ -1154,7 +1187,17 @@ class FiltererTest extends \WC_Unit_Test_Case { $this->assertEmpty( $filtered_product_ids ); } - $expected_to_be_included_in_count = 'or' === $filter_type || $expected_to_be_visible; + /* + * If a variable product defines an attribute value that isn't used by any variation: + * When using the lookup table: that value is not included in the count. + * When not using the lookup table: the value is included in the count since it is part of the parent product. + */ + if ( $using_lookup_table && 'or' === $filter_type && array( 'White' ) === $attributes ) { + $expected_to_be_included_in_count = false; + } else { + $expected_to_be_included_in_count = 'or' === $filter_type || $expected_to_be_visible; + } + $this->assert_counters( 'Color', $expected_to_be_included_in_count ? array( 'Blue', 'Red', 'Green' ) : array(), $filter_type ); } From 0b1158cf5cf4b78faeebdf619294dd4c5c7d3758 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Tue, 8 Jun 2021 10:18:35 +0200 Subject: [PATCH 7/8] Small improvements in the filtering by attribute lookup table. - Combined two 'if's in one - Added extra santitization of term ids in the Filterer class --- includes/class-wc-query.php | 20 +++++++++---------- .../ProductAttributesLookup/Filterer.php | 6 ++++++ .../ProductAttributesLookup/FiltererTest.php | 18 ++++++++--------- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/includes/class-wc-query.php b/includes/class-wc-query.php index 0f2375c6aa5..782eba92ea5 100644 --- a/includes/class-wc-query.php +++ b/includes/class-wc-query.php @@ -766,18 +766,16 @@ class WC_Query { ); } - if ( ! $this->filterer->filtering_via_lookup_table_is_active() ) { + if ( $main_query && ! $this->filterer->filtering_via_lookup_table_is_active() ) { // Layered nav filters on terms. - if ( $main_query ) { - foreach ( $this->get_layered_nav_chosen_attributes() as $taxonomy => $data ) { - $tax_query[] = array( - 'taxonomy' => $taxonomy, - 'field' => 'slug', - 'terms' => $data['terms'], - 'operator' => 'and' === $data['query_type'] ? 'AND' : 'IN', - 'include_children' => false, - ); - } + foreach ( $this->get_layered_nav_chosen_attributes() as $taxonomy => $data ) { + $tax_query[] = array( + 'taxonomy' => $taxonomy, + 'field' => 'slug', + 'terms' => $data['terms'], + 'operator' => 'and' === $data['query_type'] ? 'AND' : 'IN', + 'include_children' => false, + ); } } diff --git a/src/Internal/ProductAttributesLookup/Filterer.php b/src/Internal/ProductAttributesLookup/Filterer.php index 881b56f7f5b..ed34b97f111 100644 --- a/src/Internal/ProductAttributesLookup/Filterer.php +++ b/src/Internal/ProductAttributesLookup/Filterer.php @@ -74,6 +74,12 @@ class Filterer { $all_terms = get_terms( $taxonomy, array( 'hide_empty' => false ) ); $term_ids_by_slug = wp_list_pluck( $all_terms, 'term_id', 'slug' ); $term_ids_to_filter_by = array_values( array_intersect_key( $term_ids_by_slug, array_flip( $data['terms'] ) ) ); + $term_ids_to_filter_by = array_map( + function( $id ) { + return (int) $id; + }, + $term_ids_to_filter_by + ); $term_ids_to_filter_by_list = '(' . join( ',', $term_ids_to_filter_by ) . ')'; $is_and_query = 'and' === $data['query_type']; diff --git a/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php b/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php index b1d509897db..e5a99eed25e 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php @@ -21,15 +21,15 @@ class FiltererTest extends \WC_Unit_Test_Case { $wpdb->query( " - CREATE TABLE IF NOT EXISTS {$wpdb->prefix}wc_product_attributes_lookup ( - product_id bigint(20) NOT NULL, - product_or_parent_id bigint(20) NOT NULL, - taxonomy varchar(32) NOT NULL, - term_id bigint(20) NOT NULL, - is_variation_attribute tinyint(1) NOT NULL, - in_stock tinyint(1) NOT NULL - ); - " + CREATE TABLE IF NOT EXISTS {$wpdb->prefix}wc_product_attributes_lookup ( + product_id bigint(20) NOT NULL, + product_or_parent_id bigint(20) NOT NULL, + taxonomy varchar(32) NOT NULL, + term_id bigint(20) NOT NULL, + is_variation_attribute tinyint(1) NOT NULL, + in_stock tinyint(1) NOT NULL + ); + " ); $wpdb->query( "TRUNCATE TABLE {$wpdb->prefix}wc_product_attributes_lookup" ); From ef9145de86a7a95315770d74e7721d6b5be33784 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Tue, 8 Jun 2021 15:47:04 +0200 Subject: [PATCH 8/8] Small improvements in the filtering by attribute lookup table. - Use 'absint' instead of an '(int)' in an anonymous function. --- src/Internal/ProductAttributesLookup/Filterer.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Internal/ProductAttributesLookup/Filterer.php b/src/Internal/ProductAttributesLookup/Filterer.php index ed34b97f111..a42e3d804fe 100644 --- a/src/Internal/ProductAttributesLookup/Filterer.php +++ b/src/Internal/ProductAttributesLookup/Filterer.php @@ -74,12 +74,7 @@ class Filterer { $all_terms = get_terms( $taxonomy, array( 'hide_empty' => false ) ); $term_ids_by_slug = wp_list_pluck( $all_terms, 'term_id', 'slug' ); $term_ids_to_filter_by = array_values( array_intersect_key( $term_ids_by_slug, array_flip( $data['terms'] ) ) ); - $term_ids_to_filter_by = array_map( - function( $id ) { - return (int) $id; - }, - $term_ids_to_filter_by - ); + $term_ids_to_filter_by = array_map( 'absint', $term_ids_to_filter_by ); $term_ids_to_filter_by_list = '(' . join( ',', $term_ids_to_filter_by ) . ')'; $is_and_query = 'and' === $data['query_type'];