Commit 4bdbb7a2a0e for woocommerce
commit 4bdbb7a2a0e4f0bfb91b67c80a6d5c54d89caa56
Author: Oleksandr Aratovskyi <79862886+oaratovskyi@users.noreply.github.com>
Date: Wed Jul 29 18:10:41 2026 +0300
Fix stale variation ID when changing an order line item's product (#66734)
* test: cover set_product product reassignment on order line items
WC_Order_Item_Product::set_product() has only ever been tested against a
freshly constructed item, where variation_id already defaults to 0. The
reassignment path — calling set_product() on an item that already carries a
variation — was never exercised, which is why #52292 went unnoticed.
Add a regression test that reuses a single item across two set_product()
calls, plus a guard test for the variation-to-variation path. The regression
test fails against current behaviour and is fixed in the following commit.
Refs #52292
* fix: reset variation_id when set_product sets a non-variation product
WC_Order_Item_Product::set_product() sets variation_id only when handed a
variation. Handed a simple product, it set product_id and left variation_id
untouched, so an item previously holding a variation ended up pointing at two
different products at once. Because get_product() resolves variation_id first,
it then returned the stale variation and the setter was silently a no-op for
every consumer downstream — including stock reduction and analytics
attribution, which were applied against the wrong product.
Reset variation_id to 0 in the non-variation branch, matching the invariant
WC_Abstract_Order::add_product() has enforced since 3.0.0.
Refs #52292
* chore: add changelog for set_product variation_id reset
Refs #52292
* test: pin set_product() variable-parent and simple-to-variation behavior
The final review on the set_product() variation_id fix surfaced an
undisclosed reverse vector: handing set_product() the variable parent
of an item's own current variation now zeroes variation_id (product_id
becomes the parent's). The team decided to keep this behavior, but it
needs a test pinning it as deliberate rather than an undisclosed side
effect a future refactor could silently flip.
The review also found that a simple -> variation case from the
original plan was dropped from the test file, leaving the `if` branch
of set_product() unguarded against a future refactor.
Refs #52292
* Document why set_product() leaves stale variation attribute meta
The fix makes previously-hidden variation attribute meta visible on a
product switched to a simple product. Clearing it blindly risks deleting
a merchant's own custom meta, so it stays put, tracked in #66733. The
comment and test document the accepted tradeoff.
* Add REST tests for variation_id reset on line item product switch
The variation_id reset is exercised over REST but nothing pinned it in
CI. Cover the two external-facing payloads: switching a variation line
item to a simple product, and a product_id-only update targeting the
variable parent.
diff --git a/plugins/woocommerce/changelog/52292-set-product-reset-variation-id b/plugins/woocommerce/changelog/52292-set-product-reset-variation-id
new file mode 100644
index 00000000000..def7d5c10cd
--- /dev/null
+++ b/plugins/woocommerce/changelog/52292-set-product-reset-variation-id
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+WC_Order_Item_Product::set_product() now resets variation_id to 0 when passed a non-variation product, so reassigning a line item's product no longer leaves a stale variation ID that made get_product() return the previous variation.
diff --git a/plugins/woocommerce/includes/class-wc-order-item-product.php b/plugins/woocommerce/includes/class-wc-order-item-product.php
index 9a0158eee54..d0388e7a31a 100644
--- a/plugins/woocommerce/includes/class-wc-order-item-product.php
+++ b/plugins/woocommerce/includes/class-wc-order-item-product.php
@@ -259,6 +259,11 @@ class WC_Order_Item_Product extends WC_Order_Item {
$this->set_variation( is_callable( array( $product, 'get_variation_attributes' ) ) ? $product->get_variation_attributes() : array() );
} else {
$this->set_product_id( $product->get_id() );
+ $this->set_variation_id( 0 );
+ // Any variation attribute meta written by a previous set_variation() call is left in
+ // place on purpose: it is stored with the `attribute_` prefix stripped, so a key like
+ // `color` is indistinguishable from a merchant's own custom meta and clearing it here
+ // risks deleting real data. Removing that stale display meta is tracked in #66733.
}
$this->set_name( $product->get_name() );
$this->set_tax_class( $product->get_tax_class() );
diff --git a/plugins/woocommerce/tests/php/includes/class-wc-order-item-product-test.php b/plugins/woocommerce/tests/php/includes/class-wc-order-item-product-test.php
index fcfc489c912..602aecbbcae 100644
--- a/plugins/woocommerce/tests/php/includes/class-wc-order-item-product-test.php
+++ b/plugins/woocommerce/tests/php/includes/class-wc-order-item-product-test.php
@@ -622,4 +622,137 @@ class WC_Order_Item_Product_Test extends WC_Unit_Test_Case {
// Clean up order.
$order->delete( true );
}
+
+ /**
+ * @testdox Should reset variation_id to 0 when set_product() switches the item to a non-variation product.
+ */
+ public function test_set_product_resets_variation_id_when_switching_to_simple_product(): void {
+ $parent = new WC_Product_Variable();
+ $parent->set_name( 'Dummy Variable Product' );
+ $parent->save();
+
+ $variation = WC_Helper_Product::create_product_variation_object(
+ $parent->get_id(),
+ 'VARIATION SKU ' . wp_generate_uuid4(),
+ 10,
+ array()
+ );
+
+ $item = new WC_Order_Item_Product();
+ $item->set_product( $variation );
+
+ $this->assertSame( $variation->get_id(), $item->get_variation_id(), 'Precondition: the item should carry the variation ID before switching.' );
+
+ $item->set_product( $this->product );
+
+ $this->assertSame( 0, $item->get_variation_id(), 'variation_id should be reset to 0 when switching to a non-variation product.' );
+ $this->assertSame( $this->product->get_id(), $item->get_product_id(), 'product_id should point at the newly set product.' );
+ $this->assertSame( $this->product->get_id(), $item->get_product()->get_id(), 'get_product() should return the newly set product, not the stale variation.' );
+ }
+
+ /**
+ * @testdox Should update variation_id when set_product() switches the item between two variations.
+ */
+ public function test_set_product_updates_variation_id_when_switching_between_variations(): void {
+ $parent = new WC_Product_Variable();
+ $parent->set_name( 'Dummy Variable Product' );
+ $parent->save();
+
+ $variation_a = WC_Helper_Product::create_product_variation_object(
+ $parent->get_id(),
+ 'VARIATION A SKU ' . wp_generate_uuid4(),
+ 10,
+ array()
+ );
+ $variation_b = WC_Helper_Product::create_product_variation_object(
+ $parent->get_id(),
+ 'VARIATION B SKU ' . wp_generate_uuid4(),
+ 15,
+ array()
+ );
+
+ $item = new WC_Order_Item_Product();
+ $item->set_product( $variation_a );
+ $item->set_product( $variation_b );
+
+ $this->assertSame( $variation_b->get_id(), $item->get_variation_id(), 'variation_id should track the most recently set variation.' );
+ $this->assertSame( $parent->get_id(), $item->get_product_id(), 'product_id should remain the shared parent.' );
+ }
+
+ /**
+ * @testdox Should clear variation_id when set_product() is given the variable parent of the item's own current variation.
+ */
+ public function test_set_product_clears_variation_id_when_switched_to_own_variable_parent(): void {
+ $parent = new WC_Product_Variable();
+ $parent->set_name( 'T-Shirt' );
+ $parent->save();
+
+ $variation = WC_Helper_Product::create_product_variation_object(
+ $parent->get_id(),
+ 'VARIATION SKU ' . wp_generate_uuid4(),
+ 10,
+ array()
+ );
+
+ $item = new WC_Order_Item_Product();
+ $item->set_product( $variation );
+
+ $this->assertSame( $variation->get_id(), $item->get_variation_id(), 'Precondition: the item should carry the variation ID before switching to the parent.' );
+
+ $this->assertFalse( $parent->is_type( 'variation' ), 'Precondition: a WC_Product_Variable is not itself a variation.' );
+
+ $item->set_product( $parent );
+
+ $this->assertSame( 0, $item->get_variation_id(), 'A variable parent is not itself a sellable line item, so setting it deliberately clears variation_id rather than preserving the previously selected variation.' );
+ $this->assertSame( $parent->get_id(), $item->get_product_id(), 'product_id should point at the variable parent.' );
+ }
+
+ /**
+ * @testdox Should set product_id and variation_id when set_product() switches the item from a simple product to a variation.
+ */
+ public function test_set_product_sets_variation_id_when_switching_from_simple_product_to_variation(): void {
+ $parent = new WC_Product_Variable();
+ $parent->set_name( 'Dummy Variable Product' );
+ $parent->save();
+
+ $variation = WC_Helper_Product::create_product_variation_object(
+ $parent->get_id(),
+ 'VARIATION SKU ' . wp_generate_uuid4(),
+ 10,
+ array()
+ );
+
+ $item = new WC_Order_Item_Product();
+ $item->set_product( $this->product );
+ $item->set_product( $variation );
+
+ $this->assertSame( $parent->get_id(), $item->get_product_id(), 'product_id should point at the variation\'s parent when switching from a simple product.' );
+ $this->assertSame( $variation->get_id(), $item->get_variation_id(), 'variation_id should be set to the variation ID when switching from a simple product.' );
+ }
+
+ /**
+ * @testdox Should leave stale variation attribute meta in place when set_product() switches to a simple product (documents the tradeoff tracked in #66733).
+ */
+ public function test_set_product_leaves_stale_variation_attribute_meta_when_switching_to_simple_product(): void {
+ $parent = new WC_Product_Variable();
+ $parent->set_name( 'Dummy Variable Product' );
+ $parent->save();
+
+ $variation = WC_Helper_Product::create_product_variation_object(
+ $parent->get_id(),
+ 'VARIATION SKU ' . wp_generate_uuid4(),
+ 10,
+ array( 'color' => 'blue' )
+ );
+
+ $item = new WC_Order_Item_Product();
+ $item->set_product( $variation );
+
+ $this->assertSame( 'blue', $item->get_meta( 'color' ), 'Precondition: set_variation() writes the variation attribute as display meta with the attribute_ prefix stripped.' );
+
+ $item->set_product( $this->product );
+
+ $this->assertSame( $this->product->get_id(), $item->get_product()->get_id(), 'get_product() should resolve to the simple product once variation_id is cleared.' );
+ $this->assertSame( 'blue', $item->get_meta( 'color' ), 'Accepted behavior: the stale "color" attribute meta survives the switch because clearing it blindly could delete a merchant\'s own custom meta. Removing it is tracked in #66733.' );
+ }
}
diff --git a/plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-orders-controller-tests.php b/plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-orders-controller-tests.php
index cefddd484f7..d7c5e16fe21 100644
--- a/plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-orders-controller-tests.php
+++ b/plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version3/class-wc-rest-orders-controller-tests.php
@@ -1134,4 +1134,113 @@ class WC_REST_Orders_Controller_Tests extends WC_REST_Unit_Test_Case {
$this->assertTrue( $line_item_meta['display_value']['readonly'] );
$this->assertTrue( $line_item_meta['display_key']['readonly'] );
}
+
+ /**
+ * Builds a variable product with a single variation carrying a real "color" attribute.
+ *
+ * @return array Two-element array: the WC_Product_Variable parent and its WC_Product_Variation.
+ */
+ private function create_variable_product_with_color_variation(): array {
+ $parent = new WC_Product_Variable();
+ $parent->set_name( 'REST Switch Parent' );
+
+ $attribute = new WC_Product_Attribute();
+ $attribute->set_name( 'color' );
+ $attribute->set_options( array( 'blue', 'green' ) );
+ $attribute->set_variation( true );
+ $parent->set_attributes( array( $attribute ) );
+ $parent->save();
+
+ $variation = WC_Helper_Product::create_product_variation_object(
+ $parent->get_id(),
+ 'REST-VAR-' . wp_generate_uuid4(),
+ 10,
+ array( 'color' => 'blue' )
+ );
+
+ return array( $parent, $variation );
+ }
+
+ /**
+ * Creates an order carrying the given variation as its single line item.
+ *
+ * @param WC_Product_Variation $variation Variation to add as a line item.
+ * @return array Two-element array: the saved WC_Order and the line item ID.
+ */
+ private function create_order_with_variation_line_item( $variation ): array {
+ $order = new WC_Order();
+ $item = new WC_Order_Item_Product();
+ $item->set_product( $variation );
+ $item->set_quantity( 1 );
+ $item->set_total( 10 );
+ $order->add_item( $item );
+ $order->save();
+
+ return array( $order, $item->get_id() );
+ }
+
+ /**
+ * @testdox PUT /orders that switches a variation line item to a simple product clears variation_id over the REST round trip.
+ */
+ public function test_update_line_item_to_simple_product_clears_variation_id(): void {
+ $fixture = $this->create_variable_product_with_color_variation();
+ $variation = $fixture[1];
+ list( $order, $item_id ) = $this->create_order_with_variation_line_item( $variation );
+ $simple = WC_Helper_Product::create_simple_product();
+
+ $this->assertSame( $variation->get_id(), (int) wc_get_order_item_meta( $item_id, '_variation_id' ), 'Precondition: the line item stored the variation ID.' );
+
+ $request = new WP_REST_Request( 'PUT', '/wc/v3/orders/' . $order->get_id() );
+ $request->set_header( 'content-type', 'application/json' );
+ $request->set_body(
+ wp_json_encode(
+ array(
+ 'line_items' => array(
+ array(
+ 'id' => $item_id,
+ 'product_id' => $simple->get_id(),
+ ),
+ ),
+ )
+ )
+ );
+
+ $response = $this->server->dispatch( $request );
+ $this->assertSame( 200, $response->get_status(), 'The update should succeed.' );
+
+ $reloaded = new WC_Order_Item_Product( $item_id );
+ $this->assertSame( 0, $reloaded->get_variation_id(), 'variation_id should be reset to 0 after switching to a simple product over REST.' );
+ $this->assertSame( 0, (int) wc_get_order_item_meta( $item_id, '_variation_id' ), 'The persisted _variation_id meta should be 0.' );
+ $this->assertSame( $simple->get_id(), $reloaded->get_product()->get_id(), 'get_product() should resolve to the simple product, not the old variation.' );
+ }
+
+ /**
+ * @testdox PUT /orders with a product_id-only payload targeting the variable parent demotes the item and clears variation_id.
+ */
+ public function test_update_line_item_to_variable_parent_clears_variation_id(): void {
+ list( $parent, $variation ) = $this->create_variable_product_with_color_variation();
+ list( $order, $item_id ) = $this->create_order_with_variation_line_item( $variation );
+
+ $request = new WP_REST_Request( 'PUT', '/wc/v3/orders/' . $order->get_id() );
+ $request->set_header( 'content-type', 'application/json' );
+ $request->set_body(
+ wp_json_encode(
+ array(
+ 'line_items' => array(
+ array(
+ 'id' => $item_id,
+ 'product_id' => $parent->get_id(),
+ ),
+ ),
+ )
+ )
+ );
+
+ $response = $this->server->dispatch( $request );
+ $this->assertSame( 200, $response->get_status(), 'A product_id-only payload targeting the variable parent succeeds with no error.' );
+
+ $reloaded = new WC_Order_Item_Product( $item_id );
+ $this->assertSame( 0, $reloaded->get_variation_id(), 'Omitting variation_id demotes the item to its parent and clears variation_id.' );
+ $this->assertSame( $parent->get_id(), $reloaded->get_product()->get_id(), 'get_product() should resolve to the variable parent.' );
+ }
}