From 3dd69708f9db7c1ab264bc040dd3ca2122ed3bf4 Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Mon, 11 Aug 2025 14:01:24 -0400 Subject: [PATCH] Fix brand count calculation and REST API endpoint (#60158) * Fix incorrect count for brands through the /wp-json/wc/v3/products/brands endpoint - Changed product_brand taxonomy to use _wc_term_recount instead of _update_post_term_count - Added product_brand to the valid taxonomies in wc_change_term_counts filter - This ensures brand counts properly account for WooCommerce product visibility logic * Fix brand count test to respect product visibility settings - Use go_to() to simulate frontend context instead of admin context - Use get_terms() instead of get_term() to trigger wc_change_term_counts filter - Test now properly validates that brand counts respect 'hide out of stock items' setting * Add comprehensive brand count tests for admin and frontend contexts - Add test_brand_count_respects_product_visibility() that uses go_to() and get_terms() to simulate frontend behavior - Add test_brand_count_ignores_product_visibility_in_admin_context() that uses get_term() to simulate admin behavior - Tests demonstrate different behaviors: frontend respects out-of-stock settings, admin ignores them - Both tests pass and provide complete coverage of the context-dependent behavior * Add REST API brands controller test file - Add comprehensive test suite for WC_REST_Product_Brands_Controller - Tests cover brand listing, count accuracy, product visibility respect - Includes test for brand counts respecting out-of-stock product settings - All 5 tests pass with 29 assertions - Provides end-to-end testing of brands API functionality * Remove admin vs frontend context documentation from woo-phpunit rule The context-dependent behavior documentation was removed as it was found to be inaccurate: - is_admin() returns false in test environment, not true - The real issue is function choice (get_terms vs get_term), not context - Simplified rule to focus on core testing practices * Remove duplicate product_brand filter from WC_Brands class * Add changefile(s) from automation for the following project(s): woocommerce * Delete plugins/woocommerce/changelog/fix-wooplug-5231-incorrect-count-for-brands-through-the-wp Remove superfluous changelog added by AI. * update brands rest controller test to remove unnecessary setup * Update doc block for method to include Brands * Add test for deleting product a brand is attached to. * Simplify the tests with helper for getting first brand term. * Add test for visibility affected count on specific brand REST request. * Move tests to category controller. The brands controller extends the category controller so this ensures tests live in the appropriate place. As a part of this refactor I also improved some of the test architecture and removed superfluous term_cache clears (WP scaffolding already clears caches on tearDown). --------- Co-authored-by: github-actions --- .cursor/rules/woo-phpunit.mdc | 66 +++- ...-incorrect-count-for-brands-through-the-wp | 4 + .../woocommerce/includes/class-wc-brands.php | 15 +- .../includes/wc-term-functions.php | 10 +- .../php/includes/class-wc-brands-test.php | 129 +++++++- ...est-product-categories-controller-test.php | 287 ++++++++++++++++++ 6 files changed, 489 insertions(+), 22 deletions(-) create mode 100644 plugins/woocommerce/changelog/60158-fix-wooplug-5231-incorrect-count-for-brands-through-the-wp create mode 100644 plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-product-categories-controller-test.php diff --git a/.cursor/rules/woo-phpunit.mdc b/.cursor/rules/woo-phpunit.mdc index 826663ae15f..24856647d3f 100644 --- a/.cursor/rules/woo-phpunit.mdc +++ b/.cursor/rules/woo-phpunit.mdc @@ -1,8 +1,9 @@ --- -description: +description: globs: plugins/woocommerce/tests/**/*.php alwaysApply: false --- + # WooCommerce PHPUnit Tests Rules for running WooCommerce PHPUnit tests. @@ -19,4 +20,67 @@ pnpm run test:php:env {relative_path} --verbose And the command must be run in the `plugins/woocommerce` directory. +## Test Execution Best Practices +### Command Structure + +- **Basic test file**: `pnpm run test:php:env tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-product-brands-controller-test.php --verbose` +- **Filter by test method**: `pnpm run test:php:env -- --filter="test_get_brands_with_correct_count"` +- **Filter by test class**: `pnpm run test:php:env -- --filter="WC_REST_Product_Brands_Controller_Test"` + +### Common Issues & Solutions + +#### 1. Class Not Found Errors + +If you get "Class could not be found" errors: + +- Use the `--filter` approach instead of direct file paths +- Ensure test classes have `public` setUp() and tearDown() methods (not `protected`) +- Add `declare( strict_types = 1 );` at the top of test files + +#### 2. Test Method Visibility + +All test methods must be `public`, not `protected`: + +```php +public function setUp(): void { + parent::setUp(); + // test setup +} + +public function tearDown(): void { + parent::tearDown(); + // test cleanup +} +``` + +#### 3. Test File Structure + +Ensure proper test file structure (the below is an example): + +```php + true, - 'update_count_callback' => '_update_post_term_count', + 'update_count_callback' => '_wc_term_recount', 'label' => __( 'Brands', 'woocommerce' ), 'labels' => array( 'name' => __( 'Brands', 'woocommerce' ), diff --git a/plugins/woocommerce/includes/wc-term-functions.php b/plugins/woocommerce/includes/wc-term-functions.php index 50551089595..648309946ff 100644 --- a/plugins/woocommerce/includes/wc-term-functions.php +++ b/plugins/woocommerce/includes/wc-term-functions.php @@ -323,12 +323,12 @@ function wc_reorder_terms( $the_term, $next_id, $taxonomy, $index = 0, $terms = } // the nextid of our term to order, lets move our term here. if ( null !== $next_id && $term_id === $next_id ) { - $index++; + ++$index; $index = wc_set_term_order( $id, $index, $taxonomy, true ); } // Set order. - $index++; + ++$index; $index = wc_set_term_order( $term_id, $index, $taxonomy ); /** @@ -374,7 +374,7 @@ function wc_set_term_order( $term_id, $index, $taxonomy, $recursive = false ) { $children = get_terms( $taxonomy, "parent=$term_id&hide_empty=0&menu_order=ASC" ); foreach ( $children as $term ) { - $index++; + ++$index; $index = wc_set_term_order( $term->term_id, $index, $taxonomy, true ); } @@ -386,7 +386,7 @@ function wc_set_term_order( $term_id, $index, $taxonomy, $recursive = false ) { /** * Function for recounting product terms, ignoring hidden products. * - * This is used as the update_count_callback for the Product Category and Product Tag + * This is used as the update_count_callback for the Product Category, Product Tag, and Product Brand * taxonomies. By default, it actually calculates two (possibly different) counts for each * term, which it stores in two different places. The first count is the one done by WordPress * itself, and is based on the status of the objects that are assigned the terms. In this case, @@ -569,7 +569,7 @@ function wc_change_term_counts( $terms, $taxonomies ) { * * @param array $valid_taxonomies List of taxonomy slugs. */ - $valid_taxonomies = apply_filters( 'woocommerce_change_term_counts', array( 'product_cat', 'product_tag' ) ); + $valid_taxonomies = apply_filters( 'woocommerce_change_term_counts', array( 'product_cat', 'product_tag', 'product_brand' ) ); $current_taxonomies = array_intersect( (array) $taxonomies, $valid_taxonomies ); if ( empty( $current_taxonomies ) ) { diff --git a/plugins/woocommerce/tests/php/includes/class-wc-brands-test.php b/plugins/woocommerce/tests/php/includes/class-wc-brands-test.php index 1f1981645d9..75b82395356 100644 --- a/plugins/woocommerce/tests/php/includes/class-wc-brands-test.php +++ b/plugins/woocommerce/tests/php/includes/class-wc-brands-test.php @@ -7,12 +7,20 @@ declare( strict_types = 1); -require_once WC_ABSPATH . '/includes/class-wc-brands.php'; - /** * WC Brands test */ class WC_Brands_Test extends WC_Unit_Test_Case { + + /** + * Tear down test data. + */ + public function tearDown(): void { + parent::tearDown(); + + // Clear term cache to prevent interference between tests. + clean_term_cache( array(), 'product_brand' ); + } /** * Test that `product_brand_thumbnails` shortcode's `show_empty` argument works as expected. * This test prevents regression of the issue where double filtering caused no brands to be displayed. @@ -74,6 +82,105 @@ class WC_Brands_Test extends WC_Unit_Test_Case { $this->assertStringContainsString( 'Empty Brand', $output ); } + /** + * Test that brand counts are correctly calculated and cached. + */ + public function test_brand_count_calculation_and_caching() { + $data = $this->setup_brand_test_data(); + + // Get the brand term. + clean_term_cache( $data['brand_with_products']['term_id'], 'product_brand' ); + $brand_term = $this->get_first_brand_term( array( $data['brand_with_products']['term_id'] ) ); + + // Test that the count is correctly calculated. + $this->assertEquals( 1, $brand_term->count, 'Brand should have 1 product' ); + + // Test that the count is cached in term meta. + $cached_count = get_term_meta( $brand_term->term_id, 'product_count_product_brand', true ); + $this->assertEquals( '1', $cached_count, 'Brand count should be cached in term meta' ); + } + + /** + * Test that brand counts respect product visibility settings. + */ + public function test_brand_count_respects_product_visibility() { + $data = $this->setup_brand_test_data(); + $product = $data['product']; + + // Enable hide out of stock setting FIRST. + update_option( 'woocommerce_hide_out_of_stock_items', 'yes' ); + + // THEN set product to out of stock (hook will fire with correct setting). + $product->set_stock_status( 'outofstock' ); + $product->save(); + + // Get the brand term. + $brand_term = $this->get_first_brand_term( array( $data['brand_with_products']['term_id'] ) ); + + // Test that the count is 0 when product is out of stock and hidden. + $this->assertEquals( 0, $brand_term->count, 'Brand count should be 0 when product is out of stock and hidden' ); + + // Test that the count is cached correctly. + $cached_count = get_term_meta( $brand_term->term_id, 'product_count_product_brand', true ); + $this->assertEquals( '0', $cached_count, 'Brand count should be cached as 0 when product is out of stock' ); + + // Reset the setting. + update_option( 'woocommerce_hide_out_of_stock_items', 'no' ); + } + + /** + * Test that brand counts are updated when products are added/removed. + */ + public function test_brand_count_updates_when_products_change() { + $data = $this->setup_brand_test_data(); + $product = $data['product']; + + // Initially should have 1 product. + $brand_term = get_term( $data['brand_with_products']['term_id'], 'product_brand' ); + $this->assertEquals( 1, $brand_term->count, 'Brand should initially have 1 product' ); + + // Remove the product from the brand. + wp_set_object_terms( $product->get_id(), array(), 'product_brand' ); + + // Count should be updated to 0. + $brand_term_updated = get_term( $data['brand_with_products']['term_id'], 'product_brand' ); + $this->assertEquals( 0, $brand_term_updated->count, 'Brand should have 0 products after removal' ); + + // Add the product back. + wp_set_object_terms( $product->get_id(), array( $data['brand_with_products']['term_id'] ), 'product_brand' ); + + // Count should be updated back to 1. + $brand_term_final = get_term( $data['brand_with_products']['term_id'], 'product_brand' ); + $this->assertEquals( 1, $brand_term_final->count, 'Brand should have 1 product after re-adding' ); + } + + /** + * Test that brand counts ignore product visibility in admin context. + */ + public function test_brand_count_ignores_product_visibility_in_admin_context() { + $data = $this->setup_brand_test_data(); + $product = $data['product']; + + // Enable hide out of stock setting. + update_option( 'woocommerce_hide_out_of_stock_items', 'yes' ); + + // Set product to out of stock. + $product->set_stock_status( 'outofstock' ); + $product->save(); + + // Set admin context. + set_current_screen( 'edit-post' ); + + // Get the brand term using helper. + $brand_term = $this->get_first_brand_term( array( $data['brand_with_products']['term_id'] ) ); + + // Test that the count is 1 (ignores out of stock setting in admin context). + $this->assertEquals( 1, $brand_term->count, 'Brand count should be 1 in admin context, ignoring out of stock setting' ); + + // Reset the setting. + update_option( 'woocommerce_hide_out_of_stock_items', 'no' ); + } + /** * Helper method to set up test data for brand shortcode tests. * @@ -96,4 +203,22 @@ class WC_Brands_Test extends WC_Unit_Test_Case { 'product' => $product, ); } + + /** + * Helper method to get the first brand term. + * + * @param array $term_ids Array of brand term IDs to include. + * @return WP_Term The first brand term. + */ + private function get_first_brand_term( $term_ids = array() ) { + $args = array( + 'taxonomy' => 'product_brand', + 'hide_empty' => false, + ); + if ( ! empty( $term_ids ) ) { + $args['include'] = $term_ids; + } + $brand_terms = get_terms( $args ); + return $brand_terms[0]; + } } diff --git a/plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-product-categories-controller-test.php b/plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-product-categories-controller-test.php new file mode 100644 index 00000000000..dcffcae92c9 --- /dev/null +++ b/plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-product-categories-controller-test.php @@ -0,0 +1,287 @@ +user = $this->factory->user->create( array( 'role' => 'administrator' ) ); + wp_set_current_user( $this->user ); + } + + /** + * Helper method to create a test category. + * + * @param string $name Category name. + * @return array Category data. + */ + private function create_test_category( string $name ): array { + $category = wp_insert_term( $name, 'product_cat' ); + return $category; + } + + /** + * Helper method to create a test product. + * + * @return WC_Product_Simple Product object. + */ + private function create_test_product(): WC_Product_Simple { + $product = WC_Helper_Product::create_simple_product(); + $product->save(); + return $product; + } + + /** + * Helper method to make a GET request to the categories endpoint. + * + * @param string|int $endpoint Optional specific endpoint. + * @param array $params Optional query parameters. + * @return WP_REST_Response Response object. + */ + private function make_categories_request( $endpoint = '', array $params = array() ): WP_REST_Response { + $url = '/wc/v3/products/categories'; + if ( $endpoint ) { + $url .= '/' . $endpoint; + } + + $request = new WP_REST_Request( 'GET', $url ); + foreach ( $params as $key => $value ) { + $request->set_param( $key, $value ); + } + + return $this->server->dispatch( $request ); + } + + /** + * Helper method to find test category data in response. + * + * @param array $response_data Response data array. + * @param int $category_id Category ID to find. + * @return array|null Category data or null if not found. + */ + private function find_category_in_response( array $response_data, int $category_id ): ?array { + foreach ( $response_data as $category_data ) { + if ( $category_data['id'] === $category_id ) { + return $category_data; + } + } + return null; + } + + /** + * Test getting categories with correct count. + */ + public function test_get_categories_with_correct_count() { + // Create a category. + $category = $this->create_test_category( 'Test Category' ); + + // Create a product and assign it to the category. + $product = $this->create_test_product(); + wp_set_object_terms( $product->get_id(), array( $category['term_id'] ), 'product_cat' ); + + // Make the request. + $response = $this->make_categories_request(); + + // Check response. + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + + // Find our test category in the response. + $test_category_data = $this->find_category_in_response( $data, $category['term_id'] ); + $this->assertNotNull( $test_category_data, 'Test category should be found in response' ); + + // Assert category data. + $this->assertEquals( $category['term_id'], $test_category_data['id'] ); + $this->assertEquals( 'Test Category', $test_category_data['name'] ); + $this->assertEquals( 'test-category', $test_category_data['slug'] ); + $this->assertEquals( 1, $test_category_data['count'], 'Category should have count of 1' ); + + // Clean up. + wp_delete_post( $product->get_id(), true ); + wp_delete_term( $category['term_id'], 'product_cat' ); + } + + /** + * Test getting categories with zero count when no products assigned. + */ + public function test_get_categories_with_zero_count() { + // Create a category without any products. + $category = $this->create_test_category( 'Empty Category' ); + + // Make the request. + $response = $this->make_categories_request(); + + // Check response. + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + + // Find our test category in the response. + $test_category_data = $this->find_category_in_response( $data, $category['term_id'] ); + $this->assertNotNull( $test_category_data, 'Test category should be found in response' ); + + // Assert category data. + $this->assertEquals( $category['term_id'], $test_category_data['id'] ); + $this->assertEquals( 'Empty Category', $test_category_data['name'] ); + $this->assertEquals( 0, $test_category_data['count'], 'Category should have count of 0' ); + + // Clean up. + wp_delete_term( $category['term_id'], 'product_cat' ); + } + + /** + * Test getting categories respects product visibility settings. + */ + public function test_get_categories_respects_product_visibility() { + // Create a category. + $category = $this->create_test_category( 'Visibility Test Category' ); + + // Create a product and assign it to the category. + $product = $this->create_test_product(); + wp_set_object_terms( $product->get_id(), array( $category['term_id'] ), 'product_cat' ); + + // Initially should have count of 1. + $response = $this->make_categories_request(); + $data = $response->get_data(); + + // Find our test category in the response. + $test_category_data = $this->find_category_in_response( $data, $category['term_id'] ); + $this->assertNotNull( $test_category_data, 'Test category should be found in response' ); + $this->assertEquals( 1, $test_category_data['count'], 'Category should initially have count of 1' ); + + // Set product to out of stock and enable hide out of stock setting. + update_option( 'woocommerce_hide_out_of_stock_items', 'yes' ); + $product->set_stock_status( 'outofstock' ); + $product->save(); + + // Now should have count of 0. + $response = $this->make_categories_request(); + $data = $response->get_data(); + + // Find our test category in the response. + $test_category_data = $this->find_category_in_response( $data, $category['term_id'] ); + $this->assertNotNull( $test_category_data, 'Test category should be found in response' ); + $this->assertEquals( 0, $test_category_data['count'], 'Category should have count of 0 when product is out of stock and hidden' ); + + // Category specific request should have count of 1. + $response = $this->make_categories_request( $category['term_id'] ); + $data = $response->get_data(); + $this->assertEquals( 1, $data['count'], 'Category should have count of 1 when product is out of stock and hidden' ); + + // Reset the setting. + update_option( 'woocommerce_hide_out_of_stock_items', 'no' ); + + // Clean up. + wp_delete_post( $product->get_id(), true ); + wp_delete_term( $category['term_id'], 'product_cat' ); + } + + /** + * Test getting categories with include parameter. + */ + public function test_get_categories_with_include_parameter() { + // Create multiple categories. + $category1 = $this->create_test_category( 'Category 1' ); + $category2 = $this->create_test_category( 'Category 2' ); + $category3 = $this->create_test_category( 'Category 3' ); + + // Assign products to categories 1 and 3. + $product1 = $this->create_test_product(); + wp_set_object_terms( $product1->get_id(), array( $category1['term_id'] ), 'product_cat' ); + + $product3 = $this->create_test_product(); + wp_set_object_terms( $product3->get_id(), array( $category3['term_id'] ), 'product_cat' ); + + // Make the request with include parameter. + $response = $this->make_categories_request( '', array( 'include' => array( $category1['term_id'], $category3['term_id'] ) ) ); + + // Check response. + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + + // Should have two categories. + $this->assertCount( 2, $data ); + + // Check that the counts are correct. + $category1_data = $this->find_category_in_response( $data, $category1['term_id'] ); + $category3_data = $this->find_category_in_response( $data, $category3['term_id'] ); + + $this->assertNotNull( $category1_data, 'Category 1 should be included' ); + $this->assertNotNull( $category3_data, 'Category 3 should be included' ); + $this->assertEquals( 1, $category1_data['count'], 'Category 1 should have count of 1' ); + $this->assertEquals( 1, $category3_data['count'], 'Category 3 should have count of 1' ); + + // Clean up. + wp_delete_post( $product1->get_id(), true ); + wp_delete_post( $product3->get_id(), true ); + wp_delete_term( $category1['term_id'], 'product_cat' ); + wp_delete_term( $category2['term_id'], 'product_cat' ); + wp_delete_term( $category3['term_id'], 'product_cat' ); + } + + /** + * Test that category counts are updated when products are added/removed. + */ + public function test_category_counts_update_when_products_change() { + // Create a category. + $category = $this->create_test_category( 'Dynamic Count Category' ); + + // Initially should have count of 0. + $response = $this->make_categories_request( $category['term_id'] ); + $data = $response->get_data(); + $this->assertEquals( 0, $data['count'], 'Category should initially have count of 0' ); + + // Create a product and assign it to the category. + $product = $this->create_test_product(); + wp_set_object_terms( $product->get_id(), array( $category['term_id'] ), 'product_cat' ); + + // Now should have count of 1. + $response = $this->make_categories_request( $category['term_id'] ); + $data = $response->get_data(); + $this->assertEquals( 1, $data['count'], 'Category should have count of 1 after adding product' ); + + // Remove the product from the category. + wp_set_object_terms( $product->get_id(), array(), 'product_cat' ); + + // Now should have count of 0 again. + $response = $this->make_categories_request( $category['term_id'] ); + $data = $response->get_data(); + $this->assertEquals( 0, $data['count'], 'Category should have count of 0 after removing product' ); + + // Attach the product to the category again. + wp_set_object_terms( $product->get_id(), array( $category['term_id'] ), 'product_cat' ); + + // Now should have count of 1 again. + $response = $this->make_categories_request( $category['term_id'] ); + $data = $response->get_data(); + + // Delete the product. + wp_delete_post( $product->get_id() ); + + // Now should have count of 0 again. + $response = $this->make_categories_request( $category['term_id'] ); + $data = $response->get_data(); + $this->assertEquals( 0, $data['count'], 'Category should have count of 0 after deleting product' ); + + // Clean up. + wp_delete_term( $category['term_id'], 'product_cat' ); + } +}