diff --git a/plugins/computers/plugin.py b/plugins/computers/plugin.py index cb1fd19..781f963 100644 --- a/plugins/computers/plugin.py +++ b/plugins/computers/plugin.py @@ -798,19 +798,26 @@ class ComputersPlugin(BasePlugin): rel.isactive = False operation = self._link_marker_to_operation( - markerasset, machinenumber, warnings) + markerasset, machinenumber, pcasset, warnings) return [{'assetid': markerasset.assetid, 'assetnumber': markerasset.assetnumber, 'operationassetid': operation}] - def _link_marker_to_operation(self, markerasset, machinenumber, warnings): + def _link_marker_to_operation(self, markerasset, machinenumber, pcasset, + warnings): """Make a marker `partof` the operation whose number its PC reports. Unlike the PC-to-machine link this does NOT contest: an operation can hold any number of markers, which is the whole point. Moving a marker to another operation archives the old membership rather than deleting it, so where a marker used to live stays answerable. + + Refuses to file a marker under the reporting PC. A PC first seen before + the machine-number fix was created with the machine number as its OWN + asset number, and that is deliberately never overwritten, so looking up + the number can return the PC itself. Filing the marker partof its own PC + would read, on the machine page, as the PC being the operation. """ from shopdb.api import AssetRelationship, RelationshipType, Asset @@ -834,6 +841,13 @@ class ComputersPlugin(BasePlugin): return None if operation.assetid == markerasset.assetid: return None + if pcasset is not None and operation.assetid == pcasset.assetid: + warnings.append( + 'machine number {!r} is this PC\'s own asset number; marker ' + 'not filed under an operation. Rename the PC asset to its ' + 'hostname, or create the operation asset.'.format( + machinenumber)) + return None links = AssetRelationship.query.filter( AssetRelationship.sourceassetid == markerasset.assetid, diff --git a/tests/test_core/test_collector_contract.py b/tests/test_core/test_collector_contract.py index ed27ba6..71e3d25 100644 --- a/tests/test_core/test_collector_contract.py +++ b/tests/test_core/test_collector_contract.py @@ -1008,3 +1008,41 @@ def test_an_ordinary_pc_gets_no_marker( 'pctype': 'gea-shopfloor-collections'}, headers={'X-API-Key': collector_key}) assert Asset.query.filter(Asset.assetnumber.like('%-PARTMARKER')).all() == [] + + +def test_a_marker_is_never_filed_under_its_own_pc( + client, db, collector_key, computer_assettype, machine_3015): + """A PC first seen before the machine-number fix was created with the + machine number as its OWN asset number, and that is never overwritten. So + resolving the number can return the PC itself, and filing the marker partof + its own PC reads on the machine page as the PC being the operation.""" + from shopdb.core.models import Asset, AssetRelationship, RelationshipType + from plugins.computers.models import Computer + + db.session.add(RelationshipType(relationshiptype='partof', + isdirectional=True)) + db.session.commit() + + # A legacy PC: its own asset carries the machine number, and no separate + # asset for the operation exists. + client.post('/api/collector/computers', + json={'hostname': 'LEGACYPC', + 'pctype': 'gea-shopfloor-partmarker'}, + headers={'X-API-Key': collector_key}) + comp = Computer.query.filter(Computer.hostname.ilike('LEGACYPC')).first() + comp.asset.assetnumber = '0615' + db.session.commit() + + r = client.post('/api/collector/computers', + json={'hostname': 'LEGACYPC', 'machinenumber': '0615', + 'pctype': 'gea-shopfloor-partmarker'}, + headers={'X-API-Key': collector_key}) + + partof = RelationshipType.query.filter_by(relationshiptype='partof').first() + links = AssetRelationship.query.filter_by( + relationshiptypeid=partof.relationshiptypeid, isactive=True).all() + targets = [db.session.get(Asset, rel.targetassetid).assetid + for rel in links] + assert comp.assetid not in targets, 'marker filed under its own PC' + assert any("own asset number" in w + for w in r.get_json()['data']['warnings'])