fix: move network-activate handler to external JS file#871
Conversation
The inline <script> in requirements_table.php was rendered as visible text because wu_get_template_contents() captures output via ob_start and injects it as innerHTML — browsers do not execute script tags inserted this way. Extract the click handler to assets/js/network-activate.js, enqueue it via wp_enqueue_script in register_scripts(), and pass the translated error string through wp_localize_script. Resolves #836
📝 WalkthroughWalkthroughThe changes extract an inline network activation handler from the setup wizard template into a separate, reusable JavaScript file ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@assets/js/network-activate.js`:
- Around line 46-50: The code deriving errorMsg currently only checks
response.data.message; update the logic in network-activate.js (the block that
sets var errorMsg = wu_network_activate.error_message) to handle WP_Error
serialized payloads by detecting when response.data is an array
(Array.isArray(response.data)) and, if so, extracting the message from the first
element (e.g., response.data[0].message) before falling back to
response.data.message and then to the original
wu_network_activate.error_message; ensure this also guards for missing
elements/fields to avoid runtime errors.
🪄 Autofix (Beta)
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
Run ID: 21f6edf3-1442-4966-887c-5d43f3f8e20a
📒 Files selected for processing (3)
assets/js/network-activate.jsinc/admin-pages/class-setup-wizard-admin-page.phpviews/wizards/setup/requirements_table.php
💤 Files with no reviewable changes (1)
- views/wizards/setup/requirements_table.php
| var errorMsg = wu_network_activate.error_message; | ||
|
|
||
| if (response.data && response.data.message) { | ||
| errorMsg = response.data.message; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In WordPress core, what JSON structure is returned by wp_send_json_error( new WP_Error('code', 'message') )?
💡 Result:
The JSON structure returned by wp_send_json_error( new WP_Error('code', 'message') ) in WordPress core is: json { "success": false, "data": [ { "code": "code", "message": "message" } ] } This is generated by processing the WP_Error object's errors property, which for a single error is an associative array with the code as key and array of messages as value. The function iterates over each code and message, creating an array of objects under data, with success always set to false. The response is then passed to wp_send_json for output and termination.
Citations:
- 1: https://developer.wordpress.org/reference/functions/wp_send_json_error/
- 2: https://developer.wordpress.org/reference/functions/wp_send_json_error/?output_format=md
- 3: http://codex.wordpress.org/Function_Reference/wp_send_json_error
- 4: https://wp-kama.com/function/wp_send_json_error
🏁 Script executed:
# Find and read the network-activate.js file
fd -t f network-activate.jsRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 110
🏁 Script executed:
# Read the relevant section of the file to confirm the code snippet
if [ -f "assets/js/network-activate.js" ]; then
wc -l assets/js/network-activate.js
sed -n '40,60p' assets/js/network-activate.js
fiRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 536
🏁 Script executed:
# Check the AJAX handler file mentioned in the review
fd -t f class-setup-wizard-admin-page.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 130
🏁 Script executed:
# If found, check lines 160-185 for WP_Error usage
if [ -f "inc/admin-pages/class-setup-wizard-admin-page.php" ]; then
sed -n '160,185p' inc/admin-pages/class-setup-wizard-admin-page.php
fiRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 659
Handle WP_Error array payloads when deriving the error text.
At lines 48–49, only response.data.message is checked. The AJAX handler returns WP_Error instances (lines 167 and 179 in inc/admin-pages/class-setup-wizard-admin-page.php), and WordPress serializes these as { "success": false, "data": [ { "code": "...", "message": "..." } ] }. The current code will not extract the backend message from this array format.
🔧 Suggested patch
- var errorMsg = wu_network_activate.error_message;
-
- if (response.data && response.data.message) {
- errorMsg = response.data.message;
- }
+ var errorMsg = wu_network_activate.error_message;
+
+ if (response && response.data) {
+ if (response.data.message) {
+ errorMsg = response.data.message;
+ } else if (Array.isArray(response.data) && response.data[0] && response.data[0].message) {
+ errorMsg = response.data[0].message;
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@assets/js/network-activate.js` around lines 46 - 50, The code deriving
errorMsg currently only checks response.data.message; update the logic in
network-activate.js (the block that sets var errorMsg =
wu_network_activate.error_message) to handle WP_Error serialized payloads by
detecting when response.data is an array (Array.isArray(response.data)) and, if
so, extracting the message from the first element (e.g.,
response.data[0].message) before falling back to response.data.message and then
to the original wu_network_activate.error_message; ensure this also guards for
missing elements/fields to avoid runtime errors.
|
Performance Test Results Performance test results for 1e036f7 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:
|
🔨 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: |
1 participant
Summary
<script>tag was rendered as visible text instead of executingassets/js/network-activate.js, enqueued viawp_enqueue_scriptwithwp_localize_scriptfor the translated error string<script>block fromviews/wizards/setup/requirements_table.phpRoot Cause
The view is loaded via
wu_get_template_contents()(output buffering withob_start/ob_get_clean). The captured string is returned as thedescvalue for a wizard section. When the wizard framework renders the section, the HTML is inserted as innerHTML — browsers do not execute<script>tags injected this way.Files Changed
assets/js/network-activate.js(new) — standalone click handler usingwu_network_activate.error_messagefrom localized datainc/admin-pages/class-setup-wizard-admin-page.php— enqueues the new script + localizes error string inregister_scripts()views/wizards/setup/requirements_table.php— removed inline<script>blockTesting
Setup_Wizard_Admin_Page_Testsuite passes (37 tests)Resolves #836
Summary by CodeRabbit