Repository navigation
Relocate theme CSS to css/ and keywords to docs/, add the file manifest + upgrade prune (compat 1.2.29) - #46
Merged
Merged
Conversation
TheWitness
requested review from
bmfmancini,
browniebraun,
cigamit and
xmacan
and
a balanced review from Copilot
September 30, 2026 14:51
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Compatibility documentation and the changelog must be synchronized with the new support floor.
Review effort: Balanced
Findings: 1
What changed in this PR
Lowers the minimum supported Cacti version from 1.2.32 to 1.2.29.
Changes:
- Updates the plugin compatibility floor.
| File | Description |
|---|---|
INFO |
Changes minimum Cacti compatibility to 1.2.29. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
added 3 commits
September 30, 2026 16:04
phpunit.xml ships in the repo for CI but is a dev-only test artifact, so it is removed from manifest.json 'expected', pruned from installs on upgrade (like tests/), and excluded from the manifest drift check. The retired markdown-lint configs (.mdlrc, .md_style.rb) are deleted, and .md* is now ignored like .git*: protected from pruning and excluded from drift if it reappears.
The reserved-words seed file moves to docs/; includes/database.php loads it from the new path during table setup, and it is tombstoned so existing installs drop the stale top-level copy on upgrade.
added 7 commits
September 30, 2026 22:17
The upgrade-time prune now rejects any tombstone containing '.'/'..' segments (which could escape the plugin directory or resolve to its root) and treats ancestors of whitelist entries as protected, so a tombstone on a parent directory can no longer delete a whitelisted file beneath it.
Point the structure tree at the relocated files (libraries now under includes/, data files under docs/, stylesheets under css/) and align the trailing '# ...' comments to a single column so they no longer drift right.
The themes/ -> css/ flatten dropped the $selected_theme directory segment, producing "plugins/slowlog/css//apexcharts.css", which never resolves the relocated css/modern/apexcharts.css. Both the import and chart views therefore always fell back to the generic stylesheet. Reference css/$selected_theme/apexcharts.css to match the new layout.
- Use the full license header (from the plugin's own setup.php) in tests/Unit/PruneFilesTest.php instead of the abbreviated copyright banner. - Document that the manifest drift check and the upgrade-time prune also handle phpunit.xml and .md* files, matching the implemented behavior.
The repository naming contract reserves the plugin_<name>_ prefix for plugin lifecycle / hook-registration functions; all other functions use the plain <name>_ prefix. Rename the internal upgrade helpers accordingly: plugin_<name>_prune_files() -> <name>_prune_files() plugin_<name>_rmtree() -> <name>_rmtree() The call site, unit tests, and the copilot-instructions.md references are updated to match. No behavioral change.
Align the compat metadata (and the contributor-guide references) with the team decision to standardize the minimum supported Cacti version at 1.2.29; the previously proposed 1.2.32 floor was not adopted.
Replace the per-theme css/<theme>/apexcharts.css subdirectory layout with a
flat css/<theme>_apexcharts.css naming scheme (e.g. css/modern_apexcharts.css),
repointing both the import and chart views at
css/{$selected_theme}_apexcharts.css.
TheWitness
changed the title
cigamit
approved these changes
Oct 1, 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.
Summary
Brings
plugin_slowlogin line with the fleet-wide conventions:manifest.jsonand prune mechanism, with drift enforcement wired into CI.INFOcompat floor to 1.2.29.File relocations (recorded as manifest tombstones)
themes/→css/(flattened) —themes/modern/apexcharts.cssis relocated tocss/modern_apexcharts.css(git mv, history preserved) using a flat, theme-prefixed filename; the fourthemes/$selected_theme/apexcharts.cssreferences inslowlog.phpare repointed tocss/{$selected_theme}_apexcharts.css. The retiredthemes/directory is tombstoned.keywords.txt→docs/keywords.txt— the SQL reserved-word list used by the parser moves underdocs/; the old top-level path is tombstoned.File manifest + upgrade-time pruning
manifest.json:tombstones=["keywords.txt", "themes/"],expected(the current top-level tree; directories carry a trailing/), andwhitelist(empty — no user-writable runtime tree).slowlog_prune_files()(insetup.php, called from the version-change branch ofslowlog_check_upgrade()afterslowlog_upgrade_tables()) removes the tombstoned paths, the dev-onlytests/tree, andphpunit.xmlon upgrade; leaveswhitelist,.git*, and.md*files in place; and logs — without removing — any unaccounted top-level entry../..traversal segment or resolves outside the plugin directory (including via a symlinked directory), and protects a directory when a whitelisted entry lives beneath it. Warns in the Cacti log on anything it cannot remove.tests/bin/validate-manifest.php(wired intoplugin-ci-workflow.yml) fails on drift betweenexpectedand the real top-level tree (it ignorestests/,phpunit.xml,.git*,.md*, and whitelisted paths).Compatibility
INFOcompatfloor set to 1.2.29 (the proposed 1.2.32 floor was not adopted).db_update_table()unique-key drop: itsdb_update_table()call targetsplugin_slowlog_details(which declares no unique key), while its onlyunique_keys-declared index (plugin_slowlog_table_names.table_name) is never passed throughdb_update_table(). No re-introduction helper is required.Docs & tests
tests/Unit/PruneFilesTest.php(tombstone/tests//phpunit.xmlremoval, whitelist &.gitprotection, traversal-segment and symlink-escape refusal, whitelist-ancestor protection), using the full standard GPL v2 header.SlowlogCheckUpgradeTestsandboxesbase_path(a temp tree with a copy ofINFO, an emptyincludes/database.php, and no manifest) for its drift cases so the prune is a no-op there while the new call line stays covered..github/copilot-instructions.mdto the new layout.Validation
php -lclean on all changed PHP files; full Pest suite green and patch-coverage passes at 100% of changed measured lines; manifest drift-check passes and the translation template is up to date.