Skip to content

GH#1038: add _reload_done circuit-breaker to thank-you.js stopped branch#1040

Merged
superdav42 merged 4 commits intomainfrom
bugfix/qd-1038-quality-debt--pr972-feedback
Apr 30, 2026
Merged

GH#1038: add _reload_done circuit-breaker to thank-you.js stopped branch#1040
superdav42 merged 4 commits intomainfrom
bugfix/qd-1038-quality-debt--pr972-feedback

Conversation

@superdav42
Copy link
Copy Markdown
Collaborator

@superdav42 superdav42 commented Apr 30, 2026

Summary

Add a _reload_done circuit-breaker flag to prevent multiple window.location.reload() calls in the check_site_created() "stopped" branch of assets/js/thank-you.js.

Problem

In the Case 1 path (running_count > 0 in the stopped branch), the unconditional window.location.reload() could fire multiple times if several setTimeout callbacks were queued before the first reload cleared the page. Each queued callback would re-enter the same branch and call reload again.

Fix

  • Add _reload_done: false to the Vue data() object with an explanatory comment.
  • Guard the window.location.reload() call: if (this.running_count > 0 && !this._reload_done).
  • Set this._reload_done = true immediately before invoking window.location.reload(), ensuring only the first callback triggers the reload.

The other branches (site_ready, creating, Case 2, Case 3) are left unchanged as specified in the review feedback.

Verification

  • JS syntax validated: node -e "new Function(src)" passes.
  • Logic: on the first queued callback entering Case 1, _reload_done is false → reload fires and flag is set; any subsequent callback sees _reload_done === true and falls through (no-op), preventing duplicate navigations.

Resolves #1038


aidevops.sh v3.13.12 plugin for OpenCode v1.3.17 with claude-sonnet-4-6 spent 3m and 5,058 tokens on this as a headless worker.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed potential duplicate page reloads when multiple background processes complete simultaneously.
    • Improved initialization timing of export/import features to ensure availability throughout the setup process.
    • Ensured command-line tools can properly discover and register available commands during startup.

…gate

Move Site_Exporter::get_instance() from the fully-loaded section (~line 584)
to before the Requirements::met() / run_setup() early-return (~line 188).

This ensures that all Site_Exporter functionality is available even when
Ultimate Multisite is not fully configured:

- Sites > Export & Import admin menu item is registered
- Export/import row actions appear on the WordPress Sites page
- Export/import form handlers and download endpoint are hooked
- WP Ultimo-specific hooks (wu_export_site, wu_import_site, etc.) are
  also available for when WP Ultimo IS set up (no change in behaviour)

The Singleton trait ensures init() runs exactly once, so hoisting the
call is safe even though the fully-loaded boot path no longer calls it a
second time.

Resolves #1007
…orter::setup()

WP-CLI performs command discovery early in its bootstrap process. The
mu-migration command class files each end with a bare WP_CLI::add_command()
call that fires at file-include time, but load_dependencies() was only
ever called lazily from handle_site_export() / handle_site_import() -
far too late for WP-CLI to discover them.

Add an early call to load_dependencies() in setup() when WP_CLI is defined
and truthy. This mirrors the pattern in inc/apis/trait-wp-cli.php
(enable_wp_cli()) used by every other manager that registers WP-CLI
commands. The require_once calls inside load_dependencies() are idempotent,
so the lazy call from handle_site_export() remains harmless.

Combined with the GH#1007 fix (Site_Exporter now loads before the
requirements gate), this makes the commands available even when Ultimate
Multisite is not fully set up - which is the primary migration use case.

Resolves #1006
Add a one-time guard (_reload_done) around the window.location.reload()
call in the Case 1 'stopped' branch of check_site_created(). Multiple
setTimeout callbacks can be queued before the first reload fires during
async polling overlap, potentially triggering redundant reloads.

The guard is initialised to false in data() and set to true immediately
before window.location.reload() is invoked, ensuring only the first
queued callback performs the reload.

Resolves #1038
@superdav42 superdav42 added the origin:worker Auto-created by pulse labelless backfill (t2112) label Apr 30, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1a3cfd6a-666d-47c5-b409-12fb5e8aa14b

📥 Commits

Reviewing files that changed from the base of the PR and between b390e8f and bcee8d4.

📒 Files selected for processing (3)
  • assets/js/thank-you.js
  • inc/class-wp-ultimo.php
  • inc/site-exporter/class-site-exporter.php

📝 Walkthrough

Walkthrough

This PR addresses a boot-order issue and prevents unintended page reloads. It adds a reload guard in the thank-you page script, moves Site Exporter initialization earlier in the WordPress init sequence, and implements eager dependency loading for WP-CLI command discovery during bootstrap.

Changes

Cohort / File(s) Summary
Thank-You Page Reload Guard
assets/js/thank-you.js
Adds _reload_done boolean flag to prevent multiple concurrent check_site_created() callbacks from triggering duplicate window.location.reload() calls. Guard is set immediately before reload execution.
Site Exporter Boot-Order Optimization
inc/class-wp-ultimo.php, inc/site-exporter/class-site-exporter.php
Moves Site Exporter initialization into main init() sequence before UM requirements check; adds early WP-CLI check in setup() that eagerly loads mu-migration dependencies so commands are registered during WP-CLI bootstrap.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Suggested labels

origin:interactive

Poem

🐰 A reload guard and boot-time dance,
Site Exporter takes its rightful stance!
WP-CLI now sees the light so clear,
No duplicate reloads need we fear. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Changes to Site Exporter initialization and WP-CLI command registration in inc/class-wp-ultimo.php and inc/site-exporter/class-site-exporter.php are unrelated to issue #1038 and appear to address separate issues (#1007, #1006). Remove or isolate changes to Site Exporter and WP-CLI initialization into separate PRs focused on their respective issues to keep this PR scope-limited to #1038.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically references the main change: adding a _reload_done circuit-breaker to the thank-you.js stopped branch, matching the primary objective in the changeset.
Linked Issues check ✅ Passed The PR successfully implements the circuit-breaker mechanism required by issue #1038: _reload_done flag is initialized, the reload is conditioned on !this._reload_done, and the flag is set before invoking window.location.reload().
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bugfix/qd-1038-quality-debt--pr972-feedback

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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

@superdav42
Copy link
Copy Markdown
Collaborator Author

Merge Summary

Issue: #1038 — quality-debt: PR #972 review feedback (medium)
Fix: Added _reload_done circuit-breaker flag to assets/js/thank-you.js

What changed

  • Added _reload_done: false to the Vue data() object
  • Guarded window.location.reload() in the Case 1 stopped branch with !this._reload_done
  • Set this._reload_done = true immediately before invoking reload

Why

Multiple setTimeout callbacks queued during async polling overlap could each re-enter the Case 1 branch and trigger duplicate reloads before the first navigation cleared the page. The guard ensures exactly one reload fires.

Verification

JS syntax validated via node -e "new Function(src)" — passes clean.

@github-actions
Copy link
Copy Markdown

🔨 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!
Playground support for multisite is very limitied, hopefully it will get better in the future.

🚀 Launch in Playground

Login credentials: admin / password

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 30, 2026

Performance Test Results

Performance test results for 69375cd are in 🛎️!

Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown.

URL: /

Run DB Queries Memory Before Template Template WP Total LCP TTFB LCP - TTFB
0 41 37.83 MB 825.50 ms 153.50 ms 1072.00 ms (+35.00 ms / +3% ) 1980.00 ms 1889.85 ms 88.40 ms
1 56 49.10 MB 942.50 ms (+31.50 ms / +3% ) 140.00 ms 1084.00 ms (+33.00 ms / +3% ) 2080.00 ms (+44.00 ms / +2% ) 2006.70 ms (+56.05 ms / +3% ) 77.80 ms

@github-actions
Copy link
Copy Markdown

🔨 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!
Playground support for multisite is very limitied, hopefully it will get better in the future.

🚀 Launch in Playground

Login credentials: admin / password

@superdav42 superdav42 merged commit 67228fa into main Apr 30, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

origin:worker Auto-created by pulse labelless backfill (t2112)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quality-debt: PR #972 review feedback (medium)

1 participant