Merge pull request #104 from liquidweb/fix/billing-email-validation

Don't let existing, invalid emails break migrations

Author: stevegrunwell

.travis.yml

@@ -51,7 +51,7 @@ matrix:
   allow_failures:
     - name: Code Coverage
       php: 7.2
-      env: WP_VERSION=latest WP_MULTISITE=0 WC_VERSION=latest RUN_CODE_COVERAGE=1
+      env: WP_VERSION=latest WC_VERSION=latest RUN_CODE_COVERAGE=1
     - name: Bleeding Edge
       php: 7.3
       env: WP_VERSION=trunk WC_VERSION=latest

includes/class-woocommerce-custom-orders-table.php

@@ -160,6 +160,24 @@ public static function populate_order_from_post_meta( $order ) {
 					case 'shipping_index':
 						break;
 
+					/*
+					 * Migration isn't the time to validate (and potentially throw exceptions);
+					 * if it was accepted into WooCommerce core, let it persist.
+					 *
+					 * If we're unable to set an email address due to $order->set_billing_email(),
+					 * try to circumvent the check by using reflection to call the protected
+					 * $order->set_address_prop() method.
+					 */
+					case 'billing_email':
+						try {
+							$order->set_billing_email( $meta );
+						} catch ( WC_Data_Exception $e ) {
+							$method = new ReflectionMethod( $order, 'set_address_prop' );
+							$method->setAccessible( true );
+							$method->invoke( $order, 'email', 'billing', $meta );
+						}
+						break;
+
 					case 'prices_include_tax':
 						$order->set_prices_include_tax( 'yes' === $meta );
 						break;

tests/test-cli.php

@@ -204,8 +204,8 @@ public function test_migrate_handles_exceptions() {
 		$order_ids = $this->generate_orders( 3 );
 		$this->toggle_use_custom_table( true );
 
-		// Break the billing email on the first item.
-		update_post_meta( $order_ids[0], '_billing_email', 'this is not an email address' );
+		// Set a duplicate order key for the middle order.
+		update_post_meta( $order_ids[1], '_order_key', get_post_meta( $order_ids[0], '_order_key' ) );
 
 		$this->cli->migrate();
 
@@ -215,7 +215,7 @@ public function test_migrate_handles_exceptions() {
 			'Expected to only see two orders in the custom table.'
 		);
 
-		$this->assertContains( $order_ids[0], $this->get_skipped_ids() );
+		$this->assertContains( $order_ids[1], $this->get_skipped_ids() );
 	}
 
 	public function test_migrate_with_duplicate_ids() {

tests/test-core.php

@@ -15,6 +15,24 @@ public function test_order_row_exists() {
 		$this->assertFalse( wc_custom_order_table()->row_exists( $order->get_id() + 1 ) );
 	}
 
+	/**
+	 * @ticket https://github.com/liquidweb/woocommerce-custom-orders-table/issues/98
+	 */
+	public function test_populate_order_from_post_meta_handles_invalid_billing_emails() {
+		$this->toggle_use_custom_table( false );
+		$order = WC_Helper_Order::create_order();
+		update_post_meta( $order->get_id(), '_billing_email', 'this is an invalid email address' );
+		$this->toggle_use_custom_table( true );
+
+		WooCommerce_Custom_Orders_Table::populate_order_from_post_meta( $order );
+
+		$this->assertSame(
+			'this is an invalid email address',
+			$order->get_billing_email(),
+			'Don\'t let an invalid email address cause a migration failure.'
+		);
+	}
+
 	public function test_migrate_to_post_meta() {
 		$order    = WC_Helper_Order::create_order();
 		$row      = $this->get_order_row( $order->get_id() );

tests/test-order-data-store.php

@@ -287,13 +287,23 @@ public function test_populate_from_meta_handles_wc_data_exceptions() {
 		$order = WC_Helper_Order::create_order();
 		$this->toggle_use_custom_table( true );
 
-		// Give an invalid billing email.
-		update_post_meta( $order->get_id(), '_billing_email', 'this is not an email address' );
-
 		// Refresh the instance.
 		$order = wc_get_order( $order->get_id() );
 
+		add_action( 'woocommerce_order_object_updated_props', array( $this, 'throw_wc_data_exception' ) );
+
 		$this->assertInstanceOf( 'WP_Error', $order->get_data_store()->populate_from_meta( $order ) );
+
+		remove_action( 'woocommerce_order_object_updated_props', array( $this, 'throw_wc_data_exception' ) );
+	}
+
+	/**
+	 * Hooked method that simply throws a WC_Data_Exception exception.
+	 *
+	 * @throws WC_Data_Exception
+	 */
+	public function throw_wc_data_exception() {
+		throw new WC_Data_Exception( 'test-data-exception', 'A sample WC_Data_Exception' );
 	}
 
 	/**