From 2246a1e07588a1b2e9690808eba2c8f92524ac98 Mon Sep 17 00:00:00 2001 From: Jeff Stieler Date: Wed, 9 Jan 2019 14:19:37 -0700 Subject: [PATCH] Expect array value for _between params. Add param validation method and normalize to min/max based by comparing values. --- ...dmin-rest-reports-customers-controller.php | 12 ++--- .../class-wc-admin-reports-interval.php | 50 ++++++++++++++++--- .../tests/api/reports-interval.php | 43 ++++++++++------ 3 files changed, 77 insertions(+), 28 deletions(-) diff --git a/plugins/woocommerce-admin/includes/api/class-wc-admin-rest-reports-customers-controller.php b/plugins/woocommerce-admin/includes/api/class-wc-admin-rest-reports-customers-controller.php index 91b4c386195..523fba26387 100644 --- a/plugins/woocommerce-admin/includes/api/class-wc-admin-rest-reports-customers-controller.php +++ b/plugins/woocommerce-admin/includes/api/class-wc-admin-rest-reports-customers-controller.php @@ -389,8 +389,8 @@ 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' => 'string', - 'validate_callback' => 'rest_validate_request_arg', + 'type' => 'array', + 'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_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' ), @@ -404,8 +404,8 @@ 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' => 'string', - 'validate_callback' => 'rest_validate_request_arg', + 'type' => 'array', + 'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_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' ), @@ -419,8 +419,8 @@ 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' => 'string', - 'validate_callback' => 'rest_validate_request_arg', + 'type' => 'array', + 'validate_callback' => array( 'WC_Admin_Reports_Interval', 'rest_validate_between_arg' ), ); $params['last_order_before'] = array( 'description' => __( 'Limit response to objects with last order before (or at) a given ISO8601 compliant datetime.', 'wc-admin' ), diff --git a/plugins/woocommerce-admin/includes/class-wc-admin-reports-interval.php b/plugins/woocommerce-admin/includes/class-wc-admin-reports-interval.php index cdfb3a6a422..92e8627f4de 100644 --- a/plugins/woocommerce-admin/includes/class-wc-admin-reports-interval.php +++ b/plugins/woocommerce-admin/includes/class-wc-admin-reports-interval.php @@ -489,23 +489,57 @@ class WC_Admin_Reports_Interval { $normalized = array(); foreach ( $param_names as $param_name ) { - if ( empty( $request[ $param_name . '_between' ] ) ) { + if ( ! is_array( $request[ $param_name . '_between' ] ) ) { continue; } - $min_param = $param_name . '_min'; - $max_param = $param_name . '_max'; - $range = explode( ',', $request[ $param_name . '_between' ] ); + $range = $request[ $param_name . '_between' ]; - if ( isset( $range[0] ) && is_numeric( $range[0] ) ) { - $normalized[ $min_param ] = $range[0]; + if ( 2 !== count( $range ) ) { + continue; } - if ( isset( $range[1] ) && is_numeric( $range[1] ) ) { - $normalized[ $max_param ] = $range[1]; + if ( $range[0] < $range[1] ) { + $normalized[ $param_name . '_min' ] = $range[0]; + $normalized[ $param_name . '_max' ] = $range[1]; + } else { + $normalized[ $param_name . '_min' ] = $range[1]; + $normalized[ $param_name . '_max' ] = $range[0]; } } return $normalized; } + + /** + * Validate a "*_between" range argument (an array with 2 numeric 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_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 ) || + ! is_numeric( $value[0] ) || + ! is_numeric( $value[1] ) + ) { + return new WP_Error( + 'rest_invalid_param', + /* translators: %s: parameter name */ + sprintf( __( '%s must contain 2 numbers.', 'wc-admin' ), $param ) + ); + } + + return true; + } } diff --git a/plugins/woocommerce-admin/tests/api/reports-interval.php b/plugins/woocommerce-admin/tests/api/reports-interval.php index ffee0008331..3334e5b2721 100644 --- a/plugins/woocommerce-admin/tests/api/reports-interval.php +++ b/plugins/woocommerce-admin/tests/api/reports-interval.php @@ -801,25 +801,40 @@ class WC_Tests_Reports_Interval_Stats extends WC_Unit_Test_Case { */ public function test_normalize_between_params() { $request = array( - 'a_between' => 'malformed', // won't be normalized. - 'b_between' => '1,5', // results in min=1, max=5. - 'c_between' => ',6', // results in max=6. - 'd_between' => '7', // results in min=7. - 'e_between' => '8,', // results in min=8. - 'f_between' => '10,12', // not in params, skipped. - 'g_between' => '-1,a', // results in min=-1. + 'a_between' => 'malformed', // won't be normalized (not an array). + 'b_between' => array( 1, 5 ), // results in min=1, max=5. + 'c_between' => array( 4, 2 ), // results in min=2, max=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', 'e', 'g' ); + $params = array( 'a', 'b', 'c', 'd' ); $result = WC_Admin_Reports_Interval::normalize_between_params( $request, $params ); $expected = array( - 'b_min' => '1', - 'b_max' => '5', - 'c_max' => '6', - 'd_min' => '7', - 'e_min' => '8', - 'g_min' => '-1', + 'b_min' => 1, + 'b_max' => 5, + 'c_min' => 2, + 'c_max' => 4, ); $this->assertEquals( $result, $expected ); } + + /** + * Test function that validates *_between query parameters. + */ + public function test_rest_validate_between_arg() { + $this->assertIsWPError( + WC_Admin_Reports_Interval::rest_validate_between_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' ), + 'param must contain 2 numbers.' + ); + + $this->assertTrue( + WC_Admin_Reports_Interval::rest_validate_between_arg( array( 1, 2 ), null, 'param' ) + ); + } }