diff --git a/.claude/skills/pr-analysis/SKILL.md b/.claude/skills/pr-analysis/SKILL.md index 3a17b8a88..1f97474d7 100644 --- a/.claude/skills/pr-analysis/SKILL.md +++ b/.claude/skills/pr-analysis/SKILL.md @@ -47,7 +47,7 @@ For each comment, determine: 1. **Identify all actionable comments** before touching any file. 2. **Load relevant skills** to understand conventions that apply. -3. **Prepare a plan** — list each file and the exact change required. +3. **Prepare a plan** — list each file and the exact change required. If a comment calls for new logic (a new check, helper, or condition), search for an existing equivalent first - the whole file being edited, not just the section in question, plus sibling pages/the Python backend - and extract/reuse it rather than planning a parallel implementation (see `code-standards`' DRY Principle section). 4. **Make changes one comment at a time** — keep commits focused. 5. **Run targeted tests** after each change (`testing-workflow` skill). 6. **Reply** only after the commit is pushed. Include the short SHA. diff --git a/.claude/skills/prd-writing/SKILL.md b/.claude/skills/prd-writing/SKILL.md index 08a539fea..85facbd2f 100644 --- a/.claude/skills/prd-writing/SKILL.md +++ b/.claude/skills/prd-writing/SKILL.md @@ -9,6 +9,8 @@ description: Read before writing a PRD, design doc, or feature proposal. Covers Triggered by: "write a PRD", "draft a design doc", "spec out this feature", "create a PRD for X". Reserve this for changes where getting the design wrong is expensive to unwind — new cross-cutting mechanisms, schema changes, anything touching multiple subsystems. A one-file bug fix doesn't need this process. +**A UI feature that needs to survive a page reload, coordinate across tabs, or react to a backend push/poll is a cross-cutting mechanism even when it looks like "just a small UI feature."** It's easy to start implementing straight away because the visible surface (a badge, an icon) looks trivial - but the hard part is always the state-propagation mechanism underneath, and that mechanism usually has hidden dependencies on other code that already touches the same data. A real case: a "settings still applying" indicator went through three full implementation-and-break cycles (a cookie with a guessed timeout, a new PHP endpoint duplicating existing logic, a `localStorage`/polling redesign with its own bugs) before anyone traced every existing consumer of `app_state.json` and found that `front/js/sse_manager.js`'s `handleStateUpdate()` already pushed the exact signal needed to every open page. Step 4 ("trace every downstream consumer") below would have caught this on attempt #1 if it had been applied before writing any code, not after three failures. + ## Core principle: a PRD is a claim-verification exercise, not a writing exercise Every sentence that asserts something about how the code currently works must be checked against the actual code before it goes in — not written from memory, not inferred from a plugin's name or reputation, not assumed because it sounds plausible. Two failure patterns to watch for: diff --git a/.gemini/skills/pr-analysis/SKILL.md b/.gemini/skills/pr-analysis/SKILL.md index edd32ba48..8202014db 100644 --- a/.gemini/skills/pr-analysis/SKILL.md +++ b/.gemini/skills/pr-analysis/SKILL.md @@ -47,7 +47,7 @@ For each comment, determine: 1. **Identify all actionable comments** before touching any file. 2. **Load relevant skills** to understand conventions that apply. -3. **Prepare a plan** — list each file and the exact change required. +3. **Prepare a plan** — list each file and the exact change required. If a comment calls for new logic (a new check, helper, or condition), search for an existing equivalent first - the whole file being edited, not just the section in question, plus sibling pages/the Python backend - and extract/reuse it rather than planning a parallel implementation (see `code-standards`' DRY Principle section). 4. **Make changes one comment at a time** — keep commits focused. For a comment claiming a bug: write the test that should catch it first, run it against the current (unfixed) code, and confirm it fails for the right reason before writing the fix - see `prd-writing`'s test-first step for why (a test that never failed red can't be trusted to have caught anything). 5. **Rerun tests after each change** (`testing-workflow` skill) - confirm the new/updated test now passes, not just that nothing else broke. 6. **Reply** only after the commit is pushed. Include the short SHA. diff --git a/.gemini/skills/prd-writing/SKILL.md b/.gemini/skills/prd-writing/SKILL.md index 41fdb1896..313d7b26f 100644 --- a/.gemini/skills/prd-writing/SKILL.md +++ b/.gemini/skills/prd-writing/SKILL.md @@ -9,6 +9,8 @@ description: Rigorous PRD-writing methodology — challenge the idea, verify eve Triggered by: "write a PRD", "draft a design doc", "spec out this feature", "create a PRD for X". Reserve this for changes where getting the design wrong is expensive to unwind — new cross-cutting mechanisms, schema changes, anything touching multiple subsystems. A one-file bug fix doesn't need this process. +**A UI feature that needs to survive a page reload, coordinate across tabs, or react to a backend push/poll is a cross-cutting mechanism even when it looks like "just a small UI feature."** It's easy to start implementing straight away because the visible surface (a badge, an icon) looks trivial - but the hard part is always the state-propagation mechanism underneath, and that mechanism usually has hidden dependencies on other code that already touches the same data. A real case: a "settings still applying" indicator went through three full implementation-and-break cycles (a cookie with a guessed timeout, a new PHP endpoint duplicating existing logic, a `localStorage`/polling redesign with its own bugs) before anyone traced every existing consumer of `app_state.json` and found that `front/js/sse_manager.js`'s `handleStateUpdate()` already pushed the exact signal needed to every open page. Step 4 ("trace every downstream consumer") below would have caught this on attempt #1 if it had been applied before writing any code, not after three failures. + ## Core principle: a PRD is a claim-verification exercise, not a writing exercise Every sentence that asserts something about how the code currently works must be checked against the actual code before it goes in — not written from memory, not inferred from a plugin's name or reputation, not assumed because it sounds plausible. Two failure patterns to watch for: diff --git a/.github/skills/code-standards/SKILL.md b/.github/skills/code-standards/SKILL.md index 41a1a90ba..07c7c3066 100644 --- a/.github/skills/code-standards/SKILL.md +++ b/.github/skills/code-standards/SKILL.md @@ -27,6 +27,7 @@ description: NetAlertX coding standards and conventions. Use this when writing c - when using `server/logger.py` `mylog()`, only use valid levels: `none`, `minimal`, `verbose`, `debug`, `trace`; invalid levels silently degrade to `none` - every Python function/method needs a succinct docstring describing its current use and behavior — not what changed or why (see Docstrings section below) - before adding a new frontend language string, search `front/php/templates/language/en_us.json` for an existing key with the same text/purpose and reuse it — don't add a near-duplicate key just because it's needed on a new page (see Language Strings section below) +- never add new server-side PHP logic (a new endpoint, new computation inside an existing PHP file) — `front/` is being migrated away from PHP, so any new backend state/computation belongs in the Python server, exposed to the frontend via an existing read path (see PHP/Python Boundary section below) ## File Length @@ -37,6 +38,10 @@ Keep code files under 500 lines. Split larger files into modules. Do not re-implement functionality. Reuse existing methods or refactor to create shared methods. +**This is a required pre-step, not a cleanup pass to do later.** Before writing any new check/condition/helper, search for an existing implementation of the same or similar logic first - grep the codebase, and read the *whole* file you're already touching, not just the section being edited. If something equivalent exists, extract it into a shared function and call it from the new site instead of writing a parallel implementation. + +A real case this was missed on: a new frontend indicator needed to know "is the backend still applying a settings change." That exact check already existed inline in `settings.php`'s own polling loop (`handleLoadingDialog()`, further down the same file being edited) - it took two rounds of reinventing it elsewhere (a cookie-based guess, then a duplicate PHP endpoint computing the same thing a second time) before it got extracted into one shared function (`isSettingsPending()` in `common.js`) that both the original page and the new consumer call. Read the existing code first; refactor into something reusable *while* implementing, not after a reviewer points out the duplication. + ## Database Access - Never access DB directly from application layers @@ -109,6 +114,26 @@ grep -n "Next\|Previous\|Showing" front/php/templates/language/en_us.json Prefer the generic `Gen_*` keys (e.g. `Gen_Prev`, `Gen_Next`) over a page-scoped name (`Presence_Page_Prev`) for genuinely generic UI text — a future page needing the same label should find it already there. Only add a new key when nothing existing fits; only that one file needs the addition — `getString()`/`lang()` fall back to the English string for any locale missing a key, so the other ~23 locale files don't need touching. +## PHP/Python Boundary — No New PHP Backend Logic + +`front/` is being migrated away from PHP. Never add a new PHP endpoint, or new server-side computation inside an existing PHP file - if a feature needs backend state or computation, it belongs in the Python server (`server/`), exposed to the frontend through an existing read path: + +- `app_state.json`, read via the generic `front/php/server/query_json.php` file-passthrough (no settings/state-specific logic lives in that file - it just serves raw JSON) +- `table_settings.json` (same passthrough) +- an existing REST or GraphQL endpoint + +A real case this was caught on: a new "settings still applying" UI indicator needed to know whether the backend had caught up on a config reload. The correct signal (`showSpinner` state + a config-file-mtime comparison) already existed in Python (`server/initialise.py`'s `importConfigs()`) - the first draft instead re-derived the same comparison in a new PHP endpoint, duplicating logic that the Python backend already computed and should have just exposed into existing shared state. + +Editing *existing* PHP page logic - templating, fixing a bug like a broken `explode()` parse, wiring up a new `
` - is fine and expected during the migration period. This rule is about not growing the PHP surface area with new backend-side logic, not about avoiding PHP entirely. + +## No Test Harness? Simulate Before Asking for a Live Test + +`front/` has no automated JS/PHP test suite. That makes it *more* important to verify a change before calling it done, not less - without a harness, "the user tests it live" becomes the only feedback loop, and that loop is slow and expensive (a real save, a real scan cycle, real timing) compared to a throwaway script. + +Before telling anyone a JS/PHP change is ready to test: write a small disposable Node (or PHP CLI) script that extracts the actual function(s) involved and runs them against realistic inputs - including the inputs that come from a different code path than the one being edited (a real `app_state.json` sample, a real cookie value, a renamed parameter actually being passed through). Do this on the *first* attempt, not after a live test comes back broken. + +A real case: a settings-reload indicator went through several rounds of "should work" before any of its logic was actually run. A standalone simulation run at that point would have immediately caught a renamed-parameter typo that a diff review missed, and an ordering bug (a cookie needing to clear before a reload fires, not inside the reload's own callback) - both found only after a live test failed, when a five-line script could have found them in seconds. + ## Devcontainer Constraints - Never `chmod` or `chown` during operations diff --git a/.github/skills/pr-analysis/SKILL.md b/.github/skills/pr-analysis/SKILL.md index 0a9c37da5..a9aaa2513 100644 --- a/.github/skills/pr-analysis/SKILL.md +++ b/.github/skills/pr-analysis/SKILL.md @@ -47,7 +47,7 @@ For each comment, determine: 1. **Identify all actionable comments** before touching any file. 2. **Load relevant skills** to understand conventions that apply. -3. **Prepare a plan** — list each file and the exact change required. +3. **Prepare a plan** — list each file and the exact change required. If a comment calls for new logic (a new check, helper, or condition), search for an existing equivalent first - the whole file being edited, not just the section in question, plus sibling pages/the Python backend - and extract/reuse it rather than planning a parallel implementation (see `code-standards`' DRY Principle section). 4. **Make changes one comment at a time** — keep commits focused. For a comment claiming a bug: write the test that should catch it first, run it against the current (unfixed) code, and confirm it fails for the right reason before writing the fix - see `prd-writing`'s test-first step for why (a test that never failed red can't be trusted to have caught anything). 5. **Rerun tests after each change** (`testing-workflow` skill) - confirm the new/updated test now passes, not just that nothing else broke. 6. **Reply** only after the commit is pushed via `report_progress`. Include the short SHA. diff --git a/.github/skills/prd-writing/SKILL.md b/.github/skills/prd-writing/SKILL.md index c0fbdbe76..f74c3cc15 100644 --- a/.github/skills/prd-writing/SKILL.md +++ b/.github/skills/prd-writing/SKILL.md @@ -9,6 +9,8 @@ description: Rigorous PRD-writing methodology for NetAlertX — challenge the id Triggered by: "write a PRD", "draft a design doc", "spec out this feature", "create a PRD for X". Reserve this for changes where getting the design wrong is expensive to unwind — new cross-cutting mechanisms, schema changes, anything touching multiple subsystems. A one-file bug fix doesn't need this process. +**A UI feature that needs to survive a page reload, coordinate across tabs, or react to a backend push/poll is a cross-cutting mechanism even when it looks like "just a small UI feature."** It's easy to start implementing straight away because the visible surface (a badge, an icon) looks trivial - but the hard part is always the state-propagation mechanism underneath, and that mechanism usually has hidden dependencies on other code that already touches the same data. A real case: a "settings still applying" indicator went through three full implementation-and-break cycles (a cookie with a guessed timeout, a new PHP endpoint duplicating existing logic, a `localStorage`/polling redesign with its own bugs) before anyone traced every existing consumer of `app_state.json` and found that `front/js/sse_manager.js`'s `handleStateUpdate()` already pushed the exact signal needed to every open page. Step 4 ("trace every downstream consumer") below would have caught this on attempt #1 if it had been applied before writing any code, not after three failures. + ## Core principle: a PRD is a claim-verification exercise, not a writing exercise Every sentence that asserts something about how the code currently works must be checked against the actual code before it goes in — not written from memory, not inferred from a plugin's name or reputation, not assumed because it sounds plausible. Two failure patterns to watch for: diff --git a/CLAUDE.md b/CLAUDE.md index 87c774b29..e36b7ea20 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -91,3 +91,7 @@ Procedural/how-to knowledge (running tests, resetting the DB, devcontainer manag - Keep files under ~500 lines; split rather than grow. - Every Python function/method gets a succinct docstring describing its current use and behavior — one or two sentences, not a changelog of what changed or why (that belongs in the commit/PR, not the docstring). Same rule for JS: a JSDoc `/** ... */` block, not a plain `//` line above the function. Whenever you touch a function that only has a plain description comment (Python or JS), convert it to a proper docstring as part of that edit rather than leaving the old style next to new code. - Before adding a new key to `front/php/templates/language/en_us.json`, search it for an existing key with the same text/purpose and reuse it - prefer generic `Gen_*` keys over page-scoped names for genuinely generic UI text (e.g. `Gen_Prev`/`Gen_Next`, not `Presence_Page_Prev`). Only the English file needs a real translation; other locales fall back to it automatically at runtime for a key they don't have. After adding or changing any key in `en_us.json`, run `python3 front/php/templates/language/merge_translations.py` (plain stdlib, no deps) - it re-sorts `en_us.json` alphabetically and propagates the new key into every other locale file with an empty placeholder value, so translators see what needs translating. Skipping this leaves the other 23 locale files out of sync with `en_us.json`'s key set. +- **Search before you build.** Before writing a new check/condition/helper for something (an "is X true" computation, a UI state signal, a utility), search the codebase for an existing implementation of the same or similar logic first - the same file (read the whole file, not just the section being edited), a sibling page, the Python backend. If one exists, extract it into a shared function and call it from the new site; don't write a parallel implementation planning to deduplicate later. A real case: a new frontend indicator needed to know "is the backend still applying a settings change" - that exact check already existed inline in `settings.php`'s own polling loop (`handleLoadingDialog()`), found only after two rounds of reinventing it elsewhere (a cookie-based guess, then a duplicate PHP endpoint) instead of reading the rest of the file first. +- **No new PHP backend logic.** `front/` is being migrated away from PHP, so never add a new PHP endpoint or new server-side computation inside an existing PHP file. If a feature needs backend state or computation, add it to the Python server and expose it to the frontend through an existing read path (`app_state.json` via `query_json.php`, `table_settings.json`, a REST/GraphQL endpoint) - never re-derive logic in PHP that the Python side already knows or could easily expose. Editing existing PHP page logic (templating, bug fixes) is fine; this is about not growing the PHP surface area. +- **A stateful UI feature (survives a reload, coordinates across tabs, reacts to a backend push) is a cross-cutting mechanism, not "just a UI feature."** Treat it like one before writing code: trace every existing consumer of the data it needs (e.g. everything that already reads `app_state.json`), not just the one file being edited. The visible surface looking small (a badge, an icon) says nothing about whether the state-propagation mechanism underneath already exists elsewhere. +- **No JS/PHP test harness exists in `front/` — simulate before asking for a live test, every time, not after a live test fails.** Write a disposable Node/PHP script that runs the actual function(s) against realistic inputs (including inputs crossing from a different file than the one being edited) before calling a change ready to test. A diff review misses things a five-second script run catches - a renamed parameter still referenced by its old name, an ordering assumption that's wrong once two async steps are both in play. diff --git a/front/css/app.css b/front/css/app.css index fdc2afbfc..739c47726 100755 --- a/front/css/app.css +++ b/front/css/app.css @@ -1604,6 +1604,26 @@ textarea[readonly], font-size: smaller; } +.main-header .sidebar-toggle +{ + /* .nav-pending-dot below needs a positioned ancestor to anchor to - + .sidebar-toggle has none by default (AdminLTE.css only sets float:left), + so without this it escapes to the nearest positioned element elsewhere + on the page instead of sitting on the toggle icon itself. */ + position: relative; +} + +.nav-pending-dot +{ + position: absolute; + top: 14px; + right: 10px; + width: 8px; + height: 8px; + border-radius: 50%; + display: inline-block; +} + .drag { cursor: move; /* fallback if grab cursor is unsupported */ diff --git a/front/js/common.js b/front/js/common.js index b6d763c7e..0e6fa49c1 100755 --- a/front/js/common.js +++ b/front/js/common.js @@ -902,6 +902,41 @@ function isRandomMAC(mac) // getDevDataByMac, cacheDevices, devicesListAll_JSON moved to cache.js +// ----------------------------------------------------------------------------- +/** + * Returns true if the backend hasn't yet confirmed importing settings as + * recent as referenceTimeMs (appState.settingsImported, from app_state.json). + * No fixed timeout: server/__main__.py's main loop only calls importConfigs() + * at the top of each iteration, and a full scan cycle (every plugin, + * potentially tens of thousands of objects) can legitimately take minutes, + * so this stays pending for exactly as long as the backend actually takes. + * Used by settings.php's own handleLoadingDialog(), passing the config + * file's mtime*1000 (via PHP's filemtime()) as referenceTimeMs - that page's + * own full-page blocking spinner, unrelated to the settingsPendingReload + * nav indicator (handle_pending_settings.js / sse_manager.js), which doesn't + * need a reference time at all since its resolution is pushed via SSE. + * @param {object} appState - parsed app_state.json. + * @param {number} referenceTimeMs - a moment (ms since epoch) that should + * already be reflected in settingsImported if the backend has caught up. + * @returns {boolean} + */ +function isSettingsPending(appState, referenceTimeMs) { + var importedMs = parseInt(appState["settingsImported"] * 1000, 10); + return referenceTimeMs > importedMs; +} + +// ----------------------------------------------------------------------------- +/** + * Shows/hides the sidebar-toggle's attention dot based on whether any + * .info-icon-nav badge in the sidebar is currently visible (not .myhidden) - + * deliberately doesn't know which badge triggered it, so a future badge + * lights this dot up for free without this function needing to change. + */ +function updateNavPendingDot() { + var anyVisible = $('.info-icon-nav').not('.myhidden').length > 0; + $('#navPendingDot').toggleClass('myhidden', !anyVisible); +} + // ----------------------------------------------------------------------------- function isEmpty(value) { diff --git a/front/js/handle_pending_settings.js b/front/js/handle_pending_settings.js new file mode 100644 index 000000000..4ff6d4d8f --- /dev/null +++ b/front/js/handle_pending_settings.js @@ -0,0 +1,14 @@ +//-------------------------------------------------------------- +// Show the "settings still applying" indicator on page load if a save left +// the settingsPendingReload cookie set (front/settings.php's save handler). +// No polling: resolution is pushed via SSE and handled entirely in +// sse_manager.js's handleStateUpdate() (step 4), which clears this same +// cookie the moment appState.settingsImported confirms the import landed. +function settingsPendingUpdateUI() { + var isPending = getCookie("settingsPendingReload") === "true"; + + $('#settingsPendingReload').toggleClass('myhidden', !isPending); + updateNavPendingDot(); +} + +settingsPendingUpdateUI(); diff --git a/front/js/handle_version.js b/front/js/handle_version.js index 01b3eb607..24bda4afa 100755 --- a/front/js/handle_version.js +++ b/front/js/handle_version.js @@ -21,13 +21,15 @@ function versionUpdateUI(){ maintenanceDiv = $('#current-version-text') } - // handling the maintenance section message + // handling the maintenance section message if(emptyArr.includes(maintenanceDiv) == false && $(maintenanceDiv).length != 0) - { + { $(maintenanceDiv).attr("class", $(maintenanceDiv).attr("class").replace("myhidden", "")) - } + } -} + updateNavPendingDot(); + +} //-------------------------------------------------------------- // Checks if a new version is available via the global app_state.json diff --git a/front/js/sse_manager.js b/front/js/sse_manager.js index c8536a2a5..211e8c6a2 100644 --- a/front/js/sse_manager.js +++ b/front/js/sse_manager.js @@ -170,6 +170,14 @@ class NetAlertXStateManager { const importedMs = parseInt(appState["settingsImported"] * 1000); const lastReloaded = parseInt(getCache(CACHE_KEYS.INIT_TIMESTAMP)); if (importedMs > lastReloaded) { + // Clear the settings-pending indicator (cookie + DOM) synchronously, + // before scheduling the reload below - not inside clearCache()'s own + // timeout. Otherwise the freshly-reloaded page would briefly re-read + // the still-present cookie and flash the indicator back on. + setCookie("settingsPendingReload", "", -1); + $('#settingsPendingReload').addClass('myhidden'); + updateNavPendingDot(); + console.log("[NetAlertX State] Settings changed — clearing cache and reloading"); setTimeout(() => clearCache(), 500); } diff --git a/front/php/templates/footer.php b/front/php/templates/footer.php index dc6998772..3161cdea9 100755 --- a/front/php/templates/footer.php +++ b/front/php/templates/footer.php @@ -56,7 +56,8 @@ - + + diff --git a/front/php/templates/header.php b/front/php/templates/header.php index 481764115..8d85a5b65 100755 --- a/front/php/templates/header.php +++ b/front/php/templates/header.php @@ -185,6 +185,10 @@ + + @@ -402,6 +406,10 @@