Merge pull request #64 from liquidweb/fix/duplicate-ids-on-update-pos…

…t-meta Prevent duplicate IDs on order/refund save
liquidweb · May 25, 2018 · e7c8b52 · e7c8b52
2 parents 5980d3e + 837f15c
commit e7c8b52
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 95 deletions.
diff --git a/includes/class-wc-order-data-store-custom-table.php b/includes/class-wc-order-data-store-custom-table.php
@@ -14,13 +14,6 @@
  */
 class WC_Order_Data_Store_Custom_Table extends WC_Order_Data_Store_CPT {
 
-	/**
-	 * Set to true when creating so we know to insert meta data.
-	 *
-	 * @var boolean
-	 */
-	protected $creating = false;
-
 	/**
 	 * Hook into WooCommerce database queries related to orders.
 	 */
@@ -30,17 +23,6 @@ public function __construct() {
 		add_filter( 'woocommerce_order_data_store_cpt_get_orders_query', 'WooCommerce_Custom_Orders_Table_Filters::filter_database_queries', 10, 2 );
 	}
 
-	/**
-	 * Create a new order in the database.
-	 *
-	 * @param WC_Order $order The order object, passed by reference.
-	 */
-	public function create( &$order ) {
-		$this->creating = true;
-
-		parent::create( $order );
-	}
-
 	/**
 	 * Delete an order from the database.
 	 *
@@ -99,20 +81,15 @@ public function get_order_data_from_table( $order ) {
 		global $wpdb;
 
 		$table = wc_custom_order_table()->get_table_name();
-		$data  = $wpdb->get_row( $wpdb->prepare(
+		$data  = (array) $wpdb->get_row( $wpdb->prepare(
 			'SELECT * FROM ' . esc_sql( $table ) . ' WHERE order_id = %d LIMIT 1',
 			$order->get_id()
 		), ARRAY_A ); // WPCS: DB call OK.
 
-		// If no matches were found, this record needs to be created.
-		if ( null === $data ) {
-			$this->creating = true;
-
-			return array();
-		}
-
 		// Expand anything that might need assistance.
-		$data['prices_include_tax'] = wc_string_to_bool( $data['prices_include_tax'] );
+		if ( isset( $data['prices_include_tax'] ) ) {
+			$data['prices_include_tax'] = wc_string_to_bool( $data['prices_include_tax'] );
+		}
 
 		return $data;
 	}
@@ -189,7 +166,7 @@ protected function update_post_meta( &$order ) {
 		}
 
 		// Insert or update the database record.
-		if ( $this->creating ) {
+		if ( ! wc_custom_order_table()->row_exists( $order_data['order_id'] ) ) {
 			$inserted = $wpdb->insert( $table, $order_data ); // WPCS: DB call OK.
 
 			if ( 1 !== $inserted ) {
@@ -205,9 +182,6 @@ protected function update_post_meta( &$order ) {
 				update_post_meta( $order->get_id(), '_billing_email', $order->get_billing_email() );
 				update_post_meta( $order->get_id(), '_customer_user', $order->get_customer_id() );
 			}
-
-			$this->creating = false;
-
 		} else {
 			$changes = array_intersect_key( $order_data, $order->get_changes() );
 

diff --git a/includes/class-wc-order-refund-data-store-custom-table.php b/includes/class-wc-order-refund-data-store-custom-table.php
@@ -14,24 +14,6 @@
  */
 class WC_Order_Refund_Data_Store_Custom_Table extends WC_Order_Refund_Data_Store_CPT {
 
-	/**
-	 * Set to true when creating so we know to insert meta data.
-	 *
-	 * @var boolean
-	 */
-	protected $creating = false;
-
-	/**
-	 * Create a new refund in the database.
-	 *
-	 * @param WC_Order_Refund $refund The refund object, passed by reference.
-	 */
-	public function create( &$refund ) {
-		$this->creating = true;
-
-		parent::create( $refund );
-	}
-
 	/**
 	 * Read refund data.
 	 *
@@ -66,20 +48,15 @@ protected function read_order_data( &$refund, $post_object ) {
 	public function get_order_data_from_table( $refund ) {
 		global $wpdb;
 
-		$data = $wpdb->get_row( $wpdb->prepare(
+		$data = (array) $wpdb->get_row( $wpdb->prepare(
 			'SELECT * FROM ' . esc_sql( wc_custom_order_table()->get_table_name() ) . ' WHERE order_id = %d LIMIT 1',
 			$refund->get_id()
 		), ARRAY_A ); // WPCS: DB call OK.
 
-		// If no matches were found, this record needs to be created.
-		if ( null === $data ) {
-			$this->creating = true;
-
-			return array();
-		}
-
 		// Expand anything that might need assistance.
-		$data['prices_include_tax'] = wc_string_to_bool( $data['prices_include_tax'] );
+		if ( isset( $data['prices_include_tax'] ) ) {
+			$data['prices_include_tax'] = wc_string_to_bool( $data['prices_include_tax'] );
+		}
 
 		return $data;
 	}
@@ -113,15 +90,12 @@ protected function update_post_meta( &$refund ) {
 		);
 
 		// Insert or update the database record.
-		if ( $this->creating ) {
+		if ( ! wc_custom_order_table()->row_exists( $refund_data['order_id'] ) ) {
 			$inserted = $wpdb->insert( $table, $refund_data ); // WPCS: DB call OK.
 
 			if ( 1 !== $inserted ) {
 				return;
 			}
-
-			$this->creating = false;
-
 		} else {
 			$refund_data = array_intersect_key( $refund_data, $refund->get_changes() );
 

diff --git a/includes/class-woocommerce-custom-orders-table.php b/includes/class-woocommerce-custom-orders-table.php
@@ -65,6 +65,24 @@ public function get_table_name() {
 		return apply_filters( 'wc_customer_order_table_name', $this->table_name );
 	}
 
+	/**
+	 * Simple helper method to determine if a row already exists for the given order ID.
+	 *
+	 * @global $wpdb
+	 *
+	 * @param int $order_id The order ID.
+	 *
+	 * @return bool Whether or not a row already exists for this order ID.
+	 */
+	public function row_exists( $order_id ) {
+		global $wpdb;
+
+		return (bool) $wpdb->get_var( $wpdb->prepare(
+			'SELECT COUNT(order_id) FROM ' . esc_sql( $this->get_table_name() ) . ' WHERE order_id = %d',
+			$order_id
+		) );
+	}
+
 	/**
 	 * Retrieve the database table column => post_meta mapping.
 	 *

diff --git a/tests/test-core.php b/tests/test-core.php
@@ -0,0 +1,17 @@
+<?php
+/**
+ * Tests for the core WooCommerce_Custom_Orders_Table class.
+ *
+ * @package WooCommerce_Custom_Orders_Table
+ * @author  Liquid Web
+ */
+
+class CoreTest extends TestCase {
+
+	public function test_order_row_exists() {
+		$order = WC_Helper_Order::create_order();
+
+		$this->assertTrue( wc_custom_order_table()->row_exists( $order->get_id() ) );
+		$this->assertFalse( wc_custom_order_table()->row_exists( $order->get_id() + 1 ) );
+	}
+}
diff --git a/tests/test-order-data-store.php b/tests/test-order-data-store.php
@@ -8,29 +8,6 @@
 
 class OrderDataStoreTest extends TestCase {
 
-	/**
-	 * @requires PHP 5.4 In order to support inline closures for hook callbacks.
-	 */
-	public function test_create() {
-		$instance = new WC_Order_Data_Store_Custom_Table();
-		$property = new ReflectionProperty( $instance, 'creating' );
-		$property->setAccessible( true );
-		$order    = new WC_Order( wp_insert_post( array(
-			'post_type' => 'product',
-		) ) );
-
-		add_action( 'wp_insert_post', function () use ( $property, $instance ) {
-			$this->assertTrue(
-				$property->getValue( $instance ),
-				'As an order is being created, WC_Order_Data_Store_Custom_Table::$creating should be true'
-			);
-		} );
-
-		$instance->create( $order );
-
-		$this->assertEquals( 1, did_action( 'wp_insert_post' ), 'Expected the "wp_insert_post" action to have been fired.' );
-	}
-
 	public function test_loading_a_product_can_automatically_populate_from_meta() {
 		$this->toggle_use_custom_table( false );
 		$order_id = WC_Helper_Order::create_order()->get_id();
@@ -108,7 +85,7 @@ public function test_update_post_meta_for_new_order() {
 		$order->set_customer_ip_address( '127.0.0.1' );
 		$order->set_customer_user_agent( 'PHPUnit' );
 
-		$this->invoke_update_post_meta( $order, true );
+		$this->invoke_update_post_meta( $order );
 
 		$row = $this->get_order_row( $order->get_id() );
 
@@ -117,6 +94,20 @@ public function test_update_post_meta_for_new_order() {
 		$this->assertEquals( 'PHPUnit', $row['customer_user_agent'] );
 	}
 
+	/**
+	 * @link https://github.com/liquidweb/woocommerce-custom-orders-table/issues/49
+	 */
+	public function test_update_post_meta_for_existing_order_id() {
+		$order = WC_Helper_Order::create_order();
+		$order->set_customer_user_agent( 'PHPUnit' );
+
+		$this->invoke_update_post_meta( $order );
+
+		$row = $this->get_order_row( $order->get_id() );
+
+		$this->assertEquals( 'PHPUnit', $row['customer_user_agent'] );
+	}
+
 	public function test_get_order_id_by_order_key() {
 		$order = WC_Helper_Order::create_order();
 		$instance = new WC_Order_Data_Store_Custom_Table();
@@ -320,17 +311,10 @@ public function test_backfill_postmeta_returns_early_if_table_row_is_empty() {
 	/**
 	 * Shortcut for setting up reflection methods + properties for update_post_meta().
 	 *
-	 * @param WC_Order $order    The order object, passed by reference.
-	 * @param bool     $creating Optional. The value 'creating' property in the new instance.
-	 *                           Default is false.
+	 * @param WC_Order $order The order object, passed by reference.
 	 */
-	protected function invoke_update_post_meta( &$order, $creating = false ) {
+	protected function invoke_update_post_meta( &$order ) {
 		$instance = new WC_Order_Data_Store_Custom_Table();
-
-		$property = new ReflectionProperty( $instance, 'creating' );
-		$property->setAccessible( true );
-		$property->setValue( $instance, (bool) $creating );
-
 		$method   = new ReflectionMethod( $instance, 'update_post_meta' );
 		$method->setAccessible( true );
 		$method->invokeArgs( $instance, array( &$order ) );

diff --git a/tests/test-order-refund-data-store.php b/tests/test-order-refund-data-store.php
@@ -50,4 +50,35 @@ public function test_update_post_meta() {
 		$this->assertEquals( $this->user, $row['refunded_by'] );
 		$this->assertEquals( 'For testing', $row['reason'] );
 	}
+
+	/**
+	 * @link https://github.com/liquidweb/woocommerce-custom-orders-table/issues/49
+	 */
+	public function test_update_post_meta_handles_duplicate_ids() {
+		$order  = WC_Helper_Order::create_order();
+		$refund = wc_create_refund( array(
+			'order_id' => $order->get_id(),
+			'amount'   => 7,
+			'reason'   => 'For testing',
+		) );
+		$refund->set_reason( 'Different reason' );
+
+		$this->invoke_update_post_meta( $refund );
+
+		$row = $this->get_order_row( $refund->get_id() );
+
+		$this->assertEquals( 'Different reason', $row['reason'] );
+	}
+
+	/**
+	 * Shortcut for setting up reflection methods + properties for update_post_meta().
+	 *
+	 * @param WC_Order_Refund $refund   The order refund object, passed by reference.
+	 */
+	protected function invoke_update_post_meta( &$refund ) {
+		$instance = new WC_Order_Refund_Data_Store_Custom_Table();
+		$method   = new ReflectionMethod( $instance, 'update_post_meta' );
+		$method->setAccessible( true );
+		$method->invokeArgs( $instance, array( &$refund ) );
+	}
 }