From 22df8cabb645b77aff13270c794004d8508851a0 Mon Sep 17 00:00:00 2001 From: James Allan Date: Fri, 16 Oct 2020 11:50:28 +1000 Subject: [PATCH 01/25] Update the rate when recalculating and updating order tax items --- includes/abstracts/abstract-wc-order.php | 1 + 1 file changed, 1 insertion(+) diff --git a/includes/abstracts/abstract-wc-order.php b/includes/abstracts/abstract-wc-order.php index a694d1aa151..57e38100094 100644 --- a/includes/abstracts/abstract-wc-order.php +++ b/includes/abstracts/abstract-wc-order.php @@ -1610,6 +1610,7 @@ abstract class WC_Abstract_Order extends WC_Abstract_Legacy_Order { continue; } $saved_rate_ids[] = $tax->get_rate_id(); + $tax->set_rate( $tax->get_rate_id() ); $tax->set_tax_total( isset( $cart_taxes[ $tax->get_rate_id() ] ) ? $cart_taxes[ $tax->get_rate_id() ] : 0 ); $tax->set_shipping_tax_total( ! empty( $shipping_taxes[ $tax->get_rate_id() ] ) ? $shipping_taxes[ $tax->get_rate_id() ] : 0 ); $tax->save(); From f237642af4ebcc247dbb38efeceac1700c59d182 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Fri, 4 Jun 2021 12:21:29 +0200 Subject: [PATCH 02/25] Refactor save method in WC_Product and WC_Product_Variable The 'save' method in WC_Product_Variable was almost identical to the parent method in WC_Product, only adding a few bits of extra processing before and after the actual datastore save. This commit adds a couple of protected methods, before_data_store_save_or_update and after_data_store_save_or_update, and implements them in WC_Product_Variable so that the extra 'save' method in WC_Product_Variable can be removed. --- includes/abstracts/abstract-wc-product.php | 23 +++++++++- includes/class-wc-product-variable.php | 50 ++++++++-------------- 2 files changed, 39 insertions(+), 34 deletions(-) diff --git a/includes/abstracts/abstract-wc-product.php b/includes/abstracts/abstract-wc-product.php index ccecf15e367..bcb853d28b3 100644 --- a/includes/abstracts/abstract-wc-product.php +++ b/includes/abstracts/abstract-wc-product.php @@ -1374,13 +1374,15 @@ class WC_Product extends WC_Abstract_Legacy_Product { */ do_action( 'woocommerce_before_' . $this->object_type . '_object_save', $this, $this->data_store ); + $state = $this->before_data_store_save_or_update(); + if ( $this->get_id() ) { $this->data_store->update( $this ); } else { $this->data_store->create( $this ); } - $this->maybe_defer_product_sync(); + $this->after_data_store_save_or_update( $state ); /** * Trigger action after saving to the DB. @@ -1393,6 +1395,25 @@ class WC_Product extends WC_Abstract_Legacy_Product { return $this->get_id(); } + /** + * Do any extra processing needed before the actual product save + * (but after triggering the 'woocommerce_before_..._object_save' action) + * + * @return mixed A state value that will be passed to after_data_store_save_or_update. + */ + protected function before_data_store_save_or_update() { + } + + /** + * Do any extra processing needed after the actual product save + * (but before triggering the 'woocommerce_after_..._object_save' action) + * + * @param mixed $state The state object that was returned by before_data_store_save_or_update. + */ + protected function after_data_store_save_or_update( $state ) { + $this->maybe_defer_product_sync(); + } + /** * Delete the product, set its ID to 0, and return result. * diff --git a/includes/class-wc-product-variable.php b/includes/class-wc-product-variable.php index 78900dd9fc1..a240c406a08 100644 --- a/includes/class-wc-product-variable.php +++ b/includes/class-wc-product-variable.php @@ -449,47 +449,31 @@ class WC_Product_Variable extends WC_Product { } /** - * Save data (either create or update depending on if we are working on an existing product). + * Do any extra processing needed before the actual product save + * (but after triggering the 'woocommerce_before_..._object_save' action) * - * @since 3.0.0 + * @return mixed A state value that will be passed to after_data_store_save_or_update. */ - public function save() { - $this->validate_props(); - - if ( ! $this->data_store ) { - return $this->get_id(); - } - - /** - * Trigger action before saving to the DB. Allows you to adjust object props before save. - * - * @param WC_Data $this The object being saved. - * @param WC_Data_Store_WP $data_store The data store persisting the data. - */ - do_action( 'woocommerce_before_' . $this->object_type . '_object_save', $this, $this->data_store ); - + protected function before_data_store_save_or_update() { // Get names before save. $previous_name = $this->data['name']; $new_name = $this->get_name( 'edit' ); - if ( $this->get_id() ) { - $this->data_store->update( $this ); - } else { - $this->data_store->create( $this ); - } + return array( + 'previous_name' => $previous_name, + 'new_name' => $new_name, + ); + } - $this->data_store->sync_variation_names( $this, $previous_name, $new_name ); + /** + * Do any extra processing needed after the actual product save + * (but before triggering the 'woocommerce_after_..._object_save' action) + * + * @param mixed $state The state object that was returned by before_data_store_save_or_update. + */ + protected function after_data_store_save_or_update( $state ) { + $this->data_store->sync_variation_names( $this, $state['previous_name'], $state['new_name'] ); $this->data_store->sync_managed_variation_stock_status( $this ); - - /** - * Trigger action after saving to the DB. - * - * @param WC_Data $this The object being saved. - * @param WC_Data_Store_WP $data_store The data store persisting the data. - */ - do_action( 'woocommerce_after_' . $this->object_type . '_object_save', $this, $this->data_store ); - - return $this->get_id(); } /* From 7f86d1988dc323df0b472dcc7e0424cbefc8c79b Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Mon, 7 Jun 2021 16:52:22 +0200 Subject: [PATCH 03/25] Remove superfluous 'delete' method in WC_Product_Variation class. --- includes/class-wc-product-variation.php | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/includes/class-wc-product-variation.php b/includes/class-wc-product-variation.php index ebd2838b46b..83b80fc206c 100644 --- a/includes/class-wc-product-variation.php +++ b/includes/class-wc-product-variation.php @@ -583,21 +583,4 @@ class WC_Product_Variation extends WC_Product_Simple { return $valid_classes; } - - /** - * Delete variation, set the ID to 0, and return result. - * - * @since 4.4.0 - * @param bool $force_delete Should the variation be deleted permanently. - * @return bool result - */ - public function delete( $force_delete = false ) { - $variation_id = $this->get_id(); - - if ( ! parent::delete( $force_delete ) ) { - return false; - } - - return true; - } } From e54ff461f9b56f0e415a573e1a9aeba3f283bb90 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Mon, 7 Jun 2021 16:54:08 +0200 Subject: [PATCH 04/25] Implement the product attributes lookup table data deletion. Lookup entries for a product or a variation are deleted whenever the product is deleted or trashed. --- includes/abstracts/abstract-wc-product.php | 6 +- includes/class-wc-post-data.php | 11 +++- .../LookupDataStore.php | 61 ++++++++++++++++++- 3 files changed, 73 insertions(+), 5 deletions(-) diff --git a/includes/abstracts/abstract-wc-product.php b/includes/abstracts/abstract-wc-product.php index bcb853d28b3..0136ccda54c 100644 --- a/includes/abstracts/abstract-wc-product.php +++ b/includes/abstracts/abstract-wc-product.php @@ -9,6 +9,8 @@ if ( ! defined( 'ABSPATH' ) ) { exit; } +use Automattic\WooCommerce\Internal\ProductAttributesLookup\LookupDataStore as ProductAttributesLookupDataStore; + /** * Legacy product contains all deprecated methods for this class and can be * removed in the future. @@ -1421,10 +1423,12 @@ class WC_Product extends WC_Abstract_Legacy_Product { * @return bool result */ public function delete( $force_delete = false ) { - $deleted = parent::delete( $force_delete ); + $product_id = $this->get_id(); + $deleted = parent::delete( $force_delete ); if ( $deleted ) { $this->maybe_defer_product_sync(); + wc_get_container()->get( ProductAttributesLookupDataStore::class )->on_product_deleted( $product_id ); } return $deleted; diff --git a/includes/class-wc-post-data.php b/includes/class-wc-post-data.php index 5ea84ff351f..83f241b9809 100644 --- a/includes/class-wc-post-data.php +++ b/includes/class-wc-post-data.php @@ -8,6 +8,8 @@ * @version 2.2.0 */ +use Automattic\WooCommerce\Internal\ProductAttributesLookup\LookupDataStore as ProductAttributesLookupDataStore; + defined( 'ABSPATH' ) || exit; /** @@ -307,16 +309,20 @@ class WC_Post_Data { $data_store = WC_Data_Store::load( 'product-variable' ); $data_store->delete_variations( $id, true ); $data_store->delete_from_lookup_table( $id, 'wc_product_meta_lookup' ); - $parent_id = wp_get_post_parent_id( $id ); + wc_get_container()->get( ProductAttributesLookupDataStore::class )->on_product_deleted( $id ); + $parent_id = wp_get_post_parent_id( $id ); if ( $parent_id ) { wc_delete_product_transients( $parent_id ); } + break; case 'product_variation': $data_store = WC_Data_Store::load( 'product' ); $data_store->delete_from_lookup_table( $id, 'wc_product_meta_lookup' ); wc_delete_product_transients( wp_get_post_parent_id( $id ) ); + wc_get_container()->get( ProductAttributesLookupDataStore::class )->on_product_deleted( $id ); + break; case 'shop_order': global $wpdb; @@ -360,6 +366,9 @@ class WC_Post_Data { } elseif ( 'product' === $post_type ) { $data_store = WC_Data_Store::load( 'product-variable' ); $data_store->delete_variations( $id, false ); + wc_get_container()->get( ProductAttributesLookupDataStore::class )->on_product_deleted( $id ); + } elseif ( 'product_variation' === $post_type ) { + wc_get_container()->get( ProductAttributesLookupDataStore::class )->on_product_deleted( $id ); } } diff --git a/src/Internal/ProductAttributesLookup/LookupDataStore.php b/src/Internal/ProductAttributesLookup/LookupDataStore.php index 77cc2845238..68d6fe6f568 100644 --- a/src/Internal/ProductAttributesLookup/LookupDataStore.php +++ b/src/Internal/ProductAttributesLookup/LookupDataStore.php @@ -28,6 +28,15 @@ class LookupDataStore { */ private $is_feature_visible; + /** + * Does the lookup table exist? + * + * TODO: Remove once the lookup table is created via data migration. + * + * @var bool + */ + private $lookup_table_exists; + /** * LookupDataStore constructor. Makes the feature hidden by default. */ @@ -36,6 +45,30 @@ class LookupDataStore { $this->lookup_table_name = $wpdb->prefix . 'wc_product_attributes_lookup'; $this->is_feature_visible = false; + + $this->lookup_table_exists = $this->check_lookup_table_exists(); + } + + /** + * Check if the lookup table exists in the database. + * + * TODO: Remove this method and references to it once the lookup table is created via data migration. + * + * @return bool + */ + public function check_lookup_table_exists() { + global $wpdb; + + $query = $wpdb->prepare( + 'SELECT count(*) +FROM information_schema.tables +WHERE table_schema = DATABASE() +AND table_name = %s;', + $this->lookup_table_name + ); + + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + return 0 !== $wpdb->get_var( $query ); } /** @@ -61,6 +94,27 @@ class LookupDataStore { $this->is_feature_visible = false; } + /** + * Delete the lookup table contents related to a given product or variation, + * if it's a variable product it deletes the information for variations too. + * This must be invoked after a product or a variation is trashed or deleted. + * + * @param int|\WC_Product $product Product object or product id. + */ + public function on_product_deleted( $product ) { + if ( ! $this->lookup_table_exists ) { + return; + } + + if ( is_a( $product, \WC_Product::class ) ) { + $product_id = $product->get_id(); + } else { + $product_id = $product; + } + + $this->delete_lookup_table_entries_for( $product_id ); + } + /** * 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. @@ -89,8 +143,8 @@ class LookupDataStore { } /** - * Delete all the lookup table entries for a given product - * (entries are identified by the "parent_or_product_id" field) + * Delete all the lookup table entries for a given product, + * if it's a variable product information for variations is deleted too. * * @param int $product_id Simple product id, or main/parent product id for variable products. */ @@ -100,7 +154,8 @@ class LookupDataStore { // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared $wpdb->query( $wpdb->prepare( - 'DELETE FROM ' . $this->lookup_table_name . ' WHERE product_or_parent_id = %d', + 'DELETE FROM ' . $this->lookup_table_name . ' WHERE product_id = %d OR product_or_parent_id = %d', + $product_id, $product_id ) ); From 039f81ea50497cb457d8e53a9546896930be3a1e Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Tue, 8 Jun 2021 17:53:45 +0200 Subject: [PATCH 05/25] Implement the product attributes lookup table data creation. Lookup entries for a product or a variation are created whenever the product is created, including untrashing and duplication (not yet when an existing product is modified). --- includes/abstracts/abstract-wc-product.php | 7 ++ includes/class-wc-post-data.php | 4 + .../DataRegenerator.php | 2 +- .../LookupDataStore.php | 101 +++++++++++++++--- .../DataRegeneratorTest.php | 2 +- .../LookupDataStoreTest.php | 16 +-- 6 files changed, 105 insertions(+), 27 deletions(-) diff --git a/includes/abstracts/abstract-wc-product.php b/includes/abstracts/abstract-wc-product.php index 0136ccda54c..5f43b8780a0 100644 --- a/includes/abstracts/abstract-wc-product.php +++ b/includes/abstracts/abstract-wc-product.php @@ -1379,13 +1379,20 @@ class WC_Product extends WC_Abstract_Legacy_Product { $state = $this->before_data_store_save_or_update(); if ( $this->get_id() ) { + $changeset = $this->get_changes(); $this->data_store->update( $this ); } else { + $changeset = null; $this->data_store->create( $this ); } $this->after_data_store_save_or_update( $state ); + // Update attributes lookup table if the product is new OR it's not but there are actually any changes. + if ( is_null( $changeset ) || ! empty( $changeset ) ) { + wc_get_container()->get( ProductAttributesLookupDataStore::class )->on_product_changed( $this, $changeset ); + } + /** * Trigger action after saving to the DB. * diff --git a/includes/class-wc-post-data.php b/includes/class-wc-post-data.php index 83f241b9809..94b6c3d0843 100644 --- a/includes/class-wc-post-data.php +++ b/includes/class-wc-post-data.php @@ -400,6 +400,10 @@ class WC_Post_Data { $data_store->untrash_variations( $id ); wc_product_force_unique_sku( $id ); + + wc_get_container()->get( ProductAttributesLookupDataStore::class )->on_product_changed( $id ); + } elseif ( 'product_variation' === $post_type ) { + wc_get_container()->get( ProductAttributesLookupDataStore::class )->on_product_changed( $id ); } } diff --git a/src/Internal/ProductAttributesLookup/DataRegenerator.php b/src/Internal/ProductAttributesLookup/DataRegenerator.php index e7a60bffd35..8068abffb7e 100644 --- a/src/Internal/ProductAttributesLookup/DataRegenerator.php +++ b/src/Internal/ProductAttributesLookup/DataRegenerator.php @@ -233,7 +233,7 @@ CREATE TABLE ' . $this->lookup_table_name . '( } foreach ( $product_ids as $id ) { - $this->data_store->update_data_for_product( $id ); + $this->data_store->insert_data_for_product( $id ); } update_option( 'woocommerce_attribute_lookup__last_products_page_processed', $current_products_page ); diff --git a/src/Internal/ProductAttributesLookup/LookupDataStore.php b/src/Internal/ProductAttributesLookup/LookupDataStore.php index 68d6fe6f568..41d8969d028 100644 --- a/src/Internal/ProductAttributesLookup/LookupDataStore.php +++ b/src/Internal/ProductAttributesLookup/LookupDataStore.php @@ -94,6 +94,39 @@ AND table_name = %s;', $this->is_feature_visible = false; } + /** + * Insert/update the appropriate lookup table entries for a new or modified product or variation. + * This must be invoked after a product or a variation is created (including untrashing and duplication) + * or modified. + * + * @param int|\WC_Product $product Product object or product id. + * @param null|array $changeset Changes as provided by 'get_changes' method in the product object, null if it's being created. + */ + public function on_product_changed( $product, $changeset = null ) { + if ( ! $this->lookup_table_exists ) { + return; + } + + if ( ! is_a( $product, \WC_Product::class ) ) { + $product = WC()->call_function( 'wc_get_product', $product ); + } + + // No changeset available: the product/variation is new, so just insert its data. + + if ( is_null( $changeset ) ) { + $this->delete_lookup_table_entries_for( $product->get_id() ); + if ( $this->is_variation( $product ) ) { + $this->insert_data_for_variation( $product ); + } else { + $this->insert_data_for_product( $product ); + } + } + + // Changeset available: process it. + + // TODO: Implement changeset processing. + } + /** * Delete the lookup table contents related to a given product or variation, * if it's a variable product it deletes the information for variations too. @@ -116,21 +149,19 @@ AND table_name = %s;', } /** - * 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. + * Insert the lookup data for a given product or variation. + * If a variable product is passed the information is created for all of its variations. * * @param int|WC_Product $product Product object or id. * @throws \Exception A variation object is passed. */ - public function update_data_for_product( $product ) { - // TODO: For now data is always deleted and fully regenerated, existing data should be updated instead. - + public function insert_data_for_product( $product ) { if ( ! is_a( $product, \WC_Product::class ) ) { $product = WC()->call_function( 'wc_get_product', $product ); } if ( $this->is_variation( $product ) ) { - throw new \Exception( "LookupDataStore::update_data_for_product can't be called for variations." ); + throw new \Exception( "LookupDataStore::insert_data_for_product can't be called for variations." ); } $this->delete_lookup_table_entries_for( $product->get_id() ); @@ -216,17 +247,53 @@ AND table_name = %s;', foreach ( $variation_attributes_data as $taxonomy => $data ) { foreach ( $variations as $variation ) { - $variation_id = $variation->get_id(); - $variation_has_stock = $variation->is_in_stock(); - $variation_definition_term_id = $this->get_variation_definition_term_id( $variation, $taxonomy, $term_ids_by_slug_cache ); - if ( $variation_definition_term_id ) { - $this->insert_lookup_table_data( $variation_id, $main_product_id, $taxonomy, $variation_definition_term_id, true, $variation_has_stock ); - } else { - $term_ids_for_taxonomy = $data['term_ids']; - foreach ( $term_ids_for_taxonomy as $term_id ) { - $this->insert_lookup_table_data( $variation_id, $main_product_id, $taxonomy, $term_id, true, $variation_has_stock ); - } - } + $this->insert_lookup_table_data_for_variation( $variation, $taxonomy, $main_product_id, $data['term_ids'], $term_ids_by_slug_cache ); + } + } + } + + /** + * Create all the necessary lookup data for a given variation. + * + * @param \WC_Product_Variation $variation The variation to create entries for. + */ + private function insert_data_for_variation( \WC_Product_Variation $variation ) { + $main_product = wc_get_product( $variation->get_parent_id() ); + + $product_attributes_data = $this->get_attribute_taxonomies( $main_product ); + $variation_attributes_data = array_filter( + $product_attributes_data, + function( $item ) { + return $item['used_for_variations']; + } + ); + + $term_ids_by_slug_cache = $this->get_term_ids_by_slug_cache( array_keys( $variation_attributes_data ) ); + + foreach ( $variation_attributes_data as $taxonomy => $data ) { + $this->insert_lookup_table_data_for_variation( $variation, $taxonomy, $main_product->get_id(), $data['term_ids'], $term_ids_by_slug_cache ); + } + } + + /** + * Create lookup table entries for a given variation, corresponding to a given taxonomy and a set of term ids. + * + * @param \WC_Product_Variation $variation The variation to create entries for. + * @param string $taxonomy The taxonomy to create the entries for. + * @param int $main_product_id The parent product id. + * @param array $term_ids The term ids to create entries for. + * @param array $term_ids_by_slug_cache A dictionary of term ids by term slug, as returned by 'get_term_ids_by_slug_cache'. + */ + private function insert_lookup_table_data_for_variation( \WC_Product_Variation $variation, string $taxonomy, int $main_product_id, array $term_ids, array $term_ids_by_slug_cache ) { + $variation_id = $variation->get_id(); + $variation_has_stock = $variation->is_in_stock(); + $variation_definition_term_id = $this->get_variation_definition_term_id( $variation, $taxonomy, $term_ids_by_slug_cache ); + if ( $variation_definition_term_id ) { + $this->insert_lookup_table_data( $variation_id, $main_product_id, $taxonomy, $variation_definition_term_id, true, $variation_has_stock ); + } else { + $term_ids_for_taxonomy = $term_ids; + foreach ( $term_ids_for_taxonomy as $term_id ) { + $this->insert_lookup_table_data( $variation_id, $main_product_id, $taxonomy, $term_id, true, $variation_has_stock ); } } } diff --git a/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php b/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php index a40f97c42a2..5f8b0df35af 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php @@ -51,7 +51,7 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { $this->lookup_data_store = new class() extends LookupDataStore { public $passed_products = array(); - public function update_data_for_product( $product ) { + public function insert_data_for_product( $product ) { $this->passed_products[] = $product; } }; diff --git a/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php b/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php index 58895eb4746..0f69db06a3b 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php @@ -42,19 +42,19 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { } /** - * @testdox `test_update_data_for_product` throws an exception if a variation is passed. + * @testdox `insert_data_for_product` throws an exception if a variation is passed. */ - public function test_update_data_for_product_throws_if_variation_is_passed() { + public function test_insert_data_for_product_throws_if_variation_is_passed() { $product = new \WC_Product_Variation(); $this->expectException( \Exception::class ); - $this->expectExceptionMessage( "LookupDataStore::update_data_for_product can't be called for variations." ); + $this->expectExceptionMessage( "LookupDataStore::insert_data_for_product can't be called for variations." ); - $this->sut->update_data_for_product( $product ); + $this->sut->insert_data_for_product( $product ); } /** - * @testdox `test_update_data_for_product` creates the appropriate entries for simple products, skipping custom product attributes. + * @testdox `insert_data_for_product` creates the appropriate entries for simple products, skipping custom product attributes. * * @testWith [true] * [false] @@ -90,7 +90,7 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { $expected_in_stock = 0; } - $this->sut->update_data_for_product( $product ); + $this->sut->insert_data_for_product( $product ); $expected = array( array( @@ -133,7 +133,7 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { } /** - * @testdox `test_update_data_for_product` creates the appropriate entries for variable products. + * @testdox `insert_data_for_product` creates the appropriate entries for variable products. */ public function test_update_data_for_variable_product() { $products = array(); @@ -239,7 +239,7 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { $products[1001] = $variation_1; $products[1002] = $variation_2; - $this->sut->update_data_for_product( $product ); + $this->sut->insert_data_for_product( $product ); $expected = array( // Main product: one entry for each of the regular attribute values, From bbddcbcc15949c1febafc3915cffee5d33378e83 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Thu, 10 Jun 2021 17:00:09 +0200 Subject: [PATCH 06/25] Fix in FiltererTest after mrging from trunk. --- .../ProductAttributesLookup/FiltererTest.php | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php b/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php index e5a99eed25e..f0c4626a596 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/FiltererTest.php @@ -74,7 +74,7 @@ class FiltererTest extends \WC_Unit_Test_Case { $child->delete( true ); } else { $child->set_parent_id( 0 ); - $child->save(); + $this->save( $child ); } } @@ -86,6 +86,27 @@ class FiltererTest extends \WC_Unit_Test_Case { \WC_Query::reset_chosen_attributes(); } + /** + * Save a product and delete any lookup table data that may have been automatically inserted + * (for the purposes of unit testing we want to insert this data manually) + * + * @param \WC_Product $product The product to save and delete lookup table data for. + */ + private function save( \WC_Product $product ) { + global $wpdb; + + $product->save(); + + // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared + $wpdb->query( + $wpdb->prepare( + "DELETE FROM {$wpdb->prefix}wc_product_attributes_lookup WHERE product_id = %d", + $product->get_id() + ) + ); + // phpcs:enable WordPress.DB.PreparedSQL.NotPrepared + } + /** * Core function to create a product. * @@ -165,7 +186,7 @@ class FiltererTest extends \WC_Unit_Test_Case { $product->set_stock_status( $in_stock ? 'instock' : 'outofstock' ); - $product->save(); + $this->save( $product ); if ( empty( $attribute_terms_by_name ) ) { return $product; @@ -234,7 +255,7 @@ class FiltererTest extends \WC_Unit_Test_Case { ) ); - $product->save(); + $this->save( $product ); $product_id = $product->get_id(); @@ -259,7 +280,7 @@ class FiltererTest extends \WC_Unit_Test_Case { } $variation->set_attributes( $attributes ); $variation->set_stock_status( $variation_data['in_stock'] ? 'instock' : 'outofstock' ); - $variation->save(); + $this->save( $variation ); $variation_ids[] = $variation->get_id(); } From 7c9137698b4e3177e2afa988b9764e5b564a4159 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Fri, 11 Jun 2021 11:44:57 +0200 Subject: [PATCH 07/25] Implement handling of changesets in the LookupDataStore class. --- .../DataRegenerator.php | 2 +- .../LookupDataStore.php | 136 ++++++++++++++---- .../DataRegeneratorTest.php | 2 +- .../LookupDataStoreTest.php | 16 +-- 4 files changed, 119 insertions(+), 37 deletions(-) diff --git a/src/Internal/ProductAttributesLookup/DataRegenerator.php b/src/Internal/ProductAttributesLookup/DataRegenerator.php index 8068abffb7e..a6d34f5533e 100644 --- a/src/Internal/ProductAttributesLookup/DataRegenerator.php +++ b/src/Internal/ProductAttributesLookup/DataRegenerator.php @@ -233,7 +233,7 @@ CREATE TABLE ' . $this->lookup_table_name . '( } foreach ( $product_ids as $id ) { - $this->data_store->insert_data_for_product( $id ); + $this->data_store->create_data_for_product( $id ); } update_option( 'woocommerce_attribute_lookup__last_products_page_processed', $current_products_page ); diff --git a/src/Internal/ProductAttributesLookup/LookupDataStore.php b/src/Internal/ProductAttributesLookup/LookupDataStore.php index 74f3ec6438f..d5c242fba5f 100644 --- a/src/Internal/ProductAttributesLookup/LookupDataStore.php +++ b/src/Internal/ProductAttributesLookup/LookupDataStore.php @@ -14,6 +14,15 @@ defined( 'ABSPATH' ) || exit; */ class LookupDataStore { + /** + * Types of updates to perform depending on the current changest + */ + + const ACTION_NONE = 0; + const ACTION_INSERT = 1; + const ACTION_UPDATE_STOCK = 2; + const ACTION_DELETE = 3; + /** * The lookup table name. * @@ -95,13 +104,13 @@ AND table_name = %s;', } /** - * Get the name of the lookup table. - * - * @return string - */ - public function get_lookup_table_name() { - return $this->lookup_table_name; - } + * Get the name of the lookup table. + * + * @return string + */ + public function get_lookup_table_name() { + return $this->lookup_table_name; + } /** * Insert/update the appropriate lookup table entries for a new or modified product or variation. @@ -120,20 +129,82 @@ AND table_name = %s;', $product = WC()->call_function( 'wc_get_product', $product ); } - // No changeset available: the product/variation is new, so just insert its data. + $update_type = $this->get_update_type( $changeset ); + switch ( $update_type ) { + case self::ACTION_INSERT: + $this->delete_data_for( $product->get_id() ); + $this->create_data_for( $product ); + break; + case self::ACTION_UPDATE_STOCK: + $this->update_stock_status_for( $product ); + break; + case self::ACTION_DELETE: + $this->delete_data_for( $product->get_id() ); + break; + } + } + + /** + * Determine the type of action to perform depending on the received changeset. + * + * @param array|null $changeset The changeset received by on_product_changed. + * @return int One of the ACTION_ constants. + */ + private function get_update_type( $changeset ) { if ( is_null( $changeset ) ) { - $this->delete_lookup_table_entries_for( $product->get_id() ); - if ( $this->is_variation( $product ) ) { - $this->insert_data_for_variation( $product ); + // No changeset at all means that the product is new. + return self::ACTION_INSERT; + } + + $keys = array_keys( $changeset ); + + // Order matters: + // - The change with the most precedence is a change in catalog visibility + // (which will result in all data being regenerated or deleted). + // - Then a change in attributes (all data will be regenerated). + // - And finally a change in stock status (existing data will be updated). + // Thus these conditions must be checked in that same order. + + if ( in_array( 'catalog_visibility', $keys, true ) ) { + $new_visibility = $changeset['catalog_visibility']; + if ( 'visible' === $new_visibility || 'catalog' === $new_visibility ) { + return self::ACTION_INSERT; } else { - $this->insert_data_for_product( $product ); + return self::ACTION_DELETE; } } - // Changeset available: process it. + if ( in_array( 'attributes', $keys, true ) ) { + return self::ACTION_INSERT; + } - // TODO: Implement changeset processing. + if ( array_intersect( $keys, array( 'stock_quantity', 'stock_status', 'manage_stock' ) ) ) { + return self::ACTION_UPDATE_STOCK; + } + + return self::ACTION_NONE; + } + + /** + * Update the stock status of the lookup table entries for a given product. + * + * @param \WC_Product $product The product to update the entries for. + */ + private function update_stock_status_for( \WC_Product $product ) { + global $wpdb; + + $in_stock = $product->is_in_stock(); + + // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared + $wpdb->query( + $wpdb->prepare( + 'UPDATE ' . $this->lookup_table_name . ' SET in_stock = %d WHERE product_id = %d', + $in_stock ? 1 : 0, + $product->get_id() + ) + ); + // phpcs:enable WordPress.DB.PreparedSQL.NotPrepared } /** @@ -154,31 +225,42 @@ AND table_name = %s;', $product_id = $product; } - $this->delete_lookup_table_entries_for( $product_id ); + $this->delete_data_for( $product_id ); } /** - * Insert the lookup data for a given product or variation. - * If a variable product is passed the information is created for all of its variations. + * Create the lookup data for a given product, if a variable product is passed + * the information is created for all of its variations. + * This method is intended to be called from the data regenerator. * * @param int|WC_Product $product Product object or id. * @throws \Exception A variation object is passed. */ - public function insert_data_for_product( $product ) { + public function create_data_for_product( $product ) { if ( ! is_a( $product, \WC_Product::class ) ) { $product = WC()->call_function( 'wc_get_product', $product ); } if ( $this->is_variation( $product ) ) { - throw new \Exception( "LookupDataStore::insert_data_for_product can't be called for variations." ); + throw new \Exception( "LookupDataStore::create_data_for_product can't be called for variations." ); } - $this->delete_lookup_table_entries_for( $product->get_id() ); + $this->delete_data_for( $product->get_id() ); + $this->create_data_for( $product ); + } - if ( $this->is_variable_product( $product ) ) { - $this->create_lookup_table_entries_for_variable_product( $product ); + /** + * Create lookup table data for a given product. + * + * @param \WC_Product $product The product to create the data for. + */ + private function create_data_for( \WC_Product $product ) { + if ( $this->is_variation( $product ) ) { + $this->create_data_for_variation( $product ); + } elseif ( $this->is_variable_product( $product ) ) { + $this->create_data_for_variable_product( $product ); } else { - $this->create_lookup_table_entries_for_simple_product( $product ); + $this->create_data_for_simple_product( $product ); } } @@ -188,7 +270,7 @@ AND table_name = %s;', * * @param int $product_id Simple product id, or main/parent product id for variable products. */ - private function delete_lookup_table_entries_for( int $product_id ) { + private function delete_data_for( int $product_id ) { global $wpdb; // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared @@ -208,7 +290,7 @@ AND table_name = %s;', * * @param \WC_Product $product The product to create the entries for. */ - private function create_lookup_table_entries_for_simple_product( \WC_Product $product ) { + private function create_data_for_simple_product( \WC_Product $product ) { $product_attributes_data = $this->get_attribute_taxonomies( $product ); $has_stock = $product->is_in_stock(); $product_id = $product->get_id(); @@ -226,7 +308,7 @@ AND table_name = %s;', * * @param \WC_Product_Variable $product The product to create the entries for. */ - private function create_lookup_table_entries_for_variable_product( \WC_Product_Variable $product ) { + private function create_data_for_variable_product( \WC_Product_Variable $product ) { $product_attributes_data = $this->get_attribute_taxonomies( $product ); $variation_attributes_data = array_filter( $product_attributes_data, @@ -266,7 +348,7 @@ AND table_name = %s;', * * @param \WC_Product_Variation $variation The variation to create entries for. */ - private function insert_data_for_variation( \WC_Product_Variation $variation ) { + private function create_data_for_variation( \WC_Product_Variation $variation ) { $main_product = wc_get_product( $variation->get_parent_id() ); $product_attributes_data = $this->get_attribute_taxonomies( $main_product ); diff --git a/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php b/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php index 5f8b0df35af..8265371dda7 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php @@ -51,7 +51,7 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { $this->lookup_data_store = new class() extends LookupDataStore { public $passed_products = array(); - public function insert_data_for_product( $product ) { + public function create_data_for_product( $product ) { $this->passed_products[] = $product; } }; diff --git a/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php b/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php index 0f69db06a3b..79d81a51f8a 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php @@ -42,19 +42,19 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { } /** - * @testdox `insert_data_for_product` throws an exception if a variation is passed. + * @testdox `create_data_for_product` throws an exception if a variation is passed. */ - public function test_insert_data_for_product_throws_if_variation_is_passed() { + public function test_create_data_for_product_throws_if_variation_is_passed() { $product = new \WC_Product_Variation(); $this->expectException( \Exception::class ); - $this->expectExceptionMessage( "LookupDataStore::insert_data_for_product can't be called for variations." ); + $this->expectExceptionMessage( "LookupDataStore::create_data_for_product can't be called for variations." ); - $this->sut->insert_data_for_product( $product ); + $this->sut->create_data_for_product( $product ); } /** - * @testdox `insert_data_for_product` creates the appropriate entries for simple products, skipping custom product attributes. + * @testdox `create_data_for_product` creates the appropriate entries for simple products, skipping custom product attributes. * * @testWith [true] * [false] @@ -90,7 +90,7 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { $expected_in_stock = 0; } - $this->sut->insert_data_for_product( $product ); + $this->sut->create_data_for_product( $product ); $expected = array( array( @@ -133,7 +133,7 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { } /** - * @testdox `insert_data_for_product` creates the appropriate entries for variable products. + * @testdox `create_data_for_product` creates the appropriate entries for variable products. */ public function test_update_data_for_variable_product() { $products = array(); @@ -239,7 +239,7 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { $products[1001] = $variation_1; $products[1002] = $variation_2; - $this->sut->insert_data_for_product( $product ); + $this->sut->create_data_for_product( $product ); $expected = array( // Main product: one entry for each of the regular attribute values, From 6dbcd64a21a1bca9e4e8996f8700acdae8621f02 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Fri, 11 Jun 2021 11:59:30 +0200 Subject: [PATCH 08/25] Disable attributes lookup table usage before initiating regeneration. --- src/Internal/ProductAttributesLookup/DataRegenerator.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Internal/ProductAttributesLookup/DataRegenerator.php b/src/Internal/ProductAttributesLookup/DataRegenerator.php index a6d34f5533e..d027fb2b8a5 100644 --- a/src/Internal/ProductAttributesLookup/DataRegenerator.php +++ b/src/Internal/ProductAttributesLookup/DataRegenerator.php @@ -93,6 +93,8 @@ class DataRegenerator { * (Note how we are returning "false" since the class handles the step scheduling by itself). */ public function initiate_regeneration() { + $this->enable_or_disable_lookup_table_usage( false ); + $this->delete_all_attributes_lookup_data(); $products_exist = $this->initialize_table_and_data(); if ( $products_exist ) { From 0192ed0b936378b801cdf01c7122471c9f3afec5 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Tue, 15 Jun 2021 16:33:29 +0200 Subject: [PATCH 09/25] Change LookupDataStore to allow updates being done in a scheduled action. There's a new option, 'woocommerce_attribute_lookup__direct_updates'. When set to 'yes', updates to the lookup table are performed as soon as the change happen; otherwise, a scheduled action will do it, the hook name is 'woocommerce_run_product_attribute_lookup_update_callback' (the existing hook in the DataRegenerator class is renamed to 'woocommerce_run_product_attribute_lookup_update_callback') Also, the settings page has a new "Advanced" section with a checkbox to control the value of that new option; the section is visible only when the feature has been enabled via LookupDataStore::show_feature. --- includes/class-woocommerce.php | 6 +- .../DataRegenerator.php | 4 +- .../LookupDataStore.php | 116 +++++++++++++++++- .../DataRegeneratorTest.php | 10 +- 4 files changed, 123 insertions(+), 13 deletions(-) diff --git a/includes/class-woocommerce.php b/includes/class-woocommerce.php index c75a69cf2c9..b05bdb26e02 100644 --- a/includes/class-woocommerce.php +++ b/includes/class-woocommerce.php @@ -8,10 +8,11 @@ defined( 'ABSPATH' ) || exit; -use Automattic\WooCommerce\Internal\DownloadPermissionsAdjuster; -use Automattic\WooCommerce\Internal\RestockRefundedItemsAdjuster; use Automattic\WooCommerce\Internal\AssignDefaultCategory; +use Automattic\WooCommerce\Internal\DownloadPermissionsAdjuster; use Automattic\WooCommerce\Internal\ProductAttributesLookup\DataRegenerator; +use Automattic\WooCommerce\Internal\ProductAttributesLookup\LookupDataStore; +use Automattic\WooCommerce\Internal\RestockRefundedItemsAdjuster; use Automattic\WooCommerce\Proxies\LegacyProxy; /** @@ -213,6 +214,7 @@ final class WooCommerce { wc_get_container()->get( DownloadPermissionsAdjuster::class ); wc_get_container()->get( AssignDefaultCategory::class ); wc_get_container()->get( DataRegenerator::class ); + wc_get_container()->get( LookupDataStore::class ); wc_get_container()->get( RestockRefundedItemsAdjuster::class ); } diff --git a/src/Internal/ProductAttributesLookup/DataRegenerator.php b/src/Internal/ProductAttributesLookup/DataRegenerator.php index d027fb2b8a5..88f0539d799 100644 --- a/src/Internal/ProductAttributesLookup/DataRegenerator.php +++ b/src/Internal/ProductAttributesLookup/DataRegenerator.php @@ -61,7 +61,7 @@ class DataRegenerator { ); add_action( - 'woocommerce_run_product_attribute_lookup_update_callback', + 'woocommerce_run_product_attribute_lookup_regeneration_callback', function () { $this->run_regeneration_step_callback(); } @@ -202,7 +202,7 @@ CREATE TABLE ' . $this->lookup_table_name . '( $queue = WC()->get_instance_of( \WC_Queue::class ); $queue->schedule_single( WC()->call_function( 'time' ) + 1, - 'woocommerce_run_product_attribute_lookup_update_callback', + 'woocommerce_run_product_attribute_lookup_regeneration_callback', array(), 'woocommerce-db-updates' ); diff --git a/src/Internal/ProductAttributesLookup/LookupDataStore.php b/src/Internal/ProductAttributesLookup/LookupDataStore.php index d5c242fba5f..c9945c069fd 100644 --- a/src/Internal/ProductAttributesLookup/LookupDataStore.php +++ b/src/Internal/ProductAttributesLookup/LookupDataStore.php @@ -56,6 +56,61 @@ class LookupDataStore { $this->is_feature_visible = false; $this->lookup_table_exists = $this->check_lookup_table_exists(); + + $this->init_hooks(); + } + + /** + * Initialize the hooks used by the class. + */ + private function init_hooks() { + add_action( + 'woocommerce_run_product_attribute_lookup_update_callback', + function ( $product_id, $action ) { + $this->run_update_callback( $product_id, $action ); + }, + 10, + 2 + ); + + add_filter( + 'woocommerce_get_sections_products', + function ( $products ) { + if ( $this->is_feature_visible() ) { + $products['advanced'] = __( 'Advanced', 'woocommerce' ); + } + return $products; + }, + 100, + 1 + ); + + add_filter( + 'woocommerce_get_settings_products', + function ( $settings, $section_id ) { + if ( 'advanced' === $section_id && $this->is_feature_visible() ) { + $settings[] = + array( + 'title' => __( 'Product attributes lookup table', 'woocommerce' ), + 'type' => 'title', + ); + + $settings[] = array( + 'title' => __( 'Direct updates', 'woocommerce' ), + 'desc' => __( 'Update the table directly upon product changes, instead of scheduling a deferred update.', 'woocommerce' ), + 'id' => 'woocommerce_attribute_lookup__direct_updates', + 'default' => 'no', + 'type' => 'checkbox', + 'checkboxgroup' => 'start', + ); + + $settings[] = array( 'type' => 'sectionend' ); + } + return $settings; + }, + 100, + 2 + ); } /** @@ -129,9 +184,62 @@ AND table_name = %s;', $product = WC()->call_function( 'wc_get_product', $product ); } - $update_type = $this->get_update_type( $changeset ); + $action = $this->get_update_action( $changeset ); + $this->maybe_schedule_update( $product->get_id(), $action ); + } - switch ( $update_type ) { + /** + * Schedule an update of the product attributes lookup table for a given product. + * If an update for the same action is already scheduled, nothing is done. + * + * If the 'woocommerce_attribute_lookup__direct_update' option is set to 'yes', + * the update is done directly, without scheduling. + * + * @param int $product_id The product id to schedule the update for. + * @param int $action The action to perform, one of the ACTION_ constants. + */ + private function maybe_schedule_update( int $product_id, int $action ) { + if ( 'yes' === get_option( 'woocommerce_attribute_lookup__direct_updates' ) ) { + $this->run_update_callback( $product_id, $action ); + return; + } + + $args = array( $product_id, $action ); + + $queue = WC()->get_instance_of( \WC_Queue::class ); + $already_scheduled = $queue->search( + array( + 'hook' => 'woocommerce_run_product_attribute_lookup_update_callback', + 'args' => $args, + 'status' => \ActionScheduler_Store::STATUS_PENDING, + ), + 'ids' + ); + + if ( empty( $already_scheduled ) ) { + $queue->schedule_single( + WC()->call_function( 'time' ) + 1, + 'woocommerce_run_product_attribute_lookup_update_callback', + $args, + 'woocommerce-db-updates' + ); + } + } + + /** + * Perform an update of the lookup table for a specific product. + * + * @param int $product_id The product id to perform the update for. + * @param int $action The action to perform, one of the ACTION_ constants. + */ + private function run_update_callback( int $product_id, int $action ) { + if ( ! $this->lookup_table_exists ) { + return; + } + + $product = WC()->call_function( 'wc_get_product', $product_id ); + + switch ( $action ) { case self::ACTION_INSERT: $this->delete_data_for( $product->get_id() ); $this->create_data_for( $product ); @@ -151,7 +259,7 @@ AND table_name = %s;', * @param array|null $changeset The changeset received by on_product_changed. * @return int One of the ACTION_ constants. */ - private function get_update_type( $changeset ) { + private function get_update_action( $changeset ) { if ( is_null( $changeset ) ) { // No changeset at all means that the product is new. return self::ACTION_INSERT; @@ -225,7 +333,7 @@ AND table_name = %s;', $product_id = $product; } - $this->delete_data_for( $product_id ); + $this->maybe_schedule_update( $product_id, self::ACTION_DELETE ); } /** diff --git a/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php b/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php index 8265371dda7..2699cc81537 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php @@ -58,7 +58,7 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { // phpcs:enable Squiz.Commenting // This is needed to prevent the hook to act on the already registered LookupDataStore class. - remove_all_actions( 'woocommerce_run_product_attribute_lookup_update_callback' ); + remove_all_actions( 'woocommerce_run_product_attribute_lookup_regeneration_callback' ); $container = wc_get_container(); $container->reset_all_resolved(); @@ -128,7 +128,7 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { 'method' => 'schedule_single', 'args' => array(), 'timestamp' => 1001, - 'hook' => 'woocommerce_run_product_attribute_lookup_update_callback', + 'hook' => 'woocommerce_run_product_attribute_lookup_regeneration_callback', 'group' => 'woocommerce-db-updates', ); $actual_enqueued = current( $this->queue->methods_called ); @@ -188,7 +188,7 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { update_option( 'woocommerce_attribute_lookup__last_products_page_processed', 7 ); - do_action( 'woocommerce_run_product_attribute_lookup_update_callback' ); + do_action( 'woocommerce_run_product_attribute_lookup_regeneration_callback' ); $this->assertEquals( array( 1, 2, 3 ), $this->lookup_data_store->passed_products ); $this->assertEquals( array( 8 ), $requested_products_pages ); @@ -198,7 +198,7 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { 'method' => 'schedule_single', 'args' => array(), 'timestamp' => 1001, - 'hook' => 'woocommerce_run_product_attribute_lookup_update_callback', + 'hook' => 'woocommerce_run_product_attribute_lookup_regeneration_callback', 'group' => 'woocommerce-db-updates', ); $actual_enqueued = current( $this->queue->methods_called ); @@ -233,7 +233,7 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { $this->sut->initiate_regeneration(); $this->queue->methods_called = array(); - do_action( 'woocommerce_run_product_attribute_lookup_update_callback' ); + do_action( 'woocommerce_run_product_attribute_lookup_regeneration_callback' ); $this->assertEquals( $product_ids, $this->lookup_data_store->passed_products ); $this->assertFalse( get_option( 'woocommerce_attribute_lookup__last_product_id_to_process' ) ); From 48c44a6128fe67d055faebb26770914a761a539e Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Thu, 17 Jun 2021 15:31:23 +0200 Subject: [PATCH 10/25] Add tests for product deletion in LookupDataStoreTest. --- includes/class-wc-post-data.php | 25 +- .../LookupDataStore.php | 7 +- tests/Tools/FakeQueue.php | 8 +- .../LookupDataStoreTest.php | 408 +++++++++++++++++- 4 files changed, 437 insertions(+), 11 deletions(-) diff --git a/includes/class-wc-post-data.php b/includes/class-wc-post-data.php index 94b6c3d0843..8473de148c2 100644 --- a/includes/class-wc-post-data.php +++ b/includes/class-wc-post-data.php @@ -9,6 +9,7 @@ */ use Automattic\WooCommerce\Internal\ProductAttributesLookup\LookupDataStore as ProductAttributesLookupDataStore; +use Automattic\WooCommerce\Proxies\LegacyProxy; defined( 'ABSPATH' ) || exit; @@ -298,18 +299,18 @@ class WC_Post_Data { * @param mixed $id ID of post being deleted. */ public static function delete_post( $id ) { - if ( ! current_user_can( 'delete_posts' ) || ! $id ) { + $container = wc_get_container(); + if ( ! $container->get( LegacyProxy::class )->call_function( 'current_user_can', 'delete_posts' ) || ! $id ) { return; } - $post_type = get_post_type( $id ); - + $post_type = self::get_post_type( $id ); switch ( $post_type ) { case 'product': $data_store = WC_Data_Store::load( 'product-variable' ); $data_store->delete_variations( $id, true ); $data_store->delete_from_lookup_table( $id, 'wc_product_meta_lookup' ); - wc_get_container()->get( ProductAttributesLookupDataStore::class )->on_product_deleted( $id ); + $container->get( ProductAttributesLookupDataStore::class )->on_product_deleted( $id ); $parent_id = wp_get_post_parent_id( $id ); if ( $parent_id ) { @@ -321,7 +322,7 @@ class WC_Post_Data { $data_store = WC_Data_Store::load( 'product' ); $data_store->delete_from_lookup_table( $id, 'wc_product_meta_lookup' ); wc_delete_product_transients( wp_get_post_parent_id( $id ) ); - wc_get_container()->get( ProductAttributesLookupDataStore::class )->on_product_deleted( $id ); + $container->get( ProductAttributesLookupDataStore::class )->on_product_deleted( $id ); break; case 'shop_order': @@ -348,7 +349,7 @@ class WC_Post_Data { return; } - $post_type = get_post_type( $id ); + $post_type = self::get_post_type( $id ); // If this is an order, trash any refunds too. if ( in_array( $post_type, wc_get_order_types( 'order-count' ), true ) ) { @@ -382,7 +383,7 @@ class WC_Post_Data { return; } - $post_type = get_post_type( $id ); + $post_type = self::get_post_type( $id ); if ( in_array( $post_type, wc_get_order_types( 'order-count' ), true ) ) { global $wpdb; @@ -407,6 +408,16 @@ class WC_Post_Data { } } + /** + * Get the post type for a given post. + * + * @param int $id The post id. + * @return string The post type. + */ + private static function get_post_type( $id ) { + return wc_get_container()->get( LegacyProxy::class )->call_function( 'get_post_type', $id ); + } + /** * Before deleting an order, do some cleanup. * diff --git a/src/Internal/ProductAttributesLookup/LookupDataStore.php b/src/Internal/ProductAttributesLookup/LookupDataStore.php index c9945c069fd..0edf87732ff 100644 --- a/src/Internal/ProductAttributesLookup/LookupDataStore.php +++ b/src/Internal/ProductAttributesLookup/LookupDataStore.php @@ -238,17 +238,20 @@ AND table_name = %s;', } $product = WC()->call_function( 'wc_get_product', $product_id ); + if ( ! $product ) { + $action = self::ACTION_DELETE; + } switch ( $action ) { case self::ACTION_INSERT: - $this->delete_data_for( $product->get_id() ); + $this->delete_data_for( $product_id ); $this->create_data_for( $product ); break; case self::ACTION_UPDATE_STOCK: $this->update_stock_status_for( $product ); break; case self::ACTION_DELETE: - $this->delete_data_for( $product->get_id() ); + $this->delete_data_for( $product_id ); break; } } diff --git a/tests/Tools/FakeQueue.php b/tests/Tools/FakeQueue.php index a8dd9151b55..07604bc71a6 100644 --- a/tests/Tools/FakeQueue.php +++ b/tests/Tools/FakeQueue.php @@ -72,7 +72,13 @@ class FakeQueue implements \WC_Queue_Interface { } public function search( $args = array(), $return_format = OBJECT ) { - // TODO: Implement search() method. + $result = array(); + foreach ( $this->methods_called as $method_called ) { + if ( $method_called['args'] === $args['args'] && $method_called['hook'] === $args['hook'] ) { + $result[] = $method_called; + } + } + return $result; } // phpcs:enable Squiz.Commenting.FunctionComment.Missing diff --git a/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php b/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php index 79d81a51f8a..c7056d3f3fc 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php @@ -9,6 +9,7 @@ use Automattic\WooCommerce\Internal\ProductAttributesLookup\DataRegenerator; use Automattic\WooCommerce\Internal\ProductAttributesLookup\LookupDataStore; use Automattic\WooCommerce\Testing\Tools\FakeQueue; use Automattic\WooCommerce\Utilities\ArrayUtil; +use Automattic\WooCommerce\Testing\Tools\CodeHacking\Hacks\FunctionsMockerHack; /** * Tests for the LookupDataStore class. @@ -23,13 +24,21 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { */ private $sut; + /** + * The lookup table name. + * + * @var string + */ + private $lookup_table_name; + /** * Runs before each test. */ public function setUp() { global $wpdb; - $this->sut = new LookupDataStore(); + $this->lookup_table_name = $wpdb->prefix . 'wc_product_attributes_lookup'; + $this->sut = new LookupDataStore(); // Initiating regeneration with a fake queue will just create the lookup table in the database. add_filter( @@ -39,6 +48,11 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { } ); $this->get_instance_of( DataRegenerator::class )->initiate_regeneration(); + + $queue = WC()->get_instance_of( \WC_Queue::class ); + $queue->methods_called = array(); + + $this->reset_legacy_proxy_mocks(); } /** @@ -331,6 +345,329 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { $this->assertEquals( sort( $expected ), sort( $actual ) ); } + /** + * @testdox Deleting a simple product schedules deletion of lookup table entries when the "direct updates" option is off. + * + * @testWith ["wp_trash_post"] + * ["delete_post"] + * ["delete_method_in_product"] + * ["force_delete_method_in_product"] + * + * @param string $deletion_mechanism The mechanism used for deletion, one of: 'wp_trash_post', 'delete_post', 'delete_method_in_product', 'force_delete_method_in_product'. + */ + public function test_deleting_simple_product_schedules_deletion( $deletion_mechanism ) { + $this->set_direct_update_option( false ); + + $product = new \WC_Product_Simple(); + $product_id = 10; + $product->set_id( $product_id ); + $this->save( $product ); + + $this->register_legacy_proxy_function_mocks( + array( + 'get_post_type' => function( $id ) use ( $product ) { + if ( $id === $product->get_id() || $id === $product ) { + return 'product'; + } else { + return get_post_type( $id ); + } + }, + 'time' => function() { + return 100; + }, + 'current_user_can' => function( $capability, ...$args ) { + if ( 'delete_posts' === $capability ) { + return true; + } else { + return current_user_can( $capability, $args ); + } + }, + ) + ); + + $this->delete_product( $product, $deletion_mechanism ); + + $queue_calls = WC()->get_instance_of( \WC_Queue::class )->methods_called; + + $this->assertEquals( 1, count( $queue_calls ) ); + + $expected = array( + 'method' => 'schedule_single', + 'args' => + array( + $product_id, + LookupDataStore::ACTION_DELETE, + ), + 'group' => 'woocommerce-db-updates', + 'timestamp' => 101, + 'hook' => 'woocommerce_run_product_attribute_lookup_update_callback', + ); + $this->assertEquals( $expected, $queue_calls[0] ); + } + + /** + * Delete a product or variation. + * + * @param \WC_Product $product The product to delete. + * @param string $deletion_mechanism The mechanism used for deletion, one of: 'wp_trash_post', 'delete_post', 'delete_method_in_product', 'force_delete_method_in_product'. + */ + private function delete_product( \WC_Product $product, string $deletion_mechanism ) { + // We can't use the 'wp_trash_post' and 'delete_post' functions directly + // because these invoke 'get_post', which fails because tests runs within an + // uncommitted database transaction. Being WP core functions they can't be mocked or hacked. + // So instead, we trigger the actions that the tested functionality captures. + + switch ( $deletion_mechanism ) { + case 'wp_trash_post': + do_action( 'wp_trash_post', $product ); + break; + case 'delete_post': + do_action( 'delete_post', $product->get_id() ); + break; + case 'delete_method_in_product': + $product->delete( false ); + break; + case 'force_delete_method_in_product': + $product->delete( true ); + break; + } + } + + /** + * @testdox Deleting a variable product schedules deletion of lookup table entries when the "direct updates" option is off. + * + * @testWith ["wp_trash_post"] + * ["delete_post"] + * ["delete_method_in_product"] + * ["force_delete_method_in_product"] + * + * @param string $deletion_mechanism The mechanism used for deletion, one of: 'wp_trash_post', 'delete_post', 'delete_method_in_product', 'force_delete_method_in_product'. + */ + public function test_deleting_variable_product_schedules_deletion( $deletion_mechanism ) { + $this->set_direct_update_option( false ); + + $product = new \WC_Product_Variable(); + $product->set_id( 1000 ); + + $variation = new \WC_Product_Variation(); + $variation->set_id( 1001 ); + + $product->set_children( array( 1001 ) ); + $this->save( $product ); + + $product_id = $product->get_id(); + + $this->register_legacy_proxy_function_mocks( + array( + 'get_post_type' => function( $id ) use ( $product, $variation ) { + if ( $id === $product->get_id() || $id === $product ) { + return 'product'; + } elseif ( $id === $variation->get_id() || $id === $variation ) { + return 'product_variation'; + } else { + return get_post_type( $id ); + } + }, + 'time' => function() { + return 100; + }, + 'current_user_can' => function( $capability, ...$args ) { + if ( 'delete_posts' === $capability ) { + return true; + } else { + return current_user_can( $capability, $args ); + } + }, + ) + ); + + $this->delete_product( $product, $deletion_mechanism ); + + $queue_calls = WC()->get_instance_of( \WC_Queue::class )->methods_called; + + $this->assertEquals( 1, count( $queue_calls ) ); + + $expected = array( + 'method' => 'schedule_single', + 'args' => + array( + $product_id, + LookupDataStore::ACTION_DELETE, + ), + 'group' => 'woocommerce-db-updates', + 'timestamp' => 101, + 'hook' => 'woocommerce_run_product_attribute_lookup_update_callback', + ); + + $this->assertEquals( $expected, $queue_calls[0] ); + } + + /** + * @testdox Deleting a variation schedules deletion of lookup table entries when the "direct updates" option is off. + * + * @testWith ["wp_trash_post"] + * ["delete_post"] + * ["delete_method_in_product"] + * ["force_delete_method_in_product"] + * + * @param string $deletion_mechanism The mechanism used for deletion, one of: 'wp_trash_post', 'delete_post', 'delete_method_in_product', 'force_delete_method_in_product'. + */ + public function test_deleting_variation_schedules_deletion( $deletion_mechanism ) { + $this->set_direct_update_option( false ); + + $product = new \WC_Product_Variable(); + $product->set_id( 1000 ); + + $variation = new \WC_Product_Variation(); + $variation->set_id( 1001 ); + + $product->set_children( array( 1001 ) ); + $this->save( $product ); + + $variation_id = $product->get_id(); + + $this->register_legacy_proxy_function_mocks( + array( + 'get_post_type' => function( $id ) use ( $product, $variation ) { + if ( $id === $product->get_id() || $id === $product ) { + return 'product'; + } elseif ( $id === $variation->get_id() || $id === $variation ) { + return 'product_variation'; + } else { + return get_post_type( $id ); + } + }, + 'time' => function() { + return 100; + }, + 'current_user_can' => function( $capability, ...$args ) { + if ( 'delete_posts' === $capability ) { + return true; + } else { + return current_user_can( $capability, $args ); + } + }, + ) + ); + + $this->delete_product( $product, $deletion_mechanism ); + + $queue_calls = WC()->get_instance_of( \WC_Queue::class )->methods_called; + + $this->assertEquals( 1, count( $queue_calls ) ); + + $expected = array( + 'method' => 'schedule_single', + 'args' => + array( + $variation_id, + LookupDataStore::ACTION_DELETE, + ), + 'group' => 'woocommerce-db-updates', + 'timestamp' => 101, + 'hook' => 'woocommerce_run_product_attribute_lookup_update_callback', + ); + + $this->assertEquals( $expected, $queue_calls[0] ); + } + + /** + * @testdox 'on_product_deleted' doesn't schedule duplicate deletions (for the same product). + */ + public function test_no_duplicate_deletions_are_scheduled() { + $this->set_direct_update_option( false ); + + $this->register_legacy_proxy_function_mocks( + array( + 'time' => function() { + return 100; + }, + ) + ); + + $this->sut->on_product_deleted( 1 ); + $this->sut->on_product_deleted( 1 ); + $this->sut->on_product_deleted( 2 ); + + $queue_calls = WC()->get_instance_of( \WC_Queue::class )->methods_called; + + $this->assertEquals( 2, count( $queue_calls ) ); + + $expected = array( + array( + 'method' => 'schedule_single', + 'args' => + array( + 1, + LookupDataStore::ACTION_DELETE, + ), + 'group' => 'woocommerce-db-updates', + 'timestamp' => 101, + 'hook' => 'woocommerce_run_product_attribute_lookup_update_callback', + ), + array( + 'method' => 'schedule_single', + 'args' => + array( + 2, + LookupDataStore::ACTION_DELETE, + ), + 'group' => 'woocommerce-db-updates', + 'timestamp' => 101, + 'hook' => 'woocommerce_run_product_attribute_lookup_update_callback', + ), + ); + + $this->assertEquals( $expected, $queue_calls ); + } + + /** + * @testdox 'on_product_deleted' deletes the data for a variation when the "direct updates" option is on. + */ + public function test_direct_deletion_of_variation() { + global $wpdb; + + $this->set_direct_update_option( true ); + + $variation = new \WC_Product_Variation(); + $variation->set_id( 2 ); + $this->save( $variation ); + + $this->insert_lookup_table_data( 1, 1, 'pa_foo', 10, false, true ); + $this->insert_lookup_table_data( 2, 1, 'pa_bar', 20, true, true ); + + $this->sut->on_product_deleted( $variation ); + + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + $rows = $wpdb->get_results( 'SELECT DISTINCT product_id FROM ' . $this->lookup_table_name, ARRAY_N ); + + $this->assertEquals( array( 1 ), $rows[0] ); + } + + /** + * @testdox 'on_product_deleted' deletes the data for a product and its variations when the "direct updates" option is on. + */ + public function test_direct_deletion_of_product() { + global $wpdb; + + $this->set_direct_update_option( true ); + + $product = new \WC_Product(); + $product->set_id( 1 ); + $this->save( $product ); + + $this->insert_lookup_table_data( 1, 1, 'pa_foo', 10, false, true ); + $this->insert_lookup_table_data( 2, 1, 'pa_bar', 20, true, true ); + $this->insert_lookup_table_data( 3, 3, 'pa_foo', 10, false, true ); + + $this->sut->on_product_deleted( $product ); + + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + $rows = $wpdb->get_results( 'SELECT DISTINCT product_id FROM ' . $this->lookup_table_name, ARRAY_N ); + + $this->assertEquals( array( 3 ), $rows[0] ); + } + /** * Set the product attributes from an array with this format: * @@ -380,4 +717,73 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { return $result; } + + /** + * Set the value of the option for direct lookup table updates. + * + * @param bool $value True to set the option to 'yes', false for 'no'. + */ + private function set_direct_update_option( bool $value ) { + update_option( 'woocommerce_attribute_lookup__direct_updates', $value ? 'yes' : 'no' ); + } + + /** + * Save a product and delete any lookup table data that may have been automatically inserted + * (for the purposes of unit testing we want to insert this data manually) + * + * @param \WC_Product $product The product to save and delete lookup table data for. + */ + private function save( \WC_Product $product ) { + global $wpdb; + + $product->save(); + + // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared + $wpdb->query( + $wpdb->prepare( + "DELETE FROM {$wpdb->prefix}wc_product_attributes_lookup WHERE product_id = %d", + $product->get_id() + ) + ); + // phpcs:enable WordPress.DB.PreparedSQL.NotPrepared + + $queue = WC()->get_instance_of( \WC_Queue::class ); + $queue->methods_called = array(); + } + + /** + * Insert one entry in the lookup table. + * + * @param int $product_id The product id. + * @param int $product_or_parent_id The product id for non-variable products, the main/parent product id for variations. + * @param string $taxonomy Taxonomy name. + * @param int $term_id Term id. + * @param bool $is_variation_attribute True if the taxonomy corresponds to an attribute used to define variations. + * @param bool $has_stock True if the product is in stock. + */ + private function insert_lookup_table_data( int $product_id, int $product_or_parent_id, string $taxonomy, int $term_id, bool $is_variation_attribute, bool $has_stock ) { + global $wpdb; + + // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared + $wpdb->query( + $wpdb->prepare( + 'INSERT INTO ' . $this->lookup_table_name . ' ( + product_id, + product_or_parent_id, + taxonomy, + term_id, + is_variation_attribute, + in_stock) + VALUES + ( %d, %d, %s, %d, %d, %d )', + $product_id, + $product_or_parent_id, + $taxonomy, + $term_id, + $is_variation_attribute ? 1 : 0, + $has_stock ? 1 : 0 + ) + ); + // phpcs:enable WordPress.DB.PreparedSQL.NotPrepared + } } From 38897a42e28077c9c72fc6128631e8761657fd1c Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Mon, 28 Jun 2021 12:25:48 +0200 Subject: [PATCH 11/25] Add tests for product changes changes in LookupDataStore. --- .../LookupDataStore.php | 6 +- tests/Tools/FakeQueue.php | 19 +- .../LookupDataStoreTest.php | 437 +++++++++++++++++- 3 files changed, 450 insertions(+), 12 deletions(-) diff --git a/src/Internal/ProductAttributesLookup/LookupDataStore.php b/src/Internal/ProductAttributesLookup/LookupDataStore.php index 0edf87732ff..c57956b7275 100644 --- a/src/Internal/ProductAttributesLookup/LookupDataStore.php +++ b/src/Internal/ProductAttributesLookup/LookupDataStore.php @@ -185,7 +185,9 @@ AND table_name = %s;', } $action = $this->get_update_action( $changeset ); - $this->maybe_schedule_update( $product->get_id(), $action ); + if ( self::ACTION_NONE !== $action ) { + $this->maybe_schedule_update( $product->get_id(), $action ); + } } /** @@ -460,7 +462,7 @@ AND table_name = %s;', * @param \WC_Product_Variation $variation The variation to create entries for. */ private function create_data_for_variation( \WC_Product_Variation $variation ) { - $main_product = wc_get_product( $variation->get_parent_id() ); + $main_product = WC()->call_function( 'wc_get_product', $variation->get_parent_id() ); $product_attributes_data = $this->get_attribute_taxonomies( $main_product ); $variation_attributes_data = array_filter( diff --git a/tests/Tools/FakeQueue.php b/tests/Tools/FakeQueue.php index 07604bc71a6..cf60f524bf9 100644 --- a/tests/Tools/FakeQueue.php +++ b/tests/Tools/FakeQueue.php @@ -31,7 +31,7 @@ class FakeQueue implements \WC_Queue_Interface { * * @var array */ - public $methods_called = array(); + private $methods_called = array(); // phpcs:disable Squiz.Commenting.FunctionComment.Missing @@ -100,4 +100,21 @@ class FakeQueue implements \WC_Queue_Interface { $this->methods_called[] = array_merge( $value, $extra_args ); } + + /** + * Get the data about the methods called so far. + * + * @return array The current value of $methods_called. + */ + public function get_methods_called() { + return $this->methods_called; + } + + /** + * Clears the collection of the methods called so far. + */ + public function clear_methods_called() { + $this->methods_called = array(); + } + } diff --git a/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php b/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php index c7056d3f3fc..5fc418dfff7 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php @@ -49,8 +49,8 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { ); $this->get_instance_of( DataRegenerator::class )->initiate_regeneration(); - $queue = WC()->get_instance_of( \WC_Queue::class ); - $queue->methods_called = array(); + $queue = WC()->get_instance_of( \WC_Queue::class ); + $queue->clear_methods_called(); $this->reset_legacy_proxy_mocks(); } @@ -75,7 +75,7 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { * * @param bool $in_stock 'true' if the product is supposed to be in stock. */ - public function test_update_data_for_simple_product( $in_stock ) { + public function test_create_data_for_simple_product( $in_stock ) { $product = new \WC_Product_Simple(); $product->set_id( 10 ); $this->set_product_attributes( @@ -387,7 +387,7 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { $this->delete_product( $product, $deletion_mechanism ); - $queue_calls = WC()->get_instance_of( \WC_Queue::class )->methods_called; + $queue_calls = WC()->get_instance_of( \WC_Queue::class )->get_methods_called(); $this->assertEquals( 1, count( $queue_calls ) ); @@ -483,7 +483,7 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { $this->delete_product( $product, $deletion_mechanism ); - $queue_calls = WC()->get_instance_of( \WC_Queue::class )->methods_called; + $queue_calls = WC()->get_instance_of( \WC_Queue::class )->get_methods_called(); $this->assertEquals( 1, count( $queue_calls ) ); @@ -552,7 +552,7 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { $this->delete_product( $product, $deletion_mechanism ); - $queue_calls = WC()->get_instance_of( \WC_Queue::class )->methods_called; + $queue_calls = WC()->get_instance_of( \WC_Queue::class )->get_methods_called(); $this->assertEquals( 1, count( $queue_calls ) ); @@ -589,7 +589,7 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { $this->sut->on_product_deleted( 1 ); $this->sut->on_product_deleted( 2 ); - $queue_calls = WC()->get_instance_of( \WC_Queue::class )->methods_called; + $queue_calls = WC()->get_instance_of( \WC_Queue::class )->get_methods_called(); $this->assertEquals( 2, count( $queue_calls ) ); @@ -668,6 +668,425 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { $this->assertEquals( array( 3 ), $rows[0] ); } + /** + * @testdox Changing the stock status of a simple product schedules update of lookup table entries when the "direct updates" option is off. + * + * @testWith ["instock", "outofstock"] + * ["outofstock", "instock"] + * + * @param string $old_status Original status of the product. + * @param string $new_status New status of the product. + */ + public function test_changing_simple_product_stock_schedules_update( string $old_status, string $new_status ) { + $this->set_direct_update_option( false ); + + $product = new \WC_Product_Simple(); + $product_id = 10; + $product->set_id( $product_id ); + $product->set_stock_status( $old_status ); + $this->save( $product ); + + $this->register_legacy_proxy_function_mocks( + array( + 'time' => function() { + return 100; + }, + ) + ); + + $product->set_stock_status( $new_status ); + $product->save(); + + $queue_calls = WC()->get_instance_of( \WC_Queue::class )->get_methods_called(); + + $this->assertEquals( 1, count( $queue_calls ) ); + + $expected = array( + 'method' => 'schedule_single', + 'args' => + array( + $product_id, + LookupDataStore::ACTION_UPDATE_STOCK, + ), + 'group' => 'woocommerce-db-updates', + 'timestamp' => 101, + 'hook' => 'woocommerce_run_product_attribute_lookup_update_callback', + ); + $this->assertEquals( $expected, $queue_calls[0] ); + } + + /** + * @testdox Changing the stock status of a variable product or a variation schedules update of lookup table entries when the "direct updates" option is off. + * + * @testWith ["instock", "outofstock", true] + * ["outofstock", "instock", true] + * ["instock", "outofstock", false] + * ["outofstock", "instock", false] + * + * @param string $old_status Original status of the product. + * @param string $new_status New status of the product. + * @param bool $change_variation_stock True if the stock of the variation changes. + */ + public function test_changing_variable_product_or_variation_stock_schedules_update( string $old_status, string $new_status, bool $change_variation_stock ) { + $this->set_direct_update_option( false ); + + $product = new \WC_Product_Variable(); + $product_id = 1000; + $product->set_id( $product_id ); + + $variation = new \WC_Product_Variation(); + $variation_id = 1001; + $variation->set_id( $variation_id ); + $variation->set_stock_status( $old_status ); + $variation->save(); + + $product->set_children( array( 1001 ) ); + $product->set_stock_status( $old_status ); + $this->save( $product ); + + $this->register_legacy_proxy_function_mocks( + array( + 'time' => function () { + return 100; + }, + ) + ); + + if ( $change_variation_stock ) { + $variation->set_stock_status( $new_status ); + $variation->save(); + } else { + $product->set_stock_status( $new_status ); + $product->save(); + } + + $queue_calls = WC()->get_instance_of( \WC_Queue::class )->get_methods_called(); + + $this->assertEquals( 1, count( $queue_calls ) ); + + $expected = array( + 'method' => 'schedule_single', + 'args' => + array( + $change_variation_stock ? $variation_id : $product_id, + LookupDataStore::ACTION_UPDATE_STOCK, + ), + 'group' => 'woocommerce-db-updates', + 'timestamp' => 101, + 'hook' => 'woocommerce_run_product_attribute_lookup_update_callback', + ); + + $this->assertEquals( $expected, $queue_calls[0] ); + } + + /** + * Data provider for on_product_changed tests with direct update option set. + * + * @return array[] + */ + public function data_provider_for_test_on_product_changed_with_direct_updates() { + return array( + array( + null, + 'creation', + ), + array( + array( 'attributes' => array() ), + 'creation', + ), + array( + array( 'stock_quantity' => 1 ), + 'update', + ), + array( + array( 'stock_status' => 'instock' ), + 'update', + ), + array( + array( 'manage_stock' => true ), + 'update', + ), + array( + array( 'catalog_visibility' => 'visible' ), + 'creation', + ), + array( + array( 'catalog_visibility' => 'catalog' ), + 'creation', + ), + array( + array( 'catalog_visibility' => 'search' ), + 'deletion', + ), + array( + array( 'catalog_visibility' => 'hidden' ), + 'deletion', + ), + array( + array( 'foo' => 'bar' ), + 'none', + ), + ); + } + + /** + * @testdox 'on_product_changed' creates, updates deletes the data for a simple product depending on the changeset when the "direct updates" option is on. + * + * @dataProvider data_provider_for_test_on_product_changed_with_direct_updates + * + * @param array $changeset The changeset to test. + * @param string $expected_action The expected performed action, one of 'none', 'creation', 'update' or 'deletion'. + */ + public function test_on_product_changed_for_simple_product_with_direct_updates( $changeset, $expected_action ) { + global $wpdb; + + $this->set_direct_update_option( true ); + + $product = new \WC_Product_Simple(); + $product->set_id( 2 ); + $product->set_stock_status( 'instock' ); + $this->set_product_attributes( + $product, + array( + 'pa_bar' => array( + 'id' => 100, + 'options' => array( 20 ), + ), + ) + ); + $this->register_legacy_proxy_function_mocks( + array( + 'wc_get_product' => function( $id ) use ( $product ) { + if ( $id === $product->get_id() || $id === $product ) { + return $product; + } else { + return wc_get_product( $id ); + } + }, + ) + ); + + $this->insert_lookup_table_data( 1, 1, 'pa_foo', 10, false, true ); + if ( 'creation' !== $expected_action ) { + $this->insert_lookup_table_data( 2, 2, 'pa_bar', 20, false, false ); + } + + $this->sut->on_product_changed( $product, $changeset ); + + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + $rows = $wpdb->get_results( 'SELECT * FROM ' . $this->lookup_table_name, ARRAY_N ); + + $expected = array( array( '1', '1', 'pa_foo', '10', '0', '1' ) ); + + // Differences: + // Creation or update: the product is stored as having stock. + // None: the product remains stored as not having stock. + if ( 'creation' === $expected_action || 'update' === $expected_action ) { + $expected[] = array( '2', '2', 'pa_bar', '20', '0', '1' ); + } elseif ( 'none' === $expected_action ) { + $expected[] = array( '2', '2', 'pa_bar', '20', '0', '0' ); + } + + $this->assertEquals( $expected, $rows ); + } + + /** + * @testdox 'on_product_changed' creates, updates deletes the data for a variable product and if needed its variations depending on the changeset when the "direct updates" option is on. + * + * @dataProvider data_provider_for_test_on_product_changed_with_direct_updates + * + * @param array $changeset The changeset to test. + * @param string $expected_action The expected performed action, one of 'none', 'creation', 'update' or 'deletion'. + */ + public function test_on_variable_product_changed_for_variable_product_with_direct_updates( $changeset, $expected_action ) { + global $wpdb; + + $this->set_direct_update_option( true ); + + $product = new \WC_Product_Variable(); + $product->set_id( 2 ); + $this->set_product_attributes( + $product, + array( + 'non-variation-attribute' => array( + 'id' => 100, + 'options' => array( 10 ), + ), + 'variation-attribute' => array( + 'id' => 200, + 'options' => array( 20 ), + 'variation' => true, + ), + ) + ); + $product->set_stock_status( 'instock' ); + + $variation = new \WC_Product_Variation(); + $variation->set_id( 3 ); + $variation->set_attributes( + array( + 'variation-attribute' => 'term_20', + ) + ); + $variation->set_stock_status( 'instock' ); + $variation->set_parent_id( 2 ); + + $product->set_children( array( 3 ) ); + + $this->register_legacy_proxy_function_mocks( + array( + 'get_terms' => function( $args ) { + switch ( $args['taxonomy'] ) { + case 'non-variation-attribute': + return array( + 10 => 'term_10', + ); + case 'variation-attribute': + return array( + 20 => 'term_20', + ); + default: + throw new \Exception( "Unexpected call to 'get_terms'" ); + } + }, + 'wc_get_product' => function( $id ) use ( $product, $variation ) { + if ( $id === $product->get_id() || $id === $product ) { + return $product; + } elseif ( $id === $variation->get_id() || $id === $variation ) { + return $variation; + } else { + return wc_get_product( $id ); + } + }, + ) + ); + + $this->insert_lookup_table_data( 1, 1, 'pa_foo', 10, false, true ); + if ( 'creation' !== $expected_action ) { + $this->insert_lookup_table_data( 2, 2, 'non-variation-attribute', 10, false, false ); + $this->insert_lookup_table_data( 3, 2, 'variation-attribute', 20, true, false ); + } + + $this->sut->on_product_changed( $product, $changeset ); + + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + $rows = $wpdb->get_results( 'SELECT * FROM ' . $this->lookup_table_name, ARRAY_N ); + + $expected = array( array( '1', '1', 'pa_foo', '10', '0', '1' ) ); + + // Differences: + // Creation: both main product and variation are stored as having stock. + // Update: main product only is updated as having stock (variation is supposed to get a separate update). + // None: both main product and variation are still stored as not having stock. + if ( 'creation' === $expected_action ) { + $expected[] = array( '2', '2', 'non-variation-attribute', '10', '0', '1' ); + $expected[] = array( '3', '2', 'variation-attribute', '20', '1', '1' ); + } elseif ( 'update' === $expected_action ) { + $expected[] = array( '2', '2', 'non-variation-attribute', '10', '0', '1' ); + $expected[] = array( '3', '2', 'variation-attribute', '20', '1', '0' ); + } elseif ( 'none' === $expected_action ) { + $expected[] = array( '2', '2', 'non-variation-attribute', '10', '0', '0' ); + $expected[] = array( '3', '2', 'variation-attribute', '20', '1', '0' ); + } + + $this->assertEquals( $expected, $rows ); + } + + /** + * @testdox 'on_product_changed' creates, updates deletes the data for a variation depending on the changeset when the "direct updates" option is on. + * + * @dataProvider data_provider_for_test_on_product_changed_with_direct_updates + * + * @param array $changeset The changeset to test. + * @param string $expected_action The expected performed action, one of 'none', 'creation', 'update' or 'deletion'. + */ + public function test_on_variation_changed_for_variable_product_with_direct_updates( $changeset, $expected_action ) { + global $wpdb; + + $this->set_direct_update_option( true ); + + $product = new \WC_Product_Variable(); + $product->set_id( 2 ); + $this->set_product_attributes( + $product, + array( + 'non-variation-attribute' => array( + 'id' => 100, + 'options' => array( 10 ), + ), + 'variation-attribute' => array( + 'id' => 200, + 'options' => array( 20 ), + 'variation' => true, + ), + ) + ); + $product->set_stock_status( 'instock' ); + + $variation = new \WC_Product_Variation(); + $variation->set_id( 3 ); + $variation->set_attributes( + array( + 'variation-attribute' => 'term_20', + ) + ); + $variation->set_stock_status( 'instock' ); + $variation->set_parent_id( 2 ); + + $product->set_children( array( 3 ) ); + + $this->register_legacy_proxy_function_mocks( + array( + 'get_terms' => function( $args ) { + switch ( $args['taxonomy'] ) { + case 'non-variation-attribute': + return array( + 10 => 'term_10', + ); + case 'variation-attribute': + return array( + 20 => 'term_20', + ); + default: + throw new \Exception( "Unexpected call to 'get_terms'" ); + } + }, + 'wc_get_product' => function( $id ) use ( $product, $variation ) { + if ( $id === $product->get_id() || $id === $product ) { + return $product; + } elseif ( $id === $variation->get_id() || $id === $variation ) { + return $variation; + } else { + return wc_get_product( $id ); + } + }, + ) + ); + + $this->insert_lookup_table_data( 1, 1, 'pa_foo', 10, false, true ); + if ( 'creation' !== $expected_action ) { + $this->insert_lookup_table_data( 3, 2, 'variation-attribute', 20, true, false ); + } + + $this->sut->on_product_changed( $variation, $changeset ); + + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + $rows = $wpdb->get_results( 'SELECT * FROM ' . $this->lookup_table_name, ARRAY_N ); + + $expected = array( array( '1', '1', 'pa_foo', '10', '0', '1' ) ); + + // Differences: + // Creation or update: the variation is stored as having stock. + // None: the variation is still stored as not having stock. + if ( 'creation' === $expected_action || 'update' === $expected_action ) { + $expected[] = array( '3', '2', 'variation-attribute', '20', '1', '1' ); + } elseif ( 'none' === $expected_action ) { + $expected[] = array( '3', '2', 'variation-attribute', '20', '1', '0' ); + } + + $this->assertEquals( $expected, $rows ); + } + /** * Set the product attributes from an array with this format: * @@ -747,8 +1166,8 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { ); // phpcs:enable WordPress.DB.PreparedSQL.NotPrepared - $queue = WC()->get_instance_of( \WC_Queue::class ); - $queue->methods_called = array(); + $queue = WC()->get_instance_of( \WC_Queue::class ); + $queue->clear_methods_called(); } /** From cf7d96d867cce6670dd8a1d1a3c21b990ef686a6 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Wed, 30 Jun 2021 10:38:20 +0200 Subject: [PATCH 12/25] Add a lookup data regeneration for single product mechanism. A new "Lookup data" metabox has been added to the product page with a "Renegerate" button that regenerates the attributes lookup data for that product. --- .../DataRegenerator.php | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/src/Internal/ProductAttributesLookup/DataRegenerator.php b/src/Internal/ProductAttributesLookup/DataRegenerator.php index 88f0539d799..b466832e849 100644 --- a/src/Internal/ProductAttributesLookup/DataRegenerator.php +++ b/src/Internal/ProductAttributesLookup/DataRegenerator.php @@ -6,6 +6,7 @@ namespace Automattic\WooCommerce\Internal\ProductAttributesLookup; use Automattic\WooCommerce\Internal\ProductAttributesLookup\LookupDataStore; +use Automattic\WooCommerce\Utilities\ArrayUtil; defined( 'ABSPATH' ) || exit; @@ -66,6 +67,23 @@ class DataRegenerator { $this->run_regeneration_step_callback(); } ); + + add_action( + 'add_meta_boxes', + function() { + $this->add_product_regeneration_metabox(); + }, + 999 + ); + + add_action( + 'save_post_product', + function( $product_id ) { + $this->on_save_product( $product_id ); + }, + 999, + 1 + ); } /** @@ -395,4 +413,60 @@ CREATE TABLE ' . $this->lookup_table_name . '( update_option( 'woocommerce_attribute_lookup__enabled', $enable ? 'yes' : 'no' ); } + + /** + * Add a metabox in the product page with a button to regenerate the product attributes lookup data for the product. + */ + private function add_product_regeneration_metabox() { + if ( ! $this->data_store->is_feature_visible() ) { + return; + } + + add_meta_box( + 'woocommerce-product-foobars', + __( 'Lookup data', 'woocommerce' ), + function() { + $this->metabox_output(); + }, + 'product', + 'side', + 'low' + ); + } + + /** + * HTML output for the lookup data regeneration metabox. + */ + private function metabox_output() { + // phpcs:disable WordPress.Security.EscapeOutput.OutputNotEscaped + wp_nonce_field( 'regenerate-attributes-lookup-data', '_wc_regenerate_attributes_lookup_data_nonce' ); + ?> +

+ + data_store->is_feature_visible() || 'regenerate-attributes-lookup-data' !== ArrayUtil::get_value_or_default( $_POST, 'woocommerce-product-lookup-action' ) ) { + return; + } + + if ( ! wc_get_product( $product_id ) ) { + return; + } + + $this->data_store->create_data_for_product( $product_id ); + } } From b97e792f40f98647fa98426c85c4762adcdaa19f Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Wed, 30 Jun 2021 15:14:14 +0200 Subject: [PATCH 13/25] Move the UI to set/unset product attribute lookup table usage to settings. The UI was previously in the tools page, it's now a setting under products - advanced. --- ...ProductAttributesLookupServiceProvider.php | 3 +- .../DataRegenerator.php | 64 +++-------------- .../LookupDataStore.php | 71 +++++++++++++++---- 3 files changed, 65 insertions(+), 73 deletions(-) diff --git a/src/Internal/DependencyManagement/ServiceProviders/ProductAttributesLookupServiceProvider.php b/src/Internal/DependencyManagement/ServiceProviders/ProductAttributesLookupServiceProvider.php index 31fba73fe63..173621078d6 100644 --- a/src/Internal/DependencyManagement/ServiceProviders/ProductAttributesLookupServiceProvider.php +++ b/src/Internal/DependencyManagement/ServiceProviders/ProductAttributesLookupServiceProvider.php @@ -9,7 +9,6 @@ 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. @@ -32,7 +31,7 @@ class ProductAttributesLookupServiceProvider extends AbstractServiceProvider { */ public function register() { $this->share( DataRegenerator::class )->addArgument( LookupDataStore::class ); - $this->share( Filterer::class )->addArgument( LookupDataStore::class )->addArgument( LegacyProxy::class ); + $this->share( Filterer::class )->addArgument( LookupDataStore::class ); $this->share( LookupDataStore::class ); } } diff --git a/src/Internal/ProductAttributesLookup/DataRegenerator.php b/src/Internal/ProductAttributesLookup/DataRegenerator.php index b466832e849..f47a1e1836d 100644 --- a/src/Internal/ProductAttributesLookup/DataRegenerator.php +++ b/src/Internal/ProductAttributesLookup/DataRegenerator.php @@ -122,15 +122,6 @@ class DataRegenerator { } } - /** - * Tells if a regeneration is already in progress. - * - * @return bool True if a regeneration is already in progress. - */ - public function regeneration_is_in_progress() { - return ! is_null( get_option( 'woocommerce_attribute_lookup__last_products_page_processed', null ) ); - } - /** * Delete all the existing data related to the lookup table, including the table itself. * @@ -144,6 +135,7 @@ class DataRegenerator { delete_option( 'woocommerce_attribute_lookup__enabled' ); delete_option( 'woocommerce_attribute_lookup__last_product_id_to_process' ); delete_option( 'woocommerce_attribute_lookup__last_products_page_processed' ); + $this->data_store->unset_regeneration_in_progress_flag(); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared $wpdb->query( 'DROP TABLE IF EXISTS ' . $this->lookup_table_name ); @@ -190,6 +182,7 @@ CREATE TABLE ' . $this->lookup_table_name . '( return false; } + $this->data_store->set_regeneration_in_progress_flag(); update_option( 'woocommerce_attribute_lookup__last_product_id_to_process', current( $last_existing_product_id ) ); update_option( 'woocommerce_attribute_lookup__last_products_page_processed', 0 ); @@ -201,7 +194,7 @@ CREATE TABLE ' . $this->lookup_table_name . '( * schedules the next step if necessary. */ private function run_regeneration_step_callback() { - if ( ! $this->regeneration_is_in_progress() ) { + if ( ! $this->data_store->regeneration_is_in_progress() ) { return; } @@ -269,19 +262,7 @@ CREATE TABLE ' . $this->lookup_table_name . '( delete_option( 'woocommerce_attribute_lookup__last_product_id_to_process' ); delete_option( 'woocommerce_attribute_lookup__last_products_page_processed' ); update_option( 'woocommerce_attribute_lookup__enabled', 'no' ); - } - - /** - * Check if the lookup table exists in the database. - * - * @return bool True if the lookup table exists in the database. - */ - private function lookup_table_exists() { - global $wpdb; - $query = $wpdb->prepare( 'SHOW TABLES LIKE %s', $wpdb->esc_like( $this->lookup_table_name ) ); - - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared - return $this->lookup_table_name === $wpdb->get_var( $query ); + $this->data_store->unset_regeneration_in_progress_flag(); } /** @@ -295,8 +276,8 @@ CREATE TABLE ' . $this->lookup_table_name . '( return $tools_array; } - $lookup_table_exists = $this->lookup_table_exists(); - $generation_is_in_progress = $this->regeneration_is_in_progress(); + $lookup_table_exists = $this->data_store->check_lookup_table_exists(); + $generation_is_in_progress = $this->data_store->regeneration_is_in_progress(); // Regenerate table. @@ -355,35 +336,6 @@ CREATE TABLE ' . $this->lookup_table_name . '( ); } - if ( $lookup_table_exists && ! $generation_is_in_progress ) { - - // Enable or disable table usage. - - if ( 'yes' === get_option( 'woocommerce_attribute_lookup__enabled' ) ) { - $tools_array['disable_product_attributes_lookup_table_usage'] = array( - 'name' => __( 'Disable the product attributes lookup table usage', 'woocommerce' ), - 'desc' => __( 'The product attributes lookup table usage is currently enabled, use this tool to disable it.', 'woocommerce' ), - 'button' => __( 'Disable', 'woocommerce' ), - 'requires_refresh' => true, - 'callback' => function () { - $this->enable_or_disable_lookup_table_usage( false ); - return __( 'Product attributes lookup table usage has been disabled.', 'woocommerce' ); - }, - ); - } else { - $tools_array['enable_product_attributes_lookup_table_usage'] = array( - 'name' => __( 'Enable the product attributes lookup table usage', 'woocommerce' ), - 'desc' => __( 'The product attributes lookup table usage is currently disabled, use this tool to enable it.', 'woocommerce' ), - 'button' => __( 'Enable', 'woocommerce' ), - 'requires_refresh' => true, - 'callback' => function () { - $this->enable_or_disable_lookup_table_usage( true ); - return __( 'Product attributes lookup table usage has been enabled.', 'woocommerce' ); - }, - ); - } - } - return $tools_array; } @@ -393,7 +345,7 @@ CREATE TABLE ' . $this->lookup_table_name . '( * @throws \Exception The regeneration is already in progress. */ private function initiate_regeneration_from_tools_page() { - if ( $this->regeneration_is_in_progress() ) { + if ( $this->data_store->regeneration_is_in_progress() ) { throw new \Exception( 'Product attributes lookup table is already regenerating.' ); } @@ -407,7 +359,7 @@ CREATE TABLE ' . $this->lookup_table_name . '( * @throws \Exception A lookup table regeneration is currently in progress. */ private function enable_or_disable_lookup_table_usage( $enable ) { - if ( $this->regeneration_is_in_progress() ) { + if ( $this->data_store->regeneration_is_in_progress() ) { throw new \Exception( "Can't enable or disable the attributes lookup table usage while it's regenerating." ); } diff --git a/src/Internal/ProductAttributesLookup/LookupDataStore.php b/src/Internal/ProductAttributesLookup/LookupDataStore.php index c57956b7275..bf82cda15e5 100644 --- a/src/Internal/ProductAttributesLookup/LookupDataStore.php +++ b/src/Internal/ProductAttributesLookup/LookupDataStore.php @@ -76,7 +76,7 @@ class LookupDataStore { add_filter( 'woocommerce_get_sections_products', function ( $products ) { - if ( $this->is_feature_visible() ) { + if ( $this->is_feature_visible() && $this->check_lookup_table_exists() ) { $products['advanced'] = __( 'Advanced', 'woocommerce' ); } return $products; @@ -88,21 +88,39 @@ class LookupDataStore { add_filter( 'woocommerce_get_settings_products', function ( $settings, $section_id ) { - if ( 'advanced' === $section_id && $this->is_feature_visible() ) { - $settings[] = - array( - 'title' => __( 'Product attributes lookup table', 'woocommerce' ), - 'type' => 'title', + if ( 'advanced' === $section_id && $this->is_feature_visible() && $this->check_lookup_table_exists() ) { + $title_item = array( + 'title' => __( 'Product attributes lookup table', 'woocommerce' ), + 'type' => 'title', + ); + + $regeneration_is_in_progress = $this->regeneration_is_in_progress(); + + if ( $regeneration_is_in_progress ) { + $title_item['desc'] = __( 'These settings are not available while the lookup table regeneration is in progress.', 'woocommerce' ); + } + + $settings[] = $title_item; + + if ( ! $regeneration_is_in_progress ) { + $settings[] = array( + 'title' => __( 'Enable table usage', 'woocommerce' ), + 'desc' => __( 'Use the product attributes lookup table for catalog filtering.', 'woocommerce' ), + 'id' => 'woocommerce_attribute_lookup__enable', + 'default' => 'no', + 'type' => 'checkbox', + 'checkboxgroup' => 'start', ); - $settings[] = array( - 'title' => __( 'Direct updates', 'woocommerce' ), - 'desc' => __( 'Update the table directly upon product changes, instead of scheduling a deferred update.', 'woocommerce' ), - 'id' => 'woocommerce_attribute_lookup__direct_updates', - 'default' => 'no', - 'type' => 'checkbox', - 'checkboxgroup' => 'start', - ); + $settings[] = array( + 'title' => __( 'Direct updates', 'woocommerce' ), + 'desc' => __( 'Update the table directly upon product changes, instead of scheduling a deferred update.', 'woocommerce' ), + 'id' => 'woocommerce_attribute_lookup__direct_updates', + 'default' => 'no', + 'type' => 'checkbox', + 'checkboxgroup' => 'start', + ); + } $settings[] = array( 'type' => 'sectionend' ); } @@ -132,7 +150,7 @@ AND table_name = %s;', ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared - return 0 !== $wpdb->get_var( $query ); + return (bool) $wpdb->get_var( $query ); } /** @@ -655,4 +673,27 @@ AND table_name = %s;', ); // phpcs:enable WordPress.DB.PreparedSQL.NotPrepared } + + /** + * Tells if a lookup table regeneration is currently in progress. + * + * @return bool True if a lookup table regeneration is already in progress. + */ + public function regeneration_is_in_progress() { + return 'yes' === get_option( 'woocommerce_attribute_lookup__regeneration_in_progress', null ); + } + + /** + * Set a permanent flag (via option) indicating that the lookup table regeneration is in process. + */ + public function set_regeneration_in_progress_flag() { + update_option( 'woocommerce_attribute_lookup__regeneration_in_progress', 'yes' ); + } + + /** + * Remove the flag indicating that the lookup table regeneration is in process. + */ + public function unset_regeneration_in_progress_flag() { + delete_option( 'woocommerce_attribute_lookup__regeneration_in_progress' ); + } } From d3131a67e640b600dcea987d7acfcec925983fec Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Thu, 1 Jul 2021 09:32:57 +0200 Subject: [PATCH 14/25] Fix unit tests for the DataRegenerator class --- .../ProductAttributesLookup/DataRegeneratorTest.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php b/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php index 2699cc81537..a96bc96c0da 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/DataRegeneratorTest.php @@ -131,7 +131,7 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { 'hook' => 'woocommerce_run_product_attribute_lookup_regeneration_callback', 'group' => 'woocommerce-db-updates', ); - $actual_enqueued = current( $this->queue->methods_called ); + $actual_enqueued = current( $this->queue->get_methods_called() ); $this->assertEquals( sort( $expected_enqueued ), sort( $actual_enqueued ) ); } @@ -158,7 +158,7 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { $this->assertFalse( get_option( 'woocommerce_attribute_lookup__last_product_id_to_process' ) ); $this->assertFalse( get_option( 'woocommerce_attribute_lookup__last_products_page_processed' ) ); $this->assertEquals( 'no', get_option( 'woocommerce_attribute_lookup__enabled' ) ); - $this->assertEmpty( $this->queue->methods_called ); + $this->assertEmpty( $this->queue->get_methods_called() ); } /** @@ -184,7 +184,7 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { ); $this->sut->initiate_regeneration(); - $this->queue->methods_called = array(); + $this->queue->clear_methods_called(); update_option( 'woocommerce_attribute_lookup__last_products_page_processed', 7 ); @@ -201,7 +201,7 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { 'hook' => 'woocommerce_run_product_attribute_lookup_regeneration_callback', 'group' => 'woocommerce-db-updates', ); - $actual_enqueued = current( $this->queue->methods_called ); + $actual_enqueued = current( $this->queue->get_methods_called() ); $this->assertEquals( sort( $expected_enqueued ), sort( $actual_enqueued ) ); } @@ -231,7 +231,7 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { ); $this->sut->initiate_regeneration(); - $this->queue->methods_called = array(); + $this->queue->clear_methods_called(); do_action( 'woocommerce_run_product_attribute_lookup_regeneration_callback' ); @@ -239,6 +239,6 @@ class DataRegeneratorTest extends \WC_Unit_Test_Case { $this->assertFalse( get_option( 'woocommerce_attribute_lookup__last_product_id_to_process' ) ); $this->assertFalse( get_option( 'woocommerce_attribute_lookup__last_products_page_processed' ) ); $this->assertEquals( 'no', get_option( 'woocommerce_attribute_lookup__enabled' ) ); - $this->assertEmpty( $this->queue->methods_called ); + $this->assertEmpty( $this->queue->get_methods_called() ); } } From 2fe8cce9b0a2f9287120cc3dbe07aaed79dbfc26 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Thu, 1 Jul 2021 09:57:44 +0200 Subject: [PATCH 15/25] Fix unit tests for the LookupDataStore class --- .../LookupDataStore.php | 17 ++--------- .../LookupDataStoreTest.php | 28 +++++++++++-------- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/src/Internal/ProductAttributesLookup/LookupDataStore.php b/src/Internal/ProductAttributesLookup/LookupDataStore.php index bf82cda15e5..42471a7249d 100644 --- a/src/Internal/ProductAttributesLookup/LookupDataStore.php +++ b/src/Internal/ProductAttributesLookup/LookupDataStore.php @@ -37,15 +37,6 @@ class LookupDataStore { */ private $is_feature_visible; - /** - * Does the lookup table exist? - * - * TODO: Remove once the lookup table is created via data migration. - * - * @var bool - */ - private $lookup_table_exists; - /** * LookupDataStore constructor. Makes the feature hidden by default. */ @@ -55,8 +46,6 @@ class LookupDataStore { $this->lookup_table_name = $wpdb->prefix . 'wc_product_attributes_lookup'; $this->is_feature_visible = false; - $this->lookup_table_exists = $this->check_lookup_table_exists(); - $this->init_hooks(); } @@ -194,7 +183,7 @@ AND table_name = %s;', * @param null|array $changeset Changes as provided by 'get_changes' method in the product object, null if it's being created. */ public function on_product_changed( $product, $changeset = null ) { - if ( ! $this->lookup_table_exists ) { + if ( ! $this->check_lookup_table_exists() ) { return; } @@ -253,7 +242,7 @@ AND table_name = %s;', * @param int $action The action to perform, one of the ACTION_ constants. */ private function run_update_callback( int $product_id, int $action ) { - if ( ! $this->lookup_table_exists ) { + if ( ! $this->check_lookup_table_exists() ) { return; } @@ -346,7 +335,7 @@ AND table_name = %s;', * @param int|\WC_Product $product Product object or product id. */ public function on_product_deleted( $product ) { - if ( ! $this->lookup_table_exists ) { + if ( ! $this->check_lookup_table_exists() ) { return; } diff --git a/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php b/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php index 5fc418dfff7..3f79beb72f4 100644 --- a/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php +++ b/tests/php/src/Internal/ProductAttributesLookup/LookupDataStoreTest.php @@ -31,6 +31,14 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { */ private $lookup_table_name; + /** + * Runs after all the tests in the class. + */ + public static function tearDownAfterClass() { + parent::tearDownAfterClass(); + wc_get_container()->get( DataRegenerator::class )->delete_all_attributes_lookup_data(); + } + /** * Runs before each test. */ @@ -40,19 +48,15 @@ class LookupDataStoreTest extends \WC_Unit_Test_Case { $this->lookup_table_name = $wpdb->prefix . 'wc_product_attributes_lookup'; $this->sut = new LookupDataStore(); - // Initiating regeneration with a fake queue will just create the lookup table in the database. - add_filter( - 'woocommerce_queue_class', - function() { - return FakeQueue::class; - } - ); - $this->get_instance_of( DataRegenerator::class )->initiate_regeneration(); - - $queue = WC()->get_instance_of( \WC_Queue::class ); - $queue->clear_methods_called(); - $this->reset_legacy_proxy_mocks(); + $this->register_legacy_proxy_class_mocks( + array( + \WC_Queue::class => new FakeQueue(), + ) + ); + + // Initiating regeneration with a fake queue will just create the lookup table in the database. + $this->get_instance_of( DataRegenerator::class )->initiate_regeneration(); } /** From 70431ead2bd029e056b79d3854aa83b658e0cca6 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Thu, 1 Jul 2021 12:50:30 +0200 Subject: [PATCH 16/25] Small adjustments to the per-product lookup data regeneration metabox. --- .../DataRegenerator.php | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/Internal/ProductAttributesLookup/DataRegenerator.php b/src/Internal/ProductAttributesLookup/DataRegenerator.php index f47a1e1836d..2b0550057cb 100644 --- a/src/Internal/ProductAttributesLookup/DataRegenerator.php +++ b/src/Internal/ProductAttributesLookup/DataRegenerator.php @@ -370,7 +370,7 @@ CREATE TABLE ' . $this->lookup_table_name . '( * Add a metabox in the product page with a button to regenerate the product attributes lookup data for the product. */ private function add_product_regeneration_metabox() { - if ( ! $this->data_store->is_feature_visible() ) { + if ( ! $this->can_do_per_product_regeneration() ) { return; } @@ -407,18 +407,24 @@ CREATE TABLE ' . $this->lookup_table_name . '( * @param int $product_id The product id. */ private function on_save_product( int $product_id ) { - if ( ! wp_verify_nonce( ArrayUtil::get_value_or_default( $_POST, '_wc_regenerate_attributes_lookup_data_nonce' ), 'regenerate-attributes-lookup-data' ) ) { - return; - } - - if ( ! $this->data_store->is_feature_visible() || 'regenerate-attributes-lookup-data' !== ArrayUtil::get_value_or_default( $_POST, 'woocommerce-product-lookup-action' ) ) { - return; - } - - if ( ! wc_get_product( $product_id ) ) { + if ( + ! wp_verify_nonce( ArrayUtil::get_value_or_default( $_POST, '_wc_regenerate_attributes_lookup_data_nonce' ), 'regenerate-attributes-lookup-data' ) || + ! $this->can_do_per_product_regeneration() || + 'regenerate-attributes-lookup-data' !== ArrayUtil::get_value_or_default( $_POST, 'woocommerce-product-lookup-action' ) || + ! wc_get_product( $product_id ) + ) { return; } $this->data_store->create_data_for_product( $product_id ); } + + /** + * Check if everything is good to go to perform a per product lookup table data regeneration. + * + * @return bool True if per product lookup table data regeneration can be performed. + */ + private function can_do_per_product_regeneration() { + return $this->data_store->is_feature_visible() && $this->data_store->check_lookup_table_exists() && ! $this->data_store->regeneration_is_in_progress(); + } } From ce1c0dcd5709d0e8a7550ecd6b9efd72d83bbef6 Mon Sep 17 00:00:00 2001 From: "Michael P. Pfeiffer" Date: Tue, 6 Jul 2021 13:14:14 +0200 Subject: [PATCH 17/25] Update WooCommerce Blocks package to 5.5.0 --- composer.json | 2 +- composer.lock | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/composer.json b/composer.json index 79d23dbc607..54017c2962d 100644 --- a/composer.json +++ b/composer.json @@ -22,7 +22,7 @@ "psr/container": "1.0.0", "woocommerce/action-scheduler": "3.2.1", "woocommerce/woocommerce-admin": "2.4.1", - "woocommerce/woocommerce-blocks": "5.3.1" + "woocommerce/woocommerce-blocks": "5.5.0" }, "require-dev": { "bamarni/composer-bin-plugin": "^1.4" diff --git a/composer.lock b/composer.lock index c25864dd87f..80c020a0bb2 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "78971f2035da15d44d3941df88d5afbc", + "content-hash": "6b6ffc4afddcbe536297cd42e497d82a", "packages": [ { "name": "automattic/jetpack-autoloader", @@ -584,16 +584,16 @@ }, { "name": "woocommerce/woocommerce-blocks", - "version": "v5.3.1", + "version": "v5.5.0", "source": { "type": "git", "url": "https://github.com/woocommerce/woocommerce-gutenberg-products-block.git", - "reference": "28c7c4f9b5cace9098fb2246ff93abe110a26bca" + "reference": "4ec2a9bfbae319342ee0303b1dece41d341b71f8" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/woocommerce/woocommerce-gutenberg-products-block/zipball/28c7c4f9b5cace9098fb2246ff93abe110a26bca", - "reference": "28c7c4f9b5cace9098fb2246ff93abe110a26bca", + "url": "https://api.github.com/repos/woocommerce/woocommerce-gutenberg-products-block/zipball/4ec2a9bfbae319342ee0303b1dece41d341b71f8", + "reference": "4ec2a9bfbae319342ee0303b1dece41d341b71f8", "shasum": "" }, "require": { @@ -629,9 +629,9 @@ ], "support": { "issues": "https://github.com/woocommerce/woocommerce-gutenberg-products-block/issues", - "source": "https://github.com/woocommerce/woocommerce-gutenberg-products-block/tree/v5.3.1" + "source": "https://github.com/woocommerce/woocommerce-gutenberg-products-block/tree/v5.5.0" }, - "time": "2021-06-15T09:12:48+00:00" + "time": "2021-07-06T07:22:58+00:00" } ], "packages-dev": [ @@ -698,5 +698,5 @@ "platform-overrides": { "php": "7.0" }, - "plugin-api-version": "2.0.0" + "plugin-api-version": "2.1.0" } From 4b4f9a2b5d23f2df7a8ad6c78d8eb6c6db21b625 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Mon, 12 Jul 2021 10:23:59 +0200 Subject: [PATCH 18/25] Add missing JOIN parts from tax meta and meta queries. The SQL query used to calculate the product counts using the product attributes lookup table for the attribute filtering widget was missing the JOIN parts coming from the tax meta and meta queries. The WHERE parts however were being used, so the enitre query could fail if there was a tax or meta query in place. --- src/Internal/ProductAttributesLookup/Filterer.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Internal/ProductAttributesLookup/Filterer.php b/src/Internal/ProductAttributesLookup/Filterer.php index a42e3d804fe..4971a5b1d3c 100644 --- a/src/Internal/ProductAttributesLookup/Filterer.php +++ b/src/Internal/ProductAttributesLookup/Filterer.php @@ -196,7 +196,9 @@ class Filterer { $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"; + $query['join'] = " + {$tax_query_sql['join']} {$meta_query_sql['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'] = " From aa967cab4f4e90616aca4d2cb4f450579d8cc21a Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Mon, 12 Jul 2021 10:32:14 +0200 Subject: [PATCH 19/25] Optimize the query to count products using the attributes lookup table. The query to count products using the attributes lookup table for the filter by attribute widget was adding an "AND term_ids in (...)"-type subquery for each attribute participating in the filtering. Now at most two such subqueries are generated, one for attributes configured for OR type filtering and another one for the ones configured as AND; this speeds up the query significantly when many attributes are used simultaneously for the filtering. --- .../ProductAttributesLookup/Filterer.php | 75 +++++++++++-------- 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/src/Internal/ProductAttributesLookup/Filterer.php b/src/Internal/ProductAttributesLookup/Filterer.php index 4971a5b1d3c..6f34dc02b7a 100644 --- a/src/Internal/ProductAttributesLookup/Filterer.php +++ b/src/Internal/ProductAttributesLookup/Filterer.php @@ -213,42 +213,51 @@ class Filterer { $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 ) . ')'; + $and_term_ids = array(); + $or_term_ids = array(); - $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} - )"; - } + 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'] ) ) ); + if ( 'and' === $data['query_type'] ) { + $and_term_ids = array_merge( $and_term_ids, $term_ids_to_filter_by ); + } else { + $or_term_ids = array_merge( $or_term_ids, $term_ids_to_filter_by ); } } + + if ( ! empty( $and_term_ids ) ) { + $terms_count = count( $and_term_ids ); + $term_ids_list = '(' . join( ',', $and_term_ids ) . ')'; + $query['where'] .= " + AND product_or_parent_id IN ( + 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_list} + GROUP BY product_id + HAVING COUNT(product_id)={$terms_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_list} + )"; + } + + if ( ! empty( $or_term_ids ) ) { + $term_ids_list = '(' . join( ',', $or_term_ids ) . ')'; + $query['where'] .= " + AND product_or_parent_id IN ( + SELECT product_or_parent_id FROM {$this->lookup_table_name} + WHERE term_id in {$term_ids_list} + {$in_stock_clause} + )"; + + } } else { $query['where'] .= $in_stock_clause; } From 9017c953bd72dc076342e56026167a2bae599b53 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Wed, 14 Jul 2021 12:01:42 +0200 Subject: [PATCH 20/25] Remove the metabox for regenerating attribute lookup data for single product Instead, a product selector has been added to the "Regenerate product attributes lookup table" entry in the tools page. If a product is selected, the tool regenerates the data only for that product; otherwise, it regenerates the entire table. This has forced a change on how the tools page is rendered. Now, instead of each tool being just a description and a trigger link, a form with GET method is rendered for each tool. The forms are rendered first and then the tools, since HTML doesn't allow to include forms inside tables; each button is associated to its form with a "form" attribute. Additionally, now the tools array returned by the woocommerce_debug_tools` hook can have a 'selector' array with the details needed to render a selector, which will also be part of the form for the tool. --- .../views/html-admin-page-status-tools.php | 60 ++++++--- .../DataRegenerator.php | 115 ++++++------------ 2 files changed, 80 insertions(+), 95 deletions(-) diff --git a/includes/admin/views/html-admin-page-status-tools.php b/includes/admin/views/html-admin-page-status-tools.php index b6d9dacaafc..34680ef827b 100644 --- a/includes/admin/views/html-admin-page-status-tools.php +++ b/includes/admin/views/html-admin-page-status-tools.php @@ -11,22 +11,46 @@ if ( ! defined( 'ABSPATH' ) ) { exit; } +foreach ( $tools as $action_name => $tool ) { + // phpcs:disable WordPress.Security.EscapeOutput.OutputNotEscaped + ?> +
+ + + + +
+ -
- - - - $tool ) : ?> - - - - - - -
- -

- 		- href="" class="button button-large "> -
-
+ + + + $tool ) : ?> + + + + + + +
+ +

+

'; + echo wp_kses_post( $selector['description'] ); + } + // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + echo "  "; + } + ?> +

+ 		+ + type="submit" form="" class="button button-large" value="" /> +
diff --git a/src/Internal/ProductAttributesLookup/DataRegenerator.php b/src/Internal/ProductAttributesLookup/DataRegenerator.php index 2b0550057cb..6eb52ada465 100644 --- a/src/Internal/ProductAttributesLookup/DataRegenerator.php +++ b/src/Internal/ProductAttributesLookup/DataRegenerator.php @@ -67,23 +67,6 @@ class DataRegenerator { $this->run_regeneration_step_callback(); } ); - - add_action( - 'add_meta_boxes', - function() { - $this->add_product_regeneration_metabox(); - }, - 999 - ); - - add_action( - 'save_post_product', - function( $product_id ) { - $this->on_save_product( $product_id ); - }, - 999, - 1 - ); } /** @@ -283,7 +266,7 @@ CREATE TABLE ' . $this->lookup_table_name . '( if ( $lookup_table_exists ) { $generate_item_name = __( 'Regenerate the product attributes lookup table', 'woocommerce' ); - $generate_item_desc = __( 'This tool will regenerate the product attributes lookup table data from existing products data. This process may take a while.', 'woocommerce' ); + $generate_item_desc = __( 'This tool will regenerate the product attributes lookup table data from existing product(s) data. This process may take a while.', 'woocommerce' ); $generate_item_return = __( 'Product attributes lookup table data is regenerating', 'woocommerce' ); $generate_item_button = __( 'Regenerate', 'woocommerce' ); } else { @@ -303,6 +286,16 @@ CREATE TABLE ' . $this->lookup_table_name . '( }, ); + if ( $lookup_table_exists ) { + $entry['selector'] = array( + 'description' => __( 'Select a product to regenerate the data for, or leave empty for a full table regeneration:', 'woocommerce' ), + 'class' => 'wc-product-search', + 'search_action' => 'woocommerce_json_search_products', + 'name' => 'regenerate_product_attribute_lookup_data_product_id', + 'placeholder' => esc_attr__( 'Search for a product…', 'woocommerce' ), + ); + } + if ( $generation_is_in_progress ) { $entry['button'] = sprintf( /* translators: %d: How many products have been processed so far. */ @@ -345,11 +338,19 @@ CREATE TABLE ' . $this->lookup_table_name . '( * @throws \Exception The regeneration is already in progress. */ private function initiate_regeneration_from_tools_page() { - if ( $this->data_store->regeneration_is_in_progress() ) { - throw new \Exception( 'Product attributes lookup table is already regenerating.' ); + // phpcs:ignore WordPress.Security.ValidatedSanitizedInput + if ( ! isset( $_REQUEST['_wpnonce'] ) || false === wp_verify_nonce( $_REQUEST['_wpnonce'], 'debug_action' ) ) { + throw new \Exception( 'Invalid nonce' ); } - $this->initiate_regeneration(); + if ( isset( $_REQUEST['regenerate_product_attribute_lookup_data_product_id'] ) ) { + $product_id = (int) $_REQUEST['regenerate_product_attribute_lookup_data_product_id']; + $this->check_can_do_lookup_table_regeneration( $product_id ); + $this->data_store->create_data_for_product( $product_id ); + } else { + $this->check_can_do_lookup_table_regeneration(); + $this->initiate_regeneration(); + } } /** @@ -367,64 +368,24 @@ CREATE TABLE ' . $this->lookup_table_name . '( } /** - * Add a metabox in the product page with a button to regenerate the product attributes lookup data for the product. - */ - private function add_product_regeneration_metabox() { - if ( ! $this->can_do_per_product_regeneration() ) { - return; - } - - add_meta_box( - 'woocommerce-product-foobars', - __( 'Lookup data', 'woocommerce' ), - function() { - $this->metabox_output(); - }, - 'product', - 'side', - 'low' - ); - } - - /** - * HTML output for the lookup data regeneration metabox. - */ - private function metabox_output() { - // phpcs:disable WordPress.Security.EscapeOutput.OutputNotEscaped - wp_nonce_field( 'regenerate-attributes-lookup-data', '_wc_regenerate_attributes_lookup_data_nonce' ); - ?> -

- - can_do_per_product_regeneration() || - 'regenerate-attributes-lookup-data' !== ArrayUtil::get_value_or_default( $_POST, 'woocommerce-product-lookup-action' ) || - ! wc_get_product( $product_id ) - ) { - return; + private function check_can_do_lookup_table_regeneration( $product_id = null ) { + if ( ! $this->data_store->is_feature_visible() ) { + throw new \Exception( "Can't do product attribute lookup data regeneration: feature is not visible" ); + } + if ( ! $this->data_store->check_lookup_table_exists() ) { + throw new \Exception( "Can't do product attribute lookup data regeneration: lookup table doesn't exist" ); + } + if ( $this->data_store->regeneration_is_in_progress() ) { + throw new \Exception( "Can't do product attribute lookup data regeneration: regeneration is already in progress" ); + } + if ( $product_id && ! wc_get_product( $product_id ) ) { + throw new \Exception( "Can't do product attribute lookup data regeneration: product doesn't exist" ); } - - $this->data_store->create_data_for_product( $product_id ); - } - - /** - * Check if everything is good to go to perform a per product lookup table data regeneration. - * - * @return bool True if per product lookup table data regeneration can be performed. - */ - private function can_do_per_product_regeneration() { - return $this->data_store->is_feature_visible() && $this->data_store->check_lookup_table_exists() && ! $this->data_store->regeneration_is_in_progress(); } } From aa939f6081d8513701e5156047b1d3f5ec1ce4df Mon Sep 17 00:00:00 2001 From: roykho Date: Wed, 14 Jul 2021 08:45:01 -0700 Subject: [PATCH 21/25] Use single quotes to delineate mysql values instead of double quotes closes #29969 --- includes/data-stores/class-wc-product-data-store-cpt.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/data-stores/class-wc-product-data-store-cpt.php b/includes/data-stores/class-wc-product-data-store-cpt.php index 8a71ed6de96..190749d7db3 100644 --- a/includes/data-stores/class-wc-product-data-store-cpt.php +++ b/includes/data-stores/class-wc-product-data-store-cpt.php @@ -1118,7 +1118,7 @@ class WC_Product_Data_Store_CPT extends WC_Data_Store_WP implements WC_Object_Da $product->get_id() ); - $query .= ' AND postmeta.meta_key IN ( "' . implode( '","', array_map( 'esc_sql', $meta_attribute_names ) ) . '" )'; + $query .= " AND postmeta.meta_key IN ( '" . implode( "','", array_map( 'esc_sql', $meta_attribute_names ) ) . "' )"; $query .= ' ORDER BY posts.menu_order ASC, postmeta.post_id ASC;'; From 24a0551b27515190cb96a1ff7c33b91a1f5a0221 Mon Sep 17 00:00:00 2001 From: Claudio Sanches Date: Thu, 15 Jul 2021 22:37:51 -0300 Subject: [PATCH 22/25] Updated to Blocks 5.5.1 --- composer.json | 2 +- composer.lock | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/composer.json b/composer.json index 54017c2962d..ee94ca040ca 100644 --- a/composer.json +++ b/composer.json @@ -22,7 +22,7 @@ "psr/container": "1.0.0", "woocommerce/action-scheduler": "3.2.1", "woocommerce/woocommerce-admin": "2.4.1", - "woocommerce/woocommerce-blocks": "5.5.0" + "woocommerce/woocommerce-blocks": "5.5.1" }, "require-dev": { "bamarni/composer-bin-plugin": "^1.4" diff --git a/composer.lock b/composer.lock index 80c020a0bb2..472d1eafe05 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "6b6ffc4afddcbe536297cd42e497d82a", + "content-hash": "acab5cd3f2509342ed733e770638ab4c", "packages": [ { "name": "automattic/jetpack-autoloader", @@ -584,16 +584,16 @@ }, { "name": "woocommerce/woocommerce-blocks", - "version": "v5.5.0", + "version": "v5.5.1", "source": { "type": "git", "url": "https://github.com/woocommerce/woocommerce-gutenberg-products-block.git", - "reference": "4ec2a9bfbae319342ee0303b1dece41d341b71f8" + "reference": "f3d8dbadb745cbb2544e86dfb3864e870146d197" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/woocommerce/woocommerce-gutenberg-products-block/zipball/4ec2a9bfbae319342ee0303b1dece41d341b71f8", - "reference": "4ec2a9bfbae319342ee0303b1dece41d341b71f8", + "url": "https://api.github.com/repos/woocommerce/woocommerce-gutenberg-products-block/zipball/f3d8dbadb745cbb2544e86dfb3864e870146d197", + "reference": "f3d8dbadb745cbb2544e86dfb3864e870146d197", "shasum": "" }, "require": { @@ -629,9 +629,9 @@ ], "support": { "issues": "https://github.com/woocommerce/woocommerce-gutenberg-products-block/issues", - "source": "https://github.com/woocommerce/woocommerce-gutenberg-products-block/tree/v5.5.0" + "source": "https://github.com/woocommerce/woocommerce-gutenberg-products-block/tree/v5.5.1" }, - "time": "2021-07-06T07:22:58+00:00" + "time": "2021-07-14T20:59:04+00:00" } ], "packages-dev": [ @@ -698,5 +698,5 @@ "platform-overrides": { "php": "7.0" }, - "plugin-api-version": "2.1.0" + "plugin-api-version": "2.0.0" } From d3bfd33e981ecd93e56567225872ccd23d07e979 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Fri, 16 Jul 2021 09:16:20 +0200 Subject: [PATCH 23/25] Use 'esc_attr' instead of suppressing the PHPCS warning --- includes/admin/views/html-admin-page-status-tools.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/includes/admin/views/html-admin-page-status-tools.php b/includes/admin/views/html-admin-page-status-tools.php index 34680ef827b..31489a6ccdd 100644 --- a/includes/admin/views/html-admin-page-status-tools.php +++ b/includes/admin/views/html-admin-page-status-tools.php @@ -12,16 +12,14 @@ if ( ! defined( 'ABSPATH' ) ) { } foreach ( $tools as $action_name => $tool ) { - // phpcs:disable WordPress.Security.EscapeOutput.OutputNotEscaped ?> -
+ - +
@@ -48,7 +46,7 @@ foreach ( $tools as $action_name => $tool ) { - type="submit" form="" class="button button-large" value="" /> + type="submit" form="" class="button button-large" value="" /> From db44d15e3f220c8a0e0c5fc8ab534ee66f34c25c Mon Sep 17 00:00:00 2001 From: Jonathan Sadowski Date: Fri, 16 Jul 2021 14:59:02 -0500 Subject: [PATCH 24/25] Apply patch to class-wc-webhook-data-store.php to fix sqli issue --- includes/data-stores/class-wc-webhook-data-store.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/data-stores/class-wc-webhook-data-store.php b/includes/data-stores/class-wc-webhook-data-store.php index 7feed6e4cc4..0815e28fe09 100644 --- a/includes/data-stores/class-wc-webhook-data-store.php +++ b/includes/data-stores/class-wc-webhook-data-store.php @@ -277,7 +277,7 @@ class WC_Webhook_Data_Store implements WC_Webhook_Data_Store_Interface { $limit = -1 < $args['limit'] ? $wpdb->prepare( 'LIMIT %d', $args['limit'] ) : ''; $offset = 0 < $args['offset'] ? $wpdb->prepare( 'OFFSET %d', $args['offset'] ) : ''; $status = ! empty( $args['status'] ) ? $wpdb->prepare( 'AND `status` = %s', isset( $statuses[ $args['status'] ] ) ? $statuses[ $args['status'] ] : $args['status'] ) : ''; - $search = ! empty( $args['search'] ) ? "AND `name` LIKE '%" . $wpdb->esc_like( sanitize_text_field( $args['search'] ) ) . "%'" : ''; + $search = ! empty( $args['search'] ) ? $wpdb->prepare( "AND `name` LIKE %s", '%' . $wpdb->esc_like( sanitize_text_field( $args['search'] ) ) . '%' ) : ''; $include = ''; $exclude = ''; $date_created = ''; From 91c55b54c0220071dd4332b151167e7946600dd6 Mon Sep 17 00:00:00 2001 From: Jonathan Sadowski Date: Fri, 16 Jul 2021 15:00:02 -0500 Subject: [PATCH 25/25] Update prepared query to use single-quote instead of double-quotes --- includes/data-stores/class-wc-webhook-data-store.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/data-stores/class-wc-webhook-data-store.php b/includes/data-stores/class-wc-webhook-data-store.php index 0815e28fe09..e8b417e3a8f 100644 --- a/includes/data-stores/class-wc-webhook-data-store.php +++ b/includes/data-stores/class-wc-webhook-data-store.php @@ -277,7 +277,7 @@ class WC_Webhook_Data_Store implements WC_Webhook_Data_Store_Interface { $limit = -1 < $args['limit'] ? $wpdb->prepare( 'LIMIT %d', $args['limit'] ) : ''; $offset = 0 < $args['offset'] ? $wpdb->prepare( 'OFFSET %d', $args['offset'] ) : ''; $status = ! empty( $args['status'] ) ? $wpdb->prepare( 'AND `status` = %s', isset( $statuses[ $args['status'] ] ) ? $statuses[ $args['status'] ] : $args['status'] ) : ''; - $search = ! empty( $args['search'] ) ? $wpdb->prepare( "AND `name` LIKE %s", '%' . $wpdb->esc_like( sanitize_text_field( $args['search'] ) ) . '%' ) : ''; + $search = ! empty( $args['search'] ) ? $wpdb->prepare( 'AND `name` LIKE %s', '%' . $wpdb->esc_like( sanitize_text_field( $args['search'] ) ) . '%' ) : ''; $include = ''; $exclude = ''; $date_created = '';