From 2415cf1095f20829342d1868254f0b6c7af29582 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Wed, 28 Feb 2024 10:02:28 -0600 Subject: [PATCH] Scheduled Updates: Align responses for 404s (#35963) * Scheduled Updates: Align responses for 404s Makes sure we handle schedules that can't be found in the same way. Adds unit tests as well. * Update Delete endpoint as well. Props @zaerl. * Adjust significance of this patch. We don't need to bump a minor version for this. --- .../changelog/fix-schedule-not-found | 4 ++ ...-rest-api-v2-endpoint-update-schedules.php | 26 +++++---- ...-api-v2-endpoint-update-schedules-test.php | 54 +++++++++++++++++++ 3 files changed, 70 insertions(+), 14 deletions(-) create mode 100644 projects/packages/scheduled-updates/changelog/fix-schedule-not-found diff --git a/projects/packages/scheduled-updates/changelog/fix-schedule-not-found b/projects/packages/scheduled-updates/changelog/fix-schedule-not-found new file mode 100644 index 0000000000000..af8cbfb9aa837 --- /dev/null +++ b/projects/packages/scheduled-updates/changelog/fix-schedule-not-found @@ -0,0 +1,4 @@ +Significance: patch +Type: changed + +Aligned handling of schedules that can't be found to return the same error messages. diff --git a/projects/packages/scheduled-updates/src/wpcom-endpoints/class-wpcom-rest-api-v2-endpoint-update-schedules.php b/projects/packages/scheduled-updates/src/wpcom-endpoints/class-wpcom-rest-api-v2-endpoint-update-schedules.php index a46b5ae07cb88..62fee8101befe 100644 --- a/projects/packages/scheduled-updates/src/wpcom-endpoints/class-wpcom-rest-api-v2-endpoint-update-schedules.php +++ b/projects/packages/scheduled-updates/src/wpcom-endpoints/class-wpcom-rest-api-v2-endpoint-update-schedules.php @@ -208,7 +208,7 @@ public function get_item_permissions_check( $request ) { // phpcs:ignore Variabl * Returns information about an update schedule. * * @param WP_REST_Request $request Request object. - * @return WP_REST_Response + * @return WP_REST_Response|WP_Error The scheduled event or a WP_Error if the schedule could not be found. */ public function get_item( $request ) { $schedules = get_option( 'jetpack_update_schedules', array() ); @@ -221,6 +221,10 @@ public function get_item( $request ) { } } + if ( empty( $event ) ) { + return new WP_Error( 'rest_invalid_schedule', __( 'The schedule could not be found.', 'jetpack-scheduled-updates' ), array( 'status' => 404 ) ); + } + return rest_ensure_response( $event ); } @@ -267,17 +271,14 @@ public function update_item_permissions_check( $request ) { * Updates an existing update schedule. * * @param WP_REST_Request $request Request object. - * @return WP_REST_Response|WP_Error + * @return WP_REST_Response|WP_Error The updated event or a WP_Error if the schedule could not be found. */ public function update_item( $request ) { $schedules = get_option( 'jetpack_update_schedules', array() ); - $found = array(); + $event = array(); foreach ( $schedules as $key => $schedule_args ) { if ( $this->generate_schedule_id( $schedule_args ) === $request['schedule_id'] ) { - // We found the schedule to update. - $found = true; - $event = wp_get_scheduled_event( 'jetpack_scheduled_update', $schedule_args ); $result = wp_unschedule_event( $event->timestamp, 'jetpack_scheduled_update', $schedule_args, true ); if ( is_wp_error( $result ) ) { @@ -292,8 +293,8 @@ public function update_item( $request ) { } } - if ( ! $found ) { - return new WP_Error( 'rest_invalid_schedule', __( 'The schedule could not be found.', 'jetpack-scheduled-updates' ), array( 'status' => 400 ) ); + if ( empty( $event ) ) { + return new WP_Error( 'rest_invalid_schedule', __( 'The schedule could not be found.', 'jetpack-scheduled-updates' ), array( 'status' => 404 ) ); } return $this->create_item( $request ); @@ -321,13 +322,10 @@ public function delete_item_permissions_check( $request ) { // phpcs:ignore Vari */ public function delete_item( $request ) { $schedules = get_option( 'jetpack_update_schedules', array() ); - $found = array(); + $event = array(); foreach ( $schedules as $key => $schedule_args ) { if ( $this->generate_schedule_id( $schedule_args ) === $request['schedule_id'] ) { - // We found the schedule to delete. - $found = true; - $event = wp_get_scheduled_event( 'jetpack_scheduled_update', $schedule_args ); $result = wp_unschedule_event( $event->timestamp, 'jetpack_scheduled_update', $schedule_args, true ); if ( is_wp_error( $result ) ) { @@ -340,8 +338,8 @@ public function delete_item( $request ) { } } - if ( ! $found ) { - return new WP_Error( 'rest_invalid_schedule', __( 'The schedule could not be found.', 'jetpack-scheduled-updates' ), array( 'status' => 400 ) ); + if ( empty( $event ) ) { + return new WP_Error( 'rest_invalid_schedule', __( 'The schedule could not be found.', 'jetpack-scheduled-updates' ), array( 'status' => 404 ) ); } update_option( 'jetpack_update_schedules', $schedules ); diff --git a/projects/packages/scheduled-updates/tests/php/class-wpcom-rest-api-v2-endpoint-update-schedules-test.php b/projects/packages/scheduled-updates/tests/php/class-wpcom-rest-api-v2-endpoint-update-schedules-test.php index 1a2fa9837117d..a5d3f1ef9e30b 100644 --- a/projects/packages/scheduled-updates/tests/php/class-wpcom-rest-api-v2-endpoint-update-schedules-test.php +++ b/projects/packages/scheduled-updates/tests/php/class-wpcom-rest-api-v2-endpoint-update-schedules-test.php @@ -309,6 +309,21 @@ public function test_get_item() { ); } + /** + * Test get_item with invalid schedule ID. + * + * @covers ::get_item + */ + public function test_get_invalid_item() { + wp_set_current_user( $this->admin_id ); + + $request = new WP_REST_Request( 'GET', '/wpcom/v2/update-schedules/' . $this->generate_schedule_id( array() ) ); + $result = rest_do_request( $request ); + + $this->assertSame( 404, $result->get_status() ); + $this->assertSame( 'rest_invalid_schedule', $result->get_data()['code'] ); + } + /** * Test update item. * @@ -355,6 +370,30 @@ public function test_update_item() { $this->assertSame( $schedule_id, $result->get_data() ); } + /** + * Test update_item with invalid schedule ID. + * + * @covers ::update_item + */ + public function test_update_invalid_item() { + wp_set_current_user( $this->admin_id ); + + $request = new WP_REST_Request( 'PUT', '/wpcom/v2/update-schedules/' . $this->generate_schedule_id( array() ) ); + $request->set_body_params( + array( + 'plugins' => array(), + 'schedule' => array( + 'timestamp' => strtotime( 'next Tuesday 9:00' ), + 'interval' => 'daily', + ), + ) + ); + $result = rest_do_request( $request ); + + $this->assertSame( 404, $result->get_status() ); + $this->assertSame( 'rest_invalid_schedule', $result->get_data()['code'] ); + } + /** * Test delete item. * @@ -396,6 +435,21 @@ public function test_delete_item() { $this->assertFalse( wp_get_scheduled_event( 'jetpack_scheduled_update', $plugins ) ); } + /** + * Test delete_item with invalid schedule ID. + * + * @covers ::delete_item + */ + public function test_delete_invalid_item() { + wp_set_current_user( $this->admin_id ); + + $request = new WP_REST_Request( 'DELETE', '/wpcom/v2/update-schedules/' . $this->generate_schedule_id( array() ) ); + $result = rest_do_request( $request ); + + $this->assertSame( 404, $result->get_status() ); + $this->assertSame( 'rest_invalid_schedule', $result->get_data()['code'] ); + } + /** * Generates a unique schedule ID. *