From 7151b68bdd1f58d9c6cbe6c69929cf525c0171a4 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Tue, 11 Aug 2026 14:13:26 -0400 Subject: [PATCH] dashboard: printer supplies, expiring warranties, mis-numbered bays Wave one complete. Three cards, no new data and no migrations. Printer supplies reuses the existing low-supplies query and its five-minute cache; a Zabbix round-trip per printer on every dashboard load would make this the slowest page in the app. One row per printer listing every depleted cartridge, criticals first - a row per cartridge would report one printer three times and read as three problems, and showing only the worst class would hide a low cartridge behind a critical one on the same machine when whoever walks out there wants to carry both. While there: the low-supplies REPORT itself was including healthy cartridges. A printer with one empty black and three full colour ones listed all four, so the reader had to find the problem inside the row. It now lists only what needs replacing, and the test that asserted the old behaviour now asserts the new. Expiring warranties keeps already-expired entries on the list rather than dropping them the day they lapse, which is how they get missed. Horizon is warranty_expiringdays, default 90, because that suits a site budgeting quarterly and nobody else. Mis-numbered bays promotes check-shared-machines out of a CLI command nobody will remember to run - it found seven bays that had been wrong for weeks. It reports only numbers with NO child assets, so part markers legitimately sharing an operation stay silent: that distinction is the whole card, and without it it would list correct data beside faults and be ignored. Printers also loses its dead component-named widget; notifications, network and machines still have theirs. --- CHANGELOG.md | 9 ++- plugins/computers/api/routes.py | 70 +++++++++++++++++ plugins/computers/plugin.py | 16 ++++ plugins/printers/api/asset_routes.py | 51 +++++++++++- plugins/printers/plugin.py | 26 +++++-- plugins/warranty/api/routes.py | 53 ++++++++++++- plugins/warranty/plugin.py | 43 ++++++++++ .../test_plugins/test_computers_dashboard.py | 78 +++++++++++++++++++ tests/test_plugins/test_zabbix_live.py | 6 +- 9 files changed, 339 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6f91d9..8dd94e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,8 +19,13 @@ ADR-007 and ADR-002. The only evidence a backup still ran was a line in a log file on the PC. `lastseenat` is touched on every matching post, including the no-op, and is backfilled from the timestamps that already exist. -- Dashboard cards for enforcement failures, PCs not reporting, and backups that - have stopped. The dashboard now renders cards plugins declare, which it never +- The low-supplies report lists only the cartridges that need replacing. A + printer with one empty black and three full colour cartridges was listing all + four, so the reader had to hunt for the problem inside the row. The report + exists to answer "what needs replacing". +- Dashboard cards for enforcement failures, machine numbers claimed by two PCs, + PCs not reporting, backups that have stopped, printer supplies, and expiring + warranties. The dashboard now renders cards plugins declare, which it never did before - five plugins had been declaring widgets into a void, pointing at components nobody wrote. Cards are ordered by severity, hide themselves when there is nothing to report, gate on a permission, and each fetches diff --git a/plugins/computers/api/routes.py b/plugins/computers/api/routes.py index 0be067c..3f2e2f8 100644 --- a/plugins/computers/api/routes.py +++ b/plugins/computers/api/routes.py @@ -994,3 +994,73 @@ def dashboard_quiet(): # down the list, not the order the rows came out of the table. rows.sort(key=lambda r: (r['quietdays'] is not None, -(r['quietdays'] or 0))) return success_response(rows[:50]) + + +@computers_bp.route('/dashboard/sharedmachines', methods=['GET']) +@jwt_required() +@require_permission('computers.view') +def dashboard_sharedmachines(): + """Machine numbers claimed by more than one PC, with nothing filed under + them. + + Several devices genuinely sharing a number is legitimate - part markers do + it - and those are modelled: each device is its own asset filed `partof` the + operation, so the operation has CHILD ASSETS. Two PCs carrying the same + machine number by mistake looks identical from a count and has none. That + distinction is the whole card; without it this would list correct data + alongside faults and get ignored. + + Promoted from `flask relationships check-shared-machines`, which answers the + same question and which nobody will remember to run. This one found seven + mis-numbered bays that had been that way for weeks. + """ + from shopdb.api import AssetRelationship, RelationshipType + from sqlalchemy.orm import aliased + + controls = RelationshipType.query.filter_by( + relationshiptype='controls').first() + partof = RelationshipType.query.filter_by(relationshiptype='partof').first() + if not controls: + return success_response([]) + + pcasset = aliased(Asset) + machineasset = aliased(Asset) + + rows = (db.session.query(machineasset.assetid, machineasset.assetnumber, + pcasset.assetnumber) + .select_from(AssetRelationship) + .join(pcasset, AssetRelationship.sourceassetid == pcasset.assetid) + .join(machineasset, + AssetRelationship.targetassetid == machineasset.assetid) + .filter(AssetRelationship.relationshiptypeid == + controls.relationshiptypeid, + AssetRelationship.label == 'collector:machine', + AssetRelationship.isactive.is_(True)) + .all()) + + bymachine = {} + for assetid, machinenumber, pcnumber in rows: + bymachine.setdefault((assetid, machinenumber), []).append(pcnumber) + + out = [] + for (assetid, machinenumber), pcs in bymachine.items(): + if len(pcs) < 2: + continue + children = 0 + if partof: + children = (AssetRelationship.query + .filter_by(targetassetid=assetid, + relationshiptypeid=partof.relationshiptypeid, + isactive=True) + .count()) + if children: + continue # modelled: the devices are assets in their own right + out.append({ + 'assetid': assetid, + 'machinenumber': machinenumber, + 'pccount': len(pcs), + 'pcs': ', '.join(sorted(p for p in pcs if p)), + }) + + out.sort(key=lambda r: -r['pccount']) + return success_response(out) diff --git a/plugins/computers/plugin.py b/plugins/computers/plugin.py index ff21e05..92c7bb3 100644 --- a/plugins/computers/plugin.py +++ b/plugins/computers/plugin.py @@ -1280,6 +1280,22 @@ class ComputersPlugin(BasePlugin): 'link': '/pcs/{computerid}', }, }, + { + 'id': 'computers-sharedmachines', + 'title': 'Machine numbers on two PCs', + 'endpoint': '/api/computers/dashboard/sharedmachines', + 'render': 'exceptions', + 'severity': 'critical', + 'permission': 'computers.view', + 'empty': 'hide', + 'position': 15, + 'map': { + 'title': 'machinenumber', + 'detail': 'pcs', + 'meta': [{'key': 'pccount', 'suffix': ' PCs'}], + 'link': '/machines/{assetid}', + }, + }, ] def get_navigation_items(self) -> List[Dict]: diff --git a/plugins/printers/api/asset_routes.py b/plugins/printers/api/asset_routes.py index e951e7e..a1a17a7 100644 --- a/plugins/printers/api/asset_routes.py +++ b/plugins/printers/api/asset_routes.py @@ -1005,12 +1005,17 @@ def _get_low_supplies_data(): model_number = model.modelnumber if model else None modelnumberid = model.modelnumberid if model else None + # ONLY the supplies that need attention. A printer reporting one empty + # black cartridge alongside three full colour ones was listing all four, + # so the reader had to find the problem inside the row rather than being + # shown it. The whole report exists to answer "what needs replacing". annotated = [] has_low = False for s in supplies: item = _annotate_supply(s, vendor_name, modelnumberid) - if item['status'] != 'ok': - has_low = True + if item['status'] == 'ok': + continue + has_low = True annotated.append(item) if has_low: @@ -1388,3 +1393,45 @@ def delete_model_supply(modelsupplyid: int): db.session.delete(supply) db.session.commit() return success_response(message='Supply deleted') + + +@printers_asset_bp.route('/dashboard/supplies', methods=['GET']) +@jwt_required() +@require_permission('printers.view') +def dashboard_supplies(): + """Printers needing a cartridge, flattened to one row per printer. + + Reuses the existing low-supplies query and its five-minute cache, so the + card costs nothing extra: a Zabbix round-trip per printer on every dashboard + load would make this the slowest page in the app. + + Critical first, then low. A printer with several depleted cartridges appears + once, listing them - a row per cartridge would report one printer three + times and read as three problems. + """ + data = _get_low_supplies_data() + + rows = [] + for printer in data.get('printers', []): + criticals = [s for s in printer['supplies'] if s['status'] == 'critical'] + lows = [s for s in printer['supplies'] if s['status'] == 'low'] + if not criticals and not lows: + continue + # Every depleted supply, criticals first. Listing only the worst class + # would hide a low cartridge behind a critical one on the same printer, + # and whoever walks out there wants to carry both. + worst = criticals + lows + rows.append({ + 'printerid': printer['printerid'], + 'printername': printer['printername'] or printer['assetnumber'], + 'location': printer['location'], + 'status': 'critical' if criticals else 'low', + 'supplies': ', '.join( + '{} {}%'.format(s.get('name') or s.get('type') or 'supply', + s.get('percent')) + for s in worst), + 'iscritical': bool(criticals), + }) + + rows.sort(key=lambda r: (not r['iscritical'], r['printername'])) + return success_response(rows) diff --git a/plugins/printers/plugin.py b/plugins/printers/plugin.py index fdabfd6..fd6cf8a 100644 --- a/plugins/printers/plugin.py +++ b/plugins/printers/plugin.py @@ -287,14 +287,28 @@ class PrintersPlugin(BasePlugin): return [printerscli] def get_dashboard_widgets(self) -> List[Dict]: - """Return dashboard widget definitions.""" + """Dashboard card: printers needing a cartridge. + + Replaces a declaration naming a component nobody wrote. Reuses the + low-supplies query and its cache - a Zabbix round-trip per printer on + every dashboard load would make this the slowest page in the app. + """ return [ { - 'name': 'Printer Status', - 'component': 'PrinterStatusWidget', - 'endpoint': '/api/printers/dashboard/summary', - 'size': 'medium', - 'position': 10, + 'id': 'printers-supplies', + 'title': 'Printer supplies', + 'endpoint': '/api/printers/dashboard/supplies', + 'render': 'exceptions', + 'severity': 'warning', + 'permission': 'printers.view', + 'empty': 'hide', + 'position': 40, + 'map': { + 'title': 'printername', + 'detail': 'supplies', + 'meta': [{'key': 'location'}, {'key': 'status'}], + 'link': '/printers/{printerid}', + }, }, ] diff --git a/plugins/warranty/api/routes.py b/plugins/warranty/api/routes.py index 9111a55..232565b 100644 --- a/plugins/warranty/api/routes.py +++ b/plugins/warranty/api/routes.py @@ -5,7 +5,7 @@ never stored. Warranties link to assets many-to-many via warrantyassets, though the common case is one warranty per asset. """ -from datetime import date, datetime, timezone +from datetime import date, datetime, timedelta, timezone from flask import Blueprint, request from flask_jwt_extended import jwt_required @@ -391,3 +391,54 @@ def warranty_report(): 'counts': {k: len(v) for k, v in buckets.items()}, 'buckets': buckets, }) + + +@warranty_bp.route('/dashboard/expiring', methods=['GET']) +@jwt_required() +@require_permission('warranty.view') +def dashboard_expiring(): + """Warranties running out, and ones that already have. + + Already-expired first, then soonest. Expired stays on the list rather than + dropping off: a machine out of warranty is a purchasing decision someone + still has to make, and silently removing it the day it lapses is how it gets + missed entirely. + + Horizon is a setting because 90 days suits a site that budgets quarterly and + nobody else (ADR-015). + """ + from shopdb.api import Setting + + days = 90 + setting = Setting.query.filter_by(key='warranty_expiringdays').first() + if setting and (setting.value or '').strip(): + try: + days = int(setting.value) + except (TypeError, ValueError): + pass + + today = date.today() + horizon = today + timedelta(days=days) + + rows = [] + query = (db.session.query(Warranty, WarrantyAsset, Asset) + .join(WarrantyAsset, WarrantyAsset.warrantyid == Warranty.warrantyid) + .join(Asset, Asset.assetid == WarrantyAsset.assetid) + .filter(Warranty.isactive.is_(True), + Warranty.enddate.isnot(None), + Warranty.enddate <= horizon, + Asset.isactive.is_(True))) + + for warranty, _link, asset in query.all(): + remaining = (warranty.enddate - today).days + rows.append({ + 'assetid': asset.assetid, + 'assetnumber': asset.assetnumber or asset.name or str(asset.assetid), + 'enddate': warranty.enddate.isoformat(), + 'provider': warranty.provider, + 'daysleft': remaining, + 'state': 'expired' if remaining < 0 else 'expiring', + }) + + rows.sort(key=lambda r: r['daysleft']) + return success_response(rows[:50]) diff --git a/plugins/warranty/plugin.py b/plugins/warranty/plugin.py index 146a8e3..b36caa6 100644 --- a/plugins/warranty/plugin.py +++ b/plugins/warranty/plugin.py @@ -114,6 +114,49 @@ class WarrantyPlugin(BasePlugin): }, ] + def get_dashboard_widgets(self) -> List[Dict]: + """Dashboard card: warranties running out, and ones already expired. + + Expired stays listed rather than dropping off. A machine out of + warranty is a purchasing decision someone still has to make, and + removing it the day it lapses is how it gets missed entirely. + """ + return [ + { + 'id': 'warranty-expiring', + 'title': 'Warranties expiring', + 'endpoint': '/api/warranty/dashboard/expiring', + 'render': 'exceptions', + 'severity': 'info', + 'permission': 'warranty.view', + 'empty': 'hide', + 'position': 50, + 'map': { + 'title': 'assetnumber', + 'detail': 'state', + 'meta': [ + {'key': 'enddate', 'label': 'ends', 'format': 'date'}, + {'key': 'provider'}, + ], + 'link': '/assets/{assetid}', + }, + }, + ] + + def get_settings_defaults(self) -> List[Dict]: + """Settings this plugin owns.""" + return [ + { + 'key': 'warranty_expiringdays', + 'value': '90', + 'valuetype': 'integer', + 'category': 'warranty', + 'description': 'Days ahead to list an expiring warranty on the ' + 'dashboard. Already-expired warranties are always ' + 'listed.', + }, + ] + def get_permissions(self) -> List: """Return the RBAC permissions this plugin owns.""" return [ diff --git a/tests/test_plugins/test_computers_dashboard.py b/tests/test_plugins/test_computers_dashboard.py index b93f5bc..fe2bc94 100644 --- a/tests/test_plugins/test_computers_dashboard.py +++ b/tests/test_plugins/test_computers_dashboard.py @@ -118,3 +118,81 @@ def test_the_widget_uses_the_new_contract(app): assert widget['render'] == 'exceptions' assert widget['permission'] == 'computers.view' assert widget['empty'] == 'hide' + + +# ============================================================================= +# Machine numbers claimed by two PCs - a fault, unless the devices are modelled +# ============================================================================= + +SHAREDURL = '/api/computers/dashboard/sharedmachines' + + +def _machine(db, assetnumber): + mt = AssetType.query.filter_by(assettype='machine').first() + if not mt: + mt = AssetType(assettype='machine') + db.session.add(mt) + db.session.flush() + asset = Asset(assetnumber=assetnumber, assettypeid=mt.assettypeid, + statusid=1) + db.session.add(asset) + db.session.commit() + return asset + + +def _controls(db, pchostname, machineasset, label='collector:machine'): + from shopdb.core.models import AssetRelationship, RelationshipType + controls = RelationshipType.query.filter_by(relationshiptype='controls').first() + if not controls: + controls = RelationshipType(relationshiptype='controls', isdirectional=True) + db.session.add(controls) + db.session.flush() + comp = _pc(db, pchostname) + db.session.add(AssetRelationship( + sourceassetid=comp.assetid, targetassetid=machineasset.assetid, + relationshiptypeid=controls.relationshiptypeid, label=label, + isactive=True)) + db.session.commit() + return comp + + +def test_one_pc_per_machine_is_not_a_finding(client, db, auth_headers): + machine = _machine(db, '3015') + _controls(db, 'ONLYPC', machine) + assert client.get(SHAREDURL, headers=auth_headers).get_json()['data'] == [] + + +def test_two_pcs_on_one_number_is_reported(client, db, auth_headers): + machine = _machine(db, '2026') + _controls(db, 'PCONE', machine) + _controls(db, 'PCTWO', machine) + [row] = client.get(SHAREDURL, headers=auth_headers).get_json()['data'] + assert row['machinenumber'] == '2026' + assert row['pccount'] == 2 + assert 'PCONE' in row['pcs'] and 'PCTWO' in row['pcs'] + + +def test_a_modelled_shared_number_is_not_a_finding(client, db, auth_headers): + """Part markers legitimately share an operation number. Modelled correctly, + each device is its own asset filed partof the operation - so the operation + has child assets, and that is what tells a real fault from correct data.""" + from shopdb.core.models import AssetRelationship, RelationshipType + operation = _machine(db, '0615') + _controls(db, 'MARKERPCA', operation) + _controls(db, 'MARKERPCB', operation) + + partof = RelationshipType(relationshiptype='partof', isdirectional=True) + db.session.add(partof) + db.session.flush() + marker = _machine(db, 'MARKERPCA-PARTMARKER') + db.session.add(AssetRelationship( + sourceassetid=marker.assetid, targetassetid=operation.assetid, + relationshiptypeid=partof.relationshiptypeid, + label='collector:partmarker', isactive=True)) + db.session.commit() + + assert client.get(SHAREDURL, headers=auth_headers).get_json()['data'] == [] + + +def test_shared_machines_is_permission_gated(client, db, member_headers): + assert client.get(SHAREDURL, headers=member_headers).status_code == 403 diff --git a/tests/test_plugins/test_zabbix_live.py b/tests/test_plugins/test_zabbix_live.py index 5f87f24..d9edb86 100644 --- a/tests/test_plugins/test_zabbix_live.py +++ b/tests/test_plugins/test_zabbix_live.py @@ -138,5 +138,7 @@ def test_low_supplies_rollup_flags_waste_and_toner(app, db, mock_zabbix): assert statuses['Black Toner Level'] == 'critical' # 97%-full waste (HP, non-inverted) -> 3% remaining -> critical assert statuses['Waste Cartridge Level'] == 'critical' - # 60% cyan is fine - assert statuses['Cyan Toner Level'] == 'ok' + # 60% cyan is FINE, so it is not listed at all. The report answers + # "what needs replacing"; including healthy cartridges made the reader + # hunt for the problem inside the row. + assert 'Cyan Toner Level' not in statuses