From 88952a57b36c0b5705c0a4db140a8eaaf3d5d73f Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Thu, 19 Dec 2024 18:34:37 +0530 Subject: [PATCH 01/63] Add REST API health check for Optimization Detective --- .../includes/site-health/load.php | 15 ++++ .../includes/site-health/rest-api/helper.php | 85 +++++++++++++++++++ .../includes/site-health/rest-api/hooks.php | 29 +++++++ plugins/optimization-detective/load.php | 3 + 4 files changed, 132 insertions(+) create mode 100644 plugins/optimization-detective/includes/site-health/load.php create mode 100644 plugins/optimization-detective/includes/site-health/rest-api/helper.php create mode 100644 plugins/optimization-detective/includes/site-health/rest-api/hooks.php diff --git a/plugins/optimization-detective/includes/site-health/load.php b/plugins/optimization-detective/includes/site-health/load.php new file mode 100644 index 0000000000..65b8b17ee5 --- /dev/null +++ b/plugins/optimization-detective/includes/site-health/load.php @@ -0,0 +1,15 @@ + __( 'Your site has functional Optimization Detective REST API endpoint', 'optimization-detective' ), + 'status' => 'good', + 'badge' => array( + 'label' => __( 'Optimization Detective', 'optimization-detective' ), + 'color' => 'blue', + ), + 'description' => sprintf( + '

%s

', + __( 'Optimization Detective can send and store URL metrics via REST API endpoint', 'optimization-detective' ) + ), + 'actions' => '', + 'test' => 'optimization_detective_rest_api', + ); + + $rest_url = get_rest_url( null, OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ); + $response = wp_remote_post( + $rest_url, + array( + 'headers' => array( 'Content-Type' => 'application/json' ), + 'sslverify' => false, + ) + ); + + if ( is_wp_error( $response ) ) { + $result['status'] = 'recommended'; + $result['label'] = __( 'Your site does not have functional Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['description'] = sprintf( + '

%s

', + esc_html__( 'The Optimization Detective endpoint could not be reached. This might mean the REST API is disabled or blocked.', 'optimization-detective' ) + ); + $result['actions'] = sprintf( + '

%s

', + esc_html__( 'Ensure the REST API is enabled and not blocked by security settings.', 'optimization-detective' ) + ); + return $result; + } + + $status_code = wp_remote_retrieve_response_code( $response ); + $data = json_decode( wp_remote_retrieve_body( $response ), true ); + $expected_params = array( 'slug', 'current_etag', 'hmac', 'url', 'viewport', 'elements' ); + if ( + 400 === $status_code + && isset( $data['data']['params'] ) + && is_array( $data['data']['params'] ) + && count( $expected_params ) === count( array_intersect( $data['data']['params'], $expected_params ) ) + ) { + // The REST API endpoint is available. + return $result; + } + + // The REST API endpoint is blocked. + $result['status'] = 'recommended'; + $result['label'] = __( 'Your site does not have functional Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['description'] = sprintf( + '

%s

', + esc_html__( 'The Optimization Detective REST API endpoint is blocked, preventing URL metrics from being stored.', 'optimization-detective' ) + ); + $result['actions'] = sprintf( + '

%s

', + esc_html__( 'Adjust your security plugin or server settings to allow access to the endpoint.', 'optimization-detective' ) + ); + + return $result; +} diff --git a/plugins/optimization-detective/includes/site-health/rest-api/hooks.php b/plugins/optimization-detective/includes/site-health/rest-api/hooks.php new file mode 100644 index 0000000000..0765a6c45c --- /dev/null +++ b/plugins/optimization-detective/includes/site-health/rest-api/hooks.php @@ -0,0 +1,29 @@ +} $tests Site Health Tests. + * @return array{direct: array} Amended tests. + */ +function od_optimization_detective_add_rest_api_test( array $tests ): array { + $tests['direct']['optimization_detective_rest_api'] = array( + 'label' => __( 'Optimization Detective REST API Endpoint Availability', 'optimization-detective' ), + 'test' => 'od_optimization_detective_rest_api_test', + ); + + return $tests; +} +add_filter( 'site_status_tests', 'od_optimization_detective_add_rest_api_test' ); diff --git a/plugins/optimization-detective/load.php b/plugins/optimization-detective/load.php index 81b60cb75f..19d1817e6b 100644 --- a/plugins/optimization-detective/load.php +++ b/plugins/optimization-detective/load.php @@ -127,5 +127,8 @@ class_alias( OD_URL_Metric_Group_Collection::class, 'OD_URL_Metrics_Group_Collec // Add hooks for the above requires. require_once __DIR__ . '/hooks.php'; + + // Load site health checks. + require_once __DIR__ . '/includes/site-health/load.php'; } ); From 4bff9fef5beaea4803947a93692b47c2b4221a85 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Thu, 19 Dec 2024 18:40:06 +0530 Subject: [PATCH 02/63] Improve error message of site health check --- .../includes/site-health/rest-api/helper.php | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/plugins/optimization-detective/includes/site-health/rest-api/helper.php b/plugins/optimization-detective/includes/site-health/rest-api/helper.php index ba98d1b0d1..2475dfbad8 100644 --- a/plugins/optimization-detective/includes/site-health/rest-api/helper.php +++ b/plugins/optimization-detective/includes/site-health/rest-api/helper.php @@ -44,15 +44,11 @@ function od_optimization_detective_rest_api_test(): array { if ( is_wp_error( $response ) ) { $result['status'] = 'recommended'; - $result['label'] = __( 'Your site does not have functional Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['label'] = __( 'Your site encountered error accessing Optimization Detective REST API endpoint', 'optimization-detective' ); $result['description'] = sprintf( '

%s

', esc_html__( 'The Optimization Detective endpoint could not be reached. This might mean the REST API is disabled or blocked.', 'optimization-detective' ) ); - $result['actions'] = sprintf( - '

%s

', - esc_html__( 'Ensure the REST API is enabled and not blocked by security settings.', 'optimization-detective' ) - ); return $result; } @@ -67,19 +63,21 @@ function od_optimization_detective_rest_api_test(): array { ) { // The REST API endpoint is available. return $result; + } elseif ( 401 === $status_code ) { + $result['status'] = 'recommended'; + $result['label'] = __( 'Your site encountered unauthorized error for Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['description'] = sprintf( + '

%s

', + esc_html__( 'The REST API endpoint requires authentication. Ensure proper credentials are provided.', 'optimization-detective' ) + ); + } elseif ( 403 === $status_code ) { + $result['status'] = 'recommended'; + $result['label'] = __( 'Your site encountered forbidden error for Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['description'] = sprintf( + '

%s

', + esc_html__( 'The REST API endpoint is blocked check server or security settings.', 'optimization-detective' ) + ); } - // The REST API endpoint is blocked. - $result['status'] = 'recommended'; - $result['label'] = __( 'Your site does not have functional Optimization Detective REST API endpoint', 'optimization-detective' ); - $result['description'] = sprintf( - '

%s

', - esc_html__( 'The Optimization Detective REST API endpoint is blocked, preventing URL metrics from being stored.', 'optimization-detective' ) - ); - $result['actions'] = sprintf( - '

%s

', - esc_html__( 'Adjust your security plugin or server settings to allow access to the endpoint.', 'optimization-detective' ) - ); - return $result; } From 56d769a29b7116266426fafd4ef2030dbfba6f1a Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Thu, 19 Dec 2024 18:46:38 +0530 Subject: [PATCH 03/63] Add scheduled health check for Optimization Detective REST API --- .../includes/site-health/rest-api/helper.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/plugins/optimization-detective/includes/site-health/rest-api/helper.php b/plugins/optimization-detective/includes/site-health/rest-api/helper.php index 2475dfbad8..7854fbf4b2 100644 --- a/plugins/optimization-detective/includes/site-health/rest-api/helper.php +++ b/plugins/optimization-detective/includes/site-health/rest-api/helper.php @@ -81,3 +81,25 @@ function od_optimization_detective_rest_api_test(): array { return $result; } + +/** + * Periodically runs the Optimization Detective REST API health check. + * + * @since n.e.x.t + */ +function od_schedule_rest_api_health_check(): void { + if ( ! (bool) wp_next_scheduled( 'od_rest_api_health_check_event' ) ) { + wp_schedule_event( time(), 'hourly', 'od_rest_api_health_check_event' ); + } +} +add_action( 'wp', 'od_schedule_rest_api_health_check' ); + +/** + * Hook for the scheduled REST API health check. + * + * @since n.e.x.t + */ +function od_run_scheduled_rest_api_health_check(): void { + od_optimization_detective_rest_api_test(); +} +add_action( 'od_rest_api_health_check_event', 'od_run_scheduled_rest_api_health_check' ); From ba911e1cb5e7117f22827d85b0c4fca7a77b04b5 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Thu, 19 Dec 2024 18:54:02 +0530 Subject: [PATCH 04/63] Save site health check status for Optimization Detective REST API --- .../includes/site-health/rest-api/helper.php | 28 +++++++++++++++++++ .../optimization-detective/optimization.php | 6 ++++ 2 files changed, 34 insertions(+) diff --git a/plugins/optimization-detective/includes/site-health/rest-api/helper.php b/plugins/optimization-detective/includes/site-health/rest-api/helper.php index 7854fbf4b2..4c4ca56471 100644 --- a/plugins/optimization-detective/includes/site-health/rest-api/helper.php +++ b/plugins/optimization-detective/includes/site-health/rest-api/helper.php @@ -49,6 +49,13 @@ function od_optimization_detective_rest_api_test(): array { '

%s

', esc_html__( 'The Optimization Detective endpoint could not be reached. This might mean the REST API is disabled or blocked.', 'optimization-detective' ) ); + update_option( + 'od_rest_api_info', + array( + 'status' => 'error', + 'available' => false, + ) + ); return $result; } @@ -62,6 +69,13 @@ function od_optimization_detective_rest_api_test(): array { && count( $expected_params ) === count( array_intersect( $data['data']['params'], $expected_params ) ) ) { // The REST API endpoint is available. + update_option( + 'od_rest_api_info', + array( + 'status' => 'ok', + 'available' => true, + ) + ); return $result; } elseif ( 401 === $status_code ) { $result['status'] = 'recommended'; @@ -70,6 +84,13 @@ function od_optimization_detective_rest_api_test(): array { '

%s

', esc_html__( 'The REST API endpoint requires authentication. Ensure proper credentials are provided.', 'optimization-detective' ) ); + update_option( + 'od_rest_api_info', + array( + 'status' => 'unauthorized', + 'available' => false, + ) + ); } elseif ( 403 === $status_code ) { $result['status'] = 'recommended'; $result['label'] = __( 'Your site encountered forbidden error for Optimization Detective REST API endpoint', 'optimization-detective' ); @@ -77,6 +98,13 @@ function od_optimization_detective_rest_api_test(): array { '

%s

', esc_html__( 'The REST API endpoint is blocked check server or security settings.', 'optimization-detective' ) ); + update_option( + 'od_rest_api_info', + array( + 'status' => 'forbidden', + 'available' => false, + ) + ); } return $result; diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index fa72a6194e..605539d582 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -226,6 +226,12 @@ function od_optimize_template_output_buffer( string $buffer ): string { // Whether we need to add the data-od-xpath attribute to elements and whether the detection script should be injected. $needs_detection = ! $group_collection->is_every_group_complete(); + // Disable detection if the REST API is disabled. + $od_rest_api_info = get_option( 'od_rest_api_info' ); + if ( is_array( $od_rest_api_info ) && isset( $od_rest_api_info['available'] ) ) { + $needs_detection = (bool) $od_rest_api_info['available']; + } + do { $tracked_in_url_metrics = false; $processor->set_bookmark( $current_tag_bookmark ); // TODO: Should we break if this returns false? From 12a229ca3f2c419d9fb9480648dd06061cb1b6f5 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Thu, 19 Dec 2024 20:00:39 +0530 Subject: [PATCH 05/63] Add test for when REST API is available --- ...n-detective-rest-api-site-health-check.php | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php diff --git a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php new file mode 100644 index 0000000000..e0c611f0e0 --- /dev/null +++ b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php @@ -0,0 +1,90 @@ +> + */ + protected $mocked_responses = array(); + + /** + * Setup each test. + */ + public function setUp(): void { + parent::setUp(); + + // Clear any filters or mocks. + remove_all_filters( 'pre_http_request' ); + + // Add the filter to mock HTTP requests. + add_filter( 'pre_http_request', array( $this, 'mock_http_requests' ), 10, 3 ); + } + + /** + * Test that the site health check is `good` when the REST API is available. + */ + public function test_rest_api_available(): void { + $this->mocked_responses = array( + get_rest_url( null, OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ) => $this->build_mock_response( + 400, + 'Bad Request', + array( + 'data' => array( + 'params' => array( 'slug', 'current_etag', 'hmac', 'url', 'viewport', 'elements' ), + ), + ) + ), + ); + + $result = od_optimization_detective_rest_api_test(); + + $this->assertSame( 'good', $result['status'] ); + } + + /** + * Mock HTTP requests for assets to simulate different responses. + * + * @param bool $response A preemptive return value of an HTTP request. Default false. + * @param array $args Request arguments. + * @param string $url The request URL. + * @return array Mocked response. + */ + public function mock_http_requests( bool $response, array $args, string $url ): array { + if ( isset( $this->mocked_responses[ $url ] ) ) { + return $this->mocked_responses[ $url ]; + } + + // If no specific mock set, default to a generic success response. + return array( + 'response' => array( + 'code' => 200, + 'message' => 'OK', + ), + ); + } + + /** + * Build a mock response. + * + * @param int $status_code HTTP status code. + * @param string $message HTTP status message. + * @param array $body Response body. + * @return array Mocked response. + */ + protected function build_mock_response( int $status_code, string $message, array $body = array() ): array { + return array( + 'response' => array( + 'code' => $status_code, + 'message' => $message, + ), + 'body' => wp_json_encode( $body ), + ); + } +} From a0f460dc06eedad9387193a1ec220a3dbd7a2ec5 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Thu, 19 Dec 2024 23:23:22 +0530 Subject: [PATCH 06/63] Add tests for REST API unauthorized error handling --- ...n-detective-rest-api-site-health-check.php | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php index e0c611f0e0..f9cc7354e3 100644 --- a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php +++ b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php @@ -43,9 +43,31 @@ public function test_rest_api_available(): void { ), ); - $result = od_optimization_detective_rest_api_test(); + $result = od_optimization_detective_rest_api_test(); + $od_rest_api_info = get_option( 'od_rest_api_info', array() ); $this->assertSame( 'good', $result['status'] ); + $this->assertSame( 'ok', isset( $od_rest_api_info['status'] ) ? $od_rest_api_info['status'] : '' ); + $this->assertTrue( isset( $od_rest_api_info['available'] ) ? $od_rest_api_info['available'] : false ); + } + + /** + * Test behavior when REST API returns unauthorized error. + */ + public function test_rest_api_unauthorized(): void { + $this->mocked_responses = array( + get_rest_url( null, OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ) => $this->build_mock_response( + 401, + 'Unauthorized' + ), + ); + + $result = od_optimization_detective_rest_api_test(); + $od_rest_api_info = get_option( 'od_rest_api_info', array() ); + + $this->assertSame( 'recommended', $result['status'] ); + $this->assertSame( 'unauthorized', isset( $od_rest_api_info['status'] ) ? $od_rest_api_info['status'] : '' ); + $this->assertFalse( isset( $od_rest_api_info['available'] ) ? $od_rest_api_info['available'] : true ); } /** From c29cb7ce78d00639e653a3c50c2da7eb4912e668 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Thu, 19 Dec 2024 23:25:00 +0530 Subject: [PATCH 07/63] Add test for REST API forbidden error handling --- ...n-detective-rest-api-site-health-check.php | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php index f9cc7354e3..f45e0599fc 100644 --- a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php +++ b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php @@ -52,7 +52,7 @@ public function test_rest_api_available(): void { } /** - * Test behavior when REST API returns unauthorized error. + * Test behavior when REST API returns an unauthorized error. */ public function test_rest_api_unauthorized(): void { $this->mocked_responses = array( @@ -70,6 +70,25 @@ public function test_rest_api_unauthorized(): void { $this->assertFalse( isset( $od_rest_api_info['available'] ) ? $od_rest_api_info['available'] : true ); } + /** + * Test behavior when REST API returns an forbidden error. + */ + public function test_rest_api_forbidden(): void { + $this->mocked_responses = array( + get_rest_url( null, OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ) => $this->build_mock_response( + 403, + 'Forbidden' + ), + ); + + $result = od_optimization_detective_rest_api_test(); + $od_rest_api_info = get_option( 'od_rest_api_info', array() ); + + $this->assertSame( 'recommended', $result['status'] ); + $this->assertSame( 'forbidden', isset( $od_rest_api_info['status'] ) ? $od_rest_api_info['status'] : '' ); + $this->assertFalse( isset( $od_rest_api_info['available'] ) ? $od_rest_api_info['available'] : true ); + } + /** * Mock HTTP requests for assets to simulate different responses. * From 8d8ae3ea7bf0a2e69dffd6901f549c732ad0e0e1 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Fri, 20 Dec 2024 18:04:11 +0530 Subject: [PATCH 08/63] Refactor to move site-health directory to plugin root directory --- plugins/optimization-detective/load.php | 2 +- .../optimization-detective/{includes => }/site-health/load.php | 0 .../{includes => }/site-health/rest-api/helper.php | 0 .../{includes => }/site-health/rest-api/hooks.php | 0 4 files changed, 1 insertion(+), 1 deletion(-) rename plugins/optimization-detective/{includes => }/site-health/load.php (100%) rename plugins/optimization-detective/{includes => }/site-health/rest-api/helper.php (100%) rename plugins/optimization-detective/{includes => }/site-health/rest-api/hooks.php (100%) diff --git a/plugins/optimization-detective/load.php b/plugins/optimization-detective/load.php index 19d1817e6b..c14ba42407 100644 --- a/plugins/optimization-detective/load.php +++ b/plugins/optimization-detective/load.php @@ -129,6 +129,6 @@ class_alias( OD_URL_Metric_Group_Collection::class, 'OD_URL_Metrics_Group_Collec require_once __DIR__ . '/hooks.php'; // Load site health checks. - require_once __DIR__ . '/includes/site-health/load.php'; + require_once __DIR__ . '/site-health/load.php'; } ); diff --git a/plugins/optimization-detective/includes/site-health/load.php b/plugins/optimization-detective/site-health/load.php similarity index 100% rename from plugins/optimization-detective/includes/site-health/load.php rename to plugins/optimization-detective/site-health/load.php diff --git a/plugins/optimization-detective/includes/site-health/rest-api/helper.php b/plugins/optimization-detective/site-health/rest-api/helper.php similarity index 100% rename from plugins/optimization-detective/includes/site-health/rest-api/helper.php rename to plugins/optimization-detective/site-health/rest-api/helper.php diff --git a/plugins/optimization-detective/includes/site-health/rest-api/hooks.php b/plugins/optimization-detective/site-health/rest-api/hooks.php similarity index 100% rename from plugins/optimization-detective/includes/site-health/rest-api/hooks.php rename to plugins/optimization-detective/site-health/rest-api/hooks.php From 422a5bf8eef6eb7c1e131a08bcf9028d0a2af77f Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Fri, 20 Dec 2024 18:15:41 +0530 Subject: [PATCH 09/63] Clear site health check data during plugin uninstallation --- plugins/optimization-detective/uninstall.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/optimization-detective/uninstall.php b/plugins/optimization-detective/uninstall.php index 6202323d9b..a646ed011b 100644 --- a/plugins/optimization-detective/uninstall.php +++ b/plugins/optimization-detective/uninstall.php @@ -17,6 +17,10 @@ // Delete all URL Metrics posts for the current site. OD_URL_Metrics_Post_Type::delete_all_posts(); wp_unschedule_hook( OD_URL_Metrics_Post_Type::GC_CRON_EVENT_NAME ); + + // Clear out site health check data. + delete_option( 'od_rest_api_info' ); + wp_unschedule_hook( 'od_rest_api_health_check_event' ); }; $od_delete_site_data(); From 0bc74f7eac3fb6b04510706fecb1ad556336452f Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Fri, 20 Dec 2024 18:31:33 +0530 Subject: [PATCH 10/63] Move REST API availability check to appropriate function --- plugins/optimization-detective/optimization.php | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index 605539d582..a843d17c36 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -78,7 +78,12 @@ static function ( string $output, ?int $phase ): string { * @access private */ function od_maybe_add_template_output_buffer_filter(): void { - if ( ! od_can_optimize_response() || isset( $_GET['optimization_detective_disabled'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended + $od_rest_api_info = get_option( 'od_rest_api_info' ); + if ( + ! od_can_optimize_response() || + isset( $_GET['optimization_detective_disabled'] ) || // phpcs:ignore WordPress.Security.NonceVerification.Recommended + ( isset( $od_rest_api_info['available'] ) && ! (bool) $od_rest_api_info['available'] ) + ) { return; } $callback = 'od_optimize_template_output_buffer'; @@ -226,12 +231,6 @@ function od_optimize_template_output_buffer( string $buffer ): string { // Whether we need to add the data-od-xpath attribute to elements and whether the detection script should be injected. $needs_detection = ! $group_collection->is_every_group_complete(); - // Disable detection if the REST API is disabled. - $od_rest_api_info = get_option( 'od_rest_api_info' ); - if ( is_array( $od_rest_api_info ) && isset( $od_rest_api_info['available'] ) ) { - $needs_detection = (bool) $od_rest_api_info['available']; - } - do { $tracked_in_url_metrics = false; $processor->set_bookmark( $current_tag_bookmark ); // TODO: Should we break if this returns false? From 36042fdb3e310a817711b5bf1e838fbc59411bbc Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Fri, 20 Dec 2024 18:38:47 +0530 Subject: [PATCH 11/63] Add error message and error code to option --- .../optimization-detective/site-health/rest-api/helper.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/optimization-detective/site-health/rest-api/helper.php b/plugins/optimization-detective/site-health/rest-api/helper.php index 4c4ca56471..fa6136f00b 100644 --- a/plugins/optimization-detective/site-health/rest-api/helper.php +++ b/plugins/optimization-detective/site-health/rest-api/helper.php @@ -52,8 +52,9 @@ function od_optimization_detective_rest_api_test(): array { update_option( 'od_rest_api_info', array( - 'status' => 'error', - 'available' => false, + 'error_message' => $response->get_error_message(), + 'error_code' => $response->get_error_code(), + 'available' => false, ) ); return $result; From ad21df8140fe4cfae453e6b141f028fe3c01fbf2 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Fri, 20 Dec 2024 18:47:49 +0530 Subject: [PATCH 12/63] Refactor to store status code instead of string, Add fallback else condition --- .../site-health/rest-api/helper.php | 26 ++++++++++++++----- ...n-detective-rest-api-site-health-check.php | 6 ++--- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/plugins/optimization-detective/site-health/rest-api/helper.php b/plugins/optimization-detective/site-health/rest-api/helper.php index fa6136f00b..a4f0723ae6 100644 --- a/plugins/optimization-detective/site-health/rest-api/helper.php +++ b/plugins/optimization-detective/site-health/rest-api/helper.php @@ -73,8 +73,8 @@ function od_optimization_detective_rest_api_test(): array { update_option( 'od_rest_api_info', array( - 'status' => 'ok', - 'available' => true, + 'status_code' => $status_code, + 'available' => true, ) ); return $result; @@ -88,8 +88,8 @@ function od_optimization_detective_rest_api_test(): array { update_option( 'od_rest_api_info', array( - 'status' => 'unauthorized', - 'available' => false, + 'status_code' => $status_code, + 'available' => false, ) ); } elseif ( 403 === $status_code ) { @@ -102,8 +102,22 @@ function od_optimization_detective_rest_api_test(): array { update_option( 'od_rest_api_info', array( - 'status' => 'forbidden', - 'available' => false, + 'status_code' => $status_code, + 'available' => false, + ) + ); + } else { + $result['status'] = 'recommended'; + $result['label'] = __( 'Your site encountered error accessing Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['description'] = sprintf( + '

%s

', + esc_html__( 'The Optimization Detective endpoint could not be reached. This might mean the REST API is disabled or blocked.', 'optimization-detective' ) + ); + update_option( + 'od_rest_api_info', + array( + 'status_code' => $status_code, + 'available' => false, ) ); } diff --git a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php index f45e0599fc..cc5adeed5d 100644 --- a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php +++ b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php @@ -47,7 +47,7 @@ public function test_rest_api_available(): void { $od_rest_api_info = get_option( 'od_rest_api_info', array() ); $this->assertSame( 'good', $result['status'] ); - $this->assertSame( 'ok', isset( $od_rest_api_info['status'] ) ? $od_rest_api_info['status'] : '' ); + $this->assertSame( 400, isset( $od_rest_api_info['status_code'] ) ? $od_rest_api_info['status_code'] : '' ); $this->assertTrue( isset( $od_rest_api_info['available'] ) ? $od_rest_api_info['available'] : false ); } @@ -66,7 +66,7 @@ public function test_rest_api_unauthorized(): void { $od_rest_api_info = get_option( 'od_rest_api_info', array() ); $this->assertSame( 'recommended', $result['status'] ); - $this->assertSame( 'unauthorized', isset( $od_rest_api_info['status'] ) ? $od_rest_api_info['status'] : '' ); + $this->assertSame( 401, isset( $od_rest_api_info['status_code'] ) ? $od_rest_api_info['status_code'] : '' ); $this->assertFalse( isset( $od_rest_api_info['available'] ) ? $od_rest_api_info['available'] : true ); } @@ -85,7 +85,7 @@ public function test_rest_api_forbidden(): void { $od_rest_api_info = get_option( 'od_rest_api_info', array() ); $this->assertSame( 'recommended', $result['status'] ); - $this->assertSame( 'forbidden', isset( $od_rest_api_info['status'] ) ? $od_rest_api_info['status'] : '' ); + $this->assertSame( 403, isset( $od_rest_api_info['status_code'] ) ? $od_rest_api_info['status_code'] : '' ); $this->assertFalse( isset( $od_rest_api_info['available'] ) ? $od_rest_api_info['available'] : true ); } From 0bba4688a5ae93e8ef9ba03dc968fbac86273a83 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Fri, 20 Dec 2024 19:03:08 +0530 Subject: [PATCH 13/63] Refactor to only use update_option once --- .../site-health/rest-api/helper.php | 110 +++++++----------- 1 file changed, 42 insertions(+), 68 deletions(-) diff --git a/plugins/optimization-detective/site-health/rest-api/helper.php b/plugins/optimization-detective/site-health/rest-api/helper.php index a4f0723ae6..7c5af8a472 100644 --- a/plugins/optimization-detective/site-health/rest-api/helper.php +++ b/plugins/optimization-detective/site-health/rest-api/helper.php @@ -49,79 +49,53 @@ function od_optimization_detective_rest_api_test(): array { '

%s

', esc_html__( 'The Optimization Detective endpoint could not be reached. This might mean the REST API is disabled or blocked.', 'optimization-detective' ) ); - update_option( - 'od_rest_api_info', - array( - 'error_message' => $response->get_error_message(), - 'error_code' => $response->get_error_code(), - 'available' => false, - ) - ); - return $result; - } - - $status_code = wp_remote_retrieve_response_code( $response ); - $data = json_decode( wp_remote_retrieve_body( $response ), true ); - $expected_params = array( 'slug', 'current_etag', 'hmac', 'url', 'viewport', 'elements' ); - if ( - 400 === $status_code - && isset( $data['data']['params'] ) - && is_array( $data['data']['params'] ) - && count( $expected_params ) === count( array_intersect( $data['data']['params'], $expected_params ) ) - ) { - // The REST API endpoint is available. - update_option( - 'od_rest_api_info', - array( - 'status_code' => $status_code, - 'available' => true, - ) - ); - return $result; - } elseif ( 401 === $status_code ) { - $result['status'] = 'recommended'; - $result['label'] = __( 'Your site encountered unauthorized error for Optimization Detective REST API endpoint', 'optimization-detective' ); - $result['description'] = sprintf( - '

%s

', - esc_html__( 'The REST API endpoint requires authentication. Ensure proper credentials are provided.', 'optimization-detective' ) - ); - update_option( - 'od_rest_api_info', - array( - 'status_code' => $status_code, - 'available' => false, - ) - ); - } elseif ( 403 === $status_code ) { - $result['status'] = 'recommended'; - $result['label'] = __( 'Your site encountered forbidden error for Optimization Detective REST API endpoint', 'optimization-detective' ); - $result['description'] = sprintf( - '

%s

', - esc_html__( 'The REST API endpoint is blocked check server or security settings.', 'optimization-detective' ) - ); - update_option( - 'od_rest_api_info', - array( - 'status_code' => $status_code, - 'available' => false, - ) + $info = array( + 'error_message' => $response->get_error_message(), + 'error_code' => $response->get_error_code(), + 'available' => false, ); } else { - $result['status'] = 'recommended'; - $result['label'] = __( 'Your site encountered error accessing Optimization Detective REST API endpoint', 'optimization-detective' ); - $result['description'] = sprintf( - '

%s

', - esc_html__( 'The Optimization Detective endpoint could not be reached. This might mean the REST API is disabled or blocked.', 'optimization-detective' ) - ); - update_option( - 'od_rest_api_info', - array( - 'status_code' => $status_code, - 'available' => false, - ) + $status_code = wp_remote_retrieve_response_code( $response ); + $data = json_decode( wp_remote_retrieve_body( $response ), true ); + $expected_params = array( 'slug', 'current_etag', 'hmac', 'url', 'viewport', 'elements' ); + $info = array( + 'status_code' => $status_code, + 'available' => false, ); + + if ( + 400 === $status_code + && isset( $data['data']['params'] ) + && is_array( $data['data']['params'] ) + && count( $expected_params ) === count( array_intersect( $data['data']['params'], $expected_params ) ) + ) { + // The REST API endpoint is available. + $info['available'] = true; + } elseif ( 401 === $status_code ) { + $result['status'] = 'recommended'; + $result['label'] = __( 'Your site encountered unauthorized error for Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['description'] = sprintf( + '

%s

', + esc_html__( 'The REST API endpoint requires authentication. Ensure proper credentials are provided.', 'optimization-detective' ) + ); + } elseif ( 403 === $status_code ) { + $result['status'] = 'recommended'; + $result['label'] = __( 'Your site encountered forbidden error for Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['description'] = sprintf( + '

%s

', + esc_html__( 'The REST API endpoint is blocked check server or security settings.', 'optimization-detective' ) + ); + } else { + $result['status'] = 'recommended'; + $result['label'] = __( 'Your site encountered error accessing Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['description'] = sprintf( + '

%s

', + esc_html__( 'The Optimization Detective endpoint could not be reached. This might mean the REST API is disabled or blocked.', 'optimization-detective' ) + ); + } } + update_option( 'od_rest_api_info', $info ); return $result; } From e1b90043c123b52798cd2b3dc5eeae99bdd807f3 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Mon, 23 Dec 2024 19:07:09 +0530 Subject: [PATCH 14/63] Add activation hook to initialize option value --- plugins/optimization-detective/load.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/plugins/optimization-detective/load.php b/plugins/optimization-detective/load.php index c14ba42407..5b68454af5 100644 --- a/plugins/optimization-detective/load.php +++ b/plugins/optimization-detective/load.php @@ -132,3 +132,16 @@ class_alias( OD_URL_Metric_Group_Collection::class, 'OD_URL_Metrics_Group_Collec require_once __DIR__ . '/site-health/load.php'; } ); + +/** + * Activation hook for the plugin. + * + * @since n.e.x.t + */ +function od_plugin_activation(): void { + // Add the option if it doesn't exist. + if ( ! (bool) get_option( 'od_rest_api_info' ) ) { + add_option( 'od_rest_api_info', array() ); + } +} +register_activation_hook( __FILE__, 'od_plugin_activation' ); From dbede659b850ef05917c48920a04da5ace465e67 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Mon, 23 Dec 2024 19:19:58 +0530 Subject: [PATCH 15/63] Move scheduling to activation hook --- plugins/optimization-detective/load.php | 2 ++ plugins/optimization-detective/site-health/rest-api/helper.php | 2 -- plugins/optimization-detective/site-health/rest-api/hooks.php | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/plugins/optimization-detective/load.php b/plugins/optimization-detective/load.php index 5b68454af5..9e57c453c8 100644 --- a/plugins/optimization-detective/load.php +++ b/plugins/optimization-detective/load.php @@ -143,5 +143,7 @@ function od_plugin_activation(): void { if ( ! (bool) get_option( 'od_rest_api_info' ) ) { add_option( 'od_rest_api_info', array() ); } + require_once __DIR__ . '/site-health/rest-api/helper.php'; + od_schedule_rest_api_health_check(); } register_activation_hook( __FILE__, 'od_plugin_activation' ); diff --git a/plugins/optimization-detective/site-health/rest-api/helper.php b/plugins/optimization-detective/site-health/rest-api/helper.php index 7c5af8a472..7911c6223e 100644 --- a/plugins/optimization-detective/site-health/rest-api/helper.php +++ b/plugins/optimization-detective/site-health/rest-api/helper.php @@ -109,7 +109,6 @@ function od_schedule_rest_api_health_check(): void { wp_schedule_event( time(), 'hourly', 'od_rest_api_health_check_event' ); } } -add_action( 'wp', 'od_schedule_rest_api_health_check' ); /** * Hook for the scheduled REST API health check. @@ -119,4 +118,3 @@ function od_schedule_rest_api_health_check(): void { function od_run_scheduled_rest_api_health_check(): void { od_optimization_detective_rest_api_test(); } -add_action( 'od_rest_api_health_check_event', 'od_run_scheduled_rest_api_health_check' ); diff --git a/plugins/optimization-detective/site-health/rest-api/hooks.php b/plugins/optimization-detective/site-health/rest-api/hooks.php index 0765a6c45c..419494c8d1 100644 --- a/plugins/optimization-detective/site-health/rest-api/hooks.php +++ b/plugins/optimization-detective/site-health/rest-api/hooks.php @@ -27,3 +27,6 @@ function od_optimization_detective_add_rest_api_test( array $tests ): array { return $tests; } add_filter( 'site_status_tests', 'od_optimization_detective_add_rest_api_test' ); + +// Hook for the scheduled REST API health check. +add_action( 'od_rest_api_health_check_event', 'od_run_scheduled_rest_api_health_check' ); From 2b2ca3ecb36a285ac35aeb5fb474352a26d5e731 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Mon, 23 Dec 2024 19:23:32 +0530 Subject: [PATCH 16/63] Change health check scheduling from hourly to weekly --- plugins/optimization-detective/site-health/rest-api/helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/optimization-detective/site-health/rest-api/helper.php b/plugins/optimization-detective/site-health/rest-api/helper.php index 7911c6223e..2f49eeee60 100644 --- a/plugins/optimization-detective/site-health/rest-api/helper.php +++ b/plugins/optimization-detective/site-health/rest-api/helper.php @@ -106,7 +106,7 @@ function od_optimization_detective_rest_api_test(): array { */ function od_schedule_rest_api_health_check(): void { if ( ! (bool) wp_next_scheduled( 'od_rest_api_health_check_event' ) ) { - wp_schedule_event( time(), 'hourly', 'od_rest_api_health_check_event' ); + wp_schedule_event( time(), 'weekly', 'od_rest_api_health_check_event' ); } } From e7c1001aaa5a70f822c904d10010384b213b9744 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Tue, 24 Dec 2024 18:40:10 +0530 Subject: [PATCH 17/63] Run REST API health check on plugin activation, Display notice if check fails --- plugins/optimization-detective/load.php | 18 ++++++++++-- .../site-health/rest-api/helper.php | 29 +++++++++++++++++++ .../site-health/rest-api/hooks.php | 1 + 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/plugins/optimization-detective/load.php b/plugins/optimization-detective/load.php index 9e57c453c8..11d5b38963 100644 --- a/plugins/optimization-detective/load.php +++ b/plugins/optimization-detective/load.php @@ -51,6 +51,13 @@ static function ( string $global_var_name, string $version, Closure $load ): voi * action), so this is why it gets initialized at priority 9. */ add_action( 'init', $bootstrap, 9 ); + register_activation_hook( + __FILE__, + static function () use ( $bootstrap ): void { + $bootstrap(); + od_plugin_activation(); + } + ); } // Register this copy of the plugin. @@ -143,7 +150,14 @@ function od_plugin_activation(): void { if ( ! (bool) get_option( 'od_rest_api_info' ) ) { add_option( 'od_rest_api_info', array() ); } - require_once __DIR__ . '/site-health/rest-api/helper.php'; od_schedule_rest_api_health_check(); + // Run the check immediately after Optimization Detective is activated. + add_action( + 'activated_plugin', + static function ( string $plugin ): void { + if ( 'optimization-detective/load.php' === $plugin ) { + od_optimization_detective_rest_api_test(); + } + } + ); } -register_activation_hook( __FILE__, 'od_plugin_activation' ); diff --git a/plugins/optimization-detective/site-health/rest-api/helper.php b/plugins/optimization-detective/site-health/rest-api/helper.php index 2f49eeee60..acdd8ff520 100644 --- a/plugins/optimization-detective/site-health/rest-api/helper.php +++ b/plugins/optimization-detective/site-health/rest-api/helper.php @@ -93,6 +93,7 @@ function od_optimization_detective_rest_api_test(): array { esc_html__( 'The Optimization Detective endpoint could not be reached. This might mean the REST API is disabled or blocked.', 'optimization-detective' ) ); } + $info['error_message'] = $result['label']; } update_option( 'od_rest_api_info', $info ); @@ -118,3 +119,31 @@ function od_schedule_rest_api_health_check(): void { function od_run_scheduled_rest_api_health_check(): void { od_optimization_detective_rest_api_test(); } + +/** + * Displays an admin notice if the REST API health check fails. + * + * @since n.e.x.t + * + * @param string $plugin_file Plugin file. + */ +function od_rest_api_health_check_admin_notice( string $plugin_file ): void { + if ( 'optimization-detective/load.php' !== $plugin_file ) { + return; + } + + $od_rest_api_info = get_option( 'od_rest_api_info', array() ); + if ( + isset( $od_rest_api_info['available'] ) && + ! (bool) $od_rest_api_info['available'] && + isset( $od_rest_api_info['error_message'] ) + ) { + wp_admin_notice( + esc_html( $od_rest_api_info['error_message'] ), + array( + 'type' => 'warning', + 'additional_classes' => array( 'inline', 'notice-alt' ), + ) + ); + } +} diff --git a/plugins/optimization-detective/site-health/rest-api/hooks.php b/plugins/optimization-detective/site-health/rest-api/hooks.php index 419494c8d1..d217803e9b 100644 --- a/plugins/optimization-detective/site-health/rest-api/hooks.php +++ b/plugins/optimization-detective/site-health/rest-api/hooks.php @@ -30,3 +30,4 @@ function od_optimization_detective_add_rest_api_test( array $tests ): array { // Hook for the scheduled REST API health check. add_action( 'od_rest_api_health_check_event', 'od_run_scheduled_rest_api_health_check' ); +add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); From 7e4e0574814cb6e37a1a0aaf0996962f0b2fe28b Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Tue, 24 Dec 2024 18:55:18 +0530 Subject: [PATCH 18/63] Move activation logic to separate function --- plugins/optimization-detective/load.php | 20 +++++------------ .../site-health/rest-api/helper.php | 22 +++++++++++++++++++ 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/plugins/optimization-detective/load.php b/plugins/optimization-detective/load.php index 11d5b38963..11ee99392b 100644 --- a/plugins/optimization-detective/load.php +++ b/plugins/optimization-detective/load.php @@ -51,9 +51,14 @@ static function ( string $global_var_name, string $version, Closure $load ): voi * action), so this is why it gets initialized at priority 9. */ add_action( 'init', $bootstrap, 9 ); + register_activation_hook( __FILE__, static function () use ( $bootstrap ): void { + /* + * The activation hook is called before the init action, so the plugin is not loaded yet. This + * means that the plugin must be bootstrapped here to run the activation logic. + */ $bootstrap(); od_plugin_activation(); } @@ -146,18 +151,5 @@ class_alias( OD_URL_Metric_Group_Collection::class, 'OD_URL_Metrics_Group_Collec * @since n.e.x.t */ function od_plugin_activation(): void { - // Add the option if it doesn't exist. - if ( ! (bool) get_option( 'od_rest_api_info' ) ) { - add_option( 'od_rest_api_info', array() ); - } - od_schedule_rest_api_health_check(); - // Run the check immediately after Optimization Detective is activated. - add_action( - 'activated_plugin', - static function ( string $plugin ): void { - if ( 'optimization-detective/load.php' === $plugin ) { - od_optimization_detective_rest_api_test(); - } - } - ); + od_rest_api_health_check_plugin_activation(); } diff --git a/plugins/optimization-detective/site-health/rest-api/helper.php b/plugins/optimization-detective/site-health/rest-api/helper.php index acdd8ff520..fa9a7263ad 100644 --- a/plugins/optimization-detective/site-health/rest-api/helper.php +++ b/plugins/optimization-detective/site-health/rest-api/helper.php @@ -147,3 +147,25 @@ function od_rest_api_health_check_admin_notice( string $plugin_file ): void { ); } } + +/** + * Plugin activation hook for the REST API health check. + * + * @since n.e.x.t + */ +function od_rest_api_health_check_plugin_activation(): void { + // Add the option if it doesn't exist. + if ( ! (bool) get_option( 'od_rest_api_info' ) ) { + add_option( 'od_rest_api_info', array() ); + } + od_schedule_rest_api_health_check(); + // Run the check immediately after Optimization Detective is activated. + add_action( + 'activated_plugin', + static function ( string $plugin ): void { + if ( 'optimization-detective/load.php' === $plugin ) { + od_optimization_detective_rest_api_test(); + } + } + ); +} From a2baa65780543e03203c715a50168d005e1f83de Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Tue, 24 Dec 2024 19:21:05 +0530 Subject: [PATCH 19/63] Refactor activation logic to directly call REST API health check function --- plugins/optimization-detective/load.php | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/plugins/optimization-detective/load.php b/plugins/optimization-detective/load.php index 11ee99392b..be09dfdd75 100644 --- a/plugins/optimization-detective/load.php +++ b/plugins/optimization-detective/load.php @@ -60,7 +60,7 @@ static function () use ( $bootstrap ): void { * means that the plugin must be bootstrapped here to run the activation logic. */ $bootstrap(); - od_plugin_activation(); + od_rest_api_health_check_plugin_activation(); } ); } @@ -144,12 +144,3 @@ class_alias( OD_URL_Metric_Group_Collection::class, 'OD_URL_Metrics_Group_Collec require_once __DIR__ . '/site-health/load.php'; } ); - -/** - * Activation hook for the plugin. - * - * @since n.e.x.t - */ -function od_plugin_activation(): void { - od_rest_api_health_check_plugin_activation(); -} From 48c83a56928c7410e9b3039aa25d6e21a5d960eb Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Wed, 25 Dec 2024 23:30:46 +0530 Subject: [PATCH 20/63] Improve site health status messages for clarity and consistency --- .../site-health/rest-api/helper.php | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/plugins/optimization-detective/site-health/rest-api/helper.php b/plugins/optimization-detective/site-health/rest-api/helper.php index fa9a7263ad..bbda348a9b 100644 --- a/plugins/optimization-detective/site-health/rest-api/helper.php +++ b/plugins/optimization-detective/site-health/rest-api/helper.php @@ -19,7 +19,7 @@ */ function od_optimization_detective_rest_api_test(): array { $result = array( - 'label' => __( 'Your site has functional Optimization Detective REST API endpoint', 'optimization-detective' ), + 'label' => __( 'The REST API endpoint is functional.', 'optimization-detective' ), 'status' => 'good', 'badge' => array( 'label' => __( 'Optimization Detective', 'optimization-detective' ), @@ -27,7 +27,7 @@ function od_optimization_detective_rest_api_test(): array { ), 'description' => sprintf( '

%s

', - __( 'Optimization Detective can send and store URL metrics via REST API endpoint', 'optimization-detective' ) + __( 'Your site can send and receive URL metrics via the REST API endpoint.', 'optimization-detective' ) ), 'actions' => '', 'test' => 'optimization_detective_rest_api', @@ -44,10 +44,10 @@ function od_optimization_detective_rest_api_test(): array { if ( is_wp_error( $response ) ) { $result['status'] = 'recommended'; - $result['label'] = __( 'Your site encountered error accessing Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['label'] = __( 'Error accessing the REST API endpoint', 'optimization-detective' ); $result['description'] = sprintf( '

%s

', - esc_html__( 'The Optimization Detective endpoint could not be reached. This might mean the REST API is disabled or blocked.', 'optimization-detective' ) + esc_html__( 'There was an issue reaching the REST API endpoint. This might be due to server settings or the REST API being disabled.', 'optimization-detective' ) ); $info = array( 'error_message' => $response->get_error_message(), @@ -73,24 +73,24 @@ function od_optimization_detective_rest_api_test(): array { $info['available'] = true; } elseif ( 401 === $status_code ) { $result['status'] = 'recommended'; - $result['label'] = __( 'Your site encountered unauthorized error for Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['label'] = __( 'Authorization should not be required to access the REST API endpoint.', 'optimization-detective' ); $result['description'] = sprintf( '

%s

', - esc_html__( 'The REST API endpoint requires authentication. Ensure proper credentials are provided.', 'optimization-detective' ) + esc_html__( 'To collect URL metrics, the REST API endpoint should be accessible without requiring authorization.', 'optimization-detective' ) ); } elseif ( 403 === $status_code ) { $result['status'] = 'recommended'; - $result['label'] = __( 'Your site encountered forbidden error for Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['label'] = __( 'The REST API endpoint should not be forbidden.', 'optimization-detective' ); $result['description'] = sprintf( '

%s

', - esc_html__( 'The REST API endpoint is blocked check server or security settings.', 'optimization-detective' ) + esc_html__( 'The REST API endpoint is blocked. Please review your server or security settings.', 'optimization-detective' ) ); } else { $result['status'] = 'recommended'; - $result['label'] = __( 'Your site encountered error accessing Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['label'] = __( 'Error accessing the REST API endpoint', 'optimization-detective' ); $result['description'] = sprintf( '

%s

', - esc_html__( 'The Optimization Detective endpoint could not be reached. This might mean the REST API is disabled or blocked.', 'optimization-detective' ) + esc_html__( 'There was an issue reaching the REST API endpoint. This might be due to server settings or the REST API being disabled.', 'optimization-detective' ) ); } $info['error_message'] = $result['label']; From 8895d358c91d31d03bb7a5e66336d010e066e1e8 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Wed, 8 Jan 2025 20:55:31 +0530 Subject: [PATCH 21/63] Refactor site health checks files by combining them into single file --- plugins/optimization-detective/load.php | 2 +- .../rest-api/helper.php => site-health.php} | 24 +++++++++++++- .../site-health/load.php | 15 --------- .../site-health/rest-api/hooks.php | 33 ------------------- 4 files changed, 24 insertions(+), 50 deletions(-) rename plugins/optimization-detective/{site-health/rest-api/helper.php => site-health.php} (85%) delete mode 100644 plugins/optimization-detective/site-health/load.php delete mode 100644 plugins/optimization-detective/site-health/rest-api/hooks.php diff --git a/plugins/optimization-detective/load.php b/plugins/optimization-detective/load.php index be09dfdd75..f536ac3e96 100644 --- a/plugins/optimization-detective/load.php +++ b/plugins/optimization-detective/load.php @@ -141,6 +141,6 @@ class_alias( OD_URL_Metric_Group_Collection::class, 'OD_URL_Metrics_Group_Collec require_once __DIR__ . '/hooks.php'; // Load site health checks. - require_once __DIR__ . '/site-health/load.php'; + require_once __DIR__ . '/site-health.php'; } ); diff --git a/plugins/optimization-detective/site-health/rest-api/helper.php b/plugins/optimization-detective/site-health.php similarity index 85% rename from plugins/optimization-detective/site-health/rest-api/helper.php rename to plugins/optimization-detective/site-health.php index bbda348a9b..a4f5c6d43e 100644 --- a/plugins/optimization-detective/site-health/rest-api/helper.php +++ b/plugins/optimization-detective/site-health.php @@ -1,6 +1,6 @@ } $tests Site Health Tests. + * @return array{direct: array} Amended tests. + */ +function od_optimization_detective_add_rest_api_test( array $tests ): array { + $tests['direct']['optimization_detective_rest_api'] = array( + 'label' => __( 'Optimization Detective REST API Endpoint Availability', 'optimization-detective' ), + 'test' => 'od_optimization_detective_rest_api_test', + ); + + return $tests; +} +add_filter( 'site_status_tests', 'od_optimization_detective_add_rest_api_test' ); + /** * Tests availability of the Optimization Detective REST API endpoint. * @@ -169,3 +187,7 @@ static function ( string $plugin ): void { } ); } + +// Hook for the scheduled REST API health check. +add_action( 'od_rest_api_health_check_event', 'od_run_scheduled_rest_api_health_check' ); +add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); diff --git a/plugins/optimization-detective/site-health/load.php b/plugins/optimization-detective/site-health/load.php deleted file mode 100644 index 65b8b17ee5..0000000000 --- a/plugins/optimization-detective/site-health/load.php +++ /dev/null @@ -1,15 +0,0 @@ -} $tests Site Health Tests. - * @return array{direct: array} Amended tests. - */ -function od_optimization_detective_add_rest_api_test( array $tests ): array { - $tests['direct']['optimization_detective_rest_api'] = array( - 'label' => __( 'Optimization Detective REST API Endpoint Availability', 'optimization-detective' ), - 'test' => 'od_optimization_detective_rest_api_test', - ); - - return $tests; -} -add_filter( 'site_status_tests', 'od_optimization_detective_add_rest_api_test' ); - -// Hook for the scheduled REST API health check. -add_action( 'od_rest_api_health_check_event', 'od_run_scheduled_rest_api_health_check' ); -add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); From 2fbd5301dbdbcd82f0440f7958a9d62334a4c40e Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Wed, 8 Jan 2025 21:06:57 +0530 Subject: [PATCH 22/63] Move added action and filters for site health checks to plugins's hooks.php --- plugins/optimization-detective/hooks.php | 3 +++ plugins/optimization-detective/site-health.php | 5 ----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/plugins/optimization-detective/hooks.php b/plugins/optimization-detective/hooks.php index c0f94d148c..13b1d390bd 100644 --- a/plugins/optimization-detective/hooks.php +++ b/plugins/optimization-detective/hooks.php @@ -15,3 +15,6 @@ OD_URL_Metrics_Post_Type::add_hooks(); add_action( 'wp', 'od_maybe_add_template_output_buffer_filter' ); add_action( 'wp_head', 'od_render_generator_meta_tag' ); +add_filter( 'site_status_tests', 'od_optimization_detective_add_rest_api_test' ); +add_action( 'od_rest_api_health_check_event', 'od_run_scheduled_rest_api_health_check' ); +add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index a4f5c6d43e..fa9b49ad57 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -26,7 +26,6 @@ function od_optimization_detective_add_rest_api_test( array $tests ): array { return $tests; } -add_filter( 'site_status_tests', 'od_optimization_detective_add_rest_api_test' ); /** * Tests availability of the Optimization Detective REST API endpoint. @@ -187,7 +186,3 @@ static function ( string $plugin ): void { } ); } - -// Hook for the scheduled REST API health check. -add_action( 'od_rest_api_health_check_event', 'od_run_scheduled_rest_api_health_check' ); -add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); From c095436dd95bcadd2f66580cabe73d3bdfa99483 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Mon, 13 Jan 2025 14:11:10 +0530 Subject: [PATCH 23/63] Remove scheduled REST API health check functions and related action hooks --- plugins/optimization-detective/hooks.php | 1 - .../optimization-detective/site-health.php | 21 ------------------- 2 files changed, 22 deletions(-) diff --git a/plugins/optimization-detective/hooks.php b/plugins/optimization-detective/hooks.php index 13b1d390bd..5be25a27f0 100644 --- a/plugins/optimization-detective/hooks.php +++ b/plugins/optimization-detective/hooks.php @@ -16,5 +16,4 @@ add_action( 'wp', 'od_maybe_add_template_output_buffer_filter' ); add_action( 'wp_head', 'od_render_generator_meta_tag' ); add_filter( 'site_status_tests', 'od_optimization_detective_add_rest_api_test' ); -add_action( 'od_rest_api_health_check_event', 'od_run_scheduled_rest_api_health_check' ); add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index fa9b49ad57..7619f84ba2 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -117,26 +117,6 @@ function od_optimization_detective_rest_api_test(): array { return $result; } -/** - * Periodically runs the Optimization Detective REST API health check. - * - * @since n.e.x.t - */ -function od_schedule_rest_api_health_check(): void { - if ( ! (bool) wp_next_scheduled( 'od_rest_api_health_check_event' ) ) { - wp_schedule_event( time(), 'weekly', 'od_rest_api_health_check_event' ); - } -} - -/** - * Hook for the scheduled REST API health check. - * - * @since n.e.x.t - */ -function od_run_scheduled_rest_api_health_check(): void { - od_optimization_detective_rest_api_test(); -} - /** * Displays an admin notice if the REST API health check fails. * @@ -175,7 +155,6 @@ function od_rest_api_health_check_plugin_activation(): void { if ( ! (bool) get_option( 'od_rest_api_info' ) ) { add_option( 'od_rest_api_info', array() ); } - od_schedule_rest_api_health_check(); // Run the check immediately after Optimization Detective is activated. add_action( 'activated_plugin', From ebcd804e8a480b2d6806c4ca12d70ad0e9cf282f Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Mon, 13 Jan 2025 16:07:18 +0530 Subject: [PATCH 24/63] Refactor to use admin_init instead of plugin activation hook, On plugin activation show error notice at top of plugins page --- plugins/optimization-detective/hooks.php | 1 + plugins/optimization-detective/load.php | 12 --- .../optimization-detective/site-health.php | 73 ++++++++++--------- 3 files changed, 39 insertions(+), 47 deletions(-) diff --git a/plugins/optimization-detective/hooks.php b/plugins/optimization-detective/hooks.php index 5be25a27f0..2ec71a1097 100644 --- a/plugins/optimization-detective/hooks.php +++ b/plugins/optimization-detective/hooks.php @@ -16,4 +16,5 @@ add_action( 'wp', 'od_maybe_add_template_output_buffer_filter' ); add_action( 'wp_head', 'od_render_generator_meta_tag' ); add_filter( 'site_status_tests', 'od_optimization_detective_add_rest_api_test' ); +add_action( 'admin_init', 'od_rest_api_health_check_plugin_activation' ); add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); diff --git a/plugins/optimization-detective/load.php b/plugins/optimization-detective/load.php index f536ac3e96..6253bb388e 100644 --- a/plugins/optimization-detective/load.php +++ b/plugins/optimization-detective/load.php @@ -51,18 +51,6 @@ static function ( string $global_var_name, string $version, Closure $load ): voi * action), so this is why it gets initialized at priority 9. */ add_action( 'init', $bootstrap, 9 ); - - register_activation_hook( - __FILE__, - static function () use ( $bootstrap ): void { - /* - * The activation hook is called before the init action, so the plugin is not loaded yet. This - * means that the plugin must be bootstrapped here to run the activation logic. - */ - $bootstrap(); - od_rest_api_health_check_plugin_activation(); - } - ); } // Register this copy of the plugin. diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 7619f84ba2..20511440ac 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -62,12 +62,9 @@ function od_optimization_detective_rest_api_test(): array { if ( is_wp_error( $response ) ) { $result['status'] = 'recommended'; $result['label'] = __( 'Error accessing the REST API endpoint', 'optimization-detective' ); - $result['description'] = sprintf( - '

%s

', - esc_html__( 'There was an issue reaching the REST API endpoint. This might be due to server settings or the REST API being disabled.', 'optimization-detective' ) - ); + $result['description'] = esc_html__( 'There was an issue reaching the REST API endpoint. This might be due to server settings or the REST API being disabled.', 'optimization-detective' ); $info = array( - 'error_message' => $response->get_error_message(), + 'error_message' => $result['description'], 'error_code' => $response->get_error_code(), 'available' => false, ); @@ -91,28 +88,24 @@ function od_optimization_detective_rest_api_test(): array { } elseif ( 401 === $status_code ) { $result['status'] = 'recommended'; $result['label'] = __( 'Authorization should not be required to access the REST API endpoint.', 'optimization-detective' ); - $result['description'] = sprintf( - '

%s

', - esc_html__( 'To collect URL metrics, the REST API endpoint should be accessible without requiring authorization.', 'optimization-detective' ) - ); + $result['description'] = esc_html__( 'To collect URL metrics, the REST API endpoint should be accessible without requiring authorization.', 'optimization-detective' ); } elseif ( 403 === $status_code ) { $result['status'] = 'recommended'; $result['label'] = __( 'The REST API endpoint should not be forbidden.', 'optimization-detective' ); - $result['description'] = sprintf( - '

%s

', - esc_html__( 'The REST API endpoint is blocked. Please review your server or security settings.', 'optimization-detective' ) - ); + $result['description'] = esc_html__( 'The REST API endpoint is blocked. Please review your server or security settings.', 'optimization-detective' ); } else { $result['status'] = 'recommended'; $result['label'] = __( 'Error accessing the REST API endpoint', 'optimization-detective' ); - $result['description'] = sprintf( - '

%s

', - esc_html__( 'There was an issue reaching the REST API endpoint. This might be due to server settings or the REST API being disabled.', 'optimization-detective' ) - ); + $result['description'] = esc_html__( 'There was an issue reaching the REST API endpoint. This might be due to server settings or the REST API being disabled.', 'optimization-detective' ); } - $info['error_message'] = $result['label']; + $info['error_message'] = $result['description']; } + $result['description'] = sprintf( + '

%s

', + $result['description'] + ); + update_option( 'od_rest_api_info', $info ); return $result; } @@ -129,14 +122,14 @@ function od_rest_api_health_check_admin_notice( string $plugin_file ): void { return; } - $od_rest_api_info = get_option( 'od_rest_api_info', array() ); + $rest_api_info = get_option( 'od_rest_api_info', array() ); if ( - isset( $od_rest_api_info['available'] ) && - ! (bool) $od_rest_api_info['available'] && - isset( $od_rest_api_info['error_message'] ) + isset( $rest_api_info['available'] ) && + false === $rest_api_info['available'] && + isset( $rest_api_info['error_message'] ) ) { wp_admin_notice( - esc_html( $od_rest_api_info['error_message'] ), + esc_html( $rest_api_info['error_message'] ), array( 'type' => 'warning', 'additional_classes' => array( 'inline', 'notice-alt' ), @@ -151,17 +144,27 @@ function od_rest_api_health_check_admin_notice( string $plugin_file ): void { * @since n.e.x.t */ function od_rest_api_health_check_plugin_activation(): void { - // Add the option if it doesn't exist. - if ( ! (bool) get_option( 'od_rest_api_info' ) ) { - add_option( 'od_rest_api_info', array() ); + // If the option already exists, do nothing. + if ( false !== get_option( 'od_rest_api_info' ) ) { + return; + } + + // This will populate the od_rest_api_info option so that the function won't execute on the next page load. + od_optimization_detective_rest_api_test(); + + // Get the updated option. + $rest_api_info = get_option( 'od_rest_api_info', array() ); + if ( + isset( $rest_api_info['available'] ) && + false === $rest_api_info['available'] && + isset( $rest_api_info['error_message'] ) + ) { + wp_admin_notice( + esc_html( $rest_api_info['error_message'] ), + array( + 'type' => 'warning', + 'additional_classes' => array( 'notice-alt' ), + ) + ); } - // Run the check immediately after Optimization Detective is activated. - add_action( - 'activated_plugin', - static function ( string $plugin ): void { - if ( 'optimization-detective/load.php' === $plugin ) { - od_optimization_detective_rest_api_test(); - } - } - ); } From 57f7566ba0517521b572a3aac0f61764648ae856 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Mon, 13 Jan 2025 16:34:17 +0530 Subject: [PATCH 25/63] Show global admin notice on plugin activation and meta row notice on subsequent admin pages --- plugins/optimization-detective/hooks.php | 1 - plugins/optimization-detective/site-health.php | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/optimization-detective/hooks.php b/plugins/optimization-detective/hooks.php index 2ec71a1097..5ef7156527 100644 --- a/plugins/optimization-detective/hooks.php +++ b/plugins/optimization-detective/hooks.php @@ -17,4 +17,3 @@ add_action( 'wp_head', 'od_render_generator_meta_tag' ); add_filter( 'site_status_tests', 'od_optimization_detective_add_rest_api_test' ); add_action( 'admin_init', 'od_rest_api_health_check_plugin_activation' ); -add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 20511440ac..cb62f5036b 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -146,6 +146,7 @@ function od_rest_api_health_check_admin_notice( string $plugin_file ): void { function od_rest_api_health_check_plugin_activation(): void { // If the option already exists, do nothing. if ( false !== get_option( 'od_rest_api_info' ) ) { + add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); return; } From 226584fb554349c00503e8c0b91322e52d212b67 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Tue, 14 Jan 2025 22:06:21 +0530 Subject: [PATCH 26/63] Update REST API endpoint messages to include 'Optimization Detective' for clarity --- .../optimization-detective/site-health.php | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index cb62f5036b..8f026db29b 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -36,7 +36,7 @@ function od_optimization_detective_add_rest_api_test( array $tests ): array { */ function od_optimization_detective_rest_api_test(): array { $result = array( - 'label' => __( 'The REST API endpoint is functional.', 'optimization-detective' ), + 'label' => __( 'The Optimization Detective REST API endpoint is functional.', 'optimization-detective' ), 'status' => 'good', 'badge' => array( 'label' => __( 'Optimization Detective', 'optimization-detective' ), @@ -44,7 +44,7 @@ function od_optimization_detective_rest_api_test(): array { ), 'description' => sprintf( '

%s

', - __( 'Your site can send and receive URL metrics via the REST API endpoint.', 'optimization-detective' ) + __( 'Your site can send and receive URL metrics via the Optimization Detective REST API endpoint.', 'optimization-detective' ) ), 'actions' => '', 'test' => 'optimization_detective_rest_api', @@ -61,8 +61,8 @@ function od_optimization_detective_rest_api_test(): array { if ( is_wp_error( $response ) ) { $result['status'] = 'recommended'; - $result['label'] = __( 'Error accessing the REST API endpoint', 'optimization-detective' ); - $result['description'] = esc_html__( 'There was an issue reaching the REST API endpoint. This might be due to server settings or the REST API being disabled.', 'optimization-detective' ); + $result['label'] = __( 'Error accessing the Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['description'] = esc_html__( 'There was an issue reaching the Optimization Detective REST API endpoint. This might be due to server settings or the REST API being disabled.', 'optimization-detective' ); $info = array( 'error_message' => $result['description'], 'error_code' => $response->get_error_code(), @@ -87,16 +87,16 @@ function od_optimization_detective_rest_api_test(): array { $info['available'] = true; } elseif ( 401 === $status_code ) { $result['status'] = 'recommended'; - $result['label'] = __( 'Authorization should not be required to access the REST API endpoint.', 'optimization-detective' ); - $result['description'] = esc_html__( 'To collect URL metrics, the REST API endpoint should be accessible without requiring authorization.', 'optimization-detective' ); + $result['label'] = __( 'Authorization should not be required to access the Optimization Detective REST API endpoint.', 'optimization-detective' ); + $result['description'] = esc_html__( 'To collect URL metrics, the Optimization Detective REST API endpoint should be accessible without requiring authorization.', 'optimization-detective' ); } elseif ( 403 === $status_code ) { $result['status'] = 'recommended'; - $result['label'] = __( 'The REST API endpoint should not be forbidden.', 'optimization-detective' ); - $result['description'] = esc_html__( 'The REST API endpoint is blocked. Please review your server or security settings.', 'optimization-detective' ); + $result['label'] = __( 'The Optimization Detective REST API endpoint should not be forbidden.', 'optimization-detective' ); + $result['description'] = esc_html__( 'The Optimization Detective REST API endpoint is blocked. Please review your server or security settings.', 'optimization-detective' ); } else { $result['status'] = 'recommended'; - $result['label'] = __( 'Error accessing the REST API endpoint', 'optimization-detective' ); - $result['description'] = esc_html__( 'There was an issue reaching the REST API endpoint. This might be due to server settings or the REST API being disabled.', 'optimization-detective' ); + $result['label'] = __( 'Error accessing the Optimization Detective REST API endpoint', 'optimization-detective' ); + $result['description'] = esc_html__( 'There was an issue reaching the Optimization Detective REST API endpoint. This might be due to server settings or the REST API being disabled.', 'optimization-detective' ); } $info['error_message'] = $result['description']; } From 1855ecb595bea77d2a012ae27da34345c0afae44 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Tue, 14 Jan 2025 22:30:05 +0530 Subject: [PATCH 27/63] Make params check simple, add default empty error_message, add check for empty string --- .../optimization-detective/site-health.php | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 8f026db29b..291d56d10a 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -69,19 +69,19 @@ function od_optimization_detective_rest_api_test(): array { 'available' => false, ); } else { - $status_code = wp_remote_retrieve_response_code( $response ); - $data = json_decode( wp_remote_retrieve_body( $response ), true ); - $expected_params = array( 'slug', 'current_etag', 'hmac', 'url', 'viewport', 'elements' ); - $info = array( - 'status_code' => $status_code, - 'available' => false, + $status_code = wp_remote_retrieve_response_code( $response ); + $data = json_decode( wp_remote_retrieve_body( $response ), true ); + $info = array( + 'status_code' => $status_code, + 'available' => false, + 'error_message' => '', ); if ( - 400 === $status_code - && isset( $data['data']['params'] ) - && is_array( $data['data']['params'] ) - && count( $expected_params ) === count( array_intersect( $data['data']['params'], $expected_params ) ) + 400 === $status_code && + isset( $data['data']['params'] ) && + is_array( $data['data']['params'] ) && + count( $data['data']['params'] ) > 0 ) { // The REST API endpoint is available. $info['available'] = true; @@ -126,7 +126,8 @@ function od_rest_api_health_check_admin_notice( string $plugin_file ): void { if ( isset( $rest_api_info['available'] ) && false === $rest_api_info['available'] && - isset( $rest_api_info['error_message'] ) + isset( $rest_api_info['error_message'] ) && + '' !== $rest_api_info['error_message'] ) { wp_admin_notice( esc_html( $rest_api_info['error_message'] ), @@ -158,7 +159,8 @@ function od_rest_api_health_check_plugin_activation(): void { if ( isset( $rest_api_info['available'] ) && false === $rest_api_info['available'] && - isset( $rest_api_info['error_message'] ) + isset( $rest_api_info['error_message'] ) && + '' !== $rest_api_info['error_message'] ) { wp_admin_notice( esc_html( $rest_api_info['error_message'] ), From 0ad417a0f17bbfc0542f374bb28019ee7a33dc42 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Tue, 14 Jan 2025 23:24:31 +0530 Subject: [PATCH 28/63] Improve REST API health check notices logic and message clarity --- .../optimization-detective/site-health.php | 50 ++++++++----------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 291d56d10a..abfb520699 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -91,8 +91,8 @@ function od_optimization_detective_rest_api_test(): array { $result['description'] = esc_html__( 'To collect URL metrics, the Optimization Detective REST API endpoint should be accessible without requiring authorization.', 'optimization-detective' ); } elseif ( 403 === $status_code ) { $result['status'] = 'recommended'; - $result['label'] = __( 'The Optimization Detective REST API endpoint should not be forbidden.', 'optimization-detective' ); - $result['description'] = esc_html__( 'The Optimization Detective REST API endpoint is blocked. Please review your server or security settings.', 'optimization-detective' ); + $result['label'] = __( 'Access to the Optimization Detective REST API endpoint was denied.', 'optimization-detective' ); + $result['description'] = esc_html__( 'Access was denied because the user does not have the necessary capabilities. Please check user roles and capabilities, as all users should have access to the Optimization Detective REST API endpoint.', 'optimization-detective' ); } else { $result['status'] = 'recommended'; $result['label'] = __( 'Error accessing the Optimization Detective REST API endpoint', 'optimization-detective' ); @@ -111,17 +111,13 @@ function od_optimization_detective_rest_api_test(): array { } /** - * Displays an admin notice if the REST API health check fails. + * Renders an admin notice if the REST API health check fails. * * @since n.e.x.t * - * @param string $plugin_file Plugin file. + * @param array $additional_classes Additional classes to add to the notice. */ -function od_rest_api_health_check_admin_notice( string $plugin_file ): void { - if ( 'optimization-detective/load.php' !== $plugin_file ) { - return; - } - +function od_render_rest_api_health_check_notice( array $additional_classes ): void { $rest_api_info = get_option( 'od_rest_api_info', array() ); if ( isset( $rest_api_info['available'] ) && @@ -133,19 +129,33 @@ function od_rest_api_health_check_admin_notice( string $plugin_file ): void { esc_html( $rest_api_info['error_message'] ), array( 'type' => 'warning', - 'additional_classes' => array( 'inline', 'notice-alt' ), + 'additional_classes' => $additional_classes, ) ); } } +/** + * Displays an admin notice on the plugin row if the REST API health check fails. + * + * @since n.e.x.t + * + * @param string $plugin_file Plugin file. + */ +function od_rest_api_health_check_admin_notice( string $plugin_file ): void { + if ( 'optimization-detective/load.php' !== $plugin_file ) { + return; + } + od_render_rest_api_health_check_notice( array( 'inline', 'notice-alt' ) ); +} + /** * Plugin activation hook for the REST API health check. * * @since n.e.x.t */ function od_rest_api_health_check_plugin_activation(): void { - // If the option already exists, do nothing. + // If the option already exists, do nothing except attach our plugin-row notice. if ( false !== get_option( 'od_rest_api_info' ) ) { add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); return; @@ -153,21 +163,5 @@ function od_rest_api_health_check_plugin_activation(): void { // This will populate the od_rest_api_info option so that the function won't execute on the next page load. od_optimization_detective_rest_api_test(); - - // Get the updated option. - $rest_api_info = get_option( 'od_rest_api_info', array() ); - if ( - isset( $rest_api_info['available'] ) && - false === $rest_api_info['available'] && - isset( $rest_api_info['error_message'] ) && - '' !== $rest_api_info['error_message'] - ) { - wp_admin_notice( - esc_html( $rest_api_info['error_message'] ), - array( - 'type' => 'warning', - 'additional_classes' => array( 'notice-alt' ), - ) - ); - } + od_render_rest_api_health_check_notice( array( 'notice-alt' ) ); } From 7ad36cd94b72a3ad27a2714cd93575f19023cdb6 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 14 Jan 2025 17:09:34 -0800 Subject: [PATCH 29/63] Print REST API health check notice at admin_notices action without notice-alt class --- plugins/optimization-detective/hooks.php | 3 ++- .../optimization-detective/site-health.php | 25 ++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/plugins/optimization-detective/hooks.php b/plugins/optimization-detective/hooks.php index 5ef7156527..2d20e853f1 100644 --- a/plugins/optimization-detective/hooks.php +++ b/plugins/optimization-detective/hooks.php @@ -16,4 +16,5 @@ add_action( 'wp', 'od_maybe_add_template_output_buffer_filter' ); add_action( 'wp_head', 'od_render_generator_meta_tag' ); add_filter( 'site_status_tests', 'od_optimization_detective_add_rest_api_test' ); -add_action( 'admin_init', 'od_rest_api_health_check_plugin_activation' ); +add_action( 'admin_init', 'od_maybe_run_rest_api_health_check' ); +add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index abfb520699..2f8e2d8986 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -117,7 +117,7 @@ function od_optimization_detective_rest_api_test(): array { * * @param array $additional_classes Additional classes to add to the notice. */ -function od_render_rest_api_health_check_notice( array $additional_classes ): void { +function od_render_rest_api_health_check_notice( array $additional_classes = array() ): void { $rest_api_info = get_option( 'od_rest_api_info', array() ); if ( isset( $rest_api_info['available'] ) && @@ -150,18 +150,31 @@ function od_rest_api_health_check_admin_notice( string $plugin_file ): void { } /** - * Plugin activation hook for the REST API health check. + * Runs the REST API health check if it hasn't been run yet. + * + * This happens at the `admin_init` action to avoid running the check on the frontend. This will run on the first admin + * page load after the plugin has been activated. This allows for this function to add an action at `admin_notices` so + * that an error message can be displayed after performing that plugin activation request. Note that a plugin activation + * hook cannot be used for this purpose due to not being compatible with multisite. While the site health notice is + * shown at the `admin_notices` action once, the notice will only be displayed inline with the plugin row thereafter + * via {@see od_rest_api_health_check_admin_notice()}. * * @since n.e.x.t */ -function od_rest_api_health_check_plugin_activation(): void { - // If the option already exists, do nothing except attach our plugin-row notice. +function od_maybe_run_rest_api_health_check(): void { + // If the option already exists, then the REST API health check has already been performed. if ( false !== get_option( 'od_rest_api_info' ) ) { - add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); return; } // This will populate the od_rest_api_info option so that the function won't execute on the next page load. od_optimization_detective_rest_api_test(); - od_render_rest_api_health_check_notice( array( 'notice-alt' ) ); + + // Show any notice in the main admin notices area for the first page load (e.g. after plugin activation). + add_action( + 'admin_notices', + static function (): void { + od_render_rest_api_health_check_notice(); + } + ); } From d9577451acfd12db5bfd37544412f55351f4e431 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 14 Jan 2025 18:06:11 -0800 Subject: [PATCH 30/63] Refactor construction of site health strings --- .../optimization-detective/site-health.php | 79 +++++++++++-------- 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 2f8e2d8986..f2d967014b 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -35,17 +35,23 @@ function od_optimization_detective_add_rest_api_test( array $tests ): array { * @return array{label: string, status: string, badge: array{label: string, color: string}, description: string, actions: string, test: string} Result. */ function od_optimization_detective_rest_api_test(): array { + $common_description_html = '

' . wp_kses( + sprintf( + /* translators: %s is the REST API endpoint */ + __( 'To collect URL Metrics from visitors the REST API must be accessible to unauthenticated users. Specifically, visitors must be able to perform a POST request to the %s endpoint.', 'optimization-detective' ), + '/' . OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE + ), + array( 'code' => array() ) + ) . '

'; + $result = array( - 'label' => __( 'The Optimization Detective REST API endpoint is functional.', 'optimization-detective' ), + 'label' => __( 'Optimization Detective\'s REST API endpoint is accessible', 'optimization-detective' ), 'status' => 'good', 'badge' => array( 'label' => __( 'Optimization Detective', 'optimization-detective' ), 'color' => 'blue', ), - 'description' => sprintf( - '

%s

', - __( 'Your site can send and receive URL metrics via the Optimization Detective REST API endpoint.', 'optimization-detective' ) - ), + 'description' => $common_description_html . '

' . esc_html__( 'This appears to be working properly.', 'optimization-detective' ) . '

', 'actions' => '', 'test' => 'optimization_detective_rest_api', ); @@ -59,11 +65,23 @@ function od_optimization_detective_rest_api_test(): array { ) ); + $error_label = __( 'Optimization Detective\'s REST API endpoint is inaccessible', 'optimization-detective' ); + $error_description_html = '

' . esc_html__( 'You may have a plugin active or server configuration which restricts access to logged-in users. Unauthenticated access must be restored in order for Optimization Detective to work.', 'optimization-detective' ) . '

'; + if ( is_wp_error( $response ) ) { $result['status'] = 'recommended'; - $result['label'] = __( 'Error accessing the Optimization Detective REST API endpoint', 'optimization-detective' ); - $result['description'] = esc_html__( 'There was an issue reaching the Optimization Detective REST API endpoint. This might be due to server settings or the REST API being disabled.', 'optimization-detective' ); - $info = array( + $result['label'] = $error_label; + $result['description'] = $common_description_html . $error_description_html . '

' . wp_kses( + sprintf( + /* translators: 1: the error code, 2: the error message */ + __( 'The REST API responded with the error code %1$s and this error message: %2$s.', 'optimization-detective' ), + esc_html( (string) $response->get_error_code() ), + esc_html( rtrim( $response->get_error_message(), '.' ) ) + ), + array( 'code' => array() ) + ) . '

'; + + $info = array( 'error_message' => $result['description'], 'error_code' => $response->get_error_code(), 'available' => false, @@ -71,11 +89,6 @@ function od_optimization_detective_rest_api_test(): array { } else { $status_code = wp_remote_retrieve_response_code( $response ); $data = json_decode( wp_remote_retrieve_body( $response ), true ); - $info = array( - 'status_code' => $status_code, - 'available' => false, - 'error_message' => '', - ); if ( 400 === $status_code && @@ -84,28 +97,32 @@ function od_optimization_detective_rest_api_test(): array { count( $data['data']['params'] ) > 0 ) { // The REST API endpoint is available. - $info['available'] = true; - } elseif ( 401 === $status_code ) { - $result['status'] = 'recommended'; - $result['label'] = __( 'Authorization should not be required to access the Optimization Detective REST API endpoint.', 'optimization-detective' ); - $result['description'] = esc_html__( 'To collect URL metrics, the Optimization Detective REST API endpoint should be accessible without requiring authorization.', 'optimization-detective' ); - } elseif ( 403 === $status_code ) { - $result['status'] = 'recommended'; - $result['label'] = __( 'Access to the Optimization Detective REST API endpoint was denied.', 'optimization-detective' ); - $result['description'] = esc_html__( 'Access was denied because the user does not have the necessary capabilities. Please check user roles and capabilities, as all users should have access to the Optimization Detective REST API endpoint.', 'optimization-detective' ); + $info = array( + 'status_code' => $status_code, + 'available' => true, + 'error_message' => '', + ); } else { $result['status'] = 'recommended'; - $result['label'] = __( 'Error accessing the Optimization Detective REST API endpoint', 'optimization-detective' ); - $result['description'] = esc_html__( 'There was an issue reaching the Optimization Detective REST API endpoint. This might be due to server settings or the REST API being disabled.', 'optimization-detective' ); + $result['label'] = __( 'The Optimization Detective REST API endpoint is inaccessible to logged-out users', 'optimization-detective' ); + $result['description'] = $common_description_html . $error_description_html . '

' . wp_kses( + sprintf( + /* translators: %d is the HTTP status code, %s is the status header description */ + __( 'The REST API returned with an HTTP status of %1$d %2$s.', 'optimization-detective' ), + $status_code, + get_status_header_desc( (int) $status_code ) + ), + array( 'code' => array() ) + ) . '

'; + + $info = array( + 'status_code' => $status_code, + 'available' => false, + 'error_message' => $result['description'], + ); } - $info['error_message'] = $result['description']; } - $result['description'] = sprintf( - '

%s

', - $result['description'] - ); - update_option( 'od_rest_api_info', $info ); return $result; } @@ -126,7 +143,7 @@ function od_render_rest_api_health_check_notice( array $additional_classes = arr '' !== $rest_api_info['error_message'] ) { wp_admin_notice( - esc_html( $rest_api_info['error_message'] ), + wp_kses( $rest_api_info['error_message'], array_fill_keys( array( 'p', 'code' ), array() ) ), array( 'type' => 'warning', 'additional_classes' => $additional_classes, From 4c1b2dc35e1d64951f2bb33b35cda7ee971a8cab Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 14 Jan 2025 18:28:45 -0800 Subject: [PATCH 31/63] Include Site Health information in admin notices --- .../optimization-detective/site-health.php | 85 +++++++++++-------- ...n-detective-rest-api-site-health-check.php | 12 +-- 2 files changed, 55 insertions(+), 42 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index f2d967014b..dc3509f2e0 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -14,6 +14,7 @@ * Adds the Optimization Detective REST API check to site health tests. * * @since n.e.x.t + * @access private * * @param array{direct: array} $tests Site Health Tests. * @return array{direct: array} Amended tests. @@ -31,10 +32,40 @@ function od_optimization_detective_add_rest_api_test( array $tests ): array { * Tests availability of the Optimization Detective REST API endpoint. * * @since n.e.x.t + * @access private * * @return array{label: string, status: string, badge: array{label: string, color: string}, description: string, actions: string, test: string} Result. */ function od_optimization_detective_rest_api_test(): array { + $rest_url = get_rest_url( null, OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ); + $response = wp_remote_post( + $rest_url, + array( + 'headers' => array( 'Content-Type' => 'application/json' ), + 'sslverify' => false, + ) + ); + $result = od_construct_site_health_result( $response ); + $info = array( + 'available' => 'good' === $result['status'], + 'response' => $response, + ); + + update_option( 'od_rest_api_info', $info ); + return $result; +} + +/** + * Tests availability of the Optimization Detective REST API endpoint. + * + * @since n.e.x.t + * @access private + * + * @param array|WP_Error $response REST API response. + * + * @return array{label: string, status: string, badge: array{label: string, color: string}, description: string, actions: string, test: string} Result. + */ +function od_construct_site_health_result( $response ): array { $common_description_html = '

' . wp_kses( sprintf( /* translators: %s is the REST API endpoint */ @@ -56,15 +87,6 @@ function od_optimization_detective_rest_api_test(): array { 'test' => 'optimization_detective_rest_api', ); - $rest_url = get_rest_url( null, OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ); - $response = wp_remote_post( - $rest_url, - array( - 'headers' => array( 'Content-Type' => 'application/json' ), - 'sslverify' => false, - ) - ); - $error_label = __( 'Optimization Detective\'s REST API endpoint is inaccessible', 'optimization-detective' ); $error_description_html = '

' . esc_html__( 'You may have a plugin active or server configuration which restricts access to logged-in users. Unauthenticated access must be restored in order for Optimization Detective to work.', 'optimization-detective' ) . '

'; @@ -80,29 +102,17 @@ function od_optimization_detective_rest_api_test(): array { ), array( 'code' => array() ) ) . '

'; - - $info = array( - 'error_message' => $result['description'], - 'error_code' => $response->get_error_code(), - 'available' => false, - ); } else { $status_code = wp_remote_retrieve_response_code( $response ); $data = json_decode( wp_remote_retrieve_body( $response ), true ); - if ( + $is_expected = ( 400 === $status_code && isset( $data['data']['params'] ) && is_array( $data['data']['params'] ) && count( $data['data']['params'] ) > 0 - ) { - // The REST API endpoint is available. - $info = array( - 'status_code' => $status_code, - 'available' => true, - 'error_message' => '', - ); - } else { + ); + if ( ! $is_expected ) { $result['status'] = 'recommended'; $result['label'] = __( 'The Optimization Detective REST API endpoint is inaccessible to logged-out users', 'optimization-detective' ); $result['description'] = $common_description_html . $error_description_html . '

' . wp_kses( @@ -114,16 +124,8 @@ function od_optimization_detective_rest_api_test(): array { ), array( 'code' => array() ) ) . '

'; - - $info = array( - 'status_code' => $status_code, - 'available' => false, - 'error_message' => $result['description'], - ); } } - - update_option( 'od_rest_api_info', $info ); return $result; } @@ -131,22 +133,31 @@ function od_optimization_detective_rest_api_test(): array { * Renders an admin notice if the REST API health check fails. * * @since n.e.x.t + * @access private * * @param array $additional_classes Additional classes to add to the notice. */ function od_render_rest_api_health_check_notice( array $additional_classes = array() ): void { $rest_api_info = get_option( 'od_rest_api_info', array() ); if ( - isset( $rest_api_info['available'] ) && + isset( $rest_api_info['available'], $rest_api_info['response'] ) && false === $rest_api_info['available'] && - isset( $rest_api_info['error_message'] ) && - '' !== $rest_api_info['error_message'] + ( is_array( $rest_api_info['response'] ) || is_wp_error( $rest_api_info['response'] ) ) ) { + $result = od_construct_site_health_result( $rest_api_info['response'] ); + wp_admin_notice( - wp_kses( $rest_api_info['error_message'], array_fill_keys( array( 'p', 'code' ), array() ) ), + sprintf( + '
%s %s%s %s
', + esc_html__( 'Warning:', 'optimization-detective' ), + esc_html( $result['label'] ), + wp_kses( $result['description'], array_fill_keys( array( 'p', 'code' ), array() ) ), + '

' . esc_html__( 'Please visit Site Health to re-check this once you believe you have resolved the issue.', 'optimization-detective' ) . '

' + ), array( 'type' => 'warning', 'additional_classes' => $additional_classes, + 'paragraph_wrap' => false, ) ); } @@ -156,6 +167,7 @@ function od_render_rest_api_health_check_notice( array $additional_classes = arr * Displays an admin notice on the plugin row if the REST API health check fails. * * @since n.e.x.t + * @access private * * @param string $plugin_file Plugin file. */ @@ -177,6 +189,7 @@ function od_rest_api_health_check_admin_notice( string $plugin_file ): void { * via {@see od_rest_api_health_check_admin_notice()}. * * @since n.e.x.t + * @access private */ function od_maybe_run_rest_api_health_check(): void { // If the option already exists, then the REST API health check has already been performed. diff --git a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php index cc5adeed5d..f430412b59 100644 --- a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php +++ b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php @@ -47,8 +47,8 @@ public function test_rest_api_available(): void { $od_rest_api_info = get_option( 'od_rest_api_info', array() ); $this->assertSame( 'good', $result['status'] ); - $this->assertSame( 400, isset( $od_rest_api_info['status_code'] ) ? $od_rest_api_info['status_code'] : '' ); - $this->assertTrue( isset( $od_rest_api_info['available'] ) ? $od_rest_api_info['available'] : false ); + $this->assertSame( 400, wp_remote_retrieve_response_code( $od_rest_api_info['response'] ) ); + $this->assertTrue( $od_rest_api_info['available'] ); } /** @@ -66,8 +66,8 @@ public function test_rest_api_unauthorized(): void { $od_rest_api_info = get_option( 'od_rest_api_info', array() ); $this->assertSame( 'recommended', $result['status'] ); - $this->assertSame( 401, isset( $od_rest_api_info['status_code'] ) ? $od_rest_api_info['status_code'] : '' ); - $this->assertFalse( isset( $od_rest_api_info['available'] ) ? $od_rest_api_info['available'] : true ); + $this->assertSame( 401, wp_remote_retrieve_response_code( $od_rest_api_info['response'] ) ); + $this->assertFalse( $od_rest_api_info['available'] ); } /** @@ -85,8 +85,8 @@ public function test_rest_api_forbidden(): void { $od_rest_api_info = get_option( 'od_rest_api_info', array() ); $this->assertSame( 'recommended', $result['status'] ); - $this->assertSame( 403, isset( $od_rest_api_info['status_code'] ) ? $od_rest_api_info['status_code'] : '' ); - $this->assertFalse( isset( $od_rest_api_info['available'] ) ? $od_rest_api_info['available'] : true ); + $this->assertSame( 403, wp_remote_retrieve_response_code( $od_rest_api_info['response'] ) ); + $this->assertFalse( $od_rest_api_info['available'] ); } /** From 7f76d319baf69957955d864cd7fa558bb07eb60e Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 14 Jan 2025 18:29:43 -0800 Subject: [PATCH 32/63] Add covers annotations --- ...-optimization-detective-rest-api-site-health-check.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php index f430412b59..254d262c3d 100644 --- a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php +++ b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php @@ -29,6 +29,8 @@ public function setUp(): void { /** * Test that the site health check is `good` when the REST API is available. + * + * @covers ::od_optimization_detective_rest_api_test */ public function test_rest_api_available(): void { $this->mocked_responses = array( @@ -53,6 +55,8 @@ public function test_rest_api_available(): void { /** * Test behavior when REST API returns an unauthorized error. + * + * @covers ::od_optimization_detective_rest_api_test */ public function test_rest_api_unauthorized(): void { $this->mocked_responses = array( @@ -71,7 +75,9 @@ public function test_rest_api_unauthorized(): void { } /** - * Test behavior when REST API returns an forbidden error. + * Test behavior when REST API returns a forbidden error. + * + * @covers ::od_optimization_detective_rest_api_test */ public function test_rest_api_forbidden(): void { $this->mocked_responses = array( From a0cce2120c357fd1a1176b9ce54ca988cda8ec40 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 11:01:35 -0800 Subject: [PATCH 33/63] Only store inaccessible state in option and put response in transient --- .../optimization-detective/site-health.php | 133 +++++++++++++----- ...n-detective-rest-api-site-health-check.php | 42 ++++-- plugins/optimization-detective/uninstall.php | 3 +- 3 files changed, 125 insertions(+), 53 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index dc3509f2e0..9ab931e472 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -15,6 +15,7 @@ * * @since n.e.x.t * @access private + * @todo Add coverage. * * @param array{direct: array} $tests Site Health Tests. * @return array{direct: array} Amended tests. @@ -37,24 +38,34 @@ function od_optimization_detective_add_rest_api_test( array $tests ): array { * @return array{label: string, status: string, badge: array{label: string, color: string}, description: string, actions: string, test: string} Result. */ function od_optimization_detective_rest_api_test(): array { - $rest_url = get_rest_url( null, OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ); - $response = wp_remote_post( - $rest_url, - array( - 'headers' => array( 'Content-Type' => 'application/json' ), - 'sslverify' => false, - ) - ); - $result = od_construct_site_health_result( $response ); - $info = array( - 'available' => 'good' === $result['status'], - 'response' => $response, - ); - - update_option( 'od_rest_api_info', $info ); + $response = od_get_rest_api_health_check_response( false ); + $result = od_construct_site_health_result( $response ); + $is_inaccessible = 'good' !== $result['status']; + update_option( 'od_rest_api_inaccessible', $is_inaccessible ? '1' : '0' ); return $result; } +/** + * Checks whether the Optimization Detective REST API endpoint is inaccessible. + * + * This merely checks the database option what was previously computed in the Site Health test as done in {@see od_optimization_detective_rest_api_test()}. + * This is to avoid checking for REST API accessibility during a frontend request. Note that when the plugin is first + * installed, the 'od_rest_api_inaccessible' option will not be in the database, as the check has not been performed + * yet. Once Site Health's weekly check happens or when a user accesses the admin so that the admin_init action fires, + * then at this point the check will be performed at {@see od_maybe_run_rest_api_health_check()}. In practice, this will + * happen immediately after the user activates a plugin since the user is redirected back to the plugin list table in + * the admin. The reason for storing the negative inaccessible state as opposed to the positive accessible state is that + * when an option does not exist then `get_option()` returns `false` which is the same falsy value as the stored `'0'`. + * + * @since n.e.x.t + * @access private + * + * @return bool Whether inaccessible. + */ +function od_is_rest_api_inaccessible(): bool { + return 1 === (int) get_option( 'od_rest_api_inaccessible', '0' ); +} + /** * Tests availability of the Optimization Detective REST API endpoint. * @@ -129,38 +140,84 @@ function od_construct_site_health_result( $response ): array { return $result; } +/** + * Gets the response to an Optimization Detective REST API store request to confirm it is accessible to unauthenticated requests. + * + * @since n.e.x.t + * + * @param bool $use_cached Whether to use a previous response cached in a transient. + * @return array{ response: array{ code: int, message: string }, body: string }|WP_Error Response. + */ +function od_get_rest_api_health_check_response( bool $use_cached ) { + $transient_key = 'od_rest_api_health_check_response'; + $response = $use_cached ? get_transient( $transient_key ) : null; + if ( + ( + is_array( $response ) + && + isset( $response['response']['status'], $response['response']['message'], $response['body'] ) + && + is_int( $response['response']['status'] ) + && + is_string( $response['response']['message'] ) + && + is_string( $response['body'] ) + ) + || + is_wp_error( $response ) + ) { + return $response; + } + $rest_url = get_rest_url( null, OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ); + $response = wp_remote_post( + $rest_url, + array( + 'headers' => array( 'Content-Type' => 'application/json' ), + 'sslverify' => false, + ) + ); + + // This transient will be used when showing the admin notice with the plugin on the plugins screen. + // The 1-day expiration allows for fresher content than the weekly check initiated by Site Health. + set_transient( $transient_key, $response, DAY_IN_SECONDS ); + return $response; +} + /** * Renders an admin notice if the REST API health check fails. * * @since n.e.x.t * @access private + * @todo Add coverage. * * @param array $additional_classes Additional classes to add to the notice. */ function od_render_rest_api_health_check_notice( array $additional_classes = array() ): void { - $rest_api_info = get_option( 'od_rest_api_info', array() ); - if ( - isset( $rest_api_info['available'], $rest_api_info['response'] ) && - false === $rest_api_info['available'] && - ( is_array( $rest_api_info['response'] ) || is_wp_error( $rest_api_info['response'] ) ) - ) { - $result = od_construct_site_health_result( $rest_api_info['response'] ); + if ( ! od_is_rest_api_inaccessible() ) { + return; + } - wp_admin_notice( - sprintf( - '
%s %s%s %s
', - esc_html__( 'Warning:', 'optimization-detective' ), - esc_html( $result['label'] ), - wp_kses( $result['description'], array_fill_keys( array( 'p', 'code' ), array() ) ), - '

' . esc_html__( 'Please visit Site Health to re-check this once you believe you have resolved the issue.', 'optimization-detective' ) . '

' - ), - array( - 'type' => 'warning', - 'additional_classes' => $additional_classes, - 'paragraph_wrap' => false, - ) - ); + $response = od_get_rest_api_health_check_response( true ); + $result = od_construct_site_health_result( $response ); + if ( 'good' === $result['status'] ) { + // There's a slight chance the DB option is stale in the initial if statement. + return; } + + wp_admin_notice( + sprintf( + '
%s %s%s %s
', + esc_html__( 'Warning:', 'optimization-detective' ), + esc_html( $result['label'] ), + wp_kses( $result['description'], array_fill_keys( array( 'p', 'code' ), array() ) ), + '

' . esc_html__( 'Please visit Site Health to re-check this once you believe you have resolved the issue.', 'optimization-detective' ) . '

' + ), + array( + 'type' => 'warning', + 'additional_classes' => $additional_classes, + 'paragraph_wrap' => false, + ) + ); } /** @@ -168,6 +225,7 @@ function od_render_rest_api_health_check_notice( array $additional_classes = arr * * @since n.e.x.t * @access private + * @todo Add coverage. * * @param string $plugin_file Plugin file. */ @@ -190,10 +248,11 @@ function od_rest_api_health_check_admin_notice( string $plugin_file ): void { * * @since n.e.x.t * @access private + * @todo Add coverage. */ function od_maybe_run_rest_api_health_check(): void { // If the option already exists, then the REST API health check has already been performed. - if ( false !== get_option( 'od_rest_api_info' ) ) { + if ( false !== get_option( 'od_rest_api_inaccessible' ) ) { return; } diff --git a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php index 254d262c3d..025e9cc7c5 100644 --- a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php +++ b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php @@ -26,11 +26,23 @@ public function setUp(): void { // Add the filter to mock HTTP requests. add_filter( 'pre_http_request', array( $this, 'mock_http_requests' ), 10, 3 ); } + /** + * Test that we presume the REST API is accessible before we are able to perform the Site Health check. + * + * @covers ::od_is_rest_api_inaccessible + */ + public function test_rest_api_assumed_accessible(): void { + $this->assertFalse( get_option( 'od_rest_api_inaccessible', false ) ); + $this->assertFalse( od_is_rest_api_inaccessible() ); + } /** * Test that the site health check is `good` when the REST API is available. * * @covers ::od_optimization_detective_rest_api_test + * @covers ::od_construct_site_health_result + * @covers ::od_get_rest_api_health_check_response + * @covers ::od_is_rest_api_inaccessible */ public function test_rest_api_available(): void { $this->mocked_responses = array( @@ -45,18 +57,19 @@ public function test_rest_api_available(): void { ), ); - $result = od_optimization_detective_rest_api_test(); - $od_rest_api_info = get_option( 'od_rest_api_info', array() ); - + $result = od_optimization_detective_rest_api_test(); + $this->assertSame( '0', get_option( 'od_rest_api_inaccessible', false ) ); $this->assertSame( 'good', $result['status'] ); - $this->assertSame( 400, wp_remote_retrieve_response_code( $od_rest_api_info['response'] ) ); - $this->assertTrue( $od_rest_api_info['available'] ); + $this->assertFalse( od_is_rest_api_inaccessible() ); } /** * Test behavior when REST API returns an unauthorized error. * * @covers ::od_optimization_detective_rest_api_test + * @covers ::od_construct_site_health_result + * @covers ::od_get_rest_api_health_check_response + * @covers ::od_is_rest_api_inaccessible */ public function test_rest_api_unauthorized(): void { $this->mocked_responses = array( @@ -66,18 +79,19 @@ public function test_rest_api_unauthorized(): void { ), ); - $result = od_optimization_detective_rest_api_test(); - $od_rest_api_info = get_option( 'od_rest_api_info', array() ); - + $result = od_optimization_detective_rest_api_test(); + $this->assertSame( '1', get_option( 'od_rest_api_inaccessible', false ) ); $this->assertSame( 'recommended', $result['status'] ); - $this->assertSame( 401, wp_remote_retrieve_response_code( $od_rest_api_info['response'] ) ); - $this->assertFalse( $od_rest_api_info['available'] ); + $this->assertTrue( od_is_rest_api_inaccessible() ); } /** * Test behavior when REST API returns a forbidden error. * * @covers ::od_optimization_detective_rest_api_test + * @covers ::od_construct_site_health_result + * @covers ::od_get_rest_api_health_check_response + * @covers ::od_is_rest_api_inaccessible */ public function test_rest_api_forbidden(): void { $this->mocked_responses = array( @@ -87,12 +101,10 @@ public function test_rest_api_forbidden(): void { ), ); - $result = od_optimization_detective_rest_api_test(); - $od_rest_api_info = get_option( 'od_rest_api_info', array() ); - + $result = od_optimization_detective_rest_api_test(); + $this->assertSame( '1', get_option( 'od_rest_api_inaccessible', false ) ); $this->assertSame( 'recommended', $result['status'] ); - $this->assertSame( 403, wp_remote_retrieve_response_code( $od_rest_api_info['response'] ) ); - $this->assertFalse( $od_rest_api_info['available'] ); + $this->assertTrue( od_is_rest_api_inaccessible() ); } /** diff --git a/plugins/optimization-detective/uninstall.php b/plugins/optimization-detective/uninstall.php index a646ed011b..0bc0efeacc 100644 --- a/plugins/optimization-detective/uninstall.php +++ b/plugins/optimization-detective/uninstall.php @@ -19,8 +19,9 @@ wp_unschedule_hook( OD_URL_Metrics_Post_Type::GC_CRON_EVENT_NAME ); // Clear out site health check data. - delete_option( 'od_rest_api_info' ); + delete_option( 'od_rest_api_inaccessible' ); wp_unschedule_hook( 'od_rest_api_health_check_event' ); + delete_transient( 'od_rest_api_health_check_response' ); }; $od_delete_site_data(); From 22dde1bb03fd3b96521527c0115193d71b582e47 Mon Sep 17 00:00:00 2001 From: Aditya Dhade Date: Thu, 16 Jan 2025 00:42:04 +0530 Subject: [PATCH 34/63] Remove redundant unscheduling of cron event --- plugins/optimization-detective/uninstall.php | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/optimization-detective/uninstall.php b/plugins/optimization-detective/uninstall.php index 0bc0efeacc..6d3732480b 100644 --- a/plugins/optimization-detective/uninstall.php +++ b/plugins/optimization-detective/uninstall.php @@ -20,7 +20,6 @@ // Clear out site health check data. delete_option( 'od_rest_api_inaccessible' ); - wp_unschedule_hook( 'od_rest_api_health_check_event' ); delete_transient( 'od_rest_api_health_check_response' ); }; From 98691508ebe69288e168bb68630e3f0a3b7fb449 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 11:21:56 -0800 Subject: [PATCH 35/63] Refactor test to use data provider --- ...n-detective-rest-api-site-health-check.php | 145 +++++++----------- 1 file changed, 53 insertions(+), 92 deletions(-) diff --git a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php index 025e9cc7c5..04901a45cb 100644 --- a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php +++ b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php @@ -7,25 +7,6 @@ class Test_OD_REST_API_Site_Health_Check extends WP_UnitTestCase { - /** - * Holds mocked response headers for different test scenarios. - * - * @var array> - */ - protected $mocked_responses = array(); - - /** - * Setup each test. - */ - public function setUp(): void { - parent::setUp(); - - // Clear any filters or mocks. - remove_all_filters( 'pre_http_request' ); - - // Add the filter to mock HTTP requests. - add_filter( 'pre_http_request', array( $this, 'mock_http_requests' ), 10, 3 ); - } /** * Test that we presume the REST API is accessible before we are able to perform the Site Health check. * @@ -37,96 +18,76 @@ public function test_rest_api_assumed_accessible(): void { } /** - * Test that the site health check is `good` when the REST API is available. + * Data provider. * - * @covers ::od_optimization_detective_rest_api_test - * @covers ::od_construct_site_health_result - * @covers ::od_get_rest_api_health_check_response - * @covers ::od_is_rest_api_inaccessible + * @return array */ - public function test_rest_api_available(): void { - $this->mocked_responses = array( - get_rest_url( null, OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ) => $this->build_mock_response( - 400, - 'Bad Request', - array( - 'data' => array( - 'params' => array( 'slug', 'current_etag', 'hmac', 'url', 'viewport', 'elements' ), - ), - ) + public function data_provider_test_rest_api_availability(): array { + return array( + 'available' => array( + 'mocked_response' => $this->build_mock_response( + 400, + 'Bad Request', + array( + 'data' => array( + 'params' => array( 'slug', 'current_etag', 'hmac', 'url', 'viewport', 'elements' ), + ), + ) + ), + 'expected_option' => '0', + 'expected_status' => 'good', + 'expected_unavailable' => false, ), - ); - - $result = od_optimization_detective_rest_api_test(); - $this->assertSame( '0', get_option( 'od_rest_api_inaccessible', false ) ); - $this->assertSame( 'good', $result['status'] ); - $this->assertFalse( od_is_rest_api_inaccessible() ); - } - - /** - * Test behavior when REST API returns an unauthorized error. - * - * @covers ::od_optimization_detective_rest_api_test - * @covers ::od_construct_site_health_result - * @covers ::od_get_rest_api_health_check_response - * @covers ::od_is_rest_api_inaccessible - */ - public function test_rest_api_unauthorized(): void { - $this->mocked_responses = array( - get_rest_url( null, OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ) => $this->build_mock_response( - 401, - 'Unauthorized' + 'unauthorized' => array( + 'mocked_response' => $this->build_mock_response( + 401, + 'Unauthorized' + ), + 'expected_option' => '1', + 'expected_status' => 'recommended', + 'expected_unavailable' => true, + ), + 'forbidden' => array( + 'mocked_response' => $this->build_mock_response( + 403, + 'Forbidden' + ), + 'expected_option' => '1', + 'expected_status' => 'recommended', + 'expected_unavailable' => true, ), ); - - $result = od_optimization_detective_rest_api_test(); - $this->assertSame( '1', get_option( 'od_rest_api_inaccessible', false ) ); - $this->assertSame( 'recommended', $result['status'] ); - $this->assertTrue( od_is_rest_api_inaccessible() ); } /** - * Test behavior when REST API returns a forbidden error. + * Test various conditions for the REST API being available. * * @covers ::od_optimization_detective_rest_api_test * @covers ::od_construct_site_health_result * @covers ::od_get_rest_api_health_check_response * @covers ::od_is_rest_api_inaccessible + * + * @dataProvider data_provider_test_rest_api_availability + * + * @phpstan-param array $mocked_response */ - public function test_rest_api_forbidden(): void { - $this->mocked_responses = array( - get_rest_url( null, OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ) => $this->build_mock_response( - 403, - 'Forbidden' - ), + public function test_rest_api_availability( array $mocked_response, string $expected_option, string $expected_status, bool $expected_unavailable ): void { + add_filter( + 'pre_http_request', + static function ( $pre, array $args, string $url ) use ( $mocked_response ) { + if ( rest_url( OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ) === $url ) { + return $mocked_response; + } + return $pre; + }, + 10, + 3 ); $result = od_optimization_detective_rest_api_test(); - $this->assertSame( '1', get_option( 'od_rest_api_inaccessible', false ) ); - $this->assertSame( 'recommended', $result['status'] ); - $this->assertTrue( od_is_rest_api_inaccessible() ); - } - - /** - * Mock HTTP requests for assets to simulate different responses. - * - * @param bool $response A preemptive return value of an HTTP request. Default false. - * @param array $args Request arguments. - * @param string $url The request URL. - * @return array Mocked response. - */ - public function mock_http_requests( bool $response, array $args, string $url ): array { - if ( isset( $this->mocked_responses[ $url ] ) ) { - return $this->mocked_responses[ $url ]; - } - - // If no specific mock set, default to a generic success response. - return array( - 'response' => array( - 'code' => 200, - 'message' => 'OK', - ), - ); + $this->assertSame( $expected_option, get_option( 'od_rest_api_inaccessible', '' ) ); + $this->assertSame( $expected_status, $result['status'] ); + $this->assertSame( $expected_unavailable, od_is_rest_api_inaccessible() ); } /** From 55e3b55a7c40e6efe089d84a32e4a95cfdbeaca5 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 11:30:13 -0800 Subject: [PATCH 36/63] Use 'unavailable' instead of 'inaccessible' --- .../optimization-detective/site-health.php | 28 +++++++++---------- ...n-detective-rest-api-site-health-check.php | 12 ++++---- plugins/optimization-detective/uninstall.php | 2 +- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 9ab931e472..abcd7bcafe 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -38,32 +38,32 @@ function od_optimization_detective_add_rest_api_test( array $tests ): array { * @return array{label: string, status: string, badge: array{label: string, color: string}, description: string, actions: string, test: string} Result. */ function od_optimization_detective_rest_api_test(): array { - $response = od_get_rest_api_health_check_response( false ); - $result = od_construct_site_health_result( $response ); - $is_inaccessible = 'good' !== $result['status']; - update_option( 'od_rest_api_inaccessible', $is_inaccessible ? '1' : '0' ); + $response = od_get_rest_api_health_check_response( false ); + $result = od_construct_site_health_result( $response ); + $is_unavailable = 'good' !== $result['status']; + update_option( 'od_rest_api_unavailable', $is_unavailable ? '1' : '0' ); return $result; } /** - * Checks whether the Optimization Detective REST API endpoint is inaccessible. + * Checks whether the Optimization Detective REST API endpoint is unavailable. * * This merely checks the database option what was previously computed in the Site Health test as done in {@see od_optimization_detective_rest_api_test()}. * This is to avoid checking for REST API accessibility during a frontend request. Note that when the plugin is first - * installed, the 'od_rest_api_inaccessible' option will not be in the database, as the check has not been performed + * installed, the 'od_rest_api_unavailable' option will not be in the database, as the check has not been performed * yet. Once Site Health's weekly check happens or when a user accesses the admin so that the admin_init action fires, * then at this point the check will be performed at {@see od_maybe_run_rest_api_health_check()}. In practice, this will * happen immediately after the user activates a plugin since the user is redirected back to the plugin list table in - * the admin. The reason for storing the negative inaccessible state as opposed to the positive accessible state is that + * the admin. The reason for storing the negative unavailable state as opposed to the positive accessible state is that * when an option does not exist then `get_option()` returns `false` which is the same falsy value as the stored `'0'`. * * @since n.e.x.t * @access private * - * @return bool Whether inaccessible. + * @return bool Whether unavailable. */ -function od_is_rest_api_inaccessible(): bool { - return 1 === (int) get_option( 'od_rest_api_inaccessible', '0' ); +function od_is_rest_api_unavailable(): bool { + return 1 === (int) get_option( 'od_rest_api_unavailable', '0' ); } /** @@ -98,7 +98,7 @@ function od_construct_site_health_result( $response ): array { 'test' => 'optimization_detective_rest_api', ); - $error_label = __( 'Optimization Detective\'s REST API endpoint is inaccessible', 'optimization-detective' ); + $error_label = __( 'Optimization Detective\'s REST API endpoint is unavailable', 'optimization-detective' ); $error_description_html = '

' . esc_html__( 'You may have a plugin active or server configuration which restricts access to logged-in users. Unauthenticated access must be restored in order for Optimization Detective to work.', 'optimization-detective' ) . '

'; if ( is_wp_error( $response ) ) { @@ -125,7 +125,7 @@ function od_construct_site_health_result( $response ): array { ); if ( ! $is_expected ) { $result['status'] = 'recommended'; - $result['label'] = __( 'The Optimization Detective REST API endpoint is inaccessible to logged-out users', 'optimization-detective' ); + $result['label'] = __( 'The Optimization Detective REST API endpoint is unavailable to logged-out users', 'optimization-detective' ); $result['description'] = $common_description_html . $error_description_html . '

' . wp_kses( sprintf( /* translators: %d is the HTTP status code, %s is the status header description */ @@ -193,7 +193,7 @@ function od_get_rest_api_health_check_response( bool $use_cached ) { * @param array $additional_classes Additional classes to add to the notice. */ function od_render_rest_api_health_check_notice( array $additional_classes = array() ): void { - if ( ! od_is_rest_api_inaccessible() ) { + if ( ! od_is_rest_api_unavailable() ) { return; } @@ -252,7 +252,7 @@ function od_rest_api_health_check_admin_notice( string $plugin_file ): void { */ function od_maybe_run_rest_api_health_check(): void { // If the option already exists, then the REST API health check has already been performed. - if ( false !== get_option( 'od_rest_api_inaccessible' ) ) { + if ( false !== get_option( 'od_rest_api_unavailable' ) ) { return; } diff --git a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php index 04901a45cb..761ba36400 100644 --- a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php +++ b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php @@ -10,11 +10,11 @@ class Test_OD_REST_API_Site_Health_Check extends WP_UnitTestCase { /** * Test that we presume the REST API is accessible before we are able to perform the Site Health check. * - * @covers ::od_is_rest_api_inaccessible + * @covers ::od_is_rest_api_unavailable */ public function test_rest_api_assumed_accessible(): void { - $this->assertFalse( get_option( 'od_rest_api_inaccessible', false ) ); - $this->assertFalse( od_is_rest_api_inaccessible() ); + $this->assertFalse( get_option( 'od_rest_api_unavailable', false ) ); + $this->assertFalse( od_is_rest_api_unavailable() ); } /** @@ -65,7 +65,7 @@ public function data_provider_test_rest_api_availability(): array { * @covers ::od_optimization_detective_rest_api_test * @covers ::od_construct_site_health_result * @covers ::od_get_rest_api_health_check_response - * @covers ::od_is_rest_api_inaccessible + * @covers ::od_is_rest_api_unavailable * * @dataProvider data_provider_test_rest_api_availability * @@ -85,9 +85,9 @@ static function ( $pre, array $args, string $url ) use ( $mocked_response ) { ); $result = od_optimization_detective_rest_api_test(); - $this->assertSame( $expected_option, get_option( 'od_rest_api_inaccessible', '' ) ); + $this->assertSame( $expected_option, get_option( 'od_rest_api_unavailable', '' ) ); $this->assertSame( $expected_status, $result['status'] ); - $this->assertSame( $expected_unavailable, od_is_rest_api_inaccessible() ); + $this->assertSame( $expected_unavailable, od_is_rest_api_unavailable() ); } /** diff --git a/plugins/optimization-detective/uninstall.php b/plugins/optimization-detective/uninstall.php index 6d3732480b..f5e360c6b3 100644 --- a/plugins/optimization-detective/uninstall.php +++ b/plugins/optimization-detective/uninstall.php @@ -19,7 +19,7 @@ wp_unschedule_hook( OD_URL_Metrics_Post_Type::GC_CRON_EVENT_NAME ); // Clear out site health check data. - delete_option( 'od_rest_api_inaccessible' ); + delete_option( 'od_rest_api_unavailable' ); delete_transient( 'od_rest_api_health_check_response' ); }; From 85b13f4c6782d22617ac396c81aa8eb7e8e2428a Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 11:37:59 -0800 Subject: [PATCH 37/63] Omit generator tag when REST API is unavailable --- plugins/optimization-detective/helper.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/optimization-detective/helper.php b/plugins/optimization-detective/helper.php index 27073205d1..a0236e38fd 100644 --- a/plugins/optimization-detective/helper.php +++ b/plugins/optimization-detective/helper.php @@ -61,6 +61,11 @@ function od_generate_media_query( ?int $minimum_viewport_width, ?int $maximum_vi * @since 0.1.0 */ function od_render_generator_meta_tag(): void { + // Omit the generator tag if the REST API is unavailable since in this case Optimization Detective is not generating anything on the page. + if ( od_is_rest_api_unavailable() ) { + return; + } + // Use the plugin slug as it is immutable. echo '' . "\n"; } From 51a56ba84062144abfc40ca6ca853345f530b5cd Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 13:39:25 -0800 Subject: [PATCH 38/63] Fix checking array shape --- plugins/optimization-detective/site-health.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index abcd7bcafe..0a5bbff42b 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -155,9 +155,9 @@ function od_get_rest_api_health_check_response( bool $use_cached ) { ( is_array( $response ) && - isset( $response['response']['status'], $response['response']['message'], $response['body'] ) + isset( $response['response']['code'], $response['response']['message'], $response['body'] ) && - is_int( $response['response']['status'] ) + is_int( $response['response']['code'] ) && is_string( $response['response']['message'] ) && From 2a132ca76d92ab7a0ebfa67b0b742117bdbdfe91 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 13:41:01 -0800 Subject: [PATCH 39/63] Amend generator with REST API availability --- plugins/optimization-detective/helper.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/plugins/optimization-detective/helper.php b/plugins/optimization-detective/helper.php index a0236e38fd..70ff7edac6 100644 --- a/plugins/optimization-detective/helper.php +++ b/plugins/optimization-detective/helper.php @@ -61,13 +61,15 @@ function od_generate_media_query( ?int $minimum_viewport_width, ?int $maximum_vi * @since 0.1.0 */ function od_render_generator_meta_tag(): void { - // Omit the generator tag if the REST API is unavailable since in this case Optimization Detective is not generating anything on the page. + // Use the plugin slug as it is immutable. + $content = 'optimization-detective ' . OPTIMIZATION_DETECTIVE_VERSION; + + // Indicate that the plugin will not be doing anything because the REST API is unavailable. if ( od_is_rest_api_unavailable() ) { - return; + $content .= '; rest_api_unavailable'; } - // Use the plugin slug as it is immutable. - echo '' . "\n"; + echo '' . "\n"; } /** From e22bb84ae7c1e2136c254cf2706dc9e21f45bafd Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 13:41:53 -0800 Subject: [PATCH 40/63] Rename functions to better reflect purpose --- plugins/optimization-detective/hooks.php | 2 +- .../optimization-detective/site-health.php | 25 ++++++++++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/plugins/optimization-detective/hooks.php b/plugins/optimization-detective/hooks.php index 2d20e853f1..431fb3d600 100644 --- a/plugins/optimization-detective/hooks.php +++ b/plugins/optimization-detective/hooks.php @@ -17,4 +17,4 @@ add_action( 'wp_head', 'od_render_generator_meta_tag' ); add_filter( 'site_status_tests', 'od_optimization_detective_add_rest_api_test' ); add_action( 'admin_init', 'od_maybe_run_rest_api_health_check' ); -add_action( 'after_plugin_row_meta', 'od_rest_api_health_check_admin_notice', 30 ); +add_action( 'after_plugin_row_meta', 'od_render_rest_api_health_check_admin_notice_in_plugin_row', 30 ); diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 0a5bbff42b..66dd72543f 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -192,7 +192,7 @@ function od_get_rest_api_health_check_response( bool $use_cached ) { * * @param array $additional_classes Additional classes to add to the notice. */ -function od_render_rest_api_health_check_notice( array $additional_classes = array() ): void { +function od_maybe_render_rest_api_health_check_admin_notice( array $additional_classes = array() ): void { if ( ! od_is_rest_api_unavailable() ) { return; } @@ -229,11 +229,11 @@ function od_render_rest_api_health_check_notice( array $additional_classes = arr * * @param string $plugin_file Plugin file. */ -function od_rest_api_health_check_admin_notice( string $plugin_file ): void { +function od_render_rest_api_health_check_admin_notice_in_plugin_row( string $plugin_file ): void { if ( 'optimization-detective/load.php' !== $plugin_file ) { return; } - od_render_rest_api_health_check_notice( array( 'inline', 'notice-alt' ) ); + od_maybe_render_rest_api_health_check_admin_notice( array( 'inline', 'notice-alt' ) ); } /** @@ -244,7 +244,7 @@ function od_rest_api_health_check_admin_notice( string $plugin_file ): void { * that an error message can be displayed after performing that plugin activation request. Note that a plugin activation * hook cannot be used for this purpose due to not being compatible with multisite. While the site health notice is * shown at the `admin_notices` action once, the notice will only be displayed inline with the plugin row thereafter - * via {@see od_rest_api_health_check_admin_notice()}. + * via {@see od_render_rest_api_health_check_admin_notice_in_plugin_row()}. * * @since n.e.x.t * @access private @@ -257,13 +257,14 @@ function od_maybe_run_rest_api_health_check(): void { } // This will populate the od_rest_api_info option so that the function won't execute on the next page load. - od_optimization_detective_rest_api_test(); + if ( 'good' !== od_optimization_detective_rest_api_test()['status'] ) { - // Show any notice in the main admin notices area for the first page load (e.g. after plugin activation). - add_action( - 'admin_notices', - static function (): void { - od_render_rest_api_health_check_notice(); - } - ); + // Show any notice in the main admin notices area for the first page load (e.g. after plugin activation). + add_action( + 'admin_notices', + static function (): void { + od_maybe_render_rest_api_health_check_admin_notice(); + } + ); + } } From 7eda72cf803e7ac3cd7e17eb90dd9270d832e70d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 13:45:35 -0800 Subject: [PATCH 41/63] Add missing private tags to helper functions --- plugins/optimization-detective/helper.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/plugins/optimization-detective/helper.php b/plugins/optimization-detective/helper.php index 70ff7edac6..4a93d599bb 100644 --- a/plugins/optimization-detective/helper.php +++ b/plugins/optimization-detective/helper.php @@ -14,6 +14,7 @@ * Initializes extensions for Optimization Detective. * * @since 0.7.0 + * @access private */ function od_initialize_extensions(): void { /** @@ -29,10 +30,14 @@ function od_initialize_extensions(): void { /** * Generates a media query for the provided minimum and maximum viewport widths. * + * This helper function is available for extensions to leverage when manually printing STYLE rules via + * {@see OD_HTML_Tag_Processor::append_head_html()} or {@see OD_HTML_Tag_Processor::append_body_html()} + * * @since 0.7.0 * * @param int|null $minimum_viewport_width Minimum viewport width. * @param int|null $maximum_viewport_width Maximum viewport width. + * * @return non-empty-string|null Media query, or null if the min/max were both unspecified or invalid. */ function od_generate_media_query( ?int $minimum_viewport_width, ?int $maximum_viewport_width ): ?string { @@ -59,6 +64,7 @@ function od_generate_media_query( ?int $minimum_viewport_width, ?int $maximum_vi * See {@see 'wp_head'}. * * @since 0.1.0 + * @access private */ function od_render_generator_meta_tag(): void { // Use the plugin slug as it is immutable. @@ -76,6 +82,7 @@ function od_render_generator_meta_tag(): void { * Gets the path to a script or stylesheet. * * @since 0.9.0 + * @access private * * @param string $src_path Source path, relative to plugin root. * @param string|null $min_path Minified path. If not supplied, then '.min' is injected before the file extension in the source path. From cc16600431b87926f234e5d08ed38154e9b47540 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 13:57:14 -0800 Subject: [PATCH 42/63] Pass through original HTTP message --- plugins/optimization-detective/site-health.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 66dd72543f..bf45f342eb 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -114,11 +114,12 @@ function od_construct_site_health_result( $response ): array { array( 'code' => array() ) ) . '

'; } else { - $status_code = wp_remote_retrieve_response_code( $response ); - $data = json_decode( wp_remote_retrieve_body( $response ), true ); + $code = wp_remote_retrieve_response_code( $response ); + $message = wp_remote_retrieve_response_message( $response ); + $data = json_decode( wp_remote_retrieve_body( $response ), true ); $is_expected = ( - 400 === $status_code && + 400 === $code && isset( $data['data']['params'] ) && is_array( $data['data']['params'] ) && count( $data['data']['params'] ) > 0 @@ -130,8 +131,8 @@ function od_construct_site_health_result( $response ): array { sprintf( /* translators: %d is the HTTP status code, %s is the status header description */ __( 'The REST API returned with an HTTP status of %1$d %2$s.', 'optimization-detective' ), - $status_code, - get_status_header_desc( (int) $status_code ) + $code, + esc_html( $message ) ), array( 'code' => array() ) ) . '

'; From 523daaa45e4b0bbbc4cabec3272e945a0a7b711d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 14:09:43 -0800 Subject: [PATCH 43/63] Improve styling of admin notice --- .../optimization-detective/site-health.php | 52 +++++++++++++------ 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index bf45f342eb..d98b88bb74 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -49,12 +49,12 @@ function od_optimization_detective_rest_api_test(): array { * Checks whether the Optimization Detective REST API endpoint is unavailable. * * This merely checks the database option what was previously computed in the Site Health test as done in {@see od_optimization_detective_rest_api_test()}. - * This is to avoid checking for REST API accessibility during a frontend request. Note that when the plugin is first + * This is to avoid checking for REST API availability during a frontend request. Note that when the plugin is first * installed, the 'od_rest_api_unavailable' option will not be in the database, as the check has not been performed * yet. Once Site Health's weekly check happens or when a user accesses the admin so that the admin_init action fires, * then at this point the check will be performed at {@see od_maybe_run_rest_api_health_check()}. In practice, this will * happen immediately after the user activates a plugin since the user is redirected back to the plugin list table in - * the admin. The reason for storing the negative unavailable state as opposed to the positive accessible state is that + * the admin. The reason for storing the negative unavailable state as opposed to the positive available state is that * when an option does not exist then `get_option()` returns `false` which is the same falsy value as the stored `'0'`. * * @since n.e.x.t @@ -80,14 +80,14 @@ function od_construct_site_health_result( $response ): array { $common_description_html = '

' . wp_kses( sprintf( /* translators: %s is the REST API endpoint */ - __( 'To collect URL Metrics from visitors the REST API must be accessible to unauthenticated users. Specifically, visitors must be able to perform a POST request to the %s endpoint.', 'optimization-detective' ), + __( 'To collect URL Metrics from visitors the REST API must be available to unauthenticated users. Specifically, visitors must be able to perform a POST request to the %s endpoint.', 'optimization-detective' ), '/' . OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ), array( 'code' => array() ) ) . '

'; $result = array( - 'label' => __( 'Optimization Detective\'s REST API endpoint is accessible', 'optimization-detective' ), + 'label' => __( 'Optimization Detective\'s REST API endpoint is available', 'optimization-detective' ), 'status' => 'good', 'badge' => array( 'label' => __( 'Optimization Detective', 'optimization-detective' ), @@ -142,7 +142,7 @@ function od_construct_site_health_result( $response ): array { } /** - * Gets the response to an Optimization Detective REST API store request to confirm it is accessible to unauthenticated requests. + * Gets the response to an Optimization Detective REST API store request to confirm it is available to unauthenticated requests. * * @since n.e.x.t * @@ -191,9 +191,9 @@ function od_get_rest_api_health_check_response( bool $use_cached ) { * @access private * @todo Add coverage. * - * @param array $additional_classes Additional classes to add to the notice. + * @param bool $in_plugin_row Whether the notice is to be printed in the plugin row. Default false. */ -function od_maybe_render_rest_api_health_check_admin_notice( array $additional_classes = array() ): void { +function od_maybe_render_rest_api_health_check_admin_notice( bool $in_plugin_row = false ): void { if ( ! od_is_rest_api_unavailable() ) { return; } @@ -205,17 +205,37 @@ function od_maybe_render_rest_api_health_check_admin_notice( array $additional_c return; } + $message = sprintf( + $in_plugin_row + ? '%s %s' + : '

%s %s

', + esc_html__( 'Warning:', 'optimization-detective' ), + esc_html( $result['label'] ) + ); + + $message .= wp_kses( $result['description'], array_fill_keys( array( 'p', 'code' ), array() ) ); + + if ( current_user_can( 'view_site_health_checks' ) ) { + $site_health_message = wp_kses( + sprintf( + /* translators: %s is the URL to the Site Health admin screen */ + __( 'Please visit Site Health to re-check this once you believe you have resolved the issue.', 'optimization-detective' ), + esc_url( admin_url( 'site-health.php' ) ) + ), + array( 'a' => array( 'href' => array() ) ) + ); + $message .= "

$site_health_message

"; + } + + if ( $in_plugin_row ) { + $message = "
$message
"; + } + wp_admin_notice( - sprintf( - '
%s %s%s %s
', - esc_html__( 'Warning:', 'optimization-detective' ), - esc_html( $result['label'] ), - wp_kses( $result['description'], array_fill_keys( array( 'p', 'code' ), array() ) ), - '

' . esc_html__( 'Please visit Site Health to re-check this once you believe you have resolved the issue.', 'optimization-detective' ) . '

' - ), + $message, array( 'type' => 'warning', - 'additional_classes' => $additional_classes, + 'additional_classes' => $in_plugin_row ? array( 'inline', 'notice-alt' ) : array(), 'paragraph_wrap' => false, ) ); @@ -234,7 +254,7 @@ function od_render_rest_api_health_check_admin_notice_in_plugin_row( string $plu if ( 'optimization-detective/load.php' !== $plugin_file ) { return; } - od_maybe_render_rest_api_health_check_admin_notice( array( 'inline', 'notice-alt' ) ); + od_maybe_render_rest_api_health_check_admin_notice( true ); } /** From a1335e038f94270339569fba2192b77c3a5d6610 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 14:15:49 -0800 Subject: [PATCH 44/63] Improve function names --- plugins/optimization-detective/hooks.php | 2 +- plugins/optimization-detective/site-health.php | 14 +++++++------- ...zation-detective-rest-api-site-health-check.php | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/plugins/optimization-detective/hooks.php b/plugins/optimization-detective/hooks.php index 431fb3d600..0ada1214f5 100644 --- a/plugins/optimization-detective/hooks.php +++ b/plugins/optimization-detective/hooks.php @@ -15,6 +15,6 @@ OD_URL_Metrics_Post_Type::add_hooks(); add_action( 'wp', 'od_maybe_add_template_output_buffer_filter' ); add_action( 'wp_head', 'od_render_generator_meta_tag' ); -add_filter( 'site_status_tests', 'od_optimization_detective_add_rest_api_test' ); +add_filter( 'site_status_tests', 'od_add_rest_api_availability_test' ); add_action( 'admin_init', 'od_maybe_run_rest_api_health_check' ); add_action( 'after_plugin_row_meta', 'od_render_rest_api_health_check_admin_notice_in_plugin_row', 30 ); diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index d98b88bb74..169b063e16 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -20,7 +20,7 @@ * @param array{direct: array} $tests Site Health Tests. * @return array{direct: array} Amended tests. */ -function od_optimization_detective_add_rest_api_test( array $tests ): array { +function od_add_rest_api_availability_test( array $tests ): array { $tests['direct']['optimization_detective_rest_api'] = array( 'label' => __( 'Optimization Detective REST API Endpoint Availability', 'optimization-detective' ), 'test' => 'od_optimization_detective_rest_api_test', @@ -37,9 +37,9 @@ function od_optimization_detective_add_rest_api_test( array $tests ): array { * * @return array{label: string, status: string, badge: array{label: string, color: string}, description: string, actions: string, test: string} Result. */ -function od_optimization_detective_rest_api_test(): array { +function od_test_rest_api_availability(): array { $response = od_get_rest_api_health_check_response( false ); - $result = od_construct_site_health_result( $response ); + $result = od_compose_site_health_result( $response ); $is_unavailable = 'good' !== $result['status']; update_option( 'od_rest_api_unavailable', $is_unavailable ? '1' : '0' ); return $result; @@ -48,7 +48,7 @@ function od_optimization_detective_rest_api_test(): array { /** * Checks whether the Optimization Detective REST API endpoint is unavailable. * - * This merely checks the database option what was previously computed in the Site Health test as done in {@see od_optimization_detective_rest_api_test()}. + * This merely checks the database option what was previously computed in the Site Health test as done in {@see od_test_rest_api_availability()}. * This is to avoid checking for REST API availability during a frontend request. Note that when the plugin is first * installed, the 'od_rest_api_unavailable' option will not be in the database, as the check has not been performed * yet. Once Site Health's weekly check happens or when a user accesses the admin so that the admin_init action fires, @@ -76,7 +76,7 @@ function od_is_rest_api_unavailable(): bool { * * @return array{label: string, status: string, badge: array{label: string, color: string}, description: string, actions: string, test: string} Result. */ -function od_construct_site_health_result( $response ): array { +function od_compose_site_health_result( $response ): array { $common_description_html = '

' . wp_kses( sprintf( /* translators: %s is the REST API endpoint */ @@ -199,7 +199,7 @@ function od_maybe_render_rest_api_health_check_admin_notice( bool $in_plugin_row } $response = od_get_rest_api_health_check_response( true ); - $result = od_construct_site_health_result( $response ); + $result = od_compose_site_health_result( $response ); if ( 'good' === $result['status'] ) { // There's a slight chance the DB option is stale in the initial if statement. return; @@ -278,7 +278,7 @@ function od_maybe_run_rest_api_health_check(): void { } // This will populate the od_rest_api_info option so that the function won't execute on the next page load. - if ( 'good' !== od_optimization_detective_rest_api_test()['status'] ) { + if ( 'good' !== od_test_rest_api_availability()['status'] ) { // Show any notice in the main admin notices area for the first page load (e.g. after plugin activation). add_action( diff --git a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php index 761ba36400..1414a974c1 100644 --- a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php +++ b/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php @@ -62,8 +62,8 @@ public function data_provider_test_rest_api_availability(): array { /** * Test various conditions for the REST API being available. * - * @covers ::od_optimization_detective_rest_api_test - * @covers ::od_construct_site_health_result + * @covers ::od_test_rest_api_availability + * @covers ::od_compose_site_health_result * @covers ::od_get_rest_api_health_check_response * @covers ::od_is_rest_api_unavailable * @@ -84,7 +84,7 @@ static function ( $pre, array $args, string $url ) use ( $mocked_response ) { 3 ); - $result = od_optimization_detective_rest_api_test(); + $result = od_test_rest_api_availability(); $this->assertSame( $expected_option, get_option( 'od_rest_api_unavailable', '' ) ); $this->assertSame( $expected_status, $result['status'] ); $this->assertSame( $expected_unavailable, od_is_rest_api_unavailable() ); From e5d3f72a88face4373f1143c5a23631d303b9bae Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 14:18:56 -0800 Subject: [PATCH 45/63] Account for another site_status_tests filter not returning an array --- plugins/optimization-detective/site-health.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 169b063e16..8cb386606e 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -17,10 +17,13 @@ * @access private * @todo Add coverage. * - * @param array{direct: array} $tests Site Health Tests. + * @param array{direct: array}|mixed $tests Site Health Tests. * @return array{direct: array} Amended tests. */ -function od_add_rest_api_availability_test( array $tests ): array { +function od_add_rest_api_availability_test( $tests ): array { + if ( ! is_array( $tests ) ) { + $tests = array(); + } $tests['direct']['optimization_detective_rest_api'] = array( 'label' => __( 'Optimization Detective REST API Endpoint Availability', 'optimization-detective' ), 'test' => 'od_optimization_detective_rest_api_test', @@ -191,9 +194,9 @@ function od_get_rest_api_health_check_response( bool $use_cached ) { * @access private * @todo Add coverage. * - * @param bool $in_plugin_row Whether the notice is to be printed in the plugin row. Default false. + * @param bool $in_plugin_row Whether the notice is to be printed in the plugin row. */ -function od_maybe_render_rest_api_health_check_admin_notice( bool $in_plugin_row = false ): void { +function od_maybe_render_rest_api_health_check_admin_notice( bool $in_plugin_row ): void { if ( ! od_is_rest_api_unavailable() ) { return; } @@ -284,7 +287,7 @@ function od_maybe_run_rest_api_health_check(): void { add_action( 'admin_notices', static function (): void { - od_maybe_render_rest_api_health_check_admin_notice(); + od_maybe_render_rest_api_health_check_admin_notice( false ); } ); } From c6b441797484a4f662ca46f7a99ecba1255c4ad8 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 14:25:25 -0800 Subject: [PATCH 46/63] Add od_render_generator_meta_tag() test for when the REST API is unavailable --- plugins/optimization-detective/helper.php | 1 - plugins/optimization-detective/site-health.php | 2 +- .../tests/test-helper.php | 17 +++++++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/plugins/optimization-detective/helper.php b/plugins/optimization-detective/helper.php index 4a93d599bb..8d5accde89 100644 --- a/plugins/optimization-detective/helper.php +++ b/plugins/optimization-detective/helper.php @@ -37,7 +37,6 @@ function od_initialize_extensions(): void { * * @param int|null $minimum_viewport_width Minimum viewport width. * @param int|null $maximum_viewport_width Maximum viewport width. - * * @return non-empty-string|null Media query, or null if the min/max were both unspecified or invalid. */ function od_generate_media_query( ?int $minimum_viewport_width, ?int $maximum_viewport_width ): ?string { diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 8cb386606e..6526b66938 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -76,7 +76,6 @@ function od_is_rest_api_unavailable(): bool { * @access private * * @param array|WP_Error $response REST API response. - * * @return array{label: string, status: string, badge: array{label: string, color: string}, description: string, actions: string, test: string} Result. */ function od_compose_site_health_result( $response ): array { @@ -148,6 +147,7 @@ function od_compose_site_health_result( $response ): array { * Gets the response to an Optimization Detective REST API store request to confirm it is available to unauthenticated requests. * * @since n.e.x.t + * @access private * * @param bool $use_cached Whether to use a previous response cached in a transient. * @return array{ response: array{ code: int, message: string }, body: string }|WP_Error Response. diff --git a/plugins/optimization-detective/tests/test-helper.php b/plugins/optimization-detective/tests/test-helper.php index 42bcf62dd8..3d9ae9c5ad 100644 --- a/plugins/optimization-detective/tests/test-helper.php +++ b/plugins/optimization-detective/tests/test-helper.php @@ -93,5 +93,22 @@ public function test_od_render_generator_meta_tag(): void { $this->assertStringStartsWith( 'assertStringContainsString( 'generator', $tag ); $this->assertStringContainsString( 'optimization-detective ' . OPTIMIZATION_DETECTIVE_VERSION, $tag ); + $this->assertFalse( od_is_rest_api_unavailable() ); + $this->assertStringNotContainsString( 'rest_api_unavailable', $tag ); + } + + /** + * Test printing the meta generator tag when the REST API is not available. + * + * @covers ::od_render_generator_meta_tag + */ + public function test_od_render_generator_meta_tag_rest_api_unavailable(): void { + update_option( 'od_rest_api_unavailable', '1' ); + $tag = get_echo( 'od_render_generator_meta_tag' ); + $this->assertStringStartsWith( 'assertStringContainsString( 'generator', $tag ); + $this->assertStringContainsString( 'optimization-detective ' . OPTIMIZATION_DETECTIVE_VERSION, $tag ); + $this->assertTrue( od_is_rest_api_unavailable() ); + $this->assertStringContainsString( '; rest_api_unavailable', $tag ); } } From 3212dae283a800f1e5f2cc2b8e044bdb81bfed69 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 14:51:13 -0800 Subject: [PATCH 47/63] Add test for od_get_cache_purge_post_id() --- .../tests/test-optimization.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/plugins/optimization-detective/tests/test-optimization.php b/plugins/optimization-detective/tests/test-optimization.php index cba6a73837..2ae78e2d49 100644 --- a/plugins/optimization-detective/tests/test-optimization.php +++ b/plugins/optimization-detective/tests/test-optimization.php @@ -188,13 +188,26 @@ public function data_provider_test_od_can_optimize_response(): array { 'home_as_anonymous' => array( 'set_up' => function (): void { $this->go_to( home_url( '/' ) ); + $this->assertIsInt( od_get_cache_purge_post_id() ); }, 'expected' => true, ), + 'home_but_no_posts' => array( + 'set_up' => function (): void { + $posts = get_posts(); + foreach ( $posts as $post ) { + wp_delete_post( $post->ID, true ); + } + $this->go_to( home_url( '/' ) ); + $this->assertNull( od_get_cache_purge_post_id() ); + }, + 'expected' => false, // This is because od_get_cache_purge_post_id() will return false. + ), 'home_filtered_as_anonymous' => array( 'set_up' => function (): void { $this->go_to( home_url( '/' ) ); add_filter( 'od_can_optimize_response', '__return_false' ); + $this->assertIsInt( od_get_cache_purge_post_id() ); }, 'expected' => false, ), @@ -203,6 +216,7 @@ public function data_provider_test_od_can_optimize_response(): array { $posts = get_posts(); $this->assertInstanceOf( WP_Post::class, $posts[0] ); $this->go_to( get_permalink( $posts[0] ) ); + $this->assertIsInt( od_get_cache_purge_post_id() ); }, 'expected' => true, ), @@ -210,6 +224,7 @@ public function data_provider_test_od_can_optimize_response(): array { 'set_up' => function (): void { self::factory()->post->create( array( 'post_title' => 'Hello' ) ); $this->go_to( home_url( '?s=Hello' ) ); + $this->assertIsInt( od_get_cache_purge_post_id() ); }, 'expected' => false, ), @@ -220,6 +235,7 @@ public function data_provider_test_od_can_optimize_response(): array { require_once ABSPATH . 'wp-includes/class-wp-customize-manager.php'; $wp_customize = new WP_Customize_Manager(); $wp_customize->start_previewing_theme(); + $this->assertIsInt( od_get_cache_purge_post_id() ); }, 'expected' => false, ), @@ -227,6 +243,7 @@ public function data_provider_test_od_can_optimize_response(): array { 'set_up' => function (): void { $this->go_to( home_url( '/' ) ); $_SERVER['REQUEST_METHOD'] = 'POST'; + $this->assertIsInt( od_get_cache_purge_post_id() ); }, 'expected' => false, ), @@ -234,6 +251,7 @@ public function data_provider_test_od_can_optimize_response(): array { 'set_up' => function (): void { wp_set_current_user( self::factory()->user->create( array( 'role' => 'subscriber' ) ) ); $this->go_to( home_url( '/' ) ); + $this->assertIsInt( od_get_cache_purge_post_id() ); }, 'expected' => true, ), @@ -241,6 +259,7 @@ public function data_provider_test_od_can_optimize_response(): array { 'set_up' => function (): void { $user_id = self::factory()->user->create( array( 'role' => 'author' ) ); $this->go_to( get_author_posts_url( $user_id ) ); + $this->assertNull( od_get_cache_purge_post_id() ); }, 'expected' => false, ), @@ -248,6 +267,7 @@ public function data_provider_test_od_can_optimize_response(): array { 'set_up' => function (): void { wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) ); $this->go_to( home_url( '/' ) ); + $this->assertIsInt( od_get_cache_purge_post_id() ); }, 'expected' => false, ), From 546217f1540ebf7eb6fca7bf97a5a399122d4b02 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 15:18:22 -0800 Subject: [PATCH 48/63] Update check in od_maybe_add_template_output_buffer_filter() and add tests --- .../optimization-detective/optimization.php | 6 +- .../optimization-detective/site-health.php | 2 +- .../tests/test-optimization.php | 79 ++++++++++++++----- ...-health-check.php => test-site-health.php} | 0 4 files changed, 62 insertions(+), 25 deletions(-) rename plugins/optimization-detective/tests/{test-optimization-detective-rest-api-site-health-check.php => test-site-health.php} (100%) diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index e302599ed6..081df2e17d 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -78,11 +78,11 @@ static function ( string $output, ?int $phase ): string { * @access private */ function od_maybe_add_template_output_buffer_filter(): void { - $od_rest_api_info = get_option( 'od_rest_api_info' ); if ( ! od_can_optimize_response() || - isset( $_GET['optimization_detective_disabled'] ) || // phpcs:ignore WordPress.Security.NonceVerification.Recommended - ( isset( $od_rest_api_info['available'] ) && ! (bool) $od_rest_api_info['available'] ) + od_is_rest_api_unavailable() || + isset( $_GET['optimization_detective_disabled'] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended + ) { return; } diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 6526b66938..978ab845c6 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -280,7 +280,7 @@ function od_maybe_run_rest_api_health_check(): void { return; } - // This will populate the od_rest_api_info option so that the function won't execute on the next page load. + // This will populate the od_rest_api_unavailable option so that the function won't execute on the next page load. if ( 'good' !== od_test_rest_api_availability()['status'] ) { // Show any notice in the main admin notices area for the first page load (e.g. after plugin activation). diff --git a/plugins/optimization-detective/tests/test-optimization.php b/plugins/optimization-detective/tests/test-optimization.php index f9d7c3f9f0..5436f0274f 100644 --- a/plugins/optimization-detective/tests/test-optimization.php +++ b/plugins/optimization-detective/tests/test-optimization.php @@ -145,37 +145,74 @@ function ( $buffer ) use ( $template_start, $template_middle, $template_end, &$f } /** - * Test od_maybe_add_template_output_buffer_filter(). + * Data provider. * - * @covers ::od_maybe_add_template_output_buffer_filter + * @return array */ - public function test_od_maybe_add_template_output_buffer_filter(): void { - $this->assertFalse( has_filter( 'od_template_output_buffer' ) ); - - add_filter( 'od_can_optimize_response', '__return_false', 1 ); - od_maybe_add_template_output_buffer_filter(); - $this->assertFalse( od_can_optimize_response() ); - $this->assertFalse( has_filter( 'od_template_output_buffer' ) ); - - add_filter( 'od_can_optimize_response', '__return_true', 2 ); - $this->go_to( home_url( '/' ) ); - $this->assertTrue( od_can_optimize_response() ); - od_maybe_add_template_output_buffer_filter(); - $this->assertTrue( has_filter( 'od_template_output_buffer' ) ); + public function data_provider_test_od_maybe_add_template_output_buffer_filter(): array { + return array( + 'home_enabled' => array( + 'set_up' => static function (): string { + return home_url( '/' ); + }, + 'expected_has_filter' => true, + ), + 'home_disabled_by_filter' => array( + 'set_up' => static function (): string { + add_filter( 'od_can_optimize_response', '__return_false' ); + return home_url( '/' ); + }, + 'expected_has_filter' => false, + ), + 'search_disabled' => array( + 'set_up' => static function (): string { + return home_url( '/?s=foo' ); + }, + 'expected_has_filter' => false, + ), + 'search_enabled_by_filter' => array( + 'set_up' => static function (): string { + add_filter( 'od_can_optimize_response', '__return_true' ); + return home_url( '/?s=foo' ); + }, + 'expected_has_filter' => true, + ), + 'home_disabled_by_get_param' => array( + 'set_up' => static function (): string { + return home_url( '/?optimization_detective_disabled=1' ); + }, + 'expected_has_filter' => false, + ), + 'home_disabled_by_rest_api_unavailable' => array( + 'set_up' => static function (): string { + update_option( 'od_rest_api_unavailable', '1' ); + return home_url( '/' ); + }, + 'expected_has_filter' => false, + ), + ); } + /** * Test od_maybe_add_template_output_buffer_filter(). * + * @dataProvider data_provider_test_od_maybe_add_template_output_buffer_filter + * * @covers ::od_maybe_add_template_output_buffer_filter + * @covers ::od_can_optimize_response + * @covers ::od_is_rest_api_unavailable */ - public function test_od_maybe_add_template_output_buffer_filter_with_query_var_to_disable(): void { - $this->assertFalse( has_filter( 'od_template_output_buffer' ) ); + public function test_od_maybe_add_template_output_buffer_filter( Closure $set_up, bool $expected_has_filter ): void { + // There needs to be a post so that there is a post in the loop so that od_get_cache_purge_post_id() returns a post ID. + // Otherwise, od_can_optimize_response() will return false unless forced by a filter. + self::factory()->post->create(); + + $url = $set_up(); + $this->go_to( $url ); + remove_all_filters( 'od_template_output_buffer' ); // In case go_to() caused them to be added. - add_filter( 'od_can_optimize_response', '__return_true' ); - $this->go_to( home_url( '/?optimization_detective_disabled=1' ) ); - $this->assertTrue( od_can_optimize_response() ); od_maybe_add_template_output_buffer_filter(); - $this->assertFalse( has_filter( 'od_template_output_buffer' ) ); + $this->assertSame( $expected_has_filter, has_filter( 'od_template_output_buffer' ) ); } /** diff --git a/plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php b/plugins/optimization-detective/tests/test-site-health.php similarity index 100% rename from plugins/optimization-detective/tests/test-optimization-detective-rest-api-site-health-check.php rename to plugins/optimization-detective/tests/test-site-health.php From 3d5e8a80a9bae5086ea28a9c55d9e81c8442404c Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 16:20:26 -0800 Subject: [PATCH 49/63] Add test coverage for Site Health logic --- .../optimization-detective/site-health.php | 7 +- .../tests/test-optimization.php | 48 ++-- .../tests/test-site-health.php | 243 ++++++++++++++++-- 3 files changed, 244 insertions(+), 54 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 978ab845c6..7672e9438b 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -15,7 +15,6 @@ * * @since n.e.x.t * @access private - * @todo Add coverage. * * @param array{direct: array}|mixed $tests Site Health Tests. * @return array{direct: array} Amended tests. @@ -192,7 +191,6 @@ function od_get_rest_api_health_check_response( bool $use_cached ) { * * @since n.e.x.t * @access private - * @todo Add coverage. * * @param bool $in_plugin_row Whether the notice is to be printed in the plugin row. */ @@ -249,12 +247,11 @@ function od_maybe_render_rest_api_health_check_admin_notice( bool $in_plugin_row * * @since n.e.x.t * @access private - * @todo Add coverage. * * @param string $plugin_file Plugin file. */ function od_render_rest_api_health_check_admin_notice_in_plugin_row( string $plugin_file ): void { - if ( 'optimization-detective/load.php' !== $plugin_file ) { + if ( 'optimization-detective/load.php' !== $plugin_file ) { // TODO: What if a different plugin slug is used? return; } od_maybe_render_rest_api_health_check_admin_notice( true ); @@ -272,7 +269,6 @@ function od_render_rest_api_health_check_admin_notice_in_plugin_row( string $plu * * @since n.e.x.t * @access private - * @todo Add coverage. */ function od_maybe_run_rest_api_health_check(): void { // If the option already exists, then the REST API health check has already been performed. @@ -282,7 +278,6 @@ function od_maybe_run_rest_api_health_check(): void { // This will populate the od_rest_api_unavailable option so that the function won't execute on the next page load. if ( 'good' !== od_test_rest_api_availability()['status'] ) { - // Show any notice in the main admin notices area for the first page load (e.g. after plugin activation). add_action( 'admin_notices', diff --git a/plugins/optimization-detective/tests/test-optimization.php b/plugins/optimization-detective/tests/test-optimization.php index 5436f0274f..879221c1a8 100644 --- a/plugins/optimization-detective/tests/test-optimization.php +++ b/plugins/optimization-detective/tests/test-optimization.php @@ -241,70 +241,62 @@ public function data_provider_test_od_can_optimize_response(): array { 'expected' => false, // This is because od_get_cache_purge_post_id() will return false. ), 'home_filtered_as_anonymous' => array( - 'set_up' => function (): void { - $this->go_to( home_url( '/' ) ); + 'set_up' => static function (): string { add_filter( 'od_can_optimize_response', '__return_false' ); - $this->assertIsInt( od_get_cache_purge_post_id() ); + return home_url( '/' ); }, 'expected' => false, ), 'singular_as_anonymous' => array( - 'set_up' => function (): void { + 'set_up' => function (): string { $posts = get_posts(); $this->assertInstanceOf( WP_Post::class, $posts[0] ); - $this->go_to( get_permalink( $posts[0] ) ); - $this->assertIsInt( od_get_cache_purge_post_id() ); + return get_permalink( $posts[0] ); }, 'expected' => true, ), 'search_as_anonymous' => array( - 'set_up' => function (): void { + 'set_up' => static function (): string { self::factory()->post->create( array( 'post_title' => 'Hello' ) ); - $this->go_to( home_url( '?s=Hello' ) ); - $this->assertIsInt( od_get_cache_purge_post_id() ); + return home_url( '?s=Hello' ); }, 'expected' => false, ), 'home_customizer_preview_as_anonymous' => array( - 'set_up' => function (): void { - $this->go_to( home_url( '/' ) ); + 'set_up' => static function (): string { global $wp_customize; require_once ABSPATH . 'wp-includes/class-wp-customize-manager.php'; $wp_customize = new WP_Customize_Manager(); $wp_customize->start_previewing_theme(); - $this->assertIsInt( od_get_cache_purge_post_id() ); + return home_url( '/' ); }, 'expected' => false, ), 'home_post_request_as_anonymous' => array( - 'set_up' => function (): void { - $this->go_to( home_url( '/' ) ); + 'set_up' => static function (): string { $_SERVER['REQUEST_METHOD'] = 'POST'; - $this->assertIsInt( od_get_cache_purge_post_id() ); + return home_url( '/' ); }, 'expected' => false, ), 'home_as_subscriber' => array( - 'set_up' => function (): void { + 'set_up' => static function (): string { wp_set_current_user( self::factory()->user->create( array( 'role' => 'subscriber' ) ) ); - $this->go_to( home_url( '/' ) ); - $this->assertIsInt( od_get_cache_purge_post_id() ); + return home_url( '/' ); }, 'expected' => true, ), 'empty_author_page_as_anonymous' => array( - 'set_up' => function (): void { + 'set_up' => static function (): string { $user_id = self::factory()->user->create( array( 'role' => 'author' ) ); - $this->go_to( get_author_posts_url( $user_id ) ); - $this->assertNull( od_get_cache_purge_post_id() ); + return get_author_posts_url( $user_id ); }, 'expected' => false, ), 'home_as_admin' => array( - 'set_up' => function (): void { + 'set_up' => static function (): string { wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) ); - $this->go_to( home_url( '/' ) ); - $this->assertIsInt( od_get_cache_purge_post_id() ); + return home_url( '/' ); }, 'expected' => true, ), @@ -320,8 +312,12 @@ public function data_provider_test_od_can_optimize_response(): array { * @dataProvider data_provider_test_od_can_optimize_response */ public function test_od_can_optimize_response( Closure $set_up, bool $expected ): void { - self::factory()->post->create(); // Make sure there is at least one post in the DB. - $set_up(); + // Make sure there is at least one post in the DB as otherwise od_get_cache_purge_post_id() will return false, + // causing od_can_optimize_response() to return false. + self::factory()->post->create(); + + $url = $set_up(); + $this->go_to( $url ); $this->assertSame( $expected, od_can_optimize_response() ); } diff --git a/plugins/optimization-detective/tests/test-site-health.php b/plugins/optimization-detective/tests/test-site-health.php index 1414a974c1..3e9d110e3c 100644 --- a/plugins/optimization-detective/tests/test-site-health.php +++ b/plugins/optimization-detective/tests/test-site-health.php @@ -7,6 +7,56 @@ class Test_OD_REST_API_Site_Health_Check extends WP_UnitTestCase { + const EXPECTED_MOCKED_RESPONSE_ARGS = array( + 400, + 'Bad Request', + array( + 'data' => array( + 'params' => array( 'slug', 'current_etag', 'hmac', 'url', 'viewport', 'elements' ), + ), + ), + ); + + const UNAUTHORISED_MOCKED_RESPONSE_ARGS = array( + 401, + 'Unauthorized', + ); + + const FORBIDDEN_MOCKED_RESPONSE_ARGS = array( + 403, + 'Forbidden', + ); + + /** + * @covers ::od_add_rest_api_availability_test + */ + public function test_od_add_rest_api_availability_test(): void { + $initial_tests = array( + 'direct' => array( + 'foo' => array( + 'label' => 'Foo', + 'test' => 'foo_test', + ), + ), + ); + + $tests = od_add_rest_api_availability_test( + $initial_tests + ); + $this->assertCount( 2, $tests['direct'] ); + $this->assertArrayHasKey( 'foo', $tests['direct'] ); + $this->assertSame( $initial_tests['direct']['foo'], $tests['direct']['foo'] ); + $this->assertArrayHasKey( 'optimization_detective_rest_api', $tests['direct'] ); + $this->assertArrayHasKey( 'label', $tests['direct']['optimization_detective_rest_api'] ); + $this->assertArrayHasKey( 'test', $tests['direct']['optimization_detective_rest_api'] ); + + $tests = od_add_rest_api_availability_test( + new WP_Error() + ); + $this->assertCount( 1, $tests['direct'] ); + $this->assertArrayHasKey( 'optimization_detective_rest_api', $tests['direct'] ); + } + /** * Test that we presume the REST API is accessible before we are able to perform the Site Health check. * @@ -25,33 +75,19 @@ public function test_rest_api_assumed_accessible(): void { public function data_provider_test_rest_api_availability(): array { return array( 'available' => array( - 'mocked_response' => $this->build_mock_response( - 400, - 'Bad Request', - array( - 'data' => array( - 'params' => array( 'slug', 'current_etag', 'hmac', 'url', 'viewport', 'elements' ), - ), - ) - ), + 'mocked_response' => $this->build_mock_response( ...self::EXPECTED_MOCKED_RESPONSE_ARGS ), 'expected_option' => '0', 'expected_status' => 'good', 'expected_unavailable' => false, ), 'unauthorized' => array( - 'mocked_response' => $this->build_mock_response( - 401, - 'Unauthorized' - ), + 'mocked_response' => $this->build_mock_response( ...self::UNAUTHORISED_MOCKED_RESPONSE_ARGS ), 'expected_option' => '1', 'expected_status' => 'recommended', 'expected_unavailable' => true, ), 'forbidden' => array( - 'mocked_response' => $this->build_mock_response( - 403, - 'Forbidden' - ), + 'mocked_response' => $this->build_mock_response( ...self::FORBIDDEN_MOCKED_RESPONSE_ARGS ), 'expected_option' => '1', 'expected_status' => 'recommended', 'expected_unavailable' => true, @@ -72,6 +108,174 @@ public function data_provider_test_rest_api_availability(): array { * @phpstan-param array $mocked_response */ public function test_rest_api_availability( array $mocked_response, string $expected_option, string $expected_status, bool $expected_unavailable ): void { + $this->filter_rest_api_response( $mocked_response ); + + $result = od_test_rest_api_availability(); + $this->assertSame( $expected_option, get_option( 'od_rest_api_unavailable', '' ) ); + $this->assertSame( $expected_status, $result['status'] ); + $this->assertSame( $expected_unavailable, od_is_rest_api_unavailable() ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_provider_in_plugin_row(): array { + return array( + 'in_admin_notices' => array( + 'in_plugin_row' => false, + ), + 'in_plugin_row' => array( + 'in_plugin_row' => true, + ), + ); + } + + /** + * Initial state when there is no option set yet. + * + * @dataProvider data_provider_in_plugin_row + * @covers ::od_maybe_render_rest_api_health_check_admin_notice + */ + public function test_od_maybe_render_rest_api_health_check_admin_notice_no_option_set( bool $in_plugin_row ): void { + $this->assertFalse( od_is_rest_api_unavailable() ); + $this->assertSame( '', get_echo( 'od_maybe_render_rest_api_health_check_admin_notice', array( $in_plugin_row ) ) ); + } + + /** + * When the REST API works as expected. + * + * @dataProvider data_provider_in_plugin_row + * @covers ::od_maybe_render_rest_api_health_check_admin_notice + */ + public function test_od_maybe_render_rest_api_health_check_admin_notice_rest_api_available( bool $in_plugin_row ): void { + $this->filter_rest_api_response( $this->build_mock_response( ...self::EXPECTED_MOCKED_RESPONSE_ARGS ) ); + od_test_rest_api_availability(); + $this->assertFalse( od_is_rest_api_unavailable() ); + $this->assertSame( '', get_echo( 'od_maybe_render_rest_api_health_check_admin_notice', array( $in_plugin_row ) ) ); + } + + /** + * When the REST API is not available. + * + * @dataProvider data_provider_in_plugin_row + * @covers ::od_maybe_render_rest_api_health_check_admin_notice + */ + public function test_od_maybe_render_rest_api_health_check_admin_notice_rest_api_not_available( bool $in_plugin_row ): void { + wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) ); + + $this->filter_rest_api_response( $this->build_mock_response( ...self::UNAUTHORISED_MOCKED_RESPONSE_ARGS ) ); + od_test_rest_api_availability(); + $this->assertTrue( od_is_rest_api_unavailable() ); + $notice = get_echo( 'od_maybe_render_rest_api_health_check_admin_notice', array( $in_plugin_row ) ); + $this->assertStringContainsString( '

assertStringContainsString( '
', $notice ); + $this->assertStringContainsString( '', $notice ); + $this->assertStringNotContainsString( '

', $notice ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_provider_test_od_maybe_run_rest_api_health_check(): array { + return array( + 'option_absent_and_rest_api_available' => array( + 'set_up' => function (): void { + delete_option( 'od_rest_api_unavailable' ); + $this->filter_rest_api_response( $this->build_mock_response( ...self::EXPECTED_MOCKED_RESPONSE_ARGS ) ); + }, + 'expected' => false, + ), + 'option_present_and_cached_available' => array( + 'set_up' => static function (): void { + update_option( 'od_rest_api_unavailable', '0' ); + }, + 'expected' => false, + ), + 'rest_api_unavailable' => array( + 'set_up' => function (): void { + delete_option( 'od_rest_api_unavailable' ); + $this->filter_rest_api_response( $this->build_mock_response( ...self::UNAUTHORISED_MOCKED_RESPONSE_ARGS ) ); + }, + 'expected' => true, + ), + ); + } + + /** + * @dataProvider data_provider_test_od_maybe_run_rest_api_health_check + * + * @covers ::od_maybe_run_rest_api_health_check + */ + public function test_od_maybe_run_rest_api_health( Closure $set_up, bool $expected ): void { + remove_all_actions( 'admin_notices' ); + $set_up(); + od_maybe_run_rest_api_health_check(); + $this->assertSame( $expected, (bool) has_action( 'admin_notices' ) ); + } + + /** + * Filters REST API response with mock. + * + * @param array $mocked_response Mocked response. + */ + protected function filter_rest_api_response( array $mocked_response ): void { + remove_all_filters( 'pre_http_request' ); add_filter( 'pre_http_request', static function ( $pre, array $args, string $url ) use ( $mocked_response ) { @@ -83,11 +287,6 @@ static function ( $pre, array $args, string $url ) use ( $mocked_response ) { 10, 3 ); - - $result = od_test_rest_api_availability(); - $this->assertSame( $expected_option, get_option( 'od_rest_api_unavailable', '' ) ); - $this->assertSame( $expected_status, $result['status'] ); - $this->assertSame( $expected_unavailable, od_is_rest_api_unavailable() ); } /** From c3b682f482345386d6c833de24b9ae4122384d6e Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 16:32:15 -0800 Subject: [PATCH 50/63] Account for Site Health not being available in multisite to regular admins --- plugins/optimization-detective/tests/test-site-health.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/optimization-detective/tests/test-site-health.php b/plugins/optimization-detective/tests/test-site-health.php index 3e9d110e3c..2e72eed01e 100644 --- a/plugins/optimization-detective/tests/test-site-health.php +++ b/plugins/optimization-detective/tests/test-site-health.php @@ -163,7 +163,9 @@ public function test_od_maybe_render_rest_api_health_check_admin_notice_rest_api * @covers ::od_maybe_render_rest_api_health_check_admin_notice */ public function test_od_maybe_render_rest_api_health_check_admin_notice_rest_api_not_available( bool $in_plugin_row ): void { - wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) ); + $user_id = self::factory()->user->create( array( 'role' => 'administrator' ) ); + grant_super_admin( $user_id ); // Since Site Health is only available to super admins. + wp_set_current_user( $user_id ); $this->filter_rest_api_response( $this->build_mock_response( ...self::UNAUTHORISED_MOCKED_RESPONSE_ARGS ) ); od_test_rest_api_availability(); From c085ff0ce2a8954bd34b6de6f1a032184e64dbcf Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 16:32:58 -0800 Subject: [PATCH 51/63] Add test coverage for HTTP request returning WP_Error --- .../tests/test-site-health.php | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/plugins/optimization-detective/tests/test-site-health.php b/plugins/optimization-detective/tests/test-site-health.php index 2e72eed01e..f5e64c1946 100644 --- a/plugins/optimization-detective/tests/test-site-health.php +++ b/plugins/optimization-detective/tests/test-site-health.php @@ -92,6 +92,12 @@ public function data_provider_test_rest_api_availability(): array { 'expected_status' => 'recommended', 'expected_unavailable' => true, ), + 'error' => array( + 'mocked_response' => new WP_Error( 'bad', 'Something terrible has happened' ), + 'expected_option' => '1', + 'expected_status' => 'recommended', + 'expected_unavailable' => true, + ), ); } @@ -105,12 +111,17 @@ public function data_provider_test_rest_api_availability(): array { * * @dataProvider data_provider_test_rest_api_availability * - * @phpstan-param array $mocked_response + * @phpstan-param array|WP_Error $mocked_response */ - public function test_rest_api_availability( array $mocked_response, string $expected_option, string $expected_status, bool $expected_unavailable ): void { + public function test_rest_api_availability( $mocked_response, string $expected_option, string $expected_status, bool $expected_unavailable ): void { $this->filter_rest_api_response( $mocked_response ); $result = od_test_rest_api_availability(); + $this->assertArrayHasKey( 'label', $result ); + $this->assertArrayHasKey( 'status', $result ); + $this->assertArrayHasKey( 'badge', $result ); + $this->assertArrayHasKey( 'description', $result ); + $this->assertArrayHasKey( 'test', $result ); $this->assertSame( $expected_option, get_option( 'od_rest_api_unavailable', '' ) ); $this->assertSame( $expected_status, $result['status'] ); $this->assertSame( $expected_unavailable, od_is_rest_api_unavailable() ); @@ -274,9 +285,9 @@ public function test_od_maybe_run_rest_api_health( Closure $set_up, bool $expect /** * Filters REST API response with mock. * - * @param array $mocked_response Mocked response. + * @param array|WP_Error $mocked_response Mocked response. */ - protected function filter_rest_api_response( array $mocked_response ): void { + protected function filter_rest_api_response( $mocked_response ): void { remove_all_filters( 'pre_http_request' ); add_filter( 'pre_http_request', From 50a18988d54dab0838b5f0a6719f6f929d1c52b0 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 16:43:01 -0800 Subject: [PATCH 52/63] Fix absent site health test after failed function rename --- plugins/optimization-detective/site-health.php | 5 ++++- plugins/optimization-detective/tests/test-site-health.php | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 7672e9438b..b0494d9229 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -25,7 +25,10 @@ function od_add_rest_api_availability_test( $tests ): array { } $tests['direct']['optimization_detective_rest_api'] = array( 'label' => __( 'Optimization Detective REST API Endpoint Availability', 'optimization-detective' ), - 'test' => 'od_optimization_detective_rest_api_test', + 'test' => static function () { + // Note: A closure is used here to improve symbol discovery for the sake of potential refactoring. + return od_test_rest_api_availability(); + }, ); return $tests; diff --git a/plugins/optimization-detective/tests/test-site-health.php b/plugins/optimization-detective/tests/test-site-health.php index f5e64c1946..bf89df9ef3 100644 --- a/plugins/optimization-detective/tests/test-site-health.php +++ b/plugins/optimization-detective/tests/test-site-health.php @@ -49,6 +49,10 @@ public function test_od_add_rest_api_availability_test(): void { $this->assertArrayHasKey( 'optimization_detective_rest_api', $tests['direct'] ); $this->assertArrayHasKey( 'label', $tests['direct']['optimization_detective_rest_api'] ); $this->assertArrayHasKey( 'test', $tests['direct']['optimization_detective_rest_api'] ); + $this->assertTrue( is_callable( $tests['direct']['optimization_detective_rest_api']['test'] ) ); + $this->filter_rest_api_response( $this->build_mock_response( ...self::EXPECTED_MOCKED_RESPONSE_ARGS ) ); + $result = call_user_func( $tests['direct']['optimization_detective_rest_api']['test'] ); + $this->assertSame( 'good', $result['status'] ); $tests = od_add_rest_api_availability_test( new WP_Error() From c7d31a77e887dcb231f7e726b0421e186b844bb1 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 16:46:38 -0800 Subject: [PATCH 53/63] Further account for multisite and Site Health --- .../tests/test-site-health.php | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/plugins/optimization-detective/tests/test-site-health.php b/plugins/optimization-detective/tests/test-site-health.php index bf89df9ef3..eb2edf228c 100644 --- a/plugins/optimization-detective/tests/test-site-health.php +++ b/plugins/optimization-detective/tests/test-site-health.php @@ -178,9 +178,9 @@ public function test_od_maybe_render_rest_api_health_check_admin_notice_rest_api * @covers ::od_maybe_render_rest_api_health_check_admin_notice */ public function test_od_maybe_render_rest_api_health_check_admin_notice_rest_api_not_available( bool $in_plugin_row ): void { - $user_id = self::factory()->user->create( array( 'role' => 'administrator' ) ); - grant_super_admin( $user_id ); // Since Site Health is only available to super admins. - wp_set_current_user( $user_id ); + $super_admin_user_id = self::factory()->user->create( array( 'role' => 'administrator' ) ); + grant_super_admin( $super_admin_user_id ); // Since Site Health is only available to super admins. + wp_set_current_user( $super_admin_user_id ); $this->filter_rest_api_response( $this->build_mock_response( ...self::UNAUTHORISED_MOCKED_RESPONSE_ARGS ) ); od_test_rest_api_availability(); @@ -200,13 +200,7 @@ public function test_od_maybe_render_rest_api_health_check_admin_notice_rest_api $this->assertStringContainsString( 'site-health.php', $notice ); // And also when the user doesn't have access to Site Health. - add_filter( - 'user_has_cap', - static function ( array $all_caps ): array { - $all_caps['view_site_health_checks'] = false; - return $all_caps; - } - ); + wp_set_current_user( self::factory()->user->create( array( 'role' => 'editor' ) ) ); $this->assertFalse( current_user_can( 'view_site_health_checks' ) ); $notice = get_echo( 'od_maybe_render_rest_api_health_check_admin_notice', array( $in_plugin_row ) ); $this->assertStringNotContainsString( 'site-health.php', $notice ); From 85da1622ca5f7e6c34c0b962c7bab7b0acf2a83c Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 16:55:45 -0800 Subject: [PATCH 54/63] Make Optimization Detective REST API access a critical error and improve formatting of error message --- plugins/optimization-detective/site-health.php | 13 ++++++------- .../tests/test-site-health.php | 6 +++--- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index b0494d9229..aca71894a8 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -106,17 +106,16 @@ function od_compose_site_health_result( $response ): array { $error_description_html = '

' . esc_html__( 'You may have a plugin active or server configuration which restricts access to logged-in users. Unauthenticated access must be restored in order for Optimization Detective to work.', 'optimization-detective' ) . '

'; if ( is_wp_error( $response ) ) { - $result['status'] = 'recommended'; + $result['status'] = 'critical'; $result['label'] = $error_label; $result['description'] = $common_description_html . $error_description_html . '

' . wp_kses( sprintf( - /* translators: 1: the error code, 2: the error message */ - __( 'The REST API responded with the error code %1$s and this error message: %2$s.', 'optimization-detective' ), - esc_html( (string) $response->get_error_code() ), - esc_html( rtrim( $response->get_error_message(), '.' ) ) + /* translators: %s is the error code */ + __( 'The REST API responded with the error code %s and the following error message:', 'optimization-detective' ), + esc_html( (string) $response->get_error_code() ) ), array( 'code' => array() ) - ) . '

'; + ) . '

' . esc_html( $response->get_error_message() ) . '
'; } else { $code = wp_remote_retrieve_response_code( $response ); $message = wp_remote_retrieve_response_message( $response ); @@ -129,7 +128,7 @@ function od_compose_site_health_result( $response ): array { count( $data['data']['params'] ) > 0 ); if ( ! $is_expected ) { - $result['status'] = 'recommended'; + $result['status'] = 'critical'; $result['label'] = __( 'The Optimization Detective REST API endpoint is unavailable to logged-out users', 'optimization-detective' ); $result['description'] = $common_description_html . $error_description_html . '

' . wp_kses( sprintf( diff --git a/plugins/optimization-detective/tests/test-site-health.php b/plugins/optimization-detective/tests/test-site-health.php index eb2edf228c..284f1c788e 100644 --- a/plugins/optimization-detective/tests/test-site-health.php +++ b/plugins/optimization-detective/tests/test-site-health.php @@ -87,19 +87,19 @@ public function data_provider_test_rest_api_availability(): array { 'unauthorized' => array( 'mocked_response' => $this->build_mock_response( ...self::UNAUTHORISED_MOCKED_RESPONSE_ARGS ), 'expected_option' => '1', - 'expected_status' => 'recommended', + 'expected_status' => 'critical', 'expected_unavailable' => true, ), 'forbidden' => array( 'mocked_response' => $this->build_mock_response( ...self::FORBIDDEN_MOCKED_RESPONSE_ARGS ), 'expected_option' => '1', - 'expected_status' => 'recommended', + 'expected_status' => 'critical', 'expected_unavailable' => true, ), 'error' => array( 'mocked_response' => new WP_Error( 'bad', 'Something terrible has happened' ), 'expected_option' => '1', - 'expected_status' => 'recommended', + 'expected_status' => 'critical', 'expected_unavailable' => true, ), ); From 0772ebecba043ec2d84730eab63f74f0a4e32dee Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 17:15:05 -0800 Subject: [PATCH 55/63] Increase specificity of what the expected error response looks like --- plugins/optimization-detective/site-health.php | 13 +++++++++---- .../optimization-detective/storage/rest-api.php | 3 +++ .../tests/test-site-health.php | 14 ++++++++++++-- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index aca71894a8..9e2b23dc62 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -119,13 +119,16 @@ function od_compose_site_health_result( $response ): array { } else { $code = wp_remote_retrieve_response_code( $response ); $message = wp_remote_retrieve_response_message( $response ); - $data = json_decode( wp_remote_retrieve_body( $response ), true ); + $body = wp_remote_retrieve_body( $response ); + $data = json_decode( $body, true ); - $is_expected = ( + $required_args = array( 'slug', 'current_etag', 'hmac', 'url', 'viewport', 'elements' ); + $is_expected = ( 400 === $code && - isset( $data['data']['params'] ) && + isset( $data['code'], $data['data']['params'] ) && + 'rest_missing_callback_param' === $data['code'] && is_array( $data['data']['params'] ) && - count( $data['data']['params'] ) > 0 + count( array_intersect( $data['data']['params'], $required_args ) ) === count( $required_args ) ); if ( ! $is_expected ) { $result['status'] = 'critical'; @@ -139,6 +142,8 @@ function od_compose_site_health_result( $response ): array { ), array( 'code' => array() ) ) . '

'; + + $result['description'] .= '
' . esc_html__( 'Raw response:', 'optimization-detective' ) . '
' . esc_html( $body ) . '
'; } } return $result; diff --git a/plugins/optimization-detective/storage/rest-api.php b/plugins/optimization-detective/storage/rest-api.php index 09ce02501e..1b182b0433 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -34,11 +34,14 @@ * * @since 0.1.0 * @access private + * + * @see od_compose_site_health_result() */ function od_register_endpoint(): void { // The slug and cache_purge_post_id args are further validated via the validate_callback for the 'hmac' parameter, // they are provided as input with the 'url' argument to create the HMAC by the server. + // The following args are referenced in od_compose_site_health_result(). $args = array( 'slug' => array( 'type' => 'string', diff --git a/plugins/optimization-detective/tests/test-site-health.php b/plugins/optimization-detective/tests/test-site-health.php index 284f1c788e..f262d33dbf 100644 --- a/plugins/optimization-detective/tests/test-site-health.php +++ b/plugins/optimization-detective/tests/test-site-health.php @@ -11,8 +11,18 @@ class Test_OD_REST_API_Site_Health_Check extends WP_UnitTestCase { 400, 'Bad Request', array( - 'data' => array( - 'params' => array( 'slug', 'current_etag', 'hmac', 'url', 'viewport', 'elements' ), + 'code' => 'rest_missing_callback_param', + 'message' => 'Missing parameter(s): slug, current_etag, hmac, url, viewport, elements', + 'data' => array( + 'status' => 400, + 'params' => array( + 'slug', + 'current_etag', + 'hmac', + 'url', + 'viewport', + 'elements', + ), ), ), ); From dbff1f739fc3e5f209f420ebab88e4253c5aa1d8 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 15 Jan 2025 17:32:17 -0800 Subject: [PATCH 56/63] Show REST API error message in blockquote if available; add Nginx error example --- .../optimization-detective/site-health.php | 6 +++- .../tests/test-site-health.php | 32 ++++++++++++++----- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 9e2b23dc62..040918a56c 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -143,6 +143,10 @@ function od_compose_site_health_result( $response ): array { array( 'code' => array() ) ) . '

'; + if ( isset( $data['message'] ) && is_string( $data['message'] ) ) { + $result['description'] .= '
' . esc_html( $data['message'] ) . '
'; + } + $result['description'] .= '
' . esc_html__( 'Raw response:', 'optimization-detective' ) . '
' . esc_html( $body ) . '
'; } } @@ -221,7 +225,7 @@ function od_maybe_render_rest_api_health_check_admin_notice( bool $in_plugin_row esc_html( $result['label'] ) ); - $message .= wp_kses( $result['description'], array_fill_keys( array( 'p', 'code' ), array() ) ); + $message .= $result['description']; // This has already gone through Kses. if ( current_user_can( 'view_site_health_checks' ) ) { $site_health_message = wp_kses( diff --git a/plugins/optimization-detective/tests/test-site-health.php b/plugins/optimization-detective/tests/test-site-health.php index f262d33dbf..85584be784 100644 --- a/plugins/optimization-detective/tests/test-site-health.php +++ b/plugins/optimization-detective/tests/test-site-health.php @@ -30,11 +30,19 @@ class Test_OD_REST_API_Site_Health_Check extends WP_UnitTestCase { const UNAUTHORISED_MOCKED_RESPONSE_ARGS = array( 401, 'Unauthorized', + array( + 'code' => 'unauthorized_without_message', + ), ); const FORBIDDEN_MOCKED_RESPONSE_ARGS = array( 403, 'Forbidden', + array( + 'code' => 'rest_login_required', + 'message' => 'REST API restricted to authenticated users.', + 'data' => array( 'status' => 401 ), + ), ); /** @@ -88,25 +96,37 @@ public function test_rest_api_assumed_accessible(): void { */ public function data_provider_test_rest_api_availability(): array { return array( - 'available' => array( + 'available' => array( 'mocked_response' => $this->build_mock_response( ...self::EXPECTED_MOCKED_RESPONSE_ARGS ), 'expected_option' => '0', 'expected_status' => 'good', 'expected_unavailable' => false, ), - 'unauthorized' => array( + 'unauthorized' => array( 'mocked_response' => $this->build_mock_response( ...self::UNAUTHORISED_MOCKED_RESPONSE_ARGS ), 'expected_option' => '1', 'expected_status' => 'critical', 'expected_unavailable' => true, ), - 'forbidden' => array( + 'forbidden' => array( 'mocked_response' => $this->build_mock_response( ...self::FORBIDDEN_MOCKED_RESPONSE_ARGS ), 'expected_option' => '1', 'expected_status' => 'critical', 'expected_unavailable' => true, ), - 'error' => array( + 'nginx_forbidden' => array( + 'mocked_response' => array( + 'response' => array( + 'code' => 403, + 'message' => 'Forbidden', + ), + 'body' => "\n403 Forbidden\n\n

403 Forbidden

\n
nginx
\n\n", + ), + 'expected_option' => '1', + 'expected_status' => 'critical', + 'expected_unavailable' => true, + ), + 'error' => array( 'mocked_response' => new WP_Error( 'bad', 'Something terrible has happened' ), 'expected_option' => '1', 'expected_status' => 'critical', @@ -198,12 +218,8 @@ public function test_od_maybe_render_rest_api_health_check_admin_notice_rest_api $notice = get_echo( 'od_maybe_render_rest_api_health_check_admin_notice', array( $in_plugin_row ) ); $this->assertStringContainsString( '
', $admin_notices_output ); + } else { + $this->assertStringNotContainsString( '
', $admin_notices_output ); + } } /** * Filters REST API response with mock. * * @param array|WP_Error $mocked_response Mocked response. + * @return object{ counter: int } Value which contains a counter for the number of times the filter applied. */ - protected function filter_rest_api_response( $mocked_response ): void { + protected function filter_rest_api_response( $mocked_response ): object { + $observer = (object) array( 'counter' => 0 ); remove_all_filters( 'pre_http_request' ); add_filter( 'pre_http_request', - static function ( $pre, array $args, string $url ) use ( $mocked_response ) { + static function ( $pre, array $args, string $url ) use ( $mocked_response, $observer ) { if ( rest_url( OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ) === $url ) { + $observer->counter++; return $mocked_response; } return $pre; @@ -325,6 +388,7 @@ static function ( $pre, array $args, string $url ) use ( $mocked_response ) { 10, 3 ); + return $observer; } /** From ddc6164d72ff7ceee7964cea3310cc6f403ab8b3 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 17 Jan 2025 09:59:23 -0800 Subject: [PATCH 60/63] Remove checking for specific missing required params Co-authored-by: felixarntz --- plugins/optimization-detective/site-health.php | 5 ++--- plugins/optimization-detective/storage/rest-api.php | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 478f659706..11707cdcca 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -126,13 +126,12 @@ function od_compose_site_health_result( $response ): array { $body = wp_remote_retrieve_body( $response ); $data = json_decode( $body, true ); - $required_args = array( 'slug', 'current_etag', 'hmac', 'url', 'viewport', 'elements' ); - $is_expected = ( + $is_expected = ( 400 === $code && isset( $data['code'], $data['data']['params'] ) && 'rest_missing_callback_param' === $data['code'] && is_array( $data['data']['params'] ) && - count( array_intersect( $data['data']['params'], $required_args ) ) === count( $required_args ) + count( $data['data']['params'] ) > 0 ); if ( ! $is_expected ) { $result['status'] = 'critical'; diff --git a/plugins/optimization-detective/storage/rest-api.php b/plugins/optimization-detective/storage/rest-api.php index 1b182b0433..ce278a9f57 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -41,7 +41,6 @@ function od_register_endpoint(): void { // The slug and cache_purge_post_id args are further validated via the validate_callback for the 'hmac' parameter, // they are provided as input with the 'url' argument to create the HMAC by the server. - // The following args are referenced in od_compose_site_health_result(). $args = array( 'slug' => array( 'type' => 'string', From b4736a40553deb6114ed96c28d94759cb38b1076 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 17 Jan 2025 10:03:36 -0800 Subject: [PATCH 61/63] Remove overkill type checking for transient response Co-authored-by: felixarntz --- plugins/optimization-detective/site-health.php | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 11707cdcca..3a989816c4 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -167,22 +167,8 @@ function od_compose_site_health_result( $response ): array { */ function od_get_rest_api_health_check_response( bool $use_cached ) { $transient_key = 'od_rest_api_health_check_response'; - $response = $use_cached ? get_transient( $transient_key ) : null; - if ( - ( - is_array( $response ) - && - isset( $response['response']['code'], $response['response']['message'], $response['body'] ) - && - is_int( $response['response']['code'] ) - && - is_string( $response['response']['message'] ) - && - is_string( $response['body'] ) - ) - || - is_wp_error( $response ) - ) { + $response = $use_cached ? get_transient( $transient_key ) : false; + if ( false !== $response ) { return $response; } $rest_url = get_rest_url( null, OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ); From 02953c79d49427679b1e5736667b624b96f08ad9 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 17 Jan 2025 10:16:30 -0800 Subject: [PATCH 62/63] Change Site Health test from critical back to recommended Co-authored-by: felixarntz --- plugins/optimization-detective/site-health.php | 10 +++++++--- .../optimization-detective/tests/test-site-health.php | 8 ++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index 3a989816c4..f1ef2617a5 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -110,7 +110,7 @@ function od_compose_site_health_result( $response ): array { $error_description_html = '

' . esc_html__( 'You may have a plugin active or server configuration which restricts access to logged-in users. Unauthenticated access must be restored in order for Optimization Detective to work.', 'optimization-detective' ) . '

'; if ( is_wp_error( $response ) ) { - $result['status'] = 'critical'; + $result['status'] = 'recommended'; $result['label'] = $error_label; $result['description'] = $common_description_html . $error_description_html . '

' . wp_kses( sprintf( @@ -134,8 +134,12 @@ function od_compose_site_health_result( $response ): array { count( $data['data']['params'] ) > 0 ); if ( ! $is_expected ) { - $result['status'] = 'critical'; - $result['label'] = __( 'The Optimization Detective REST API endpoint is unavailable to logged-out users', 'optimization-detective' ); + $result['status'] = 'recommended'; + if ( 401 === $code ) { + $result['label'] = __( 'The Optimization Detective REST API endpoint is unavailable to logged-out users', 'optimization-detective' ); + } else { + $result['label'] = $error_label; + } $result['description'] = $common_description_html . $error_description_html . '

' . wp_kses( sprintf( /* translators: %d is the HTTP status code, %s is the status header description */ diff --git a/plugins/optimization-detective/tests/test-site-health.php b/plugins/optimization-detective/tests/test-site-health.php index ff105ff18c..8ca97fd51f 100644 --- a/plugins/optimization-detective/tests/test-site-health.php +++ b/plugins/optimization-detective/tests/test-site-health.php @@ -105,13 +105,13 @@ public function data_provider_test_rest_api_availability(): array { 'unauthorized' => array( 'mocked_response' => $this->build_mock_response( ...self::UNAUTHORISED_MOCKED_RESPONSE_ARGS ), 'expected_option' => '1', - 'expected_status' => 'critical', + 'expected_status' => 'recommended', 'expected_unavailable' => true, ), 'forbidden' => array( 'mocked_response' => $this->build_mock_response( ...self::FORBIDDEN_MOCKED_RESPONSE_ARGS ), 'expected_option' => '1', - 'expected_status' => 'critical', + 'expected_status' => 'recommended', 'expected_unavailable' => true, ), 'nginx_forbidden' => array( @@ -123,13 +123,13 @@ public function data_provider_test_rest_api_availability(): array { 'body' => "\n403 Forbidden\n\n

403 Forbidden

\n
nginx
\n\n", ), 'expected_option' => '1', - 'expected_status' => 'critical', + 'expected_status' => 'recommended', 'expected_unavailable' => true, ), 'error' => array( 'mocked_response' => new WP_Error( 'bad', 'Something terrible has happened' ), 'expected_option' => '1', - 'expected_status' => 'critical', + 'expected_status' => 'recommended', 'expected_unavailable' => true, ), ); From 14e947195391fa5f848802eca88fe9af1b5611a8 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 17 Jan 2025 14:42:00 -0800 Subject: [PATCH 63/63] Use definite article instead of possessive Co-authored-by: Felix Arntz --- plugins/optimization-detective/site-health.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/optimization-detective/site-health.php b/plugins/optimization-detective/site-health.php index f1ef2617a5..13da11d595 100644 --- a/plugins/optimization-detective/site-health.php +++ b/plugins/optimization-detective/site-health.php @@ -95,7 +95,7 @@ function od_compose_site_health_result( $response ): array { ) . '

'; $result = array( - 'label' => __( 'Optimization Detective\'s REST API endpoint is available', 'optimization-detective' ), + 'label' => __( 'The Optimization Detective REST API endpoint is available', 'optimization-detective' ), 'status' => 'good', 'badge' => array( 'label' => __( 'Optimization Detective', 'optimization-detective' ), @@ -106,7 +106,7 @@ function od_compose_site_health_result( $response ): array { 'test' => 'optimization_detective_rest_api', ); - $error_label = __( 'Optimization Detective\'s REST API endpoint is unavailable', 'optimization-detective' ); + $error_label = __( 'The Optimization Detective REST API endpoint is unavailable', 'optimization-detective' ); $error_description_html = '

' . esc_html__( 'You may have a plugin active or server configuration which restricts access to logged-in users. Unauthenticated access must be restored in order for Optimization Detective to work.', 'optimization-detective' ) . '

'; if ( is_wp_error( $response ) ) {