From e0e4cce8bd9a4fe48e1910eab412b1f3c262ede3 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Tue, 11 Aug 2026 14:45:28 -0400 Subject: [PATCH] dashboard: fix what a real fleet showed, which tests could not Three faults, visible only once the board ran against production data. BACKUPS SAID THE WHOLE FLEET HAD STOPPED. The lastseenat backfill was wrong. It seeded from collectedat, reasoning that the last change was the last provable moment - but an unchanged config writes no revision, so a machine whose settings last changed nine months ago got a nine-month-old lastseenat and was instantly reported as a dead backup. Every chain lit up at once, which is worse than no card: it says the site is broken when it is fine. The honest value is NULL. Before the column existed nothing recorded when a config was last confirmed, and inventing a date does not change that. Migration 0003 clears the backfill, and staleness now IGNORES a NULL chain rather than substituting timestamps that mean something else. A chain becomes measurable the first time its PC posts, which for NTLARS is within a day. TONER READ "None%". The supply dict has no 'percent' key - it is 'remaining'. Supply names are also shortened, because "Black Toner Level 4%" spends three words saying what the card already says. THE CARDS READ AS WALLS OF TEXT. Rows wrapped into paragraphs and a card with forty PCs pushed everything below it off the screen. Now: at most five rows with "and N more", one line per row that truncates rather than wraps, meta pushed right and dropped first since it matters least, and severity reduced to a small dot beside an uppercase label instead of a coloured card - six severity-painted cards read as a crisis, which is how a board stops being read. Worth recording that none of this could fail in a test. Every one needed real data on a real fleet. --- frontend/src/components/DashboardCards.vue | 122 +++++++++++++----- frontend/src/components/dashboardCards.js | 14 ++ .../src/components/dashboardCards.spec.js | 19 +++ .../0003_backups_clear_backfilled_lastseen.py | 38 ++++++ plugins/backups/services/staleness.py | 12 +- plugins/printers/api/asset_routes.py | 18 ++- tests/test_plugin_migrations.py | 2 +- 7 files changed, 188 insertions(+), 37 deletions(-) create mode 100644 plugins/backups/migrations/versions/0003_backups_clear_backfilled_lastseen.py diff --git a/frontend/src/components/DashboardCards.vue b/frontend/src/components/DashboardCards.vue index 9ef787c..9e61600 100644 --- a/frontend/src/components/DashboardCards.vue +++ b/frontend/src/components/DashboardCards.vue @@ -7,6 +7,7 @@ :class="'dc-' + (card.severity || 'info')" >
+

{{ card.title }}

{{ countOf(card) }}
@@ -14,20 +15,23 @@

{{ metricValue(card) }}

- + -

Nothing to action.

+

+ and {{ overflowCount(card) }} more +

@@ -45,7 +49,7 @@ import { ref, computed, onMounted } from 'vue' import api from '../api' import { useAuthStore } from '@/stores/auth' import { - toApiPath, cardRows, metricValue, cardVisible, sortCards, + toApiPath, visibleRows, overflowCount, metricValue, cardVisible, sortCards, permittedCards, renderableCards, rows as cardData, } from './dashboardCards' @@ -54,10 +58,6 @@ const cards = ref([]) const visibleCards = computed(() => sortCards(cards.value.filter(cardVisible))) -function hasRows(card) { - return card.render === 'metric' ? metricValue(card) > 0 : cardData(card).length > 0 -} - function countOf(card) { return cardData(card).length } @@ -93,35 +93,97 @@ defineExpose({ load }) diff --git a/frontend/src/components/dashboardCards.js b/frontend/src/components/dashboardCards.js index 55c87d5..2b95d59 100644 --- a/frontend/src/components/dashboardCards.js +++ b/frontend/src/components/dashboardCards.js @@ -75,6 +75,20 @@ export function mapLink(card, item) { return missing ? null : href } +// How many rows a card shows before collapsing the rest behind a count. A card +// listing forty PCs is a report someone has to read, not a board someone can +// scan - and it pushes every card below it off the screen. Five is enough to +// see the shape of the problem; the link goes to the full list. +export const MAXROWS = 5 + +export function visibleRows(card) { + return cardRows(card).slice(0, MAXROWS) +} + +export function overflowCount(card) { + return Math.max(0, rows(card).length - MAXROWS) +} + export function cardRows(card) { return rows(card).map((item) => ({ title: mapTitle(card, item), diff --git a/frontend/src/components/dashboardCards.spec.js b/frontend/src/components/dashboardCards.spec.js index c88e0fd..3e7aa69 100644 --- a/frontend/src/components/dashboardCards.spec.js +++ b/frontend/src/components/dashboardCards.spec.js @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest' import { toApiPath, rows, mapMeta, mapLink, cardRows, metricValue, cardVisible, sortCards, permittedCards, renderableCards, + visibleRows, overflowCount, } from './dashboardCards' const failuresCard = { @@ -77,6 +78,24 @@ describe('mapping a row', () => { }) }) +describe('long lists', () => { + const many = (n) => ({ + render: 'exceptions', + map: { title: 'hostname' }, + _data: Array.from({ length: n }, (_v, i) => ({ hostname: `PC${i}` })), + }) + + it('shows at most five rows so one card cannot bury the rest', () => { + expect(visibleRows(many(40))).toHaveLength(5) + expect(overflowCount(many(40))).toBe(35) + }) + + it('does not claim an overflow when everything fits', () => { + expect(visibleRows(many(3))).toHaveLength(3) + expect(overflowCount(many(3))).toBe(0) + }) +}) + describe('empty handling', () => { it('hides a card with nothing to report by default', () => { // The whole point: a card saying "nothing wrong" daily trains people to diff --git a/plugins/backups/migrations/versions/0003_backups_clear_backfilled_lastseen.py b/plugins/backups/migrations/versions/0003_backups_clear_backfilled_lastseen.py new file mode 100644 index 0000000..81e3374 --- /dev/null +++ b/plugins/backups/migrations/versions/0003_backups_clear_backfilled_lastseen.py @@ -0,0 +1,38 @@ +"""backups: clear the backfilled lastseenat - it was a guess, and it showed. + +0002 added lastseenat and backfilled it from collectedat, reasoning that the +last change was the last moment the config could be PROVEN current. On a fleet +that was wrong in practice: an unchanged config writes no revision, so a machine +whose settings last changed nine months ago got a nine-month-old lastseenat and +was immediately reported as a stopped backup. Every chain lit up at once, which +is worse than no card - it says the fleet is broken when it is fine. + +The honest value is NULL: before this column existed, nothing recorded when a +config was last confirmed, and inventing a date does not change that. A chain +becomes measurable the first time its PC posts after the upgrade, which for +NTLARS is within a day. + +So staleness now IGNORES a chain whose lastseenat is NULL, rather than falling +back to timestamps that mean something else. +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'backups0003clearlastseen' +down_revision = 'backups0002lastseenat' +branch_labels = None +depends_on = None + + +def upgrade(): + columns = {c['name'] for c in + sa.inspect(op.get_bind()).get_columns('backuprevisions')} + if 'lastseenat' in columns: + op.execute('UPDATE backuprevisions SET lastseenat = NULL') + + +def downgrade(): + # Nothing to restore: the backfilled values were derived, not recorded. + pass diff --git a/plugins/backups/services/staleness.py b/plugins/backups/services/staleness.py index 3831fe7..8275451 100644 --- a/plugins/backups/services/staleness.py +++ b/plugins/backups/services/staleness.py @@ -65,10 +65,14 @@ def stalechains(days=None, limit=50): rows = [] for revision in latestperchain().values(): - # Rows written before lastseenat existed fall back to the timestamps - # that do exist, so an old install reports something sane on day one - # rather than every chain at once. - seen = revision.lastseenat or revision.collectedat or revision.createdat + # NULL means never confirmed since the column existed, and it is NOT + # substituted with collectedat. That substitution is what the first + # version did, and on a real fleet it reported every machine whose + # config had simply been stable for months as a stopped backup - the + # whole board lit up and said the site was broken when it was fine. + # A chain becomes measurable the first time its PC posts; until then + # this card says nothing about it, which is the truth. + seen = revision.lastseenat if seen is None or seen >= cutoff: continue asset = db.session.get(Asset, revision.assetid) diff --git a/plugins/printers/api/asset_routes.py b/plugins/printers/api/asset_routes.py index a1a17a7..399f753 100644 --- a/plugins/printers/api/asset_routes.py +++ b/plugins/printers/api/asset_routes.py @@ -1395,6 +1395,18 @@ def delete_model_supply(modelsupplyid: int): return success_response(message='Supply deleted') +def _shortsupplyname(name): + """'Black Toner Level' -> 'Black'. The card has one line per printer, and + the words Toner and Level carry no information when every row is a toner + level.""" + text = (name or 'supply').strip() + for noise in (' Cartridge Level', ' Toner Level', ' Level', ' Cartridge'): + if text.endswith(noise): + text = text[:-len(noise)] + break + return text or 'supply' + + @printers_asset_bp.route('/dashboard/supplies', methods=['GET']) @jwt_required() @require_permission('printers.view') @@ -1426,9 +1438,11 @@ def dashboard_supplies(): 'printername': printer['printername'] or printer['assetnumber'], 'location': printer['location'], 'status': 'critical' if criticals else 'low', + # 'remaining' is the percent left. There is no 'percent' key - + # reading one rendered every cartridge as "None%" on the board. 'supplies': ', '.join( - '{} {}%'.format(s.get('name') or s.get('type') or 'supply', - s.get('percent')) + '{} {}%'.format(_shortsupplyname(s.get('name')), + s.get('remaining')) for s in worst), 'iscritical': bool(criticals), }) diff --git a/tests/test_plugin_migrations.py b/tests/test_plugin_migrations.py index 80bfe3f..19ad305 100644 --- a/tests/test_plugin_migrations.py +++ b/tests/test_plugin_migrations.py @@ -49,7 +49,7 @@ EXPECTED_HEAD_REVISION['measuringtools'] = 'measuringtools0001baseline' # backups is also post-cutover: its 0001 really creates backuprevisions, and # 0002 adds lastseenat - when a config was last CONFIRMED unchanged, which dedup # otherwise throws away. -EXPECTED_HEAD_REVISION['backups'] = 'backups0002lastseenat' +EXPECTED_HEAD_REVISION['backups'] = 'backups0003clearlastseen' # geenforce adds the content-addressed blob store (manifestblobs) on top of its # baseline. EXPECTED_HEAD_REVISION['geenforce'] = 'geenforce0002blobs'