Refactor customer name retrieval (https://github.com/woocommerce/woocommerce-admin/pull/2418)

* Return early from creating customer if order is false

* Refactor customer first and last name methods

* Separate customer name tests to create guest orders

* Check for parent_id before retrieving customer ID in refund

* Use registered customer ID in import report tests

* Compare absolute integer when checking for valid customer ID

* Check for order before getting customer ID when retrieving existing customer ID

plugins/woocommerce-admin/includes/class-wc-admin-order-refund.php

@@ -49,6 +49,11 @@ class WC_Admin_Order_Refund extends WC_Order_Refund {
 	 */
 	public function get_report_customer_id() {
 		$parent_order = wc_get_order( $this->get_parent_id() );
+
+		if ( ! $parent_order ) {
+			return null;
+		}
+
 		return WC_Admin_Reports_Customers_Data_Store::get_or_create_customer_from_order( $parent_order );
 	}
 

plugins/woocommerce-admin/includes/class-wc-admin-order.php

@@ -66,4 +66,34 @@ class WC_Admin_Order extends WC_Order {
 	public function is_returning_customer() {
 		return WC_Admin_Reports_Orders_Stats_Data_Store::is_returning_customer( $this );
 	}
+
+	/**
+	 * Get the customer's first name.
+	 */
+	public function get_customer_first_name() {
+		if ( $this->get_user_id() ) {
+			return get_user_meta( $this->get_user_id(), 'first_name', true );
+		}
+
+		if ( '' !== $this->get_billing_first_name( 'edit' ) ) {
+			return $this->get_billing_first_name( 'edit' );
+		} else {
+			return $this->get_shipping_first_name( 'edit' );
+		}
+	}
+
+	/**
+	 * Get the customer's last name.
+	 */
+	public function get_customer_last_name() {
+		if ( $this->get_user_id() ) {
+			return get_user_meta( $this->get_user_id(), 'last_name', true );
+		}
+
+		if ( '' !== $this->get_billing_last_name( 'edit' ) ) {
+			return $this->get_billing_last_name( 'edit' );
+		} else {
+			return $this->get_shipping_last_name( 'edit' );
+		}
+	}
 }

plugins/woocommerce-admin/includes/data-stores/class-wc-admin-reports-customers-data-store.php

@@ -438,6 +438,10 @@ class WC_Admin_Reports_Customers_Data_Store extends WC_Admin_Reports_Data_Store
 	 * @return int|bool
 	 */
 	public static function get_existing_customer_id_from_order( $order ) {
+		if ( ! $order ) {
+			return false;
+		}
+
 		$user_id = $order->get_customer_id();
 
 		if ( 0 === $user_id ) {
@@ -460,6 +464,10 @@ class WC_Admin_Reports_Customers_Data_Store extends WC_Admin_Reports_Data_Store
 	 * @return int|bool
 	 */
 	public static function get_or_create_customer_from_order( $order ) {
+		if ( ! $order ) {
+			return false;
+		}
+
 		global $wpdb;
 		$returning_customer_id = self::get_existing_customer_id_from_order( $order );
 
@@ -467,17 +475,16 @@ class WC_Admin_Reports_Customers_Data_Store extends WC_Admin_Reports_Data_Store
 			return $returning_customer_id;
 		}
 
-		$customer_name = self::get_customer_name( $order->get_user_id(), $order );
+		$data   = array(
-		$data          = array(
+			'first_name'       => $order->get_customer_first_name(),
-			'first_name'       => $customer_name[0],
+			'last_name'        => $order->get_customer_last_name(),
-			'last_name'        => $customer_name[1],
 			'email'            => $order->get_billing_email( 'edit' ),
 			'city'             => $order->get_billing_city( 'edit' ),
 			'postcode'         => $order->get_billing_postcode( 'edit' ),
 			'country'          => $order->get_billing_country( 'edit' ),
 			'date_last_active' => date( 'Y-m-d H:i:s', $order->get_date_created( 'edit' )->getTimestamp() ),
 		);
-		$format        = array(
+		$format = array(
 			'%s',
 			'%s',
 			'%s',
@@ -512,42 +519,6 @@ class WC_Admin_Reports_Customers_Data_Store extends WC_Admin_Reports_Data_Store
 		return $result ? $customer_id : false;
 	}
 
-	/**
-	 * Try to get a customer name from user profile or order information.
-	 *
-	 * @param int      $user_id User ID.
-	 * @param WC_Order $order Order made by customer.
-	 * @return array
-	 */
-	public static function get_customer_name( $user_id = 0, $order = null ) {
-		$first_name = '';
-		$last_name  = '';
-
-		if (
-			$user_id &&
-			(
-				get_user_meta( $user_id, 'first_name', true ) ||
-				get_user_meta( $user_id, 'last_name', true )
-			)
-		) {
-			$first_name = get_user_meta( $user_id, 'first_name', true );
-			$last_name  = get_user_meta( $user_id, 'last_name', true );
-		} elseif ( $order ) {
-			if (
-				$order->get_billing_first_name( 'edit' ) ||
-				$order->get_billing_last_name( 'edit' )
-			) {
-				$first_name = $order->get_billing_first_name( 'edit' );
-				$last_name  = $order->get_billing_last_name( 'edit' );
-			} else {
-				$first_name = $order->get_shipping_first_name( 'edit' );
-				$last_name  = $order->get_shipping_last_name( 'edit' );
-			}
-		}
-
-		return apply_filters( 'woocommerce_reports_customer_name', array( $first_name, $last_name ), $order );
-	}
-
 	/**
 	 * Retrieve a guest ID (when user_id is null) by email.
 	 *
@@ -627,13 +598,22 @@ class WC_Admin_Reports_Customers_Data_Store extends WC_Admin_Reports_Data_Store
 			return false;
 		}
 
-		$customer_name = self::get_customer_name( $user_id, $customer->get_last_order() );
+		$last_order = $customer->get_last_order();
-		$last_active   = $customer->get_meta( 'wc_last_active', true, 'edit' );
+
-		$data          = array(
+		if ( ! $last_order ) {
+			$first_name = get_user_meta( $user_id, 'first_name', true );
+			$last_name  = get_user_meta( $user_id, 'last_name', true );
+		} else {
+			$first_name = $last_order->get_customer_first_name();
+			$last_name  = $last_order->get_customer_last_name();
+		}
+
+		$last_active = $customer->get_meta( 'wc_last_active', true, 'edit' );
+		$data        = array(
 			'user_id'          => $user_id,
 			'username'         => $customer->get_username( 'edit' ),
-			'first_name'       => $customer_name[0],
+			'first_name'       => $first_name,
-			'last_name'        => $customer_name[1],
+			'last_name'        => $last_name,
 			'email'            => $customer->get_email( 'edit' ),
 			'city'             => $customer->get_billing_city( 'edit' ),
 			'postcode'         => $customer->get_billing_postcode( 'edit' ),
@@ -641,7 +621,7 @@ class WC_Admin_Reports_Customers_Data_Store extends WC_Admin_Reports_Data_Store
 			'date_registered'  => $customer->get_date_created( 'edit' )->date( WC_Admin_Reports_Interval::$sql_datetime_format ),
 			'date_last_active' => $last_active ? date( 'Y-m-d H:i:s', $last_active ) : null,
 		);
-		$format        = array(
+		$format      = array(
 			'%d',
 			'%s',
 			'%s',
@@ -682,7 +662,7 @@ class WC_Admin_Reports_Customers_Data_Store extends WC_Admin_Reports_Data_Store
 	protected static function is_valid_customer( $user_id ) {
 		$customer = new WC_Customer( $user_id );
 
-		if ( $customer->get_id() != $user_id ) {
+		if ( absint( $customer->get_id() ) !== absint( $user_id ) ) {
 			return false;
 		}
 

plugins/woocommerce-admin/tests/api/reports-customers.php

@@ -328,21 +328,77 @@ class WC_Tests_API_Reports_Customers extends WC_REST_Unit_Test_Case {
 	}
 
 	/**
-	 * Test customer first and last name.
+	 * Test customer user profile name priority.
 	 */
-	public function test_customer_name() {
+	public function test_customer_user_profile_name() {
 		wp_set_current_user( $this->user );
 
 		$customer = wp_insert_user(
 			array(
+				'first_name' => 'Tyrion',
+				'last_name'  => 'Lanister',
 				'user_login' => 'daenerys',
 				'user_pass'  => null,
 				'role'       => 'customer',
 			)
 		);
 
-		// Test shipping name and empty billing name.
 		$order = WC_Helper_Order::create_order( $customer );
+		$order->set_billing_first_name( 'Jon' );
+		$order->set_billing_last_name( 'Snow' );
+		$order->set_status( 'completed' );
+		$order->set_total( 100 );
+		$order->save();
+
+		WC_Helper_Queue::run_all_pending();
+
+		$request  = new WP_REST_Request( 'GET', $this->endpoint );
+		$response = $this->server->dispatch( $request );
+		$reports  = $response->get_data();
+		$headers  = $response->get_headers();
+
+		$this->assertEquals( 200, $response->get_status() );
+		$this->assertCount( 1, $reports );
+		$this->assertEquals( 'Tyrion Lanister', $reports[0]['name'] );
+	}
+
+
+	/**
+	 * Test customer billing name priority.
+	 */
+	public function test_customer_billing_name() {
+		wp_set_current_user( $this->user );
+
+		// Test shipping name and empty billing name on a guest account.
+		$order = WC_Helper_Order::create_order( 0 );
+		$order->set_billing_first_name( 'Jon' );
+		$order->set_billing_last_name( 'Snow' );
+		$order->set_shipping_first_name( 'IgnoredFirstName' );
+		$order->set_shipping_last_name( 'IgnoredLastName' );
+		$order->set_status( 'completed' );
+		$order->set_total( 100 );
+		$order->save();
+
+		WC_Helper_Queue::run_all_pending();
+
+		$request  = new WP_REST_Request( 'GET', $this->endpoint );
+		$response = $this->server->dispatch( $request );
+		$reports  = $response->get_data();
+		$headers  = $response->get_headers();
+
+		$this->assertEquals( 200, $response->get_status() );
+		$this->assertCount( 1, $reports );
+		$this->assertEquals( 'Jon Snow', $reports[0]['name'] );
+	}
+
+	/**
+	 * Test customer shipping name priority.
+	 */
+	public function test_customer_shipping_name() {
+		wp_set_current_user( $this->user );
+
+		// Test shipping name and empty billing name on a guest account.
+		$order = WC_Helper_Order::create_order( 0 );
 		$order->set_billing_first_name( '' );
 		$order->set_billing_last_name( '' );
 		$order->set_shipping_first_name( 'Daenerys' );
@@ -361,45 +417,5 @@ class WC_Tests_API_Reports_Customers extends WC_REST_Unit_Test_Case {
 		$this->assertEquals( 200, $response->get_status() );
 		$this->assertCount( 1, $reports );
 		$this->assertEquals( 'Daenerys Targaryen', $reports[0]['name'] );
-
-		// Test billing name.
-		$order->set_billing_first_name( 'Jon' );
-		$order->set_billing_last_name( 'Snow' );
-		$order->save();
-		do_action( 'woocommerce_update_customer', $customer );
-
-		WC_Helper_Queue::run_all_pending();
-
-		$request = new WP_REST_Request( 'GET', $this->endpoint );
-		$request->set_query_params( array( 'orderby' => 'username' ) ); // Cache busting.
-		$response = $this->server->dispatch( $request );
-		$reports  = $response->get_data();
-		$headers  = $response->get_headers();
-
-		$this->assertEquals( 200, $response->get_status() );
-		$this->assertCount( 1, $reports );
-		$this->assertEquals( 'Jon Snow', $reports[0]['name'] );
-
-		// Test user profile name.
-		wp_update_user(
-			array(
-				'ID'         => $customer,
-				'first_name' => 'Tyrion',
-				'last_name'  => 'Lanister',
-			)
-		);
-		do_action( 'woocommerce_update_customer', $customer );
-
-		WC_Helper_Queue::run_all_pending();
-
-		$request = new WP_REST_Request( 'GET', $this->endpoint );
-		$request->set_query_params( array( 'orderby' => 'name' ) ); // Cache busting.
-		$response = $this->server->dispatch( $request );
-		$reports  = $response->get_data();
-		$headers  = $response->get_headers();
-
-		$this->assertEquals( 200, $response->get_status() );
-		$this->assertCount( 1, $reports );
-		$this->assertEquals( 'Tyrion Lanister', $reports[0]['name'] );
 	}
 }

plugins/woocommerce-admin/tests/api/reports-import.php

@@ -95,11 +95,11 @@ class WC_Tests_API_Reports_Import extends WC_REST_Unit_Test_Case {
 		$product->set_regular_price( 25 );
 		$product->save();
 
-		$order_1 = WC_Helper_Order::create_order( 1, $product );
+		$order_1 = WC_Helper_Order::create_order( $this->customer, $product );
 		$order_1->set_status( 'completed' );
 		$order_1->set_date_created( time() - ( 3 * DAY_IN_SECONDS ) );
 		$order_1->save();
-		$order_2 = WC_Helper_Order::create_order( 1, $product );
+		$order_2 = WC_Helper_Order::create_order( $this->customer, $product );
 		$order_2->set_total( 100 );
 		$order_2->set_status( 'completed' );
 		$order_2->save();
@@ -127,7 +127,7 @@ class WC_Tests_API_Reports_Import extends WC_REST_Unit_Test_Case {
 		$reports  = $response->get_data();
 
 		$this->assertEquals( 200, $response->get_status() );
-		$this->assertCount( 2, $reports );
+		$this->assertCount( 1, $reports );
 		$this->assertEquals( $this->customer, $reports[0]['user_id'] );
 
 		$request  = new WP_REST_Request( 'GET', '/wc/v4/reports/orders' );
@@ -172,7 +172,7 @@ class WC_Tests_API_Reports_Import extends WC_REST_Unit_Test_Case {
 		$reports  = $response->get_data();
 
 		$this->assertEquals( 200, $response->get_status() );
-		$this->assertCount( 2, $reports );
+		$this->assertCount( 1, $reports );
 		$this->assertEquals( 'Steve User', $reports[0]['name'] );
 
 		$request = new WP_REST_Request( 'GET', '/wc/v4/reports/orders' );
@@ -303,7 +303,7 @@ class WC_Tests_API_Reports_Import extends WC_REST_Unit_Test_Case {
 
 		// Create 5 completed orders.
 		for ( $i = 0; $i < 5; $i++ ) {
-			$order = WC_Helper_Order::create_order( 1, $product );
+			$order = WC_Helper_Order::create_order( $this->customer, $product );
 			$order->set_status( 'completed' );
 			$order->set_date_created( time() - ( ( $i + 1 ) * DAY_IN_SECONDS ) );
 			$order->save();
@@ -314,7 +314,7 @@ class WC_Tests_API_Reports_Import extends WC_REST_Unit_Test_Case {
 		$order->save();
 
 		// Create 1 draft order - to be excluded from totals.
-		$order = WC_Helper_Order::create_order( 1, $product );
+		$order = WC_Helper_Order::create_order( $this->customer, $product );
 		$order->set_date_created( time() - ( 5 * DAY_IN_SECONDS ) );
 		$order->save();
 		wp_update_post(