Disable DevTools by default in reader mode; add amp_dev_tools_user_de…

…fault_enabled filter
ampproject · Sep 3, 2020 · f412cca · f412cca
1 parent 8d04091
commit f412cca
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 5 deletions.
diff --git a/src/Admin/DevToolsUserAccess.php b/src/Admin/DevToolsUserAccess.php
@@ -11,6 +11,9 @@
 
 use AmpProject\AmpWP\Infrastructure\Registerable;
 use AmpProject\AmpWP\Infrastructure\Service;
+use AmpProject\AmpWP\Option;
+use AMP_Options_Manager;
+use AMP_Theme_Support;
 use AMP_Validation_Manager;
 use WP_Error;
 use WP_User;
@@ -72,11 +75,24 @@ public function get_user_enabled( $user ) {
 		if ( ! $user instanceof WP_User ) {
 			$user = new WP_User( $user );
 		}
-		$value = $user->get( self::USER_FIELD_DEVELOPER_TOOLS_ENABLED );
-		if ( '' === $value ) {
-			$value = true; // @todo Allow to be filtered so that the default status can be changed.
+		$enabled = $user->get( self::USER_FIELD_DEVELOPER_TOOLS_ENABLED );
+		if ( '' === $enabled ) {
+			// Disable Developer Tools by default when in Reader mode.
+			$enabled = AMP_Theme_Support::READER_MODE_SLUG !== AMP_Options_Manager::get_option( Option::THEME_SUPPORT );
+
+			/**
+			 * Filters whether Developer Tools is enabled by default for a user.
+			 *
+			 * When Reader mode is active, Developer Tools is currently disabled by default.
+			 *
+			 * @since 2.0.1
+			 *
+			 * @param bool $enabled DevTools enabled.
+			 * @param int  $user_id User ID.
+			 */
+			$enabled = (bool) apply_filters( 'amp_dev_tools_user_default_enabled', $enabled, $user->ID );
 		}
-		return rest_sanitize_boolean( $value );
+		return rest_sanitize_boolean( $enabled );
 	}
 
 	/**

diff --git a/tests/php/test-class-dev-tools-user-access.php b/tests/php/test-class-dev-tools-user-access.php
@@ -6,6 +6,7 @@
  */
 
 use AmpProject\AmpWP\Admin\DevToolsUserAccess;
+use AmpProject\AmpWP\Option;
 
 /**
  * Tests for DevToolsUserAccess class.
@@ -71,7 +72,55 @@ public function test_is_user_enabled() {
 	 * @covers ::get_user_enabled
 	 */
 	public function test_get_user_enabled() {
-		$this->assertTrue( $this->dev_tools_user_access->get_user_enabled( self::factory()->user->create_and_get( [ 'role' => 'administrator' ] ) ) );
+		$admin_user = self::factory()->user->create_and_get( [ 'role' => 'administrator' ] );
+
+		// Enabled by default in Transitional mode.
+		AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG );
+		$this->assertTrue( $this->dev_tools_user_access->get_user_enabled( $admin_user ) );
+
+		// Enabled by default in Standard mode.
+		AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::STANDARD_MODE_SLUG );
+		$this->assertTrue( $this->dev_tools_user_access->get_user_enabled( $admin_user->ID ) );
+
+		// Disabled by default in Reader mode.
+		AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::READER_MODE_SLUG );
+		$this->assertFalse( $this->dev_tools_user_access->get_user_enabled( $admin_user->ID ) );
+
+		// Check filter overriding default to be true in Reader mode, but then user forcing it off via user pref.
+		delete_user_meta( $admin_user->ID, DevToolsUserAccess::USER_FIELD_DEVELOPER_TOOLS_ENABLED );
+		remove_all_filters( 'amp_dev_tools_user_default_enabled' );
+		add_filter(
+			'amp_dev_tools_user_default_enabled',
+			function ( $enabled, $user_id ) use ( $admin_user ) {
+				unset( $enabled );
+				$this->assertSame( $user_id, $admin_user->ID );
+				return true;
+			},
+			10,
+			2
+		);
+		AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::READER_MODE_SLUG );
+		$this->assertTrue( $this->dev_tools_user_access->get_user_enabled( $admin_user ) );
+		update_user_meta( $admin_user->ID, DevToolsUserAccess::USER_FIELD_DEVELOPER_TOOLS_ENABLED, 'false' );
+		$this->assertFalse( $this->dev_tools_user_access->get_user_enabled( $admin_user ) );
+
+		// Check filter overriding default to be false in Standard mode, but then user forcing it on via user pref.
+		delete_user_meta( $admin_user->ID, DevToolsUserAccess::USER_FIELD_DEVELOPER_TOOLS_ENABLED );
+		remove_all_filters( 'amp_dev_tools_user_default_enabled' );
+		add_filter(
+			'amp_dev_tools_user_default_enabled',
+			function ( $enabled, $user_id ) use ( $admin_user ) {
+				unset( $enabled );
+				$this->assertSame( $user_id, $admin_user->ID );
+				return false;
+			},
+			10,
+			2
+		);
+		AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::STANDARD_MODE_SLUG );
+		$this->assertFalse( $this->dev_tools_user_access->get_user_enabled( $admin_user ) );
+		update_user_meta( $admin_user->ID, DevToolsUserAccess::USER_FIELD_DEVELOPER_TOOLS_ENABLED, 'true' );
+		$this->assertTrue( $this->dev_tools_user_access->get_user_enabled( $admin_user ) );
 	}
 
 	/**