diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index e408b48..72f4d92 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -25,12 +25,14 @@ When generating code for this repository: ``` maint/ # Repository root (install to plugins/maint/ in Cacti) -├── locales/ # Internationalization files -├── functions.php # Runtime maintenance checks (plugin_maint_check_* -> plugin_maint_check_schedule) -├── maint.php # Main UI/controller: CRUD schedules, tabbed views, host associations -├── INFO # Plugin metadata (name, version, compat) +├── includes/ # Library/helper files, require_once'd from the entry points +│ ├── database.php # Schema management: table defs + create/upgrade helpers +│ └── functions.php # Runtime maintenance checks (plugin_maint_check_* -> plugin_maint_check_schedule) +├── locales/ # Internationalization files +├── maint.php # Main UI/controller: CRUD schedules, tabbed views, host associations +├── INFO # Plugin metadata (name, version, compat) ├── README.md -└── setup.php # Plugin entry points and hooks (menu, realms, device actions, maintenance hook) +└── setup.php # Plugin entry points and hooks (menu, realms, device actions, maintenance hook) ``` ## Naming Conventions @@ -77,7 +79,14 @@ arithmetic, strict `===` comparisons). ## Database Operations -Tables are created in `maint_setup_database()` via `api_plugin_db_table_create()`; `plugin_maint_check_schedule()` updates `stime`/`etime` for recurring schedules when windows pass, and schedule listing in `maint.php` uses it to compute "Active" state. +All schema management lives in `includes/database.php` (the thold model), not in `setup.php`. `setup.php`'s +install/upgrade paths `require_once($config['base_path'] . '/plugins/maint/includes/database.php')` and +delegate. Each table is defined once (`maint_schedules_table_data()`, `maint_hosts_table_data()`) and created +via `api_plugin_db_table_create('maint', ...)`; `plugin_maint_check_upgrade()` version-gates against +`plugin_config`, refreshes the schema through `maint_upgrade_tables()` (`db_update_table()` when the table +exists, create fallback when missing), and updates the full `plugin_config` row on a version change. +`plugin_maint_check_schedule()` (in `includes/functions.php`) updates `stime`/`etime` for recurring schedules +when windows pass, and schedule listing in `maint.php` uses it to compute "Active" state. ## Internationalization @@ -126,7 +135,7 @@ Document all changes via commit history; use descriptive commit messages referen - Translatable strings are managed with GNU gettext via `locales/build_gettext.sh`. `locales/po/cacti.pot` is the source template; Weblate owns syncing the per-language `.po`/`.mo` files from it. - **Never commit the per-language `.po` or compiled `.mo` files** (`locales/po/*.po`, `locales/LC_MESSAGES/*.mo`) in a plugin PR. Weblate is the sole owner of those catalogs, and regenerating them here produces spurious diffs and merge conflicts. `locales/po/cacti.pot` is the ONLY translation artifact a PR may add or modify. -- When a pull request adds or changes a string wrapped in `__()`/`__n()`/`__esc()`/`__x()`/`__xn()`/`__gettext()`, run `locales/build_gettext.sh` before pushing and stage `locales/po/cacti.pot` only. `build_gettext.sh` also rewrites the `.po`/`.mo` files as a side effect; revert those before committing (`git checkout -- locales/po/*.po locales/LC_MESSAGES`), or run only the `xgettext` step that targets `cacti.pot`. +- When a pull request adds or changes a string wrapped in `__()`/`__n()`/`__esc()`/`__x()`/`__xn()`/`__gettext()`, run `locales/build_gettext.sh` before pushing and stage `locales/po/cacti.pot` only. `build_gettext.sh` also rewrites the `.po`/`.mo` files as a side effect; revert those before committing (`git checkout -- locales/po/*.po locales/LC_MESSAGES`), or run only the `xgettext` step that targets `cacti.pot`. ## References @@ -150,6 +159,11 @@ existing code or adding new code, not just in dedicated cleanup passes: - **i18n text domain.** Every `__()`/`__esc()` call must include this plugin's text domain as the final argument, except when deliberately comparing against a literal, untranslated Cacti-core label. +- **File inclusion uses `require`/`require_once`.** Always use `require`/`require_once` (never + `include`/`include_once`) so a missing dependency fails fast and loudly. Keep library/helper files + (e.g. `functions.php`, `includes/database.php`) under `includes/` and reference them from that path; + entry points (`maint.php`, `setup.php`) stay in the plugin root. A hook registered for a function that + lives in `includes/functions.php` must pass `'includes/functions.php'` as its file argument. - **Plugin table-creation API.** Use `api_plugin_db_table_create()`/`api_plugin_db_add_column()` (from Cacti core's `lib/plugins.php`) instead of raw `CREATE TABLE`/`ALTER TABLE ... ADD COLUMN`. Both are idempotent (safe no-ops when already applied), so the same call can run unconditionally @@ -158,3 +172,7 @@ existing code or adding new code, not just in dedicated cleanup passes: line, `@param` lines, a blank comment line, then `@return`. Infer parameter/return types from actual usage; don't change the function's real type-hints in the same pass (let static analysis flag mismatches separately). Skip vendored third-party library files. + +## File manifest & upgrade pruning + +The plugin ships a root `manifest.json` with three arrays: `tombstones` (files/directories older versions shipped that have since moved or been removed), `expected` (the top-level files and directories that ship today, directories written with a trailing `/`), and `whitelist` (paths holding user data that must never be touched). Keep `expected` current: CI runs `tests/bin/validate-manifest.php`, which fails on any drift between `expected` and the real top-level tree (it ignores `tests/`, `phpunit.xml`, `.git*`, `.md*`, and whitelisted paths). Custom customer CSS/theme files belong in `expected`, and stylesheets live in `css/` (not `themes/`). On upgrade, `maint_prune_files()` deletes the tombstoned paths, the dev-only `tests/` tree, and the `phpunit.xml` test config, leaves `whitelist`, `.git*`, and `.md*` alone, and logs (without removing) any top-level entry the manifest does not account for. As a safety measure it refuses any tombstone that resolves outside the plugin directory (a tampered manifest.json) and logs a warning for any file or directory it cannot remove. When you move or delete a shipped file, add its old path to `tombstones` and update `expected` in the same change. diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 2c09779..d3f0560 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -89,6 +89,9 @@ jobs: - name: Check PHP version run: php -v + - name: Validate plugin manifest (expected-file drift) + run: php cacti/plugins/maint/tests/bin/validate-manifest.php + - name: Run apt-get update run: sudo apt-get update diff --git a/CHANGELOG.md b/CHANGELOG.md index a4d1c76..0ba67bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ --- develop --- +* refactor: Move schema management into includes/database.php (the thold model) and relocate functions.php into includes/; setup.php delegates via require_once, and plugin_maint_check_upgrade() now refreshes the schema via db_update_table(), re-registers the is_device_in_maintenance hook against includes/functions.php, and updates the full plugin_config row on a version change. Switches every file inclusion from include/include_once to require/require_once * dev: Enforce patch coverage of changed lines in CI and remove the inert COMPOSER_ROOT_VERSION env from the Pest step * feature: Add a standard Cacti search filter to the Maintenance Schedules list that also matches the hostname/description/location/site name of associated Thold devices and the hostname/description of associated Servcheck tests * security: Add a version-safe CSP nonce (`plugin_maint_csp_nonce()`) to every inline `