Skip to content

refactor: replace Doctrine ORM and ExtJS 2.2 with Cacti-native patterns - #21

Open
somethingwithproof wants to merge 5 commits into
Cacti:developfrom
somethingwithproof:refactor/cacti-idiomatic-rewrite
Open

refactor: replace Doctrine ORM and ExtJS 2.2 with Cacti-native patterns#21
somethingwithproof wants to merge 5 commits into
Cacti:developfrom
somethingwithproof:refactor/cacti-idiomatic-rewrite

Conversation

@somethingwithproof

@somethingwithproof somethingwithproof commented Apr 9, 2026

Copy link
Copy Markdown
Member

Summary

  • Remove vendored Doctrine 1.x ORM (31K+ lines, 200+ files) and ExtJS 2.2 (2.5MB, 50+ files)
  • Replace all 69 Doctrine queries with Cacti db_fetch_*_prepared() parameterized statements
  • Replace ExtJS frontend with Cacti-native jQuery UI using standard page structure
  • Harden npc.php with module/action allowlist to prevent path traversal and arbitrary method invocation
  • Add 13 Pest v1 test suites (prepared statements, PHP 7.4 compat, auth, XSS, SQL injection, redirect safety)
  • Net: -78,262 lines removed, +3,231 lines added

Test plan

  • composer install && vendor/bin/pest passes all 13 test suites
  • NPC tab loads and displays host/service status from NDOUtils tables
  • Nagios commands submit through the command form
  • Dashboard portlets show host/service summary counts
  • Pagination works on hosts and services grids
  • Search/filter works across all grid views

@somethingwithproof
refactor: replace ExtJS 2.2 with Cacti-native jQuery UI
01f693e 
Remove vendored ExtJS 2.2 framework (2.5MB) and all ExtJS app code.
Replace with Cacti-idiomatic jQuery using standard page structure,
html_start_box/html_end_box table patterns, and jQuery UI dialogs.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof
test: add comprehensive Pest v1 test harness for NPC rewrite
aee8ab6 
13 test suites covering: prepared statement consistency, PHP 7.4
compatibility, auth guards, output escaping, SQL injection prevention,
redirect safety, setup structure, Doctrine removal verification,
ExtJS removal verification, and controller unit tests.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Copilot AI review requested due to automatic review settings April 9, 2026 08:49
Copilot AI reviewed Apr 9, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR modernizes the NPC plugin by removing legacy vendored dependencies (Doctrine 1.x ORM and ExtJS 2.2 assets) and migrating server-side data access to Cacti-native db_fetch_*_prepared() calls, while updating the UI entrypoint to a Cacti-native page/tab structure.

Changes:

  • Removed ExtJS 2.2 resources (CSS/themes/adapter) and legacy build artifacts (Ant build).
  • Replaced multiple Doctrine query paths with parameterized Cacti DB helpers and updated controllers accordingly.
  • Added Composer dev tooling configuration for Pest test execution.

Reviewed changes

Copilot reviewed 56 out of 1290 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
js/ext/resources/css/xtheme-olive.css Removed vendored ExtJS theme asset.
js/ext/resources/css/xtheme-gray.css Removed vendored ExtJS theme asset.
js/ext/resources/css/xtheme-darkgray.css Removed vendored ExtJS theme asset.
js/ext/resources/css/window.css Removed vendored ExtJS CSS.
js/ext/resources/css/tree.css Removed vendored ExtJS CSS.
js/ext/resources/css/toolbar.css Removed vendored ExtJS CSS.
js/ext/resources/css/tabs.css Removed vendored ExtJS CSS.
js/ext/resources/css/slider.css Removed vendored ExtJS CSS.
js/ext/resources/css/resizable.css Removed vendored ExtJS CSS.
js/ext/resources/css/reset.css Removed vendored ExtJS CSS reset.
js/ext/resources/css/reset-min.css Removed vendored ExtJS CSS reset (min).
js/ext/resources/css/qtips.css Removed vendored ExtJS CSS.
js/ext/resources/css/progress.css Removed vendored ExtJS CSS.
js/ext/resources/css/panel.css Removed vendored ExtJS CSS.
js/ext/resources/css/menu.css Removed vendored ExtJS CSS.
js/ext/resources/css/layout.css Removed vendored ExtJS CSS.
js/ext/resources/css/grid.css Removed vendored ExtJS CSS.
js/ext/resources/css/form.css Removed vendored ExtJS CSS.
js/ext/resources/css/editor.css Removed vendored ExtJS CSS.
js/ext/resources/css/dialog.css Removed vendored ExtJS CSS.
js/ext/resources/css/debug.css Removed vendored ExtJS debug CSS.
js/ext/resources/css/dd.css Removed vendored ExtJS drag/drop CSS.
js/ext/resources/css/date-picker.css Removed vendored ExtJS date picker CSS.
js/ext/resources/css/core.css Removed vendored ExtJS core CSS.
js/ext/resources/css/combo.css Removed vendored ExtJS combo CSS.
js/ext/resources/css/button.css Removed vendored ExtJS button CSS.
js/ext/resources/css/box.css Removed vendored ExtJS box CSS.
js/ext/resources/css/borders.css Removed vendored ExtJS borders CSS.
js/ext/resources/css/README.txt Removed ExtJS CSS packaging note.
js/ext/license.txt Removed ExtJS license file (dependency removed).
js/ext/adapter/jquery/ext-jquery-adapter.js Removed ExtJS jQuery adapter (dependency removed).
css/main.css Removed ExtJS-era plugin CSS (portlets/icons/status).
css/ext-ux-livegrid.css Removed ExtJS UX LiveGrid CSS.
controllers/statehistory.php Migrated state history paging/querying from Doctrine to prepared DB calls.
controllers/settings.php Migrated settings storage retrieval/update off Doctrine to prepared DB calls.
controllers/servicegroups.php Refactored servicegroup aggregation to DB helpers and simplified JSON response building.
controllers/notifications.php Migrated notifications paging/querying from Doctrine to prepared DB calls.
controllers/nagios.php Migrated several Doctrine queries to prepared DB calls; adjusted process info formatting output.
controllers/logentries.php Migrated log entries paging/querying to prepared DB calls; updated search binding behavior.
controllers/layoutDev.php Removed ExtJS-based dev layout controller.
controllers/layout.php Replaced ExtJS entry layout with Cacti-native header/footer and tab navigation + JS bootstrap.
controllers/hosts.php Migrated host listing/summary/mapping logic to prepared DB calls, added sort allowlist.
controllers/hostgroups.php Migrated hostgroup queries to prepared DB calls and simplified JSON responses.
controllers/downtime.php Migrated scheduled downtime and downtime history queries to prepared DB calls.
controllers/controller.php Updated shared searchClause() to generate parameterized LIKE conditions and bind params.
controllers/comments.php Migrated comment queries to prepared DB calls and updated icon lookup.
config.php Removed Doctrine bootstrap and kept Controller bootstrap only.
composer.json Added Composer config for Pest dev dependency + test bootstrap autoload.
build.xml Removed legacy Ant/YUICompressor build script.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread controllers/controller.php Outdated
Comment thread controllers/comments.php Outdated
Comment thread controllers/settings.php Outdated
Comment thread controllers/settings.php Outdated
Comment thread controllers/hostgroups.php Outdated
Comment thread controllers/layout.php Outdated
Comment thread controllers/servicegroups.php
Comment thread controllers/statehistory.php Outdated
Comment thread controllers/statehistory.php Outdated
@somethingwithproof
fix(security): whitelist sort column in comments controller to preven…
e2fff92 
…t SQL injection

The $this->sort and $this->dir values flow from user request into an
ORDER BY clause without any allowlist. Restrict $sort to known columns
and $dir to ASC/DESC.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof
fix(security): pass allowed_classes to unserialize
628b69b 
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof
somethingwithproof marked this pull request as draft April 11, 2026 00:10
@somethingwithproof

somethingwithproof commented Apr 11, 2026

Copy link
Copy Markdown
Member Author

Converted to draft to serialize the stack in this repo. Blocked by #18; will un-draft after that merges to avoid cross-PR merge conflicts.

@somethingwithproof
fix: address NPC review findings
c183136 
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof
somethingwithproof marked this pull request as ready for review August 29, 2026 05:54
@somethingwithproof somethingwithproof self-assigned this Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@xmacan xmacan xmacan approved these changes
@netniV netniV Awaiting requested review from netniV
@TheWitness TheWitness Awaiting requested review from TheWitness

Assignees

@somethingwithproof somethingwithproof

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@somethingwithproof @xmacan