From 74e25f1275d17dca3c303618de4de34659e171f3 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Fri, 16 Oct 2026 14:37:26 -0400 Subject: [PATCH 1/5] fix: centre the Fleet Dashboard Status column; apply Refresh interval without a page reload - Centre the Status header and pills in the UPSes card; the rule has to outrank .apcupsdDashTable th/td { text-align: left }. - Changing the Refresh dropdown no longer forces a full page reload. The preference is saved, then the running timer is swapped for the new interval (or stopped for No Refresh) in place. Both grids expose their refresh URL as data-refresh-page so this also works when the page was loaded with refresh off. A failed save reverts the dropdown and shows the error on the status line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 + css/apcupsd.css | 7 ++++ includes/dashboard.php | 36 +++++++++++------ js/apcupsd_dashboard.js | 62 ++++++++++++++++++++++++----- tests/Unit/DashboardRefreshTest.php | 42 ++++++++++++++++++- tests/Unit/EstimatedWattsTest.php | 24 +++++++++-- 6 files changed, 146 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9534529..5eac62f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ * feature: Add a per-UPS Timeout (seconds) setting, 5 by default and limited to 1-120, for `apcupsd` and NUT UPS'. NUT connects and conversations are now bounded by twice the timeout in total, and `apcaccess` is run through Cacti's `exec_with_timeout()` so a hung `apcaccess` can no longer stall the poller; a timed out `apcaccess` now records an error on the UPS * feature: Make the UPSes table on the Fleet Dashboard sort like the HMIB dashboards: click (or press Enter/Space on) a column header to sort and click again to reverse, in the card and in its maximize dialog * issue: Right-align the Load, Charge, Time Left, Est. Power and Last Updated columns of the Fleet Dashboard UPSes table +* issue: Centre the Status column of the Fleet Dashboard UPSes table +* issue: Changing the Refresh interval on the dashboards now applies it straight away, restarting or stopping the refresh timer, instead of forcing a full page reload; if the preference cannot be saved the dropdown returns to the interval in force * feature: Share the UPS Dashboard card arrangement across every UPS (graph cards stay per UPS since they belong to each device), hide N/A rows in the UPS Information card, combine Cable and Driver on one row, and add a per-UPS Estimated Power gauge card that reuses the shared nominal power x load % estimate * issue: Show N/A for a slave UPS's (e.g. ONLINE SLAVE) estimated power on the Fleet Dashboard and leave it out of the fleet power total, since its draw is already counted in its master's * feature: Estimate each UPS's power draw as nominal power x load % for UPSes that do not report real wattage, and show it on the Fleet Dashboard (an "Est. Power" tile, a new "Fleet Power" gauge card of total estimated watts against rated capacity, and an "Est. Power" column with a fleet total in the UPSes card) and as a sortable "Est. Power" column in the UPSes list diff --git a/css/apcupsd.css b/css/apcupsd.css index d5794e3..ab44a4d 100644 --- a/css/apcupsd.css +++ b/css/apcupsd.css @@ -530,6 +530,13 @@ white-space: nowrap; } +/* Centred columns, such as the status pill. Must outrank + ".apcupsdDashTable th, .apcupsdDashTable td { text-align: left }". */ +.apcupsdDashTable td.apcupsdDashCenter, +.apcupsdDashTable th.apcupsdDashCenter { + text-align: center; +} + /* Sortable column headers (same behaviour as the HMIB dashboards): click or press Enter/Space to sort; the arrow shows the active direction. */ .apcupsdDashTable th.apcupsdDashSortable { diff --git a/includes/dashboard.php b/includes/dashboard.php index 1ffc20d..81e2600 100644 --- a/includes/dashboard.php +++ b/includes/dashboard.php @@ -1809,8 +1809,9 @@ function apcupsd_dashboard(): void { apcupsd_dashboard_init_timespan(); $current_refresh = (int) read_user_setting('apcupsd_dashboard_refresh', 0); + $refresh_page = $config['url_path'] . 'plugins/apcupsd/upses.php?action=view&id=' . $ups_id . '&header=false'; - apcupsd_dashboard_print_refresh($current_refresh, $config['url_path'] . 'plugins/apcupsd/upses.php?action=view&id=' . $ups_id . '&header=false'); + apcupsd_dashboard_print_refresh($current_refresh, $refresh_page); print ''; @@ -1885,6 +1886,7 @@ function apcupsd_dashboard(): void { print '
'; foreach ($layout['order'] as $key) { @@ -2350,9 +2352,17 @@ function apcupsd_fleet_card_body(string $key): string { // Sortable headers follow the HMIB dashboards: js/apcupsd_dashboard.js sorts // the rows client-side, numeric columns (data-sort="num") by the cell's - // data-sort-value and the rest by their text. - $th = function(string $label, bool $numeric = false, bool $right = false) { - return '' . html_escape($label) . ''; + // data-sort-value and the rest by their text. $align is '', 'right' or 'center'. + $th = function(string $label, bool $numeric = false, string $align = '') { + $classes = ['apcupsdDashSortable']; + + if ($align === 'right') { + $classes[] = 'apcupsdDashNum'; + } elseif ($align === 'center') { + $classes[] = 'apcupsdDashCenter'; + } + + return '' . html_escape($label) . ''; }; $sort_num = function($value): string { @@ -2362,12 +2372,12 @@ function apcupsd_fleet_card_body(string $key): string { print '' . '' . $th(__('UPS', 'apcupsd')) - . $th(__('Status', 'apcupsd')) - . $th(__('Load', 'apcupsd'), true, true) - . $th(__('Charge', 'apcupsd'), true, true) - . $th(__('Time Left', 'apcupsd'), true, true) - . $th(__('Est. Power', 'apcupsd'), true, true) - . $th(__('Last Updated', 'apcupsd'), false, true) + . $th(__('Status', 'apcupsd'), false, 'center') + . $th(__('Load', 'apcupsd'), true, 'right') + . $th(__('Charge', 'apcupsd'), true, 'right') + . $th(__('Time Left', 'apcupsd'), true, 'right') + . $th(__('Est. Power', 'apcupsd'), true, 'right') + . $th(__('Last Updated', 'apcupsd'), false, 'right') . ''; if (cacti_sizeof($upses)) { @@ -2385,7 +2395,7 @@ function apcupsd_fleet_card_body(string $key): string { print '' . '' . '' - . '' + . '' . '' . '' . '' @@ -2603,8 +2613,9 @@ function apcupsd_fleet_dashboard(): void { apcupsd_dashboard_init_timespan(); $current_refresh = (int) read_user_setting('apcupsd_summary_refresh', 0); + $refresh_page = $config['url_path'] . 'plugins/apcupsd/upses.php?action=dashboard&header=false'; - apcupsd_dashboard_print_refresh($current_refresh, $config['url_path'] . 'plugins/apcupsd/upses.php?action=dashboard&header=false'); + apcupsd_dashboard_print_refresh($current_refresh, $refresh_page); print ''; @@ -2662,6 +2673,7 @@ function apcupsd_fleet_dashboard(): void { print '
'; foreach ($layout['order'] as $key) { diff --git a/js/apcupsd_dashboard.js b/js/apcupsd_dashboard.js index 218031e..ebb14a1 100644 --- a/js/apcupsd_dashboard.js +++ b/js/apcupsd_dashboard.js @@ -34,7 +34,8 @@ function apcupsdDashCfg(grid) { refreshAction: grid.dataset.refreshAction || 'dashboard_refresh', scopeParam: grid.dataset.scopeParam || '', scope: grid.dataset.scope || '', - reload: grid.dataset.reload || 'upses.php' + reload: grid.dataset.reload || 'upses.php', + refreshPage: grid.dataset.refreshPage || '' }; } @@ -241,14 +242,29 @@ function initApcupsdDashboard() { }); } - // Changing the interval persists the preference through a CSRF-protected - // POST (never a drive-by GET), then reloads so Cacti's page refresh applies. + // Changing the interval persists the preference through a CSRF-protected POST + // (never a drive-by GET), then applies it straight away without reloading the + // page. If the save fails the dropdown goes back to the interval in force. var interval = document.getElementById('apcupsd_dashboard_refresh'); if (interval) { + interval.dataset.previous = interval.value; $(interval).off('change.apcupsddash').on('change.apcupsddash', function() { + var select = this; var cfg = apcupsdDashCfg(grid); - $.post(cfg.page, apcupsdDashPost(grid, {action: cfg.refreshAction, refresh: this.value}), null, 'json').always(function() { - window.location = cfg.reload; + var revert = function(message) { + select.value = select.dataset.previous; + apcupsdDashShowStatus(grid, message); + }; + $.post(cfg.page, apcupsdDashPost(grid, {action: cfg.refreshAction, refresh: select.value}), null, 'json').done(function(data) { + if (!data || !data.ok) { + revert(data && data.error ? data.error : ''); + return; + } + select.dataset.previous = select.value; + apcupsdDashShowStatus(grid, ''); + apcupsdDashSetRefresh(parseInt(select.value, 10) || 0, cfg.refreshPage); + }).fail(function() { + revert(''); }); }); } @@ -508,6 +524,27 @@ function apcupsdDashStartRefresh() { apcupsdDashTimer = window.setTimeout(apcupsdDashOwnTick, apcupsdDashTimerMs); } +/** Apply a new auto-refresh interval without reloading the page: stop whichever + * timer is running, then start one for the new interval unless it is 0 (off). */ +function apcupsdDashSetRefresh(seconds, page) { + if (typeof myRefresh !== 'undefined') { + clearTimeout(myRefresh); + } + if (apcupsdDashTimer !== null) { + clearTimeout(apcupsdDashTimer); + apcupsdDashTimer = null; + } + + if (seconds <= 0) { + return; + } + + refreshIsLogout = false; + refreshPage = page; + refreshMSeconds = seconds * 1000; + apcupsdDashStartRefresh(); +} + /** Open a copy of a card's body in a large modal dialog. */ function apcupsdDashMaximize(card) { var dialog = document.getElementById('apcupsd_dashboard_dialog'); @@ -562,6 +599,15 @@ function apcupsdDashSyncAddOptions(grid) { }); } +/** Show (or clear, with an empty message) the status line above the grid. */ +function apcupsdDashShowStatus(grid, message) { + var status = grid.previousElementSibling && grid.previousElementSibling.querySelector('.apcupsdDashSaveStatus'); + if (status) { + status.textContent = message; + status.hidden = !status.textContent; + } +} + /** Serialize layout POSTs so rapid actions can't persist out of order: one * request is in flight at a time, and a save requested meanwhile is coalesced * into a single follow-up that reads the latest DOM, so the final state wins. */ @@ -592,11 +638,7 @@ function apcupsdDashSaveLayout(grid) { action: cfg.layoutAction, layout: JSON.stringify({order: order, expanded: expanded}) }), null, 'json').done(function(data) { - var status = grid.previousElementSibling && grid.previousElementSibling.querySelector('.apcupsdDashSaveStatus'); - if (status) { - status.textContent = data && data.error ? data.error : ''; - status.hidden = !status.textContent; - } + apcupsdDashShowStatus(grid, data && data.error ? data.error : ''); }).always(function() { apcupsdDashSaveInFlight = false; if (apcupsdDashSaveQueued) { diff --git a/tests/Unit/DashboardRefreshTest.php b/tests/Unit/DashboardRefreshTest.php index 81f48ca..13c3385 100644 --- a/tests/Unit/DashboardRefreshTest.php +++ b/tests/Unit/DashboardRefreshTest.php @@ -44,8 +44,46 @@ it('arms the timer from both the Fleet and the per-UPS dashboard', function () use ($read) { $source = $read('includes/dashboard.php'); - expect($source)->toContain("apcupsd_dashboard_print_refresh(\$current_refresh, \$config['url_path'] . 'plugins/apcupsd/upses.php?action=dashboard&header=false')"); - expect($source)->toContain("apcupsd_dashboard_print_refresh(\$current_refresh, \$config['url_path'] . 'plugins/apcupsd/upses.php?action=view&id=' . \$ups_id . '&header=false')"); + expect($source)->toContain("\$refresh_page = \$config['url_path'] . 'plugins/apcupsd/upses.php?action=dashboard&header=false';"); + expect($source)->toContain("\$refresh_page = \$config['url_path'] . 'plugins/apcupsd/upses.php?action=view&id=' . \$ups_id . '&header=false';"); + expect(substr_count($source, 'apcupsd_dashboard_print_refresh($current_refresh, $refresh_page);'))->toBe(2); + }); + + it('exposes the refresh URL on both grids so the dropdown can arm the timer', function () use ($read) { + $source = $read('includes/dashboard.php'); + + expect(substr_count($source, "' data-refresh-page=\"' . html_escape(\$refresh_page) . '\"'"))->toBe(2); + expect($read('js/apcupsd_dashboard.js'))->toContain("refreshPage: grid.dataset.refreshPage || ''"); + }); + + it('applies a changed Refresh interval without reloading the page', function () use ($read) { + $js = $read('js/apcupsd_dashboard.js'); + + $start = strpos($js, "document.getElementById('apcupsd_dashboard_refresh')"); + $end = strpos($js, "document.getElementById('apcupsd_dashboard_refresh_now')"); + + expect($start)->toBeGreaterThan(0); + expect($end)->toBeGreaterThan($start); + + $handler = substr($js, $start, $end - $start); + + expect($handler)->not->toContain('window.location'); + expect($handler)->not->toContain('location.reload'); + expect($handler)->toContain('apcupsdDashSetRefresh(parseInt(select.value, 10) || 0, cfg.refreshPage);'); + expect($handler)->toContain('select.value = select.dataset.previous;'); + }); + + it('swaps the running timer for the new interval, or stops it for No Refresh', function () use ($read) { + $js = $read('js/apcupsd_dashboard.js'); + + expect(preg_match('/function apcupsdDashSetRefresh\(seconds, page\) \{\s*if \(typeof myRefresh !== \'undefined\'\) \{\s*clearTimeout\(myRefresh\);\s*\}/', $js))->toBe(1); + expect($js)->toContain('if (seconds <= 0) {'); + expect($js)->toContain('refreshMSeconds = seconds * 1000;'); + expect($js)->toContain('refreshPage = page;'); + + // Stopping must happen before the early return for "off", and starting after it. + expect(strpos($js, 'clearTimeout(myRefresh);', strpos($js, 'function apcupsdDashSetRefresh(')))->toBeLessThan(strpos($js, 'if (seconds <= 0) {')); + expect(strpos($js, 'if (seconds <= 0) {'))->toBeLessThan(strpos($js, 'refreshMSeconds = seconds * 1000;')); }); it('hands the interval to the dashboard script and falls back to the page timer without it', function () use ($read) { diff --git a/tests/Unit/EstimatedWattsTest.php b/tests/Unit/EstimatedWattsTest.php index 27bba70..4fc423e 100644 --- a/tests/Unit/EstimatedWattsTest.php +++ b/tests/Unit/EstimatedWattsTest.php @@ -91,7 +91,7 @@ expect($source)->toContain("\$th(__('{$label}', 'apcupsd')"); } - expect($source)->toContain("class=\"apcupsdDashSortable'"); + expect($source)->toContain("\$classes = ['apcupsdDashSortable'];"); expect($source)->toContain('data-sort="num"'); expect($source)->toContain('data-sort-value="'); expect($source)->not->toContain('apcupsdDashSortBtn'); @@ -125,10 +125,10 @@ $source = $read('includes/dashboard.php'); foreach (['Load', 'Charge', 'Time Left', 'Est. Power'] as $label) { - expect($source)->toContain("\$th(__('{$label}', 'apcupsd'), true, true)"); + expect($source)->toContain("\$th(__('{$label}', 'apcupsd'), true, 'right')"); } - expect($source)->toContain("\$th(__('Last Updated', 'apcupsd'), false, true)"); + expect($source)->toContain("\$th(__('Last Updated', 'apcupsd'), false, 'right')"); expect($source)->toContain("'
+ + "; + + $save_html = " + '; } } else { raise_message(40); From dcc18d5e6c352df5839e6b49a69ccefca034f6d9 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Fri, 16 Oct 2026 14:56:21 -0400 Subject: [PATCH 5/5] fix: address review findings on the Refresh dropdown - Turning Refresh off now puts Cacti's browser auto-logout timer back. The original settings are remembered when the script loads, before the dashboard overrides them, and restored (and re-armed) for No Refresh; a page reload used to do this. - A failed save rolls the dropdown back through its Select2 / selectmenu widget (a namespaced trigger, so the save handler does not run again). - Saves are serialized: one in flight, a choice made meanwhile is sent when it settles, and only a value that was actually saved updates the timer and the rollback state. - Print the dashboard script before the script that starts the refresh, so its functions exist when Cacti runs ajax-loaded scripts in order; before, the first ajax visit fell back to the full-page timer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- includes/dashboard.php | 12 ++- js/apcupsd_dashboard.js | 116 +++++++++++++++++++++++----- tests/Unit/DashboardRefreshTest.php | 76 +++++++++++++++++- 4 files changed, 180 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78c7e5f..4299280 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ * issue: Make the UPS Dashboard bullets and gauges find the Device Package's data sources. They looked for names the package does not use (`batt_volts`, `line_volts`, `output_volts`, `batt_charge`), so Battery Voltage, Line Voltage and Output Voltage never found an RRD even when the device had graphs. The RRD profile's consolidation functions are now checked first, and when an RRD has no MIN or MAX archive (for example one created from an older or customised profile) the window is bounded by its averages instead of leaving the bound blank. A device with no graphs has no RRD files, so it still has no min/avg/max history; the readings simply show the current value * issue: The bullet charts now label the ends of the track with the axis (0 and the full-scale value) and list the current, min, avg and max readings above it, leaving out any that are unknown instead of printing N/A; gauge captions do the same * feature: The Fleet Power gauge now shows min and max ticks and a min / avg / max caption like the other fleet gauges, derived from each UPS's load RRD scaled by its (constant) nominal power -* issue: Changing the Refresh interval on the dashboards now applies it straight away, restarting or stopping the refresh timer, instead of forcing a full page reload; if the preference cannot be saved the dropdown returns to the interval in force +* issue: Changing the Refresh interval on the dashboards now applies it straight away, restarting or stopping the refresh timer, instead of forcing a full page reload; saves are sent one at a time so only a value that was actually saved takes effect, a failed save puts the dropdown (including Cacti's Select2 picker) back to the interval in force, and turning Refresh off puts Cacti's browser auto-logout timer back. The dashboard script is also now loaded before the script that starts the refresh, so the in-place refresh applies the first time the page is opened through Cacti's ajax navigation, not just on later visits * feature: Share the UPS Dashboard card arrangement across every UPS (graph cards stay per UPS since they belong to each device), hide N/A rows in the UPS Information card, combine Cable and Driver on one row, and add a per-UPS Estimated Power gauge card that reuses the shared nominal power x load % estimate * issue: Show N/A for a slave UPS's (e.g. ONLINE SLAVE) estimated power on the Fleet Dashboard and leave it out of the fleet power total, since its draw is already counted in its master's * feature: Estimate each UPS's power draw as nominal power x load % for UPSes that do not report real wattage, and show it on the Fleet Dashboard (an "Est. Power" tile, a new "Fleet Power" gauge card of total estimated watts against rated capacity, and an "Est. Power" column with a fleet total in the UPSes card) and as a sortable "Est. Power" column in the UPSes list diff --git a/includes/dashboard.php b/includes/dashboard.php index fe185ad..c145131 100644 --- a/includes/dashboard.php +++ b/includes/dashboard.php @@ -1930,10 +1930,12 @@ function apcupsd_dashboard(): void { $current_refresh = (int) read_user_setting('apcupsd_dashboard_refresh', 0); $refresh_page = $config['url_path'] . 'plugins/apcupsd/upses.php?action=view&id=' . $ups_id . '&header=false'; - apcupsd_dashboard_print_refresh($current_refresh, $refresh_page); - + // The dashboard script goes first: when Cacti loads this page through ajax the + // scripts run in order, and the refresh script below needs its functions. print ''; + apcupsd_dashboard_print_refresh($current_refresh, $refresh_page); + $ups = apcupsd_dashboard_ups($ups_id); $meta = apcupsd_dashboard_card_meta($ups_id); $layout = apcupsd_dashboard_layout($ups_id); @@ -2808,10 +2810,12 @@ function apcupsd_fleet_dashboard(): void { $current_refresh = (int) read_user_setting('apcupsd_summary_refresh', 0); $refresh_page = $config['url_path'] . 'plugins/apcupsd/upses.php?action=dashboard&header=false'; - apcupsd_dashboard_print_refresh($current_refresh, $refresh_page); - + // The dashboard script goes first: when Cacti loads this page through ajax the + // scripts run in order, and the refresh script below needs its functions. print ''; + apcupsd_dashboard_print_refresh($current_refresh, $refresh_page); + $meta = apcupsd_fleet_card_meta(); $layout = apcupsd_fleet_layout(); $absent = array_values(array_diff(apcupsd_fleet_available_cards(), $layout['order'])); diff --git a/js/apcupsd_dashboard.js b/js/apcupsd_dashboard.js index ebb14a1..40cc3e3 100644 --- a/js/apcupsd_dashboard.js +++ b/js/apcupsd_dashboard.js @@ -24,6 +24,16 @@ var apcupsdDashDragging = null; +/** Cacti's browser auto-logout timer as it was when this script loaded, before the + * dashboard replaces it with its own refresh. Choosing "No Refresh" puts it back (a + * page reload used to). It is kept across re-runs of this script, which can find the + * dashboard's own override in place of the original. */ +var apcupsdDashCoreTimer = typeof apcupsdDashCoreTimer === 'undefined' ? null : apcupsdDashCoreTimer; + +if (typeof refreshIsLogout !== 'undefined' && refreshIsLogout === true && typeof refreshMSeconds !== 'undefined' && typeof refreshPage !== 'undefined') { + apcupsdDashCoreTimer = {ms: refreshMSeconds, page: refreshPage}; +} + /** The endpoint/scope configuration for a grid, read from its data attributes so * the per-UPS and fleet dashboards can share this engine. */ function apcupsdDashCfg(grid) { @@ -249,23 +259,7 @@ function initApcupsdDashboard() { if (interval) { interval.dataset.previous = interval.value; $(interval).off('change.apcupsddash').on('change.apcupsddash', function() { - var select = this; - var cfg = apcupsdDashCfg(grid); - var revert = function(message) { - select.value = select.dataset.previous; - apcupsdDashShowStatus(grid, message); - }; - $.post(cfg.page, apcupsdDashPost(grid, {action: cfg.refreshAction, refresh: select.value}), null, 'json').done(function(data) { - if (!data || !data.ok) { - revert(data && data.error ? data.error : ''); - return; - } - select.dataset.previous = select.value; - apcupsdDashShowStatus(grid, ''); - apcupsdDashSetRefresh(parseInt(select.value, 10) || 0, cfg.refreshPage); - }).fail(function() { - revert(''); - }); + apcupsdDashSaveRefresh(grid, this); }); } @@ -524,8 +518,27 @@ function apcupsdDashStartRefresh() { apcupsdDashTimer = window.setTimeout(apcupsdDashOwnTick, apcupsdDashTimerMs); } +/** Put Cacti's own browser auto-logout timer back, as it was before the dashboard + * replaced it with a refresh. Does nothing when the page had no such timer. */ +function apcupsdDashRestoreCoreTimer() { + if (apcupsdDashCoreTimer === null || typeof setupPageTimeout !== 'function') { + return; + } + + refreshIsLogout = true; + refreshPage = apcupsdDashCoreTimer.page; + refreshMSeconds = apcupsdDashCoreTimer.ms; + + if (typeof refreshFunction !== 'undefined') { + refreshFunction = ''; + } + + setupPageTimeout(); +} + /** Apply a new auto-refresh interval without reloading the page: stop whichever - * timer is running, then start one for the new interval unless it is 0 (off). */ + * timer is running, then start one for the new interval, or for 0 (off) hand the + * page back to Cacti's own timer. */ function apcupsdDashSetRefresh(seconds, page) { if (typeof myRefresh !== 'undefined') { clearTimeout(myRefresh); @@ -536,6 +549,7 @@ function apcupsdDashSetRefresh(seconds, page) { } if (seconds <= 0) { + apcupsdDashRestoreCoreTimer(); return; } @@ -545,6 +559,72 @@ function apcupsdDashSetRefresh(seconds, page) { apcupsdDashStartRefresh(); } +/** Set a dropdown's value and bring the widget Cacti draws over it (Select2 on + * 1.2.x, a jQuery UI selectmenu on 1.3) back in line, without running the change + * handlers that save a preference. */ +function apcupsdDashSetSelect(select, value) { + var $select = $(select); + + select.value = value; + + if ($select.data('select2')) { + $select.trigger('change.select2'); + } + + if ($select.data('ui-selectmenu')) { + $select.selectmenu('refresh'); + } +} + +var apcupsdDashRefreshSaving = false; +var apcupsdDashRefreshPending = false; + +/** Save the Refresh dropdown's choice, then apply it. One save is in flight at a + * time: a choice made meanwhile is sent once it settles, so saves cannot finish + * out of order and only a value that was actually saved updates the timer or + * becomes the one a failed save rolls back to. */ +function apcupsdDashSaveRefresh(grid, select) { + if (apcupsdDashRefreshSaving) { + apcupsdDashRefreshPending = true; + return; + } + + apcupsdDashRefreshSaving = true; + + var cfg = apcupsdDashCfg(grid); + var value = select.value; + var saved = false; + var message = ''; + + $.post(cfg.page, apcupsdDashPost(grid, {action: cfg.refreshAction, refresh: value}), null, 'json').done(function(data) { + if (data && data.ok) { + saved = true; + select.dataset.previous = value; + apcupsdDashSetRefresh(parseInt(value, 10) || 0, cfg.refreshPage); + } else if (data && data.error) { + message = data.error; + } + }).always(function() { + apcupsdDashRefreshSaving = false; + + if (apcupsdDashRefreshPending) { + apcupsdDashRefreshPending = false; + + if (select.value !== select.dataset.previous) { + apcupsdDashSaveRefresh(grid, select); + return; + } + } + + if (saved) { + apcupsdDashShowStatus(grid, ''); + } else { + apcupsdDashSetSelect(select, select.dataset.previous); + apcupsdDashShowStatus(grid, message); + } + }); +} + /** Open a copy of a card's body in a large modal dialog. */ function apcupsdDashMaximize(card) { var dialog = document.getElementById('apcupsd_dashboard_dialog'); diff --git a/tests/Unit/DashboardRefreshTest.php b/tests/Unit/DashboardRefreshTest.php index 13c3385..949cfb3 100644 --- a/tests/Unit/DashboardRefreshTest.php +++ b/tests/Unit/DashboardRefreshTest.php @@ -67,10 +67,80 @@ $handler = substr($js, $start, $end - $start); + expect($handler)->toContain('apcupsdDashSaveRefresh(grid, this);'); expect($handler)->not->toContain('window.location'); - expect($handler)->not->toContain('location.reload'); - expect($handler)->toContain('apcupsdDashSetRefresh(parseInt(select.value, 10) || 0, cfg.refreshPage);'); - expect($handler)->toContain('select.value = select.dataset.previous;'); + + $save = substr($js, (int) strpos($js, 'function apcupsdDashSaveRefresh(')); + + expect($save)->not->toContain('window.location'); + expect($save)->not->toContain('location.reload'); + expect($save)->toContain('apcupsdDashSetRefresh(parseInt(value, 10) || 0, cfg.refreshPage);'); + }); + + it('applies only the interval that was saved, using the value that was submitted', function () use ($read) { + $js = $read('js/apcupsd_dashboard.js'); + + expect($js)->toContain('var value = select.value;'); + expect($js)->toContain('refresh: value}'); + expect($js)->toContain('select.dataset.previous = value;'); + expect($js)->not->toContain('select.dataset.previous = select.value;'); + }); + + it('keeps one save in flight and sends a choice made meanwhile after it settles', function () use ($read) { + $js = $read('js/apcupsd_dashboard.js'); + + expect($js)->toContain('var apcupsdDashRefreshSaving = false;'); + expect($js)->toContain('apcupsdDashRefreshPending = true;'); + expect($js)->toContain('if (select.value !== select.dataset.previous) {'); + expect($js)->toContain('apcupsdDashSaveRefresh(grid, select);'); + }); + + it('rolls the picker back through its widget without running the save handler', function () use ($read) { + $js = $read('js/apcupsd_dashboard.js'); + + expect($js)->toContain('function apcupsdDashSetSelect(select, value) {'); + expect($js)->toContain("\$select.trigger('change.select2');"); + expect($js)->toContain("\$select.selectmenu('refresh');"); + expect($js)->toContain('apcupsdDashSetSelect(select, select.dataset.previous);'); + + // A namespaced trigger, so the 'change.apcupsddash' save handler does not run. + expect($js)->not->toContain("trigger('change')"); + }); + + it('puts Cacti\'s browser logout timer back when Refresh is turned off', function () use ($read) { + $js = $read('js/apcupsd_dashboard.js'); + + // Remembered when the script loads, before the dashboard overrides it. + expect($js)->toContain('var apcupsdDashCoreTimer = typeof apcupsdDashCoreTimer === \'undefined\' ? null : apcupsdDashCoreTimer;'); + expect($js)->toContain('refreshIsLogout === true'); + expect(strpos($js, 'apcupsdDashCoreTimer = {ms: refreshMSeconds, page: refreshPage};'))->toBeLessThan(strpos($js, 'function apcupsdDashCfg(')); + + expect($js)->toContain('function apcupsdDashRestoreCoreTimer() {'); + expect($js)->toContain('refreshIsLogout = true;'); + expect($js)->toContain('refreshPage = apcupsdDashCoreTimer.page;'); + expect($js)->toContain('refreshMSeconds = apcupsdDashCoreTimer.ms;'); + + // "Off" restores it, and never when there was no such timer to restore. + expect(preg_match('/if \(seconds <= 0\) \{\s*apcupsdDashRestoreCoreTimer\(\);\s*return;\s*\}/', $js))->toBe(1); + expect($js)->toContain('if (apcupsdDashCoreTimer === null || typeof setupPageTimeout !== \'function\') {'); + }); + + it('loads the dashboard script before the script that starts the refresh', function () use ($read) { + $source = $read('includes/dashboard.php'); + + // Cacti runs ajax-loaded scripts in order, so the functions must be defined first. + expect(substr_count($source, 'plugins/apcupsd/js/apcupsd_dashboard.js?v='))->toBe(2); + + $offset = 0; + + for ($i = 0; $i < 2; $i++) { + $script = strpos($source, 'plugins/apcupsd/js/apcupsd_dashboard.js?v=', $offset); + $refresh = strpos($source, 'apcupsd_dashboard_print_refresh($current_refresh, $refresh_page);', $offset); + + expect($script)->toBeLessThan($refresh); + + $offset = $refresh + 1; + } }); it('swaps the running timer for the new interval, or stops it for No Refresh', function () use ($read) {
' . html_escape($ups['name']) . '' . apcupsd_dashboard_status_pill((string) ($ups['ups_status'] ?? '')) . '' . apcupsd_dashboard_status_pill((string) ($ups['ups_status'] ?? '')) . '' . html_escape($load) . '' . html_escape($charge) . '' . html_escape($time) . '' . html_escape((string) (\$ups['last_updated']"); $css = $read('css/apcupsd.css'); @@ -138,6 +138,24 @@ expect($css)->toContain('.apcupsdDashTable th.apcupsdDashNum {'); }); + it('centres the Status column of the UPSes card, header and cells', function () use ($read) { + $source = $read('includes/dashboard.php'); + + expect($source)->toContain("\$th(__('Status', 'apcupsd'), false, 'center')"); + expect($source)->toContain("elseif (\$align === 'center') {"); + expect($source)->toContain("'' . apcupsd_dashboard_status_pill("); + + // No other column is centred by accident. + expect(substr_count($source, ", 'center')"))->toBe(1); + + $css = $read('css/apcupsd.css'); + + // Must outrank ".apcupsdDashTable th, .apcupsdDashTable td { text-align: left }". + expect($css)->toContain('.apcupsdDashTable td.apcupsdDashCenter,'); + expect($css)->toContain('.apcupsdDashTable th.apcupsdDashCenter {'); + expect(preg_match('/\.apcupsdDashTable th\.apcupsdDashCenter \{\s*text-align: center;\s*\}/', $css))->toBe(1); + }); + it('adds a sortable estimated power column to the UPS list', function () use ($read) { $source = $read('upses.php'); From 8fbbcfbd6daaf07f51a8504aee174b9eb765d030 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Fri, 16 Oct 2026 14:48:46 -0400 Subject: [PATCH 2/5] fix: UPS Dashboard min/avg/max from the RRDs; Fleet Power range; bullet and gauge text - Match the Device Package's real data source names (batt_volts, line_volts, output_volts, batt_charge) so Battery, Line and Output Voltage find an RRD. - Read the profile's consolidation functions first and, without a MIN or MAX archive (many RRDs have no MIN), bound the window by its averages instead of leaving the lower bound blank. One shared apcupsd_dashboard_rrd_stats() now serves the UPS bullets, UPS gauges and the fleet aggregates. - Bullets label the track ends with the axis (0 .. scale) and list the known now/min/avg/max readings; unknown ones are left out instead of N/A. Gauge captions do the same. - Fleet Power gets min/max ticks and a min/avg/max caption from each UPS's load RRD scaled by its constant nominal power; UPSes without history count at their latest estimate so the range matches the total shown. - Debug log lines say why a reading has no RRD history. - NutWiringTest requires nut.php itself instead of relying on test order. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 3 + includes/dashboard.php | 347 +++++++++++++++++++++------ locales/po/cacti.pot | 20 +- tests/Unit/DashboardRrdStatsTest.php | 317 ++++++++++++++++++++++++ tests/Unit/NutWiringTest.php | 1 + 5 files changed, 601 insertions(+), 87 deletions(-) create mode 100644 tests/Unit/DashboardRrdStatsTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 5eac62f..3c5d795 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ * feature: Make the UPSes table on the Fleet Dashboard sort like the HMIB dashboards: click (or press Enter/Space on) a column header to sort and click again to reverse, in the card and in its maximize dialog * issue: Right-align the Load, Charge, Time Left, Est. Power and Last Updated columns of the Fleet Dashboard UPSes table * issue: Centre the Status column of the Fleet Dashboard UPSes table +* issue: Fix the UPS Dashboard bullets and gauges missing their min/avg/max. They looked for data source names the Device Package does not use (`batt_volts`, `line_volts`, `output_volts`, `batt_charge`), so Battery Voltage, Line Voltage and Output Voltage never found an RRD; and the minimum was fetched with a MIN consolidation that RRDs created from many profiles do not keep, leaving the lower bound blank. The profile is now checked first, and without a MIN or MAX archive the window is bounded by its averages +* issue: The bullet charts now label the ends of the track with the axis (0 and the full-scale value) and list the current, min, avg and max readings above it, leaving out any that are unknown instead of printing N/A; gauge captions do the same +* feature: The Fleet Power gauge now shows min and max ticks and a min / avg / max caption like the other fleet gauges, derived from each UPS's load RRD scaled by its (constant) nominal power * issue: Changing the Refresh interval on the dashboards now applies it straight away, restarting or stopping the refresh timer, instead of forcing a full page reload; if the preference cannot be saved the dropdown returns to the interval in force * feature: Share the UPS Dashboard card arrangement across every UPS (graph cards stay per UPS since they belong to each device), hide N/A rows in the UPS Information card, combine Cable and Driver on one row, and add a per-UPS Estimated Power gauge card that reuses the shared nominal power x load % estimate * issue: Show N/A for a slave UPS's (e.g. ONLINE SLAVE) estimated power on the Fleet Dashboard and leave it out of the fleet power total, since its draw is already counted in its master's diff --git a/includes/dashboard.php b/includes/dashboard.php index 81e2600..fe185ad 100644 --- a/includes/dashboard.php +++ b/includes/dashboard.php @@ -214,7 +214,7 @@ function apcupsd_dashboard_metrics(): array { 'col' => 'ups_line_voltage', 'unit' => ' V', 'group' => 'power', - 'aliases' => ['linev', 'line_voltage', 'voltage', 'inputv'], + 'aliases' => ['line_volts', 'linev', 'line_voltage', 'voltage', 'inputv'], 'scale' => 150.0, 'nominal' => 'ups_nominal_voltage', 'pct' => false, @@ -224,7 +224,7 @@ function apcupsd_dashboard_metrics(): array { 'col' => 'ups_output_voltage', 'unit' => ' V', 'group' => 'power', - 'aliases' => ['outputv', 'output_voltage', 'outv'], + 'aliases' => ['output_volts', 'outputv', 'output_voltage', 'outv'], 'scale' => 150.0, 'nominal' => 'ups_nominal_output_voltage', 'pct' => false, @@ -254,7 +254,7 @@ function apcupsd_dashboard_metrics(): array { 'col' => 'ups_battery_charge', 'unit' => '%', 'group' => 'battery', - 'aliases' => ['bcharge', 'battery_charge', 'charge'], + 'aliases' => ['batt_charge', 'bcharge', 'battery_charge', 'charge'], 'scale' => 100.0, 'nominal' => '', 'pct' => true, @@ -264,7 +264,7 @@ function apcupsd_dashboard_metrics(): array { 'col' => 'ups_battery_voltage', 'unit' => ' V', 'group' => 'battery', - 'aliases' => ['battv', 'battery_voltage', 'batteryv'], + 'aliases' => ['batt_volts', 'battv', 'battery_voltage', 'batteryv'], 'scale' => 30.0, 'nominal' => 'ups_nominal_batt_voltage', 'pct' => false, @@ -379,23 +379,61 @@ function apcupsd_dashboard_find_data_source($host_id, array $aliases): array { } } -if (!function_exists('apcupsd_dashboard_rrd_value')) { +if (!function_exists('apcupsd_dashboard_rrd_consolidations')) { /** - * Fetch a single consolidated value for a data source over a window from its - * RRD file. The requested consolidation function is applied across the whole - * series (min of the MINs, max of the MAXs, etc.). + * The consolidation functions a data source's RRD keeps, from its data source + * profile. Cacti's current default profiles keep AVERAGE, MIN, MAX and LAST, + * but RRDs created from older or customised profiles often have no MIN + * archive, and fetching a consolidation the file lacks only returns an error. + * + * @param int $local_data_id The data source to look up. + * + * @return string[] Any of 'AVERAGE', 'MIN', 'MAX', 'LAST'; AVERAGE and MAX when the profile cannot be read. + */ + function apcupsd_dashboard_rrd_consolidations($local_data_id): array { + static $cache = []; + + $local_data_id = (int) $local_data_id; + + if (!array_key_exists($local_data_id, $cache)) { + $rows = db_fetch_assoc_prepared('SELECT dspc.consolidation_function_id AS cf + FROM data_template_data AS dtd + INNER JOIN data_source_profiles_cf AS dspc ON dspc.data_source_profile_id = dtd.data_source_profile_id + WHERE dtd.local_data_id = ?', + [$local_data_id]); + + $names = [1 => 'AVERAGE', 2 => 'MIN', 3 => 'MAX', 4 => 'LAST']; + $cfs = []; + + foreach (is_array($rows) ? $rows : [] as $row) { + if (isset($names[(int) $row['cf']])) { + $cfs[] = $names[(int) $row['cf']]; + } + } + + $cache[$local_data_id] = cacti_sizeof($cfs) ? $cfs : ['AVERAGE', 'MAX']; + } + + return $cache[$local_data_id]; + } +} + +if (!function_exists('apcupsd_dashboard_rrd_series')) { + /** + * Fetch the finite values of one data source over a window from its RRD file, + * oldest first, for one consolidation function. * * @param int $local_data_id The data source to read. * @param string $ds_name The RRD data-source (column) name. - * @param int $start The window start (relative seconds, negative). - * @param int $end The window end (relative seconds, <= 0). - * @param string $cf 'MIN', 'MAX', 'AVERAGE' or 'LAST'. + * @param int $start The window start (absolute, or relative seconds when negative). + * @param int $end The window end (absolute, or relative seconds when <= 0). + * @param string $cf 'AVERAGE', 'MIN', 'MAX' or 'LAST'. * - * @return float|null The reduced value, or null when no finite data exists. + * @return float[] The values, or [] when the RRD is missing, empty or has no such column. */ - function apcupsd_dashboard_rrd_value($local_data_id, string $ds_name, $start, $end, string $cf): ?float { + function apcupsd_dashboard_rrd_series($local_data_id, string $ds_name, $start, $end, string $cf): array { if ((int) $local_data_id <= 0) { - return null; + return []; } // Resolve the RRD path explicitly: on Cacti develop the sixth parameter of @@ -404,19 +442,19 @@ function apcupsd_dashboard_rrd_value($local_data_id, string $ds_name, $start, $e $rrd_path = get_data_source_path((int) $local_data_id, true); if (empty($rrd_path)) { - return null; + return []; } $fetch = rrdtool_function_fetch((int) $local_data_id, $start, $end, 0, false, $rrd_path, $cf); if (!is_array($fetch) || empty($fetch['data_source_names']) || empty($fetch['values'])) { - return null; + return []; } $index = array_search($ds_name, $fetch['data_source_names'], true); if ($index === false || !isset($fetch['values'][$index])) { - return null; + return []; } $numbers = []; @@ -427,28 +465,81 @@ function apcupsd_dashboard_rrd_value($local_data_id, string $ds_name, $start, $e } } - if (!cacti_sizeof($numbers)) { - return null; + return $numbers; + } +} + +if (!function_exists('apcupsd_dashboard_rrd_stats')) { + /** + * The min, average, max and most recent value of a data source over a window. + * The averages are always read. MIN and MAX archives are used when the RRD + * keeps them; without one (RRDs often have no MIN archive) the window is + * bounded by its averages instead, so the lower and upper bounds are not lost. + * Memoized per request. + * + * @param int $local_data_id The data source to read. + * @param string $ds_name The RRD data-source (column) name. + * @param int $start The window start. + * @param int $end The window end. + * + * @return array{min: float|null, avg: float|null, max: float|null, last: float|null} All null when the RRD has no data in the window. + */ + function apcupsd_dashboard_rrd_stats($local_data_id, string $ds_name, $start, $end): array { + static $cache = []; + + $key = (int) $local_data_id . ':' . $ds_name . ':' . $start . ':' . $end; + + if (array_key_exists($key, $cache)) { + return $cache[$key]; } - switch ($cf) { - case 'MIN': - return min($numbers); - case 'MAX': - return max($numbers); - case 'LAST': - return (float) end($numbers); - default: - return array_sum($numbers) / count($numbers); + $average = apcupsd_dashboard_rrd_series($local_data_id, $ds_name, $start, $end, 'AVERAGE'); + $cfs = apcupsd_dashboard_rrd_consolidations($local_data_id); + $mins = in_array('MIN', $cfs, true) ? apcupsd_dashboard_rrd_series($local_data_id, $ds_name, $start, $end, 'MIN') : []; + $maxs = in_array('MAX', $cfs, true) ? apcupsd_dashboard_rrd_series($local_data_id, $ds_name, $start, $end, 'MAX') : []; + + if (!cacti_sizeof($average) && !cacti_sizeof($mins) && !cacti_sizeof($maxs)) { + apcupsd_dashboard_debug(sprintf('no RRD data for data source %d (%s) between %s and %s', (int) $local_data_id, $ds_name, $start, $end)); + + return $cache[$key] = ['min' => null, 'avg' => null, 'max' => null, 'last' => null]; } + + // The average and latest value only exist when the AVERAGE archive has data; + // the bounds fall back to the averages when a MIN or MAX archive is missing. + $lows = cacti_sizeof($mins) ? $mins : $average; + $highs = cacti_sizeof($maxs) ? $maxs : $average; + + return $cache[$key] = [ + 'min' => cacti_sizeof($lows) ? min($lows) : null, + 'avg' => cacti_sizeof($average) ? array_sum($average) / count($average) : null, + 'max' => cacti_sizeof($highs) ? max($highs) : null, + 'last' => cacti_sizeof($average) ? (float) end($average) : null, + ]; + } +} + +if (!function_exists('apcupsd_dashboard_debug')) { + /** + * Write a line to the Cacti log at debug verbosity, to help work out why a + * dashboard reading has no RRD history (no matching data source, no data in + * the window, ...). + * + * @param string $message The message. + * + * @return void + */ + function apcupsd_dashboard_debug(string $message): void { + cacti_log('DEBUG: ' . $message, false, 'APCUPSD', defined('POLLER_VERBOSITY_DEBUG') ? POLLER_VERBOSITY_DEBUG : 5); } } if (!function_exists('apcupsd_dashboard_bullet')) { /** * Render a min/avg/max bullet chart: a 0..scale track, a translucent band - * from min to max, and a strong marker at the current value. The average and - * min/max endpoints are labelled. + * from min to max, and a strong marker at the current value. The track ends + * are labelled with the axis (0 and the full-scale value); the current, min, + * average and max readings are written above it, leaving out any that are + * unknown rather than printing N/A. * * @param string $label The reading label. * @param float|null $current The current value (marker), or null. @@ -462,26 +553,24 @@ function apcupsd_dashboard_rrd_value($local_data_id, string $ds_name, $start, $e * @return string The bullet HTML. */ function apcupsd_dashboard_bullet(string $label, ?float $current, ?float $min, ?float $avg, ?float $max, float $scale, string $unit, string $severity): string { - $fmt = function(?float $value) use ($unit): string { - if ($value === null) { - return __('N/A', 'apcupsd'); - } - - $decimals = (abs($value) >= 100 || floor($value) == $value) ? 0 : 1; - - return number_format_i18n($value, $decimals) . $unit; - }; - $marker_value = $current !== null ? $current : $avg; $summary = []; if ($current !== null) { - $summary[] = __('now %s', $fmt($current), 'apcupsd'); + $summary[] = sprintf(__('now %s', 'apcupsd'), apcupsd_dashboard_format_value($current, $unit)); + } + + if ($min !== null) { + $summary[] = sprintf(__('min %s', 'apcupsd'), apcupsd_dashboard_format_value($min, $unit)); } if ($avg !== null) { - $summary[] = __('avg %s', $fmt($avg), 'apcupsd'); + $summary[] = sprintf(__('avg %s', 'apcupsd'), apcupsd_dashboard_format_value($avg, $unit)); + } + + if ($max !== null) { + $summary[] = sprintf(__('max %s', 'apcupsd'), apcupsd_dashboard_format_value($max, $unit)); } $head = '
' @@ -518,7 +607,7 @@ function apcupsd_dashboard_bullet(string $label, ?float $current, ?float $min, ? . $avg_marker . '
' . '
' - . '
' . html_escape($fmt($min)) . '' . html_escape($fmt($max)) . '
' + . '
' . html_escape(apcupsd_dashboard_format_value(0.0, $unit)) . '' . html_escape(apcupsd_dashboard_format_value($scale, $unit)) . '
' . ''; } } @@ -617,12 +706,14 @@ function apcupsd_dashboard_render_bullets(string $group, $ups_id): string { $min = $avg = $max = null; if ($local_data_id > 0) { - $min = apcupsd_dashboard_rrd_value($local_data_id, $ds_name, $start, $end, 'MIN'); - $avg = apcupsd_dashboard_rrd_value($local_data_id, $ds_name, $start, $end, 'AVERAGE'); - $max = apcupsd_dashboard_rrd_value($local_data_id, $ds_name, $start, $end, 'MAX'); + $stats = apcupsd_dashboard_rrd_stats($local_data_id, $ds_name, $start, $end); + + $min = $stats['min']; + $avg = $stats['avg']; + $max = $stats['max']; if ($current === null) { - $current = apcupsd_dashboard_rrd_value($local_data_id, $ds_name, $start, $end, 'LAST'); + $current = $stats['last']; } } @@ -742,13 +833,17 @@ function apcupsd_dashboard_metric_reading(string $key, $ups_id): ?array { $min = $avg = $max = null; if ($local_data_id > 0) { - $min = apcupsd_dashboard_rrd_value($local_data_id, $ds_name, $start, $end, 'MIN'); - $avg = apcupsd_dashboard_rrd_value($local_data_id, $ds_name, $start, $end, 'AVERAGE'); - $max = apcupsd_dashboard_rrd_value($local_data_id, $ds_name, $start, $end, 'MAX'); + $stats = apcupsd_dashboard_rrd_stats($local_data_id, $ds_name, $start, $end); + + $min = $stats['min']; + $avg = $stats['avg']; + $max = $stats['max']; if ($current === null) { - $current = apcupsd_dashboard_rrd_value($local_data_id, $ds_name, $start, $end, 'LAST'); + $current = $stats['last']; } + } else { + apcupsd_dashboard_debug(sprintf('no data source on device %d matches %s (looked for %s)', $host_id, $key, implode(', ', $def['aliases']))); } $scale = (float) $def['scale']; @@ -782,6 +877,37 @@ function apcupsd_dashboard_metric_reading(string $key, $ups_id): ?array { } } +if (!function_exists('apcupsd_dashboard_stats_caption')) { + /** + * The "min x · avg y · max z" caption under a gauge. Only the readings that + * are known are listed; an unknown one is left out rather than printed as N/A. + * + * @param float|null $min The window minimum, or null. + * @param float|null $avg The window average, or null (also null to leave it out, e.g. when it is the centre value). + * @param float|null $max The window maximum, or null. + * @param string $unit The value unit suffix. + * + * @return string The caption, or '' when nothing is known. + */ + function apcupsd_dashboard_stats_caption(?float $min, ?float $avg, ?float $max, string $unit): string { + $parts = []; + + if ($min !== null) { + $parts[] = sprintf(__('min %s', 'apcupsd'), apcupsd_dashboard_format_value($min, $unit)); + } + + if ($avg !== null) { + $parts[] = sprintf(__('avg %s', 'apcupsd'), apcupsd_dashboard_format_value($avg, $unit)); + } + + if ($max !== null) { + $parts[] = sprintf(__('max %s', 'apcupsd'), apcupsd_dashboard_format_value($max, $unit)); + } + + return implode(' · ', $parts); + } +} + if (!function_exists('apcupsd_dashboard_gauge_from_reading')) { /** * Render a gauge from a resolved reading: the current value swept on the dial @@ -805,14 +931,7 @@ function apcupsd_dashboard_gauge_from_reading(?array $reading): string { $unit = $reading['unit']; $center = apcupsd_dashboard_format_value($value, $unit); - - $sub = ''; - - if ($reading['avg'] !== null) { - $sub = __('min %s · avg %s · max %s', apcupsd_dashboard_format_value($reading['min'], $unit), apcupsd_dashboard_format_value($reading['avg'], $unit), apcupsd_dashboard_format_value($reading['max'], $unit), 'apcupsd'); - } elseif ($reading['min'] !== null || $reading['max'] !== null) { - $sub = __('min %s · max %s', apcupsd_dashboard_format_value($reading['min'], $unit), apcupsd_dashboard_format_value($reading['max'], $unit), 'apcupsd'); - } + $sub = apcupsd_dashboard_stats_caption($reading['min'], $reading['avg'], $reading['max'], $unit); return apcupsd_dashboard_gauge($value, $reading['min'], $reading['max'], $reading['scale'], $reading['severity'], $center, $sub); } @@ -1948,6 +2067,79 @@ function apcupsd_dashboard_tabs(string $current): void { } } +if (!function_exists('apcupsd_fleet_estimable_sql')) { + /** + * The SQL condition (over the `ups` and `stats` aliases) for the UPSes whose + * power is estimated and counted in the fleet totals: enabled, not a slave + * (its draw is already part of its master's) and with a nominal power rating + * and a load reading. + * + * @return string The condition; constant text, safe to embed in a query. + */ + function apcupsd_fleet_estimable_sql(): string { + return "ups.enabled = 'on' AND COALESCE(stats.ups_status, '') NOT LIKE '%SLAVE%' AND stats.ups_nominal_power > 0 AND stats.ups_load_percent >= 0"; + } +} + +if (!function_exists('apcupsd_fleet_power_stats')) { + /** + * The min, average and max of the fleet's estimated power draw over a window. + * Each counted UPS has a load RRD and a nominal power rating that is a + * constant, so its power history is the load history scaled by the rating; the + * fleet figure is the sum across UPSes. A UPS with no load history counts at + * its latest estimate, so the range stays consistent with the total shown. The + * minimum and maximum are the sums of each UPS's own minimum and maximum, so + * they bound the fleet's draw rather than being the exact extremes of the sum. + * + * @param int $start The window start. + * @param int $end The window end. + * + * @return array{min: float|null, avg: float|null, max: float|null} All null when no UPS has load history. + */ + function apcupsd_fleet_power_stats($start, $end): array { + static $cache = []; + + $key = $start . ':' . $end; + + if (array_key_exists($key, $cache)) { + return $cache[$key]; + } + + $rows = db_fetch_assoc('SELECT ups.host_id, stats.ups_nominal_power, stats.ups_load_percent + FROM apcupsd_ups AS ups + INNER JOIN apcupsd_ups_stats AS stats ON stats.ups_id = ups.id + WHERE ' . apcupsd_fleet_estimable_sql()); + + $aliases = apcupsd_dashboard_metrics()['load_percent']['aliases']; + $totals = ['min' => 0.0, 'avg' => 0.0, 'max' => 0.0]; + $with = 0; + + foreach (is_array($rows) ? $rows : [] as $row) { + $nominal = (float) $row['ups_nominal_power']; + $now = (float) apcupsd_estimate_watts($nominal, $row['ups_load_percent']); + $stats = ['min' => null, 'avg' => null, 'max' => null]; + + list($local_data_id, $ds_name) = apcupsd_dashboard_find_data_source((int) $row['host_id'], $aliases); + + if ($local_data_id > 0) { + $stats = apcupsd_dashboard_rrd_stats($local_data_id, $ds_name, $start, $end); + } + + if ($stats['avg'] !== null || $stats['min'] !== null || $stats['max'] !== null) { + $with++; + } + + foreach ($totals as $name => $total) { + $watts = $stats[$name] !== null ? apcupsd_estimate_watts($nominal, $stats[$name]) : null; + + $totals[$name] += $watts !== null ? $watts : $now; + } + } + + return $cache[$key] = $with > 0 ? $totals : ['min' => null, 'avg' => null, 'max' => null]; + } +} + if (!function_exists('apcupsd_fleet_stats')) { /** * Memoized fleet aggregates: UPS counts by status (online/on-battery/other/ @@ -1962,7 +2154,7 @@ function apcupsd_fleet_stats(): array { static $cache = null; if ($cache === null) { - $estimable = "ups.enabled = 'on' AND COALESCE(stats.ups_status, '') NOT LIKE '%SLAVE%' AND stats.ups_nominal_power > 0 AND stats.ups_load_percent >= 0"; + $estimable = apcupsd_fleet_estimable_sql(); $row = db_fetch_row("SELECT COUNT(*) AS total, @@ -2104,20 +2296,18 @@ function apcupsd_fleet_rrd_stats(string $metric_key): array { continue; } - $min = apcupsd_dashboard_rrd_value($local_data_id, $ds_name, $start, $end, 'MIN'); - $avg = apcupsd_dashboard_rrd_value($local_data_id, $ds_name, $start, $end, 'AVERAGE'); - $max = apcupsd_dashboard_rrd_value($local_data_id, $ds_name, $start, $end, 'MAX'); + $stats = apcupsd_dashboard_rrd_stats($local_data_id, $ds_name, $start, $end); - if ($min !== null) { - $mins[] = $min; + if ($stats['min'] !== null) { + $mins[] = $stats['min']; } - if ($avg !== null) { - $avgs[] = $avg; + if ($stats['avg'] !== null) { + $avgs[] = $stats['avg']; } - if ($max !== null) { - $maxs[] = $max; + if ($stats['max'] !== null) { + $maxs[] = $stats['max']; } } } @@ -2294,14 +2484,21 @@ function apcupsd_fleet_card_body(string $key): string { } $est_load = $est_watts / $est_capacity * 100; - $est_sub = __('%s of %s rated', apcupsd_dashboard_format_value($est_load, '%'), apcupsd_dashboard_format_value($est_capacity, ' W'), 'apcupsd'); + $rated = sprintf(__('%s of %s rated', 'apcupsd'), apcupsd_dashboard_format_value($est_load, '%'), apcupsd_dashboard_format_value($est_capacity, ' W')); + + // The range comes from each UPS's load RRD scaled by its (constant) nominal power, + // like the Load gauge; without load history the rating is the caption instead. + apcupsd_dashboard_init_timespan(); + + $range = apcupsd_fleet_power_stats(get_current_graph_start(), get_current_graph_end()); + $sub = apcupsd_dashboard_stats_caption($range['min'], $range['avg'], $range['max'], ' W'); print '
' . '' . __esc('Estimated Power', 'apcupsd') . '' - . apcupsd_dashboard_gauge($est_watts, null, null, $est_capacity, apcupsd_dashboard_metric_severity('load_percent', $est_load), apcupsd_dashboard_format_value($est_watts, ' W'), $est_sub) + . apcupsd_dashboard_gauge($est_watts, $range['min'], $range['max'], $est_capacity, apcupsd_dashboard_metric_severity('load_percent', $est_load), apcupsd_dashboard_format_value($est_watts, ' W'), $sub !== '' ? $sub : $rated) . '
'; - print '

' . __esc('Estimated as nominal power x load %, from the latest poll.', 'apcupsd') . ' ' + print '

' . html_escape($sub !== '' ? $rated . '. ' : '') . __esc('Estimated as nominal power x load %, from the latest poll.', 'apcupsd') . ' ' . html_escape(__('Counted %s of %s enabled UPSes.', number_format_i18n($est_count, 0), number_format_i18n($enabled, 0), 'apcupsd')) . '

'; break; @@ -2326,11 +2523,7 @@ function apcupsd_fleet_card_body(string $key): string { $reading = $gauge['reading']; $center = apcupsd_dashboard_format_value($reading['avg'], $gauge['unit']); - $sub = ''; - - if ($reading['min'] !== null || $reading['max'] !== null) { - $sub = __('min %s · max %s', apcupsd_dashboard_format_value($reading['min'], $gauge['unit']), apcupsd_dashboard_format_value($reading['max'], $gauge['unit']), 'apcupsd'); - } + $sub = apcupsd_dashboard_stats_caption($reading['min'], null, $reading['max'], $gauge['unit']); $severity = apcupsd_dashboard_metric_severity($gauge['metric'], $reading['avg']); diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index 003735d..7a346a5 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -64,6 +64,16 @@ msgstr "" msgid "avg %s" msgstr "" +#: includes/dashboard.php +#, php-format +msgid "min %s" +msgstr "" + +#: includes/dashboard.php +#, php-format +msgid "max %s" +msgstr "" + #: includes/dashboard.php msgid "Average" msgstr "" @@ -981,11 +991,6 @@ msgstr "" msgid "No readings are available for these gauges yet." msgstr "" -#: includes/dashboard.php -#, php-format -msgid "min %s · max %s" -msgstr "" - #: includes/dashboard.php msgid "Nominal Battery Voltage" msgstr "" @@ -1006,11 +1011,6 @@ msgstr "" msgid "Est. Power" msgstr "" -#: includes/dashboard.php -#, php-format -msgid "min %s · avg %s · max %s" -msgstr "" - #: includes/dashboard.php msgid "Estimated Power" msgstr "" diff --git a/tests/Unit/DashboardRrdStatsTest.php b/tests/Unit/DashboardRrdStatsTest.php new file mode 100644 index 0000000..10d707c --- /dev/null +++ b/tests/Unit/DashboardRrdStatsTest.php @@ -0,0 +1,317 @@ + $value) { + $series[1700000000 + $i * 300] = (string) $value; + } + + return ['data_source_names' => [$ds], 'values' => [0 => $series]]; +} + +/** + * Describe a data source's RRD: its path, the consolidations its profile keeps + * and the series each consolidation returns. + * + * @param int $id The local data id. + * @param string $ds The data source name. + * @param array $series Consolidation function => values. + * + * @return void + */ +function apcupsd_test_rrd_setup(int $id, string $ds, array $series): void { + $cf_ids = ['AVERAGE' => 1, 'MIN' => 2, 'MAX' => 3, 'LAST' => 4]; + + $GLOBALS['__rrd_paths'][$id] = "/rra/ds_{$id}.rrd"; + + foreach ($series as $cf => $values) { + $GLOBALS['__rrd_data'][$id][$cf] = apcupsd_test_rrd_fetch($ds, $values); + } + + apcupsd_test_mock_db('db_fetch_assoc_prepared', function ($sql, $params) use ($id) { + return strpos($sql, 'data_source_profiles_cf') !== false && $params === [$id]; + }, array_map(function ($cf) use ($cf_ids) { + return ['cf' => $cf_ids[$cf]]; + }, $GLOBALS['__rrd_profile'][$id] ?? array_keys($series))); +} + +/** + * Register the fleet's estimable UPSes and which host has a load data source. + * + * @param array $upses host_id, nominal watts, load %, load data source id (0 for none) + * + * @return void + */ +function apcupsd_test_fleet(array $upses): void { + $rows = []; + + foreach ($upses as [$host, $nominal, $load, $ds]) { + $rows[] = ['host_id' => $host, 'ups_nominal_power' => $nominal, 'ups_load_percent' => $load]; + + apcupsd_test_mock_db('db_fetch_assoc_prepared', function ($sql, $params) use ($host) { + return strpos($sql, 'FROM data_local') !== false && $params === [$host]; + }, $ds > 0 ? [['local_data_id' => $ds, 'data_source_name' => 'loadpct']] : []); + } + + apcupsd_test_mock_db('db_fetch_assoc', 'FROM apcupsd_ups AS ups', $rows); +} + +beforeEach(function () { + $GLOBALS['__rrd_paths'] = []; + $GLOBALS['__rrd_data'] = []; + $GLOBALS['__rrd_calls'] = []; + $GLOBALS['__rrd_profile'] = []; + + apcupsd_test_reset_db_mocks(); +}); + +describe('data source matching against the Device Package names', function () { + it('finds a data source for every dashboard metric', function () { + $names = ['comonbatt', 'numxfers', 'max_line_volts', 'min_line_volts', 'high_transfer', 'low_transfer', 'internal_temp', + 'frequency', 'batt_charge', 'loadpct', 'timeleft', 'batt_volts', 'line_volts', 'output_volts']; + + $rows = []; + + foreach ($names as $i => $name) { + $rows[] = ['local_data_id' => 100 + $i, 'data_source_name' => $name]; + } + + apcupsd_test_mock_db('db_fetch_assoc_prepared', 'FROM data_local', $rows); + + $expected = [ + 'line_voltage' => 'line_volts', + 'output_voltage' => 'output_volts', + 'line_frequency' => 'frequency', + 'load_percent' => 'loadpct', + 'battery_charge' => 'batt_charge', + 'battery_voltage' => 'batt_volts', + 'timeleft' => 'timeleft', + 'temperature' => 'internal_temp', + ]; + + foreach (apcupsd_dashboard_metrics() as $key => $def) { + [$id, $ds] = apcupsd_dashboard_find_data_source(901, $def['aliases']); + + expect($ds)->toBe($expected[$key], "metric {$key}"); + expect($id)->toBeGreaterThan(0); + } + }); + + it('prefers the exact data source over a longer name that merely contains it', function () { + apcupsd_test_mock_db('db_fetch_assoc_prepared', 'FROM data_local', [ + ['local_data_id' => 1, 'data_source_name' => 'max_line_volts'], + ['local_data_id' => 2, 'data_source_name' => 'line_volts'], + ]); + + expect(apcupsd_dashboard_find_data_source(902, apcupsd_dashboard_metrics()['line_voltage']['aliases']))->toBe([2, 'line_volts']); + }); +}); + +describe('RRD series and stats', function () { + it('reads the finite values of one data source', function () { + apcupsd_test_rrd_setup(11, 'loadpct', ['AVERAGE' => [10, 'nan', 30]]); + + expect(apcupsd_dashboard_rrd_series(11, 'loadpct', -3600, 0, 'AVERAGE'))->toBe([10.0, 30.0]); + }); + + it('returns nothing for a missing RRD, an unknown column or an empty fetch', function () { + expect(apcupsd_dashboard_rrd_series(0, 'loadpct', -3600, 0, 'AVERAGE'))->toBe([]); + expect(apcupsd_dashboard_rrd_series(12, 'loadpct', -3600, 0, 'AVERAGE'))->toBe([]); + + apcupsd_test_rrd_setup(13, 'loadpct', ['AVERAGE' => [5]]); + + expect(apcupsd_dashboard_rrd_series(13, 'other', -3600, 0, 'AVERAGE'))->toBe([]); + expect(apcupsd_dashboard_rrd_series(13, 'loadpct', -3600, 0, 'MAX'))->toBe([]); + }); + + it('uses the MIN and MAX archives when the RRD keeps them', function () { + $GLOBALS['__rrd_profile'][21] = ['AVERAGE', 'MIN', 'MAX', 'LAST']; + apcupsd_test_rrd_setup(21, 'timeleft', ['AVERAGE' => [20, 24, 28], 'MIN' => [2, 9], 'MAX' => [30, 31]]); + + $stats = apcupsd_dashboard_rrd_stats(21, 'timeleft', -3600, 0); + + expect($stats)->toBe(['min' => 2.0, 'avg' => 24.0, 'max' => 31.0, 'last' => 28.0]); + expect($GLOBALS['__rrd_calls'])->toBe([[21, 'AVERAGE'], [21, 'MIN'], [21, 'MAX']]); + }); + + it('bounds the window by its averages when the RRD has no MIN archive, without fetching one', function () { + $GLOBALS['__rrd_profile'][22] = ['AVERAGE', 'MAX']; + apcupsd_test_rrd_setup(22, 'timeleft', ['AVERAGE' => [20, 24, 28], 'MAX' => [30, 31]]); + + $stats = apcupsd_dashboard_rrd_stats(22, 'timeleft', -3600, 0); + + expect($stats['min'])->toBe(20.0); + expect($stats['max'])->toBe(31.0); + expect($GLOBALS['__rrd_calls'])->toBe([[22, 'AVERAGE'], [22, 'MAX']]); + }); + + it('bounds the window by its averages when the RRD has neither MIN nor MAX', function () { + $GLOBALS['__rrd_profile'][23] = ['AVERAGE']; + apcupsd_test_rrd_setup(23, 'batt_volts', ['AVERAGE' => [26.5, 27.0, 27.5]]); + + $stats = apcupsd_dashboard_rrd_stats(23, 'batt_volts', -3600, 0); + + expect($stats)->toBe(['min' => 26.5, 'avg' => 27.0, 'max' => 27.5, 'last' => 27.5]); + }); + + it('assumes AVERAGE and MAX when the profile cannot be read', function () { + apcupsd_test_rrd_setup(24, 'loadpct', ['AVERAGE' => [10, 20], 'MAX' => [40]]); + apcupsd_test_reset_db_mocks(); + + expect(apcupsd_dashboard_rrd_consolidations(24))->toBe(['AVERAGE', 'MAX']); + }); + + it('reports every statistic as unknown when the RRD has no data', function () { + apcupsd_test_rrd_setup(25, 'loadpct', []); + + expect(apcupsd_dashboard_rrd_stats(25, 'loadpct', -3600, 0))->toBe(['min' => null, 'avg' => null, 'max' => null, 'last' => null]); + expect($GLOBALS['__rrd_calls'])->toBe([[25, 'AVERAGE'], [25, 'MAX']]); + }); + + it('still reports the bounds when only the MIN and MAX archives have data', function () { + $GLOBALS['__rrd_profile'][27] = ['AVERAGE', 'MIN', 'MAX']; + apcupsd_test_rrd_setup(27, 'loadpct', ['MIN' => [4, 6], 'MAX' => [30, 25]]); + + expect(apcupsd_dashboard_rrd_stats(27, 'loadpct', -3600, 0))->toBe(['min' => 4.0, 'avg' => null, 'max' => 30.0, 'last' => null]); + }); + + it('reads each window once per request', function () { + $GLOBALS['__rrd_profile'][26] = ['AVERAGE']; + apcupsd_test_rrd_setup(26, 'loadpct', ['AVERAGE' => [10, 20]]); + + apcupsd_dashboard_rrd_stats(26, 'loadpct', -3600, 0); + apcupsd_dashboard_rrd_stats(26, 'loadpct', -3600, 0); + + expect(count($GLOBALS['__rrd_calls']))->toBe(1); + }); +}); + +describe('bullet and gauge text', function () { + it('writes the current, min, avg and max readings above the track', function () { + $html = apcupsd_dashboard_bullet('Battery Voltage', 27.2, 26.1, 27.0, 27.4, 30.0, ' V', ''); + + foreach (['now 27.2 V', 'min 26.1 V', 'avg 27 V', 'max 27.4 V'] as $text) { + expect($html)->toContain($text); + } + }); + + it('labels the track ends with the axis, not with N/A', function () { + $html = apcupsd_dashboard_bullet('Battery Voltage', 27.2, null, null, null, 30.0, ' V', ''); + + expect($html)->toContain('
0 V30 V
'); + expect($html)->not->toContain('N/A'); + }); + + it('leaves out only the readings that are unknown', function () { + $html = apcupsd_dashboard_bullet('Load', 20.0, null, null, 35.0, 100.0, ' W', ''); + + expect($html)->toContain('now 20 W'); + expect($html)->toContain('max 35 W'); + expect($html)->not->toContain('min '); + expect($html)->not->toContain('avg '); + expect($html)->not->toContain('N/A'); + }); + + it('shows time left from zero minutes', function () { + $html = apcupsd_dashboard_bullet('Time Left', 25.0, null, null, 40.0, 52.0, ' min', ''); + + expect($html)->toContain('0 min52 min'); + }); + + it('lists only the known statistics under a gauge', function () { + expect(apcupsd_dashboard_stats_caption(1.0, 2.0, 3.0, '%'))->toBe('min 1% · avg 2% · max 3%'); + expect(apcupsd_dashboard_stats_caption(1.0, null, 3.0, '%'))->toBe('min 1% · max 3%'); + expect(apcupsd_dashboard_stats_caption(null, 2.0, null, '%'))->toBe('avg 2%'); + expect(apcupsd_dashboard_stats_caption(null, null, null, '%'))->toBe(''); + }); + + it('does not print N/A in a gauge when a bound is unknown', function () { + $html = apcupsd_dashboard_gauge_from_reading(['label' => 'Load', 'current' => 12.0, 'min' => null, 'avg' => 14.0, 'max' => 20.0, 'scale' => 100.0, 'unit' => '%', 'severity' => 'up']); + + expect($html)->toContain('avg 14% · max 20%'); + expect($html)->not->toContain('N/A'); + }); +}); + +describe('fleet power range from the load RRDs', function () { + it('scales each UPS\'s load history by its nominal power and sums the fleet', function () { + $GLOBALS['__rrd_profile'][71] = ['AVERAGE', 'MIN', 'MAX']; + apcupsd_test_rrd_setup(71, 'loadpct', ['AVERAGE' => [10, 14], 'MIN' => [8], 'MAX' => [20]]); + + // UPS 1: 1000 W rated with load history. UPS 2: 500 W rated at 40% with no history. + apcupsd_test_fleet([[7, 1000.0, 12.0, 71], [8, 500.0, 40.0, 0]]); + + // UPS 2 counts at its latest estimate (200 W) in every bound, so the range stays consistent. + expect(apcupsd_fleet_power_stats(-3600, 0))->toBe(['min' => 280.0, 'avg' => 320.0, 'max' => 400.0]); + }); + + it('reports no range when no UPS has load history', function () { + apcupsd_test_fleet([[9, 1000.0, 12.0, 0], [10, 500.0, 40.0, 0]]); + + expect(apcupsd_fleet_power_stats(-7200, 0))->toBe(['min' => null, 'avg' => null, 'max' => null]); + }); + + it('only counts the UPSes the fleet total counts', function () { + $source = (string) file_get_contents(__DIR__ . '/../../includes/dashboard.php'); + + expect(substr_count($source, 'apcupsd_fleet_estimable_sql()'))->toBeGreaterThan(2); + expect($source)->toContain('$range = apcupsd_fleet_power_stats(get_current_graph_start(), get_current_graph_end());'); + expect($source)->toContain('apcupsd_dashboard_gauge($est_watts, $range[\'min\'], $range[\'max\'], $est_capacity'); + }); +}); + +describe('RRD reading wiring', function () { + it('reads every dashboard surface through the shared stats function', function () { + $source = (string) file_get_contents(__DIR__ . '/../../includes/dashboard.php'); + + expect($source)->not->toContain('apcupsd_dashboard_rrd_value'); + expect(substr_count($source, 'apcupsd_dashboard_rrd_stats('))->toBeGreaterThan(3); + }); +}); diff --git a/tests/Unit/NutWiringTest.php b/tests/Unit/NutWiringTest.php index 7bb79b1..4c68795 100644 --- a/tests/Unit/NutWiringTest.php +++ b/tests/Unit/NutWiringTest.php @@ -13,6 +13,7 @@ */ require_once __DIR__ . '/../../includes/database.php'; +require_once __DIR__ . '/../../includes/nut.php'; describe('NUT schema', function () { $columns = function (array $data): array { From e6d490290fdf68dbffb2668c818ead593941e14c Mon Sep 17 00:00:00 2001 From: TheWitness Date: Fri, 16 Oct 2026 14:49:23 -0400 Subject: [PATCH 3/5] docs: correct the changelog wording for the RRD data source fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c5d795..239629d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ * feature: Make the UPSes table on the Fleet Dashboard sort like the HMIB dashboards: click (or press Enter/Space on) a column header to sort and click again to reverse, in the card and in its maximize dialog * issue: Right-align the Load, Charge, Time Left, Est. Power and Last Updated columns of the Fleet Dashboard UPSes table * issue: Centre the Status column of the Fleet Dashboard UPSes table -* issue: Fix the UPS Dashboard bullets and gauges missing their min/avg/max. They looked for data source names the Device Package does not use (`batt_volts`, `line_volts`, `output_volts`, `batt_charge`), so Battery Voltage, Line Voltage and Output Voltage never found an RRD; and the minimum was fetched with a MIN consolidation that RRDs created from many profiles do not keep, leaving the lower bound blank. The profile is now checked first, and without a MIN or MAX archive the window is bounded by its averages +* issue: Make the UPS Dashboard bullets and gauges find the Device Package's data sources. They looked for names the package does not use (`batt_volts`, `line_volts`, `output_volts`, `batt_charge`), so Battery Voltage, Line Voltage and Output Voltage never found an RRD even when the device had graphs. The RRD profile's consolidation functions are now checked first, and when an RRD has no MIN or MAX archive (for example one created from an older or customised profile) the window is bounded by its averages instead of leaving the bound blank. A device with no graphs has no RRD files, so it still has no min/avg/max history; the readings simply show the current value * issue: The bullet charts now label the ends of the track with the axis (0 and the full-scale value) and list the current, min, avg and max readings above it, leaving out any that are unknown instead of printing N/A; gauge captions do the same * feature: The Fleet Power gauge now shows min and max ticks and a min / avg / max caption like the other fleet gauges, derived from each UPS's load RRD scaled by its (constant) nominal power * issue: Changing the Refresh interval on the dashboards now applies it straight away, restarting or stopping the refresh timer, instead of forcing a full page reload; if the preference cannot be saved the dropdown returns to the interval in force From aa443e2e856ab649c1eee519ebb8473813deb1f1 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Fri, 16 Oct 2026 14:52:46 -0400 Subject: [PATCH 4/5] feature: Run Graph Automation action on the UPSes list Add a bulk action that runs Cacti's graph automation (the same as the Devices page's Apply Automation Rules) for the Cacti Device linked to each selected UPS, so a device that never got its graphs, and so has no RRD history for the dashboards, can be given them without leaving the plugin. - Offered only to users with the Sites/Devices/Data realm (core's realm for the equivalent device action) and refused server side otherwise. - Skips and reports a UPS that has no Cacti Device yet; the device id is looked up from the UPS id with a prepared query. - Documented in the README and CHANGELOG, with translatable strings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 1 + README.md | 9 +++ locales/po/cacti.pot | 28 ++++++++ tests/Unit/GraphAutomationActionTest.php | 85 ++++++++++++++++++++++++ upses.php | 74 ++++++++++++++++++++- 5 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 tests/Unit/GraphAutomationActionTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 239629d..78c7e5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * feature: Add a per-UPS Timeout (seconds) setting, 5 by default and limited to 1-120, for `apcupsd` and NUT UPS'. NUT connects and conversations are now bounded by twice the timeout in total, and `apcaccess` is run through Cacti's `exec_with_timeout()` so a hung `apcaccess` can no longer stall the poller; a timed out `apcaccess` now records an error on the UPS * feature: Make the UPSes table on the Fleet Dashboard sort like the HMIB dashboards: click (or press Enter/Space on) a column header to sort and click again to reverse, in the card and in its maximize dialog * issue: Right-align the Load, Charge, Time Left, Est. Power and Last Updated columns of the Fleet Dashboard UPSes table +* feature: Add a "Run Graph Automation" action to the UPSes list bulk-action dropdown. It runs Cacti's graph automation (the same as the Devices page's "Apply Automation Rules") for the Cacti Device linked to each selected UPS, creating the graphs, data sources and tree items its templates and automation rules call for, so a device that never got its graphs can be given them without leaving the plugin. Graphs that already exist are left alone, a UPS with no Cacti Device yet is skipped and reported, and the action needs the same realm as Cacti's own device action (Sites/Devices/Data) * issue: Centre the Status column of the Fleet Dashboard UPSes table * issue: Make the UPS Dashboard bullets and gauges find the Device Package's data sources. They looked for names the package does not use (`batt_volts`, `line_volts`, `output_volts`, `batt_charge`), so Battery Voltage, Line Voltage and Output Voltage never found an RRD even when the device had graphs. The RRD profile's consolidation functions are now checked first, and when an RRD has no MIN or MAX archive (for example one created from an older or customised profile) the window is bounded by its averages instead of leaving the bound blank. A device with no graphs has no RRD files, so it still has no min/avg/max history; the readings simply show the current value * issue: The bullet charts now label the ends of the track with the axis (0 and the full-scale value) and list the current, min, avg and max readings above it, leaving out any that are unknown instead of printing N/A; gauge captions do the same diff --git a/README.md b/README.md index ed6c6c5..b7e63a2 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,15 @@ After you have installed the plugin, install the Device Package in the templates folder from Console > Import/Export > Import Package. This Device Template will be used for showing available graphs. +The dashboards read each reading's min, average and max from the RRD files of +the UPS's Cacti Device, so the device needs its graphs. They are created when +the device is first added, but if a device has none (for example the Device +Package was imported afterwards), tick the UPS on the UPSes page and choose +`Run Graph Automation` from the action dropdown. It runs Cacti's graph +automation for the linked device, the same as `Apply Automation Rules` on the +Devices page, and leaves graphs that already exist alone. It needs the +`Sites/Devices/Data` permission. + ## Monitoring UPS' with NUT Support for Network UPS Tools (NUT) is new in version 1.4.0. The plugin talks diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index 7a346a5..f87c895 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -796,6 +796,34 @@ msgid_plural "Click 'Continue' to Reset Discovery for the following UPSes. Note msgstr[0] "" msgstr[1] "" +#: upses.php +msgid "Run Graph Automation" +msgstr "" + +#: upses.php +msgid "(no Cacti Device yet)" +msgstr "" + +#: upses.php +msgid "Click 'Continue' to run Graph Automation for the following UPS. Graphs, data sources and tree items are created from the device's templates and the matching automation rules; graphs that already exist are left as they are. Only a UPS that already has a Cacti Device is affected." +msgid_plural "Click 'Continue' to run Graph Automation for the following UPSes. Graphs, data sources and tree items are created from each device's templates and the matching automation rules; graphs that already exist are left as they are. Only a UPS that already has a Cacti Device is affected." +msgstr[0] "" +msgstr[1] "" + +#: upses.php +msgid "None of the selected UPSes has a Cacti Device yet, so Graph Automation was not run. A device is created on the first successful poll." +msgstr "" + +#: upses.php +#, php-format +msgid "Graph Automation was run for %d UPS(es). %d UPS(es) have no Cacti Device yet and were skipped." +msgstr "" + +#: upses.php +#, php-format +msgid "Graph Automation was run for %d UPS(es)." +msgstr "" + #: upses.php msgid "Reset UPS" msgid_plural "Reset UPSes" diff --git a/tests/Unit/GraphAutomationActionTest.php b/tests/Unit/GraphAutomationActionTest.php new file mode 100644 index 0000000..ef2ce2b --- /dev/null +++ b/tests/Unit/GraphAutomationActionTest.php @@ -0,0 +1,85 @@ +not->toBeFalse("Unable to read {$relative}"); + + return (string) $contents; + }; + + it('loads the Cacti libraries graph automation needs', function () use ($read) { + $page = $read('upses.php'); + + foreach (['api_automation', 'api_device', 'api_tree', 'data_query', 'template'] as $library) { + expect($page)->toContain("require_once(\$config['base_path'] . '/lib/{$library}.php');"); + } + }); + + it('only offers the action to users who may apply automation rules to devices', function () use ($read) { + $page = $read('upses.php'); + + // The same realm core's Devices page uses for "Apply Automation Rules". + expect($page)->toContain('if (is_realm_allowed(3)) {'); + expect($page)->toContain("\$ups_actions[4] = __('Run Graph Automation', 'apcupsd');"); + expect(strpos($page, 'if (is_realm_allowed(3)) {'))->toBeLessThan(strpos($page, "\$ups_actions[4] = ")); + }); + + it('refuses an action that was not offered, including this one without the realm', function () use ($read) { + $page = $read('upses.php'); + + expect($page)->toContain('array_key_exists((int) get_nfilter_request_var(\'drp_action\'), $ups_actions)'); + expect($page)->toContain('ctype_digit((string) get_nfilter_request_var(\'drp_action\'))'); + expect($page)->not->toContain("in_array(get_nfilter_request_var('drp_action'), ['1', '2', '3'], true)"); + }); + + it('runs it from the confirmation form for the selected UPSes', function () use ($read) { + $page = $read('upses.php'); + + expect($page)->toContain("elseif (get_nfilter_request_var('drp_action') == '4') { // Run Graph Automation"); + expect($page)->toContain('run_graph_automation($selected_items);'); + expect($page)->toContain("elseif (get_nfilter_request_var('drp_action') == '4') { // run graph automation"); + }); + + it('runs automation only for a UPS that has a linked Cacti device and reports the rest', function () use ($read) { + $page = $read('upses.php'); + + expect($page)->toContain('function run_graph_automation(array $ups_ids): void {'); + expect($page)->toContain('INNER JOIN host ON host.id = ups.host_id'); + expect($page)->toContain('automation_update_device($host_id);'); + expect($page)->toContain('$skipped++;'); + expect(substr_count($page, "raise_message('apcupsd_automation'"))->toBe(3); + expect($page)->toContain('MESSAGE_LEVEL_INFO'); + expect($page)->toContain('MESSAGE_LEVEL_WARN'); + + // The device id comes from a prepared query on the UPS id, never from the request. + expect($page)->toContain("[(int) \$ups_id]);"); + expect(preg_match('/automation_update_device\(\$host_id\)/', $page))->toBe(1); + }); + + it('marks a UPS without a device in the confirmation list', function () use ($read) { + $page = $read('upses.php'); + + expect($page)->toContain("__esc('(no Cacti Device yet)', 'apcupsd')"); + }); + + it('is documented', function () use ($read) { + expect($read('README.md'))->toContain('Run Graph Automation'); + expect($read('CHANGELOG.md'))->toContain('"Run Graph Automation"'); + }); +}); diff --git a/upses.php b/upses.php index 03a3d72..fd83ff0 100644 --- a/upses.php +++ b/upses.php @@ -25,9 +25,14 @@ require('../../include/auth.php'); /** @var array $config */ +require_once($config['base_path'] . '/lib/api_automation.php'); require_once($config['base_path'] . '/lib/api_graph.php'); require_once($config['base_path'] . '/lib/api_data_source.php'); +require_once($config['base_path'] . '/lib/api_device.php'); +require_once($config['base_path'] . '/lib/api_tree.php'); +require_once($config['base_path'] . '/lib/data_query.php'); require_once($config['base_path'] . '/lib/poller.php'); +require_once($config['base_path'] . '/lib/template.php'); require_once($config['base_path'] . '/lib/utility.php'); require_once($config['base_path'] . '/lib/rrd.php'); require_once(__DIR__ . '/includes/functions.php'); @@ -41,6 +46,11 @@ 3 => __('Reset Detection', 'apcupsd') ]; +// Creating graphs needs the same realm as Cacti's own "Apply Automation Rules" device action. +if (is_realm_allowed(3)) { + $ups_actions[4] = __('Run Graph Automation', 'apcupsd'); +} + $ups_types = [ 1 => __('APC UPSD Based', 'apcupsd'), 2 => __('SNMP Based', 'apcupsd'), @@ -564,6 +574,49 @@ function duplicate_ups($template_id, $name): void { The 'actions' function ------------------------ */ +/** + * Runs Cacti's Graph Automation (the same as the Devices page's "Apply + * Automation Rules" action) for the Cacti Device linked to each selected UPS, + * creating the graphs, data sources and tree items its templates and + * automation rules call for. Graphs that already exist are left alone. A UPS + * that has no Cacti Device yet (one is created on the first successful poll) + * is skipped, and the outcome is reported as a message on the UPSes page. + * + * @param array $ups_ids The validated apcupsd_ups ids to run it for. + * + * @return void + */ +function run_graph_automation(array $ups_ids): void { + ini_set('max_execution_time', '-1'); + + $ran = 0; + $skipped = 0; + + foreach ($ups_ids as $ups_id) { + $host_id = (int) db_fetch_cell_prepared('SELECT host.id + FROM apcupsd_ups AS ups + INNER JOIN host ON host.id = ups.host_id + WHERE ups.id = ?', + [(int) $ups_id]); + + if ($host_id > 0) { + automation_update_device($host_id); + + $ran++; + } else { + $skipped++; + } + } + + if ($ran === 0) { + raise_message('apcupsd_automation', __('None of the selected UPSes has a Cacti Device yet, so Graph Automation was not run. A device is created on the first successful poll.', 'apcupsd'), MESSAGE_LEVEL_WARN); + } elseif ($skipped > 0) { + raise_message('apcupsd_automation', sprintf(__('Graph Automation was run for %d UPS(es). %d UPS(es) have no Cacti Device yet and were skipped.', 'apcupsd'), $ran, $skipped), MESSAGE_LEVEL_WARN); + } else { + raise_message('apcupsd_automation', sprintf(__('Graph Automation was run for %d UPS(es).', 'apcupsd'), $ran), MESSAGE_LEVEL_INFO); + } +} + /** * Handles the bulk-actions form for the UPSes list (delete/duplicate/ * reset detection). On first display, renders the confirmation dialog @@ -586,7 +639,7 @@ function form_actions(): void { get_filter_request_var('drp_action', FILTER_VALIDATE_REGEXP, ['options' => ['regexp' => '/^([a-zA-Z0-9_]+)$/']]); // ==================================================== - if (!in_array(get_nfilter_request_var('drp_action'), ['1', '2', '3'], true)) { + if (!array_key_exists((int) get_nfilter_request_var('drp_action'), $ups_actions) || !ctype_digit((string) get_nfilter_request_var('drp_action'))) { header('Location: upses.php?header=false'); exit; } @@ -610,6 +663,8 @@ function form_actions(): void { SET snmp_skipped = '' WHERE id IN ($selected_placeholders)", $selected_items); + } elseif (get_nfilter_request_var('drp_action') == '4') { // Run Graph Automation + run_graph_automation($selected_items); } } @@ -629,9 +684,12 @@ function form_actions(): void { input_validate_input_number($matches[1]); // ==================================================== - $name = db_fetch_cell_prepared('SELECT name FROM apcupsd_ups WHERE id = ?', [$matches[1]]); + $name = db_fetch_cell_prepared('SELECT name FROM apcupsd_ups WHERE id = ?', [$matches[1]]); + $host_id = (int) db_fetch_cell_prepared('SELECT host_id FROM apcupsd_ups WHERE id = ?', [$matches[1]]); - $ups_list .= '
  • ' . html_escape($name) . '
  • '; + $ups_list .= '
  • ' . html_escape($name) + . (get_nfilter_request_var('drp_action') == '4' && $host_id <= 0 ? ' ' . __esc('(no Cacti Device yet)', 'apcupsd') . '' : '') + . '
  • '; $ups_array[$i] = $matches[1]; $i++; @@ -678,6 +736,16 @@ function form_actions(): void { $save_html = " " . __esc('Continue', 'apcupsd') . ''; + } elseif (get_nfilter_request_var('drp_action') == '4') { // run graph automation + print "
    +

    " . __n('Click \'Continue\' to run Graph Automation for the following UPS. Graphs, data sources and tree items are created from the device\'s templates and the matching automation rules; graphs that already exist are left as they are. Only a UPS that already has a Cacti Device is affected.', 'Click \'Continue\' to run Graph Automation for the following UPSes. Graphs, data sources and tree items are created from each device\'s templates and the matching automation rules; graphs that already exist are left as they are. Only a UPS that already has a Cacti Device is affected.', cacti_sizeof($ups_array), 'apcupsd') . "

    +
      $ups_list
    +