Merge pull request #5010 from ampproject/update/theme-support-paired-…

…default Revamp notice appearing in AMP Enabled toggle, revamp how plugin determines whether AMP can be disabled
ampproject · Jul 14, 2020 · 9f7b74c · 9f7b74c
2 parents 6b8449d + e07656a
commit 9f7b74c
Show file tree

Hide file tree

Showing 18 changed files with 650 additions and 498 deletions.
diff --git a/assets/css/src/amp-block-editor.css b/assets/css/src/amp-block-editor.css
@@ -33,3 +33,18 @@
 .wp-core-ui .edit-post-header .editor-post-preview {
 	height: 34px;
 }
+
+.amp-unavailable-notice {
+	width: 100%;
+	margin: 5px 0 2px;
+}
+
+.amp-unavailable-notice details > summary {
+	font-weight: bold;
+	user-select: none;
+	cursor: pointer;
+}
+
+.amp-unavailable-notice details[open] > summary {
+	margin-bottom: 1em;
+}
diff --git a/assets/src/block-editor/plugins/amp-toggle.js b/assets/src/block-editor/plugins/amp-toggle.js
@@ -51,16 +51,24 @@ function AMPToggle( { isEnabled, onChange } ) {
 				Boolean( errorMessages.length ) &&
 					(
 						<Notice
-							status="warning"
+							status="info"
 							isDismissible={ false }
+							className="amp-unavailable-notice"
 						>
-							{
-								errorMessages.map( ( message, index ) => (
-									<RawHTML key={ index }>
-										{ message }
-									</RawHTML>
-								) )
-							}
+							<details>
+								<summary>
+									{ __( 'AMP Unavailable', 'amp' ) }
+								</summary>
+								{
+									errorMessages.map( ( message, index ) => (
+										<p key={ index }>
+											<RawHTML>
+												{ message }
+											</RawHTML>
+										</p>
+									) )
+								}
+							</details>
 						</Notice>
 					)
 			}

diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php
@@ -212,6 +212,13 @@ public function enqueue_block_assets() {
 			return;
 		}
 
+		$status_and_errors = self::get_status_and_errors( $post );
+
+		// Skip proceeding if there are errors blocking AMP and the user can't do anything about it.
+		if ( ! empty( $status_and_errors['errors'] ) && ! current_user_can( 'manage_options' ) ) {
+			return;
+		}
+
 		wp_enqueue_style(
 			self::BLOCK_ASSET_HANDLE,
 			amp_get_asset_url( 'css/' . self::BLOCK_ASSET_HANDLE . '.css' ),
@@ -234,12 +241,9 @@ public function enqueue_block_assets() {
 			true
 		);
 
-		$status_and_errors = self::get_status_and_errors( get_post() );
-		$error_messages    = $this->get_error_messages( $status_and_errors['status'], $status_and_errors['errors'] );
-
 		$data = [
 			'ampSlug'         => amp_get_slug(),
-			'errorMessages'   => $error_messages,
+			'errorMessages'   => $this->get_error_messages( $status_and_errors['errors'] ),
 			'hasThemeSupport' => ! amp_is_legacy(),
 			'isStandardMode'  => amp_is_canonical(),
 		];
@@ -284,11 +288,16 @@ public function render_status( $post ) {
 		}
 
 		$status_and_errors = self::get_status_and_errors( $post );
-		$status            = $status_and_errors['status'];
+		$status            = $status_and_errors['status']; // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Used in amp-enabled-classic-editor-toggle.php.
 		$errors            = $status_and_errors['errors'];
 
+		// Skip showing any error message if the user doesn't have the ability to do anything about it.
+		if ( ! empty( $errors ) && ! current_user_can( 'manage_options' ) ) {
+			return;
+		}
+
 		// phpcs:disable VariableAnalysis.CodeAnalysis.VariableAnalysis
-		$error_messages = $this->get_error_messages( $status, $errors );
+		$error_messages = $this->get_error_messages( $errors );
 
 		$labels = [
 			'enabled'  => __( 'Enabled', 'amp' ),
@@ -322,9 +331,6 @@ public static function get_status_and_errors( $post ) {
 			$availability = AMP_Theme_Support::get_template_availability( $post );
 			$status       = $availability['supported'] ? self::ENABLED_STATUS : self::DISABLED_STATUS;
 			$errors       = array_diff( $availability['errors'], [ 'post-status-disabled' ] ); // Subtract the status which the metabox will allow to be toggled.
-			if ( true === $availability['immutable'] ) {
-				$errors[] = 'status_immutable';
-			}
 		} else {
 			$errors = AMP_Post_Type_Support::get_support_errors( $post );
 			$status = empty( $errors ) ? self::ENABLED_STATUS : self::DISABLED_STATUS;
@@ -338,40 +344,33 @@ public static function get_status_and_errors( $post ) {
 	 * Gets the AMP enabled error message(s).
 	 *
 	 * @since 1.0
-	 * @param string $status The AMP enabled status.
-	 * @param array  $errors The AMP enabled errors.
+	 * @see AMP_Post_Type_Support::get_support_errors()
+	 *
+	 * @param string[] $errors The AMP enabled errors.
 	 * @return array $error_messages The error messages, as an array of strings.
 	 */
-	public function get_error_messages( $status, $errors ) {
+	public function get_error_messages( $errors ) {
+		$settings_screen_url = admin_url( 'admin.php?page=' . AMP_Options_Manager::OPTION_NAME );
+
 		$error_messages = [];
-		if ( in_array( 'status_immutable', $errors, true ) ) {
-			if ( self::ENABLED_STATUS === $status ) {
-				$error_messages[] = __( 'Your site does not allow AMP to be disabled.', 'amp' );
-			} else {
-				$error_messages[] = __( 'Your site does not allow AMP to be enabled.', 'amp' );
-			}
-		}
 		if ( in_array( 'template_unsupported', $errors, true ) || in_array( 'no_matching_template', $errors, true ) ) {
 			$error_messages[] = sprintf(
 				/* translators: %s is a link to the AMP settings screen */
-				__( 'There are no <a href="%s">supported templates</a> to display this in AMP.', 'amp' ),
-				esc_url( admin_url( 'admin.php?page=' . AMP_Options_Manager::OPTION_NAME ) )
+				__( 'There are no <a href="%s" target="_blank">supported templates</a>.', 'amp' ),
+				esc_url( $settings_screen_url )
 			);
 		}
-		if ( in_array( 'password-protected', $errors, true ) ) {
-			$error_messages[] = __( 'AMP cannot be enabled on password protected posts.', 'amp' );
-		}
 		if ( in_array( 'post-type-support', $errors, true ) ) {
 			$error_messages[] = sprintf(
 				/* translators: %s is a link to the AMP settings screen */
-				__( 'AMP cannot be enabled because this <a href="%s">post type does not support it</a>.', 'amp' ),
-				esc_url( admin_url( 'admin.php?page=' . AMP_Options_Manager::OPTION_NAME ) )
+				__( 'This post type is not <a href="%s" target="_blank">enabled</a>.', 'amp' ),
+				esc_url( $settings_screen_url )
 			);
 		}
 		if ( in_array( 'skip-post', $errors, true ) ) {
 			$error_messages[] = __( 'A plugin or theme has disabled AMP support.', 'amp' );
 		}
-		if ( count( array_diff( $errors, [ 'status_immutable', 'page-on-front', 'page-for-posts', 'password-protected', 'post-type-support', 'skip-post', 'template_unsupported', 'no_matching_template' ] ) ) > 0 ) {
+		if ( count( array_diff( $errors, [ 'post-type-support', 'skip-post', 'template_unsupported', 'no_matching_template' ] ) ) > 0 ) {
 			$error_messages[] = __( 'Unavailable for an unknown reason.', 'amp' );
 		}
 

diff --git a/includes/admin/functions.php b/includes/admin/functions.php
@@ -40,7 +40,7 @@ function amp_admin_get_preview_permalink() {
 	$post_type = (string) apply_filters( 'amp_customizer_post_type', 'post' );
 
 	// Make sure the desired post type is actually supported, and if so, prefer it.
-	$supported_post_types = get_post_types_by_support( AMP_Post_Type_Support::SLUG );
+	$supported_post_types = AMP_Post_Type_Support::get_supported_post_types();
 	if ( in_array( $post_type, $supported_post_types, true ) ) {
 		$supported_post_types = array_unique( array_merge( [ $post_type ], $supported_post_types ) );
 	}
@@ -63,11 +63,10 @@ function amp_admin_get_preview_permalink() {
 			'no_found_rows'    => true,
 			'suppress_filters' => false,
 			'post_status'      => 'publish',
-			'post_password'    => '',
 			'post_type'        => $supported_post_types,
 			'posts_per_page'   => 1,
 			'fields'           => 'ids',
-		// @todo This should eventually do a meta_query to make sure there are none that have AMP_Post_Meta_Box::STATUS_POST_META_KEY = DISABLED_STATUS.
+			// @todo This should eventually do a meta_query to make sure there are none that have AMP_Post_Meta_Box::STATUS_POST_META_KEY = DISABLED_STATUS.
 		]
 	);
 

diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php
@@ -105,8 +105,6 @@ function amp_init() {
 	add_action( 'wp_loaded', 'amp_bootstrap_admin' );
 
 	add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK );
-	AMP_Post_Type_Support::add_post_type_support();
-	add_action( 'init', [ 'AMP_Post_Type_Support', 'add_post_type_support' ], 1000 ); // After post types have been defined.
 	add_action( 'parse_query', 'amp_correct_query_when_is_front_page' );
 	add_action( 'admin_bar_menu', 'amp_add_admin_bar_view_link', 100 );
 
@@ -143,14 +141,22 @@ function () {
 	 */
 	$options     = get_option( AMP_Options_Manager::OPTION_NAME, [] );
 	$old_version = isset( $options[ Option::VERSION ] ) ? $options[ Option::VERSION ] : '0.0';
+
 	if ( AMP__VERSION !== $old_version && is_admin() && current_user_can( 'manage_options' ) ) {
-		/**
-		 * Triggers when after amp_init when the plugin version has updated.
-		 *
-		 * @param string $old_version Old version.
-		 */
-		do_action( 'amp_plugin_update', $old_version );
-		AMP_Options_Manager::update_option( Option::VERSION, AMP__VERSION );
+		// This waits to happen until the very end of init to ensure that amp theme support and amp post type support have all been added.
+		add_action(
+			'init',
+			static function () use ( $old_version ) {
+				/**
+				 * Triggers when after amp_init when the plugin version has updated.
+				 *
+				 * @param string $old_version Old version.
+				 */
+				do_action( 'amp_plugin_update', $old_version );
+				AMP_Options_Manager::update_option( Option::VERSION, AMP__VERSION );
+			},
+			PHP_INT_MAX
+		);
 	}
 
 	add_action(

diff --git a/includes/class-amp-post-type-support.php b/includes/class-amp-post-type-support.php
@@ -58,7 +58,7 @@ public static function get_eligible_post_types() {
 	 */
 	public static function get_post_types_for_rest_api() {
 		return array_intersect(
-			get_post_types_by_support( 'amp' ),
+			self::get_supported_post_types(),
 			get_post_types(
 				[
 					'show_in_rest' => true,
@@ -67,21 +67,31 @@ public static function get_post_types_for_rest_api() {
 		);
 	}
 
+	/**
+	 * Get supported post types.
+	 *
+	 * @return string[] List of post types that support AMP.
+	 */
+	public static function get_supported_post_types() {
+		if ( ! amp_is_legacy() && AMP_Options_Manager::get_option( Option::ALL_TEMPLATES_SUPPORTED ) ) {
+			return self::get_eligible_post_types();
+		}
+		return AMP_Options_Manager::get_option( Option::SUPPORTED_POST_TYPES, [] );
+	}
+
 	/**
 	 * Declare support for post types.
 	 *
 	 * This function should only be invoked through the 'after_setup_theme' action to
 	 * allow plugins/theme to overwrite the post types support.
 	 *
+	 * @codeCoverageIgnore
 	 * @since 0.6
+	 * @deprecated The 'amp' post type support is no longer used at runtime to determine whether AMP is supported.
 	 */
 	public static function add_post_type_support() {
-		if ( ! amp_is_legacy() && AMP_Options_Manager::get_option( Option::ALL_TEMPLATES_SUPPORTED ) ) {
-			$post_types = self::get_eligible_post_types();
-		} else {
-			$post_types = AMP_Options_Manager::get_option( Option::SUPPORTED_POST_TYPES, [] );
-		}
-		foreach ( $post_types as $post_type ) {
+		_deprecated_function( __METHOD__, '1.6' );
+		foreach ( self::get_supported_post_types() as $post_type ) {
 			add_post_type_support( $post_type, self::SLUG );
 		}
 	}
@@ -100,7 +110,7 @@ public static function get_support_errors( $post ) {
 		}
 		$errors = [];
 
-		if ( ! post_type_supports( $post->post_type, self::SLUG ) ) {
+		if ( ! in_array( $post->post_type, self::get_supported_post_types(), true ) ) {
 			$errors[] = 'post-type-support';
 		}