From 6644a07ae1290d8344952e32f314880a3276fe7e Mon Sep 17 00:00:00 2001 From: cproudlock Date: Mon, 31 Aug 2026 07:59:38 -0400 Subject: [PATCH] Order the supplies card by how empty the cartridge is Rows sorted on iscritical then on printer NAME. Critical is everything at or below 5%, so a cartridge at 5% and one at 0% tied on the first key and the alphabet decided which came first. Wrong order on its own, and worse than it looks: the dashboard queue keeps only the first few rows per source, so a printer that was genuinely empty was truncated off the card while a fuller one held its place. Reported from the floor as "starts at 5% and hides one at 0%", which is exactly the two bugs compounding. remaining is already normalised for waste cartridges - a full waste bin reads as low remaining - so one ascending sort is worst-first for every supply type, and iscritical falls out of it rather than needing its own key. Name only breaks ties. worstremaining is carried on the row because the supplies list is rendered to text ("Black 0%") before it reaches the client, so the number the sort needs is gone by then. The toner forecast report was already worst-first (it sorts on daysleft) and is unchanged. Scope: product --- CHANGELOG.md | 14 +++ plugins/printers/api/asset_routes.py | 16 ++- .../test_supplies_card_ordering.py | 101 ++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 tests/test_plugins/test_supplies_card_ordering.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 69979dbe..c7f54a08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,6 +128,20 @@ ADR-007 and ADR-002. ### Fixed +- **The printer supplies card was ordered by printer name, so the emptiest + cartridge was not first and sometimes was not shown at all.** Rows sorted on + "is critical" and then alphabetically, and critical is everything at or below + 5% - so a cartridge at 5% and one at 0% tied on the first key and the alphabet + broke the tie. That is the wrong order on its own, and worse than it looks: + the dashboard queue keeps only the first few rows per source, so a printer + that was genuinely empty got truncated off the card while a fuller one held + its place. The card exists to answer "what do I walk out and change today" and + was answering it with the alphabet. Rows now sort by the emptiest cartridge on + the printer, ascending. Waste cartridges were already normalised to percent + remaining, so one ascending sort is worst-first for every supply type and the + critical flag falls out of it rather than needing its own key; the name only + breaks ties. + - **A permission became grantable to anonymous visitors because of how it was spelled.** `guest_grantable_permissions()` opened any permission whose name ended `.view` or `.read` to the guest role, on the reasoning that reading is diff --git a/plugins/printers/api/asset_routes.py b/plugins/printers/api/asset_routes.py index da463726..3b3e7d39 100644 --- a/plugins/printers/api/asset_routes.py +++ b/plugins/printers/api/asset_routes.py @@ -2659,6 +2659,9 @@ def dashboard_supplies(): # dashboard card and the toner report were both doing. 'levelid': printer.get('levelid'), 'iscritical': any(s['status'] == 'critical' for s in depleted), + # The emptiest cartridge on this printer, which is what decides the + # row's place in the queue. depleted is sorted and non-empty here. + 'worstremaining': depleted[0]['remaining'], 'supplies': [{ 'text': '{} {}%'.format(_shortsupplyname(supply.get('name')), supply.get('remaining')), @@ -2667,7 +2670,18 @@ def dashboard_supplies(): } for supply in depleted], }) - rows.sort(key=lambda r: (not r['iscritical'], r['printername'])) + # Emptiest first. This sorted on iscritical then NAME, and critical is + # everything at or below 5%, so a cartridge at 5% and one at 0% tied on the + # first key and the alphabet decided which came first. That is bad ordering + # on its own and worse than it looks: the dashboard queue takes only the + # first few rows per source, so the printer that is actually empty was + # being truncated away while a fuller one held its place. + # + # remaining is already normalised for waste cartridges (a full waste bin + # reads as low remaining), so one ascending sort is worst-first for every + # supply type, and iscritical falls out of it rather than needing its own + # key. Name only breaks ties. + rows.sort(key=lambda r: (r['worstremaining'], r['printername'])) # 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, diff --git a/tests/test_plugins/test_supplies_card_ordering.py b/tests/test_plugins/test_supplies_card_ordering.py new file mode 100644 index 00000000..1eb40fef --- /dev/null +++ b/tests/test_plugins/test_supplies_card_ordering.py @@ -0,0 +1,101 @@ +"""The supplies card must put the emptiest cartridge first. + +It sorted on iscritical, then on PRINTER NAME. Critical is everything at or +below 5%, so a cartridge at 5% and one at 0% tied on the first key and the +alphabet decided the order. + +That is bad ordering on its own, and worse than it looks: the dashboard queue +takes only the first few rows per source, so a printer that is genuinely empty +was truncated out of the card while a fuller one held its place. The card +existed to answer "what do I walk out and change today" and was answering it +with the alphabet. +""" +from unittest.mock import patch + +import pytest + + +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'] + + +@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 _printer(printerid, name, remaining, status='critical'): + return { + 'printerid': printerid, + 'printername': name, + 'assetnumber': 'A{}'.format(printerid), + 'location': 'Shop floor', + 'mapx': None, 'mapy': None, 'levelid': None, + 'supplies': [{ + 'name': 'Black Toner', 'level': remaining, 'remaining': remaining, + 'status': status, 'iswaste': False, 'isdrum': False, + 'partnumbers': [], 'lastseen': None, 'color': 'black', + 'supplytype': 'toner', + }], + } + + +def _patched(rows): + return patch('plugins.printers.api.asset_routes._get_low_supplies_data', + return_value={'printers': rows, + 'summary': {'total_checked': len(rows), + 'low': 0, 'critical': len(rows)}}) + + +def test_the_empty_cartridge_comes_before_the_nearly_empty_one(client, + auth_headers): + """'Alpha' at 5% used to precede 'Zulu' at 0% purely on the name.""" + with _patched([_printer(1, 'Alpha', 5), _printer(2, 'Zulu', 0)]): + data = _call(client, auth_headers) + + order = [row['printername'] for row in data['rows']] + assert order == ['Zulu', 'Alpha'] + + +def test_rows_are_ordered_by_how_empty_they_are(client, auth_headers): + with _patched([_printer(1, 'Alpha', 4), + _printer(2, 'Bravo', 0), + _printer(3, 'Charlie', 2), + _printer(4, 'Delta', 5)]): + data = _call(client, auth_headers) + + remaining = [row['worstremaining'] for row in data['rows']] + assert remaining == sorted(remaining) + assert remaining == [0, 2, 4, 5] + + +def test_a_zero_percent_printer_survives_the_queue_truncation(client, + auth_headers): + """The dashboard queue keeps only the first few rows per source. + + With five fuller printers whose names sort earlier, the empty one used to + fall off the end of the card entirely. + """ + rows = [_printer(index, name, 5) for index, name in enumerate( + ['Alpha', 'Bravo', 'Charlie', 'Delta', 'Echo'], start=1)] + rows.append(_printer(6, 'Zulu', 0)) + + with _patched(rows): + data = _call(client, auth_headers) + + # QUEUEPERSOURCE is 5 on the frontend; first place is what matters here. + assert data['rows'][0]['printername'] == 'Zulu' + assert [r['printername'] for r in data['rows'][:5]].count('Zulu') == 1 + + +def test_the_name_still_breaks_a_tie(client, auth_headers): + with _patched([_printer(1, 'Bravo', 3), _printer(2, 'Alpha', 3)]): + data = _call(client, auth_headers) + + assert [row['printername'] for row in data['rows']] == ['Alpha', 'Bravo']