Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test AMP compatibility of entire site #1183

Merged
merged 91 commits into from
Aug 29, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
91 commits
Select commit Hold shift + click to select a range
305c28b
Begin testing of site urls, starting with helper method.
kienstra May 30, 2018
9e9df92
Change helper method to output IDs, for use with existing method.
kienstra May 30, 2018
fd50993
Change method name in test, assign $post_ids.
kienstra May 30, 2018
1324044
Query for the latest posts, instead of by ID.
kienstra May 30, 2018
3c4db61
Change helper method to get post permalinks.
kienstra May 30, 2018
bbfafa4
Add a method to get links for public terms.
kienstra May 30, 2018
db54a53
Also get links for post_format terms.
kienstra May 30, 2018
6713ff3
Begin to apply review suggestion for get_taxonomy_links().
kienstra May 30, 2018
2655310
Add an $offset parameter to get_taxonomy_links().
kienstra May 30, 2018
32ab635
Add (optional) to the @param tag for $offset.
kienstra May 30, 2018
1a862c7
Pass 'fields' => 'ids' to WP_Query().
kienstra May 31, 2018
c250bd5
Allow paging through posts in get_post_permalinks().
kienstra May 31, 2018
71f7543
Add a method to crawl the entire site.
kienstra May 31, 2018
4aade51
Address Travis error by using full class name, not self::
kienstra May 31, 2018
584ffea
Force theme support of 'amp' on validation requests.
kienstra May 31, 2018
ae4371c
Add an 'amp' query arg if it's in Paired Mode.
kienstra May 31, 2018
7b67efe
Begin WP-CLI script to crawl the site.
kienstra Jun 1, 2018
15ec0ed
Display the number of validation issues in the sucess message:
kienstra Jun 1, 2018
1ef10a7
Site crawling script: output 'more detail' link.
kienstra Jun 1, 2018
ba08f3d
Update PR for latest changes to validator, merge develop.
kienstra Jul 12, 2018
bfe37a8
Register a WP-CLI command: wp amp validate-site
kienstra Jul 12, 2018
3d9a01c
Call the WP-CLI tick() method for every URL validated.
kienstra Jul 12, 2018
c586c24
Remove 'This might take a while...'
kienstra Jul 12, 2018
128ca9c
Only report unaccepted validation errors.
kienstra Jul 12, 2018
441044f
Remove the return value of validate_entire_site_urls().
kienstra Jul 12, 2018
875d4ee
Address PHPCS error by aligning =
kienstra Jul 12, 2018
4e8c00c
Remove function that forced AMP theme support.
kienstra Jul 12, 2018
c4b8f5f
Go back to displaying total errors,
kienstra Jul 12, 2018
c28e5ee
Remove wrapping in add_query_arg()
kienstra Jul 12, 2018
b6393cc
Fix the progress bar, by making counts more accurate.
kienstra Jul 12, 2018
c638e7e
Align = to address failed Travis build.
kienstra Jul 12, 2018
ad43100
Remove empty lines to address Travis issue.
kienstra Jul 12, 2018
5af8dc9
Output a count of unaccepted errors.
kienstra Jul 12, 2018
adcb901
Stop storing the validated URLs in a property.
kienstra Jul 13, 2018
9d11439
Make count_posts_and_terms() private.
kienstra Jul 13, 2018
5e19020
Improve PHP DocBlocks.
kienstra Aug 2, 2018
f4e6bfd
Address failed unit test.
kienstra Aug 2, 2018
8e1b031
Make function static, and other documentation changes
kienstra Aug 3, 2018
a4c1f7a
Merge branch 'develop' into valid-site
kienstra Aug 3, 2018
ce2b044
Add to DocBlock, remove needless empty line.
kienstra Aug 3, 2018
cd1d09e
Remove 'posts_per_page' => -1 from WP_Query arguments
kienstra Aug 11, 2018
edfdd65
Address failed unit test
kienstra Aug 12, 2018
19f4feb
Address Travis error by aligning =.
kienstra Aug 12, 2018
e34ebda
Merge branch 'develop' into valid-site
kienstra Aug 12, 2018
57411c6
Remove extra conditional block to call tick()
kienstra Aug 14, 2018
6802175
Improve documentation, remove duplicated code.
kienstra Aug 14, 2018
a53bb45
Add a way to force crawling non-AMP-enabled URLs
kienstra Aug 15, 2018
d44cde9
Exclude taxonomy templates if the user has unchecked them
kienstra Aug 15, 2018
71f4fcb
Add a flag to force validation of the entire site (but not yet implem…
kienstra Aug 15, 2018
ff3a5c0
Allow crawling templates the user has unchecked in 'Supported Templates'
kienstra Aug 15, 2018
d378a37
If there are no AMP-enabled taxonomies, don't count them.
kienstra Aug 15, 2018
4f8b3e9
Add an argument --include to the WP-CLI command.
kienstra Aug 15, 2018
4ea25bf
Implement the --include argument for taxonomies and posts
kienstra Aug 16, 2018
dadab10
Validate author pages, including with include=is_author
kienstra Aug 16, 2018
c687ea0
Validate the search template
kienstra Aug 16, 2018
09fa083
If there are no URLs to crawl, call WP_CLI:error()
kienstra Aug 16, 2018
eadb61c
Merge branch 'develop' into valid-site
kienstra Aug 16, 2018
e8717fd
Update dev-lib: Install WP-CLI after installing WordPress on Travis CI
westonruter Aug 16, 2018
13a0cc7
Ensure WP-CLI is available for deploy script
westonruter Aug 16, 2018
f054f13
Update Node so that Object.values is available
westonruter Aug 16, 2018
be32dcf
Refactor get_author_page_urls() to use round-robin validation
kienstra Aug 16, 2018
ff8d365
Refactor validate_entire_site_url() to use round-robin validation
kienstra Aug 16, 2018
1387c5b
Apply the maximum URL property to count_urls_to_validate()
kienstra Aug 17, 2018
41cffdc
Add a --max-url-count argument
kienstra Aug 17, 2018
0202432
Account for the homepage in the --include argument
kienstra Aug 17, 2018
bff8fc7
Refactor validate_urls() to validate a single URL
kienstra Aug 17, 2018
5d0d464
Display the validity by template type, like category: 15/16
kienstra Aug 17, 2018
86cd224
Get the date template, and validate it
kienstra Aug 21, 2018
c31fa0e
Remove default value in get_taxonomy_links()
kienstra Aug 21, 2018
be8d552
Remove exclusion of attachments from tests, other documentations
kienstra Aug 21, 2018
3dac2fd
Test removing the array_merge() call (will probably revert)
kienstra Aug 21, 2018
fe8eab3
Revert "Test removing the array_merge() call (will probably revert)"
kienstra Aug 21, 2018
b741f01
Merge branch 'develop' into valid-site
kienstra Aug 21, 2018
4cf9e9d
Account for the date page in the initial count
kienstra Aug 21, 2018
907facd
If there are no matched templates from --include, output error.
kienstra Aug 21, 2018
52ff854
Merge branch 'develop' of https://github.com/Automattic/amp-wp into v…
westonruter Aug 23, 2018
ab9c2d9
Tidy static analysis complaints
westonruter Aug 24, 2018
1f8f40b
Rename AMP_Site_Validation to AMP_CLI; init even in Classic mode
westonruter Aug 24, 2018
aca16f4
Refactor AMP_CLI to use methods as subcommands
westonruter Aug 24, 2018
2637fb8
Ignore auto-sanitization when looking for unaccepted errors
westonruter Aug 24, 2018
da22efc
In Classic Mode, call WP_CLI::error() if the --force flag isn't present
kienstra Aug 24, 2018
bfadbe2
Allow validating the site in 'Classic' mode
kienstra Aug 24, 2018
30f7522
Fix phpdoc for WP-CLI
westonruter Aug 28, 2018
e1760aa
Prevent crawling search or date URLs if empty; use for loop
westonruter Aug 28, 2018
ad7e0e2
Show warning when validate_url call fails
westonruter Aug 28, 2018
7a52751
Fix classic mode site validation by forcing native mode
westonruter Aug 28, 2018
8e2d028
Re-use amp_validate query param for forcing AMP theme support in clas…
westonruter Aug 28, 2018
edcb23a
Unconditionally initialize validation manager w/ registered post type…
westonruter Aug 29, 2018
0e9bf1b
Switch to query posts in descending order for improved recency releva…
westonruter Aug 29, 2018
a83a569
Simplify should_show_in_menu return condition
westonruter Aug 29, 2018
19ead16
Fix grammar typo in CLI success message
westonruter Aug 29, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ function amp_init() {
add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK );

AMP_Theme_Support::init();
AMP_Validation_Manager::init();
AMP_Post_Type_Support::add_post_type_support();

if ( defined( 'WP_CLI' ) ) {
Expand Down
30 changes: 11 additions & 19 deletions includes/class-amp-cli.php
Original file line number Diff line number Diff line change
Expand Up @@ -169,18 +169,14 @@ public function validate_site( $args, $assoc_args ) {
self::$force_crawl_urls = true;
}

$theme_support_amp = current_theme_supports( 'amp' );

if ( ! $theme_support_amp ) {
if ( ! current_theme_supports( 'amp' ) ) {
Copy link
Contributor Author

@kienstra kienstra Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conditional will capture the Template Mode being Classic, though maybe you had in mind a more narrow check, like:

! current_theme_supports( 'amp' ) && 'disabled' === AMP_Options_Manager::get_option( 'theme_support' ) 

if ( self::$force_crawl_urls ) {
/*
* There is no theme support added programmatically or via options.
* So the validation taxonomy and post type would not normally be registered.
* Register them here in order to use them to crawl the site.
* So make sure that theme support is present so that AMP_Validation_Manager::validate_url()
* will use a canonical URL as the basis for obtaining validation results.
*/
add_theme_support( 'amp' );
AMP_Validation_Error_Taxonomy::register();
AMP_Invalid_URL_Post_Type::register();
} else {
WP_CLI::error(
sprintf(
Expand Down Expand Up @@ -252,23 +248,19 @@ public function validate_site( $args, $assoc_args ) {
);

// Output a table of validity by template/content type.
if ( $theme_support_amp ) {
$url_more_details = add_query_arg(
'post_type',
AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG,
admin_url( 'edit.php' )
);
/* translators: %s is the URL to the admin */
WP_CLI::line( sprintf( __( 'For more details, please see: %s', 'amp' ), $url_more_details ) );
} else {
WP_CLI::warning( __( 'You cannot currently access the detailed validation results because you have not switched to native or paired AMP template modes.', 'amp' ) );
}

WP_CLI\Utils\format_items(
'table',
$table_validation_by_type,
array( $key_template_type, $key_url_count, $key_validity_rate )
);

$url_more_details = add_query_arg(
'post_type',
AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG,
admin_url( 'edit.php' )
);
/* translators: %s is the URL to the admin */
WP_CLI::line( sprintf( __( 'For more details, please see: %s', 'amp' ), $url_more_details ) );
}

/**
Expand Down
4 changes: 0 additions & 4 deletions includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,6 @@ public static function init() {
return;
}

AMP_Validation_Manager::init( array(
'should_locate_sources' => AMP_Validation_Manager::should_validate_response(),
) );

self::$init_start_time = microtime( true );

self::purge_amp_query_vars();
Expand Down
33 changes: 33 additions & 0 deletions includes/options/class-amp-options-menu.php
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,39 @@ public function render_theme_support() {
</dt>
<dd>
<?php esc_html_e( 'Display AMP responses in classic (legacy) post templates in a basic design that does not match your theme\'s templates.', 'amp' ); ?>

<?php if ( ! current_theme_supports( 'amp' ) && wp_count_posts( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG )->publish > 0 ) : ?>
<div class="notice notice-info inline notice-alt">
<p>
<?php
echo wp_kses_post(
sprintf(
/* translators: %1$s is link to invalid URLs and %2$s is link to validation errors */
__( 'View current site compatibility results for native and paired modes: %1$s and %2$s', 'amp' ),
sprintf(
'<a href="%s">%s</a>',
esc_url( add_query_arg( 'post_type', AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, admin_url( 'edit.php' ) ) ),
esc_html( get_post_type_object( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG )->labels->name )
),
sprintf(
'<a href="%s">%s</a>',
esc_url(
add_query_arg(
array(
'taxonomy' => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
'post_type' => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG,
),
admin_url( 'edit-tags.php' )
)
),
esc_html( get_taxonomy( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG )->labels->name )
)
)
);
?>
</p>
</div>
<?php endif; ?>
</dd>
</dl>
</fieldset>
Expand Down
18 changes: 17 additions & 1 deletion includes/validation/class-amp-invalid-url-post-type.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public static function register() {
'supports' => false,
'public' => false,
'show_ui' => true,
'show_in_menu' => AMP_Options_Manager::OPTION_NAME,
'show_in_menu' => ( self::should_show_in_menu() || AMP_Validation_Error_Taxonomy::should_show_in_menu() ) ? AMP_Options_Manager::OPTION_NAME : false,
// @todo Show in rest.
)
);
Expand All @@ -109,6 +109,22 @@ public static function register() {
}
}

/**
* Determine whether the admin menu item should be included.
*
* @return bool Whether to show in menu.
*/
public static function should_show_in_menu() {
global $pagenow;
if ( current_theme_supports( 'amp' ) ) {
return true;
}
if ( 'edit.php' === $pagenow && ( isset( $_GET['post_type'] ) && self::POST_TYPE_SLUG === $_GET['post_type'] ) ) { // WPCS: CSRF OK.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not return the result of the conditional instead of explicitedly retuning true or false?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Done in a83a569.

return true;
}
return false;
}

/**
* Add admin hooks.
*/
Expand Down
22 changes: 20 additions & 2 deletions includes/validation/class-amp-validation-error-taxonomy.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public static function register() {
'show_tagcloud' => false,
'show_in_quick_edit' => false,
'hierarchical' => false, // Or true? Code could be the parent term?
'show_in_menu' => true,
'show_in_menu' => ( self::should_show_in_menu() || AMP_Invalid_URL_Post_Type::should_show_in_menu() ),
'meta_box_cb' => false, // See print_validation_errors_meta_box().
'capabilities' => array(
'assign_terms' => 'do_not_allow',
Expand All @@ -162,6 +162,22 @@ public static function register() {
self::accept_validation_errors( AMP_Core_Theme_Sanitizer::get_acceptable_errors( get_template() ) );
}

/**
* Determine whether the admin menu item should be included.
*
* @return bool Whether to show in menu.
*/
public static function should_show_in_menu() {
global $pagenow;
if ( current_theme_supports( 'amp' ) ) {
return true;
}
if ( 'edit-tags.php' === $pagenow && ( isset( $_GET['taxonomy'] ) && self::TAXONOMY_SLUG === $_GET['taxonomy'] ) ) { // WPCS: CSRF OK.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here....return the result of the conditional expression instead of explicit trueor false. It’s less code and more clear.

return true;
}
return false;
}

/**
* Prepare a validation error for lookup or insertion as taxonomy term.
*
Expand Down Expand Up @@ -459,7 +475,9 @@ public static function add_admin_hooks() {
add_filter( 'terms_clauses', array( __CLASS__, 'filter_terms_clauses_for_description_search' ), 10, 3 );
add_action( 'admin_notices', array( __CLASS__, 'add_admin_notices' ) );
add_filter( 'tag_row_actions', array( __CLASS__, 'filter_tag_row_actions' ), 10, 2 );
add_action( 'admin_menu', array( __CLASS__, 'add_admin_menu_validation_error_item' ) );
if ( get_taxonomy( self::TAXONOMY_SLUG )->show_in_menu ) {
add_action( 'admin_menu', array( __CLASS__, 'add_admin_menu_validation_error_item' ) );
}
add_filter( 'manage_' . self::TAXONOMY_SLUG . '_custom_column', array( __CLASS__, 'filter_manage_custom_columns' ), 10, 3 );
add_filter( 'views_edit-' . self::TAXONOMY_SLUG, array( __CLASS__, 'filter_views_edit' ) );
add_filter( 'posts_where', array( __CLASS__, 'filter_posts_where_for_validation_error_status' ), 10, 2 );
Expand Down
7 changes: 6 additions & 1 deletion includes/validation/class-amp-validation-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ class AMP_Validation_Manager {
public static function init( $args = array() ) {
$args = array_merge(
array(
'should_locate_sources' => false,
'should_locate_sources' => self::should_validate_response(),
),
$args
);
Expand All @@ -158,6 +158,11 @@ public static function init( $args = array() ) {
AMP_Invalid_URL_Post_Type::register();
AMP_Validation_Error_Taxonomy::register();

// Short-circuit if AMP is not supported as only the post types should be available.
if ( ! current_theme_supports( 'amp' ) ) {
return;
}

add_action( 'save_post', array( __CLASS__, 'handle_save_post_prompting_validation' ) );
add_action( 'enqueue_block_editor_assets', array( __CLASS__, 'enqueue_block_validation' ) );

Expand Down
22 changes: 22 additions & 0 deletions tests/validation/test-class-amp-invalid-url-post-type.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public function tearDown() {
* @covers \AMP_Invalid_URL_Post_Type::add_admin_hooks()
*/
public function test_register() {
add_theme_support( 'amp' );
$this->assertFalse( is_admin() );

AMP_Invalid_URL_Post_Type::register();
Expand All @@ -51,6 +52,27 @@ public function test_register() {
$this->assertContains( AMP_Invalid_URL_Post_Type::REMAINING_ERRORS, wp_removable_query_args() );
}

/**
* Test should_show_in_menu.
*
* @covers AMP_Invalid_URL_Post_Type::should_show_in_menu()
*/
public function test_should_show_in_menu() {
global $pagenow;
add_theme_support( 'amp' );
$this->assertTrue( AMP_Invalid_URL_Post_Type::should_show_in_menu() );

remove_theme_support( 'amp' );
$this->assertFalse( AMP_Invalid_URL_Post_Type::should_show_in_menu() );

$pagenow = 'edit.php'; // WPCS: override ok.
$_GET['post_type'] = 'post';
$this->assertFalse( AMP_Invalid_URL_Post_Type::should_show_in_menu() );

$_GET['post_type'] = AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG;
$this->assertTrue( AMP_Invalid_URL_Post_Type::should_show_in_menu() );
}

/**
* Test add_admin_hooks.
*
Expand Down
24 changes: 24 additions & 0 deletions tests/validation/test-class-amp-validation-error-taxonomy.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public function tearDown() {
*/
public function test_register() {
global $wp_taxonomies;
add_theme_support( 'amp' );

AMP_Validation_Error_Taxonomy::register();
$taxonomy_object = $wp_taxonomies[ AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG ];
Expand Down Expand Up @@ -79,6 +80,27 @@ public function test_register() {
$this->assertEquals( 'Most Used Validation Errors', $labels->most_used );
}

/**
* Test should_show_in_menu.
*
* @covers AMP_Validation_Error_Taxonomy::should_show_in_menu()
*/
public function test_should_show_in_menu() {
global $pagenow;
add_theme_support( 'amp' );
$this->assertTrue( AMP_Validation_Error_Taxonomy::should_show_in_menu() );

remove_theme_support( 'amp' );
$this->assertFalse( AMP_Validation_Error_Taxonomy::should_show_in_menu() );

$pagenow = 'edit-tags.php'; // WPCS: override ok.
$_GET['taxonomy'] = 'post_tag';
$this->assertFalse( AMP_Validation_Error_Taxonomy::should_show_in_menu() );

$_GET['taxonomy'] = AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG;
$this->assertTrue( AMP_Validation_Error_Taxonomy::should_show_in_menu() );
}

/**
* Test prepare_validation_error_taxonomy_term.
*
Expand Down Expand Up @@ -315,6 +337,8 @@ public function test_summarize_validation_errors() {
* @covers \AMP_Validation_Error_Taxonomy::add_admin_hooks()
*/
public function test_add_admin_hooks() {
add_theme_support( 'amp' );
AMP_Validation_Error_Taxonomy::register();

// add_group_terms_clauses_filter() needs the screen to be set.
set_current_screen( 'front' );
Expand Down