plugins/woocommerce/includes/data-stores/class-wc-product-variation-data-store-cpt.php (6)

366-401: Bulk meta read is solid; avoid setting COGS flags when the feature is disabled.
Right now cogs_value_is_additive may be set via set_props even if the feature is off. Gate it by not piping it through the generic map.

Apply:

 $meta_key_to_props = array(
   '_variation_description'  => 'description',
   '_regular_price'          => 'regular_price',
   '_sale_price'             => 'sale_price',
   '_sale_price_dates_from'  => 'date_on_sale_from',
   '_sale_price_dates_to'    => 'date_on_sale_to',
   '_manage_stock'           => 'manage_stock',
   '_stock_status'           => 'stock_status',
   '_virtual'                => 'virtual',
   '_product_image_gallery'  => 'gallery_image_ids',
   '_download_limit'         => 'download_limit',
   '_download_expiry'        => 'download_expiry',
   '_downloadable'           => 'downloadable',
   '_sku'                    => 'sku',
   '_global_unique_id'       => 'global_unique_id',
   '_stock'                  => 'stock_quantity',
   '_weight'                 => 'weight',
   '_length'                 => 'length',
   '_width'                  => 'width',
   '_height'                 => 'height',
   '_low_stock_amount'       => 'low_stock_amount',
   '_backorders'             => 'backorders',
   '_cogs_total_value'       => 'cogs_total_value',
-  '_cogs_value_is_additive' => 'cogs_value_is_additive',
   '_tax_class'              => 'tax_class',
 );

---

402-406: Normalize gallery IDs and shipping class type.
Use core helpers for robustness and consistent typing.

-$set_props['gallery_image_ids'] = array_filter( explode( ',', $set_props['gallery_image_ids'] ?? '' ) );
-$set_props['shipping_class_id'] = current( $this->get_term_ids( $id, 'product_shipping_class' ) );
+$set_props['gallery_image_ids'] = wp_parse_id_list( (string) ( $set_props['gallery_image_ids'] ?? '' ) );
+$set_props['shipping_class_id'] = absint( current( $this->get_term_ids( $id, 'product_shipping_class' ) ) );

---

409-419: Avoid double-setting/extra query for COGS; use preloaded meta and apply the existing filter inline.
Currently you set COGS here and then call load_cogs_data() later, which re-reads and re-sets. Use the pre-fetched values and apply the filter to keep semantics while saving a query.

 if ( $this->cogs_feature_is_enabled() ) {
-	$cogs_value             = $set_props['cogs_total_value'] ?? '';
-	$cogs_value             = '' === $cogs_value ? null : (float) $cogs_value;
-	$cogs_value_is_additive = 'yes' === ( $set_props['cogs_value_is_additive'] ?? '' );
+	$cogs_value             = $set_props['cogs_total_value'] ?? '';
+	$cogs_value             = '' === $cogs_value ? null : (float) $cogs_value;
+	$cogs_value_is_additive = 'yes' === ( $post_meta_values['_cogs_value_is_additive'][0] ?? '' );
+	// Preserve filter behavior from load_cogs_data().
+	$cogs_value_is_additive = apply_filters( 'woocommerce_load_product_cogs_is_additive_flag', $cogs_value_is_additive, $product );
 	$product->set_props(
 		array(
 			'cogs_value'             => $cogs_value,
 			'cogs_value_is_additive' => $cogs_value_is_additive,
 		)
 	);
 }

Then remove the later load_cogs_data() call (see below).

---

469-477: Type normalization on parent data is good; one tiny consistency tweak.
You already absint shipping class and normalize stock. Consider also casting image_id to absint for consistency.

-$parent_data['image_id']           = get_post_thumbnail_id( $product->get_parent_id() );
+$parent_data['image_id']           = absint( get_post_thumbnail_id( $product->get_parent_id() ) );

---

479-482: Cross-sell IDs default handling.
Passing a possibly empty string into set_cross_sell_ids() is safe (it casts to array and filters empties). If you want to be explicit:

-$product->set_cross_sell_ids( $parent_data['_crosssell_ids'] );
+$product->set_cross_sell_ids( (array) ( $parent_data['_crosssell_ids'] ?? array() ) );

---

483-485: Remove redundant COGS reload once inline filter is applied.
If you adopt the inline-filter approach above, this second load is redundant and adds queries.

-if ( $this->cogs_feature_is_enabled() ) {
-	$this->load_cogs_data( $product );
-}
+// COGS already set from preloaded meta with filter applied.

plugins/woocommerce/tests/php/includes/data-stores/class-wc-product-variations-data-store-cpt-test.php (1)

39-109: Broad property load test is comprehensive; one assertion may be brittle.
get_gallery_image_ids() typically returns ints; asserting strings could be fragile across environments.

- $this->assertEquals( array( '123', '456', '789' ), $product->get_gallery_image_ids() );
+ $this->assertEquals( array( 123, 456, 789 ), $product->get_gallery_image_ids() );

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/woo-phpunit.mdc:0-0
Timestamp: 2025-08-18T06:11:48.768Z
Learning: Applies to plugins/woocommerce/tests/**/*.php : Ensure test classes define public setUp() and tearDown() methods (not protected)

Learnt from: Aljullu
PR: woocommerce/woocommerce#60667
File: plugins/woocommerce/src/Blocks/BlockTypes/AddToCartForm.php:122-125
Timestamp: 2025-08-29T10:13:44.268Z
Learning: In WooCommerce, the ProductType class in plugins/woocommerce/src/Enums/ProductType.php contains string constants (not a PHP backed enum), so ProductType::VARIATION directly returns the string 'variation' and can be passed directly to methods like WC_Product::is_type() without needing ->value.

Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/woo-phpunit.mdc:0-0
Timestamp: 2025-08-18T06:11:48.768Z
Learning: Applies to plugins/woocommerce/tests/**/*.php : Add declare(strict_types=1); at the top of each test file

Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/woo-phpunit.mdc:0-0
Timestamp: 2025-08-18T06:11:48.768Z
Learning: Applies to plugins/woocommerce/tests/**/*.php : Test classes should extend WC_Unit_Test_Case

Learnt from: nerrad
PR: woocommerce/woocommerce#60158
File: plugins/woocommerce/tests/php/includes/class-wc-brands-test.php:207-223
Timestamp: 2025-08-11T18:01:34.499Z
Learning: In the WooCommerce test suite, the team prefers test helper methods to fail immediately with errors (like undefined index) rather than adding defensive checks, as this makes test failures more visible and helps identify setup issues quickly. This applies to helper methods like `get_first_brand_term()` in test files.

Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/woo-phpunit.mdc:0-0
Timestamp: 2025-08-18T06:11:48.768Z
Learning: Applies to plugins/woocommerce/tests/**/*.php : All PHPUnit test methods must be public, not protected

plugins/woocommerce/includes/data-stores/class-wc-product-variation-data-store-cpt.php (1)

427-433: Nice: parent_id cached locally.

This addresses the earlier nit about repeated get_parent_id() calls.

plugins/woocommerce/includes/data-stores/class-wc-product-variation-data-store-cpt.php (4)

366-408: Harden types for gallery/shipping class and avoid unknown props in set_props.

• gallery_image_ids: parse to int IDs (handles whitespace/dupes).
• shipping_class_id: cast to int for consistency with parent block.
• Optional: don’t feed unknown props (e.g., cogs_total_value, cogs_value_is_additive) into set_props since there are no setters; compute them outside.

Apply:

-		$variation_data['gallery_image_ids'] = array_filter( explode( ',', $variation_data['gallery_image_ids'] ?? '' ) );
-		$variation_data['shipping_class_id'] = current( $this->get_term_ids( $id, 'product_shipping_class' ) );
+		$variation_data['gallery_image_ids'] = wp_parse_id_list( $variation_data['gallery_image_ids'] ?? '' );
+		$variation_data['shipping_class_id'] = absint( current( $this->get_term_ids( $id, 'product_shipping_class' ) ) );

Optionally trim the mapping to avoid unknown setters:

-			'_cogs_total_value'       => 'cogs_total_value',
-			'_cogs_value_is_additive' => 'cogs_value_is_additive',

---

409-417: Avoid double-setting the COGS “additive” flag.

You set cogs_value_is_additive here and then again in load_cogs_data(). To reduce churn and keep filter application in one place, set only cogs_value here and leave the additive flag to load_cogs_data().

 			$product->set_props(
 				array(
 					'cogs_value'             => $cogs_value,
-					'cogs_value_is_additive' => $cogs_value_is_additive,
 				)
 			);

---

444-478: Cast cross-sell IDs to ints before use.

set_cross_sell_ids() filters empties but doesn’t cast. Normalize to ints once when building $parent_data.

 		foreach ( $parent_meta_key_to_props as $meta_key => $prop ) {
 			$meta_value           = $parent_post_meta_values[ $meta_key ][0] ?? '';
 			$parent_data[ $prop ] = maybe_unserialize( $meta_value ); // get_post_meta only unserializes single values.
 		}
 
+		$parent_data['_crosssell_ids'] = array_map( 'absint', (array) $parent_data['_crosssell_ids'] );

---

497-513: COGS loader OK; minor note on duplication.

This re-sets the additive flag (and hits the meta cache). Consider relying solely on this path for the flag (see earlier comment) to keep filtering and assignment in one place.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.