From b16f14346726aa5add8f50530e7ffe926df5ed4d Mon Sep 17 00:00:00 2001 From: cproudlock Date: Fri, 31 Jul 2026 09:02:51 -0400 Subject: [PATCH] search: asset lists search the type column they display Every asset list shows a Type column (and printers a Model, machines and network a Vendor), but the search filters only looked at the asset number, name, serial and hostname. Searching a type returned zero rows: 'Part Washer' on machines, 'Standard' on PCs, 'Thermal' on printers. Extend the search on machines, computers, printers, network devices, measuring tools and the unified asset list to cover the type name plus the vendor/model where the list shows them. Joins are outer joins so an asset missing a type or vendor still matches on its own fields; the core list uses a correlated EXISTS instead, since its type-name filter already joins AssetType. --- plugins/computers/api/routes.py | 18 +- plugins/machines/api/routes.py | 22 ++- plugins/measuringtools/api/routes.py | 15 +- plugins/network/api/routes.py | 22 ++- plugins/printers/api/asset_routes.py | 23 ++- shopdb/core/api/assets.py | 12 +- .../test_list_search_type_columns.py | 166 ++++++++++++++++++ tests/test_plugins/test_machines_search.py | 95 ++++++++++ 8 files changed, 340 insertions(+), 33 deletions(-) create mode 100644 tests/test_plugins/test_list_search_type_columns.py create mode 100644 tests/test_plugins/test_machines_search.py diff --git a/plugins/computers/api/routes.py b/plugins/computers/api/routes.py index ba35708..aac1eb3 100644 --- a/plugins/computers/api/routes.py +++ b/plugins/computers/api/routes.py @@ -363,14 +363,20 @@ def list_computers(): if exactassetnumber := request.args.get('assetnumber'): query = query.filter(Asset.assetnumber == exactassetnumber) - # Search filter + # Search filter. Covers the type name too - it is a column in the list, so + # searching 'Standard' must find the PCs of that type. Outer join so a PC + # with no type still matches on its own fields. if search := request.args.get('search'): - query = query.filter( + pattern = f'%{search}%' + query = query.outerjoin( + ComputerType, Computer.computertypeid == ComputerType.computertypeid + ).filter( db.or_( - Asset.assetnumber.ilike(f'%{search}%'), - Asset.name.ilike(f'%{search}%'), - Asset.serialnumber.ilike(f'%{search}%'), - Computer.hostname.ilike(f'%{search}%') + Asset.assetnumber.ilike(pattern), + Asset.name.ilike(pattern), + Asset.serialnumber.ilike(pattern), + Computer.hostname.ilike(pattern), + ComputerType.computertype.ilike(pattern) ) ) diff --git a/plugins/machines/api/routes.py b/plugins/machines/api/routes.py index f309e32..a44bf6c 100644 --- a/plugins/machines/api/routes.py +++ b/plugins/machines/api/routes.py @@ -3,7 +3,7 @@ from flask import Blueprint, request from flask_jwt_extended import jwt_required -from shopdb.api import db, Asset, AssetType, AuditLog, success_response, error_response, paginated_response, ErrorCodes, get_pagination_params, paginate_query, resolve_dualpath_pairs, dualpath_single_machine_enabled +from shopdb.api import db, Asset, AssetType, AuditLog, Vendor, success_response, error_response, paginated_response, ErrorCodes, get_pagination_params, paginate_query, resolve_dualpath_pairs, dualpath_single_machine_enabled from ..models import Machine, MachineType @@ -174,13 +174,23 @@ def list_machines(): if exactassetnumber := request.args.get('assetnumber'): query = query.filter(Asset.assetnumber == exactassetnumber) - # Search filter + # Search filter. Covers what the list actually SHOWS - the asset fields plus + # the type and vendor names, which are columns in the table. Searching a type + # like 'Part Washer' used to return nothing. Outer joins so a machine with no + # type or vendor still matches on its own fields. if search := request.args.get('search'): - query = query.filter( + pattern = f'%{search}%' + query = query.outerjoin( + MachineType, Machine.machinetypeid == MachineType.machinetypeid + ).outerjoin( + Vendor, Machine.vendorid == Vendor.vendorid + ).filter( db.or_( - Asset.assetnumber.ilike(f'%{search}%'), - Asset.name.ilike(f'%{search}%'), - Asset.serialnumber.ilike(f'%{search}%') + Asset.assetnumber.ilike(pattern), + Asset.name.ilike(pattern), + Asset.serialnumber.ilike(pattern), + MachineType.machinetype.ilike(pattern), + Vendor.vendor.ilike(pattern) ) ) diff --git a/plugins/measuringtools/api/routes.py b/plugins/measuringtools/api/routes.py index 89c6fc6..ab69583 100644 --- a/plugins/measuringtools/api/routes.py +++ b/plugins/measuringtools/api/routes.py @@ -168,11 +168,18 @@ def list_tools(): # Exact-match natural-key lookup for idempotent import (asset number). if exactassetnumber := request.args.get('assetnumber'): query = query.filter(Asset.assetnumber == exactassetnumber) + # Type is a column in the list, so it has to be searchable. Outer join so a + # tool with no type still matches on its own fields. if search := request.args.get('search'): - query = query.filter(db.or_( - Asset.assetnumber.ilike(f'%{search}%'), - Asset.name.ilike(f'%{search}%'), - Asset.serialnumber.ilike(f'%{search}%'), + pattern = f'%{search}%' + query = query.outerjoin( + MeasuringToolType, + MeasuringTool.measuringtooltypeid == MeasuringToolType.measuringtooltypeid + ).filter(db.or_( + Asset.assetnumber.ilike(pattern), + Asset.name.ilike(pattern), + Asset.serialnumber.ilike(pattern), + MeasuringToolType.name.ilike(pattern), )) if type_id := request.args.get('typeid', type=int): query = query.filter(MeasuringTool.measuringtooltypeid == type_id) diff --git a/plugins/network/api/routes.py b/plugins/network/api/routes.py index 751ae65..46516b3 100644 --- a/plugins/network/api/routes.py +++ b/plugins/network/api/routes.py @@ -201,14 +201,24 @@ def list_network_devices(): if exactassetnumber := request.args.get('assetnumber'): query = query.filter(Asset.assetnumber == exactassetnumber) - # Search filter + # Search filter. Type and vendor are columns in the list, so both must be + # searchable. Outer joins so a device missing either still matches on its + # own fields. if search := request.args.get('search'): - query = query.filter( + pattern = f'%{search}%' + query = query.outerjoin( + NetworkDeviceType, + NetworkDevice.networkdevicetypeid == NetworkDeviceType.networkdevicetypeid + ).outerjoin( + Vendor, NetworkDevice.vendorid == Vendor.vendorid + ).filter( db.or_( - Asset.assetnumber.ilike(f'%{search}%'), - Asset.name.ilike(f'%{search}%'), - Asset.serialnumber.ilike(f'%{search}%'), - NetworkDevice.hostname.ilike(f'%{search}%') + Asset.assetnumber.ilike(pattern), + Asset.name.ilike(pattern), + Asset.serialnumber.ilike(pattern), + NetworkDevice.hostname.ilike(pattern), + NetworkDeviceType.networkdevicetype.ilike(pattern), + Vendor.vendor.ilike(pattern) ) ) diff --git a/plugins/printers/api/asset_routes.py b/plugins/printers/api/asset_routes.py index c5b52f6..e951e7e 100644 --- a/plugins/printers/api/asset_routes.py +++ b/plugins/printers/api/asset_routes.py @@ -240,15 +240,24 @@ def list_printers(): if exactassetnumber := request.args.get('assetnumber'): query = query.filter(Asset.assetnumber == exactassetnumber) - # Search filter + # Search filter. Type and model are columns in the list, so searching + # 'Thermal' must find the thermal printers. Outer joins so a printer missing + # either still matches on its own fields. if search := request.args.get('search'): - query = query.filter( + pattern = f'%{search}%' + query = query.outerjoin( + PrinterType, Printer.printertypeid == PrinterType.printertypeid + ).outerjoin( + Model, Printer.modelnumberid == Model.modelnumberid + ).filter( db.or_( - Asset.assetnumber.ilike(f'%{search}%'), - Asset.name.ilike(f'%{search}%'), - Asset.serialnumber.ilike(f'%{search}%'), - Printer.hostname.ilike(f'%{search}%'), - Printer.windowsname.ilike(f'%{search}%') + Asset.assetnumber.ilike(pattern), + Asset.name.ilike(pattern), + Asset.serialnumber.ilike(pattern), + Printer.hostname.ilike(pattern), + Printer.windowsname.ilike(pattern), + PrinterType.printertype.ilike(pattern), + Model.modelnumber.ilike(pattern) ) ) diff --git a/shopdb/core/api/assets.py b/shopdb/core/api/assets.py index 2403765..4081eff 100644 --- a/shopdb/core/api/assets.py +++ b/shopdb/core/api/assets.py @@ -354,13 +354,17 @@ def list_assets(): if request.args.get('active', 'true').lower() != 'false': query = query.filter(Asset.isactive == True) - # Search filter + # Search filter. The asset type is a column in the list, so it has to be + # searchable. Matched via .has() (a correlated EXISTS) rather than a join, + # because the type-name filter below joins AssetType itself. if search := request.args.get('search'): + pattern = f'%{search}%' query = query.filter( db.or_( - Asset.assetnumber.ilike(f'%{search}%'), - Asset.name.ilike(f'%{search}%'), - Asset.serialnumber.ilike(f'%{search}%') + Asset.assetnumber.ilike(pattern), + Asset.name.ilike(pattern), + Asset.serialnumber.ilike(pattern), + Asset.assettype.has(AssetType.assettype.ilike(pattern)) ) ) diff --git a/tests/test_plugins/test_list_search_type_columns.py b/tests/test_plugins/test_list_search_type_columns.py new file mode 100644 index 0000000..1b7f7e9 --- /dev/null +++ b/tests/test_plugins/test_list_search_type_columns.py @@ -0,0 +1,166 @@ +"""Every asset list searches the type (and vendor/model) columns it displays. + +Each list shows a Type column, and printers/network also show Model/Vendor, but +the search filters only looked at the asset fields - so searching 'Standard' on +PCs or 'Thermal' on printers returned nothing. One test per list, each also +proving the outer joins did not turn into inner joins. +""" + +from shopdb.extensions import db as _db +from shopdb.core.models import Asset, AssetType +from shopdb.core.models.model import Model +from shopdb.core.models.vendor import Vendor + +from plugins.computers.models import Computer, ComputerType +from plugins.printers.models import Printer, PrinterType +from plugins.network.models import NetworkDevice, NetworkDeviceType +from plugins.measuringtools.models import MeasuringTool, MeasuringToolType + + +def _assettype(name): + assettype = AssetType(assettype=name) + _db.session.add(assettype) + _db.session.flush() + return assettype + + +def _numbers(resp): + return sorted(item['assetnumber'] for item in resp.get_json()['data']) + + +def test_pcs_search_matches_computer_type(client, db): + assettype = _assettype('computer') + standard = ComputerType(computertype='Standard') + _db.session.add(standard) + _db.session.flush() + + typed = Asset(assetnumber='PC-1', assettypeid=assettype.assettypeid) + untyped = Asset(assetnumber='PC-2', name='spare bench box', + assettypeid=assettype.assettypeid) + _db.session.add_all([typed, untyped]) + _db.session.flush() + _db.session.add_all([ + Computer(assetid=typed.assetid, computertypeid=standard.computertypeid), + Computer(assetid=untyped.assetid), + ]) + _db.session.commit() + + resp = client.get('/api/computers?search=Standard') + assert resp.status_code == 200, resp.get_json() + assert _numbers(resp) == ['PC-1'] + + # the typeless PC is still findable on its own fields + resp = client.get('/api/computers?search=bench') + assert _numbers(resp) == ['PC-2'] + + +def test_printers_search_matches_type_and_model(client, db): + assettype = _assettype('printer') + thermal = PrinterType(printertype='Thermal') + model = Model(modelnumber='ZT411') + _db.session.add_all([thermal, model]) + _db.session.flush() + + labeler = Asset(assetnumber='PR-1', assettypeid=assettype.assettypeid) + plain = Asset(assetnumber='PR-2', name='old plotter', + assettypeid=assettype.assettypeid) + _db.session.add_all([labeler, plain]) + _db.session.flush() + _db.session.add_all([ + Printer(assetid=labeler.assetid, printertypeid=thermal.printertypeid, + modelnumberid=model.modelnumberid), + Printer(assetid=plain.assetid), + ]) + _db.session.commit() + + for term in ['Thermal', 'ZT411']: + resp = client.get(f'/api/printers?search={term}') + assert resp.status_code == 200, resp.get_json() + assert _numbers(resp) == ['PR-1'], term + + resp = client.get('/api/printers?search=plotter') + assert _numbers(resp) == ['PR-2'] + + +def test_network_search_matches_type_and_vendor(client, db): + assettype = _assettype('network_device') + switchtype = NetworkDeviceType(networkdevicetype='Switch') + vendor = Vendor(vendor='Cisco') + _db.session.add_all([switchtype, vendor]) + _db.session.flush() + + switch = Asset(assetnumber='NET-1', assettypeid=assettype.assettypeid) + plain = Asset(assetnumber='NET-2', name='unmanaged hub', + assettypeid=assettype.assettypeid) + _db.session.add_all([switch, plain]) + _db.session.flush() + _db.session.add_all([ + NetworkDevice(assetid=switch.assetid, + networkdevicetypeid=switchtype.networkdevicetypeid, + vendorid=vendor.vendorid), + NetworkDevice(assetid=plain.assetid), + ]) + _db.session.commit() + + for term in ['Switch', 'Cisco']: + resp = client.get(f'/api/network?search={term}') + assert resp.status_code == 200, resp.get_json() + assert _numbers(resp) == ['NET-1'], term + + resp = client.get('/api/network?search=hub') + assert _numbers(resp) == ['NET-2'] + + +def test_measuringtools_search_matches_type(client, db): + assettype = _assettype('measuring_tool') + caliper = MeasuringToolType(name='Caliper') + _db.session.add(caliper) + _db.session.flush() + + typed = Asset(assetnumber='MT-1', assettypeid=assettype.assettypeid) + untyped = Asset(assetnumber='MT-2', name='height gage', + assettypeid=assettype.assettypeid) + _db.session.add_all([typed, untyped]) + _db.session.flush() + _db.session.add_all([ + MeasuringTool(assetid=typed.assetid, + measuringtooltypeid=caliper.measuringtooltypeid), + MeasuringTool(assetid=untyped.assetid), + ]) + _db.session.commit() + + resp = client.get('/api/measuringtools?search=Caliper') + assert resp.status_code == 200, resp.get_json() + assert _numbers(resp) == ['MT-1'] + + resp = client.get('/api/measuringtools?search=height') + assert _numbers(resp) == ['MT-2'] + + +def test_core_assets_search_matches_asset_type(client, db): + """The unified asset list shows a Type column too.""" + machine_type = _assettype('machine') + printer_type = _assettype('printer') + _db.session.add_all([ + Asset(assetnumber='A-1', assettypeid=machine_type.assettypeid), + Asset(assetnumber='A-2', assettypeid=printer_type.assettypeid), + ]) + _db.session.commit() + + resp = client.get('/api/assets?search=machine') + assert resp.status_code == 200, resp.get_json() + assert _numbers(resp) == ['A-1'] + + +def test_core_assets_search_type_filter_still_works(client, db): + """The type-name filter joins AssetType; search uses EXISTS, so both apply.""" + machine_type = _assettype('machine') + _db.session.add_all([ + Asset(assetnumber='A-1', name='mill', assettypeid=machine_type.assettypeid), + Asset(assetnumber='A-2', name='lathe', assettypeid=machine_type.assettypeid), + ]) + _db.session.commit() + + resp = client.get('/api/assets?type=machine&search=mill') + assert resp.status_code == 200, resp.get_json() + assert _numbers(resp) == ['A-1'] diff --git a/tests/test_plugins/test_machines_search.py b/tests/test_plugins/test_machines_search.py new file mode 100644 index 0000000..1ef0537 --- /dev/null +++ b/tests/test_plugins/test_machines_search.py @@ -0,0 +1,95 @@ +"""Machines list search covers the columns the list actually shows. + +The table displays Machine #, Name, Serial Number, Type and Vendor, so all of +them must be searchable. Searching a type name ('Part Washer') used to return +zero rows because the filter only looked at the asset fields. +""" + +from shopdb.extensions import db as _db +from shopdb.core.models import Asset, AssetType +from shopdb.core.models.vendor import Vendor + +from plugins.machines.models import Machine, MachineType + + +def _seed_machines(): + """Two machines: one Part Washer by Acme, one Lathe with no type or vendor. + + The typeless machine guards the outer joins - it must still be findable by + its own asset fields. + """ + assettype = AssetType(assettype='machine') + _db.session.add(assettype) + _db.session.flush() + + washertype = MachineType(machinetype='Part Washer') + lathe_vendor = Vendor(vendor='Acme Industrial') + _db.session.add_all([washertype, lathe_vendor]) + _db.session.flush() + + washer_asset = Asset(assetnumber='0410', name='Cell 4 washer', + serialnumber='SN-WASH-1', assettypeid=assettype.assettypeid) + plain_asset = Asset(assetnumber='0999', name='Old lathe', + serialnumber='SN-LATHE-9', assettypeid=assettype.assettypeid) + _db.session.add_all([washer_asset, plain_asset]) + _db.session.flush() + + _db.session.add_all([ + Machine(assetid=washer_asset.assetid, + machinetypeid=washertype.machinetypeid, + vendorid=lathe_vendor.vendorid), + Machine(assetid=plain_asset.assetid), + ]) + _db.session.commit() + + +def _numbers(resp): + return sorted(item['assetnumber'] for item in resp.get_json()['data']) + + +def test_search_matches_machine_type_name(client, db): + _seed_machines() + + resp = client.get('/api/machines?search=Part Washer') + + assert resp.status_code == 200, resp.get_json() + assert _numbers(resp) == ['0410'] + + +def test_search_matches_vendor_name(client, db): + _seed_machines() + + resp = client.get('/api/machines?search=Acme') + + assert resp.status_code == 200, resp.get_json() + assert _numbers(resp) == ['0410'] + + +def test_search_still_matches_asset_fields(client, db): + _seed_machines() + + for term, expected in [('0999', ['0999']), + ('Old lathe', ['0999']), + ('SN-WASH', ['0410'])]: + resp = client.get(f'/api/machines?search={term}') + assert resp.status_code == 200, resp.get_json() + assert _numbers(resp) == expected, term + + +def test_search_does_not_drop_machines_without_type_or_vendor(client, db): + """The outer joins must not turn into inner joins.""" + _seed_machines() + + resp = client.get('/api/machines?search=lathe') + + assert resp.status_code == 200, resp.get_json() + assert _numbers(resp) == ['0999'] + + +def test_search_miss_returns_empty(client, db): + _seed_machines() + + resp = client.get('/api/machines?search=zzzznotathing') + + assert resp.status_code == 200, resp.get_json() + assert _numbers(resp) == []