Unactioned Review Feedback
Source PR: #821
File: tests/WP_Ultimo/Dashboard_Widgets_Test.php
Reviewers: coderabbit
Findings: 1
Max severity: high
HIGH: coderabbit (coderabbitai[bot])
File: tests/WP_Ultimo/Dashboard_Widgets_Test.php:131
⚠️ Potential issue | 🟠 Major
🧩 Analysis chain
🌐 Web query:
In WordPress test environments, after calling set_current_screen('dashboard') or set_current_screen('dashboard-network'), what is the recommended way to restore a previously-null screen state: unset($GLOBALS['current_screen']) or set_current_screen('')?
💡 Result:
In WordPress test environments, the recommended way to restore a previously-null screen state after calling set_current_screen('dashboard') or set_current_screen('dashboard-network') is unset($GLOBALS['current_screen']) (along with related globals $taxnow and $typenow).
Citations:
🏁 Script executed:
cat -n tests/WP_Ultimo/Dashboard_Widgets_Test.php | sed -n '125,220p'
Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 3808
Unset $GLOBALS['current_screen'] when restoring to null state.
Both finally blocks skip restoration when $original_screen_id is null/falsy, allowing forced screen states to leak into subsequent tests. The finally blocks must explicitly unset the screen global when the original screen did not exist.
Proposed fix
} finally {
if (isset($wp_scripts)) {
$wp_scripts->queue = $original_queue;
$wp_scripts->done = $original_done;
}
- if ($original_screen_id) {
- set_current_screen($original_screen_id);
- }
+ if (null !== $original_screen_id) {
+ set_current_screen($original_screen_id);
+ } else {
+ unset($GLOBALS['current_screen']);
+ }
$pagenow = $original;
}
Applies to both test methods (lines 165–167 and 212–214).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
} finally {
if (isset($wp_scripts)) {
$wp_scripts->queue = $original_queue;
$wp_scripts->done = $original_done;
}
if (null !== $original_screen_id) {
set_current_screen($original_screen_id);
} else {
unset($GLOBALS['current_screen']);
}
$pagenow = $original;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/Dashboard_Widgets_Test.php` around lines 130 - 131, The
finally blocks that restore the admin screen state must also clear the global
when there was no original screen; update the restore logic around
$original_screen and $original_screen_id so that if $original_screen_id is
truthy you restore the original screen (e.g., via set_current_screen or
assigning $GLOBALS['current_screen'] back to $original_screen), and otherwise
explicitly unset($GLOBALS['current_screen']) to prevent the forced screen from
leaking into other tests; apply this change in both test methods that capture
$original_screen/$original_screen_id.
View comment
Auto-generated by quality-feedback-helper.sh scan-merged. Review each finding and either fix the code or dismiss with a reason.
aidevops.sh v3.8.23 automated scan.
Unactioned Review Feedback
Source PR: #821
File:
tests/WP_Ultimo/Dashboard_Widgets_Test.phpReviewers: coderabbit
Findings: 1
Max severity: high
HIGH: coderabbit (coderabbitai[bot])
File:
⚠️ Potential issue | 🟠 Major
tests/WP_Ultimo/Dashboard_Widgets_Test.php:131🧩 Analysis chain
🌐 Web query:
In WordPress test environments, after calling set_current_screen('dashboard') or set_current_screen('dashboard-network'), what is the recommended way to restore a previously-null screen state: unset($GLOBALS['current_screen']) or set_current_screen('')?💡 Result:
In WordPress test environments, the recommended way to restore a previously-null screen state after calling set_current_screen('dashboard') or set_current_screen('dashboard-network') is unset($GLOBALS['current_screen']) (along with related globals $taxnow and $typenow).
Citations:
🏁 Script executed:
Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 3808
Unset
$GLOBALS['current_screen']when restoring to null state.Both
finallyblocks skip restoration when$original_screen_idis null/falsy, allowing forced screen states to leak into subsequent tests. The finally blocks must explicitly unset the screen global when the original screen did not exist.Proposed fix
} finally { if (isset($wp_scripts)) { $wp_scripts->queue = $original_queue; $wp_scripts->done = $original_done; } - if ($original_screen_id) { - set_current_screen($original_screen_id); - } + if (null !== $original_screen_id) { + set_current_screen($original_screen_id); + } else { + unset($GLOBALS['current_screen']); + } $pagenow = $original; }Applies to both test methods (lines 165–167 and 212–214).
📝 Committable suggestion
🤖 Prompt for AI Agents
View comment
Auto-generated by
quality-feedback-helper.sh scan-merged. Review each finding and either fix the code or dismiss with a reason.aidevops.sh v3.8.23 automated scan.