From 57416a2bdb495e2f746cb000c7abee25cbee713b Mon Sep 17 00:00:00 2001 From: Joshua T Flowers Date: Thu, 20 Jun 2019 11:22:47 +0800 Subject: [PATCH] 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 --- .../includes/class-wc-admin-order-refund.php | 5 + .../includes/class-wc-admin-order.php | 30 ++++++ ...-wc-admin-reports-customers-data-store.php | 76 +++++-------- .../tests/api/reports-customers.php | 102 ++++++++++-------- .../tests/api/reports-import.php | 12 +-- 5 files changed, 128 insertions(+), 97 deletions(-) diff --git a/plugins/woocommerce-admin/includes/class-wc-admin-order-refund.php b/plugins/woocommerce-admin/includes/class-wc-admin-order-refund.php index b7bf44854f2..62fe13571ee 100644 --- a/plugins/woocommerce-admin/includes/class-wc-admin-order-refund.php +++ b/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 ); } diff --git a/plugins/woocommerce-admin/includes/class-wc-admin-order.php b/plugins/woocommerce-admin/includes/class-wc-admin-order.php index 6bd21d71bc9..705b82b3f19 100644 --- a/plugins/woocommerce-admin/includes/class-wc-admin-order.php +++ b/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' ); + } + } } diff --git a/plugins/woocommerce-admin/includes/data-stores/class-wc-admin-reports-customers-data-store.php b/plugins/woocommerce-admin/includes/data-stores/class-wc-admin-reports-customers-data-store.php index 3b73efd0e16..3147d6ccdb7 100644 --- a/plugins/woocommerce-admin/includes/data-stores/class-wc-admin-reports-customers-data-store.php +++ b/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( - 'first_name' => $customer_name[0], - 'last_name' => $customer_name[1], + $data = array( + 'first_name' => $order->get_customer_first_name(), + 'last_name' => $order->get_customer_last_name(), '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_active = $customer->get_meta( 'wc_last_active', true, 'edit' ); - $data = array( + $last_order = $customer->get_last_order(); + + 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], - 'last_name' => $customer_name[1], + 'first_name' => $first_name, + '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; } diff --git a/plugins/woocommerce-admin/tests/api/reports-customers.php b/plugins/woocommerce-admin/tests/api/reports-customers.php index 9cc62060cbc..25284bf579c 100644 --- a/plugins/woocommerce-admin/tests/api/reports-customers.php +++ b/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'] ); } } diff --git a/plugins/woocommerce-admin/tests/api/reports-import.php b/plugins/woocommerce-admin/tests/api/reports-import.php index 1429bc9f407..21b46108022 100644 --- a/plugins/woocommerce-admin/tests/api/reports-import.php +++ b/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(