From 8806d95678f0b135f30d79d521d48a63a379c552 Mon Sep 17 00:00:00 2001 From: Chris Reynolds Date: Mon, 20 May 2024 15:38:57 -0600 Subject: [PATCH] [CMSP-995] one more notice (#271) * add a global admin notice that appears once * remove seconds from the notice * test the new notice * set dismissable depending on which notice we're showing * show the max age rank html comment * fix the conditionals for cs * update behat tests for new notice language * bugfix: don't update user meta that max_age was updated if it hasn't been * add one more thing to test in behat * add a test for the error, specifically * add a test for the warning * simplify the conditionals * lint * specify the pages that we're on? * remove specific element * fix typo * take out the dashboard main page goto * validate the setting was saved * let's just remove the 5 days notice * just remove the test we have similar things being tested in phpunit * we can actually remove current_max_age >= WEEK_IN_SECONDS this is covered by max_age_rank. if max_age_rank is 0 it, by definition, means that the current max age is >= WEEK_IN_SECONDS * if we're only comparing two things, we don't need to break the if clauses up * we do want to warn users if they have manually set their max age to 600 we just don't want to warn them if we're about to (or just did) change it to a week for them --- inc/admin-interface.php | 42 ++++++++++++++++++++------ tests/behat/admin-interface.feature | 8 +---- tests/phpunit/test-admin-interface.php | 35 +++++++++++++++++++++ 3 files changed, 69 insertions(+), 16 deletions(-) diff --git a/inc/admin-interface.php b/inc/admin-interface.php index 8e66f97a..96a8839b 100644 --- a/inc/admin-interface.php +++ b/inc/admin-interface.php @@ -111,31 +111,51 @@ function admin_notice_old_mu_plugin() { */ function admin_notice_maybe_recommend_higher_max_age() { $current_screen = get_current_screen(); + $global_warning_shown = current_user_can( 'manage_options' ) ? get_user_meta( get_current_user_id(), 'pantheon_max_age_global_warning_notice', true ) : true; - if ( apply_filters( 'pantheon_apc_disable_admin_notices', false, __FUNCTION__ ) || 'settings_page_pantheon-cache' !== $current_screen->id ) { + if ( apply_filters( 'pantheon_apc_disable_admin_notices', false, __FUNCTION__ ) || $global_warning_shown ) { return; } + $message = ''; + $dismissable = true; $max_age_rank = max_age_compare(); $current_max_age = get_current_max_age(); + + // Check if the max-age rank is acceptable or if current max-age is 600 seconds and we haven't yet reset it to the default. if ( - $max_age_rank > 0 && - $current_max_age < WEEK_IN_SECONDS && - $current_max_age !== 600 + $max_age_rank === 0 || + ( $current_max_age === 600 && ! get_option( 'pantheon_max_age_updated', false ) ) ) { - // If the current max-age value has a rank of 3 or more (10 is the highest), we'll note that it's very low. + return; + } + + if ( isset( $current_screen->id ) && 'settings_page_pantheon-cache' === $current_screen->id ) { + // If the current max-age value has a rank of 3 or more (10 is the highest), we'll note that it's very low. $very_low = $max_age_rank > 3 ? __( 'This is a very low value and may not be optimal for your site.', 'pantheon-advanced-page-cache' ) : ''; $message = sprintf( // translators: %1$s is the current max-age, %2$d is the current max-age in seconds, %3$s is a message that displays if the value is very low, %44d is the recommended max age in seconds, %5$s is the humanized recommended max age, %6$s is debug information that is written to the HTML DOM but not displayed. - __( 'The cache max-age is currently set to %1$s (%2$s seconds). %3$s Consider increasing the cache max-age to at least %4$d seconds (%5$s).%6$s', 'pantheon-advanced-page-cache' ), + __( 'The cache max-age is currently set to %1$s. %2$s Consider increasing the cache max-age to at least %3$s.%4$s', 'pantheon-advanced-page-cache' ), humanized_max_age(), - $current_max_age, $very_low, - WEEK_IN_SECONDS, humanized_max_age( true ), sprintf( '', $max_age_rank ) ); + } + // Global notice on all pages _except_ the Pantheon cache settings page. + if ( ! isset( $current_screen->id ) || 'settings_page_pantheon-cache' !== $current_screen->id ) { + $message = sprintf( + // translators: %s is a link to the Pantheon GCDN configuration page. + __( 'Your site\'s cache max-age is set below the recommendation (1 week). Visit the Pantheon GCDN configuration page to update the setting.%2$s' ), + admin_url( 'options-general.php?page=pantheon-cache' ), + sprintf( '', $max_age_rank ) + ); + $dismissable = false; + update_user_meta( get_current_user_id(), 'pantheon_max_age_global_warning_notice', true ); + } + + if ( ! empty( $message ) ) { // Escalating notice types based on the max-age rank. $notice_type = ( $max_age_rank === 1 ? 'info' : $max_age_rank > 3 ) ? 'error' : 'warning'; @@ -143,7 +163,7 @@ function admin_notice_maybe_recommend_higher_max_age() { $message, [ 'type' => $notice_type, - 'dismissible' => true, + 'dismissible' => $dismissable, ] ); } @@ -400,6 +420,10 @@ function max_age_updated_admin_notice() { ] ); + if ( ! $max_age_updated ) { + return; + } + // Update the user meta to prevent this notice from showing again after they've seen it once. update_user_meta( $current_user_id, 'pantheon_max_age_updated_notice', true ); } diff --git a/tests/behat/admin-interface.feature b/tests/behat/admin-interface.feature index 7221b623..efcec270 100644 --- a/tests/behat/admin-interface.feature +++ b/tests/behat/admin-interface.feature @@ -16,13 +16,7 @@ Scenario: Change the cache max age And I fill in "pantheon-cache[default_ttl]" with "300" And I press "Save Changes" Then I should see "This is a very low value and may not be optimal for your site" in the ".notice" element - And I should see "Consider increasing the cache max-age to at least 604800 seconds (1 week)" in the ".notice" element - -Scenario: Change the cache max age to 5 days - When I go to "/wp-admin/options-general.php?page=pantheon-cache" - And I fill in "pantheon-cache[default_ttl]" with "432000" - And I press "Save Changes" - Then I should see "Consider increasing the cache max-age to at least 604800 seconds (1 week)" in the ".notice" element + And I should see "Consider increasing the cache max-age to at least 1 week" in the ".notice" element Scenario: Change the cache max age to 1 week When I go to "/wp-admin/options-general.php?page=pantheon-cache" diff --git a/tests/phpunit/test-admin-interface.php b/tests/phpunit/test-admin-interface.php index 4f91f469..f8038ab7 100644 --- a/tests/phpunit/test-admin-interface.php +++ b/tests/phpunit/test-admin-interface.php @@ -271,4 +271,39 @@ function test_max_age_updated_admin_notice() { // The user meta should have been updated in the process. $this->assertEquals( 1, get_user_meta( $current_user_id, 'pantheon_max_age_updated_notice', true ) ); } + + /** + * Test that the user meta for the global admin notice is created. + */ + function test_low_max_age_admin_notice_user_meta() { + // Switch to admin. + wp_set_current_user( 1 ); + delete_user_meta( 1, 'pantheon_max_age_global_warning_notice' ); + ob_start(); + admin_notice_maybe_recommend_higher_max_age(); + $notice = ob_get_clean(); + + $notice_shown = get_user_meta( 1, 'pantheon_max_age_global_warning_notice', true ); + $this->assertStringContainsString( 'notice-error', $notice ); + $this->assertStringContainsString( 'Your site\'s cache max-age is set below the recommendation (1 week).' , $notice ); + $this->assertEquals( 1, $notice_shown ); + } + + /** + * Test that the user meta for the global admin notice is created. + */ + function test_low_max_age_admin_notice_user_meta_warning() { + // Switch to admin. + wp_set_current_user( 1 ); + delete_user_meta( 1, 'pantheon_max_age_global_warning_notice' ); + update_option( 'pantheon-cache', [ 'default_ttl' => 432000 ] ); + ob_start(); + admin_notice_maybe_recommend_higher_max_age(); + $notice = ob_get_clean(); + + $notice_shown = get_user_meta( 1, 'pantheon_max_age_global_warning_notice', true ); + $this->assertStringContainsString( 'notice-warning', $notice ); + $this->assertStringContainsString( 'Your site\'s cache max-age is set below the recommendation (1 week).' , $notice ); + $this->assertEquals( 1, $notice_shown ); + } }