From edcb23aed859848bb3babcdeea29a1cb86c57c8e Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 28 Aug 2018 17:28:45 -0700 Subject: [PATCH] Unconditionally initialize validation manager w/ registered post type and taxonomy * Hide the admin menu links for invalid URLs and validation errors by default in classic mode. * Invoking `wp amp validate-site` will cause the post type and taxonomy to be populated. * When in classic mode, show links to invalid pages and validation errors with template mode as opposed to in admin menu. * When viewing the admin screens for the post type and taxonomy, show the admin menu links for both. --- amp.php | 1 + includes/class-amp-cli.php | 30 +++++++---------- includes/class-amp-theme-support.php | 4 --- includes/options/class-amp-options-menu.php | 33 +++++++++++++++++++ .../class-amp-invalid-url-post-type.php | 18 +++++++++- .../class-amp-validation-error-taxonomy.php | 22 +++++++++++-- .../class-amp-validation-manager.php | 7 +++- .../test-class-amp-invalid-url-post-type.php | 22 +++++++++++++ ...st-class-amp-validation-error-taxonomy.php | 24 ++++++++++++++ 9 files changed, 134 insertions(+), 27 deletions(-) diff --git a/amp.php b/amp.php index 91ca59a6fc7..8c7924c56c4 100644 --- a/amp.php +++ b/amp.php @@ -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' ) ) { diff --git a/includes/class-amp-cli.php b/includes/class-amp-cli.php index f257eb5e825..e06204b6efa 100644 --- a/includes/class-amp-cli.php +++ b/includes/class-amp-cli.php @@ -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' ) ) { 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( @@ -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 ) ); } /** diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index d489756b57a..b213d055eb1 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -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(); diff --git a/includes/options/class-amp-options-menu.php b/includes/options/class-amp-options-menu.php index 860fba5a9f7..4cb64373050 100644 --- a/includes/options/class-amp-options-menu.php +++ b/includes/options/class-amp-options-menu.php @@ -190,6 +190,39 @@ public function render_theme_support() {
+ + publish > 0 ) : ?> +
+

+ %s', + 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( + '%s', + 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 ) + ) + ) + ); + ?> +

+
+
diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 58ead72abce..785928e4054 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -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. ) ); @@ -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. + return true; + } + return false; + } + /** * Add admin hooks. */ diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 9c8414dc7b7..fa19ec7f8ab 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -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', @@ -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. + return true; + } + return false; + } + /** * Prepare a validation error for lookup or insertion as taxonomy term. * @@ -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 ); diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index c8cca429149..42f3b0dc5d8 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -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 ); @@ -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' ) ); diff --git a/tests/validation/test-class-amp-invalid-url-post-type.php b/tests/validation/test-class-amp-invalid-url-post-type.php index c2ab90ae52e..9d53e0d908a 100644 --- a/tests/validation/test-class-amp-invalid-url-post-type.php +++ b/tests/validation/test-class-amp-invalid-url-post-type.php @@ -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(); @@ -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. * diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php index 4d625b0f48a..d18d0618f4d 100644 --- a/tests/validation/test-class-amp-validation-error-taxonomy.php +++ b/tests/validation/test-class-amp-validation-error-taxonomy.php @@ -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 ]; @@ -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. * @@ -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' );