Feature/8103 add settings changes detection#8140
Feature/8103 add settings changes detection#8140Miraeld wants to merge 11 commits intofeature/ri-recommendationsfrom
Conversation
- Create SettingsSubscriber to monitor WP Rocket settings changes - Subscribe to rocket_after_save_options hook - Detect relevant settings changes (minify, lazyload, CDN, etc.) - Only fetch recommendations when status is completed/failed - Add has_required_metrics() method to DataManager - Add should_fetch_recommendations() method with hash comparison - Add calculate_metrics_hash() to prevent redundant fetches - Update save_recommendations() to store metrics hash Fixes #8103
- Add RecommendationsSettingsSubscriber to use statement - Register ri_recommendations_settings_subscriber in ServiceProvider - Add to $provides array for DI container - Register subscriber in Plugin.php init_valid_key_subscribers() Part of #8103
- Add MaybeFetchAfterSettingsChangeTest with 8 test scenarios - Add GetSubscribedEventsTest for hook verification - Add HasRequiredMetricsTest for metrics validation - Test status conditions (pending, loading, completed, failed) - Test relevant vs irrelevant settings changes - Test hash comparison to prevent redundant fetches - Add comprehensive fixtures for all test cases Part of #8103
- Add MaybeFetchAfterSettingsChangeTest for WordPress hook integration - Test rocket_after_save_options action trigger - Test transient operations with real WordPress functions - Test scenarios with completed/failed/pending statuses - Verify fetch triggered only when conditions met - Mark tests with @group RocketInsights and @group AdminOnly Part of #8103
There was a problem hiding this comment.
Pull request overview
Adds an automatic “refresh recommendations on relevant WP Rocket settings changes” mechanism for Rocket Insights, using a metrics/options hash to avoid unnecessary API fetches, and introduces new unit/integration test coverage for the decision logic.
Changes:
- Introduces a new Rocket Insights Recommendations
SettingsSubscriberintended to react to settings saves and conditionally trigger a recommendations fetch. - Extends
Recommendations\DataManagerwith required-metrics validation plus hash-based “should fetch” logic, and persistsmetrics_hashinto the cached recommendations payload. - Adds unit + integration tests and fixtures covering settings-change detection and required-metrics checks.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| inc/Plugin.php | Registers the new recommendations settings subscriber in the common subscriber list. |
| inc/Engine/Admin/RocketInsights/ServiceProvider.php | Registers the new subscriber service in the Rocket Insights service provider. |
| inc/Engine/Admin/RocketInsights/Recommendations/SettingsSubscriber.php | Implements settings-change detection and conditional fetch trigger. |
| inc/Engine/Admin/RocketInsights/Recommendations/DataManager.php | Adds required-metrics check, hash computation/compare, and stores metrics_hash in cache. |
| tests/Unit/.../SettingsSubscriber/MaybeFetchAfterSettingsChangeTest.php | Unit tests for the subscriber’s fetch decision logic. |
| tests/Unit/.../SettingsSubscriber/GetSubscribedEventsTest.php | Unit test for subscriber hook registration. |
| tests/Unit/.../DataManager/HasRequiredMetricsTest.php | Unit tests for required-metrics validation. |
| tests/Integration/.../SettingsSubscriber/MaybeFetchAfterSettingsChangeTest.php | Integration test intended to validate hook-triggered behavior. |
| tests/Fixtures/.../SettingsSubscriber/MaybeFetchAfterSettingsChangeTest.php | Fixture data for settings-change scenarios. |
| tests/Fixtures/.../DataManager/HasRequiredMetricsTest.php | Fixture data for required-metrics scenarios. |
...dmin/RocketInsights/Recommendations/SettingsSubscriber/MaybeFetchAfterSettingsChangeTest.php
Outdated
Show resolved
Hide resolved
...dmin/RocketInsights/Recommendations/SettingsSubscriber/MaybeFetchAfterSettingsChangeTest.php
Outdated
Show resolved
Hide resolved
...dmin/RocketInsights/Recommendations/SettingsSubscriber/MaybeFetchAfterSettingsChangeTest.php
Outdated
Show resolved
Hide resolved
...dmin/RocketInsights/Recommendations/SettingsSubscriber/MaybeFetchAfterSettingsChangeTest.php
Outdated
Show resolved
Hide resolved
...c/Engine/Admin/RocketInsights/Recommendations/SettingsSubscriber/GetSubscribedEventsTest.php
Outdated
Show resolved
Hide resolved
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
inc/Engine/Admin/RocketInsights/Recommendations/DataManager.php
Outdated
Show resolved
Hide resolved
inc/Engine/Admin/RocketInsights/Recommendations/SettingsSubscriber.php
Outdated
Show resolved
Hide resolved
- Change SettingsSubscriber to use update_option_wp_rocket_settings instead of custom hook - Add TRACKED_OPTIONS constant in DataManager as single source of truth - Add get_tracked_option_keys() method to share option keys across classes - Update get_enabled_options() to use constant instead of duplicate array - Update has_relevant_changes() to use shared method from DataManager - Remove custom rocket_after_save_options action from options.php Addresses review feedback on PR #8140
- Update GetSubscribedEventsTest to check for update_option_wp_rocket_settings - Update MaybeFetchAfterSettingsChangeTest to use WordPress native hook - Remove unnecessary admin context setup from integration test All tests passing: 16 unit tests, 2 integration tests
- Removed has_required_metrics() method - Removed should_fetch_recommendations() method - Removed calculate_metrics_hash() method - Removed metrics_hash tracking from save_recommendations() - Simplified SettingsSubscriber to not check hash (Task 2.1 responsibility) - Deleted HasRequiredMetricsTest and related fixtures - Updated MaybeFetchAfterSettingsChangeTest to reflect simplified logic These methods will be provided by PR #8131 (Task 2.1) to avoid duplication.
Description
Fixes #8103
This pull request introduces a new mechanism to automatically refresh performance recommendations when relevant WP Rocket settings change. It ensures that recommendations are only fetched when necessary, based on changes to key options or metrics, and adds robust unit and integration tests for this logic. The most important changes are as follows:
Type of change
Detailed scenario
What was tested
Automated Tests (✅ Passing):
SettingsSubscriber::maybe_fetch_after_settings_change()- 8 scenarios covering all status conditions (pending, loading, completed, failed), relevant vs irrelevant changes, and hash-based optimizationSettingsSubscriber::get_subscribed_events()- Verifies hook registration with correct parametersDataManager::has_required_metrics()- 6 scenarios covering all core metrics (LCP, TTFB, CLS, TBT) validationManual Tests (Recommended):
How to test
Prerequisites:
WP_DEBUG_LOGto monitor log entries in/wp-content/debug.logTest 1: Relevant Setting Change Triggers Fetch
Steps:
Expected:
wpr_ri_recommendationsupdated with newmetrics_hash"Recommendations: Relevant settings changed, fetching new recommendations"Test 2: Irrelevant Setting Change Does NOT Trigger Fetch
Steps:
Expected:
"Recommendations: Settings changed but none affect recommendations"Test 3: Multiple Relevant Settings at Once
Steps:
Expected:
Monitored Settings (21 total):
Verification Methods:
Affected Features & Quality Assurance Scope
Primary Feature:
Technical description
Documentation
Feature: Automatic Recommendations Refresh on Settings Change
SettingsSubscriberclass that listens for changes to WP Rocket settings and triggers a recommendations refresh only if relevant settings are changed and the underlying metrics hash differs, preventing unnecessary API calls.Enhancement: Metrics and Hash Validation
DataManagerfor checking if required metrics exist and for determining if recommendations should be fetched, based on a hash of current metrics and enabled options. This helps avoid redundant recommendation fetches.Testing: Unit, Fixture, and Integration Coverage
has_required_metricsinDataManager, covering all edge cases of metric presence. [1] [2]Mandatory Checklist
Code validation
Code style