Repository navigation
Shorten 'Add to QuickTree' graph button title to 'Add to Quicktree' - #44
Merged
Merged
Conversation
TheWitness
requested review from
bmfmancini and
xmacan
and
a balanced review from Copilot
October 12, 2026 18:08
There was a problem hiding this comment.
🟡 Changes recommended
The diff contains broken tests, compatibility regressions, missing coverage, and substantial undocumented behavior beyond the stated title-only change.
6 open findings
What changed in this PR
Shortens the graph action label, but the diff also introduces extensive runtime, compatibility, security, styling, upgrade, and CI changes beyond the stated scope.
Changes:
- Renames the graph action to “Add to Quicktree” and updates gettext.
- Refactors plugin UI, assets, database access, redirects, upgrades, and compatibility.
- Adds Pest tests, security checks, manifest validation, and CI workflows.
| File | Description |
|---|---|
tests/Unit/QuicktreeVersionTest.php |
Tests plugin metadata parsing. |
tests/Unit/QuicktreeShowTabTest.php |
Tests tab visibility and URL generation. |
tests/Unit/QuicktreeSetupTableTest.php |
Tests page-style defaults. |
tests/Unit/QuicktreePageLocationTest.php |
Tests location selection. |
tests/Unit/QuicktreePageHeadTest.php |
Tests asset inclusion. |
tests/Unit/QuicktreeLifecycleTest.php |
Tests lifecycle hooks. |
tests/Unit/QuicktreeGraphButtonsTest.php |
Tests graph action markup. |
tests/Unit/QuicktreeConfigSettingsAndArraysTest.php |
Tests settings and menus. |
tests/Unit/QuicktreeCheckUpgradeTest.php |
Adds upgrade tests, currently not parsed as PHP. |
tests/Unit/PruneFilesTest.php |
Tests upgrade pruning. |
tests/Unit/LocationNormalizationTest.php |
Tests location allowlisting. |
tests/Unit/FormWrapperTest.php |
Tests form helpers. |
tests/Unit/CspNonceTest.php |
Tests CSP nonce handling. |
tests/Security/SetupStructureTest.php |
Checks setup structure. |
tests/Security/RedirectSafetyTest.php |
Checks redirect termination. |
tests/Security/PreparedStatementsMigrationTest.php |
Checks prepared-query migration. |
tests/Security/PreparedStatementConsistencyTest.php |
Checks database helper usage. |
tests/Security/PhpCompatibilityTest.php |
Checks selected PHP features. |
tests/Security/OutputEscapingTest.php |
Checks output escaping. |
tests/Security/AuthGuardTest.php |
Checks authentication and ID validation. |
tests/Pest.php |
Documents Pest configuration. |
tests/Integration/QuicktreeRedirectSafetyTest.php |
Checks redirect wiring. |
tests/Integration/QuicktreeInstallHooksTest.php |
Tests hook installation. |
tests/E2E/QuicktreeNoDebugHeaderTest.php |
Guards against debug headers. |
tests/bootstrap-unit.php |
Adds the test harness and Cacti stubs. |
tests/bin/validate-manifest.php |
Validates manifest contents. |
tests/bin/patch-coverage.php |
Adds changed-line coverage enforcement. |
tests/bin/check-i18n-pot.php |
Checks gettext-template updates. |
tests/.cacti-version |
Selects Cacti 1.2.x for tests. |
setup.php |
Refactors hooks, assets, upgrades, and graph actions. |
README.md |
Documents Cancel-button compatibility. |
quicktree.php |
Refactors UI, queries, redirects, and CSP handling. |
phpunit.xml |
Configures Pest/PHPUnit. |
manifest.json |
Defines shipped and obsolete paths. |
locales/po/index.php |
Updates copyright metadata. |
locales/po/cacti.pot |
Regenerates translations. |
locales/LC_MESSAGES/index.php |
Updates copyright metadata. |
locales/index.php |
Updates header and PHP style. |
locales/build_gettext.sh |
Changes gettext generation. |
js/quicktree.js |
Adds delegated graph-action handling. |
INFO |
Raises compatibility and updates metadata. |
index.php |
Updates copyright metadata. |
includes/security.php |
Adds location normalization. |
includes/helpers.php |
Adds action-form helpers. |
css/sunrise.css |
Adds theme-specific glyph colors. |
css/quicktree.css |
Adds default glyph colors. |
css/paw.css |
Adds theme-specific glyph colors. |
css/paper-plane.css |
Adds theme-specific glyph colors. |
css/modern.css |
Adds theme-specific glyph colors. |
css/midwinter.css |
Adds theme-specific glyph colors. |
css/deepness.css |
Adds theme-specific glyph colors. |
css/dark.css |
Adds theme-specific glyph colors. |
css/classic.css |
Adds theme-specific glyph colors. |
CHANGELOG.md |
Documents broad feature and security changes. |
.gitignore |
Ignores generated translations and dependencies. |
.github/workflows/plugin-ci-workflow.yml |
Adds integration and test CI. |
.github/workflows/codeql.yml |
Adds JavaScript CodeQL analysis. |
.github/PULL_REQUEST_TEMPLATE.md |
Adds a pull-request template. |
.github/ISSUE_TEMPLATE/feature_request.md |
Adds a feature template. |
.github/ISSUE_TEMPLATE/bug_report.md |
Adds a bug template. |
.github/instructions/php.instructions.md |
Adds PHP contribution guidance. |
.github/dependabot.yml |
Configures Actions updates. |
.github/copilot-instructions.md |
Adds repository development rules. |
.gitattributes |
Enforces LF line endings. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…cktree' The graph drilldown action icons now render their title as a visible menu label in Cacti core; shorten this plugin's icon title from 'Add this graph to QuickTree' to 'Add to Quicktree' so it reads cleanly there. Regenerated locales/po/cacti.pot for the changed string.
TheWitness
force-pushed
the
fix/shorten-quicktree-graph-button-title
branch
from
October 12, 2026 20:01
bba9042 to
d3b40df
Compare
Adds the missing assertion for this PR's sole behavior (the new visible action label) and refreshes the stale docblock reference.
There was a problem hiding this comment.
🟡 Changes recommended
The new test lines violate the repository’s required tab indentation and may fail formatting checks.
1 open finding
6 resolved since last review
🧠 Review effort: Balanced
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
Cacti core now renders each graph drilldown action icon's
titleas a visible text label in the hamburger action menu (Cacti PR #8316). This plugin's graph button titleAdd this graph to QuickTreereads awkwardly as a menu label, so shorten it to Add to Quicktree.Changes
setup.php—quicktree_graph_buttons(): title__esc('Add this graph to QuickTree', 'quicktree')→__esc('Add to Quicktree', 'quicktree')(and the matching docblock line).locales/po/cacti.pot— regenerated for the changed string (vialocales/build_gettext.sh); only the template is committed, the per-language.po/.moare left to the normal translation workflow.No behavioural change — the icon, its
quicktreeAdd/quicktreeAddLinkclasses, data attributes and delegated click binding are unchanged, sotests/Unit/QuicktreeGraphButtonsTest.php(which asserts those classes) still passes.