From 32cce6032d9d9db6e88b28b234ad9aeca4650023 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Thu, 13 May 2021 12:29:23 +0200 Subject: [PATCH 1/9] 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/9] 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/9] 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/9] 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/9] 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/9] 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/9] 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/9] 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']; From dfca2e61352e36c78f7cfd38f531686bfa0a2f85 Mon Sep 17 00:00:00 2001 From: Christopher Allford <6451942+ObliviousHarmony@users.noreply.github.com> Date: Tue, 8 Jun 2021 12:17:07 -0700 Subject: [PATCH 9/9] Added 5.4 Changelog Content --- changelog.txt | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/changelog.txt b/changelog.txt index dc87ea85f2e..bb11c160d54 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,5 +1,91 @@ == Changelog == += 5.4.0 2021-06-08 = + +**WooCommerce** + +* Localization - Added Venezuelan states. #29477 +* Add - Feature WooCommerce Payments in the extensions store for stores in the US, Canada, UK, Ireland, Australia and New Zealand. #29843 +* Add - Product attributes lookup table and debug tools to manually fill or delete it. #29778 +* Add - dates_are_gmt parameters in REST API to searched posts using the post_date_gmt column. +* Add - labels and searching terms in WooCommerce Navigation link block variations. #29772 +* Enhancement - Improved accessibility of the country and state address fields. #29706 +* Tweak - Updates the date_query usage in the CRUD controller to be consistent, generating an array of queries. #29909 +* Tweak - Search all extension categories instead of just searching the selected category. #29694 +* Tweak - Use WC Admin's native notice nonce generation. #29637 +* Tweak - Change email settings help text to include troubleshooting steps. #29599 +* Fix - Bulk edit on external products causes an error when changing the Backorders setting. #29766 +* Fix - wc_get_low_stock_amount was returning a string, not an integer, for products not having an explicit low stock value set. #29721 +* Fix - Products left without default category assignment when all categories are deleted. #29681 +* Fix - Removed rounding at several places to better support precision when prices are entered more than 2dp. #29318 +* Fix - Migrate deprecated jQuery 3 functions. #29044 +* Dev - Refactored Tracker to use direct DB calls instead of CRUD. #29877 +* Dev - Introduce an option for assignment of variations for Remote Inbox Notification A/B testing. #29894 + +**WooCommerce Admin - 2.3.0 & 2.3.1** + +* Add - Add plugin installer to allow installation of plugins via URL #6805 +* Add - Optional children prop to SummaryNumber component #6748 +* Dev - Add data source filter to remote inbox notification system #6794 +* Dev - Add A/A test #6669 +* Dev - Add support for nonces in note actions #6726 +* Dev - Add support for running php unit tests in PHP 8. #6678 +* Dev - Add event recording to start of gateway connections #6801 +* Dev - Do a git clean before the core release. #6945 +* Dev - Fix a bug where trying to load an asset registry causes a crash. #6951 +* Feature - Add recommended payment methods in payment settings. #6760 +* Fix - Disable the continue btn on OBW when requested are being made #6838 +* Fix - Event tracking for merchant email notes #6616 +* Fix - Use the store timezone to make time data requests #6632 +* Fix - Update the checked input radio button margin style #6701 +* Fix - Convert date to timestamp before passing to set_date_prop to persist timezone #6795 +* Fix - Make pagination buttons height and width consistent #6725 +* Fix - Retain persisted queries when navigating to Homescreen #6614 +* Fix - Update folded header style #6724 +* Fix - Unreleated variations showing up in the Products reports #6647 +* Fix - Check active plugins before getting the PayPal onboarding status #6625 +* Fix - Remove no-reply from inbox notification emails #6644 +* Fix - Set up shipping costs task, redirect to shipping settings after completion. #6791 +* Fix - Onboarding logic on WooCommerce update to keep task list present. #6803 +* Fix - Pause inbox message “GivingFeedbackNotes” #6802 +* Fix - Missed DB version number updates causing unnecessary upgrades. #6818 +* Fix - Parsing bad JSON string data from user WooCommerce meta. #6819 +* Fix - Remove PayPal for India #6828 +* Fix - Address an issue with OBW when installing only WooCommerce payments and Jetpack. #6957 +* Fix - Calling of get_script_asset_filename with extra parameter #6955 +* Fix - Show Google Listing and Ads in installed marketing extensions section. #7029 +* Performance - Avoid updating customer info synchronously from the front end. #6765 +* Tweak - Add settings_section event prop for CES #6762 +* Tweak - Refactor payments to allow management of methods #6786 +* Tweak - Add tracking data for the preview site button #6623 +* Tweak - Update WC Payments copy on the task list #6734 +* Tweak - Add check to see if value for contains is array, show warning if not. #6645 +* Tweak - Sort the extension task list by completion status and allow toggling visibility. #6792 +* Tweak - Update PayU logo #6829 +* Tweak - Store profiler - Changed MailPoet's title and description #6886 +* Tweak - Store profiler - Changed MailPoet's title and description #6990 +* Tweak - Adjust WC Pay supported countries #7048 +* Update - Replace marketing extension - Google Listings and Ads. #6939 +* Update - Update choose niche note cta URL #6733 +* Update - UI updates to Payment Task screen #6766 +* Update - Adding setup required icon for non-configured payment methods #6811 +* Update - Payment recommendation screen transition and add external link icon. #7022 + +**WooCommerce Blocks Package - 5.1.0** + +* Add - Introduced AssetsController and BlockTypesController classes (which replace Assets.php and Library.php). #4094 +* Tweak - Replaced usage of the `woocommerce_shared_settings` hook. This will be deprecated. #4092 + +**WooCommerce Blocks Feature Plugin - 5.1.0** + +* Add - Added support to the Store API for batching requests. This allows multiple POST requests to be made at once to reduce the number of separate requests being made to the API. #4075 +* Tweak - Improve error message displayed when a payment method didn't have all its dependencies registered. #4176 +* Tweak - Improvements to `emitEventWithAbort`. #4158 +* Tweak - Rename onCheckoutBeforeProcessing to onCheckoutValidationBeforeProcessing. +* Tweak - Switched to `rest_preload_api_request` for API hydration in cart and checkout blocks. #4090 +* Fix - Prevent parts of old addresses being displayed in the shipping calculator when changing countries. #4038 +* Fix - issue in which email and phone fields are cleared when using a separate billing address. #4162 + = 5.3.0 2021-05-11 = **WooCommerce**