Add button into Site Health to reenable CSS transient caching by schlessera · Pull Request #4522 · ampproject/amp-wp

schlessera · 2020-04-03T17:26:51Z

Add CSS monitoring time series to Site Health debugging info

Summary

This PR adds a button to the Site Health AMP panel to reenable CSS transient caching when it was disabled.

Button:

Button when the AJAX request failed:

Button when the AJAX request succeeded:

Fixes #4523

Checklist

My pull request is addressing an open issue (please create one otherwise).
My code is tested and passes existing tests.
My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

schlessera · 2020-04-03T17:28:54Z

~~The colors still need to be adapted to fit the WordPress color palette.~~

However, there's currently a problem with the AMP_Options_Manager: Within the AJAX request, the update_option() call just fails and returns false.

@westonruter Are there any requirements to using the AMP_Options_Manager ? Do you have an idea of why this would fail within an AJAX request?

pierlon · 2020-04-03T17:38:29Z

Do you have an idea of why this would fail within an AJAX request?

Perhaps the WP nonce is not being sent along with the request?

schlessera · 2020-04-03T17:40:20Z

The code does execute the call to the AMP_Options_Manager, I could verify that.

src/Admin/AjaxAction.php

Co-Authored-By: Weston Ruter <westonruter@google.com>

schlessera · 2020-04-04T08:26:46Z

Figured it out. The AMP_Options_Manager has an unexpected validation behavior. If an unknown option is passed through, it doesn't allow it to make a change, but also doesn't unset it either, so the option just remains as it was.

schlessera · 2020-04-06T13:50:09Z

Tests failed for unrelated timeout in E2E tests:
https://travis-ci.org/github/ampproject/amp-wp/jobs/671649112#L861-L871

 ● Featured Image Notice › should not display a notice, nor suggest cropping, when the image is the expected size
    TimeoutError: Text found "Crop Image"
      waiting for function failed: timeout 500ms exceeded
  ● Featured Image Notice › should display a notice when the image is too small, but not suggest cropping
    TimeoutError: Text found "Crop Image"
      waiting for function failed: timeout 500ms exceeded

src/Admin/AjaxAction.php

pierlon · 2020-04-07T02:49:30Z

Tests failed for unrelated timeout in E2E tests

@schlessera these tests are failing because of an error being thrown (see #4522 (comment)).

pierlon · 2020-04-07T03:17:40Z

I think this button could be highlighted a little better, given how deep it's hidden in Site Health. I don't see how the user would know about it unless they've been told where it is.

schlessera · 2020-04-07T03:59:14Z

I don't see how the user would know about it unless they've been told where it is.

I assumed for now that this is mainly for the support team to be able to point them to some place to reset the cache instead of handing them a snippet filter or a custom filter to do so.

I don't think this should be a standard user operation.

But I'm happy to change this if my assumption is incorrect.

@westonruter @amedina Thoughts?

Co-Authored-By: Weston Ruter <westonruter@google.com>

schlessera · 2020-04-09T16:03:17Z

As there's no code in place for replacing the nonce once it's been used, the button only works once per page load. However, there's no valid reason to push it multiple times, so I disabled it after the first use, and any further tries will need a reload.

…aching-reenable-button * 'develop' of github.com:ampproject/amp-wp: Restore unification of multi-page post content in Reader mode (#4547) Prevent styles from being removed when in Customizer preview with Standard mode (#4553) Omit Jetpack from being activated during PHPUnit test runs Use title case for Paired Browsing link in edit post screen (#4540) Ensure that validation query vars persist through redirects (#4544) Update dependency babel-jest to v25.2.6 (#4510) Update dependency css-loader to v3.5.0 (#4537) Update dependency autoprefixer to v9.7.6 (#4539) Add requirements to plugin file header (#4543) Force status code of validation responses to be 200 (#4533) Update optimizer test specs (#4527) Bump stable tag to 1.5.2 Cache response status and headers when fetching external stylesheets (#4509) Fix securing multi-line mustache templates (#4521) Add CSS monitoring time series to Site Health debugging info (#4519) Update hostname used for WordPress TV embeds to fix external HTTP requests (#4524) Mock Imgur embed tests Mock Facebook embed tests Standardize file and class names for embed handlers

westonruter · 2020-04-10T03:19:22Z

I fleshed out the guidance a bit more, and added link to learn more about object caching:

When enabled:

westonruter · 2020-04-10T03:39:05Z

Confirmed error condition is shown properly:

…aping

src/Admin/ReenableCssTransientCachingAjaxAction.php

Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>

* Add button into Site Health to reenable CSS transient caching * Use wp_jsonencode() instead of addslashes() Co-Authored-By: Weston Ruter <westonruter@google.com> * Remove unneeded PHPCS exception * Use wp.ajax and heredoc for JS script * Update styling * Add capability check * Use current_user_can() * Add CSS transient caching option to options manager validation * Fix code style issues * Only read Option::DISABLE_CSS_TRANSIENT_CACHING if exists * Remove <script> tags from JS code passed to wp_add_inline_script() * Remove pesky 2nd blank line Co-Authored-By: Weston Ruter <westonruter@google.com> * Rename AjaxAction to ReenableCssTransientCachingAjaxAction * Remove $access and default to authenticated users only * Remove $scope and default to admin backend only * Remove $action and hardcode as const * Remove $callback and move method into AjaxAction * Remove $selector and hardcode as const * Only register AJAX action on site-health.php screen * Use tabs instead of spaces to indent CSS * Escape button label translations * Add nonce verification * Disable button after click * Use DOMContentLoaded instead of a timeout * Indent JS code with tabs instead of spaces * Document $hook_suffix argument * Flesh out guidance for CSS transient caching * Avoid string interpolation for JS injection to better ensure late-escaping * Use nowdoc instead of heredoc Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com> Co-authored-by: Weston Ruter <westonruter@google.com>

kienstra · 2020-04-14T19:11:32Z

Looks good

Hi @amedina,
This worked as expected in my local.

After forcing the option to be true:

AMP_Options_Manager::update_option( 'amp_css_transient_monitor_disable_caching', true );

...the test appeared in Site Health:

* tag '1.5.3': Bump 1.5.3 Bump version to 1.5.3-RC1 Fix handling of Mustache templates (#4583) Stub request based on test scenario (#4588) Update tests after block-library/style.css changes in Gutenberg 7.9 (#4579) Restrict doing plugin upgrade routine when not in admin (#4538) Add new accessibility sanitizer (#4535) Fix unit tests (#4564) Add button into Site Health to reenable CSS transient caching (#4522) Restore unification of multi-page post content in Reader mode (#4547) Prevent styles from being removed when in Customizer preview with Standard mode (#4553) Omit Jetpack from being activated during PHPUnit test runs (#4474) Mock Facebook embed tests (#4474) Mock Imgur embed tests (#4474) Use title case for Paired Browsing link in edit post screen (#4540) Ensure that validation query vars persist through redirects (#4544) Add requirements to plugin file header (#4543) Force status code of validation responses to be 200 (#4533) Update optimizer test specs (#4527) Bump 1.5.3-alpha

Add button into Site Health to reenable CSS transient caching

13359f8

schlessera added CSS Integration: WP Core Caching labels Apr 3, 2020

googlebot added the cla: yes Signed the Google CLA label Apr 3, 2020

westonruter requested changes Apr 3, 2020

View reviewed changes

westonruter added this to the v1.5.3 milestone Apr 3, 2020

schlessera and others added 8 commits April 4, 2020 13:44

Use wp_jsonencode() instead of addslashes()

c19c1ac

Co-Authored-By: Weston Ruter <westonruter@google.com>

Remove unneeded PHPCS exception

7b56dd1

Use wp.ajax and heredoc for JS script

d1e2aa2

Update styling

cc2bd07

Add capability check

e820e65

Use current_user_can()

8869d99

Add CSS transient caching option to options manager validation

24fa804

Fix code style issues

ce1682f

schlessera requested review from kienstra, pierlon and westonruter April 4, 2020 08:26

Only read Option::DISABLE_CSS_TRANSIENT_CACHING if exists

a258998

pierlon reviewed Apr 7, 2020

View reviewed changes

src/Admin/AjaxAction.php Outdated Show resolved Hide resolved

Remove <script> tags from JS code passed to wp_add_inline_script()

3ae8fed

schlessera requested a review from pierlon April 7, 2020 04:38

schlessera and others added 14 commits April 9, 2020 13:31

Remove pesky 2nd blank line

41bf7a9

Co-Authored-By: Weston Ruter <westonruter@google.com>

Rename AjaxAction to ReenableCssTransientCachingAjaxAction

61f7d8b

Remove $access and default to authenticated users only

25ca0c8

Remove $scope and default to admin backend only

caea4dd

Remove $action and hardcode as const

fcaeffa

Remove $callback and move method into AjaxAction

d60da1e

Remove $selector and hardcode as const

50e1ca1

Only register AJAX action on site-health.php screen

f0e1db0

Use tabs instead of spaces to indent CSS

9c41211

Escape button label translations

66a7b73

Add nonce verification

de1038c

Disable button after click

f94dbf3

Use DOMContentLoaded instead of a timeout

5cbe86b

Indent JS code with tabs instead of spaces

712c1f2

schlessera and others added 2 commits April 9, 2020 18:17

Document $hook_suffix argument

a52b391

Flesh out guidance for CSS transient caching

b9a1eb1

Avoid string interpolation for JS injection to better ensure late-esc…

ff9a37e

…aping

westonruter approved these changes Apr 10, 2020

View reviewed changes

schlessera commented Apr 10, 2020

View reviewed changes

src/Admin/ReenableCssTransientCachingAjaxAction.php Outdated Show resolved Hide resolved

Use nowdoc instead of heredoc

a26112b

Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>

westonruter merged commit d1ea42f into develop Apr 10, 2020

westonruter deleted the add-css-caching-reenable-button branch April 10, 2020 06:07

schlessera added the WS:Perf Work stream for Metrics, Performance and Optimizer label Aug 12, 2020

Conversation

schlessera commented Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

schlessera commented Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pierlon commented Apr 3, 2020

Uh oh!

schlessera commented Apr 3, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

schlessera commented Apr 4, 2020

Uh oh!

schlessera commented Apr 6, 2020

Uh oh!

Uh oh!

pierlon commented Apr 7, 2020

Uh oh!

pierlon commented Apr 7, 2020

Uh oh!

schlessera commented Apr 7, 2020

Uh oh!

schlessera commented Apr 9, 2020

Uh oh!

westonruter commented Apr 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

westonruter commented Apr 10, 2020

Uh oh!

Uh oh!

kienstra commented Apr 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

schlessera commented Apr 3, 2020 •

edited

Loading

schlessera commented Apr 3, 2020 •

edited

Loading

westonruter commented Apr 10, 2020 •

edited

Loading

kienstra commented Apr 14, 2020 •

edited

Loading