Skip to content

Commit

Permalink
Boost: Fix case where it's possible to not store CSS coming from the …
Browse files Browse the repository at this point in the history
…cloud (#38985)

* Fix case where it's possible to send an empty list of providers

* Fix resetting critical css state without providers for Cloud CSS

* Reset regeneration flag when invalidating cloud css

* add changelog

* Fix incorrect number of params

* Fix static analysis issues

* Clean up admin notice transient when requesting cloud css
  • Loading branch information
dilirity authored Aug 22, 2024
1 parent ba84857 commit 7624a6e
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 8 deletions.
3 changes: 1 addition & 2 deletions projects/plugins/boost/.phan/baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// PhanTypeMismatchReturnProbablyReal : 10+ occurrences
// PhanTypeArraySuspicious : 9 occurrences
// PhanTypeMismatchArgument : 8 occurrences
// PhanParamTooMany : 7 occurrences
// PhanParamTooMany : 6 occurrences
// PhanPossiblyUndeclaredVariable : 6 occurrences
// PhanUndeclaredConstant : 5 occurrences
// PhanUndeclaredFunction : 4 occurrences
Expand Down Expand Up @@ -50,7 +50,6 @@
'app/data-sync/Performance_History_Entry.php' => ['PhanPluginDuplicateConditionalNullCoalescing', 'PhanTypeArraySuspicious'],
'app/lib/class-cli.php' => ['PhanPluginDuplicateConditionalNullCoalescing', 'PhanTypeMismatchArgument'],
'app/lib/critical-css/Critical_CSS_State.php' => ['PhanPluginDuplicateConditionalNullCoalescing', 'PhanTypeArraySuspiciousNullable'],
'app/lib/critical-css/Regenerate.php' => ['PhanParamTooMany'],
'app/lib/critical-css/source-providers/providers/Archive_Provider.php' => ['PhanTypeMismatchReturnProbablyReal'],
'app/lib/critical-css/source-providers/providers/Post_ID_Provider.php' => ['PhanTypeMismatchReturnProbablyReal'],
'app/lib/critical-css/source-providers/providers/Provider.php' => ['PhanTypeMismatchArgumentInternal'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@

namespace Automattic\Jetpack_Boost\Lib\Critical_CSS;

use Automattic\Jetpack_Boost\Admin\Regenerate_Admin_Notice;
use Automattic\Jetpack_Boost\Lib\Boost_Health;
use Automattic\Jetpack_Boost\Lib\Critical_CSS\Source_Providers\Source_Providers;
use Automattic\Jetpack_Boost\Modules\Modules_Setup;
use Automattic\Jetpack_Boost\Modules\Optimizations\Cloud_CSS\Cloud_CSS;
use Automattic\Jetpack_Boost\Modules\Optimizations\Cloud_CSS\Cloud_CSS_Followup;

/**
Expand All @@ -21,21 +25,35 @@ class Critical_CSS_Invalidator {
* Register hooks.
*/
public static function init() {
add_action( 'jetpack_boost_deactivate', array( __CLASS__, 'clear_data' ) );
add_action( 'jetpack_boost_deactivate', array( __CLASS__, 'reset_data' ) );
add_action( 'jetpack_boost_critical_css_environment_changed', array( __CLASS__, 'handle_environment_change' ) );
add_filter( 'jetpack_boost_total_problem_count', array( __CLASS__, 'update_boost_problem_count' ) );
}

/**
* Clear Critical CSS data.
* Reset Critical CSS data.
* For Cloud CSS, we need to make sure there are providers in the state,
* otherwise Cloud CSS cannot be stored after it is generated by the cloud.
*/
public static function clear_data() {
public static function reset_data() {
$storage = new Critical_CSS_Storage();
$storage->clear();

$state = new Critical_CSS_State();
$state->clear();

if ( self::is_cloud_css() ) {
$state->prepare_for_generation( ( new Source_Providers() )->get_provider_sources() );
$state->save();

// Clear the regenerate flag so things are nice and tidy.
jetpack_boost_ds_delete( 'critical_css_suggest_regenerate' );
// Also clear the admin notice flag, so the notice doesn't show up in case
// the user is reverted to free Boost.
Regenerate_Admin_Notice::dismiss();

}

Cloud_CSS_Followup::unschedule();
}

Expand All @@ -44,7 +62,7 @@ public static function clear_data() {
*/
public static function handle_environment_change( $is_major_change ) {
if ( $is_major_change ) {
self::clear_data();
self::reset_data();

do_action( 'jetpack_boost_critical_css_invalidated' );
}
Expand All @@ -58,4 +76,9 @@ public static function update_boost_problem_count( $count ) {

return $count;
}

public static function is_cloud_css() {
$optimizations = ( new Modules_Setup() )->get_status();
return isset( $optimizations[ Cloud_CSS::get_slug() ] ) && $optimizations[ Cloud_CSS::get_slug() ];
}
}
13 changes: 13 additions & 0 deletions projects/plugins/boost/app/lib/critical-css/Critical_CSS_State.php
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,19 @@ public function set_pending_providers( $providers ) {
return $this;
}

/**
* Add providers to the state, sets their status to pending
* and sets the generation status to pending.
*
* @param array $providers The providers to include in the state and set as pending.
* @return $this
*/
public function prepare_for_generation( $providers ) {
$this->set_pending_providers( $providers );
$this->state['status'] = self::GENERATION_STATES['pending'];
return $this;
}

/**
* Get fresh state
*/
Expand Down
2 changes: 1 addition & 1 deletion projects/plugins/boost/app/lib/critical-css/Regenerate.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function start() {

// Dismiss admin notices
Regenerate_Admin_Notice::dismiss();
jetpack_boost_ds_delete( 'critical_css_suggest_regenerate', null );
jetpack_boost_ds_delete( 'critical_css_suggest_regenerate' );

return $data;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public function handle_critical_css_invalidated() {
public function get_existing_sources() {
$state = new Critical_CSS_State();
$data = $state->get();
if ( isset( $data['providers'] ) ) {
if ( ! empty( $data['providers'] ) ) {
$providers = $data['providers'];
} else {
$source_providers = new Source_Providers();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Cloud CSS: Fixed not properly storing CSS returned from the cloud after a theme switch.

0 comments on commit 7624a6e

Please sign in to comment.