diff --git a/assets/src/amp-validation/counts/index.js b/assets/src/amp-validation/counts/index.js index 2a3f2a7dd3f..e571a770bd9 100644 --- a/assets/src/amp-validation/counts/index.js +++ b/assets/src/amp-validation/counts/index.js @@ -11,21 +11,73 @@ import domReady from '@wordpress/dom-ready'; import './style.css'; /** - * Updates a menu item with its count. + * Get session storage key for storing the previously-fetched count. * - * If the count is not a number or is `0`, the element that contains the count is instead removed (as it would be no longer relevant). + * @param {string} itemId Menu item ID. + * @return {string} Session storage key. + */ +function getSessionStorageKey( itemId ) { + return `${ itemId }-last-count`; +} + +/** + * Sets the loading state on a menu item. + * + * @param {string} itemId Menu item ID. + */ +function setMenuItemIsLoading( itemId ) { + const itemEl = document.getElementById( itemId ); + + if ( ! itemEl || itemEl.querySelector( '.amp-count-loading' ) ) { + return; + } + + // Add a loading spinner if we haven't obtained the count before or the last count was not zero. + const lastCount = sessionStorage.getItem( getSessionStorageKey( itemId ) ); + if ( ! lastCount || '0' !== lastCount ) { + const loadingEl = document.createElement( 'span' ); + loadingEl.classList.add( 'amp-count-loading' ); + itemEl.append( loadingEl ); + + itemEl.classList.add( 'awaiting-mod' ); + } +} + +/** + * Sets a menu item count value. + * + * If the count is not a number or is `0`, the element that contains the count is instead removed (as it would be no + * longer relevant). * - * @param {HTMLElement} itemEl Menu item element. - * @param {number} count Count to set. + * @param {string} itemId Menu item ID. + * @param {number} count Count to set. */ -function updateMenuItem( itemEl, count ) { +function setMenuItemCountValue( itemId, count ) { + const itemEl = document.getElementById( itemId ); + + if ( ! itemEl ) { + return; + } + if ( isNaN( count ) || count === 0 ) { itemEl.parentNode.removeChild( itemEl ); + sessionStorage.setItem( getSessionStorageKey( itemId ), '0' ); } else { - itemEl.textContent = count.toLocaleString(); + const text = count.toLocaleString(); + itemEl.textContent = text; + itemEl.classList.add( 'awaiting-mod' ); // In case it wasn't set in setMenuItemIsLoading(). + sessionStorage.setItem( getSessionStorageKey( itemId ), text ); } } +/** + * Initializes the 'Validated URLs' and 'Error Index' menu items. + */ +function initializeMenuItemCounts() { + setMenuItemIsLoading( 'amp-new-error-index-count' ); + setMenuItemIsLoading( 'amp-new-validation-url-count' ); +} + /** * Updates the 'Validated URLs' and 'Error Index' menu items with their respective unreviewed count. * @@ -36,15 +88,23 @@ function updateMenuItem( itemEl, count ) { function updateMenuItemCounts( counts ) { const { validated_urls: newValidatedUrlCount, errors: newErrorCount } = counts; - const errorCountEl = document.getElementById( 'new-error-index-count' ); - if ( errorCountEl ) { - updateMenuItem( errorCountEl, newErrorCount ); - } + setMenuItemCountValue( 'amp-new-error-index-count', newErrorCount ); + setMenuItemCountValue( 'amp-new-validation-url-count', newValidatedUrlCount ); +} - const validatedUrlsCountEl = document.getElementById( 'new-validation-url-count' ); - if ( validatedUrlsCountEl ) { - updateMenuItem( validatedUrlsCountEl, newValidatedUrlCount ); - } +/** + * Requests validation counts. + */ +function fetchValidationCounts() { + apiFetch( { path: '/amp/v1/unreviewed-validation-counts' } ).then( ( counts ) => { + updateMenuItemCounts( counts ); + } ).catch( ( error ) => { + updateMenuItemCounts( { validated_urls: 0, errors: 0 } ); + + const message = error?.message || __( 'An unknown error occurred while retrieving the validation counts', 'amp' ); + // eslint-disable-next-line no-console + console.error( `[AMP Plugin] ${ message }` ); + } ); } /** @@ -68,15 +128,7 @@ function createObserver( root ) { observer.unobserve( target ); - apiFetch( { path: '/amp/v1/unreviewed-validation-counts' } ).then( ( counts ) => { - updateMenuItemCounts( counts ); - } ).catch( ( error ) => { - updateMenuItemCounts( { validated_urls: 0, errors: 0 } ); - - const message = error?.message || __( 'An unknown error occurred while retrieving the validation counts', 'amp' ); - // eslint-disable-next-line no-console - console.error( `[AMP Plugin] ${ message }` ); - } ); + fetchValidationCounts(); }, { root } ); observer.observe( target ); @@ -90,5 +142,13 @@ domReady( () => { return; } - createObserver( ampMenuItem ); + initializeMenuItemCounts(); + + // If the AMP submenu is opened, fetch validation counts as soon as possible. Thanks to the preload middleware for + // `wp.apiFetch`, the validation count data should be available right away, so no actual HTTP request will be made. + if ( ampMenuItem.classList.contains( 'wp-menu-open' ) ) { + fetchValidationCounts(); + } else { + createObserver( ampMenuItem ); + } } ); diff --git a/assets/src/amp-validation/counts/style.css b/assets/src/amp-validation/counts/style.css index 2d0f708215b..f7d613c737a 100644 --- a/assets/src/amp-validation/counts/style.css +++ b/assets/src/amp-validation/counts/style.css @@ -23,7 +23,7 @@ display: inline-block; } -body.no-js #new-validation-url-count, -body.no-js #new-error-index-count { +body.no-js #amp-new-validation-url-count, +body.no-js #amp-new-error-index-count { display: none; } diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 18bb7a08c18..e7b827555a0 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -469,7 +469,7 @@ public static function update_validated_url_menu_item() { if ( ValidationCounts::is_needed() ) { // Append markup to display a loading spinner while the unreviewed count is being fetched. - $submenu_item[0] .= ' '; + $submenu_item[0] .= ' '; } break; diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 19170e701f5..da47ae73238 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -1745,7 +1745,7 @@ public static function add_admin_menu_validation_error_item() { if ( ValidationCounts::is_needed() ) { // Append markup to display a loading spinner while the unreviewed count is being fetched. - $menu_item_label .= ' '; + $menu_item_label .= ' '; } $post_menu_slug = 'edit.php?post_type=' . AMP_Validated_URL_Post_Type::POST_TYPE_SLUG; diff --git a/src/Admin/ValidationCounts.php b/src/Admin/ValidationCounts.php index 78585a88f28..3825b84ef87 100644 --- a/src/Admin/ValidationCounts.php +++ b/src/Admin/ValidationCounts.php @@ -1,12 +1,14 @@ rest_preloader = $rest_preloader; + } + /** * Get the action to use for registering the service. * @@ -93,5 +111,23 @@ public function enqueue_scripts() { false, $version ); + + $this->maybe_add_preload_rest_paths(); + } + + /** + * Adds REST paths to preload. + * + * Preload validation counts data on an admin screen that has the AMP Options page as a parent or on any admin + * screen related to `amp_validation_error` post type (which includes the `amp_validation_error` taxonomy). + */ + protected function maybe_add_preload_rest_paths() { + if ( + AMP_Options_Manager::OPTION_NAME === get_admin_page_parent() + || + AMP_Validated_URL_Post_Type::POST_TYPE_SLUG === get_current_screen()->post_type + ) { + $this->rest_preloader->add_preloaded_path( '/amp/v1/unreviewed-validation-counts' ); + } } } diff --git a/tests/php/src/Admin/ValidationCountsTest.php b/tests/php/src/Admin/ValidationCountsTest.php index b02b57b9862..565aa1cd6ae 100644 --- a/tests/php/src/Admin/ValidationCountsTest.php +++ b/tests/php/src/Admin/ValidationCountsTest.php @@ -10,6 +10,7 @@ use AMP_Options_Manager; use AMP_Theme_Support; use AMP_Validated_URL_Post_Type; +use AmpProject\AmpWP\Admin\RESTPreloader; use AmpProject\AmpWP\Admin\ValidationCounts; use AmpProject\AmpWP\DevTools\UserAccess; use AmpProject\AmpWP\Infrastructure\Conditional; @@ -19,6 +20,7 @@ use AmpProject\AmpWP\Infrastructure\Service; use AmpProject\AmpWP\Option; use AmpProject\AmpWP\Services; +use AmpProject\AmpWP\Tests\Helpers\PrivateAccess; use AmpProject\AmpWP\Tests\TestCase; /** @@ -28,6 +30,8 @@ */ class ValidationCountsTest extends TestCase { + use PrivateAccess; + /** * Test instance. * @@ -43,9 +47,13 @@ class ValidationCountsTest extends TestCase { public function setUp() { parent::setUp(); - $this->instance = new ValidationCounts(); + set_current_screen( 'edit' ); + get_current_screen()->post_type = 'post'; + + $this->instance = new ValidationCounts( new RESTPreloader() ); } + /** @covers ::__construct() */ public function test__construct() { $this->assertInstanceOf( ValidationCounts::class, $this->instance ); $this->assertInstanceOf( Delayed::class, $this->instance ); @@ -132,4 +140,74 @@ public function test_is_needed() { delete_user_meta( $admin_user->ID, UserAccess::USER_FIELD_DEVELOPER_TOOLS_ENABLED ); $this->assertTrue( ValidationCounts::is_needed() ); } + + /** @return array */ + public function maybe_add_preload_rest_paths_data() { + return [ + 'no_type_post' => [ + 'set_up' => static function () { + set_current_screen( 'user' ); + }, + 'should_preload_path' => false, + ], + 'post_type_post' => [ + 'set_up' => static function () { + set_current_screen( 'edit' ); + get_current_screen()->post_type = 'post'; + }, + 'should_preload_path' => false, + ], + 'post_type_page' => [ + 'set_up' => static function () { + set_current_screen( 'edit' ); + get_current_screen()->post_type = 'page'; + }, + 'should_preload_path' => false, + ], + 'post_type_amp_validated_url' => [ + 'set_up' => static function () { + set_current_screen( 'edit' ); + get_current_screen()->post_type = AMP_Validated_URL_Post_Type::POST_TYPE_SLUG; + }, + 'should_preload_path' => true, + ], + 'settings_screen' => [ + 'set_up' => function () { + global $pagenow, $plugin_page, $menu; + $pagenow = 'admin.php'; + $plugin_page = 'amp-options'; + $menu = [ + [ + 2 => $plugin_page, + ], + ]; + $this->assertEquals( AMP_Options_Manager::OPTION_NAME, get_admin_page_parent() ); + }, + 'should_preload_path' => true, + ], + ]; + } + + /** + * @dataProvider maybe_add_preload_rest_paths_data + * @covers ::maybe_add_preload_rest_paths() + */ + public function test_maybe_add_preload_rest_paths( callable $set_up, $should_preload_path ) { + if ( ! function_exists( 'rest_preload_api_request' ) ) { + $this->markTestIncomplete( 'REST preload is not available so skipping.' ); + } + + $set_up(); + + $this->call_private_method( $this->instance, 'maybe_add_preload_rest_paths' ); + + $rest_preloader = $this->get_private_property( $this->instance, 'rest_preloader' ); + $paths = $this->get_private_property( $rest_preloader, 'paths' ); + + if ( $should_preload_path ) { + $this->assertContains( '/amp/v1/unreviewed-validation-counts', $paths ); + } else { + $this->assertNotContains( '/amp/v1/unreviewed-validation-counts', $paths ); + } + } } diff --git a/tests/php/validation/test-class-amp-validated-url-post-type.php b/tests/php/validation/test-class-amp-validated-url-post-type.php index 0c281720446..7f3ff57f2fc 100644 --- a/tests/php/validation/test-class-amp-validated-url-post-type.php +++ b/tests/php/validation/test-class-amp-validated-url-post-type.php @@ -170,7 +170,7 @@ public function test_update_validated_url_menu_item() { AMP_Validated_URL_Post_Type::update_validated_url_menu_item(); if ( Services::get( 'dependency_support' )->has_support() ) { - $this->assertSame( 'Validated URLs ', $submenu[ AMP_Options_Manager::OPTION_NAME ][2][0] ); + $this->assertSame( 'Validated URLs ', $submenu[ AMP_Options_Manager::OPTION_NAME ][2][0] ); } else { $this->assertSame( 'Validated URLs', $submenu[ AMP_Options_Manager::OPTION_NAME ][2][0] ); } diff --git a/tests/php/validation/test-class-amp-validation-error-taxonomy.php b/tests/php/validation/test-class-amp-validation-error-taxonomy.php index 2e4c5d6e2b5..cdd652f2346 100644 --- a/tests/php/validation/test-class-amp-validation-error-taxonomy.php +++ b/tests/php/validation/test-class-amp-validation-error-taxonomy.php @@ -1034,8 +1034,8 @@ public function test_add_admin_menu_validation_error_item() { 'Error Index', ]; if ( Services::get( 'dependency_support' )->has_support() ) { - $expected_submenu[0] .= ' '; - $expected_submenu[3] .= ' '; + $expected_submenu[0] .= ' '; + $expected_submenu[3] .= ' '; } $amp_options = $submenu[ AMP_Options_Manager::OPTION_NAME ]; $this->assertEquals( $expected_submenu, end( $amp_options ) );