From 870e068c447b029de94e5b542e871629042b0a7d Mon Sep 17 00:00:00 2001 From: Cam Proudlock Date: Sat, 29 Aug 2026 21:35:03 -0400 Subject: [PATCH] Rebuild the dashboard around what needs a person The board was one card per source in a fixed grid: every source got the same small box whatever it had to say (warranties showed 5 of 50), and a box per source flattened priority, while the largest type on the page belonged to counts that need nobody. Now a severity rollup, one worst-first queue of everything needing a person, and a row of context beneath. The queue is full width, capped per source so a noisy check cannot bury a rare one, and names what did not fit with a link to it. Chips filter the queue by check as well as by severity; both narrow what is already loaded, so the counts and rows cannot disagree. Dropped: an 'All assets' tile shown beside its own four addends, a printer strip carrying the same count a third time, three cards whose job was to display 0, and a Recent Devices table listing five machines all 'In Use'. Contract 0.26.0 adds optional 'shortname' on dashboard cards - a title is written as a heading and ran wider than the hostname it qualified as a row badge. Also: the supplies card reported 'Low Supplies 0' when Zabbix was unreachable, which reads as reassurance rather than the truth that nothing could see the cartridges. It now distinguishes unreachable (says so) from unconfigured (stays quiet - the site declined the feature) from a real zero. Full suite 2182 passed, frontend 314 passed. --- frontend/src/components/DashboardCards.vue | 535 ++++++++++++------ frontend/src/components/DashboardRow.vue | 154 +++++ frontend/src/components/dashboardCards.js | 175 +++++- .../src/components/dashboardCards.spec.js | 242 ++++++++ frontend/src/views/Dashboard.vue | 261 +++++---- plugins/backups/plugin.py | 1 + plugins/computers/plugin.py | 2 + plugins/endpoints/plugin.py | 2 + plugins/geenforce/plugin.py | 1 + plugins/machines/plugin.py | 1 + plugins/notifications/plugin.py | 1 + plugins/printers/api/asset_routes.py | 20 +- plugins/printers/plugin.py | 1 + plugins/warranty/plugin.py | 1 + shopdb/__init__.py | 7 +- shopdb/plugins/base.py | 3 + .../test_supplies_card_blindness.py | 90 +++ 17 files changed, 1191 insertions(+), 306 deletions(-) create mode 100644 frontend/src/components/DashboardRow.vue create mode 100644 tests/test_plugins/test_supplies_card_blindness.py diff --git a/frontend/src/components/DashboardCards.vue b/frontend/src/components/DashboardCards.vue index 90ced355..4ce82ba4 100644 --- a/frontend/src/components/DashboardCards.vue +++ b/frontend/src/components/DashboardCards.vue @@ -1,71 +1,136 @@ diff --git a/frontend/src/components/DashboardRow.vue b/frontend/src/components/DashboardRow.vue new file mode 100644 index 00000000..81a1346e --- /dev/null +++ b/frontend/src/components/DashboardRow.vue @@ -0,0 +1,154 @@ + + + + + diff --git a/frontend/src/components/dashboardCards.js b/frontend/src/components/dashboardCards.js index d35e5ed3..75963f87 100644 --- a/frontend/src/components/dashboardCards.js +++ b/frontend/src/components/dashboardCards.js @@ -51,6 +51,26 @@ export function mapDetailTip(card, item) { return full && full !== mapDetail(card, item) ? full : '' } +// "silent 1 days". A suffix is written for the common case, which is plural, +// and the card declaring it has no way to know the value. One row saying it was +// a shrug; the merged queue puts every quiet endpoint on one screen, so it now +// says it five times in a column and reads as broken. +// +// Trims a trailing 's' at exactly 1, on the FIRST word only, keeping whatever +// follows it: ' events (48h)' becomes ' event (48h)'. +// +// The length floor is not arbitrary. It stops abbreviated units being mangled, +// since 'ms' and 'hrs' end in s and are not plurals - a test asserting ' ms' +// stays ' ms' is what caught that. The first version also refused multiword +// suffixes outright to avoid chopping the wrong token, which was too blunt: +// it left ' events (48h)' plural, and capturing the word fixes the same hazard +// properly. +export function singularise(suffix, value) { + const text = suffix || '' + if (Number(value) !== 1) return text + return text.replace(/^ ([a-z]{3,})s(?=$|[ (])/, ' $1') +} + export function mapMeta(card, item) { const meta = (card.map && card.map.meta) || [] return meta @@ -59,7 +79,7 @@ export function mapMeta(card, item) { if (value === '') return null // label prefixes, suffix trails: 'quiet for 3 days' reads as a sentence, // where 'quiet for 3' reads as a truncation. - const text = `${spec.label ? spec.label + ' ' : ''}${value}${spec.suffix || ''}` + const text = `${spec.label ? spec.label + ' ' : ''}${value}${singularise(spec.suffix, value)}` return { text, mono: !!spec.mono } }) .filter(Boolean) @@ -164,12 +184,30 @@ export function metricValue(card) { // "nothing wrong" every day teaches people to stop reading the page - the same // way a fleet log reached 3,234 lines of which 3,217 were one repeated line. // Default is to disappear; 'line' is for the few where absence is itself news. +// Why a card has no rows, when it knows. "Nothing is low" and "we cannot tell +// whether anything is low" are different answers, and a card that gives the +// same empty for both is not merely unhelpful - it reads as reassurance. The +// printer supplies card returns this when Zabbix is unconfigured or +// unreachable, which is how a dead monitor used to render as "Low Supplies 0". +export function cardUnavailable(card) { + const data = card._data + if (!data || Array.isArray(data)) return '' + return typeof data.unavailable === 'string' ? data.unavailable : '' +} + +export function unavailableCards(cards) { + return cards.filter((card) => cardUnavailable(card)) +} + export function cardVisible(card) { if (card._data === null || card._data === undefined) return false // fetch failed const populated = card.render === 'metric' ? metricValue(card) > 0 : rows(card).length > 0 if (populated) return true + // A card that cannot answer says so, whatever its empty rule: 'hide' means + // "nothing to report", and this is not nothing to report. + if (cardUnavailable(card)) return true return card.empty === 'line' } @@ -202,3 +240,138 @@ export const RENDERERS = ['exceptions', 'metric', 'list'] export function renderableCards(cards) { return cards.filter((card) => RENDERERS.includes(card.render)) } + +// --------------------------------------------------------------------------- +// THE MERGED QUEUE +// +// The board used to be one card per source in a fixed grid, and both halves of +// that hurt. Every source got the same small box whatever it had to say, so +// warranties showed 5 of 50 while a one-row card sat beside it leaving a hole; +// and giving each source its own box flattened priority, because an enforcement +// failure and a warranty notice look equally urgent when each owns a card. +// +// Merging them into one list sorted worst-first makes reading order the work +// order. The severity that drives it is already declared per card - this only +// stops the layout from throwing it away. +// --------------------------------------------------------------------------- + +// Severities that are somebody's job right now, in the order they are worked. +export const QUEUE_SEVERITIES = ['critical', 'warning'] + +// Rows one source may contribute to the queue. Without a per-source cap, fifty +// expiring warranties bury two crashed PCs; the cap is what keeps a noisy +// source from crowding out a rare one. +export const QUEUEPERSOURCE = 5 + +// Total rows on the board. Past this it is a report, not something to scan. +export const QUEUEMAX = 12 + +function severityRank(severity) { + const rank = SEVERITY_ORDER[severity] + return rank === undefined ? 3 : rank +} + +/** + * One flat, worst-first list of rows from every queue-severity card. + * + * Each row keeps the fields its own card mapped, plus where it came from, so a + * merged row can still say which check produced it. + */ +/** + * The checks currently contributing to the queue, for the filter chips: + * [{key, label, count}] ordered worst-first then biggest-first. + * + * Severity answers "how bad"; this answers "what kind". On a fleet where five + * endpoints go quiet at once, being able to set those aside and look at the + * crashes is the difference between a board and a list. + */ +export function sourceFacets(cards) { + const seen = new Map() + for (const card of sortCards(cards.filter( + (card) => QUEUE_SEVERITIES.includes(card.severity)))) { + const count = rows(card).length + if (!count) continue + seen.set(card.id, { + key: card.id, + label: card.shortname || card.title || card.id, + count, + severity: card.severity || 'info', + }) + } + return [...seen.values()] +} + +export function queueRows(cards) { + const out = [] + for (const card of sortCards(cards.filter( + (card) => QUEUE_SEVERITIES.includes(card.severity)))) { + const taken = cardRows(card).slice(0, QUEUEPERSOURCE) + taken.forEach((row, index) => { + out.push({ + ...row, + key: card.id + ':' + index, + severity: card.severity || 'info', + // The badge in a merged queue. Cards declare `shortname` for this: + // their `title` is a card HEADER ("Endpoints with crashes or + // shutdowns") and ran wider than the hostname it qualified, putting the + // badge rather than the asset at the start of every line. Falls back to + // the title so a plugin that has not declared one still reads. + source: card.shortname || card.title || '', + sourceid: card.id, + viewall: card.viewall || '', + }) + }) + } + return out.slice(0, QUEUEMAX) +} + +/** + * What the queue could not show, per source: [{id, title, hidden, viewall}]. + * + * Kept per source rather than as one number because the number alone is not + * actionable - "and 45 more" with no way to reach them is worse than silence, + * and each source has its own list to link to. + */ +export function queueOverflow(cards) { + const shown = {} + for (const row of queueRows(cards)) { + shown[row.sourceid] = (shown[row.sourceid] || 0) + 1 + } + return cards + .filter((card) => QUEUE_SEVERITIES.includes(card.severity)) + .map((card) => ({ + id: card.id, + title: card.title || '', + hidden: Math.max(0, rows(card).length - (shown[card.id] || 0)), + viewall: card.viewall || '', + })) + .filter((entry) => entry.hidden > 0) + .sort((a, b) => b.hidden - a.hidden) +} + +/** + * Cards that are context rather than work - the info severity. They keep their + * own list rendering in a rail beside the queue, because 50 expiring warranties + * are worth knowing about and are not worth pushing a crashed PC down the page. + */ +export function contextCards(cards) { + return sortCards(cards.filter( + (card) => !QUEUE_SEVERITIES.includes(card.severity))) +} + +/** + * Row counts by severity, for the rollup above the queue: {critical, warning, + * info}. Counts ROWS, not cards - three cards holding one row each is three + * things to do, not three. + */ +export function severityTotals(cards) { + const totals = { critical: 0, warning: 0, info: 0 } + for (const card of cards) { + const severity = card.severity || 'info' + if (totals[severity] === undefined) continue + totals[severity] += card.render === 'metric' + ? metricValue(card) + : rows(card).length + } + return totals +} diff --git a/frontend/src/components/dashboardCards.spec.js b/frontend/src/components/dashboardCards.spec.js index 5cdfb39f..20939ccb 100644 --- a/frontend/src/components/dashboardCards.spec.js +++ b/frontend/src/components/dashboardCards.spec.js @@ -3,6 +3,9 @@ import { toApiPath, rows, mapMeta, mapLink, cardRows, metricValue, cardVisible, sortCards, permittedCards, renderableCards, visibleRows, overflowCount, + queueRows, queueOverflow, contextCards, severityTotals, + QUEUEPERSOURCE, QUEUEMAX, singularise, + cardUnavailable, unavailableCards, sourceFacets, } from './dashboardCards' const failuresCard = { @@ -226,3 +229,242 @@ describe('stacked rows', () => { .toBe('Start of a long message that continues well past the card') }) }) + +// --------------------------------------------------------------------------- +// The merged queue. These exist because the old grid failed in ways that were +// invisible: a card truncated at 5 of 50 and nothing said where the rest went, +// and severity was declared but the layout ignored it. +// --------------------------------------------------------------------------- + +const critA = { + id: 'endpoints-stability', title: 'Endpoints with crashes', render: 'exceptions', + severity: 'critical', position: 20, viewall: '/pcs', + map: { title: 'hostname' }, + _data: [{ hostname: 'PC-CMM-TEST' }, { hostname: 'HOST-7' }], +} +const critB = { + id: 'geenforce-failures', title: 'Enforcement failures', render: 'exceptions', + severity: 'critical', position: 10, viewall: '/geenforce', + map: { title: 'hostname' }, + _data: [{ hostname: 'WJPC0615' }], +} +const warnA = { + id: 'computers-quiet', title: 'PCs gone silent', render: 'exceptions', + severity: 'warning', position: 30, + map: { title: 'hostname' }, + _data: [{ hostname: 'FFBWTH63' }], +} +const infoA = { + id: 'warranty-expiring', title: 'Warranties expiring', render: 'exceptions', + severity: 'info', position: 40, viewall: '/reports/warranty', + map: { title: 'assetnumber' }, + _data: Array.from({ length: 50 }, (_, i) => ({ assetnumber: 'A' + i })), +} + +describe('queueRows', () => { + it('merges every queue card into one list, worst first', () => { + const queue = queueRows([warnA, critA, critB]) + expect(queue.map((r) => r.severity)).toEqual( + ['critical', 'critical', 'critical', 'warning']) + }) + + it('orders by declared position within a severity', () => { + // geenforce is position 10, endpoints 20: a failure outranks a crash. + const queue = queueRows([critA, critB]) + expect(queue[0].title).toBe('WJPC0615') + }) + + it('keeps the source on every row, so a merged row can still say why', () => { + const queue = queueRows([critB]) + expect(queue[0].source).toBe('Enforcement failures') + expect(queue[0].sourceid).toBe('geenforce-failures') + expect(queue[0].viewall).toBe('/geenforce') + }) + + it('leaves info cards out - they are context, not work', () => { + const queue = queueRows([critB, infoA]) + expect(queue).toHaveLength(1) + expect(queue.every((r) => r.severity !== 'info')).toBe(true) + }) + + it('caps one source so a noisy check cannot bury a rare one', () => { + const noisy = { ...critA, _data: Array.from({ length: 40 }, (_, i) => ({ hostname: 'H' + i })) } + const queue = queueRows([noisy, warnA]) + expect(queue.filter((r) => r.sourceid === 'endpoints-stability')).toHaveLength(QUEUEPERSOURCE) + // the warning still made it onto the board + expect(queue.some((r) => r.sourceid === 'computers-quiet')).toBe(true) + }) + + it('caps the whole board', () => { + const many = Array.from({ length: 6 }, (_, i) => ({ + ...critA, id: 'c' + i, position: i, + _data: Array.from({ length: 5 }, (_, j) => ({ hostname: 'H' + i + j })), + })) + expect(queueRows(many)).toHaveLength(QUEUEMAX) + }) + + it('gives every row a stable key', () => { + const keys = queueRows([critA, critB]).map((r) => r.key) + expect(new Set(keys).size).toBe(keys.length) + }) +}) + +describe('queueOverflow', () => { + it('reports what was hidden, per source, with somewhere to go', () => { + const noisy = { ...critA, _data: Array.from({ length: 12 }, (_, i) => ({ hostname: 'H' + i })) } + const [entry] = queueOverflow([noisy]) + expect(entry.hidden).toBe(7) // 12 rows, 5 shown + expect(entry.viewall).toBe('/pcs') + expect(entry.title).toBe('Endpoints with crashes') + }) + + it('says nothing when everything is on the board', () => { + expect(queueOverflow([critB, warnA])).toEqual([]) + }) + + it('ignores info cards, which are not in the queue to overflow from', () => { + expect(queueOverflow([infoA])).toEqual([]) + }) + + it('lists the biggest backlog first', () => { + const a = { ...critA, id: 'a', _data: Array.from({ length: 8 }, (_, i) => ({ hostname: 'x' + i })) } + const b = { ...critA, id: 'b', _data: Array.from({ length: 20 }, (_, i) => ({ hostname: 'y' + i })) } + expect(queueOverflow([a, b])[0].id).toBe('b') + }) +}) + +describe('contextCards', () => { + it('is the info cards, sorted', () => { + const context = contextCards([critA, infoA, warnA]) + expect(context.map((c) => c.id)).toEqual(['warranty-expiring']) + }) +}) + +describe('severityTotals', () => { + it('counts ROWS, not cards', () => { + // Three cards holding one row each is three things to do, not three cards. + const totals = severityTotals([critA, critB, warnA, infoA]) + expect(totals).toEqual({ critical: 3, warning: 1, info: 50 }) + }) + + it('counts a metric card by its value', () => { + const metric = { id: 'm', render: 'metric', severity: 'warning', _data: { value: 7 } } + expect(severityTotals([metric]).warning).toBe(7) + }) + + it('treats an undeclared severity as info, matching the sort', () => { + const bare = { id: 'b', render: 'exceptions', _data: [{}, {}] } + expect(severityTotals([bare]).info).toBe(2) + }) +}) + +describe('singularise', () => { + it('drops the s at exactly one', () => { + expect(singularise(' days', 1)).toBe(' day') + expect(singularise(' events', 1)).toBe(' event') + }) + + it('leaves plurals alone', () => { + expect(singularise(' days', 0)).toBe(' days') + expect(singularise(' days', 2)).toBe(' days') + expect(singularise(' days', 19)).toBe(' days') + }) + + it('singularises the first word and keeps what follows', () => { + // The card that made this matter: ' events (48h)' read "1 events (48h)" + // five rows running once the queue merged. + expect(singularise(' events (48h)', 1)).toBe(' event (48h)') + expect(singularise(' events (48h)', 2)).toBe(' events (48h)') + }) + + it('handles a trailing word rather than refusing it', () => { + // "1 day ago" is correct; an earlier version left this plural out of + // caution about chopping the wrong token. + expect(singularise(' days ago', 1)).toBe(' day ago') + }) + + it('leaves a unit that merely ends in s', () => { + expect(singularise(' ms', 1)).toBe(' ms') + expect(singularise('%', 1)).toBe('%') + }) + + it('handles no suffix', () => { + expect(singularise('', 1)).toBe('') + expect(singularise(undefined, 1)).toBe('') + }) +}) + +describe('cardUnavailable', () => { + // "Nothing is low" and "we cannot tell whether anything is low" are different + // answers. The printer supplies card returned the same empty for both, so a + // dead Zabbix rendered as "Low Supplies 0" - reassurance, not information. + it('reads the reason a card gives for having no rows', () => { + const card = { _data: { rows: [], unavailable: 'Supply monitoring is not reachable' } } + expect(cardUnavailable(card)).toBe('Supply monitoring is not reachable') + }) + + it('is empty for a card that simply has nothing to report', () => { + expect(cardUnavailable({ _data: { rows: [] } })).toBe('') + expect(cardUnavailable({ _data: [] })).toBe('') + expect(cardUnavailable({ _data: null })).toBe('') + }) + + it('ignores a non-string, so bad data cannot render as a reason', () => { + expect(cardUnavailable({ _data: { rows: [], unavailable: true } })).toBe('') + }) + + it('shows an unavailable card even when its empty rule says hide', () => { + const card = { + empty: 'hide', render: 'exceptions', + _data: { rows: [], unavailable: 'Supply monitoring is not reachable' }, + } + expect(cardVisible(card)).toBe(true) + }) + + it('still hides a card that is merely empty', () => { + expect(cardVisible({ empty: 'hide', render: 'exceptions', _data: { rows: [] } })).toBe(false) + }) + + it('still hides a card whose fetch failed, which is not the same thing', () => { + expect(cardVisible({ empty: 'hide', render: 'exceptions', _data: null })).toBe(false) + }) + + it('picks out the unavailable ones', () => { + const blind = { id: 'a', _data: { rows: [], unavailable: 'not reachable' } } + const fine = { id: 'b', _data: { rows: [{}] } } + expect(unavailableCards([blind, fine]).map((c) => c.id)).toEqual(['a']) + }) +}) + +describe('sourceFacets', () => { + it('is one chip per contributing check, worst first', () => { + const facets = sourceFacets([warnA, critA, critB]) + expect(facets.map((f) => f.key)).toEqual( + ['geenforce-failures', 'endpoints-stability', 'computers-quiet']) + }) + + it('labels with the shortname, since the title is a heading', () => { + const card = { ...critA, shortname: 'Crashes' } + expect(sourceFacets([card])[0].label).toBe('Crashes') + }) + + it('falls back to the title when no shortname is declared', () => { + expect(sourceFacets([critA])[0].label).toBe('Endpoints with crashes') + }) + + it('counts every row, not just the ones the queue had room for', () => { + // The chip says how many there are; the queue caps what it shows. A chip + // reading 5 above a "3 more" line would be its own small lie. + const noisy = { ...critA, _data: Array.from({ length: 12 }, (_, i) => ({ hostname: 'H' + i })) } + expect(sourceFacets([noisy])[0].count).toBe(12) + }) + + it('leaves out checks with nothing to show', () => { + const empty = { ...critA, id: 'empty', _data: [] } + expect(sourceFacets([empty])).toEqual([]) + }) + + it('leaves out info cards, which are not in the queue', () => { + expect(sourceFacets([infoA])).toEqual([]) + }) +}) diff --git a/frontend/src/views/Dashboard.vue b/frontend/src/views/Dashboard.vue index 7600b9d9..c9fffd7f 100644 --- a/frontend/src/views/Dashboard.vue +++ b/frontend/src/views/Dashboard.vue @@ -7,147 +7,168 @@
Loading...
+ + diff --git a/plugins/backups/plugin.py b/plugins/backups/plugin.py index cb6f6575..ea7c3807 100644 --- a/plugins/backups/plugin.py +++ b/plugins/backups/plugin.py @@ -237,6 +237,7 @@ class BackupsPlugin(BasePlugin): { 'id': 'backups-stale', 'title': 'Backups stopped', + 'shortname': 'Backups', 'endpoint': '/api/backups/dashboard/stale', 'render': 'exceptions', 'severity': 'warning', diff --git a/plugins/computers/plugin.py b/plugins/computers/plugin.py index 0b3c10cb..ba77db0f 100644 --- a/plugins/computers/plugin.py +++ b/plugins/computers/plugin.py @@ -1934,6 +1934,7 @@ class ComputersPlugin(BasePlugin): 'id': 'computers-quiet', 'viewall': '/pcs', 'title': 'PCs gone silent', + 'shortname': 'PC silent', 'endpoint': '/api/computers/dashboard/quiet', 'render': 'exceptions', 'severity': 'warning', @@ -1952,6 +1953,7 @@ class ComputersPlugin(BasePlugin): 'id': 'computers-sharedmachines', 'viewall': '/machines', 'title': 'Machine numbers on two PCs', + 'shortname': 'Duplicate machine no.', 'endpoint': '/api/computers/dashboard/sharedmachines', 'render': 'exceptions', 'severity': 'critical', diff --git a/plugins/endpoints/plugin.py b/plugins/endpoints/plugin.py index 5b332855..61d5732a 100644 --- a/plugins/endpoints/plugin.py +++ b/plugins/endpoints/plugin.py @@ -387,6 +387,7 @@ class EndpointsPlugin(BasePlugin): 'id': 'endpoints-quiet', 'viewall': '/pcs', 'title': 'Endpoints gone quiet', + 'shortname': 'Agent quiet', 'endpoint': '/api/endpoints/dashboard/quiet', 'render': 'exceptions', 'severity': 'warning', @@ -405,6 +406,7 @@ class EndpointsPlugin(BasePlugin): 'id': 'endpoints-stability', 'viewall': '/pcs', 'title': 'Endpoints with crashes or shutdowns', + 'shortname': 'Crashes', 'endpoint': '/api/endpoints/dashboard/stability', 'render': 'exceptions', 'severity': 'critical', diff --git a/plugins/geenforce/plugin.py b/plugins/geenforce/plugin.py index 2e12b303..2cd3124e 100644 --- a/plugins/geenforce/plugin.py +++ b/plugins/geenforce/plugin.py @@ -86,6 +86,7 @@ class GeEnforcePlugin(BasePlugin): 'id': 'geenforce-failures', 'viewall': '/geenforce', 'title': 'Enforcement failures', + 'shortname': 'Enforcement', 'endpoint': '/api/geenforce/dashboard/failures', 'render': 'exceptions', 'severity': 'critical', diff --git a/plugins/machines/plugin.py b/plugins/machines/plugin.py index e89d582c..7eaec4a7 100644 --- a/plugins/machines/plugin.py +++ b/plugins/machines/plugin.py @@ -209,6 +209,7 @@ class MachinesPlugin(BasePlugin): 'id': 'machines-outofservice', 'viewall': '/machines', 'title': 'Machines out of service', + 'shortname': 'Out of service', 'endpoint': '/api/machines/dashboard/outofservice', 'render': 'exceptions', 'severity': 'warning', diff --git a/plugins/notifications/plugin.py b/plugins/notifications/plugin.py index 17066764..80d8ed71 100644 --- a/plugins/notifications/plugin.py +++ b/plugins/notifications/plugin.py @@ -206,6 +206,7 @@ class NotificationsPlugin(BasePlugin): 'id': 'notifications-active', 'viewall': '/notifications', 'title': 'Active notifications', + 'shortname': 'Notification', 'endpoint': '/api/notifications/dashboard/active', 'render': 'list', 'severity': 'info', diff --git a/plugins/printers/api/asset_routes.py b/plugins/printers/api/asset_routes.py index 6e3fe938..da463726 100644 --- a/plugins/printers/api/asset_routes.py +++ b/plugins/printers/api/asset_routes.py @@ -2124,8 +2124,21 @@ def _get_low_supplies_data(): return cached service = ZabbixService() - if not service.isconfigured or not service.isreachable: + # "No printer is low" and "we cannot tell whether any printer is low" are + # different answers, and this used to give the same empty one for both. The + # dashboard then showed Low Supplies 0 while the monitoring was dead, which + # is not a missing feature but a wrong statement: it reads as reassurance. + if not service.isconfigured: + # Enabled plus a URL plus a token. Absent any of those the site has not + # asked for supply monitoring, and a permanent "not configured" line + # would be a card reporting nothing wrong every day - which is how a + # board teaches people to stop reading it. Nothing to say: stay hidden. return {'printers': [], 'summary': {'total_checked': 0, 'low': 0, 'critical': 0}} + if not service.isreachable: + # Configured and unanswerable is a different thing: something meant to + # work is not working, and the cartridges are NOT known to be fine. + return {'printers': [], 'unavailable': 'Supply monitoring is not reachable', + 'summary': {'total_checked': 0, 'low': 0, 'critical': 0}} # active printers with an IP, with vendor and model for waste/part rules rows = ( @@ -2655,7 +2668,10 @@ def dashboard_supplies(): }) rows.sort(key=lambda r: (not r['iscritical'], r['printername'])) - return success_response(rows) + # Envelope rather than a bare list, so the card can say WHY it has no rows. + # An empty card and a blind card must not look the same. + return success_response({'rows': rows, + 'unavailable': data.get('unavailable')}) # ============================================================================= diff --git a/plugins/printers/plugin.py b/plugins/printers/plugin.py index bb6fa548..1d470255 100644 --- a/plugins/printers/plugin.py +++ b/plugins/printers/plugin.py @@ -354,6 +354,7 @@ class PrintersPlugin(BasePlugin): 'id': 'printers-supplies', 'viewall': '/reports/toner', 'title': 'Printer supplies', + 'shortname': 'Supplies', 'endpoint': '/api/printers/dashboard/supplies', 'render': 'exceptions', 'severity': 'warning', diff --git a/plugins/warranty/plugin.py b/plugins/warranty/plugin.py index f519025c..520c5e36 100644 --- a/plugins/warranty/plugin.py +++ b/plugins/warranty/plugin.py @@ -128,6 +128,7 @@ class WarrantyPlugin(BasePlugin): 'id': 'warranty-expiring', 'viewall': '/reports/warranty', 'title': 'Warranties expiring', + 'shortname': 'Warranty', 'endpoint': '/api/warranty/dashboard/expiring', 'render': 'exceptions', 'severity': 'info', diff --git a/shopdb/__init__.py b/shopdb/__init__.py index b61894e6..ed4572e5 100644 --- a/shopdb/__init__.py +++ b/shopdb/__init__.py @@ -88,7 +88,12 @@ from .plugins import plugin_manager # emission never consulted the caller, so a read-only viewer was shown controls # over a shop-floor PC that 403 on click. Guests are included, since a signed-out # visitor holds whatever the guest role was granted. Additive name, minor bump. -__contract_version__ = '0.25.0' +# 0.26.0: dashboard cards may declare 'shortname'. The dashboard merged its +# per-source cards into one worst-first queue, and a card's 'title' is written +# as a heading - "Endpoints with crashes or shutdowns" - which as a row badge +# ran wider than the hostname it qualified. Optional, defaults to the title, so +# a plugin that does not declare one is unaffected. Additive field, minor bump. +__contract_version__ = '0.26.0' # Product release version (see ADR-007). The product version and the # plugin-contract version above are distinct series with independent diff --git a/shopdb/plugins/base.py b/shopdb/plugins/base.py index 844224c4..6114b80a 100644 --- a/shopdb/plugins/base.py +++ b/shopdb/plugins/base.py @@ -238,6 +238,9 @@ class BasePlugin(ABC): Each card: { 'id': str, # stable, unique across plugins 'title': str, # heading + 'shortname': str, # optional: label for this card's rows in the + # merged queue. A title reads as a header and is + # too long as a row badge; defaults to title. 'endpoint': str, # API path the card fetches its own data from 'render': str, # core renderer: 'exceptions' | 'list' | 'stat' 'severity': str, # 'critical' | 'warning' | 'info' - orders cards diff --git a/tests/test_plugins/test_supplies_card_blindness.py b/tests/test_plugins/test_supplies_card_blindness.py new file mode 100644 index 00000000..1e766a24 --- /dev/null +++ b/tests/test_plugins/test_supplies_card_blindness.py @@ -0,0 +1,90 @@ +"""The supplies dashboard card must not report "nothing low" when it cannot see. + +_get_low_supplies_data returned the same empty result for three different +situations - Zabbix not configured, Zabbix unreachable, and every cartridge +full - so the dashboard displayed "Low Supplies 0 / Critical 0" while the +monitoring was dead. That is not a missing feature; it is a wrong statement, and +it reads as reassurance. + +These pin the three cases apart. +""" +from unittest.mock import patch + +import pytest + +from shopdb.api import db +from shopdb.core.models import Setting + + +def _call(client, auth_headers): + response = client.get('/api/printers/dashboard/supplies', headers=auth_headers) + assert response.status_code == 200 + return response.get_json()['data'] + + +class FakeService: + def __init__(self, configured, reachable): + self.isconfigured = configured + self.isreachable = reachable + + +@pytest.fixture(autouse=True) +def _clear_cache(app): + from shopdb.extensions import cache + cache.delete('printers_low_supplies') + yield + cache.delete('printers_low_supplies') + + +def test_unreachable_says_so_rather_than_reporting_zero(client, auth_headers): + """Configured and unanswerable: the cartridges are NOT known to be fine.""" + with patch('plugins.printers.api.asset_routes.ZabbixService', + return_value=FakeService(configured=True, reachable=False)): + data = _call(client, auth_headers) + + assert data['rows'] == [] + assert data['unavailable'] == 'Supply monitoring is not reachable' + + +def test_unconfigured_stays_quiet(client, auth_headers): + """The site did not ask for supply monitoring. + + A permanent "not configured" line would be a card reporting nothing wrong + every day, which is how a board teaches people to stop reading it. It has + nothing to say, so it says nothing and the card hides. + """ + with patch('plugins.printers.api.asset_routes.ZabbixService', + return_value=FakeService(configured=False, reachable=False)): + data = _call(client, auth_headers) + + assert data['rows'] == [] + assert data.get('unavailable') is None + + +def test_reachable_with_nothing_low_is_not_flagged_as_blind(client, auth_headers): + """Zabbix answered and every cartridge is fine. That is a real zero.""" + with patch('plugins.printers.api.asset_routes._get_low_supplies_data', + return_value={'printers': [], + 'summary': {'total_checked': 3, 'low': 0, 'critical': 0}}): + data = _call(client, auth_headers) + + assert data['rows'] == [] + assert data.get('unavailable') is None + + +def test_the_envelope_carries_rows_so_the_card_can_still_render(client, auth_headers): + """The response became an envelope to carry the reason; rows must survive.""" + printer = { + 'printerid': 1, 'assetnumber': 'PRN-1', 'printername': 'Front office', + 'location': 'Bay 3', 'mapx': None, 'mapy': None, 'levelid': None, + 'supplies': [{'name': 'Black', 'remaining': 2, 'status': 'critical'}], + } + with patch('plugins.printers.api.asset_routes._get_low_supplies_data', + return_value={'printers': [printer], + 'summary': {'total_checked': 1, 'low': 1, 'critical': 1}}): + data = _call(client, auth_headers) + + assert data.get('unavailable') is None + assert len(data['rows']) == 1 + assert data['rows'][0]['printername'] == 'Front office' + assert data['rows'][0]['iscritical'] is True