Boost: refactor Critical CSS: Part 1

projects/plugins/boost/.phpcs.dir.xml

@@ -21,4 +21,7 @@
 		</properties>
 	</rule>
 
+	<rule ref="WordPress.Files.FileName.InvalidClassFileName">
+		<severity>0</severity>
+	</rule>
 </ruleset>

projects/plugins/boost/app/admin/class-admin.php

@@ -14,6 +14,8 @@
 use Automattic\Jetpack_Boost\Lib\Analytics;
 use Automattic\Jetpack_Boost\Lib\Environment_Change_Detector;
 use Automattic\Jetpack_Boost\Lib\Speed_Score;
+use Automattic\Jetpack_Boost\Modules\Modules;
+use Automattic\Jetpack_Boost\REST_API\Permissions\Nonce;
 
 /**
  * Class Admin
@@ -45,7 +47,7 @@ class Admin {
 	 *
 	 * @var Jetpack_Boost Plugin.
 	 */
-	private $jetpack_boost;
+	private $modules;
 
 	/**
 	 * Speed_Score class instance.
@@ -61,14 +63,13 @@ class Admin {
 	 *
 	 * @since    1.0.0
 	 */
-	public function __construct( Jetpack_Boost $jetpack_boost ) {
-		$this->jetpack_boost = $jetpack_boost;
-		$this->speed_score   = new Speed_Score( $jetpack_boost );
+	public function __construct( Modules $modules ) {
+		$this->modules     = $modules;
+		$this->speed_score = new Speed_Score( $modules );
 		Environment_Change_Detector::init();
 
 		add_action( 'init', array( new Analytics(), 'init' ) );
 		add_filter( 'plugin_action_links_' . JETPACK_BOOST_PLUGIN_BASE, array( $this, 'plugin_page_settings_link' ) );
-		add_action( 'rest_api_init', array( $this, 'register_rest_routes' ) );
 		add_action( 'admin_notices', array( $this, 'show_notices' ) );
 		add_action( 'wp_ajax_set_show_rating_prompt', array( $this, 'handle_set_show_rating_prompt' ) );
 		add_filter( 'jetpack_boost_js_constants', array( $this, 'add_js_constants' ) );
@@ -102,7 +103,7 @@ public function enqueue_styles() {
 		$internal_path = apply_filters( 'jetpack_boost_asset_internal_path', 'app/assets/dist/' );
 
 		wp_enqueue_style(
-			$this->jetpack_boost->get_plugin_name() . '-css',
+			'jetpack-boost-css',
 			plugins_url( $internal_path . 'jetpack-boost.css', JETPACK_BOOST_PATH ),
 			array( 'wp-components' ),
 			JETPACK_BOOST_VERSION
@@ -117,7 +118,7 @@ public function enqueue_styles() {
 	public function enqueue_scripts() {
 		$internal_path = apply_filters( 'jetpack_boost_asset_internal_path', 'app/assets/dist/' );
 
-		$admin_js_handle = $this->jetpack_boost->get_plugin_name() . '-admin';
+		$admin_js_handle = 'jetpack-boost-admin';
 
 		wp_register_script(
 			$admin_js_handle,
@@ -127,15 +128,31 @@ public function enqueue_scripts() {
 			true
 		);
 
+		$active_modules    = array_keys( $this->modules->get_active_modules() );
+		$available_modules = array_keys( $this->modules->get_modules() );
+
+		/**
+		 * @TODO: This config here ia bit incorrect - JavaScript relies on config module containing all the modules, even the disabled ones.
+		 *      But that doesn't make much sense, since we're already passing all the available modules separately.
+		 *      The "constants" should be cleaned up a bit, but out of scope at the moment.
+		 *        The config system should be cleaned up a bit further.
+		 */
+		$module_config = array();
+		foreach ( $available_modules as $module_slug ) {
+			$module_config[ $module_slug ] = array(
+				'enabled' => in_array( $module_slug, $active_modules, true ),
+			);
+		}
+
 		// Prepare configuration constants for JavaScript.
 		$constants = array(
 			'version'             => JETPACK_BOOST_VERSION,
 			'api'                 => array(
 				'namespace' => JETPACK_BOOST_REST_NAMESPACE,
 				'prefix'    => JETPACK_BOOST_REST_PREFIX,
 			),
-			'modules'             => $this->jetpack_boost->get_available_modules(),
-			'config'              => $this->jetpack_boost->config()->get_data(),
+			'modules'             => $available_modules,
+			'config'              => $module_config,
 			'locale'              => get_locale(),
 			'site'                => array(
 				'url'       => get_home_url(),
@@ -146,6 +163,15 @@ public function enqueue_scripts() {
 			'preferences'         => array(
 				'showRatingPrompt' => $this->get_show_rating_prompt(),
 			),
+
+
+			/**
+			 * A bit of necessary magic,
+			 * Explained more in the Nonce class.
+			 *
+			 * Nonces are automatically generated when registering routes.
+			 */
+			'nonces'              => Nonce::get_generated_nonces(),
 		);
 
 		// Give each module an opportunity to define extra constants.
@@ -179,7 +205,7 @@ public function plugin_page_settings_link( $links ) {
 	 */
 	public function render_settings() {
 		wp_localize_script(
-			$this->jetpack_boost->get_plugin_name() . '-admin',
+			'jetpack-boost-admin',
 			'wpApiSettings',
 			array(
 				'root'  => esc_url_raw( rest_url() ),
@@ -200,48 +226,6 @@ public function check_for_permissions() {
 		return current_user_can( 'manage_options' );
 	}
 
-	/**
-	 * Register REST routes for settings.
-	 *
-	 * @return void
-	 */
-	public function register_rest_routes() {
-		// Activate and deactivate a module.
-		register_rest_route(
-			JETPACK_BOOST_REST_NAMESPACE,
-			JETPACK_BOOST_REST_PREFIX . '/module/(?P<slug>[a-z\-]+)/status',
-			array(
-				'methods'             => \WP_REST_Server::EDITABLE,
-				'callback'            => array( $this, 'set_module_status' ),
-				'permission_callback' => array( $this, 'check_for_permissions' ),
-			)
-		);
-	}
-
-	/**
-	 * Handler for the /module/(?P<slug>[a-z\-]+)/status endpoint.
-	 *
-	 * @param \WP_REST_Request $request The request object.
-	 *
-	 * @return \WP_REST_Response|\WP_Error The response.
-	 */
-	public function set_module_status( $request ) {
-		$params = $request->get_json_params();
-
-		if ( ! isset( $params['status'] ) ) {
-			return new \WP_Error(
-				'jetpack_boost_error_missing_module_status_param',
-				__( 'Missing status param', 'jetpack-boost' )
-			);
-		}
-
-		$module_slug = $request['slug'];
-		$this->jetpack_boost->set_module_status( (bool) $params['status'], $module_slug );
-
-		return rest_ensure_response(
-			$this->jetpack_boost->get_module_status( $module_slug )
-		);
-	}
 
 	/**
 	 * Show any admin notices from enabled modules.
@@ -250,13 +234,13 @@ public function show_notices() {
 		// Determine if we're already on the settings page.
 		// phpcs:ignore WordPress.Security.NonceVerification.Recommended
 		$on_settings_page = isset( $_GET['page'] ) && self::MENU_SLUG === $_GET['page'];
-		$notices          = $this->jetpack_boost->get_admin_notices();
+		$notices          = $this->get_admin_notices();
 
 		// Filter out any that have been dismissed, unless newer than the dismissal.
 		$dismissed_notices = \get_option( self::DISMISSED_NOTICE_OPTION, array() );
 		$notices           = array_filter(
 			$notices,
-			function ( $notice ) use ( $dismissed_notices ) {
+			function( $notice ) use ( $dismissed_notices ) {
 				$notice_slug = $notice->get_slug();
 
 				return ! in_array( $notice_slug, $dismissed_notices, true );
@@ -281,7 +265,7 @@ function ( $notice ) use ( $dismissed_notices ) {
 	 * @return array List of notice ids.
 	 */
 	private function get_shown_admin_notice_ids() {
-		$notices = $this->jetpack_boost->get_admin_notices();
+		$notices = $this->get_admin_notices();
 		$ids     = array();
 		foreach ( $notices as $notice ) {
 			$ids[] = $notice->get_id();
@@ -290,6 +274,30 @@ private function get_shown_admin_notice_ids() {
 		return $ids;
 	}
 
+	/**
+	 * Returns a list of admin notices to show. Asks each module to provide admin notices the user needs to see.
+	 * @TODO: This is still a code smell. We're carrying the whole modules instance just to get a list of admin notices.
+	 *
+	 * @return \Automattic\Jetpack_Boost\Admin\Admin_Notice[]
+	 */
+	public function get_admin_notices() {
+		$all_notices = array();
+
+		foreach ( $this->modules->get_active_modules() as $module ) {
+
+			if( ! method_exists( $module, 'get_admin_notices' ) ) {
+				continue;
+			}
+			$module_notices = $module->get_admin_notices();
+
+			if ( ! empty( $module_notices ) ) {
+				$all_notices = array_merge( $all_notices, $module_notices );
+			}
+		}
+
+		return $all_notices;
+	}
+
 	/**
 	 * Check for a GET parameter used to dismiss an admin notice.
 	 *

projects/plugins/boost/app/assets/src/js/global.d.ts

@@ -35,7 +35,6 @@ declare global {
 		connection: ConnectionStatus;
 		criticalCssStatus?: CriticalCssStatus;
 		showRatingPromptNonce?: string;
-		criticalCssDismissRecommendationsNonce?: string;
 		criticalCssDismissedRecommendations: string[];
 		site: {
 			url: string;
@@ -44,6 +43,9 @@ declare global {
 		};
 		config: ModulesState;
 		shownAdminNoticeIds: string[];
+		nonces: {
+			[ key: string ]: string;
+		};
 	};
 
 	// Critical CSS Generator library.

projects/plugins/boost/app/assets/src/js/stores/critical-css-recommendations.ts

@@ -6,11 +6,11 @@ import { writable, derived } from 'svelte/store';
 /**
  * Internal dependencies
  */
+import api from '../api/api';
 import { CriticalCssErrorDetails, criticalCssStatus } from './critical-css-status';
 import type { JSONObject } from '../utils/json-types';
 import { objectFilter } from '../utils/object-filter';
 import { sortByFrequency } from '../utils/sort-by-frequency';
-import { makeAdminAjaxRequest } from '../utils/make-admin-ajax-request';
 import { castToString } from '../utils/cast-to-string';
 
 const importantProviders = [
@@ -118,10 +118,9 @@ export function setDismissalError( title: string, error: JSONObject ): void {
  * @param {string} key Key of recommendation to dismiss.
  */
 export async function dismissRecommendation( key: string ): Promise< void > {
-	await makeAdminAjaxRequest( {
-		action: 'dismiss_recommendations',
+	await api.post( '/recommendations/dismiss', {
 		providerKey: key,
-		nonce: Jetpack_Boost.criticalCssDismissRecommendationsNonce,
+		nonce: Jetpack_Boost.nonces[ 'recommendations/dismiss' ],
 	} );
 	dismissed.update( keys => [ ...keys, key ] );
 }
@@ -130,9 +129,8 @@ export async function dismissRecommendation( key: string ): Promise< void > {
  * Clear all the dismissed recommendations.
  */
 export async function clearDismissedRecommendations(): Promise< void > {
-	await makeAdminAjaxRequest( {
-		action: 'reset_dismissed_recommendations',
-		nonce: Jetpack_Boost.criticalCssDismissRecommendationsNonce,
+	await api.post( '/recommendations/reset', {
+		nonce: Jetpack_Boost.nonces[ 'recommendations/reset' ],
 	} );
 	dismissed.set( [] );
 }