One printer picker for machines and PCs, and one default per asset

The assignment belongs to the MACHINE, and until now there was no way to set it
except the generic relationships card or the API - the form for the thing the
feature is about did not exist. MachineForm now carries the picker, and PCForm
uses the SAME component rather than its own copy: the PC's set overrides the
machine's, and two implementations of that would drift, with the two ends of an
override disagreeing being exactly the bug nobody would spot.

The shared picker also fixes what PCForm did on save. It wrote row at a time
through the generic relationship endpoints, which is a non-atomic reconcile: an
HTTP failure part way left a PC half-assigned with nothing recording what was
meant. It now calls the reconcile endpoint, which validates the default before
writing anything.

A relationship type can now say it allows one active row per asset
(relationshiptypes.issingular, migration 7d34), and defaultprinter says it.
Cardinality belongs to the type rather than the printers plugin: core's create
path is where every hand-made link passes, and the next type meaning "exactly
one" gets the rule for free. Setting a second default REPLACES the first instead
of refusing, because "make this the default" means that - and a card answering
409 would leave the user hunting for the old row.

Without it the schema was happy to hold two defaults: the unique constraint is
(source, target, type), so two different targets are two valid rows, and the
resolver takes the OLDEST - the new default silently lost. Proven by disabling
the new rule and watching the tests fail.

FOUND WHILE TESTING IN A BROWSER, and it was not mine: MachineForm read
.data.data off computersApi.listAll(), which resolves to the ARRAY - fetchAllPages
has already unwrapped every page. The whole parallel load threw into the catch,
so every dropdown on the machine edit form came up empty and the machine's own
values never loaded. A build cannot see this; only opening the page can.

GET /api/printers/assignments/for-asset/<id> returns an asset's OWN assignment,
without inheritance, because the editor must show what this asset's rows say -
otherwise a machine's printers appear ticked on the PC that inherits them and
unticking one silently creates an override.
This commit is contained in:
cproudlock
2026-08-19 11:22:48 -04:00
parent 72b3904f71
commit e2c45d33bc
12 changed files with 498 additions and 253 deletions

View File

@@ -0,0 +1,116 @@
"""A relationship type that means "at most one of these per asset".
A PC has ONE default printer. The unique constraint on assetrelationships is
(source, target, type), which accepts two DIFFERENT defaults quite happily - and
the resolver takes the oldest, so setting a new default through the generic
relationships card left the OLD one winning, with nothing to show why.
The rule lives on the type, not in the printers plugin: core's create path is
where every hand-made link passes, and the next type meaning "exactly one" gets
it for free.
"""
import pytest
from shopdb.core.models import Asset, AssetType, AssetRelationship, RelationshipType
@pytest.fixture
def scene(db):
assettype = AssetType.query.filter_by(assettype='printer').first()
if not assettype:
assettype = AssetType(assettype='printer', pluginname='printers',
tablename='printers', description='p')
db.session.add(assettype)
db.session.flush()
pc = Asset(assetnumber='PC-SINGULAR', assettypeid=assettype.assettypeid, isactive=True)
first = Asset(assetnumber='PRN-A', assettypeid=assettype.assettypeid, isactive=True)
second = Asset(assetnumber='PRN-B', assettypeid=assettype.assettypeid, isactive=True)
db.session.add_all([pc, first, second])
singular = RelationshipType.query.filter_by(relationshiptype='defaultprinter').first()
if not singular:
singular = RelationshipType(relationshiptype='defaultprinter',
description='default', isdirectional=True)
db.session.add(singular)
singular.issingular = True
plural = RelationshipType.query.filter_by(relationshiptype='usesprinter').first()
if not plural:
plural = RelationshipType(relationshiptype='usesprinter',
description='installed here', isdirectional=True)
db.session.add(plural)
plural.issingular = False
db.session.commit()
return {'pc': pc, 'first': first, 'second': second,
'singular': singular, 'plural': plural}
def _active(pc, reltype):
return AssetRelationship.query.filter_by(
sourceassetid=pc.assetid, relationshiptypeid=reltype.relationshiptypeid,
isactive=True).all()
def _link(client, headers, source, target, reltype):
return client.post('/api/assets/relationships', headers=headers, json={
'sourceassetid': source.assetid,
'targetassetid': target.assetid,
'relationshiptypeid': reltype.relationshiptypeid,
})
def test_setting_a_second_default_replaces_the_first(client, db, scene, auth_headers):
"""The bug this exists for. Both POSTs succeed today and the table then
holds two defaults."""
assert _link(client, auth_headers, scene['pc'], scene['first'], scene['singular']).status_code == 201
assert _link(client, auth_headers, scene['pc'], scene['second'], scene['singular']).status_code == 201
rows = _active(scene['pc'], scene['singular'])
assert len(rows) == 1
assert rows[0].targetassetid == scene['second'].assetid
def test_the_replaced_row_is_soft_deleted_not_destroyed(client, db, scene, auth_headers):
"""Consistent with every other delete here, and it keeps the history."""
_link(client, auth_headers, scene['pc'], scene['first'], scene['singular'])
_link(client, auth_headers, scene['pc'], scene['second'], scene['singular'])
old = AssetRelationship.query.filter_by(
sourceassetid=scene['pc'].assetid,
targetassetid=scene['first'].assetid,
relationshiptypeid=scene['singular'].relationshiptypeid).first()
assert old is not None
assert old.isactive is False
def test_a_plural_type_is_untouched(client, db, scene, auth_headers):
"""usesprinter means "installed here" and a bay has several. If the rule
leaked to every type, assigning a second printer would remove the first."""
assert _link(client, auth_headers, scene['pc'], scene['first'], scene['plural']).status_code == 201
assert _link(client, auth_headers, scene['pc'], scene['second'], scene['plural']).status_code == 201
assert len(_active(scene['pc'], scene['plural'])) == 2
def test_setting_the_same_default_twice_is_still_a_conflict(client, db, scene, auth_headers):
"""Replacing is for a DIFFERENT target. The same link twice is the existing
duplicate case and must keep answering 409, or the card loses its only
signal that nothing changed."""
assert _link(client, auth_headers, scene['pc'], scene['first'], scene['singular']).status_code == 201
assert _link(client, auth_headers, scene['pc'], scene['first'], scene['singular']).status_code == 409
assert len(_active(scene['pc'], scene['singular'])) == 1
def test_another_asset_keeps_its_own_default(client, db, scene, auth_headers):
"""The rule is per SOURCE. One PC's default must not disturb another's."""
other = Asset(assetnumber='PC-OTHER', assettypeid=scene['pc'].assettypeid, isactive=True)
db.session.add(other)
db.session.commit()
_link(client, auth_headers, scene['pc'], scene['first'], scene['singular'])
_link(client, auth_headers, other, scene['second'], scene['singular'])
assert len(_active(scene['pc'], scene['singular'])) == 1
assert len(_active(other, scene['singular'])) == 1