Merge pull request #6770 from ampproject/update/admin-validation-counts

Preload validation counts data if AMP submenu is expanded

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 );
+	}
 } );

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;
 }

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] .= ' <span id="new-validation-url-count" class="awaiting-mod"><span class="amp-count-loading"></span></span>';
+					$submenu_item[0] .= ' <span id="amp-new-validation-url-count"></span>';
 				}
 
 				break;

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 .= ' <span id="new-error-index-count" class="awaiting-mod"><span class="amp-count-loading"></span></span>';
+			$menu_item_label .= ' <span id="amp-new-error-index-count"></span>';
 		}
 
 		$post_menu_slug = 'edit.php?post_type=' . AMP_Validated_URL_Post_Type::POST_TYPE_SLUG;

src/Admin/ValidationCounts.php

@@ -1,12 +1,14 @@
 <?php
 /**
- * Class ValidatedUrlCounts.
+ * Class ValidationCounts.
  *
  * @package AmpProject\AmpWP
  */
 
 namespace AmpProject\AmpWP\Admin;
 
+use AMP_Options_Manager;
+use AMP_Validated_URL_Post_Type;
 use AmpProject\AmpWP\Infrastructure\Conditional;
 use AmpProject\AmpWP\Infrastructure\Delayed;
 use AmpProject\AmpWP\Infrastructure\HasRequirements;
@@ -30,6 +32,22 @@ final class ValidationCounts implements Service, Registerable, Conditional, Dela
 	 */
 	const ASSETS_HANDLE = 'amp-validation-counts';
 
+	/**
+	 * RESTPreloader instance.
+	 *
+	 * @var RESTPreloader
+	 */
+	private $rest_preloader;
+
+	/**
+	 * ValidationCounts constructor.
+	 *
+	 * @param RESTPreloader $rest_preloader An instance of the RESTPreloader class.
+	 */
+	public function __construct( RESTPreloader $rest_preloader ) {
+		$this->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' );
+		}
 	}
 }

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 );
+		}
+	}
 }

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 <span id="new-validation-url-count" class="awaiting-mod"><span class="amp-count-loading"></span></span>', $submenu[ AMP_Options_Manager::OPTION_NAME ][2][0] );
+			$this->assertSame( 'Validated URLs <span id="amp-new-validation-url-count"></span>', $submenu[ AMP_Options_Manager::OPTION_NAME ][2][0] );
 		} else {
 			$this->assertSame( 'Validated URLs', $submenu[ AMP_Options_Manager::OPTION_NAME ][2][0] );
 		}

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] .= ' <span id="new-error-index-count" class="awaiting-mod"><span class="amp-count-loading"></span></span>';
-			$expected_submenu[3] .= ' <span id="new-error-index-count" class="awaiting-mod"><span class="amp-count-loading"></span></span>';
+			$expected_submenu[0] .= ' <span id="amp-new-error-index-count"></span>';
+			$expected_submenu[3] .= ' <span id="amp-new-error-index-count"></span>';
 		}
 		$amp_options = $submenu[ AMP_Options_Manager::OPTION_NAME ];
 		$this->assertEquals( $expected_submenu, end( $amp_options ) );