Skip to content

Commit 2fca9b9

Browse files
westonruterwestonruter
authored
Merge pull request #6770 from ampproject/update/admin-validation-counts
Preload validation counts data if AMP submenu is expanded
2 parents 6cf0154 + 7981cb0 commit 2fca9b9

File tree

8 files changed

+207
-33
lines changed

8 files changed

+207
-33
lines changed

‎assets/src/amp-validation/counts/index.js‎

Lines changed: 84 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,73 @@ import domReady from '@wordpress/dom-ready';
1111
import './style.css';
1212

1313
/**
14-
* Updates a menu item with its count.
14+
* Get session storage key for storing the previously-fetched count.
1515
*
16-
* 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).
16+
* @param {string} itemId Menu item ID.
17+
* @return {string} Session storage key.
18+
*/
19+
function getSessionStorageKey( itemId ) {
20+
return `${ itemId }-last-count`;
21+
}
22+
23+
/**
24+
* Sets the loading state on a menu item.
25+
*
26+
* @param {string} itemId Menu item ID.
27+
*/
28+
function setMenuItemIsLoading( itemId ) {
29+
const itemEl = document.getElementById( itemId );
30+
31+
if ( ! itemEl || itemEl.querySelector( '.amp-count-loading' ) ) {
32+
return;
33+
}
34+
35+
// Add a loading spinner if we haven't obtained the count before or the last count was not zero.
36+
const lastCount = sessionStorage.getItem( getSessionStorageKey( itemId ) );
37+
if ( ! lastCount || '0' !== lastCount ) {
38+
const loadingEl = document.createElement( 'span' );
39+
loadingEl.classList.add( 'amp-count-loading' );
40+
itemEl.append( loadingEl );
41+
42+
itemEl.classList.add( 'awaiting-mod' );
43+
}
44+
}
45+
46+
/**
47+
* Sets a menu item count value.
48+
*
49+
* If the count is not a number or is `0`, the element that contains the count is instead removed (as it would be no
50+
* longer relevant).
1751
*
18-
* @param {HTMLElement} itemEl Menu item element.
19-
* @param {number} count Count to set.
52+
* @param {string} itemId Menu item ID.
53+
* @param {number} count Count to set.
2054
*/
21-
function updateMenuItem( itemEl, count ) {
55+
function setMenuItemCountValue( itemId, count ) {
56+
const itemEl = document.getElementById( itemId );
57+
58+
if ( ! itemEl ) {
59+
return;
60+
}
61+
2262
if ( isNaN( count ) || count === 0 ) {
2363
itemEl.parentNode.removeChild( itemEl );
64+
sessionStorage.setItem( getSessionStorageKey( itemId ), '0' );
2465
} else {
25-
itemEl.textContent = count.toLocaleString();
66+
const text = count.toLocaleString();
67+
itemEl.textContent = text;
68+
itemEl.classList.add( 'awaiting-mod' ); // In case it wasn't set in setMenuItemIsLoading().
69+
sessionStorage.setItem( getSessionStorageKey( itemId ), text );
2670
}
2771
}
2872

73+
/**
74+
* Initializes the 'Validated URLs' and 'Error Index' menu items.
75+
*/
76+
function initializeMenuItemCounts() {
77+
setMenuItemIsLoading( 'amp-new-error-index-count' );
78+
setMenuItemIsLoading( 'amp-new-validation-url-count' );
79+
}
80+
2981
/**
3082
* Updates the 'Validated URLs' and 'Error Index' menu items with their respective unreviewed count.
3183
*
@@ -36,15 +88,23 @@ function updateMenuItem( itemEl, count ) {
3688
function updateMenuItemCounts( counts ) {
3789
const { validated_urls: newValidatedUrlCount, errors: newErrorCount } = counts;
3890

39-
const errorCountEl = document.getElementById( 'new-error-index-count' );
40-
if ( errorCountEl ) {
41-
updateMenuItem( errorCountEl, newErrorCount );
42-
}
91+
setMenuItemCountValue( 'amp-new-error-index-count', newErrorCount );
92+
setMenuItemCountValue( 'amp-new-validation-url-count', newValidatedUrlCount );
93+
}
4394

44-
const validatedUrlsCountEl = document.getElementById( 'new-validation-url-count' );
45-
if ( validatedUrlsCountEl ) {
46-
updateMenuItem( validatedUrlsCountEl, newValidatedUrlCount );
47-
}
95+
/**
96+
* Requests validation counts.
97+
*/
98+
function fetchValidationCounts() {
99+
apiFetch( { path: '/amp/v1/unreviewed-validation-counts' } ).then( ( counts ) => {
100+
updateMenuItemCounts( counts );
101+
} ).catch( ( error ) => {
102+
updateMenuItemCounts( { validated_urls: 0, errors: 0 } );
103+
104+
const message = error?.message || __( 'An unknown error occurred while retrieving the validation counts', 'amp' );
105+
// eslint-disable-next-line no-console
106+
console.error( `[AMP Plugin] ${ message }` );
107+
} );
48108
}
49109

50110
/**
@@ -68,15 +128,7 @@ function createObserver( root ) {
68128

69129
observer.unobserve( target );
70130

71-
apiFetch( { path: '/amp/v1/unreviewed-validation-counts' } ).then( ( counts ) => {
72-
updateMenuItemCounts( counts );
73-
} ).catch( ( error ) => {
74-
updateMenuItemCounts( { validated_urls: 0, errors: 0 } );
75-
76-
const message = error?.message || __( 'An unknown error occurred while retrieving the validation counts', 'amp' );
77-
// eslint-disable-next-line no-console
78-
console.error( `[AMP Plugin] ${ message }` );
79-
} );
131+
fetchValidationCounts();
80132
}, { root } );
81133

82134
observer.observe( target );
@@ -90,5 +142,13 @@ domReady( () => {
90142
return;
91143
}
92144

93-
createObserver( ampMenuItem );
145+
initializeMenuItemCounts();
146+
147+
// If the AMP submenu is opened, fetch validation counts as soon as possible. Thanks to the preload middleware for
148+
// `wp.apiFetch`, the validation count data should be available right away, so no actual HTTP request will be made.
149+
if ( ampMenuItem.classList.contains( 'wp-menu-open' ) ) {
150+
fetchValidationCounts();
151+
} else {
152+
createObserver( ampMenuItem );
153+
}
94154
} );

‎assets/src/amp-validation/counts/style.css‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
display: inline-block;
2424
}
2525

26-
body.no-js #new-validation-url-count,
27-
body.no-js #new-error-index-count {
26+
body.no-js #amp-new-validation-url-count,
27+
body.no-js #amp-new-error-index-count {
2828
display: none;
2929
}

‎includes/validation/class-amp-validated-url-post-type.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ public static function update_validated_url_menu_item() {
469469

470470
if ( ValidationCounts::is_needed() ) {
471471
// Append markup to display a loading spinner while the unreviewed count is being fetched.
472-
$submenu_item[0] .= ' <span id="new-validation-url-count" class="awaiting-mod"><span class="amp-count-loading"></span></span>';
472+
$submenu_item[0] .= ' <span id="amp-new-validation-url-count"></span>';
473473
}
474474

475475
break;

‎includes/validation/class-amp-validation-error-taxonomy.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1745,7 +1745,7 @@ public static function add_admin_menu_validation_error_item() {
17451745

17461746
if ( ValidationCounts::is_needed() ) {
17471747
// Append markup to display a loading spinner while the unreviewed count is being fetched.
1748-
$menu_item_label .= ' <span id="new-error-index-count" class="awaiting-mod"><span class="amp-count-loading"></span></span>';
1748+
$menu_item_label .= ' <span id="amp-new-error-index-count"></span>';
17491749
}
17501750

17511751
$post_menu_slug = 'edit.php?post_type=' . AMP_Validated_URL_Post_Type::POST_TYPE_SLUG;

‎src/Admin/ValidationCounts.php‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
<?php
22
/**
3-
* Class ValidatedUrlCounts.
3+
* Class ValidationCounts.
44
*
55
* @package AmpProject\AmpWP
66
*/
77

88
namespace AmpProject\AmpWP\Admin;
99

10+
use AMP_Options_Manager;
11+
use AMP_Validated_URL_Post_Type;
1012
use AmpProject\AmpWP\Infrastructure\Conditional;
1113
use AmpProject\AmpWP\Infrastructure\Delayed;
1214
use AmpProject\AmpWP\Infrastructure\HasRequirements;
@@ -30,6 +32,22 @@ final class ValidationCounts implements Service, Registerable, Conditional, Dela
3032
*/
3133
const ASSETS_HANDLE = 'amp-validation-counts';
3234

35+
/**
36+
* RESTPreloader instance.
37+
*
38+
* @var RESTPreloader
39+
*/
40+
private $rest_preloader;
41+
42+
/**
43+
* ValidationCounts constructor.
44+
*
45+
* @param RESTPreloader $rest_preloader An instance of the RESTPreloader class.
46+
*/
47+
public function __construct( RESTPreloader $rest_preloader ) {
48+
$this->rest_preloader = $rest_preloader;
49+
}
50+
3351
/**
3452
* Get the action to use for registering the service.
3553
*
@@ -93,5 +111,23 @@ public function enqueue_scripts() {
93111
false,
94112
$version
95113
);
114+
115+
$this->maybe_add_preload_rest_paths();
116+
}
117+
118+
/**
119+
* Adds REST paths to preload.
120+
*
121+
* Preload validation counts data on an admin screen that has the AMP Options page as a parent or on any admin
122+
* screen related to `amp_validation_error` post type (which includes the `amp_validation_error` taxonomy).
123+
*/
124+
protected function maybe_add_preload_rest_paths() {
125+
if (
126+
AMP_Options_Manager::OPTION_NAME === get_admin_page_parent()
127+
||
128+
AMP_Validated_URL_Post_Type::POST_TYPE_SLUG === get_current_screen()->post_type
129+
) {
130+
$this->rest_preloader->add_preloaded_path( '/amp/v1/unreviewed-validation-counts' );
131+
}
96132
}
97133
}

‎tests/php/src/Admin/ValidationCountsTest.php‎

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use AMP_Options_Manager;
1111
use AMP_Theme_Support;
1212
use AMP_Validated_URL_Post_Type;
13+
use AmpProject\AmpWP\Admin\RESTPreloader;
1314
use AmpProject\AmpWP\Admin\ValidationCounts;
1415
use AmpProject\AmpWP\DevTools\UserAccess;
1516
use AmpProject\AmpWP\Infrastructure\Conditional;
@@ -19,6 +20,7 @@
1920
use AmpProject\AmpWP\Infrastructure\Service;
2021
use AmpProject\AmpWP\Option;
2122
use AmpProject\AmpWP\Services;
23+
use AmpProject\AmpWP\Tests\Helpers\PrivateAccess;
2224
use AmpProject\AmpWP\Tests\TestCase;
2325

2426
/**
@@ -28,6 +30,8 @@
2830
*/
2931
class ValidationCountsTest extends TestCase {
3032

33+
use PrivateAccess;
34+
3135
/**
3236
* Test instance.
3337
*
@@ -43,9 +47,13 @@ class ValidationCountsTest extends TestCase {
4347
public function setUp() {
4448
parent::setUp();
4549

46-
$this->instance = new ValidationCounts();
50+
set_current_screen( 'edit' );
51+
get_current_screen()->post_type = 'post';
52+
53+
$this->instance = new ValidationCounts( new RESTPreloader() );
4754
}
4855

56+
/** @covers ::__construct() */
4957
public function test__construct() {
5058
$this->assertInstanceOf( ValidationCounts::class, $this->instance );
5159
$this->assertInstanceOf( Delayed::class, $this->instance );
@@ -132,4 +140,74 @@ public function test_is_needed() {
132140
delete_user_meta( $admin_user->ID, UserAccess::USER_FIELD_DEVELOPER_TOOLS_ENABLED );
133141
$this->assertTrue( ValidationCounts::is_needed() );
134142
}
143+
144+
/** @return array */
145+
public function maybe_add_preload_rest_paths_data() {
146+
return [
147+
'no_type_post' => [
148+
'set_up' => static function () {
149+
set_current_screen( 'user' );
150+
},
151+
'should_preload_path' => false,
152+
],
153+
'post_type_post' => [
154+
'set_up' => static function () {
155+
set_current_screen( 'edit' );
156+
get_current_screen()->post_type = 'post';
157+
},
158+
'should_preload_path' => false,
159+
],
160+
'post_type_page' => [
161+
'set_up' => static function () {
162+
set_current_screen( 'edit' );
163+
get_current_screen()->post_type = 'page';
164+
},
165+
'should_preload_path' => false,
166+
],
167+
'post_type_amp_validated_url' => [
168+
'set_up' => static function () {
169+
set_current_screen( 'edit' );
170+
get_current_screen()->post_type = AMP_Validated_URL_Post_Type::POST_TYPE_SLUG;
171+
},
172+
'should_preload_path' => true,
173+
],
174+
'settings_screen' => [
175+
'set_up' => function () {
176+
global $pagenow, $plugin_page, $menu;
177+
$pagenow = 'admin.php';
178+
$plugin_page = 'amp-options';
179+
$menu = [
180+
[
181+
2 => $plugin_page,
182+
],
183+
];
184+
$this->assertEquals( AMP_Options_Manager::OPTION_NAME, get_admin_page_parent() );
185+
},
186+
'should_preload_path' => true,
187+
],
188+
];
189+
}
190+
191+
/**
192+
* @dataProvider maybe_add_preload_rest_paths_data
193+
* @covers ::maybe_add_preload_rest_paths()
194+
*/
195+
public function test_maybe_add_preload_rest_paths( callable $set_up, $should_preload_path ) {
196+
if ( ! function_exists( 'rest_preload_api_request' ) ) {
197+
$this->markTestIncomplete( 'REST preload is not available so skipping.' );
198+
}
199+
200+
$set_up();
201+
202+
$this->call_private_method( $this->instance, 'maybe_add_preload_rest_paths' );
203+
204+
$rest_preloader = $this->get_private_property( $this->instance, 'rest_preloader' );
205+
$paths = $this->get_private_property( $rest_preloader, 'paths' );
206+
207+
if ( $should_preload_path ) {
208+
$this->assertContains( '/amp/v1/unreviewed-validation-counts', $paths );
209+
} else {
210+
$this->assertNotContains( '/amp/v1/unreviewed-validation-counts', $paths );
211+
}
212+
}
135213
}

‎tests/php/validation/test-class-amp-validated-url-post-type.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ public function test_update_validated_url_menu_item() {
170170

171171
AMP_Validated_URL_Post_Type::update_validated_url_menu_item();
172172
if ( Services::get( 'dependency_support' )->has_support() ) {
173-
$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] );
173+
$this->assertSame( 'Validated URLs <span id="amp-new-validation-url-count"></span>', $submenu[ AMP_Options_Manager::OPTION_NAME ][2][0] );
174174
} else {
175175
$this->assertSame( 'Validated URLs', $submenu[ AMP_Options_Manager::OPTION_NAME ][2][0] );
176176
}

‎tests/php/validation/test-class-amp-validation-error-taxonomy.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,8 +1034,8 @@ public function test_add_admin_menu_validation_error_item() {
10341034
'Error Index',
10351035
];
10361036
if ( Services::get( 'dependency_support' )->has_support() ) {
1037-
$expected_submenu[0] .= ' <span id="new-error-index-count" class="awaiting-mod"><span class="amp-count-loading"></span></span>';
1038-
$expected_submenu[3] .= ' <span id="new-error-index-count" class="awaiting-mod"><span class="amp-count-loading"></span></span>';
1037+
$expected_submenu[0] .= ' <span id="amp-new-error-index-count"></span>';
1038+
$expected_submenu[3] .= ' <span id="amp-new-error-index-count"></span>';
10391039
}
10401040
$amp_options = $submenu[ AMP_Options_Manager::OPTION_NAME ];
10411041
$this->assertEquals( $expected_submenu, end( $amp_options ) );

0 commit comments

Comments
 (0)