diff --git a/CHANGELOG.md b/CHANGELOG.md index c3c714d0..2d0b1566 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,6 +105,13 @@ ADR-007 and ADR-002. the wrong code. The page index now travels with the page. - The back label's dotted preview outline outspecified the print reset and drew a dashed rule down the edge of every second card. +- **The warranty list could not be searched by machine number.** The number was + in the payload and rendered in the table as a chip, but the search box looked + only at vendor, service level, service tag and asset number. That made the + identifier the feature was built around - a shopfloor PC is warranted as a PC + but found by the machine it drives - the one thing the box could not find. + Machine number and machine name are now searched too, and the placeholder + names them. - **`flask plugin upgrade-all` failed a plugin whose tables already existed** ("table securesendsecrets already exists" on the tools plugin). Every post-cutover baseline emitted a plain `op.create_table`, which is correct only @@ -132,6 +139,20 @@ ADR-007 and ADR-002. ### Changed +- **The computers collector no longer accepts `defaultprinter` or `printers`, + and can no longer write a printer assignment.** Those keys used to become + `defaultprinter`/`connectedto` relationship rows straight from a report, which + contradicts the rule the whole printer feature rests on: a collector report is + OBSERVED state and never ASSIGNED state, or a bay that installed the wrong + printer makes itself right simply by reporting it. It was not a dormant code + path - the collector schema advertised both keys and the example client in + `docs/COLLECTOR-INTEGRATION.md` set `defaultprinter`, while another page of + the same document promised nothing on that path writes an assignment row. + **A client still sending them keeps working**: the keys are ignored, not + rejected, and the response carries a warning naming them and pointing at + `POST /api/collector/printers` for reporting queues and + `seed-from-observed` for adopting them. Silently dropping them would have left + an integrator believing assignments were still being written. - **Tech Tools no longer asks people to sign in for tools that need no account.** `/tools`, `/tools/codes` and `/tools/secure-send` were all behind `requiresAuth`. The label generator runs entirely in the browser - no API, no diff --git a/plugins/computers/plugin.py b/plugins/computers/plugin.py index 96f1e6b6..01bea5cd 100644 --- a/plugins/computers/plugin.py +++ b/plugins/computers/plugin.py @@ -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. diff --git a/plugins/warranty/frontend/views/WarrantiesList.vue b/plugins/warranty/frontend/views/WarrantiesList.vue index e208cc04..1913dd92 100644 --- a/plugins/warranty/frontend/views/WarrantiesList.vue +++ b/plugins/warranty/frontend/views/WarrantiesList.vue @@ -24,7 +24,7 @@ - + {{ filteredItems.length }} of {{ items.length }} @@ -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)) diff --git a/plugins/warranty/frontend/views/warrantySearch.js b/plugins/warranty/frontend/views/warrantySearch.js new file mode 100644 index 00000000..8579e299 --- /dev/null +++ b/plugins/warranty/frontend/views/warrantySearch.js @@ -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', +] diff --git a/plugins/warranty/frontend/views/warrantySearch.spec.js b/plugins/warranty/frontend/views/warrantySearch.spec.js new file mode 100644 index 00000000..fc2e17cd --- /dev/null +++ b/plugins/warranty/frontend/views/warrantySearch.spec.js @@ -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) + }) +}) diff --git a/tests/test_core/test_collector_printers.py b/tests/test_core/test_collector_printers.py index ac48bd93..576b039a 100644 --- a/tests/test_core/test_collector_printers.py +++ b/tests/test_core/test_collector_printers.py @@ -1,11 +1,19 @@ -"""Tests for the computers collector PC->printer relationship sync. +"""The computers collector must NEVER write a printer assignment. -A collector payload carrying defaultprinter/printers creates AssetRelationship -rows linking the PC asset to the resolved printer assets ('defaultprinter' for -the default, 'connectedto' for the rest). Stale collector links archive on -re-report; manual links survive; unresolved identifiers warn without failing. +This file used to pin the opposite: `_sync_printer_links` turned a payload's +`defaultprinter`/`printers` into AssetRelationship rows. That contradicted the +rule the printer feature rests on - ASSIGNED and OBSERVED never mix - because a +bay that installed the wrong printer would have made itself right just by +reporting it. + +It was not dormant. The collector schema advertised both keys and +COLLECTOR-INTEGRATION.md's example client set `defaultprinter`, while another +page of the same document promised nothing on this path writes an assignment +row. Retired 2026-08-24; these tests are the guard that it stays retired. + +Observed queues have their own path (POST /api/collector/printers) and become +assignments only when a person asks, via seed-from-observed. """ - import pytest from shopdb.core.models import ( @@ -81,119 +89,75 @@ def _active_links(db, pcid): sourceassetid=pcid, isactive=True).all() -def test_creates_default_and_connected_links(client, db, collector_key, scene): - """Default -> defaultprinter, others -> connectedto, resolved by three id forms.""" - resp = _post(client, { - 'hostname': 'WJPC-P1', - 'defaultprinter': 'HP-Materials', # windowsname - 'printers': ['prn-front.wjs.local', '10.1.2.3'], # hostname, IP - }) - assert resp.status_code == 200, resp.get_json() - data = resp.get_json()['data'] - assert data['printerlinkcount'] == 3 - assert not data['warnings'] - - pc = _pc_asset(db) - links = {(r.targetassetid, r.relationshiptype.relationshiptype) - for r in _active_links(db, pc.assetid)} - assert (scene['a'].assetid, 'defaultprinter') in links - assert (scene['b'].assetid, 'connectedto') in links - assert (scene['c'].assetid, 'connectedto') in links - - -def test_unresolved_identifier_warns_not_fails(client, db, collector_key, scene): - """An unknown printer identifier is a warning, the rest still link.""" - resp = _post(client, { - 'hostname': 'WJPC-P2', +def test_printer_keys_create_no_relationships(client, db, collector_key, scene): + """The reported case, inverted: sending them must change nothing.""" + response = _post(client, { + 'hostname': 'BAY-3105-PC', 'defaultprinter': 'HP-Materials', - 'printers': ['does-not-exist'], + 'printers': ['HP-Materials', 'prn-front.wjs.local', '10.1.2.3'], }) - assert resp.status_code == 200 - data = resp.get_json()['data'] - assert any('unresolved printer: does-not-exist' in w - for w in data['warnings']) - pc = _pc_asset(db) - assert data['printerlinkcount'] == 1 - assert len(_active_links(db, pc.assetid)) == 1 + assert response.status_code in (200, 201), response.get_json() + + pcid = _pc_asset(db).assetid + assert _active_links(db, pcid) == [], ( + 'a collector report wrote printer relationships') -def test_idempotent_no_duplicates(client, db, collector_key, scene): - """Re-report the same set: no new rows, still one link per printer.""" - payload = {'hostname': 'WJPC-P3', 'defaultprinter': 'HP-Materials', - 'printers': ['Zebra-Ship']} - assert _post(client, payload).status_code == 200 - assert _post(client, payload).status_code == 200 +def test_the_push_still_succeeds_and_says_why(client, db, collector_key, scene): + """Warn, do not reject. - pc = _pc_asset(db) - all_rows = AssetRelationship.query.filter_by(sourceassetid=pc.assetid).all() - assert len(all_rows) == 2 # one default + one connected, no dupes - assert all(r.isactive for r in all_rows) + Unknown keys are not validated, so a client written against the old docs + keeps pushing successfully - only its printer keys stop doing anything. + Silently dropping them would leave an integrator believing assignments were + still being written, which is worse than the original bug. + """ + response = _post(client, { + 'hostname': 'BAY-3105-PC', + 'defaultprinter': 'HP-Materials', + }) + assert response.status_code in (200, 201) + warnings = ' '.join(response.get_json()['data']['warnings']).lower() + assert 'defaultprinter' in warnings + assert 'seed-from-observed' in warnings, ( + 'the warning should say where assignment actually comes from') -def test_dropped_printer_is_archived(client, db, collector_key, scene): - """A printer that stops being reported loses its collector link.""" - _post(client, {'hostname': 'WJPC-P4', 'defaultprinter': 'HP-Materials', - 'printers': ['Zebra-Ship']}) - pc = _pc_asset(db) - assert len(_active_links(db, pc.assetid)) == 2 - - # Re-report without Zebra-Ship. - _post(client, {'hostname': 'WJPC-P4', 'defaultprinter': 'HP-Materials', - 'printers': []}) - active = _active_links(db, pc.assetid) - assert len(active) == 1 - assert active[0].targetassetid == scene['a'].assetid - # the dropped row still exists but is inactive - zebra = AssetRelationship.query.filter_by( - sourceassetid=pc.assetid, targetassetid=scene['c'].assetid).first() - assert zebra is not None and zebra.isactive is False +def test_a_payload_without_printer_keys_warns_about_nothing( + client, db, collector_key, scene): + """The normal case must stay quiet - this is every push in the fleet.""" + response = _post(client, {'hostname': 'BAY-3105-PC'}) + assert response.status_code in (200, 201) + warnings = ' '.join(response.get_json()['data']['warnings']).lower() + assert 'printer' not in warnings -def test_default_change_archives_old_default(client, db, collector_key, scene): - """Changing the default printer archives the previous defaultprinter link.""" - _post(client, {'hostname': 'WJPC-P5', 'defaultprinter': 'HP-Materials'}) - pc = _pc_asset(db) - _post(client, {'hostname': 'WJPC-P5', 'defaultprinter': 'prn-front.wjs.local'}) - - active = _active_links(db, pc.assetid) - assert len(active) == 1 - assert active[0].targetassetid == scene['b'].assetid - assert active[0].relationshiptype.relationshiptype == 'defaultprinter' - - -def test_manual_link_survives_collector_sync(client, db, collector_key, scene): - """A manually-created link (no collector label) is never archived.""" - # PC created first via a bare report. - _post(client, {'hostname': 'WJPC-P6', 'defaultprinter': 'HP-Materials', - 'printers': ['Zebra-Ship']}) - pc = _pc_asset(db) - - # Manual connectedto link PC -> printer B (label stays NULL). +def test_a_manual_assignment_survives_a_report_that_names_printers( + client, db, collector_key, scene): + """The row a person made is the one that matters, and it stays untouched.""" + _post(client, {'hostname': 'BAY-3105-PC'}) + pcasset = _pc_asset(db) manual = AssetRelationship( - sourceassetid=pc.assetid, targetassetid=scene['b'].assetid, - relationshiptypeid=scene['ct_type'].relationshiptypeid, notes='by hand') + sourceassetid=pcasset.assetid, targetassetid=scene['a'].assetid, + relationshiptypeid=scene['dp_type'].relationshiptypeid) db.session.add(manual) db.session.commit() - manual_id = manual.relationshipid - # Re-report dropping Zebra-Ship and never mentioning printer B. - _post(client, {'hostname': 'WJPC-P6', 'defaultprinter': 'HP-Materials', - 'printers': []}) + _post(client, { + 'hostname': 'BAY-3105-PC', + 'defaultprinter': 'prn-front.wjs.local', # a DIFFERENT printer + 'printers': ['prn-front.wjs.local'], + }) - surviving = db.session.get(AssetRelationship, manual_id) - assert surviving.isactive is True # manual link untouched - # Zebra (collector) archived. - zebra = AssetRelationship.query.filter_by( - sourceassetid=pc.assetid, targetassetid=scene['c'].assetid).first() - assert zebra.isactive is False + links = _active_links(db, pcasset.assetid) + assert len(links) == 1 + assert links[0].targetassetid == scene['a'].assetid, ( + 'the report retargeted a human-made default printer') -def test_no_printer_keys_leaves_links_untouched(client, db, collector_key, scene): - """A report with no printer keys does not archive existing links.""" - _post(client, {'hostname': 'WJPC-P7', 'defaultprinter': 'HP-Materials'}) - pc = _pc_asset(db) - assert len(_active_links(db, pc.assetid)) == 1 +def test_the_retired_keys_are_gone_from_the_collector_schema(app): + """The schema advertised them, which is how integrators found them.""" + from plugins.computers.plugin import ComputersPlugin - # A plain update (no defaultprinter, no printers key) must not archive. - _post(client, {'hostname': 'WJPC-P7', 'currentuser': 'alice'}) - assert len(_active_links(db, pc.assetid)) == 1 + fields = ComputersPlugin().get_collector_schema()['fields'] + assert 'defaultprinter' not in fields + assert 'printers' not in fields