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
This commit is contained in:
cproudlock
2026-08-31 07:59:38 -04:00
parent f1136d920d
commit 6644a07ae1
3 changed files with 130 additions and 1 deletions

View File

@@ -128,6 +128,20 @@ ADR-007 and ADR-002.
### Fixed ### 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 - **A permission became grantable to anonymous visitors because of how it was
spelled.** `guest_grantable_permissions()` opened any permission whose name spelled.** `guest_grantable_permissions()` opened any permission whose name
ended `.view` or `.read` to the guest role, on the reasoning that reading is ended `.view` or `.read` to the guest role, on the reasoning that reading is

View File

@@ -2659,6 +2659,9 @@ def dashboard_supplies():
# dashboard card and the toner report were both doing. # dashboard card and the toner report were both doing.
'levelid': printer.get('levelid'), 'levelid': printer.get('levelid'),
'iscritical': any(s['status'] == 'critical' for s in depleted), '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': [{ 'supplies': [{
'text': '{} {}%'.format(_shortsupplyname(supply.get('name')), 'text': '{} {}%'.format(_shortsupplyname(supply.get('name')),
supply.get('remaining')), supply.get('remaining')),
@@ -2667,7 +2670,18 @@ def dashboard_supplies():
} for supply in depleted], } 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. # 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. # An empty card and a blind card must not look the same.
return success_response({'rows': rows, return success_response({'rows': rows,

View File

@@ -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']