Merge pull request #6534 from ampproject/fix/6521-block-editor-assets

westonruter · web-flow · commit cd5ceabb247f · 2021-08-18T17:26:40.000-07:00
Only load block editor related assets when it is used for a post
diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php
@@ -146,8 +146,8 @@ public function enqueue_admin_assets() {
 		$validate = (
 			isset( $screen->base ) &&
 			'post' === $screen->base &&
-			( ! isset( $screen->is_block_editor ) || ! $screen->is_block_editor ) &&
-			in_array( $post->post_type, AMP_Post_Type_Support::get_eligible_post_types(), true )
+			empty( $screen->is_block_editor ) &&
+			in_array( $post->post_type, AMP_Post_Type_Support::get_supported_post_types(), true )
 		);
 
 		if ( ! $validate ) {
@@ -216,12 +216,16 @@ public function enqueue_admin_assets() {
 	 */
 	public function enqueue_block_assets() {
 		$post = get_post();
-		if ( ! $post instanceof WP_Post || ! in_array( $post->post_type, AMP_Post_Type_Support::get_eligible_post_types(), true ) ) {
-			return;
+
+		// Block validation script uses features only available beginning with WP 5.6.
+		$dependency_support = Services::get( 'dependency_support' );
+		if ( ! $dependency_support->has_support() ) {
+			return; // @codeCoverageIgnore
 		}
 
+		// Only enqueue scripts on the block editor for AMP-enabled posts.
 		$editor_support = Services::get( 'editor.editor_support' );
-		if ( ! $editor_support->editor_supports_amp_block_editor_features() ) {
+		if ( ! $editor_support->is_current_screen_block_editor_for_amp_enabled_post_type() ) {
 			return;
 		}
 
@@ -340,7 +344,7 @@ public function render_status( $post ) {
 		$verify = (
 			isset( $post->ID )
 			&&
-			in_array( $post->post_type, AMP_Post_Type_Support::get_eligible_post_types(), true )
+			in_array( $post->post_type, AMP_Post_Type_Support::get_supported_post_types(), true )
 			&&
 			current_user_can( 'edit_post', $post->ID )
 		);
diff --git a/includes/templates/amp-enabled-classic-editor-toggle.php b/includes/templates/amp-enabled-classic-editor-toggle.php
@@ -29,7 +29,7 @@
 	<?php if ( ! Services::get( 'dependency_support' )->has_support_from_core() ) : ?>
 		<div class="notice notice-info notice-alt inline">
 			<p>
-				<?php esc_html_e( 'Your version of WordPress is too old manage whether AMP is enabled. Please upgrade.', 'amp' ); ?>
+				<?php esc_html_e( 'Your version of WordPress is too old to manage whether AMP is enabled. Please upgrade.', 'amp' ); ?>
 			</p>
 		</div>
 	<?php else : ?>
diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php
@@ -2175,13 +2175,18 @@ public static function enqueue_block_validation() {
 			return;
 		}
 
-		$editor_support = Services::get( 'editor.editor_support' );
-
 		// Block validation script uses features only available beginning with WP 5.6.
-		if ( ! $editor_support->editor_supports_amp_block_editor_features() ) {
+		$dependency_support = Services::get( 'dependency_support' );
+		if ( ! $dependency_support->has_support() ) {
 			return; // @codeCoverageIgnore
 		}
 
+		// Only enqueue scripts on the block editor for AMP-enabled posts.
+		$editor_support = Services::get( 'editor.editor_support' );
+		if ( ! $editor_support->is_current_screen_block_editor_for_amp_enabled_post_type() ) {
+			return;
+		}
+
 		$slug = 'amp-block-validation';
 
 		$asset_file   = AMP__DIR__ . '/assets/js/' . $slug . '.asset.php';
diff --git a/src/Editor/EditorSupport.php b/src/Editor/EditorSupport.php
@@ -44,27 +44,11 @@ public function register() {
 	 * Shows a notice in the editor if the Gutenberg or WP version prevents plugin features from working.
 	 */
 	public function maybe_show_notice() {
-		$screen = get_current_screen();
-
-		if ( ! $screen ) {
-			return;
-		}
-
-		$is_block_editor = (
-			! empty( $screen->is_block_editor )
-			||
-			// Applicable to Gutenberg v5.5.0 and older.
-			( function_exists( 'is_gutenberg_page' ) && is_gutenberg_page() )
-		);
-		if ( ! $is_block_editor ) {
+		if ( $this->dependency_support->has_support() ) {
 			return;
 		}
 
-		if ( ! in_array( get_post_type(), AMP_Post_Type_Support::get_eligible_post_types(), true ) ) {
-			return;
-		}
-
-		if ( $this->editor_supports_amp_block_editor_features() ) {
+		if ( ! $this->is_current_screen_block_editor_for_amp_enabled_post_type() ) {
 			return;
 		}
 
@@ -86,11 +70,18 @@ function () {
 	}
 
 	/**
-	 * Returns whether the editor in the current environment supports plugin features.
+	 * Returns whether the current screen is using the block editor and the post being edited supports AMP.
 	 *
 	 * @return bool
 	 */
-	public function editor_supports_amp_block_editor_features() {
-		return $this->dependency_support->has_support();
+	public function is_current_screen_block_editor_for_amp_enabled_post_type() {
+		$screen = get_current_screen();
+		return (
+			$screen
+			&&
+			! empty( $screen->is_block_editor )
+			&&
+			in_array( get_post_type(), AMP_Post_Type_Support::get_supported_post_types(), true )
+		);
 	}
 }
diff --git a/tests/php/src/Editor/EditorSupportTest.php b/tests/php/src/Editor/EditorSupportTest.php
@@ -6,18 +6,23 @@
 use AmpProject\AmpWP\Editor\EditorSupport;
 use AmpProject\AmpWP\Infrastructure\Registerable;
 use AmpProject\AmpWP\Infrastructure\Service;
+use AmpProject\AmpWP\Tests\Helpers\WithBlockEditorSupport;
 use AmpProject\AmpWP\Tests\TestCase;
 
 /** @coversDefaultClass \AmpProject\AmpWP\Editor\EditorSupport */
 final class EditorSupportTest extends TestCase {
 
+	use WithBlockEditorSupport;
+
 	/** @var EditorSupport */
 	private $instance;
 
 	public function setUp() {
 		parent::setUp();
 
 		$this->instance = new EditorSupport( new DependencySupport() );
+
+		unset( $GLOBALS['current_screen'], $GLOBALS['wp_scripts'] );
 	}
 
 	public function test_it_can_be_initialized() {
@@ -33,20 +38,34 @@ public function test_register() {
 		$this->assertEquals( 99, has_action( 'admin_enqueue_scripts', [ $this->instance, 'maybe_show_notice' ] ) );
 	}
 
-	public function test_editor_supports_amp_block_editor_features() {
-		if (
-			defined( 'GUTENBERG_VERSION' )
-			&&
-			version_compare( GUTENBERG_VERSION, DependencySupport::GB_MIN_VERSION, '>=' )
-		) {
-			$this->assertTrue( $this->instance->editor_supports_amp_block_editor_features() );
-		} else {
-			if ( version_compare( get_bloginfo( 'version' ), DependencySupport::WP_MIN_VERSION, '>=' ) ) {
-				$this->assertTrue( $this->instance->editor_supports_amp_block_editor_features() );
-			} else {
-				$this->assertFalse( $this->instance->editor_supports_amp_block_editor_features() );
-			}
-		}
+	/**
+	 * Test data for test_supports_current_screen().
+	 *
+	 * @return array
+	 */
+	public function get_data_for_test_supports_current_screen() {
+		return [
+			'supports post type and amp'         => [ true, true, true ],
+			'supports only post type'            => [ true, false, false ],
+			'supports only amp'                  => [ false, true, false ],
+			'does not support post type nor amp' => [ false, false, false ],
+		];
+	}
+
+	/**
+	 * @covers ::is_current_screen_block_editor_for_amp_enabled_post_type()
+	 * @dataProvider get_data_for_test_supports_current_screen()
+	 *
+	 * @param bool $post_type_uses_block_editor Whether post type can be edited in the block editor.
+	 * @param bool $post_type_supports_amp      Whether post type supports AMP.
+	 * @param bool $expected_result             Expected result for test assertions.
+	 */
+	public function test_supports_current_screen( $post_type_uses_block_editor, $post_type_supports_amp, $expected_result ) {
+		$this->setup_environment( $post_type_uses_block_editor, $post_type_supports_amp );
+
+		// Note: Without Gutenberg being installed on WP 4.9, the expected result would be `false`
+		// when `$post_type_uses_block_editor` and `$post_type_supports_amp` are `true`.
+		$this->assertSame( $expected_result, $this->instance->is_current_screen_block_editor_for_amp_enabled_post_type() );
 	}
 
 	/** @covers ::maybe_show_notice() */
@@ -57,110 +76,56 @@ public function test_dont_show_notice_if_no_screen_defined() {
 
 	/** @covers ::maybe_show_notice() */
 	public function test_dont_show_notice_for_unsupported_post_type() {
-		global $post;
-
-		set_current_screen( 'post.php' );
-		register_post_type( 'my-post-type' );
-		$post = $this->factory()->post->create( [ 'post_type' => 'my-post-type' ] );
-		setup_postdata( get_post( $post ) );
-
-		wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) );
+		$this->setup_environment( true, false );
 
 		$this->instance->maybe_show_notice();
 		$this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) );
-		unset( $GLOBALS['current_screen'] );
-		unset( $GLOBALS['wp_scripts'] );
 	}
 
 	/** @covers ::maybe_show_notice() */
 	public function test_show_notice_for_supported_post_type() {
-		global $post;
-
 		if ( version_compare( get_bloginfo( 'version' ), DependencySupport::WP_MIN_VERSION, '<' ) ) {
 			$this->markTestSkipped();
 		}
 
-		set_current_screen( 'post.php' );
-		$post = $this->factory()->post->create();
-		setup_postdata( get_post( $post ) );
-
-		wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) );
+		$this->setup_environment( true, true );
 
 		$this->instance->maybe_show_notice();
-		if ( $this->instance->editor_supports_amp_block_editor_features() ) {
+		if ( $this->instance->is_current_screen_block_editor_for_amp_enabled_post_type() ) {
 			$this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) );
 		} else {
 			$this->assertStringContainsString(
 				'AMP functionality is not available',
 				wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false )
 			);
 		}
-		unset( $GLOBALS['current_screen'] );
-		unset( $GLOBALS['wp_scripts'] );
 	}
 
 	/** @covers ::maybe_show_notice() */
 	public function test_maybe_show_notice_for_unsupported_user() {
-		global $post;
-
-		set_current_screen( 'post.php' );
-		$post = $this->factory()->post->create();
-		setup_postdata( get_post( $post ) );
+		$this->setup_environment( true, true );
+		wp_set_current_user( self::factory()->user->create() );
 
 		$this->instance->maybe_show_notice();
 
 		$this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) );
-		unset( $GLOBALS['current_screen'] );
-		unset( $GLOBALS['wp_scripts'] );
-	}
-
-	/** @covers ::maybe_show_notice() */
-	public function test_maybe_show_notice_with_cpt_supporting_gutenberg_but_not_amp() {
-		global $post;
-
-		if ( ! $this->instance->editor_supports_amp_block_editor_features() ) {
-			$this->markTestSkipped();
-		}
-
-		register_post_type(
-			'my-gb-post-type',
-			[
-				'public'       => true,
-				'show_in_rest' => true,
-				'supports'     => [ 'editor' ],
-			]
-		);
-
-		set_current_screen( 'post.php' );
-		$post = $this->factory()->post->create( [ 'post_type' => 'my-gb-post-type' ] );
-		setup_postdata( get_post( $post ) );
-		wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) );
-
-		$this->instance->maybe_show_notice();
-		$this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) );
-		unset( $GLOBALS['current_screen'] );
-		unset( $GLOBALS['wp_scripts'] );
 	}
 
 	/** @covers ::maybe_show_notice() */
 	public function test_maybe_show_notice_for_gutenberg_4_9() {
-		global $post;
 		if ( ! defined( 'GUTENBERG_VERSION' ) || version_compare( GUTENBERG_VERSION, '4.9.0', '>' ) ) {
 			$this->markTestSkipped( 'Test only applicable to Gutenberg v4.9.0 and older.' );
 		}
 
-		$this->assertFalse( $this->instance->editor_supports_amp_block_editor_features() );
+		$this->setup_environment( true, true );
+		// WP < 5.0 doesn't include the block editor, but Gutenberg would be installed, so it should be supported.
+		$this->assertTrue( $this->instance->is_current_screen_block_editor_for_amp_enabled_post_type() );
 
 		gutenberg_register_packages_scripts();
-		set_current_screen( 'post.php' );
-		$post = $this->factory()->post->create();
-		setup_postdata( get_post( $post ) );
 		wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) );
 
 		$this->instance->maybe_show_notice();
 		$inline_script = wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false );
 		$this->assertStringContainsString( 'AMP functionality is not available', $inline_script );
-		unset( $GLOBALS['current_screen'] );
-		unset( $GLOBALS['wp_scripts'] );
 	}
 }
diff --git a/tests/php/src/Helpers/WithBlockEditorSupport.php b/tests/php/src/Helpers/WithBlockEditorSupport.php
@@ -0,0 +1,49 @@
+<?php
+/**
+ * Trait WithoutBlockPreRendering.
+ *
+ * @package AmpProject\AmpWP
+ */
+
+namespace AmpProject\AmpWP\Tests\Helpers;
+
+use AMP_Options_Manager;
+use AmpProject\AmpWP\Option;
+use WP_User;
+
+/**
+ * Helper trait to help with setting up the test environment for block editor support.
+ */
+trait WithBlockEditorSupport {
+
+	/**
+	 * Setup test environment to ensure the correct result for ::supports_current_screen().
+	 *
+	 * @param bool   $post_type_uses_block_editor Whether the post type uses the block editor.
+	 * @param bool   $post_type_supports_amp      Whether the post type supports AMP.
+	 * @param string $post_type                   Post type ID.
+	 */
+	public function setup_environment( $post_type_uses_block_editor, $post_type_supports_amp, $post_type = 'foo' ) {
+		if ( $post_type_uses_block_editor ) {
+			set_current_screen( 'post.php' );
+			add_filter( 'replace_editor', '__return_false' );
+			add_filter( 'use_block_editor_for_post', '__return_true' );
+		}
+
+		if ( $post_type_supports_amp ) {
+			register_post_type( $post_type, [ 'public' => true ] );
+			$GLOBALS['post'] = self::factory()->post->create( [ 'post_type' => $post_type ] );
+
+			$previous_user = wp_get_current_user();
+			wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) );
+
+			$supported_post_types = array_merge(
+				AMP_Options_Manager::get_option( Option::SUPPORTED_POST_TYPES ),
+				[ $post_type ]
+			);
+			AMP_Options_Manager::update_option( Option::SUPPORTED_POST_TYPES, $supported_post_types );
+
+			wp_set_current_user( $previous_user instanceof WP_User ? $previous_user->ID : $previous_user );
+		}
+	}
+}
diff --git a/tests/php/test-class-amp-meta-box.php b/tests/php/test-class-amp-meta-box.php
@@ -284,16 +284,11 @@ public function test_render_status() {
 			$this->assertStringContainsString( $no_support_notice, $output );
 		}
 
-		// Post type no longer supports AMP, so no status input.
+		// Post type no longer supports AMP, so no status input (and no mention of AMP at all).
 		$supported_post_types = array_diff( AMP_Options_Manager::get_option( Option::SUPPORTED_POST_TYPES ), [ 'post' ] );
 		AMP_Options_Manager::update_option( Option::SUPPORTED_POST_TYPES, $supported_post_types );
 		$output = get_echo( [ $this->instance, 'render_status' ], [ $post ] );
-		if ( Services::get( 'dependency_support' )->has_support_from_core() ) {
-			$this->assertStringContainsString( 'This post type is not', $output );
-			$this->assertStringNotContainsString( $checkbox_enabled, $output );
-		} else {
-			$this->assertStringContainsString( $no_support_notice, $output );
-		}
+		$this->assertEmpty( $output );
 		$supported_post_types[] = 'post';
 		AMP_Options_Manager::update_option( Option::SUPPORTED_POST_TYPES, $supported_post_types );
 
diff --git a/tests/php/validation/test-class-amp-validation-manager.php b/tests/php/validation/test-class-amp-validation-manager.php
