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.
Automattic · Feb 28, 2024 · 2415cf1 · 2415cf1
1 parent d7242d2
commit 2415cf1
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 14 deletions.
diff --git a/projects/packages/scheduled-updates/changelog/fix-schedule-not-found 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/...heduled-updates/src/wpcom-endpoints/class-wpcom-rest-api-v2-endpoint-update-schedules.php b/...heduled-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/...es/scheduled-updates/tests/php/class-wpcom-rest-api-v2-endpoint-update-schedules-test.php b/...es/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.
 	 *