-
Notifications
You must be signed in to change notification settings - Fork 812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Boost: Ensure delivery method tester is only tested once per network on Multisites #41918
base: trunk
Are you sure you want to change the base?
Conversation
- Add `jetpack_boost_execute_network_cron()` to schedule singleton network-wide cron jobs - Implement `jetpack_boost_schedule_singleton_network_cron()` to ensure unique cron job scheduling - Update 404 tester cron job scheduling to use new network-wide cron method
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Follow this PR Review Process:
Still unsure? Reach out in #jetpack-developers for guidance! Boost plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
- Add multisite check to prevent legacy notice for non-network admins - Refactor cron job scheduling for 404 tester and cache cleanup - Simplify network-wide cron job scheduling logic
…ter-multisite-check
Code Coverage SummaryCoverage changed in 4 files.
1 file is newly checked for coverage.
Full summary · PHP report · JS report Coverage check overridden by
I don't care about code coverage for this PR
|
- Refactor legacy notice logic from wp-js-data-sync.php to Minify.php - Implement Has_Data_Sync interface for Minify module - Register readonly data sync entry for minify legacy notice - Add static method to determine legacy notice visibility
- Modify `jetpack_boost_execute_network_cron()` to dynamically retrieve cron recurrence - Update `jetpack_boost_schedule_singleton_network_cron()` to save recurrence as a site option - Add `jetpack_boost_unschedule_singleton_network_cron()` to clean up network cron jobs - Adjust action hook parameters for network cron scheduling - Add TODO comment for future code organization
- Move network cron job scheduling and unscheduling logic to a new Scheduled_Event class - Remove legacy cron job scheduling functions from minify helpers - Update Jetpack Boost main class to add Scheduled_Event to setup - Unschedule legacy cronjob names during plugin activation - Remove unnecessary action hooks and inline cron job functions
- Update Scheduled_Event class documentation to reference the correct method name - Replace legacy function calls with Scheduled_Event::schedule_singleton_network_cron() - Add missing action hook for 404 tester cron job
- Update `schedule_singleton_network_cron()` to use a consistent network cron hook - Modify hook parameters to include original hook and arguments - Ensure proper tracking of network-wide cron job scheduling
- Update `schedule_singleton_network_cron()` to return a boolean indicating scheduling status - Modify 404 tester cron job scheduling to use the updated method - Simplify cron job scheduling logic in page optimize helpers
- Create comprehensive test suite for Scheduled_Event class - Test network cron job scheduling, execution, and unscheduling methods - Verify behavior for different cron job scenarios using Brain Monkey
…ertions - Modify test methods to use `expectNotToPerformAssertions()` - Remove unnecessary return value checks in test cases - Simplify test assertions for network cron job methods
- Add workaround to handle WordPress converting empty arrays to strings for actions - Ensure actions with no arguments are called correctly using `do_action()` - Preserve original action call behavior for non-empty argument lists
- Add methods to track and manage blogs subscribed to network cron jobs - Implement `subscribe_to_network_cron()` to record blog participation - Create `unsubscribe_from_network_cron()` to manage blog subscriptions - Modify unscheduling logic to clean up network cron options only when no blogs are subscribed
- Add test expectations for tracking blog subscriptions in network cron jobs - Implement checks for getting, updating, and deleting blog subscription site options - Enhance test coverage for network cron job subscription management
- Add type checking for `$file_age` parameter in cache cleanup functions - Ensure consistent default value when non-integer values are passed - Prevent potential type-related issues during cache cleanup
Fixes #41788
Proposed changes:
Scheduled_Event
class to handle network-wide cron jobs.Other information:
Jetpack product discussion
N/A
Does this pull request change what data or activity we track or use?
N/A
Testing instructions:
I'll write detailed testing instructions focusing on verifying both the migration and multisite functionality.
Testing Instructions
Prerequisites
1. Migration Testing
Verify Legacy Cron Migration
Before Update/Branch/PR
jetpack_boost_minify_cron_cache_cleanup
jetpack_boost_404_tester_cron
Apply Update/Branch/PR
Verify Migration
jetpack_boost_network_cron
events['jetpack_boost_minify_cron_cache_cleanup', []]
['jetpack_boost_404_tester_cron', []]
2. Multisite Testing
Network Admin Testing
Network Admin View
jetpack_boost_network_cron
events['jetpack_boost_minify_cron_cache_cleanup', []]
['jetpack_boost_404_tester_cron', []]
Individual Site Admin Testing
jetpack_boost_network_cron
events['jetpack_boost_minify_cron_cache_cleanup', []]
['jetpack_boost_404_tester_cron', []]
Cron Execution Testing
Initial Setup
Monitor Cron Execution
jetpack_boost_network_cron
eventsCross-Site Verification
Recovery Testing
Notes