From c90a6245cdb81564907ec4a6ab2e70f39c44b5ef Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 5 Sep 2024 21:58:16 +0200 Subject: [PATCH 1/5] Add max time to shipping time controllers --- .../ShippingTimeBatchController.php | 2 ++ .../MerchantCenter/ShippingTimeController.php | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/API/Site/Controllers/MerchantCenter/ShippingTimeBatchController.php b/src/API/Site/Controllers/MerchantCenter/ShippingTimeBatchController.php index 9ee56a9ec2..8004f97cf5 100644 --- a/src/API/Site/Controllers/MerchantCenter/ShippingTimeBatchController.php +++ b/src/API/Site/Controllers/MerchantCenter/ShippingTimeBatchController.php @@ -53,6 +53,7 @@ protected function get_batch_create_callback(): callable { return function ( Request $request ) { $country_codes = $request->get_param( 'country_codes' ); $time = $request->get_param( 'time' ); + $max_time = $request->get_param( 'max_time' ); $responses = []; $errors = []; @@ -62,6 +63,7 @@ protected function get_batch_create_callback(): callable { [ 'country_code' => $country_code, 'time' => $time, + 'max_time' => $max_time, ] ); diff --git a/src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php b/src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php index 984ce7f11a..3514339fdc 100644 --- a/src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php +++ b/src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php @@ -100,6 +100,7 @@ protected function get_read_times_callback(): callable { [ 'country_code' => $time['country'], 'time' => $time['time'], + 'max_time' => $time['max_time'], ], $request ); @@ -134,6 +135,7 @@ protected function get_read_time_callback(): callable { [ 'country_code' => $time[0]['country'], 'time' => $time[0]['time'], + 'max_time' => $time[0]['max_time'], ], $request ); @@ -153,8 +155,9 @@ protected function get_create_time_callback(): callable { try { $data = [ - 'country' => $country_code, - 'time' => $request->get_param( 'time' ), + 'country' => $country_code, + 'time' => $request->get_param( 'time' ), + 'max_time' => $request->get_param( 'max_time' ), ]; if ( $existing ) { $query->update( @@ -265,7 +268,13 @@ protected function get_schema_properties(): array { ], 'time' => [ 'type' => 'integer', - 'description' => __( 'The shipping time in days.', 'google-listings-and-ads' ), + 'description' => __( 'The minimum shipping time in days.', 'google-listings-and-ads' ), + 'context' => [ 'view', 'edit' ], + 'validate_callback' => 'rest_validate_request_arg', + ], + 'max_time' => [ + 'type' => 'integer', + 'description' => __( 'The maximum shipping time in days.', 'google-listings-and-ads' ), 'context' => [ 'view', 'edit' ], 'validate_callback' => 'rest_validate_request_arg', ], From e1a119b41cae946ac4b89ea478c4f60490e84661 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 5 Sep 2024 21:58:31 +0200 Subject: [PATCH 2/5] Allow to update max_time in the db --- src/DB/Table/ShippingTimeTable.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/DB/Table/ShippingTimeTable.php b/src/DB/Table/ShippingTimeTable.php index a80a32830e..23d05ead8f 100644 --- a/src/DB/Table/ShippingTimeTable.php +++ b/src/DB/Table/ShippingTimeTable.php @@ -50,9 +50,10 @@ public static function get_raw_name(): string { */ public function get_columns(): array { return [ - 'id' => true, - 'country' => true, - 'time' => true, + 'id' => true, + 'country' => true, + 'time' => true, + 'max_time' => true, ]; } } From b67ade9e8a10fde12e031590443a469a1ff9edc3 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 5 Sep 2024 21:58:43 +0200 Subject: [PATCH 3/5] Add tests --- .../ShippingTimeBatchControllerTest.php | 1 + .../ShippingTimeControllerTest.php | 283 ++++++++++++++++++ 2 files changed, 284 insertions(+) create mode 100644 tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeControllerTest.php diff --git a/tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeBatchControllerTest.php b/tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeBatchControllerTest.php index d1b2c621a1..88d0f945a3 100644 --- a/tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeBatchControllerTest.php +++ b/tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeBatchControllerTest.php @@ -64,6 +64,7 @@ public function test_create_shipping_time_batch() { $payload = [ 'country_codes' => [ 'US', 'GB' ], 'time' => 5, + 'max_time' => 10, ]; $response = $this->do_request( self::ROUTE_SHIPPING_TIME_BATCH, 'POST', $payload ); diff --git a/tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeControllerTest.php b/tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeControllerTest.php new file mode 100644 index 0000000000..a8c7d7f4dc --- /dev/null +++ b/tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeControllerTest.php @@ -0,0 +1,283 @@ +shiping_time_query = $this->createMock( ShippingTimeQuery::class ); + $this->iso_provider = $this->createMock( ISO3166DataProvider::class ); + + $this->container = new Container(); + $this->container->share( RESTServer::class, $this->server ); + $this->container->share( ShippingTimeQuery::class, $this->shiping_time_query ); + + $this->controller = new ShippingTimeController( $this->container ); + $this->controller->set_iso3166_provider( $this->iso_provider ); + $this->controller->register(); + } + + public function test_get_shipping_times() { + $this->shiping_time_query->expects( $this->once() ) + ->method( 'set_limit' ) + ->willReturn( + $this->shiping_time_query + ); + + $this->shiping_time_query->expects( $this->once() ) + ->method( 'get_results' ) + ->willReturn( + [ + [ + 'id' => '1', + 'country' => 'ES', + 'time' => 3, + 'max_time' => 1, + ], + ] + ); + + $this->iso_provider + ->method( 'alpha2' ) + ->willReturn( [ 'name' => 'Spain' ] ); + + $response = $this->do_request( self::ROUTE_SHIPPING_TIMES, 'GET' ); + + $this->assertEquals( 200, $response->get_status() ); + $this->assertEquals( + [ + 'ES' => [ + 'country_code' => 'ES', + 'country' => 'Spain', + 'time' => 3, + 'max_time' => 1, + ], + ], + $response->get_data() + ); + } + + public function test_create_shipping_rate_insert() { + $payload = [ + 'country_code' => 'US', + 'time' => 5, + 'max_time' => 10, + ]; + + $this->shiping_time_query->expects( $this->once() ) + ->method( 'where' ) + ->with( 'country', 'US' ) + ->willReturn( + $this->shiping_time_query + ); + + $this->shiping_time_query->expects( $this->once() ) + ->method( 'get_results' ) + ->willReturn( + [] + ); + + $this->shiping_time_query->expects( $this->once() ) + ->method( 'insert' ) + ->with( + [ + 'country' => 'US', + 'time' => 5, + 'max_time' => 10, + ] + ); + + $this->shiping_time_query->expects( $this->never() ) + ->method( 'update' ); + + $response = $this->do_request( self::ROUTE_SHIPPING_TIMES, 'POST', $payload ); + + $this->assertEquals( 201, $response->get_status() ); + $this->assertEquals( 'success', $response->get_data()['status'] ); + $this->assertEquals( 'Successfully added time for country: "US".', $response->get_data()['message'] ); + } + + public function test_create_shipping_rate_update() { + $payload = [ + 'country_code' => 'US', + 'time' => 5, + 'max_time' => 10, + ]; + + $this->shiping_time_query->expects( $this->once() ) + ->method( 'where' ) + ->with( 'country', 'US' ) + ->willReturn( + $this->shiping_time_query + ); + + $this->shiping_time_query->expects( $this->exactly( 2 ) ) + ->method( 'get_results' ) + ->willReturn( + [ + [ + 'id' => 1, + 'country' => 'US', + 'time' => 3, + 'max_time' => 1, + ], + ] + ); + + $this->shiping_time_query->expects( $this->once() ) + ->method( 'update' ) + ->with( + [ + 'country' => 'US', + 'time' => 5, + 'max_time' => 10, + ], + [ + 'id' => 1, + ] + ); + + $this->shiping_time_query->expects( $this->never() ) + ->method( 'insert' ); + + $response = $this->do_request( self::ROUTE_SHIPPING_TIMES, 'POST', $payload ); + + $this->assertEquals( 201, $response->get_status() ); + $this->assertEquals( 'success', $response->get_data()['status'] ); + $this->assertEquals( 'Successfully added time for country: "US".', $response->get_data()['message'] ); + } + + public function test_create_shipping_time_invalid_query() { + $payload = [ + 'country_code' => 'US', + 'time' => 5, + 'max_time' => 10, + ]; + + $this->shiping_time_query->expects( $this->once() ) + ->method( 'where' ) + ->with( 'country', 'US' ) + ->willThrowException( + new InvalidQuery( 'error' ) + ); + + try { + $this->do_request( self::ROUTE_SHIPPING_TIMES, 'POST', $payload ); + } catch ( InvalidQuery $e ) { + $this->assertEquals( 'error', $e->getMessage() ); + } + } + + public function test_get_shipping_time_country() { + $this->shiping_time_query->expects( $this->once() ) + ->method( 'where' ) + ->with( 'country', 'ES' ) + ->willReturn( + $this->shiping_time_query + ); + + $this->shiping_time_query->expects( $this->exactly( 1 ) ) + ->method( 'get_results' ) + ->willReturn( + [ + [ + 'id' => 1, + 'country' => 'ES', + 'time' => 3, + 'max_time' => 1, + ], + ] + ); + + $this->iso_provider + ->method( 'alpha2' ) + ->willReturn( [ 'name' => 'Spain' ] ); + + $response = $this->do_request( self::ROUTE_SHIPPING_TIMES_COUNTRY, 'GET' ); + + $this->assertEquals( 200, $response->get_status() ); + $this->assertEquals( 'ES', $response->get_data()['country_code'] ); + $this->assertEquals( 3, $response->get_data()['time'] ); + $this->assertEquals( 1, $response->get_data()['max_time'] ); + } + + public function test_get_shipping_time_country_not_found() { + $this->shiping_time_query->expects( $this->once() ) + ->method( 'where' ) + ->with( 'country', 'ES' ) + ->willReturn( + $this->shiping_time_query + ); + + $this->shiping_time_query->expects( $this->exactly( 1 ) ) + ->method( 'get_results' ) + ->willReturn( + [] + ); + + $response = $this->do_request( self::ROUTE_SHIPPING_TIMES_COUNTRY, 'GET' ); + + $this->assertEquals( 404, $response->get_status() ); + $this->assertEquals( 'No time available.', $response->get_data()['message'] ); + $this->assertEquals( 'ES', $response->get_data()['country'] ); + } + + public function test_delete_shipping_time_country() { + $this->shiping_time_query->expects( $this->once() ) + ->method( 'delete' ) + ->with( 'country', 'ES' ); + + $response = $this->do_request( self::ROUTE_SHIPPING_TIMES_COUNTRY, 'DELETE' ); + + $this->assertEquals( 200, $response->get_status() ); + $this->assertEquals( 'success', $response->get_data()['status'] ); + $this->assertEquals( 'Successfully deleted the time for country: "ES".', $response->get_data()['message'] ); + } + + public function test_delete_shipping_time_country_invalid_query() { + $this->shiping_time_query->expects( $this->once() ) + ->method( 'delete' ) + ->with( 'country', 'ES' ) + ->willThrowException( + new InvalidQuery( 'error' ) + ); + + try { + $this->do_request( self::ROUTE_SHIPPING_TIMES_COUNTRY, 'DELETE' ); + } catch ( InvalidQuery $e ) { + $this->assertEquals( 'error', $e->getMessage() ); + } + } +} From bfea9b0d96062435cac0a6bf2db96dda6ff40760 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 12 Sep 2024 20:46:17 +0200 Subject: [PATCH 4/5] Validate shippimg time args --- .../MerchantCenter/ShippingTimeController.php | 57 ++++++++++++++++++- 1 file changed, 54 insertions(+), 3 deletions(-) diff --git a/src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php b/src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php index 3514339fdc..6bb9d3fd5b 100644 --- a/src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php +++ b/src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php @@ -13,6 +13,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\Psr\Container\ContainerInterface; use WP_REST_Request as Request; use WP_REST_Response as Response; +use WP_Error; defined( 'ABSPATH' ) || exit; @@ -61,7 +62,7 @@ public function register_routes(): void { 'methods' => TransportMethods::CREATABLE, 'callback' => $this->get_create_time_callback(), 'permission_callback' => $this->get_permission_callback(), - 'args' => $this->get_schema_properties(), + 'args' => $this->get_args_schema(), ], 'schema' => $this->get_api_response_schema_callback(), ] @@ -270,17 +271,67 @@ protected function get_schema_properties(): array { 'type' => 'integer', 'description' => __( 'The minimum shipping time in days.', 'google-listings-and-ads' ), 'context' => [ 'view', 'edit' ], - 'validate_callback' => 'rest_validate_request_arg', + 'validate_callback' => [ $this, 'validate_shipping_times' ], ], 'max_time' => [ 'type' => 'integer', 'description' => __( 'The maximum shipping time in days.', 'google-listings-and-ads' ), 'context' => [ 'view', 'edit' ], - 'validate_callback' => 'rest_validate_request_arg', + 'validate_callback' => [ $this, 'validate_shipping_times' ], ], ]; } + /** + * Get the args schema for the controller. + * + * @return array + */ + protected function get_args_schema(): array { + $schema = $this->get_schema_properties(); + $schema['time']['required'] = true; + $schema['max_time']['required'] = true; + return $schema; + } + + /** + * Validate the shipping times. + * + * @param mixed $value + * @param Request $request + * @param string $param + * + * @return WP_Error|true + */ + public function validate_shipping_times( $value, $request, $param ) { + $time = $request->get_param( 'time' ); + $max_time = $request->get_param( 'max_time' ); + + if ( rest_is_integer( $value ) === false ) { + return new WP_Error( + 'rest_invalid_type', + /* translators: 1: Parameter, 2: Type name. */ + sprintf( __( '%1$s is not of type %2$s.', 'google-listings-and-ads' ), $param, 'integer' ), + [ 'param' => $param ] + ); + } + + if ( ! $value ) { + return new WP_Error( 'invalid_shipping_times', __( 'Shipping times are required.', 'google-listings-and-ads' ), [ 'param' => $param ] ); + } + + if ( $value < 0 ) { + return new WP_Error( 'invalid_shipping_times', __( 'Shipping times cannot be negative.', 'google-listings-and-ads' ), [ 'param' => $param ] ); + } + + if ( $time > $max_time ) { + return new WP_Error( 'invalid_shipping_times', __( 'The minimum shipping time cannot be greater than the maximum shipping time.', 'google-listings-and-ads' ), [ 'param' => $param ] ); + } + + return true; + } + + /** * Get the item schema name for the controller. * From af40ca3b3777565497d47def41f1ebdce2ac92bc Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 12 Sep 2024 20:46:25 +0200 Subject: [PATCH 5/5] Add tests --- .../ShippingTimeControllerTest.php | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeControllerTest.php b/tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeControllerTest.php index a8c7d7f4dc..418e71e602 100644 --- a/tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeControllerTest.php +++ b/tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeControllerTest.php @@ -280,4 +280,50 @@ public function test_delete_shipping_time_country_invalid_query() { $this->assertEquals( 'error', $e->getMessage() ); } } + + public function test_missing_time_param() { + $payload = [ + 'country_code' => 'US', + 'max_time' => 10, + ]; + + $response = $this->do_request( self::ROUTE_SHIPPING_TIMES, 'POST', $payload ); + $this->assertEquals( 400, $response->get_status() ); + $this->assertEquals( 'Missing parameter(s): time', $response->get_data()['message'] ); + } + + public function test_missing_max_time_param() { + $payload = [ + 'country_code' => 'US', + 'time' => 10, + ]; + + $response = $this->do_request( self::ROUTE_SHIPPING_TIMES, 'POST', $payload ); + $this->assertEquals( 400, $response->get_status() ); + $this->assertEquals( 'Missing parameter(s): max_time', $response->get_data()['message'] ); + } + + public function test_negative_time_param() { + $payload = [ + 'country_code' => 'US', + 'time' => -1, + 'max_time' => 10, + ]; + + $response = $this->do_request( self::ROUTE_SHIPPING_TIMES, 'POST', $payload ); + $this->assertEquals( 400, $response->get_status() ); + $this->assertEquals( 'Shipping times cannot be negative.', $response->get_data()['data']['params']['time'] ); + } + + public function test_minimum_shipping_time_bigger_than_max_time_param() { + $payload = [ + 'country_code' => 'US', + 'time' => 20, + 'max_time' => 10, + ]; + + $response = $this->do_request( self::ROUTE_SHIPPING_TIMES, 'POST', $payload ); + $this->assertEquals( 400, $response->get_status() ); + $this->assertEquals( 'The minimum shipping time cannot be greater than the maximum shipping time.', $response->get_data()['data']['params']['time'] ); + } }