Merge pull request woocommerce/woocommerce-admin#1404 from woocommerce/fix/customers-date-between-args

Customers Report: REST API: Fix between argument handling

plugins/woocommerce-admin/includes/api/class-wc-admin-rest-reports-customers-controller.php

@@ -61,9 +61,11 @@ class WC_Admin_REST_Reports_Customers_Controller extends WC_REST_Reports_Control
 		$args['last_order_before']   = $request['last_order_before'];
 		$args['last_order_after']    = $request['last_order_after'];
 
-		$between_params = array( 'orders_count', 'total_spend', 'avg_order_value' );
+		$between_params_numeric    = array( 'orders_count', 'total_spend', 'avg_order_value' );
-		$normalized     = WC_Admin_Reports_Interval::normalize_between_params( $request, $between_params );
+		$normalized_params_numeric = WC_Admin_Reports_Interval::normalize_between_params( $request, $between_params_numeric, false );
-		$args           = array_merge( $args, $normalized );
+		$between_params_date       = array( 'last_active', 'registered' );
+		$normalized_params_date    = WC_Admin_Reports_Interval::normalize_between_params( $request, $between_params_date, true );
+		$args                      = array_merge( $args, $normalized_params_numeric, $normalized_params_date );
 
 		return $args;
 	}
@@ -363,6 +365,11 @@ class WC_Admin_REST_Reports_Customers_Controller extends WC_REST_Reports_Control
 			'format'            => 'date-time',
 			'validate_callback' => 'rest_validate_request_arg',
 		);
+		$params['last_active_between']     = array(
+			'description'       => __( 'Limit response to objects last active between two given ISO8601 compliant datetime.', 'wc-admin' ),
+			'type'              => 'array',
+			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_date_arg' ),
+		);
 		$params['registered_before']       = array(
 			'description'       => __( 'Limit response to objects registered before (or at) a given ISO8601 compliant datetime.', 'wc-admin' ),
 			'type'              => 'string',
@@ -375,6 +382,11 @@ class WC_Admin_REST_Reports_Customers_Controller extends WC_REST_Reports_Control
 			'format'            => 'date-time',
 			'validate_callback' => 'rest_validate_request_arg',
 		);
+		$params['registered_between']      = array(
+			'description'       => __( 'Limit response to objects last active between two given ISO8601 compliant datetime.', 'wc-admin' ),
+			'type'              => 'array',
+			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_date_arg' ),
+		);
 		$params['orders_count_min']        = array(
 			'description'       => __( 'Limit response to objects with an order count greater than or equal to given integer.', 'wc-admin' ),
 			'type'              => 'integer',
@@ -390,7 +402,7 @@ class WC_Admin_REST_Reports_Customers_Controller extends WC_REST_Reports_Control
 		$params['orders_count_between']    = array(
 			'description'       => __( 'Limit response to objects with an order count between two given integers.', 'wc-admin' ),
 			'type'              => 'array',
-			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_arg' ),
+			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_numeric_arg' ),
 		);
 		$params['total_spend_min']         = array(
 			'description'       => __( 'Limit response to objects with a total order spend greater than or equal to given number.', 'wc-admin' ),
@@ -405,7 +417,7 @@ class WC_Admin_REST_Reports_Customers_Controller extends WC_REST_Reports_Control
 		$params['total_spend_between']     = array(
 			'description'       => __( 'Limit response to objects with a total order spend between two given numbers.', 'wc-admin' ),
 			'type'              => 'array',
-			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_arg' ),
+			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_numeric_arg' ),
 		);
 		$params['avg_order_value_min']     = array(
 			'description'       => __( 'Limit response to objects with an average order spend greater than or equal to given number.', 'wc-admin' ),
@@ -420,7 +432,7 @@ class WC_Admin_REST_Reports_Customers_Controller extends WC_REST_Reports_Control
 		$params['avg_order_value_between'] = array(
 			'description'       => __( 'Limit response to objects with an average order spend between two given numbers.', 'wc-admin' ),
 			'type'              => 'array',
-			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_arg' ),
+			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_numeric_arg' ),
 		);
 		$params['last_order_before']       = array(
 			'description'       => __( 'Limit response to objects with last order before (or at) a given ISO8601 compliant datetime.', 'wc-admin' ),

plugins/woocommerce-admin/includes/api/class-wc-admin-rest-reports-customers-stats-controller.php

@@ -56,9 +56,11 @@ class WC_Admin_REST_Reports_Customers_Stats_Controller extends WC_REST_Reports_C
 		$args['last_order_before']   = $request['last_order_before'];
 		$args['last_order_after']    = $request['last_order_after'];
 
-		$between_params = array( 'orders_count', 'total_spend', 'avg_order_value' );
+		$between_params_numeric    = array( 'orders_count', 'total_spend', 'avg_order_value' );
-		$normalized     = WC_Admin_Reports_Interval::normalize_between_params( $request, $between_params );
+		$normalized_params_numeric = WC_Admin_Reports_Interval::normalize_between_params( $request, $between_params_numeric, false );
-		$args           = array_merge( $args, $normalized );
+		$between_params_date       = array( 'last_active', 'registered' );
+		$normalized_params_date    = WC_Admin_Reports_Interval::normalize_between_params( $request, $between_params_date, true );
+		$args                      = array_merge( $args, $normalized_params_numeric, $normalized_params_date );
 
 		return $args;
 	}
@@ -276,6 +278,11 @@ class WC_Admin_REST_Reports_Customers_Stats_Controller extends WC_REST_Reports_C
 			'format'            => 'date-time',
 			'validate_callback' => 'rest_validate_request_arg',
 		);
+		$params['last_active_between']     = array(
+			'description'       => __( 'Limit response to objects last active between two given ISO8601 compliant datetime.', 'wc-admin' ),
+			'type'              => 'array',
+			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_date_arg' ),
+		);
 		$params['registered_before']       = array(
 			'description'       => __( 'Limit response to objects registered before (or at) a given ISO8601 compliant datetime.', 'wc-admin' ),
 			'type'              => 'string',
@@ -288,6 +295,11 @@ class WC_Admin_REST_Reports_Customers_Stats_Controller extends WC_REST_Reports_C
 			'format'            => 'date-time',
 			'validate_callback' => 'rest_validate_request_arg',
 		);
+		$params['registered_between']      = array(
+			'description'       => __( 'Limit response to objects last active between two given ISO8601 compliant datetime.', 'wc-admin' ),
+			'type'              => 'array',
+			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_date_arg' ),
+		);
 		$params['orders_count_min']        = array(
 			'description'       => __( 'Limit response to objects with an order count greater than or equal to given integer.', 'wc-admin' ),
 			'type'              => 'integer',
@@ -303,7 +315,7 @@ class WC_Admin_REST_Reports_Customers_Stats_Controller extends WC_REST_Reports_C
 		$params['orders_count_between']    = array(
 			'description'       => __( 'Limit response to objects with an order count between two given integers.', 'wc-admin' ),
 			'type'              => 'array',
-			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_arg' ),
+			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_numeric_arg' ),
 		);
 		$params['total_spend_min']         = array(
 			'description'       => __( 'Limit response to objects with a total order spend greater than or equal to given number.', 'wc-admin' ),
@@ -318,7 +330,7 @@ class WC_Admin_REST_Reports_Customers_Stats_Controller extends WC_REST_Reports_C
 		$params['total_spend_between']     = array(
 			'description'       => __( 'Limit response to objects with a total order spend between two given numbers.', 'wc-admin' ),
 			'type'              => 'array',
-			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_arg' ),
+			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_numeric_arg' ),
 		);
 		$params['avg_order_value_min']     = array(
 			'description'       => __( 'Limit response to objects with an average order spend greater than or equal to given number.', 'wc-admin' ),
@@ -333,7 +345,7 @@ class WC_Admin_REST_Reports_Customers_Stats_Controller extends WC_REST_Reports_C
 		$params['avg_order_value_between'] = array(
 			'description'       => __( 'Limit response to objects with an average order spend between two given numbers.', 'wc-admin' ),
 			'type'              => 'array',
-			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_arg' ),
+			'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_numeric_arg' ),
 		);
 		$params['last_order_before']       = array(
 			'description'       => __( 'Limit response to objects with last order before (or at) a given ISO8601 compliant datetime.', 'wc-admin' ),

plugins/woocommerce-admin/includes/class-wc-admin-reports-interval.php

@@ -494,13 +494,15 @@ class WC_Admin_Reports_Interval {
 	}
 
 	/**
-	 * Normalize "*_between" parameters to "*_min" and "*_max".
+	 * Normalize "*_between" parameters to "*_min" and "*_max" for numeric values
+	 * and "*_after" and "*_before" for date values.
 	 *
 	 * @param array        $request Query params from REST API request.
 	 * @param string|array $param_names One or more param names to handle. Should not include "_between" suffix.
+	 * @param bool         $is_date Boolean if the param is date is related.
 	 * @return array Normalized query values.
 	 */
-	public static function normalize_between_params( $request, $param_names ) {
+	public static function normalize_between_params( $request, $param_names, $is_date ) {
 		if ( ! is_array( $param_names ) ) {
 			$param_names = array( $param_names );
 		}
@@ -518,12 +520,15 @@ class WC_Admin_Reports_Interval {
 				continue;
 			}
 
+			$min = $is_date ? '_after' : '_min';
+			$max = $is_date ? '_before' : '_max';
+
 			if ( $range[0] < $range[1] ) {
-				$normalized[ $param_name . '_min' ] = $range[0];
+				$normalized[ $param_name . $min ] = $range[0];
-				$normalized[ $param_name . '_max' ] = $range[1];
+				$normalized[ $param_name . $max ] = $range[1];
 			} else {
-				$normalized[ $param_name . '_min' ] = $range[1];
+				$normalized[ $param_name . $min ] = $range[1];
-				$normalized[ $param_name . '_max' ] = $range[0];
+				$normalized[ $param_name . $max ] = $range[0];
 			}
 		}
 
@@ -538,7 +543,7 @@ class WC_Admin_Reports_Interval {
 	 * @param  string          $param Parameter name.
 	 * @return WP_Error|boolean
 	 */
-	public static function rest_validate_between_arg( $value, $request, $param ) {
+	public static function rest_validate_between_numeric_arg( $value, $request, $param ) {
 		if ( ! wp_is_numeric_array( $value ) ) {
 			return new WP_Error(
 				'rest_invalid_param',
@@ -561,4 +566,36 @@ class WC_Admin_Reports_Interval {
 
 		return true;
 	}
+
+	/**
+	 * Validate a "*_between" range argument (an array with 2 date items).
+	 *
+	 * @param  mixed           $value Parameter value.
+	 * @param  WP_REST_Request $request REST Request.
+	 * @param  string          $param Parameter name.
+	 * @return WP_Error|boolean
+	 */
+	public static function rest_validate_between_date_arg( $value, $request, $param ) {
+		if ( ! wp_is_numeric_array( $value ) ) {
+			return new WP_Error(
+				'rest_invalid_param',
+				/* translators: 1: parameter name */
+				sprintf( __( '%1$s is not a numerically indexed array.', 'wc-admin' ), $param )
+			);
+		}
+
+		if (
+			2 !== count( $value ) ||
+			! rest_parse_date( $value[0] ) ||
+			! rest_parse_date( $value[1] )
+		) {
+			return new WP_Error(
+				'rest_invalid_param',
+				/* translators: %s: parameter name */
+				sprintf( __( '%s must contain 2 valid dates.', 'wc-admin' ), $param )
+			);
+		}
+
+		return true;
+	}
 }

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

@@ -442,7 +442,7 @@ class WC_Admin_Reports_Customers_Data_Store extends WC_Admin_Reports_Data_Store
 	 * Retrieve a guest (no user_id) customer row by email.
 	 *
 	 * @param string $email Email address.
-	 * @returns false|array Customer array if found, boolean false if not.
+	 * @return false|array Customer array if found, boolean false if not.
 	 */
 	public function get_guest_by_email( $email ) {
 		global $wpdb;
@@ -467,7 +467,7 @@ class WC_Admin_Reports_Customers_Data_Store extends WC_Admin_Reports_Data_Store
 	 * Retrieve a registered customer row by user_id.
 	 *
 	 * @param string|int $user_id User ID.
-	 * @returns false|array Customer array if found, boolean false if not.
+	 * @return false|array Customer array if found, boolean false if not.
 	 */
 	public function get_customer_by_user_id( $user_id ) {
 		global $wpdb;
@@ -492,7 +492,7 @@ class WC_Admin_Reports_Customers_Data_Store extends WC_Admin_Reports_Data_Store
 	 * Retrieve a registered customer row id by user_id.
 	 *
 	 * @param string|int $user_id User ID.
-	 * @returns false|int Customer ID if found, boolean false if not.
+	 * @return false|int Customer ID if found, boolean false if not.
 	 */
 	public static function get_customer_id_by_user_id( $user_id ) {
 		global $wpdb;

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

@@ -822,7 +822,7 @@ class WC_Tests_Reports_Interval_Stats extends WC_Unit_Test_Case {
 			'f_between' => array( 10, 12 ), // not in params, skipped.
 		);
 		$params   = array( 'a', 'b', 'c', 'd' );
-		$result   = WC_Admin_Reports_Interval::normalize_between_params( $request, $params );
+		$result   = WC_Admin_Reports_Interval::normalize_between_params( $request, $params, false );
 		$expected = array(
 			'b_min' => 1,
 			'b_max' => 5,
@@ -834,21 +834,68 @@ class WC_Tests_Reports_Interval_Stats extends WC_Unit_Test_Case {
 	}
 
 	/**
-	 * Test function that validates *_between query parameters.
+	 * Test function that normalizes *_between query parameters for dates to *_after & *_before.
 	 */
-	public function test_rest_validate_between_arg() {
+	public function test_normalize_between_date_params() {
+		$request  = array(
+			'a_between' => 'malformed',     // won't be normalized (not an array).
+			'b_between' => array( 1, 5 ),   // results in after=1, before=5.
+			'c_between' => array( 4, 2 ),   // results in after=2, before=4.
+			'd_between' => array( 7 ),      // won't be normalized (only 1 item).
+			'f_between' => array( 10, 12 ), // not in params, skipped.
+		);
+		$params   = array( 'a', 'b', 'c', 'd' );
+		$result   = WC_Admin_Reports_Interval::normalize_between_params( $request, $params, true );
+		$expected = array(
+			'b_after' => 1,
+			'b_before' => 5,
+			'c_after' => 2,
+			'c_before' => 4,
+		);
+
+		$this->assertEquals( $result, $expected );
+	}
+
+	/**
+	 * Test function that validates *_between query parameters for numeric values.
+	 */
+	public function test_rest_validate_between_numeric_arg() {
 		$this->assertIsWPError(
-			WC_Admin_Reports_Interval::rest_validate_between_arg( 'not array', null, 'param' ),
+			WC_Admin_Reports_Interval::rest_validate_between_numeric_arg( 'not array', null, 'param' ),
 			'param is not a numerically indexed array.'
 		);
 
 		$this->assertIsWPError(
-			WC_Admin_Reports_Interval::rest_validate_between_arg( array( 1 ), null, 'param' ),
+			WC_Admin_Reports_Interval::rest_validate_between_numeric_arg( array( 1 ), null, 'param' ),
 			'param must contain 2 numbers.'
 		);
 
 		$this->assertTrue(
-			WC_Admin_Reports_Interval::rest_validate_between_arg( array( 1, 2 ), null, 'param' )
+			WC_Admin_Reports_Interval::rest_validate_between_numeric_arg( array( 1, 2 ), null, 'param' )
+		);
+	}
+
+	/**
+	 * Test function that validates *_between query parameters for date values.
+	 */
+	public function rest_validate_between_date_arg() {
+		$this->assertIsWPError(
+			WC_Admin_Reports_Interval::rest_validate_between_date_arg( 'not array', null, 'param' ),
+			'param is not a numerically indexed array.'
+		);
+
+		$this->assertIsWPError(
+			WC_Admin_Reports_Interval::rest_validate_between_date_arg( array( '2019-01-01T00:00:00' ), null, 'param' ),
+			'param must contain 2 valid dates.'
+		);
+
+		$this->assertIsWPError(
+			WC_Admin_Reports_Interval::rest_validate_between_date_arg( array( 'not a valid date' ), null, 'param' ),
+			'param must contain 2 valid dates.'
+		);
+
+		$this->assertTrue(
+			WC_Admin_Reports_Interval::rest_validate_between_date_arg( array( '2019-01-01T00:00:00', '2019-01-15T00:00:00' ), null, 'param' )
 		);
 	}
 }