Conversation
Rename functions to match PSR12 requirements
- Change tabs to spaces - add consts
Breaking up some of the large files
Replaced legacy var with const/let. Switched loose checks to strict (===, !==, strict typeof checks). Added default param: applyFilter(action = ''). Used template literals for URL/query string assembly. Converted many callbacks to arrow functions and shorthand methods. Removed one duplicate object key in saveFilter payload (trim was defined twice with same value). Kept existing flow, DOM selectors, action names, and endpoints unchanged.
Applied the Sonar-requested cleanups in monitor.js:
Refactor rending function and Ajax call functions Lowe cognetive complexity prepare for easier testing
There was a problem hiding this comment.
Pull request overview
This PR modernizes and restructures the Monitor Cacti plugin by introducing new helper modules (DB/query helpers, render/controller separation, poller helpers), tightening PHP runtime requirements, and aligning documentation/instructions with the updated structure.
Changes:
- Added new helper modules for DB query construction and for splitting monitor UI logic into controller/render responsibilities.
- Refactored poller logic into a dedicated helper file and added a PHP 8.1+ runtime guard for CLI execution.
- Updated Copilot instructions and applied small formatting/strict typing adjustments across several entrypoint/index files.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
setup.php |
Refactors hook/upgrade/install logic, adds PHP version gating, and introduces typed/camelCase functions. |
poller_monitor.php |
Adds PHP 8.1+ guard, factors logic into poller_functions.php, updates argument handling. |
poller_functions.php |
New poller helper module (uptime checks, notifications, email building, purge logic). |
monitor_controller.php |
New controller layer for request validation, dashboard actions, page orchestration, AJAX tooltip output. |
monitor_render.php |
New rendering layer for the various grouping/view modes and host tile/list rendering. |
db_functions.php |
New SQL construction/filter helpers and host selection utilities used by the UI. |
js/monitor.js |
New client-side filter/apply/save/dashboard UI logic and tooltip AJAX wiring. |
.github/copilot-instructions.md |
Updates generation guidance to reflect the new file responsibilities and conventions. |
index.php |
Adds a blank line after the PHP opening tag for consistency. |
images/index.php |
Adds strict types and minor formatting to the directory index stub. |
sounds/index.php |
Adds a blank line after the PHP opening tag for consistency. |
locales/index.php |
Adds a blank line after the PHP opening tag for consistency. |
locales/LC_MESSAGES/index.php |
Adds strict types and minor formatting to the directory index stub. |
| function pluginMonitorInstall(): void | ||
| { | ||
| if (!monitorEnsureSupportedPhpVersion()) { | ||
| return; | ||
| } | ||
|
|
||
| // core plugin functionality | ||
| api_plugin_register_hook('monitor', 'top_header_tabs', 'monitorShowTab', 'setup.php'); | ||
| api_plugin_register_hook('monitor', 'top_graph_header_tabs', 'monitorShowTab', 'setup.php'); |
There was a problem hiding this comment.
setup.php no longer defines the standard Cacti plugin entrypoints (e.g., plugin_monitor_install, plugin_monitor_uninstall, plugin_monitor_version, plugin_monitor_check_config, plugin_monitor_upgrade). Renaming these to pluginMonitor* will prevent Cacti’s plugin manager from calling them. Add backwards-compatible wrapper functions with the expected plugin_monitor_* names (or revert the rename) that delegate to the new implementations.
| } | ||
|
|
||
| if (get_request_var('rfilter') != '') { | ||
| $awhere .= ($awhere == '' ? '' : ' AND ') . " h.description RLIKE '" . get_request_var('rfilter') . "'"; |
There was a problem hiding this comment.
The renderWhereJoin() function concatenates the untrusted get_request_var('rfilter') directly into the SQL WHERE clause using RLIKE without any escaping or parameterization. An attacker controlling rfilter can break out of the quoted regex literal and inject arbitrary SQL, potentially reading or modifying data in the host table and beyond. This should instead build the RLIKE predicate with a bound parameter or a properly escaped value using the standard Cacti DB escaping utilities, and ensure that any regex metacharacters or quotes are safely handled before reaching the database.
| $awhere .= ($awhere == '' ? '' : ' AND ') . " h.description RLIKE '" . get_request_var('rfilter') . "'"; | |
| $rfilter = get_request_var('rfilter'); | |
| $awhere .= ($awhere == '' ? '' : ' AND ') . ' h.description RLIKE ' . db_qstr($rfilter); |
This pull request introduces a new helper file,
db_functions.php, containing utility functions for SQL query construction and host filtering, and updates the repository's Copilot instructions to clarify architectural, coding, and quality guidelines. Minor formatting changes are also made to other files. The most significant changes are grouped below.1. New Database Helper Utilities
db_functions.php, implementing procedural helper functions to centralize SQL predicate building, host filtering, and group concatenation logic for the plugin. These functions encapsulate common query patterns for thresholds, host status, and filtered host retrieval, improving maintainability and reducing code duplication.2. Documentation and Guidelines Update
.github/copilot-instructions.mdto provide clearer, more detailed instructions for code generation. The new guidelines emphasize strict version compatibility, procedural architecture, file responsibilities, coding patterns, and CI alignment. This ensures future contributions remain consistent with the established plugin structure and Cacti integration.3. Minor Formatting Adjustments
index.phpandimages/index.phpfor consistency with other files. [1] [2]