Do async site scan after theme activation

This utilizes the async site scan notice that is shown after plugin activation. Right now, both on `plugins.php` and on `themes.php` screens, the scan results will include plugin and theme issues.
ampproject · Feb 21, 2022 · 9566a93 · 9566a93
1 parent 0ba4427
commit 9566a93
Show file tree

Hide file tree

Showing 8 changed files with 255 additions and 80 deletions.
diff --git a/.phpstorm.meta.php b/.phpstorm.meta.php
@@ -8,6 +8,7 @@
 		// TODO: I'd like to use AmpWpPlugin::SERVICES directly here but it doesn't seem to work.
 		map( [
 			'admin.analytics_menu'               => \AmpProject\AmpWP\Admin\AnalyticsOptionsSubmenu::class,
+			'admin.after_activation_site_scan'   => Admin\AfterActivationSiteScan::class,
 			'admin.google_fonts'                 => \AmpProject\AmpWP\Admin\GoogleFonts::class,
 			'admin.onboarding_menu'              => \AmpProject\AmpWP\Admin\OnboardingWizardSubmenu::class,
 			'admin.onboarding_wizard'            => \AmpProject\AmpWP\Admin\OnboardingWizardSubmenuPage::class,
@@ -45,7 +46,6 @@
 			'paired_routing'                     => \AmpProject\AmpWP\PairedRouting::class,
 			'paired_url'                         => \AmpProject\AmpWP\PairedUrl::class,
 			'plugin_activation_notice'           => \AmpProject\AmpWP\Admin\PluginActivationNotice::class,
-			'plugin_activation_site_scan'        => \AmpProject\AmpWP\Admin\PluginActivationSiteScan::class,
 			'plugin_registry'                    => \AmpProject\AmpWP\PluginRegistry::class,
 			'plugin_suppression'                 => \AmpProject\AmpWP\PluginSuppression::class,
 			'reader_theme_loader'                => \AmpProject\AmpWP\ReaderThemeLoader::class,

diff --git a/assets/src/admin/site-scan-notice/index.js b/assets/src/admin/site-scan-notice/index.js
@@ -4,6 +4,7 @@
 import PropTypes from 'prop-types';
 import {
 	APP_ROOT_ID,
+	APP_ROOT_SIBLING_ID,
 	OPTIONS_REST_PATH,
 	SCANNABLE_URLS_REST_PATH,
 	VALIDATE_NONCE,
@@ -24,6 +25,7 @@ import { ErrorContextProvider } from '../../components/error-context-provider';
 import { OptionsContextProvider } from '../../components/options-context-provider';
 import { PluginsContextProvider } from '../../components/plugins-context-provider';
 import { SiteScanContextProvider } from '../../components/site-scan-context-provider';
+import { ThemesContextProvider } from '../../components/themes-context-provider';
 import { ErrorScreen } from '../../components/error-screen';
 import { ErrorBoundary } from '../../components/error-boundary';
 import { SiteScanNotice } from './notice';
@@ -50,12 +52,14 @@ function Providers( { children } ) {
 					populateDefaultValues={ false }
 				>
 					<PluginsContextProvider>
-						<SiteScanContextProvider
-							scannableUrlsRestPath={ SCANNABLE_URLS_REST_PATH }
-							validateNonce={ VALIDATE_NONCE }
-						>
-							{ children }
-						</SiteScanContextProvider>
+						<ThemesContextProvider>
+							<SiteScanContextProvider
+								scannableUrlsRestPath={ SCANNABLE_URLS_REST_PATH }
+								validateNonce={ VALIDATE_NONCE }
+							>
+								{ children }
+							</SiteScanContextProvider>
+						</ThemesContextProvider>
 					</PluginsContextProvider>
 				</OptionsContextProvider>
 			</ErrorBoundary>
@@ -67,10 +71,18 @@ Providers.propTypes = {
 };
 
 domReady( () => {
-	const root = document.getElementById( APP_ROOT_ID );
+	let root = document.getElementById( APP_ROOT_ID );
 
 	if ( ! root ) {
-		return;
+		const rootSibling = document.getElementById( APP_ROOT_SIBLING_ID );
+
+		if ( ! rootSibling ) {
+			return;
+		}
+
+		root = document.createElement( 'div' );
+		root.setAttribute( 'id', APP_ROOT_ID );
+		rootSibling.after( root );
 	}
 
 	errorHandler = ( event ) => {

diff --git a/assets/src/admin/site-scan-notice/notice.js b/assets/src/admin/site-scan-notice/notice.js
@@ -1,11 +1,3 @@
-/**
- * External dependencies
- */
-import {
-	AMP_COMPATIBLE_PLUGINS_URL,
-	SETTINGS_LINK,
-} from 'amp-site-scan-notice'; // From WP inline script.
-
 /**
  * WordPress dependencies
  */
@@ -24,12 +16,8 @@ import {
 	AmpAdminNotice,
 } from '../../components/amp-admin-notice';
 import { Loading } from '../../components/loading';
-import { isExternalUrl } from '../../common/helpers/is-external-url';
 import { PluginsWithAmpIncompatibility } from './plugins-with-amp-incompatibility';
-
-// Define Plugin Suppression link.
-const PLUGIN_SUPPRESSION_LINK = new URL( SETTINGS_LINK );
-PLUGIN_SUPPRESSION_LINK.hash = 'plugin-suppression';
+import { ThemeWithAmpIncompatibility } from './theme-with-amp-incompatibility';
 
 export function SiteScanNotice() {
 	const {
@@ -41,6 +29,7 @@ export function SiteScanNotice() {
 		isInitializing,
 		isReady,
 		pluginsWithAmpIncompatibility,
+		themesWithAmpIncompatibility,
 		startSiteScan,
 	} = useContext( SiteScan );
 
@@ -72,32 +61,29 @@ export function SiteScanNotice() {
 		);
 	}
 
-	if ( isCompleted && pluginsWithAmpIncompatibility.length === 0 ) {
-		return (
-			<AmpAdminNotice type={ AMP_ADMIN_NOTICE_TYPE_SUCCESS } { ...commonNoticeProps }>
-				<p>
-					{ __( 'No AMP compatibility issues detected.', 'amp' ) }
-				</p>
-			</AmpAdminNotice>
-		);
-	}
+	if ( isCompleted ) {
+		let elements = [
+			pluginsWithAmpIncompatibility.length > 0 ? <PluginsWithAmpIncompatibility pluginsWithAmpIncompatibility={ pluginsWithAmpIncompatibility } /> : null,
+			themesWithAmpIncompatibility.length > 0 ? <ThemeWithAmpIncompatibility themeWithAmpIncompatibility={ themesWithAmpIncompatibility[ 0 ] } /> : null,
+		];
+
+		// Display the theme information at the top when on the `themes.php` screen.
+		if ( document.location.href.includes( 'themes.php' ) ) {
+			elements = elements.reverse();
+		}
+
+		elements = elements.filter( Boolean );
 
-	if ( isCompleted && pluginsWithAmpIncompatibility.length > 0 ) {
 		return (
-			<AmpAdminNotice type={ AMP_ADMIN_NOTICE_TYPE_WARNING } { ...commonNoticeProps }>
-				<PluginsWithAmpIncompatibility pluginsWithAmpIncompatibility={ pluginsWithAmpIncompatibility } />
-				<div className="amp-site-scan-notice__cta">
-					<a href={ PLUGIN_SUPPRESSION_LINK } className="button">
-						{ __( 'Review Plugin Suppression', 'amp' ) }
-					</a>
-					<a
-						href={ AMP_COMPATIBLE_PLUGINS_URL }
-						className="button"
-						{ ...isExternalUrl( AMP_COMPATIBLE_PLUGINS_URL ) ? { target: '_blank', rel: 'noopener noreferrer' } : {} }
-					>
-						{ __( 'View AMP-Compatible Plugins', 'amp' ) }
-					</a>
-				</div>
+			<AmpAdminNotice
+				type={ elements.length ? AMP_ADMIN_NOTICE_TYPE_WARNING : AMP_ADMIN_NOTICE_TYPE_SUCCESS }
+				{ ...commonNoticeProps }
+			>
+				{ elements.length ? elements : (
+					<p>
+						{ __( 'No AMP compatibility issues detected.', 'amp' ) }
+					</p>
+				) }
 			</AmpAdminNotice>
 		);
 	}

diff --git a/assets/src/admin/site-scan-notice/plugins-with-amp-incompatibility.js b/assets/src/admin/site-scan-notice/plugins-with-amp-incompatibility.js
@@ -2,18 +2,24 @@
  * External dependencies
  */
 import PropTypes from 'prop-types';
+import { AMP_COMPATIBLE_PLUGINS_URL, SETTINGS_LINK } from 'amp-site-scan-notice'; // From WP inline script.
 
 /**
  * WordPress dependencies
  */
 import { useContext } from '@wordpress/element';
-import { _n, sprintf } from '@wordpress/i18n';
+import { __, _n, sprintf } from '@wordpress/i18n';
 
 /**
  * Internal dependencies
  */
 import { Plugins } from '../../components/plugins-context-provider';
 import { getPluginSlugFromFile } from '../../common/helpers/get-plugin-slug-from-file';
+import { isExternalUrl } from '../../common/helpers/is-external-url';
+
+// Define Plugin Suppression link.
+const PLUGIN_SUPPRESSION_LINK = new URL( SETTINGS_LINK );
+PLUGIN_SUPPRESSION_LINK.hash = 'plugin-suppression';
 
 /**
  * Render a DETAILS element for each plugin causing AMP incompatibilities.
@@ -77,6 +83,18 @@ export function PluginsWithAmpIncompatibility( { pluginsWithAmpIncompatibility }
 					</ul>
 				</details>
 			) ) }
+			<div className="amp-site-scan-notice__cta">
+				<a href={ PLUGIN_SUPPRESSION_LINK } className="button">
+					{ __( 'Review Plugin Suppression', 'amp' ) }
+				</a>
+				<a
+					href={ AMP_COMPATIBLE_PLUGINS_URL }
+					className="button"
+					{ ...isExternalUrl( AMP_COMPATIBLE_PLUGINS_URL ) ? { target: '_blank', rel: 'noopener noreferrer' } : {} }
+				>
+					{ __( 'View AMP-Compatible Plugins', 'amp' ) }
+				</a>
+			</div>
 		</>
 	);
 }
@@ -87,5 +105,5 @@ PluginsWithAmpIncompatibility.propTypes = {
 			slug: PropTypes.string,
 			urls: PropTypes.arrayOf( PropTypes.string ),
 		} ),
-	),
+	).isRequired,
 };
diff --git a/assets/src/admin/site-scan-notice/theme-with-amp-incompatibility.js b/assets/src/admin/site-scan-notice/theme-with-amp-incompatibility.js
@@ -0,0 +1,67 @@
+/**
+ * WordPress dependencies
+ */
+import { createInterpolateElement, useContext } from '@wordpress/element';
+import { __, sprintf } from '@wordpress/i18n';
+
+/**
+ * External dependencies
+ */
+import PropTypes from 'prop-types';
+import { AMP_COMPATIBLE_THEMES_URL } from 'amp-site-scan-notice'; // From WP inline script.
+
+/**
+ * Internal dependencies
+ */
+import { Themes } from '../../components/themes-context-provider';
+import { isExternalUrl } from '../../common/helpers/is-external-url';
+
+/**
+ * Render a message saying a theme is not AMP compatible.
+ *
+ * @param {Object} props                             Component props.
+ * @param {Object} props.themeWithAmpIncompatibility Theme with AMP incompatibilities.
+ */
+export function ThemeWithAmpIncompatibility( { themeWithAmpIncompatibility } ) {
+	const { fetchingThemes, themes } = useContext( Themes );
+
+	if ( fetchingThemes ) {
+		return null;
+	}
+
+	const themeMeta = themes.find( ( theme ) => theme.stylesheet === themeWithAmpIncompatibility.slug );
+	const themeName = themeMeta?.name?.rendered ?? themeMeta?.name;
+
+	return (
+		<>
+			<p>
+				{ createInterpolateElement(
+					sprintf(
+						// translators: %s stands for a theme name.
+						__( 'AMP compatibility issues discovered with the <b>%s</b> theme.', 'amp' ),
+						themeName,
+					),
+					{
+						b: <strong />,
+					},
+				) }
+			</p>
+			<div className="amp-site-scan-notice__cta">
+				<a
+					href={ AMP_COMPATIBLE_THEMES_URL }
+					className="button"
+					{ ...isExternalUrl( AMP_COMPATIBLE_THEMES_URL ) ? { target: '_blank', rel: 'noopener noreferrer' } : {} }
+				>
+					{ __( 'View AMP-Compatible Themes', 'amp' ) }
+				</a>
+			</div>
+		</>
+	);
+}
+
+ThemeWithAmpIncompatibility.propTypes = {
+	themeWithAmpIncompatibility: PropTypes.shape( {
+		slug: PropTypes.string,
+		urls: PropTypes.arrayOf( PropTypes.string ),
+	} ).isRequired,
+};
diff --git a/src/Admin/PluginActivationSiteScan.php → src/Admin/AfterActivationSiteScan.php b/src/Admin/PluginActivationSiteScan.php → src/Admin/AfterActivationSiteScan.php
@@ -1,6 +1,6 @@
 <?php
 /**
- * Class PluginActivationSiteScan.
+ * Class AfterActivationSiteScan.
  *
  * Does an async Site Scan whenever any plugin is activated.
  *
@@ -21,12 +21,12 @@
 use AmpProject\AmpWP\Services;
 
 /**
- * Class PluginActivationSiteScan
+ * Class AfterActivationSiteScan
  *
  * @since 2.2
  * @internal
  */
-final class PluginActivationSiteScan implements Conditional, Delayed, HasRequirements, Service, Registerable {
+final class AfterActivationSiteScan implements Conditional, Delayed, HasRequirements, Service, Registerable {
 	/**
 	 * Handle for JS file.
 	 *
@@ -41,6 +41,16 @@ final class PluginActivationSiteScan implements Conditional, Delayed, HasRequire
 	 */
 	const APP_ROOT_ID = 'amp-site-scan-notice';
 
+	/**
+	 * HTML ID for the app root sibling element.
+	 *
+	 * Since there is no action for adding notice on `themes.php` screen, we need to inject React root element with JS.
+	 * It is an ID of a success notice saying "New theme activated. Visit site" on the `themes.php` screen.
+	 *
+	 * @var string
+	 */
+	const APP_ROOT_SIBLING_ID = 'message2';
+
 	/**
 	 * RESTPreloader instance.
 	 *
@@ -81,15 +91,25 @@ public static function is_needed() {
 			&&
 			! is_network_admin()
 			&&
-			'plugins.php' === $pagenow
+			AMP_Validation_Manager::has_cap()
 			&&
 			(
-				! empty( $_GET['activate'] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended
+				(
+					'plugins.php' === $pagenow
+					&&
+					(
+						! empty( $_GET['activate'] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended
+						||
+						! empty( $_GET['activate-multi'] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended
+					)
+				)
 				||
-				! empty( $_GET['activate-multi'] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended
+				(
+					'themes.php' === $pagenow
+					&&
+					! empty( $_GET['activated'] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended
+				)
 			)
-			&&
-			AMP_Validation_Manager::has_cap()
 		);
 	}
 
@@ -145,7 +165,9 @@ public function enqueue_assets() {
 
 		$data = [
 			'AMP_COMPATIBLE_PLUGINS_URL' => $this->get_amp_compatible_plugins_url(),
+			'AMP_COMPATIBLE_THEMES_URL'  => $this->get_amp_compatible_themes_url(),
 			'APP_ROOT_ID'                => self::APP_ROOT_ID,
+			'APP_ROOT_SIBLING_ID'        => self::APP_ROOT_SIBLING_ID,
 			'OPTIONS_REST_PATH'          => '/amp/v1/options',
 			'SETTINGS_LINK'              => menu_page_url( AMP_Options_Manager::OPTION_NAME, false ),
 			'SCANNABLE_URLS_REST_PATH'   => '/amp/v1/scannable-urls',
@@ -180,6 +202,22 @@ protected function get_amp_compatible_plugins_url() {
 		return 'https://amp-wp.org/ecosystem/plugins/';
 	}
 
+	/**
+	 * Get a URL to AMP compatible themes directory.
+	 *
+	 * For users capable of installing themes, the link should lead to the Theme install page.
+	 * Other users will be directed to the themes page on amp-wp.org.
+	 *
+	 * @return string URL to AMP compatible themes directory.
+	 */
+	protected function get_amp_compatible_themes_url() {
+		if ( current_user_can( 'switch_themes' ) ) {
+			return admin_url( '/theme-install.php?tab=amp-compatible' );
+		}
+
+		return 'https://amp-wp.org/ecosystem/themes/';
+	}
+
 	/**
 	 * Adds REST paths to preload.
 	 */
@@ -197,6 +235,11 @@ protected function add_preload_rest_paths() {
 				[ 'author', 'name', 'plugin', 'status', 'version' ],
 				'/wp/v2/plugins'
 			),
+			add_query_arg(
+				'_fields',
+				[ 'author', 'name', 'status', 'stylesheet', 'version' ],
+				'/wp/v2/themes'
+			),
 			'/wp/v2/users/me',
 		];