diff --git a/frontend/src/views/SearchResults.vue b/frontend/src/views/SearchResults.vue index 3e3d4b41..41f9985b 100644 --- a/frontend/src/views/SearchResults.vue +++ b/frontend/src/views/SearchResults.vue @@ -83,6 +83,12 @@ {{ result.location }} {{ result.ticketnumber }} Active + + retired + + + + Business Unit + + Select business unit... + + {{ bu.businessunit }} + + + @@ -374,7 +395,7 @@ import { ref, onMounted, computed, watch } from 'vue' import { useRoute, useRouter } from 'vue-router' import { computersApi, assetsApi, vendorsApi, locationsApi, modelsApi, operatingsystemsApi, - printersApi, relationshipTypesApi } from '@/api' + printersApi, relationshipTypesApi, businessunitsApi } from '@/api' import ShopFloorMap from '@/components/ShopFloorMap.vue' import { loadMapConfig, levelOptions, levelName, state as mapConfig } from '@/composables/mapConfig' @@ -488,6 +509,7 @@ function setProtocolPort(protocolid, value) { const vendors = ref([]) const models = ref([]) const locations = ref([]) +const businessunits = ref([]) const operatingsystems = ref([]) @@ -541,14 +563,16 @@ onMounted(async () => { try { // Load reference data // perpage 100 so dropdowns aren't truncated to the default 20-row page - const [ptRes, statusRes, vendorRes, allModels, locRes, osRes, protoRes] = await Promise.all([ + const [ptRes, statusRes, vendorRes, allModels, locRes, osRes, protoRes, + allBusinessUnits] = await Promise.all([ computersApi.types.list({ perpage: 100 }), assetsApi.statuses.list(), - vendorsApi.list({ perpage: 100 }), + vendorsApi.listAll(), modelsApi.listAll({ category: 'PC' }), // backend caps perpage at 100; page through all locationsApi.listAll(), // backend caps perpage at 100; page through all operatingsystemsApi.list({ perpage: 100 }), computersApi.protocols.list(), + businessunitsApi.listAll(), // Handles its own failure: a site without printers still gets a form. ]) @@ -559,6 +583,7 @@ onMounted(async () => { locations.value = locRes || [] operatingsystems.value = osRes.data.data || [] protocols.value = protoRes.data.data || [] + businessunits.value = allBusinessUnits || [] // Load PC if editing (asset-based shape: extension under pc.computer) if (isEdit.value) { @@ -586,6 +611,7 @@ onMounted(async () => { // machine, and saving wrote it back as the PC's own - so an edit that // never touched location silently stopped the PC following its parent. locationid: pc.ownlocationid || '', + businessunitid: pc.businessunitid || '', osid: ext.osid || '', loggedinuser: ext.loggedinuser || '', azuredeviceid: ext.azuredeviceid || '', @@ -664,6 +690,7 @@ async function savePC() { vendorid: form.value.vendorid || null, modelnumberid: form.value.modelnumberid || null, locationid: form.value.locationid || null, + businessunitid: form.value.businessunitid || null, osid: form.value.osid || null, loggedinuser: form.value.loggedinuser || null, azuredeviceid: form.value.azuredeviceid || null, diff --git a/shopdb/core/api/search.py b/shopdb/core/api/search.py index 3f9d7c3d..39d95412 100644 --- a/shopdb/core/api/search.py +++ b/shopdb/core/api/search.py @@ -244,6 +244,18 @@ def _get_asset_result(asset, query, relevance=None): if relevance is None: relevance = 15 + # A retired asset never outranks a live one, whatever it matched on: the + # person searching almost always wants the machine that is still on the + # floor. Flagged too, so the result can say so rather than looking like any + # other hit. + # getattr, because this builder is also called with lightweight + # stand-ins that carry only the fields a result needs. An object that does + # not say is treated as live, which is the safe direction: a live asset + # ranked correctly beats a live asset buried as retired. + retired = getattr(asset, 'isactive', True) is False + if retired: + relevance = max(1, relevance - 60) + return { 'type': asset_type_name, 'id': result_id, @@ -251,7 +263,8 @@ def _get_asset_result(asset, query, relevance=None): 'subtitle': subtitle, 'location': location_name, 'url': url, - 'relevance': relevance + 'relevance': relevance, + 'retired': retired } @@ -499,11 +512,17 @@ def _search_assets(query, search_term): joinedload(Asset.assettype), joinedload(Asset.location), ).filter( - Asset.isactive == True, + # Retired assets are INCLUDED, and ranked below live ones. Retire + # is a soft delete whose confirm dialog promises the record can be + # restored - and the one place somebody looks for a machine they + # retired last month is the search box. Excluding them meant the + # record existed, was reachable by URL, and could not be found by + # anybody who did not already know where it was. _word_match(query, Asset.assetnumber, Asset.name, Asset.serialnumber, Asset.notes, Asset.gaugelabreference, Asset.maintenancereference) - ).order_by(Asset.assetnumber, Asset.assetid).limit(15).all() + ).order_by(Asset.isactive.desc(), Asset.assetnumber, + Asset.assetid).limit(15).all() for asset in assets: relevance = 15 diff --git a/tests/test_core/test_search_retired.py b/tests/test_core/test_search_retired.py new file mode 100644 index 00000000..3c0991bd --- /dev/null +++ b/tests/test_core/test_search_retired.py @@ -0,0 +1,69 @@ +"""A retired asset has to be findable, and must never outrank a live one. + +Retire is a soft delete: the confirm dialog promises history is kept and the +record can be restored. Global search excluded inactive assets, so the record +existed, was reachable by URL, and could not be found by anybody who did not +already know where it was - including whoever wanted to restore it. + +Including them is only safe with the ordering: somebody searching a machine +number almost always wants the machine still on the floor, so a retired match +is pushed below every live one and flagged, rather than being mixed in +indistinguishably. +""" +import pytest + + +@pytest.fixture +def assets(db): + from shopdb.core.models import Asset, AssetType + + assettype = AssetType(assettype='machine') + db.session.add(assettype) + db.session.flush() + + live = Asset(assetnumber='SR-1000', name='Live grinder', + assettypeid=assettype.assettypeid) + gone = Asset(assetnumber='SR-1001', name='Retired grinder', + assettypeid=assettype.assettypeid, isactive=False) + db.session.add_all([live, gone]) + db.session.commit() + return live, gone + + +def _search(client, headers, term): + response = client.get('/api/search?q=%s' % term, headers=headers) + assert response.status_code == 200, response.get_json() + payload = response.get_json()['data'] + return payload.get('results', payload if isinstance(payload, list) else []) + + +def test_a_retired_asset_can_be_found(client, db, auth_headers, assets): + _live, gone = assets + + titles = [r['title'] for r in _search(client, auth_headers, 'SR-1001')] + assert any('Retired grinder' in title or 'SR-1001' in title + for title in titles), titles + + +def test_a_retired_result_says_it_is_retired(client, db, auth_headers, assets): + results = _search(client, auth_headers, 'SR-1001') + match = next(r for r in results + if 'SR-1001' in (r['title'] + (r.get('subtitle') or ''))) + assert match['retired'] is True + + +def test_a_live_asset_outranks_a_retired_one(client, db, auth_headers, assets): + """Both match 'grinder'; the one still on the floor comes first.""" + results = [r for r in _search(client, auth_headers, 'grinder') + if r.get('retired') is not None] + assert len(results) >= 2, results + + live = next(r for r in results if not r['retired']) + retired = next(r for r in results if r['retired']) + assert live['relevance'] > retired['relevance'], (live, retired) + + +def test_a_live_asset_is_not_flagged(client, db, auth_headers, assets): + results = _search(client, auth_headers, 'SR-1000') + match = next(r for r in results if 'SR-1000' in (r['title'] + (r.get('subtitle') or ''))) + assert match['retired'] is False