From e05daaa0f355e450f010409070563058469cf23e Mon Sep 17 00:00:00 2001 From: Mike Jolley <mike.jolley@me.com> Date: Wed, 2 Dec 2020 16:03:24 +0000 Subject: [PATCH] Drop stock reservation when removing item from cart via the Store API (#3468) * Remove Blocks version of ReserveStock Class * When a cart item is removed, remove holds on stock * Move maybe_release_stock to abstract * Update ReserveStockException usage --- src/StoreApi/Routes/AbstractCartRoute.php | 15 ++ src/StoreApi/Routes/CartRemoveItem.php | 1 + src/StoreApi/Routes/Checkout.php | 6 +- src/StoreApi/Schemas/CartItemSchema.php | 13 +- src/StoreApi/Utilities/CartController.php | 12 +- src/StoreApi/Utilities/ReserveStock.php | 230 ------------------ .../Utilities/ReserveStockException.php | 57 ----- tests/php/StoreApi/Utilities/ReserveStock.php | 4 +- 8 files changed, 29 insertions(+), 309 deletions(-) delete mode 100644 src/StoreApi/Utilities/ReserveStock.php delete mode 100644 src/StoreApi/Utilities/ReserveStockException.php diff --git a/src/StoreApi/Routes/AbstractCartRoute.php b/src/StoreApi/Routes/AbstractCartRoute.php index 272656dedc7..14b877b60fd 100644 --- a/src/StoreApi/Routes/AbstractCartRoute.php +++ b/src/StoreApi/Routes/AbstractCartRoute.php @@ -44,6 +44,21 @@ protected function maybe_recalculate_totals() { } } + /** + * If there is a draft order, releases stock. + * + * @return void + */ + protected function maybe_release_stock() { + $draft_order = wc()->session->get( 'store_api_draft_order', 0 ); + + if ( ! $draft_order ) { + return; + } + + wc_release_stock_for_order( $draft_order ); + } + /** * Get route response when something went wrong. * diff --git a/src/StoreApi/Routes/CartRemoveItem.php b/src/StoreApi/Routes/CartRemoveItem.php index da6a4489183..6dd6785a2cf 100644 --- a/src/StoreApi/Routes/CartRemoveItem.php +++ b/src/StoreApi/Routes/CartRemoveItem.php @@ -57,6 +57,7 @@ protected function get_route_post_response( \WP_REST_Request $request ) { } $cart->remove_cart_item( $request['key'] ); + $this->maybe_release_stock(); return rest_ensure_response( $this->schema->get_item_response( $cart ) ); } diff --git a/src/StoreApi/Routes/Checkout.php b/src/StoreApi/Routes/Checkout.php index 30a3200c020..4ba732376e4 100644 --- a/src/StoreApi/Routes/Checkout.php +++ b/src/StoreApi/Routes/Checkout.php @@ -11,8 +11,8 @@ use Automattic\WooCommerce\Blocks\Domain\Services\CreateAccount; use Automattic\WooCommerce\Blocks\StoreApi\Utilities\CartController; use Automattic\WooCommerce\Blocks\StoreApi\Utilities\OrderController; -use Automattic\WooCommerce\Blocks\StoreApi\Utilities\ReserveStock; -use Automattic\WooCommerce\Blocks\StoreApi\Utilities\ReserveStockException; +use Automattic\WooCommerce\Checkout\Helpers\ReserveStock; +use Automattic\WooCommerce\Checkout\Helpers\ReserveStockException; use Automattic\WooCommerce\Blocks\Payments\PaymentResult; use Automattic\WooCommerce\Blocks\Payments\PaymentContext; @@ -332,7 +332,7 @@ private function is_valid_draft_order( $order_object ) { private function create_or_update_draft_order() { $cart_controller = new CartController(); $order_controller = new OrderController(); - $reserve_stock = \class_exists( '\Automattic\WooCommerce\Checkout\Helpers\ReserveStock' ) ? new \Automattic\WooCommerce\Checkout\Helpers\ReserveStock() : new ReserveStock(); + $reserve_stock = new ReserveStock(); $this->order = $this->get_draft_order_id() ? wc_get_order( $this->get_draft_order_id() ) : null; // Validate items etc are allowed in the order before it gets created. diff --git a/src/StoreApi/Schemas/CartItemSchema.php b/src/StoreApi/Schemas/CartItemSchema.php index 6c29884b965..a5efa8103e8 100644 --- a/src/StoreApi/Schemas/CartItemSchema.php +++ b/src/StoreApi/Schemas/CartItemSchema.php @@ -2,7 +2,7 @@ namespace Automattic\WooCommerce\Blocks\StoreApi\Schemas; use Automattic\WooCommerce\Blocks\Domain\Services\ExtendRestApi; - +use Automattic\WooCommerce\Checkout\Helpers\ReserveStock; /** * CartItemSchema class. @@ -346,15 +346,10 @@ protected function get_remaining_stock( \WC_Product $product ) { return null; } - $draft_order = wc()->session->get( 'store_api_draft_order', 0 ); - - if ( \class_exists( '\Automattic\WooCommerce\Checkout\Helpers\ReserveStock' ) ) { - $reserve_stock = new \Automattic\WooCommerce\Checkout\Helpers\ReserveStock(); - } else { - $reserve_stock = new \Automattic\WooCommerce\Blocks\StoreApi\Utilities\ReserveStock(); - } - + $draft_order = wc()->session->get( 'store_api_draft_order', 0 ); + $reserve_stock = new ReserveStock(); $reserved_stock = $reserve_stock->get_reserved_stock( $product, $draft_order ); + return $product->get_stock_quantity() - $reserved_stock; } diff --git a/src/StoreApi/Utilities/CartController.php b/src/StoreApi/Utilities/CartController.php index c4ffa1a2abf..12ca0184de4 100644 --- a/src/StoreApi/Utilities/CartController.php +++ b/src/StoreApi/Utilities/CartController.php @@ -3,6 +3,7 @@ use Automattic\WooCommerce\Blocks\StoreApi\Routes\RouteException; use Automattic\WooCommerce\Blocks\StoreApi\Utilities\NoticeHandler; +use Automattic\WooCommerce\Checkout\Helpers\ReserveStock; /** * Woo Cart Controller class. @@ -630,14 +631,9 @@ protected function get_product_quantity_in_cart( $product ) { * @return int */ protected function get_remaining_stock_for_product( $product ) { - if ( \class_exists( '\Automattic\WooCommerce\Checkout\Helpers\ReserveStock' ) ) { - $reserve_stock_controller = new \Automattic\WooCommerce\Checkout\Helpers\ReserveStock(); - } else { - $reserve_stock_controller = new \Automattic\WooCommerce\Blocks\StoreApi\Utilities\ReserveStock(); - } - - $draft_order = wc()->session->get( 'store_api_draft_order', 0 ); - $qty_reserved = $reserve_stock_controller->get_reserved_stock( $product, $draft_order ); + $reserve_stock = new ReserveStock(); + $draft_order = wc()->session->get( 'store_api_draft_order', 0 ); + $qty_reserved = $reserve_stock->get_reserved_stock( $product, $draft_order ); return $product->get_stock_quantity() - $qty_reserved; } diff --git a/src/StoreApi/Utilities/ReserveStock.php b/src/StoreApi/Utilities/ReserveStock.php deleted file mode 100644 index e856f9793bb..00000000000 --- a/src/StoreApi/Utilities/ReserveStock.php +++ /dev/null @@ -1,230 +0,0 @@ -<?php -namespace Automattic\WooCommerce\Blocks\StoreApi\Utilities; - -/** - * Stock Reservation class. - * Handle product stock reservation during checkout. - * - * @internal This API is used internally by Blocks--it is still in flux and may be subject to revisions. - */ -final class ReserveStock { - /** - * Is stock reservation enabled? - * - * @var boolean - */ - private $enabled = true; - - /** - * Constructor - */ - public function __construct() { - $this->enabled = get_option( 'wc_blocks_db_schema_version', 0 ) >= 260; - } - - /** - * Is stock reservation enabled? - * - * @return boolean - */ - protected function is_enabled() { - return $this->enabled; - } - - /** - * Query for any existing holds on stock for this item. - * - * @param \WC_Product $product Product to get reserved stock for. - * @param integer $exclude_order_id Optional order to exclude from the results. - * - * @return integer Amount of stock already reserved. - */ - public function get_reserved_stock( \WC_Product $product, $exclude_order_id = 0 ) { - global $wpdb; - - if ( ! $this->is_enabled() ) { - return 0; - } - - // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQL.NotPrepared - return (int) $wpdb->get_var( $this->get_query_for_reserved_stock( $product->get_stock_managed_by_id(), $exclude_order_id ) ); - } - - /** - * Put a temporary hold on stock for an order if enough is available. - * - * @throws ReserveStockException If stock cannot be reserved. - * - * @param \WC_Order $order Order object. - * @param int $minutes How long to reserve stock in minutes. Defaults to woocommerce_hold_stock_minutes. - */ - public function reserve_stock_for_order( \WC_Order $order, $minutes = 0 ) { - $minutes = $minutes ? $minutes : (int) get_option( 'woocommerce_hold_stock_minutes', 60 ); - - if ( ! $minutes || ! $this->is_enabled() ) { - return; - } - - try { - $items = array_filter( - $order->get_items(), - function( $item ) { - return $item->is_type( 'line_item' ) && $item->get_product() instanceof \WC_Product && $item->get_quantity() > 0; - } - ); - $rows = []; - - foreach ( $items as $item ) { - $product = $item->get_product(); - - if ( ! $product->is_in_stock() ) { - throw new ReserveStockException( - 'woocommerce_product_out_of_stock', - sprintf( - /* translators: %s: product name */ - __( '"%s" is out of stock and cannot be purchased.', 'woo-gutenberg-products-block' ), - $product->get_name() - ), - 400 - ); - } - - // If stock management is off, no need to reserve any stock here. - if ( ! $product->managing_stock() || $product->backorders_allowed() ) { - continue; - } - - $managed_by_id = $product->get_stock_managed_by_id(); - $rows[ $managed_by_id ] = isset( $rows[ $managed_by_id ] ) ? $rows[ $managed_by_id ] + $item->get_quantity() : $item->get_quantity(); - } - - if ( ! empty( $rows ) ) { - foreach ( $rows as $product_id => $quantity ) { - $this->reserve_stock_for_product( $product_id, $quantity, $order, $minutes ); - } - } - } catch ( ReserveStockException $e ) { - $this->release_stock_for_order( $order ); - throw $e; - } - } - - /** - * Release a temporary hold on stock for an order. - * - * @param \WC_Order $order Order object. - */ - public function release_stock_for_order( \WC_Order $order ) { - global $wpdb; - - if ( ! $this->is_enabled() ) { - return; - } - - $wpdb->delete( - $wpdb->wc_reserved_stock, - [ - 'order_id' => $order->get_id(), - ] - ); - } - - /** - * Reserve stock for a product by inserting rows into the DB. - * - * @throws ReserveStockException If a row cannot be inserted. - * - * @param int $product_id Product ID which is having stock reserved. - * @param int $stock_quantity Stock amount to reserve. - * @param \WC_Order $order Order object which contains the product. - * @param int $minutes How long to reserve stock in minutes. - */ - private function reserve_stock_for_product( $product_id, $stock_quantity, \WC_Order $order, $minutes ) { - global $wpdb; - - $query_for_stock = $this->get_query_for_stock( $product_id ); - $query_for_reserved_stock = $this->get_query_for_reserved_stock( $product_id, $order->get_id() ); - $required_stock = $stock_quantity; - - // Deals with legacy stock reservations from woo core. - $support_legacy_held_stock = ! \class_exists( '\Automattic\WooCommerce\Checkout\Helpers\ReserveStock' ) && absint( get_option( 'woocommerce_hold_stock_minutes', 0 ) ) > 0; - - if ( $support_legacy_held_stock ) { - $required_stock += wc_get_held_stock_quantity( wc_get_product( $product_id ) ); - } - - // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQL.NotPrepared - $result = $wpdb->query( - $wpdb->prepare( - " - INSERT INTO {$wpdb->wc_reserved_stock} ( `order_id`, `product_id`, `stock_quantity`, `timestamp`, `expires` ) - SELECT %d, %d, %d, NOW(), ( NOW() + INTERVAL %d MINUTE ) FROM DUAL - WHERE ( $query_for_stock FOR UPDATE ) - ( $query_for_reserved_stock FOR UPDATE ) >= %d - ON DUPLICATE KEY UPDATE `expires` = VALUES( `expires` ), `stock_quantity` = VALUES( `stock_quantity` ) - ", - $order->get_id(), - $product_id, - $stock_quantity, - $minutes, - $required_stock - ) - ); - // phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQL.NotPrepared - - if ( ! $result ) { - $product = wc_get_product( $product_id ); - throw new ReserveStockException( - 'product_not_enough_stock', - sprintf( - /* translators: %s: product name */ - __( 'Not enough units of %s are available in stock to fulfil this order.', 'woo-gutenberg-products-block' ), - $product ? $product->get_name() : '#' . $product_id - ), - 400 - ); - } - } - - /** - * Returns query statement for getting current `_stock` of a product. - * - * @todo Once merged to woo core data store, this method can be removed. - * @internal MAX function below is used to make sure result is a scalar. - * @param int $product_id Product ID. - * @return string|void Query statement. - */ - private function get_query_for_stock( $product_id ) { - global $wpdb; - return $wpdb->prepare( - " - SELECT COALESCE ( MAX( meta_value ), 0 ) FROM $wpdb->postmeta as meta_table - WHERE meta_table.meta_key = '_stock' - AND meta_table.post_id = %d - ", - $product_id - ); - } - - /** - * Returns query statement for getting reserved stock of a product. - * - * @param int $product_id Product ID. - * @param integer $exclude_order_id Optional order to exclude from the results. - * @return string|void Query statement. - */ - private function get_query_for_reserved_stock( $product_id, $exclude_order_id = 0 ) { - global $wpdb; - return $wpdb->prepare( - " - SELECT COALESCE( SUM( stock_table.`stock_quantity` ), 0 ) FROM $wpdb->wc_reserved_stock stock_table - LEFT JOIN $wpdb->posts posts ON stock_table.`order_id` = posts.ID - WHERE posts.post_status IN ( 'wc-checkout-draft', 'wc-pending' ) - AND stock_table.`expires` > NOW() - AND stock_table.`product_id` = %d - AND stock_table.`order_id` != %d - ", - $product_id, - $exclude_order_id - ); - } -} diff --git a/src/StoreApi/Utilities/ReserveStockException.php b/src/StoreApi/Utilities/ReserveStockException.php deleted file mode 100644 index 19546fd54ef..00000000000 --- a/src/StoreApi/Utilities/ReserveStockException.php +++ /dev/null @@ -1,57 +0,0 @@ -<?php -namespace Automattic\WooCommerce\Blocks\StoreApi\Utilities; - -/** - * ReserveStockException class. - * Exceptions for stock reservation. - * - * @internal This API is used internally by Blocks--it is still in flux and may be subject to revisions. - */ -class ReserveStockException extends \Exception { - /** - * Sanitized error code. - * - * @var string - */ - protected $error_code; - - /** - * Error extra data. - * - * @var array - */ - protected $error_data; - - /** - * Setup exception. - * - * @param string $code Machine-readable error code, e.g `woocommerce_invalid_product_id`. - * @param string $message User-friendly translated error message, e.g. 'Product ID is invalid'. - * @param int $http_status_code Proper HTTP status code to respond with, e.g. 400. - * @param array $data Extra error data. - */ - public function __construct( $code, $message, $http_status_code = 400, $data = array() ) { - $this->error_code = $code; - $this->error_data = $data; - - parent::__construct( $message, $http_status_code ); - } - - /** - * Returns the error code. - * - * @return string - */ - public function getErrorCode() { - return $this->error_code; - } - - /** - * Returns error data. - * - * @return array - */ - public function getErrorData() { - return $this->error_data; - } -} diff --git a/tests/php/StoreApi/Utilities/ReserveStock.php b/tests/php/StoreApi/Utilities/ReserveStock.php index 78aafb8f65c..444d42b61f7 100644 --- a/tests/php/StoreApi/Utilities/ReserveStock.php +++ b/tests/php/StoreApi/Utilities/ReserveStock.php @@ -8,7 +8,7 @@ use PHPUnit\Framework\TestCase; use \WC_Helper_Order as OrderHelper; use \WC_Helper_Product as ProductHelper; -use Automattic\WooCommerce\Blocks\StoreApi\Utilities\ReserveStock; +use Automattic\WooCommerce\Checkout\Helpers\ReserveStock; /** * ReserveStock Utility Tests. @@ -45,7 +45,7 @@ public function test_reserve_stock_for_order() { /** * Test that trying to reserve stock too much throws an exception. * - * @expectedException Automattic\WooCommerce\Blocks\StoreApi\Utilities\ReserveStockException + * @expectedException Automattic\WooCommerce\Checkout\Helpers\ReserveStockException */ public function test_reserve_stock_for_order_throws_exception() { $class = new ReserveStock();