Let a PC be assigned a business unit, and a retired asset be found

The last two items from the run.

The PC detail page has always rendered a Business Unit row, and the API has
always accepted the field. Only the form had no control, so it could be read and
never set from the UI - a value that appeared the moment somebody set it through
the API and could not be reached otherwise. The form has the picker now, loaded
with listAll() rather than a capped page.

Global search excluded inactive assets, so a retired 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, which the retire dialog
promises they can. Retired assets are searchable now.

That is only safe with the ordering, so it is part of the change rather than a
follow-up: somebody searching a machine number almost always wants the machine
still on the floor, so a retired match drops 60 points of relevance, sorts below
every live one, and carries a 'retired' flag the results page renders as a
badge. Indistinguishable retired results would be worse than none.

Four tests, verified by planting the isactive filter back: three fail.

Scope: product

frontend/src/views/SearchResults.vue

@@ -83,6 +83,12 @@
                 <span v-if="result.location" class="result-location">{{ result.location }}</span>
                 <span v-if="result.ticketnumber" class="result-ticket">{{ result.ticketnumber }}</span>
                 <span v-if="result.iscurrent" class="badge badge-success">Active</span>
+                <!-- Retired assets are searchable on purpose (the retire
+                     dialog promises they can be restored, and the search box
+                     is where somebody looks for one). They rank below live
+                     assets and say so, or a retired record looks like a
+                     current one. -->
+                <span v-if="result.retired" class="badge badge-secondary">retired</span>
               </div>
             </div>
             <button

plugins/computers/frontend/views/PCForm.vue

@@ -163,6 +163,27 @@
               override that.
             </small>
           </div>
+
+          <div class="form-group">
+            <!-- The PC detail page has always rendered a Business Unit row,
+                 and the API has always accepted the field - only the form had
+                 no control, so it could be seen and never set from the UI. -->
+            <label for="businessunitid">Business Unit</label>
+            <select
+              id="businessunitid"
+              v-model="form.businessunitid"
+              class="form-control"
+            >
+              <option value="">Select business unit...</option>
+              <option
+                v-for="bu in businessunits"
+                :key="bu.businessunitid"
+                :value="bu.businessunitid"
+              >
+                {{ bu.businessunit }}
+              </option>
+            </select>
+          </div>
         </div>
 
         <div class="form-row">
@@ -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,

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

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