Warranty search finds a machine number; the collector can no longer set a printer assignment

This commit is contained in:
2026-08-24 11:52:48 -04:00
parent f109dd1349
commit 0398c55fb5
6 changed files with 263 additions and 272 deletions

View File

@@ -16,16 +16,14 @@ from .api import computers_bp
logger = logging.getLogger(__name__)
# Marker stamped on PC->printer links this collector creates. The stale-link
# archive only touches rows carrying this label, so manually-created printer
# relationships are never removed by a collector push. Stored in the
# assetrelationships.label column (no origin column exists; see BUILD notes).
PRINTER_LINK_ORIGIN = 'collector:printers'
# Marker stamped on PC->measuringtool "controls" links this collector creates
# for metrology PCs (CMM, Keyence, Genspect, wax-and-trace). Same stale-link
# discipline as PRINTER_LINK_ORIGIN: only rows carrying this label are archived
# by a collector push, so hand-made tool links survive.
# for metrology PCs (CMM, Keyence, Genspect, wax-and-trace). Only rows carrying
# this label are archived by a collector push, so hand-made tool links survive.
# Stored in assetrelationships.label (no origin column exists; see BUILD notes).
#
# There is deliberately no printer equivalent any more. PRINTER_LINK_ORIGIN went
# with _sync_printer_links: printer ASSIGNMENT is never collected, so there are
# no collector-owned printer rows left to label or archive.
MEASURINGTOOL_LINK_ORIGIN = 'collector:measuringtool'
# Marker stamped on the PC->machine "controls" link built from the reported
@@ -139,26 +137,10 @@ class ComputersPlugin(BasePlugin):
'type': 'array',
'items': {'name': 'string', 'version': 'string'},
},
# Printer identifiers reported by the GE-Enforce side, which
# runs Get-CimInstance Win32_Printer and marks the default with
# the Default flag. An identifier is the printer's windows name,
# share name, hostname, or port/IP; the collector resolves it
# flexibly to a printer asset. Both optional. Presence of either
# key drives the PC->printer relationship sync (and stale-link
# archive); absence leaves existing printer links untouched.
'defaultprinter': {
'type': 'string',
'description': ('Default printer identifier (Win32_Printer '
'with Default=true): windows name, share '
'name, hostname, or port IP.'),
},
'printers': {
'type': 'array',
'items': {'type': 'string'},
'description': ('All installed network printer identifiers '
'(Win32_Printer): windows name / share / '
'hostname / IP. Unresolved -> warning.'),
},
# RETIRED: 'defaultprinter' and 'printers' used to live here and
# wrote PC->printer assignment rows from a report. That is the
# one thing this path must never do - see _RETIRED_PRINTER_KEYS.
# A client still sending them is warned, not rejected.
'accessprotocols': {
'type': 'array',
'items': {'type': 'string'},
@@ -423,8 +405,9 @@ class ComputersPlugin(BasePlugin):
else:
machinelinks = []
# Printer relationship sync (only when the payload carried printer data).
printerlinks = self._sync_printer_links(comp.asset, payload, warnings)
# Printer assignment is NOT collected. See _warn_retired_printer_keys.
self._warn_retired_printer_keys(payload, warnings)
printerlinks = []
# Measuring-tool sync: metrology PCs (CMM/Keyence/Genspect/wax-trace)
# get an attached MeasuringTool asset auto-created and linked.
@@ -518,134 +501,38 @@ class ComputersPlugin(BasePlugin):
AccessProtocol.protocolid.in_(wanted)).all()] if wanted else []
return sorted(names)
def _sync_printer_links(self, pcasset, payload, warnings):
"""Idempotently sync PC->printer relationships from collector printer data.
def _warn_retired_printer_keys(self, payload, warnings):
"""Refuse to turn a report into an assignment, out loud.
Default printer -> 'defaultprinter' (directional). Other reported
printers -> 'connectedto' (symmetric). Resolves each identifier to a
printer asset by windows name / share / hostname / asset number / name
or a communications IP. Unresolved identifiers add a warning and never
fail the push.
RETIRED 2026-08-24. This used to be _sync_printer_links, which wrote
'defaultprinter' and 'connectedto' rows straight from a collector
payload. That contradicted the rule the whole printer feature rests on:
ASSIGNED and OBSERVED never mix. A bay that installed the wrong printer
would have made itself right simply by reporting it, and enforcement
would stop meaning anything.
Stale-link archive: on each push, collector-tagged links (label ==
PRINTER_LINK_ORIGIN) whose (target, type) pair is not in the reported
desired set are set inactive. Only tagged rows are touched, so manual
links survive. Runs only when the payload carried a printer key
('defaultprinter' or 'printers'); a PC that reports without printer data
keeps its existing links. Returns the desired-link list (created + kept).
Worse than dormant, it was ADVERTISED: the collector schema listed both
keys and COLLECTOR-INTEGRATION.md's example client set defaultprinter,
while the same document promised on another page that nothing on this
path writes an assignment row. The document contradicted itself and the
code sided with the wrong half.
Observed queues have their own path (POST /api/collector/printers) and
become assignments only when a person asks, through
seed-from-observed after reviewing the comparison.
WARNS RATHER THAN REJECTS. Unknown keys are not validated here, so a
client written against the old docs keeps pushing successfully; only its
printer keys stop doing anything. Silently ignoring them would leave an
integrator believing assignments were being written.
"""
from shopdb.api import (
AssetRelationship, RelationshipType, Asset, Communication)
has_default = 'defaultprinter' in payload
has_list = 'printers' in payload
if not has_default and not has_list:
return []
try:
from plugins.printers.models import Printer
except ImportError:
warnings.append('printers plugin unavailable; printer links skipped')
return []
dp_type = RelationshipType.query.filter_by(
relationshiptype='defaultprinter').first()
ct_type = RelationshipType.query.filter_by(
relationshiptype='connectedto').first()
if not dp_type or not ct_type:
warnings.append('printer relationship types missing; '
'run flask seed reference-data')
return []
def resolve(identifier):
# first match wins: printer text identity, then a printer IP.
ident = (identifier or '').strip()
if not ident:
return None
printer = (
Printer.query.join(Asset, Asset.assetid == Printer.assetid)
.filter(Asset.isactive == True)
.filter(db.or_(
Printer.windowsname.ilike(ident),
Printer.hostname.ilike(ident),
Printer.sharename.ilike(ident),
Asset.assetnumber.ilike(ident),
Asset.name.ilike(ident),
)).first())
if printer:
return printer.asset
comm = (
db.session.query(Communication)
.join(Printer, Printer.assetid == Communication.assetid)
.filter(Communication.ipaddress == ident)
.first())
if comm:
return db.session.get(Asset, comm.assetid)
return None
pcid = pcasset.assetid
desired = set() # (targetassetid, relationshiptypeid) to keep
printerlinks = []
default_id = None
default_ident = (payload.get('defaultprinter') or '').strip()
if default_ident:
target = resolve(default_ident)
if target:
default_id = target.assetid
desired.add((default_id, dp_type.relationshiptypeid))
self._sync_one(pcid, default_id, dp_type)
printerlinks.append({'assetid': default_id,
'relationshiptype': 'defaultprinter'})
else:
warnings.append(f'unresolved default printer: {default_ident}')
for ident in payload.get('printers') or []:
target = resolve(ident)
if not target:
warnings.append(f'unresolved printer: {ident}')
continue
if target.assetid == default_id:
continue # already the default link
desired.add((target.assetid, ct_type.relationshiptypeid))
self._sync_one(pcid, target.assetid, ct_type)
printerlinks.append({'assetid': target.assetid,
'relationshiptype': 'connectedto'})
# Archive collector-tagged links no longer reported (manual links, with
# a NULL/other label, are never matched here).
collector_rows = AssetRelationship.query.filter(
AssetRelationship.sourceassetid == pcid,
AssetRelationship.relationshiptypeid.in_(
[dp_type.relationshiptypeid, ct_type.relationshiptypeid]),
AssetRelationship.isactive == True,
AssetRelationship.label == PRINTER_LINK_ORIGIN,
).all()
for rel in collector_rows:
if (rel.targetassetid, rel.relationshiptypeid) not in desired:
rel.isactive = False
return printerlinks
def _sync_one(self, pcid, printerid, reltype):
"""Reactivate or create one collector PC->printer link (idempotent)."""
from shopdb.api import AssetRelationship
existing = AssetRelationship.query.filter_by(
sourceassetid=pcid, targetassetid=printerid,
relationshiptypeid=reltype.relationshiptypeid).first()
if existing:
# do not re-stamp label: a pre-existing manual row stays manual.
if not existing.isactive:
existing.isactive = True
return existing
rel = AssetRelationship(
sourceassetid=pcid, targetassetid=printerid,
relationshiptypeid=reltype.relationshiptypeid,
label=PRINTER_LINK_ORIGIN)
db.session.add(rel)
return rel
# -- measuring-tool sync -----------------------------------------------
sent = [key for key in ('defaultprinter', 'printers') if key in payload]
if sent:
warnings.append(
'%s ignored: a collector report never sets printer assignment. '
'Report queues to POST /api/collector/printers, then adopt them '
'with POST /api/printers/assignments/seed-from-observed.'
% ' and '.join(sent))
def _sync_machine_link(self, comp, machinenumber, warnings):
"""Link a PC to the machine it drives, from the reported machine number.

View File

@@ -24,7 +24,7 @@
<option value="unknown">Unknown</option>
</select>
</label>
<input v-model="search" type="text" class="form-control" placeholder="Search vendor, service level, tag, asset..." />
<input v-model="search" type="text" class="form-control" placeholder="Search vendor, service level, tag, asset number, machine..." />
<span class="result-count">{{ filteredItems.length }} of {{ items.length }}</span>
</div>
@@ -186,6 +186,7 @@ import api, { warrantyApi, assetsApi, vendorsApi } from '@/api'
import { useToast } from '@/composables/toast'
import { apiError } from '@/utils/apiError'
import MachineMapChip from '../components/MachineMapChip.vue'
import { matchesWarrantySearch } from './warrantySearch'
const toast = useToast()
const route = useRoute()
@@ -197,16 +198,11 @@ const search = ref('')
const page = ref(1)
const perPage = 25
// Client-side search across vendor / service level / tag / linked asset numbers.
const filteredItems = computed(() => {
const term = search.value.trim().toLowerCase()
if (!term) return items.value
return items.value.filter(w =>
(w.vendor || '').toLowerCase().includes(term) ||
(w.servicelevel || '').toLowerCase().includes(term) ||
(w.servicetag || '').toLowerCase().includes(term) ||
(w.assets || []).some(a => (a.assetnumber || '').toLowerCase().includes(term)))
})
// Client-side search. The field list lives in warrantySearch.js, where it is
// covered by tests - the machine number was rendered in this table for a long
// time before anything let you search for it.
const filteredItems = computed(() =>
items.value.filter(w => matchesWarrantySearch(w, search.value)))
const totalPages = computed(() => Math.max(1, Math.ceil(filteredItems.value.length / perPage)))
const paginatedItems = computed(() => filteredItems.value.slice((page.value - 1) * perPage, page.value * perPage))

View File

@@ -0,0 +1,44 @@
// What the warranty list's search box matches against.
//
// Pulled out of WarrantiesList.vue so the field list is somewhere a test can
// see it. It went wrong the way a field list usually does: silently, by a
// column being added to the table and not to the filter.
//
// THE MACHINE NUMBER IS THE POINT. _related_machine in the warranty API exists
// precisely because "a shopfloor PC is bought, warranted and replaced as a PC,
// but it is FOUND by the machine it drives - nobody walks the floor looking for
// an asset number". The payload carries it, the table renders it in a chip, and
// the filter did not look at it - so the one identifier the feature was built
// around was the one thing the search box could not find.
/**
* Does this warranty match the search term?
*
* @param {object} warranty - a row from GET /api/warranties
* @param {string} term - the raw search box contents
* @returns {boolean} true when the term is empty or any searched field contains it
*/
export function matchesWarrantySearch(warranty, term) {
const needle = (term || '').trim().toLowerCase()
if (!needle) return true
if (!warranty) return false
const has = (value) => String(value ?? '').toLowerCase().includes(needle)
if (has(warranty.vendor) || has(warranty.servicelevel) || has(warranty.servicetag)) {
return true
}
return (warranty.assets || []).some((asset) => {
if (has(asset?.assetnumber)) return true
// The related machine is optional: a warranty on something that drives no
// machine has none, and a machine that was never linked leaves it null.
const machine = asset?.machine
return !!machine && (has(machine.machinenumber) || has(machine.name))
})
}
/** The fields above, in the order the placeholder should name them. */
export const WARRANTY_SEARCH_FIELDS = [
'vendor', 'service level', 'tag', 'asset number', 'machine',
]

View File

@@ -0,0 +1,79 @@
import { describe, it, expect } from 'vitest'
import { matchesWarrantySearch } from './warrantySearch'
// Reported: the warranty page could not be searched by machine number.
//
// The number was in the payload (_related_machine puts it there) and rendered
// in the table as a chip, but the filter looked only at vendor, service level,
// service tag and asset number. So the identifier the feature was built around
// - a PC is warranted as a PC but FOUND by the machine it drives - was the one
// thing the box could not find.
function warranty(overrides = {}) {
return {
vendor: 'Dell',
servicelevel: 'ProSupport',
servicetag: 'ABC1234',
assets: [{
assetnumber: 'PC-00912',
name: 'Bay 3105 workstation',
machine: { machinenumber: '3016', name: 'Makino a61nx' },
}],
...overrides,
}
}
describe('matchesWarrantySearch', () => {
it('matches on the machine number', () => {
// The reported case.
expect(matchesWarrantySearch(warranty(), '3016')).toBe(true)
})
it('matches on the machine name', () => {
expect(matchesWarrantySearch(warranty(), 'makino')).toBe(true)
})
it('still matches the fields it always did', () => {
expect(matchesWarrantySearch(warranty(), 'dell')).toBe(true)
expect(matchesWarrantySearch(warranty(), 'prosupport')).toBe(true)
expect(matchesWarrantySearch(warranty(), 'abc1234')).toBe(true)
expect(matchesWarrantySearch(warranty(), 'pc-00912')).toBe(true)
})
it('is case insensitive and ignores surrounding space', () => {
expect(matchesWarrantySearch(warranty(), ' MAKINO ')).toBe(true)
})
it('does not match an unrelated term', () => {
expect(matchesWarrantySearch(warranty(), '9999')).toBe(false)
})
it('keeps every row when the term is empty', () => {
expect(matchesWarrantySearch(warranty(), '')).toBe(true)
expect(matchesWarrantySearch(warranty(), ' ')).toBe(true)
expect(matchesWarrantySearch(warranty(), undefined)).toBe(true)
})
it('handles a warranty whose asset drives no machine', () => {
// machine is null whenever the covered asset has no controls link - a
// printer, or a PC nobody linked yet. It must not throw.
const row = warranty({ assets: [{ assetnumber: 'PR-42', machine: null }] })
expect(matchesWarrantySearch(row, 'PR-42')).toBe(true)
expect(matchesWarrantySearch(row, '3016')).toBe(false)
})
it('handles a warranty with no linked assets at all', () => {
const row = warranty({ assets: [] })
expect(matchesWarrantySearch(row, 'dell')).toBe(true)
expect(matchesWarrantySearch(row, '3016')).toBe(false)
})
it('tolerates missing fields rather than matching everything', () => {
// String(undefined) is 'undefined', which would make a search for 'und'
// match every row with an empty field.
const row = { vendor: null, servicelevel: undefined, assets: null }
expect(matchesWarrantySearch(row, 'und')).toBe(false)
expect(matchesWarrantySearch(row, 'null')).toBe(false)
})
})