Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update the shipping time controllers to handle the maximum shipping time #2590

Merged
merged 5 commits into from
Sep 15, 2024
Merged

Update the shipping time controllers to handle the maximum shipping time #2590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c90a624
Add max time to shipping time controllers
jorgemd24 Sep 5, 2024
e1a119b
Allow to update max_time in the db
jorgemd24 Sep 5, 2024
b67ade9
Add tests
jorgemd24 Sep 5, 2024
bfea9b0
Validate shippimg time args
jorgemd24 Sep 12, 2024
af40ca3
Add tests
jorgemd24 Sep 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/API/Site/Controllers/MerchantCenter/ShippingTimeBatchController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand All @@ -62,6 +63,7 @@ protected function get_batch_create_callback(): callable {
[
'country_code' => $country_code,
'time' => $time,
'max_time' => $max_time,
]
);

Expand Down
70 changes: 65 additions & 5 deletions src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -61,7 +62,7 @@
'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(),
]
Expand Down Expand Up @@ -100,6 +101,7 @@
[
'country_code' => $time['country'],
'time' => $time['time'],
'max_time' => $time['max_time'],
],
$request
);
Expand Down Expand Up @@ -134,6 +136,7 @@
[
'country_code' => $time[0]['country'],
'time' => $time[0]['time'],
'max_time' => $time[0]['max_time'],
],
$request
);
Expand All @@ -153,8 +156,9 @@

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(
Expand Down Expand Up @@ -265,13 +269,69 @@
],
'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' => [ $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;
}
Comment on lines +290 to +295
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious. Why we cannot just add the 'required' in the array inside get_schema_properties method ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my initial approach, but the schema is shared between the GET and POST methods. If I make time and max_time required, the UI will also need to include those query parameters when fetching the times.


/**
* 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',

Check warning on line 312 in src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php

View check run for this annotation

Codecov / codecov/patch

src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php#L311-L312 

Added lines #L311 - L312 were not covered by tests
/* translators: 1: Parameter, 2: Type name. */
sprintf( __( '%1$s is not of type %2$s.', 'google-listings-and-ads' ), $param, 'integer' ),
[ 'param' => $param ]

Check warning on line 315 in src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php

View check run for this annotation

Codecov / codecov/patch

src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php#L314-L315 

Added lines #L314 - L315 were not covered by tests
);
}

if ( ! $value ) {
return new WP_Error( 'invalid_shipping_times', __( 'Shipping times are required.', 'google-listings-and-ads' ), [ 'param' => $param ] );

Check warning on line 320 in src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php

View check run for this annotation

Codecov / codecov/patch

src/API/Site/Controllers/MerchantCenter/ShippingTimeController.php#L320 

Added line #L320 was not covered by tests
}

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.
*
Expand Down
7 changes: 4 additions & 3 deletions src/DB/Table/ShippingTimeTable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@
*/
public function get_columns(): array {
return [
'id' => true,
'country' => true,
'time' => true,
'id' => true,
'country' => true,
'time' => true,
'max_time' => true,

Check warning on line 56 in src/DB/Table/ShippingTimeTable.php

View check run for this annotation

Codecov / codecov/patch

src/DB/Table/ShippingTimeTable.php#L53-L56 

Added lines #L53 - L56 were not covered by tests
];
}
}
1 change: 1 addition & 0 deletions tests/Unit/API/Site/Controllers/MerchantCenter/ShippingTimeBatchControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down
Loading