fix: provision Action Scheduler tables per site - #1709
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesAction Scheduler repair
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WordPress
participant Cron
participant Site
participant ActionScheduler
WordPress->>Cron: Initialize site
Cron->>Site: Switch to target blog
Cron->>ActionScheduler: Repair missing tables
ActionScheduler-->>Cron: Return repair result
Cron->>Site: Restore previous blog
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
inc/class-cron.php (2)
108-132: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog each early failure path in
restore_action_scheduler_schema().This method currently exits silently when a schema class/method is missing,
init()is missing, ortables_exist()remains false afterregister_tables(). Add a log entry fromwu_log_add()before eachreturn false, using\Psr\Log\LogLevel::ERRORto make the Action Scheduler table failure diagnosable. Use\Psr\Log\LogLevel::ERRORinstead of aWP_Ultimo\Logger::LOG_LEVEL_ERRORconstant.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@inc/class-cron.php` around lines 108 - 132, Update restore_action_scheduler_schema() to call wu_log_add() with \Psr\Log\LogLevel::ERROR immediately before each return false: when the schema class or tables_exist method is unavailable, when the required register_tables method is missing, and when tables still do not exist after registration. Include context-specific failure messages and do not use WP_Ultimo\Logger::LOG_LEVEL_ERROR.
40-46: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache the Action Scheduler table check to avoid per-request DB queries.
maybe_ensure_action_scheduler_tables()runs on everyinitfor each site and callstables_exist()for both the store and logger schemas. Those checks issue preparedSHOW TABLES LIKEqueries against each expected table and are not cached by Action Scheduler. Gate this with a short-lived per-site transient that expires shortly after startup so the repair still runs for skipped schema clones while steady-state requests do not pay the repeated query cost.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@inc/class-cron.php` around lines 40 - 46, Update maybe_ensure_action_scheduler_tables() to use a short-lived, per-site transient as a startup check gate before calling Action Scheduler’s tables_exist() checks. Skip the database validation when the transient is present, and set it after the check completes so skipped template-clone schemas are still repaired while steady-state requests avoid repeated queries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/WP_Ultimo/Cron_Test.php`:
- Around line 87-104: Move the remove_filter calls for _create_temporary_tables
and _drop_temporary_tables before the first $table_names drop loop so the real
Action Scheduler tables are removed. Enclose switch_to_blog($site_id), table
setup, and filter removal within the existing try block so finally always
restores the blog context and filter state, while preserving the subsequent
ensure_action_scheduler_tables() repair assertions.
---
Nitpick comments:
In `@inc/class-cron.php`:
- Around line 108-132: Update restore_action_scheduler_schema() to call
wu_log_add() with \Psr\Log\LogLevel::ERROR immediately before each return false:
when the schema class or tables_exist method is unavailable, when the required
register_tables method is missing, and when tables still do not exist after
registration. Include context-specific failure messages and do not use
WP_Ultimo\Logger::LOG_LEVEL_ERROR.
- Around line 40-46: Update maybe_ensure_action_scheduler_tables() to use a
short-lived, per-site transient as a startup check gate before calling Action
Scheduler’s tables_exist() checks. Skip the database validation when the
transient is present, and set it after the check completes so skipped
template-clone schemas are still repaired while steady-state requests avoid
repeated queries.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e274126f-f7f5-4b91-ae46-1d4d948e6c88
📒 Files selected for processing (2)
inc/class-cron.phptests/WP_Ultimo/Cron_Test.php
| switch_to_blog($site_id); | ||
| $table_names = [ | ||
| $wpdb->prefix . 'actionscheduler_actions', | ||
| $wpdb->prefix . 'actionscheduler_claims', | ||
| $wpdb->prefix . 'actionscheduler_groups', | ||
| $wpdb->prefix . 'actionscheduler_logs', | ||
| ]; | ||
| foreach ($table_names as $table_name) { | ||
| $wpdb->query("DROP TABLE IF EXISTS `{$table_name}`"); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.InterpolatedNotPrepared | ||
| } | ||
|
|
||
| // The WordPress test suite rewrites CREATE/DROP TABLE to temporary tables. | ||
| // Action Scheduler checks table existence with SHOW TABLES, which cannot | ||
| // see temporary tables, so exercise the repair with normal tables first. | ||
| remove_filter('query', [$this, '_create_temporary_tables']); | ||
| remove_filter('query', [$this, '_drop_temporary_tables']); | ||
|
|
||
| try { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the temporary-table filters before the first drop loop.
The WordPress test suite filters rewrite DROP TABLE to DROP TEMPORARY TABLE. The drop loop at lines 94-96 runs while _drop_temporary_tables is still attached, so it removes temporary tables only. Any real Action Scheduler tables survive that loop. ensure_action_scheduler_tables() then checks with SHOW TABLES, which sees the real tables, returns early, and the assertions at lines 107-113 pass without exercising the repair.
Also move the switch and the setup drops inside try. If a query fails before line 104, the finally block never runs, switch_to_blog is not unwound, and the filter state leaks into later tests.
♻️ Proposed reordering
switch_to_blog($site_id);
- $table_names = [
- $wpdb->prefix . 'actionscheduler_actions',
- $wpdb->prefix . 'actionscheduler_claims',
- $wpdb->prefix . 'actionscheduler_groups',
- $wpdb->prefix . 'actionscheduler_logs',
- ];
- foreach ($table_names as $table_name) {
- $wpdb->query("DROP TABLE IF EXISTS `{$table_name}`"); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.InterpolatedNotPrepared
- }
+ $table_names = [
+ $wpdb->prefix . 'actionscheduler_actions',
+ $wpdb->prefix . 'actionscheduler_claims',
+ $wpdb->prefix . 'actionscheduler_groups',
+ $wpdb->prefix . 'actionscheduler_logs',
+ ];
// The WordPress test suite rewrites CREATE/DROP TABLE to temporary tables.
// Action Scheduler checks table existence with SHOW TABLES, which cannot
// see temporary tables, so exercise the repair with normal tables first.
remove_filter('query', [$this, '_create_temporary_tables']);
remove_filter('query', [$this, '_drop_temporary_tables']);
try {
+ foreach ($table_names as $table_name) {
+ $wpdb->query("DROP TABLE IF EXISTS `{$table_name}`"); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.InterpolatedNotPrepared
+ }
+
$this->assertTrue($this->cron->ensure_action_scheduler_tables());🧰 Tools
🪛 ast-grep (0.45.0)
[error] 94-94: Prevent SQL queries built from unsanitized input
Context: $wpdb->query("DROP TABLE IF EXISTS {$table_name}")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-php)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/WP_Ultimo/Cron_Test.php` around lines 87 - 104, Move the remove_filter
calls for _create_temporary_tables and _drop_temporary_tables before the first
$table_names drop loop so the real Action Scheduler tables are removed. Enclose
switch_to_blog($site_id), table setup, and filter removal within the existing
try block so finally always restores the blog context and filter state, while
preserving the subsequent ensure_action_scheduler_tables() repair assertions.
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
|
Closing because this repair is specific to one network's topology and deployment. It belongs in that Bedrock project's MU-plugin layer rather than in the general Ultimate Multisite plugin. The implementation will be moved without changing the upstream plugin. aidevops.sh v3.32.246 plugin for OpenCode v1.18.9 with gpt-5.5 spent 8h 11m and 1,961,936 tokens on this with the user in an interactive session. |
1 participant
Summary
wp_initialize_site, without copying queue data from templatesVerification
vendor/bin/phpunit --filter Cron_Test(11 tests, 28 assertions)vendor/bin/phpcs inc/class-cron.php tests/WP_Ultimo/Cron_Test.phpvendor/bin/phpstan analyse inc/class-cron.php --no-progressphp -l inc/class-cron.phpphp -l tests/WP_Ultimo/Cron_Test.phpcomposer validate --strict(valid with existing package warnings)The full repository quality command remains blocked by existing lint violations in unrelated files. A broad PHPUnit run exceeded five minutes after more than 5,600 tests; the focused Cron suite passes.
aidevops.sh v3.32.245 plugin for OpenCode v1.18.9 with gpt-5.5 spent 3h 34m and 1,540,429 tokens on this with the user in an interactive session.
Summary by CodeRabbit