Fix variable product stock status not being properly set on save

When a product is saved its validate_props method is invoked,
and this recalculates the stock_status property based on whether
the product manages stock or not, the stock quantity, and the
value of the woocommerce_notify_no_stock_amount option.

In the case of variable products, and when stock is managed, the stock
was set to "instock" when the current stock was enough, but only
if the "stock_quantity" property was in the list of changed properties
for the object (the method in the base product class doen't check
for changed properties). This is a problem because the
wc_update_product_stock function updates stock_quantity but via direct
database modification, and thus stock_quantity isn't considered
modified. Therefore stock modifications via wc_update_product_stock
don't update stock_status on the product (e.g. when going from 0 to 1
after a refund the stock status will remain as "outofstock").

The fix consists of removing the check for changed properties since
it's not done anyway in the other cases (when stock is below the
woocommerce_notify_no_stock_amount threshold) nor in the base class.

Also, validate_props is refactored for readabiliyy, and an useless
set_stock_status() call placed right before save()
in wc_update_product_stock is removed.

includes/abstracts/abstract-wc-product.php

@@ -1336,19 +1336,21 @@ class WC_Product extends WC_Abstract_Legacy_Product {
 			$this->set_stock_quantity( '' );
 			$this->set_backorders( 'no' );
 			$this->set_low_stock_amount( '' );
-
+			return;
-			// If we are stock managing and we don't have stock, force out of stock status.
-		} elseif ( $this->get_stock_quantity() <= get_option( 'woocommerce_notify_no_stock_amount', 0 ) && 'no' === $this->get_backorders() ) {
-			$this->set_stock_status( 'outofstock' );
-
-			// If we are stock managing, backorders are allowed, and we don't have stock, force on backorder status.
-		} elseif ( $this->get_stock_quantity() <= get_option( 'woocommerce_notify_no_stock_amount', 0 ) && 'no' !== $this->get_backorders() ) {
-			$this->set_stock_status( 'onbackorder' );
-
-			// If the stock level is changing and we do now have enough, force in stock status.
-		} elseif ( $this->get_stock_quantity() > get_option( 'woocommerce_notify_no_stock_amount', 0 ) ) {
-			$this->set_stock_status( 'instock' );
 		}
+
+		$stock_is_above_notification_threshold = ( $this->get_stock_quantity() > get_option( 'woocommerce_notify_no_stock_amount', 0 ) );
+		$backorders_are_allowed                = ( 'no' !== $this->get_backorders() );
+
+		if ( $stock_is_above_notification_threshold ) {
+			$new_stock_status = 'instock';
+		} elseif ( $backorders_are_allowed ) {
+			$new_stock_status = 'onbackorder';
+		} else {
+			$new_stock_status = 'outofstock';
+		}
+
+		$this->set_stock_status( $new_stock_status );
 	}
 
 	/**

includes/class-wc-product-variable.php

@@ -413,28 +413,10 @@ class WC_Product_Variable extends WC_Product {
 	 * @since 3.0.0
 	 */
 	public function validate_props() {
-		// Before updating, ensure stock props are all aligned. Qty and backorders are not needed if not stock managed.
+		parent::validate_props();
+
 		if ( ! $this->get_manage_stock() ) {
-			$this->set_stock_quantity( '' );
-			$this->set_backorders( 'no' );
-			$this->set_low_stock_amount( '' );
 			$this->data_store->sync_stock_status( $this );
-
-			// If we are stock managing, backorders are allowed, and we don't have stock, force on backorder status.
-		} elseif ( $this->get_stock_quantity() <= get_option( 'woocommerce_notify_no_stock_amount', 0 ) && 'no' !== $this->get_backorders() ) {
-			$this->set_stock_status( 'onbackorder' );
-
-			// If we are stock managing and we don't have stock, force out of stock status.
-		} elseif ( $this->get_stock_quantity() <= get_option( 'woocommerce_notify_no_stock_amount', 0 ) && 'no' === $this->get_backorders() ) {
-			$this->set_stock_status( 'outofstock' );
-
-			// If the stock level is changing and we do now have enough, force in stock status.
-		} elseif ( $this->get_stock_quantity() > get_option( 'woocommerce_notify_no_stock_amount', 0 ) && array_key_exists( 'stock_quantity', $this->get_changes() ) ) {
-			$this->set_stock_status( 'instock' );
-
-			// Otherwise revert to status the children have.
-		} else {
-			$this->set_stock_status( $this->child_is_in_stock() ? 'instock' : 'outofstock' );
 		}
 	}
 

includes/wc-stock-functions.php

@@ -46,7 +46,6 @@ function wc_update_product_stock( $product, $stock_quantity = null, $operation =
 
 		// If this is not being called during an update routine, save the product so stock status etc is in sync, and caches are cleared.
 		if ( ! $updating ) {
-			$product_with_stock->set_stock_status();
 			$product_with_stock->save();
 		}
 

tests/legacy/unit-tests/product/class-wc-tests-product.php

@@ -12,6 +12,22 @@
  */
 class WC_Tests_Product extends WC_Unit_Test_Case {
 
+	/**
+	 * @var WC_Product
+	 */
+	protected $product;
+
+	/**
+	 * Runs before every test.
+	 */
+	public function setUp() {
+		parent::setUp();
+
+		$this->product = new WC_Product();
+		$this->product->save();
+	}
+
+
 	/**
 	 * @testdox When a product is saved or deleted its parent should be scheduled for sync at the end of the request.
 	 *
@@ -23,19 +39,45 @@ class WC_Tests_Product extends WC_Unit_Test_Case {
 	public function test_deferred_sync_on_save_and_delete( $operation ) {
 		$defer_sync_invoked = false;
 
-		$defer_product_callback = function() use ( &$defer_sync_invoked ) {
+		$defer_product_callback = function () use ( &$defer_sync_invoked ) {
 			$defer_sync_invoked = true;
 		};
 
 		$product = $this->getMockBuilder( WC_Product::class )
-						->setMethods( array( 'maybe_defer_product_sync' ) )
+		                ->setMethods( array( 'maybe_defer_product_sync' ) )
-						->getMock();
+		                ->getMock();
 
 		$product->method( 'maybe_defer_product_sync' )
-				->will( $this->returnCallback( $defer_product_callback ) );
+		        ->will( $this->returnCallback( $defer_product_callback ) );
 
 		$product->$operation();
 
 		$this->assertTrue( $defer_sync_invoked );
 	}
+
+	/**
+	 * @testdox Test that stock status is set to the proper value when saving, if the product manages stock levels.
+	 *
+	 * @testWith [5, 4, true, "instock"]
+	 *           [5, 4, false, "instock"]
+	 *           [4, 4, true, "onbackorder"]
+	 *           [4, 4, false, "outofstock"]
+	 *           [3, 4, true, "onbackorder"]
+	 *           [3, 4, false, "outofstock"]
+	 *
+	 * @param int    $stock_quantity Current stock quantity for the product.
+	 * @param bool   $notify_no_stock_amount Value for the woocommerce_notify_no_stock_amount option.
+	 * @param bool   $accepts_backorders Whether the product accepts backorders or not.
+	 * @param string $expected_stock_status The expected stock status of the product after being saved.
+	 */
+	public function test_stock_status_on_save_when_managing_stock( $stock_quantity, $notify_no_stock_amount, $accepts_backorders, $expected_stock_status ) {
+		update_option( 'woocommerce_notify_no_stock_amount', $notify_no_stock_amount );
+		$this->product->set_backorders( $accepts_backorders ? 'yes' : 'no' );
+		$this->product->set_manage_stock( 'yes' );
+		$this->product->set_stock_status( '' );
+		$this->product->set_stock_quantity( $stock_quantity );
+		$this->product->save();
+
+		$this->assertEquals( $expected_stock_status, $this->product->get_stock_status() );
+	}
 }

tests/legacy/unit-tests/product/product-variable.php

@@ -76,11 +76,11 @@ class WC_Tests_Product_Variable extends WC_Unit_Test_Case {
 	}
 
 	/**
-	 * Test that variable products have the correct status when syncing with their children.
+	 * Create a variable product with two variations.
 	 *
-	 * @since 3.3.0
+	 * @return array An array containing first the main product, and then the two variation products.
 	 */
-	public function test_variable_product_stock_status_sync() {
+	private function get_variable_product_with_children() {
 		$product = new WC_Product_Variable();
 		$product->save();
 
@@ -94,6 +94,17 @@ class WC_Tests_Product_Variable extends WC_Unit_Test_Case {
 
 		$product->set_children( array( $child1->get_id(), $child2->get_id() ) );
 
+		return array( $product, $child1, $child2 );
+	}
+
+	/**
+	 * Test that variable products have the correct status when syncing with their children.
+	 *
+	 * @since 3.3.0
+	 */
+	public function test_variable_product_stock_status_sync() {
+		list($product, $child1, $child2) = $this->get_variable_product_with_children();
+
 		// Product should be in stock if a child is in stock.
 		$child1->set_stock_status( 'instock' );
 		$child1->save();
@@ -131,4 +142,38 @@ class WC_Tests_Product_Variable extends WC_Unit_Test_Case {
 		WC_Product_Variable::sync( $product );
 		$this->assertEquals( 'onbackorder', $product->get_stock_status() );
 	}
+
+	/**
+	 * @testdox Test that stock status is set to the proper value when saving, if the product manages stock levels.
+	 *
+	 * @testWith [5, 4, true, "instock"]
+	 *           [5, 4, false, "instock"]
+	 *           [4, 4, true, "onbackorder"]
+	 *           [4, 4, false, "outofstock"]
+	 *           [3, 4, true, "onbackorder"]
+	 *           [3, 4, false, "outofstock"]
+	 *
+	 * @param int    $stock_quantity Current stock quantity for the product.
+	 * @param bool   $notify_no_stock_amount Value for the woocommerce_notify_no_stock_amount option.
+	 * @param bool   $accepts_backorders Whether the product accepts backorders or not.
+	 * @param string $expected_stock_status The expected stock status of the product after being saved.
+	 */
+	public function test_stock_status_on_save_when_managing_stock( $stock_quantity, $notify_no_stock_amount, $accepts_backorders, $expected_stock_status ) {
+		list($product, $child1, $child2) = $this->get_variable_product_with_children();
+
+		update_option( 'woocommerce_notify_no_stock_amount', $notify_no_stock_amount );
+
+		$child1->set_stock_status( '' );
+		$child1->save();
+		$child2->set_stock_status( '' );
+		$child2->save();
+
+		$product->set_manage_stock( 'yes' );
+		$product->set_stock_status( '' );
+		$product->set_backorders( $accepts_backorders ? 'yes' : 'no' );
+		$product->set_stock_quantity( $stock_quantity );
+		$product->save();
+
+		$this->assertEquals( $expected_stock_status, $product->get_stock_status() );
+	}
 }