Amp-wp: [Compatibility Tool] Better tooling for evaluating plugins

[postphotos]

As a WordPress site, my Compatibility Tool should have better tooling for evaluating plugins' AMP compatibility programmatically.

• [ ] AC1: Currently, AMP validation only occurs on plugin activation and when editing a post: This story addresses user experience. We would like to additionally allow a site admin to manually trigger a check of the plugins' AMP compatibility ~inside the user interface from #1006~ via WP-CLI.
• [ ] AC2: A validation test should be able to evaluate an entire site even if AMP theme support is not declared yet. (i.e. Be able to force the equivalent of AMP theme support when crawling.)
• [ ] AC3: The compatibility tool should be able to target the various templates that now are presented to the user (as as described by @westonruter below, related #1235.)

The goal is to spawn validation process and get results programmatically for an entire site. WP-Cron and WP-CLI are in view here, as is integrating with logic in XML Sitemap generators to obtain list of URLs potentially.

[postphotos]

The main issue description has been updated with a formal Story and Acceptance Criteria.

[westonruter]

Validation test on entire site even if AMP theme support is not declared yet. Be able to force amp theme support while crawling.

[postphotos]

Added AC2 per our meeting yesterday.

[westonruter]

Aside: This is useful for trying out AMP theme support for a given theme without manually doing add_theme_support('amp') in the theme. Add a new plugin (e.g. mu-plugin) that contains:

add_action( 'after_setup_theme', function() {
    if ( isset( $_REQUEST['amp_forced'] ) ) {
        add_theme_support( 'amp' );
    }
} );

Then to see how a theme works with AMP just activate the theme and access the homepage or any other page with ?amp_forced in the URL.

[kienstra]

Question About Working On This Issue

Hi @westonruter,
Would it help if I worked on this issue, and having the validator:

...spawn the validation process and get results programmatically for an entire site.

Otherwise, I could start working on #1036.

[westonruter]

@kienstra yes, go for it. Let's start with a WP-CLI command (with progress bar) that crawls the site and validates each URL it can find (that is, all AMP documents). As part of this, there probably needs to be something like I have in https://github.com/Automattic/amp-wp/issues/1007#issuecomment-379819279 to be able to force amp theme support. When the WP-CLI command finishes then there will need to be some way to visualize the results. While all of the validation errors will be populated in the amp_validation_error posts, there should be some more helpful summarizing of the most common issues as noted in https://github.com/Automattic/amp-wp/issues/1003#issuecomment-378775312

[kienstra]

Thanks, Working On This Now

Hi @westonruter,
Thanks, starting with creating a WP-CLI command sounds good. Your snippet above to force support for 'amp' will help.

[westonruter]

Once we have the WP-CLI command in place, then we can look at how to present that same information in the admin. We'll also then need to figure out a good way to use admin-ajax to reliably make requests to batch validate URLs without timing-out. But these things would essentially be GUI improvements over the raw work performed by WP-CLI, so good to focus on that first. On that note, most of the WP-CLI commands should be invoking logic in AMP_Validation_Utils which can be re-shared with the admin integration.

[westonruter]

As part of this we might want to resurrect the REST API endpoint for AMP validation. There could be a amp/v1/validation-errors endpoint which returns a list of all validation error posts. Then this endpoint could maybe accept POST request with a url to check, which then could result in a validation error post being created, or maybe return 400 error when there are no validation errors on the page. That feels somewhat odd, to return an error for there not being errors. Some more thought on the REST API design should be made.

[kienstra]

Moving Into "To Do"

Hi @westonruter,
That sounds good to have an endpoint to get AMP validation results. Like we discussed, I'm moving this issue back into "To Do" status, to signal that I'm not actively working on it.

If you're not working on it by the time I'm free, I'll probably come back to it.

Thanks!

[kienstra]

Still Needed?

Hi @westonruter,
Could we still use the WP-CLI command that you mentioned above to check for validation errors with forced theme support? If so, I'd be happy to work on it if that would help.

[westonruter]

Yes, but first we really need to finish #1093.

[westonruter]

A key dependency for this is the ability to obtain validation results for a URL even when the user is not authenticated. This has been implemented via nonce in https://github.com/Automattic/amp-wp/pull/1093/commits/90eff4fb0e2f070cf3ebe126f5d26265e8f62fac via #1093.

[kienstra]

@westonruter mentioned that this should involve:

• Getting a list of URLs of the site, and a way to iterate over them in efficient way that won't time out WordPress
• Process the URLs in batches
• WP-CLI
• WP Cron
• User-initiated: a button that makes AJAX request with spinner

Of course, feel free to edit this if these notes didn't capture the points exactly.

[westonruter]

@postphotos New suggested AC: I think we should have the compatibility tool target the various templates that now are presented to the user. Then we can surface next to each supportable template checkbox an indicator for whether AMP is known to be supported. Each item can include a “Check compatibility“ and a link to “Review issues”. Maybe also a button to re-check compatibility for all of the templates. These template checks should be done each time the user switches themes (though the request should be queued via cron and not done synchronously, since it will take a minute or so).

It is more important that the automated site checker look at these templates instead of looking at every single URL. We should still do the scanning of all URLs periodically but when a user initiates a scan it should focus on the main site templates. /cc @kienstra

See also #1254 for an admin pointer to draw users to the admin screen. We should use the tool to help provide an onboarding experience for users to AMP theme support.

[postphotos]

Hi @westonruter - I didn't flag you here, but I added your AC from the comment above to this ticket last week.

[westonruter]

I've tweaked the AC1 to make it explicit that this issue is regarding the initiation of a site validation scan via WP-CLI _not_ via the WordPress admin. An admin UI would need to be proposed in a separate issue along with designs; the admin UI would have the added complexity of needing to guard against timeouts. The PR #1183 resolves the WP-CLI issue.

[kienstra]

Moving To 'Ready For Merging'

If it's alright, I'm moving this to 'Ready For Merging,' as I don't think it needs functional testing beyond the 'technical QA' from PR #1183.