refactor: add intropage_device_scope() and adopt it across the busiest panels - #379
Merged
TheWitness merged 3 commits intoSep 10, 2026
Merged
Conversation
Extract the repeated get_simple_device_perms + intropage_get_allowed_devices preamble into one documented helper returning the user's device scope, and adopt it in alert.php's two panels. Add a phpunit.xml coverage source config and a behavioural Pest test (with permission-chain stubs) that exercises every branch of the helper, since the plugin previously had only source-scan tests and no coverage config. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Replace the copied permission preamble in all 14 busiest update/detail functions with the device_scope helper, and add a behavioural test that drives busiest_cpu and asserts the host filter (IN clause for a restricted user, none for a simple user). Extend the test bootstrap so fetch results and config options are controllable and SELECT SQL is captured. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
somethingwithproof
changed the title
Cacti core now requires PHP >= 8.2, so the 8.1 integration job fails the composer platform check; Ubuntu Noble also lacks libapache2-mod-php for non-native versions without the ondrej PPA. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new test configuration and CI workflow changes introduce likely breakage/incompatibilities (PHPUnit config vs Pest/PHPUnit versions, CI matrix/runtime setup changes) that should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refactors repeated device-permission scoping logic into a new intropage_device_scope($user_id) helper and adopts it across the busiest/alert panels, while adding a Pest-based coverage/test harness to exercise the helper and verify the busiest panels’ host filtering behavior.
Changes:
- Add
intropage_device_scope()to centralize the “simple perms vs allowed host IDs” resolution used by panels. - Update
panellib/busiest.phpandpanellib/alert.phpto use the helper instead of duplicating the permission preamble. - Introduce a test/coverage harness (Pest + phpunit.xml + bootstrap stubs) and add unit tests for the helper and busiest scope SQL.
File summaries
| File | Description |
|---|---|
include/functions.php |
Adds intropage_device_scope() helper wrapping the device-permission chain. |
panellib/busiest.php |
Adopts the helper across busiest panels/detail views to de-duplicate scope construction. |
panellib/alert.php |
Adopts the helper for host-alert panels to reduce repeated permission logic. |
tests/bootstrap.php |
Expands test stubs to support scope resolution and capture SQL for assertions. |
tests/Unit/DeviceScopeTest.php |
Adds unit coverage for all branches of intropage_device_scope(). |
tests/Unit/BusiestScopeTest.php |
Adds a SQL-capture test ensuring busiest CPU queries apply host scoping correctly. |
phpunit.xml |
Adds test suite + coverage source configuration for the plugin. |
.gitignore |
Ignores PHPUnit/PHP-CS-Fixer caches. |
.github/workflows/plugin-ci-workflow.yml |
Adjusts CI PHP matrix and changes apt update behavior. |
Review details
- Files reviewed: 8/9 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
xmacan
approved these changes
Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits: the helper plus a coverage harness, then adopting it across the panels that repeat the same device-scope query construction.
Kept together because #378 was fully contained in this branch — reviewing them separately meant reviewing the helper twice.
Supersedes #378.