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) == []