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.
ampproject · Aug 29, 2018 · edcb23a · edcb23a
1 parent 8e2d028
commit edcb23a
Show file tree

Hide file tree

Showing 9 changed files with 134 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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>

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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' );