woocommerce/woocommerce

Unintended product change on order_item_product with non-unique SKU's

Closed

#27,989 opened on Apr 28, 2020

 (3 comments) (0 reactions) (0 assignees)PHP (10,695 forks)batch import
BugGood First IssueOrder ManagementREST/Store API/WebhooksRubikplugin: woocommerce

Repository metrics

Stars
 (10,294 stars)
PR merge metrics
 (Avg merge 9d 12h) (423 merged PRs in 30d)

Description

If multiple products can have the same SKU, updating an order through the WooCommerce API (tested v2, v3 and v4) can unintentionally change products within an order.

To reproduce:

  1. Add filter to functions.php add_filter( 'wc_product_has_unique_sku', '__return_false' );
  2. Create product 1 with SKU non-unique. (product_id example: 1111)
  3. Create product 2 with the same SKU non-unique. (product_id example: 2222)
  4. Create an order with product 2 through the checkout
  5. GET the order from the WooCommerce API, (observe that product_id = 2222)
  6. PUT the exact same payload.
  7. GET the order from the WooCommerce API, (observe that product_id = 1111)

Cause

In wc/v2/orders and wc/v3/orders this is caused by https://github.com/woocommerce/woocommerce-rest-api/blob/58bab7a5563ddb0f4eede348ed3dbd1d4bca86d2/src/Controllers/Version2/class-wc-rest-orders-v2-controller.php#L597-L609

In wc/v4/orders it's https://github.com/woocommerce/woocommerce-rest-api/blob/a518b9cfd1dec42a59aa10240a31782bc1d43d13/src/Controllers/Version4/Requests/OrderRequest.php#L177-L178

If the order already exists, and a product is set, it should always have an SKU, so it wil (almost) always use the SKU method for finding the product_id. When multiple products can have the same SKU, wc_get_product_id_by_sku() always uses the first one found.

Possible solutions:

Use product_id first, SKU second.

This seems to be more intuitive (and might be faster), however it would probably mean breaking backward compatibility for API clients that only change the SKU, and not the product id when editing an order.

Add a check for wc_product_has_unique_sku

If apply_filters( 'wc_product_has_unique_sku', .... ); return false, it doesn't make sense to use the SKU to change the product in the API, and wc_get_product_id_by_sku() should not be used. However, not all data that is passed in the wc_product_has_unique_sku filter is available at this point (possibly no known product ID).

Contributor guide