Show the controls link from both ends, and fix the dead chip links
All checks were successful
CI / naming (push) Successful in 4s
CI / frontend (push) Successful in 13s
CI / migrations-mysql (push) Successful in 41s
CI / backend (push) Successful in 4m7s

THE RECIPROCAL LINKS BUG

A PC's page showed "Serves machine 7803" at the top. The machine's page showed
nothing equivalent, though the same edge was right there in the relationships
card at the bottom. That asymmetry read as missing data and was reported as
such.

It was neither missing data nor an API fault. The endpoint returns both
directions and always did; AssetTriageBand already walked outgoing AND incoming
and deduped, with a comment explaining that production holds reciprocal rows.
It then discarded every peer whose asset type was not 'machine', and hardcoded
the label "Serves machine". So the component could only ever work from the PC
side, and it was mounted only on the PC page.

The counterpart type and label are now props, defaulting to the old values so
the PC page is unchanged. The machine page mounts the band with
counterparttype="computer" and the label "Controlled by".

Eliminated with evidence rather than assumed, because this bug had already been
misdiagnosed twice: the endpoint runs two queries and returns both arrays; the
generic card renders both; that route has no pagination to clamp; and a single
permission gate cannot admit one direction and not the other.

THE RELATIONSHIP TYPE WAS ALSO HARDCODED

Not every asset pair is linked by 'controls'. A machine reaches a printer
through usesprinter and defaultprinter, so the band was blank on the printer
page - the one page where the link is not obvious from the asset itself. The
types are now a prop, and the printer page passes its own.

THE CHIPS WERE DEAD LINKS

Found while adopting the band more widely. It linked to /assets/<assetid>.
There is no /assets/:id route and no catch-all, so every chip in the band was
dead - and this change would have propagated that to four more pages.

AssetRelationships.vue already resolved this correctly and privately. That
function is now frontend/src/utils/assetRoute.js and both components use it, so
a third caller cannot get it wrong again. Two things it gets right that are easy
to miss: the detail routes key on pluginid, not assetid, so passing assetid
opens the wrong record; and the lookup is now case-insensitive, because
csvimport.py seeds 'Computer' and 'Machine' capitalised while this site's
database holds them lowercase - a case-sensitive match produced dead links at
whichever site seeded the other way.

The band now carries pluginid and assettypename on each chip, which the
resolver needs.

TESTS

assetRoute had none and neither did the band. Seven cover the resolver,
including the assetid-vs-pluginid confusion and the seeding-case difference.

On the backend, nothing asserted that the TARGET of a directional edge receives
it in `incoming` - only that rows carry isdirectional. That missing assertion is
this bug's exact shape, so it is now pinned: the endpoint was right all along,
and the next such report should not cost anyone a re-diagnosis of the API.

Verified on screen: the machine page shows "Controlled by G6K76CW3ESF" in the
hero, the PC page is unchanged, and a page with no such link renders no empty
band. Full suite 2162 passed, frontend 280 passed.
This commit is contained in:
cproudlock
2026-08-29 18:58:06 -04:00
parent 70e909054f
commit c22c81afa5
9 changed files with 229 additions and 32 deletions

View File

@@ -186,6 +186,7 @@
</template> </template>
<script setup> <script setup>
import { assetRoute } from '@/utils/assetRoute'
import { ref, computed, onMounted, watch } from 'vue' import { ref, computed, onMounted, watch } from 'vue'
import { Cog, Monitor, Printer, Globe, Package, Ruler } from 'lucide-vue-next' import { Cog, Monitor, Printer, Globe, Package, Ruler } from 'lucide-vue-next'
import { assetsApi, relationshipTypesApi } from '../api' import { assetsApi, relationshipTypesApi } from '../api'
@@ -472,28 +473,7 @@ function getAssetIcon(assettype) {
return icons[assettype] || Package return icons[assettype] || Package
} }
function getAssetRoute(asset) { const getAssetRoute = assetRoute
if (!asset) return '#'
const routeMap = {
'machine': '/machines',
'computer': '/pcs',
'printer': '/printers',
'network_device': '/network',
'measuring_tool': '/measuringtools'
}
// serializer sends assettypename; old shape used assettype
const typename = asset.assettypename || asset.assettype
const basePath = routeMap[typename] || '/assets'
// pluginid is the extension-table id the detail routes key on
const id = asset.pluginid
|| asset.typedata?.networkdeviceid
|| asset.typedata?.machineid
|| asset.assetid
return `${basePath}/${id}`
}
</script> </script>
<style scoped> <style scoped>

View File

@@ -8,13 +8,13 @@
</div> </div>
<div v-if="machines.length" class="triage-item"> <div v-if="machines.length" class="triage-item">
<span class="triage-label">{{ machines.length === 1 ? 'Serves machine' : 'Serves machines' }}</span> <span class="triage-label">{{ counterpartheading }}</span>
<span class="triage-values"> <span class="triage-values">
<router-link <router-link
v-for="machine in machines" v-for="machine in machines"
:key="machine.assetid" :key="machine.assetid"
class="triage-chip link" class="triage-chip link"
:to="`/assets/${machine.assetid}`" :to="assetRoute(machine)"
>{{ machine.assetnumber || machine.name }}</router-link> >{{ machine.assetnumber || machine.name }}</router-link>
</span> </span>
</div> </div>
@@ -63,6 +63,7 @@
*/ */
import { ref, computed, onMounted, watch, useSlots } from 'vue' import { ref, computed, onMounted, watch, useSlots } from 'vue'
import api from '@/api' import api from '@/api'
import { assetRoute } from '@/utils/assetRoute'
const props = defineProps({ const props = defineProps({
assetid: { type: Number, required: true }, assetid: { type: Number, required: true },
@@ -71,11 +72,30 @@ const props = defineProps({
// Shape: { level: 'ok'|'warning'|'critical'|'unknown', label: 'Reporting, 4m ago' } // Shape: { level: 'ok'|'warning'|'critical'|'unknown', label: 'Reporting, 4m ago' }
liveness: { type: Object, default: null }, liveness: { type: Object, default: null },
accessmethods: { type: Array, default: () => [] }, accessmethods: { type: Array, default: () => [] },
locationname: { type: String, default: '' } locationname: { type: String, default: '' },
// Which side of a 'controls' edge this page wants to see, by asset type
// name. A PC page wants the machines it serves; a machine page wants the
// PCs that drive it. Defaults to 'machine' so the PC page - the only
// caller when this was written - keeps its exact behaviour.
counterparttype: { type: String, default: 'machine' },
// Overrides the heading. Empty keeps the shop-floor phrasing that earned
// its place: a PC matters because a machine stops without it.
counterpartlabel: { type: String, default: '' },
// Which relationship types link this asset to its counterpart. Not every
// pairing is 'controls': a PC controls a machine, but a machine reaches a
// printer through usesprinter and defaultprinter. Hardcoding 'controls'
// left the band blank on the printer page, which is the one page where the
// link is not obvious from the asset itself.
relationshiptypes: { type: Array, default: () => ['controls'] }
}) })
const machines = ref([]) const machines = ref([])
const counterpartheading = computed(() => {
if (props.counterpartlabel) return props.counterpartlabel
return machines.value.length === 1 ? 'Serves machine' : 'Serves machines'
})
const slots = useSlots() const slots = useSlots()
const hasAnything = computed(() => const hasAnything = computed(() =>
@@ -87,7 +107,7 @@ const hasAnything = computed(() =>
) )
onMounted(loadMachines) onMounted(loadMachines)
watch(() => props.assetid, loadMachines) watch(() => [props.assetid, props.counterparttype, props.relationshiptypes], loadMachines)
async function loadMachines() { async function loadMachines() {
machines.value = [] machines.value = []
@@ -96,9 +116,11 @@ async function loadMachines() {
const response = await api.get(`/assets/${props.assetid}/relationships`) const response = await api.get(`/assets/${props.assetid}/relationships`)
const data = response.data.data || {} const data = response.data.data || {}
const found = [] const found = []
// BOTH directions. The canonical edge is PC controls machine, but // BOTH directions, then deduped. Production holds reciprocal rows for
// production also holds reciprocal rows, so reading one direction shows // the same link, so reading one direction shows it on half the pages and
// the link on half the pages and not the other half. // not the other half - and reading both without deduping shows it twice.
// Direction is therefore not trustworthy here and is deliberately not
// used to label anything.
for (const edge of data.outgoing || []) collect(edge, edge.targetasset, found) for (const edge of data.outgoing || []) collect(edge, edge.targetasset, found)
for (const edge of data.incoming || []) collect(edge, edge.sourceasset, found) for (const edge of data.incoming || []) collect(edge, edge.sourceasset, found)
machines.value = found machines.value = found
@@ -111,10 +133,19 @@ async function loadMachines() {
function collect(edge, other, found) { function collect(edge, other, found) {
if (!other) return if (!other) return
if ((edge.relationshiptypename || '').toLowerCase() !== 'controls') return const wanted = props.relationshiptypes.map(t => t.toLowerCase())
if ((other.assettypename || '').toLowerCase() !== 'machine') return if (!wanted.includes((edge.relationshiptypename || '').toLowerCase())) return
if ((other.assettypename || '').toLowerCase() !== props.counterparttype.toLowerCase()) return
if (found.some(m => m.assetid === other.assetid)) return if (found.some(m => m.assetid === other.assetid)) return
found.push({ assetid: other.assetid, assetnumber: other.assetnumber, name: other.name }) // pluginid and assettypename are what assetRoute() needs; without them the
// chip falls back to /assets/<assetid>, which is not a route.
found.push({
assetid: other.assetid,
assetnumber: other.assetnumber,
name: other.name,
pluginid: other.pluginid,
assettypename: other.assettypename
})
} }
</script> </script>

View File

@@ -0,0 +1,43 @@
/**
* Where an asset's detail page lives.
*
* Extracted from AssetRelationships.vue because a second caller appeared (the
* triage band) and got it wrong: it linked to `/assets/<assetid>`, which is not
* a route. There is no `/assets/:id` page and no catch-all, so every chip was a
* dead link. One copy, so a third caller cannot repeat it.
*
* Two ids are in play and they are not interchangeable. `assetid` is the core
* row; the detail routes key on `pluginid`, the id of the extension-table row
* (machineid, computerid, printerid). Sending assetid to /machines/:id opens
* the wrong machine, or none.
*/
// Asset type NAME to route base. The names are seed data, and seeding is not
// consistent about case: shopdb/core/services/csvimport.py seeds 'Computer'
// and 'Machine' capitalised, while this site's database holds them lowercase.
// The lookup is therefore lowercased on both sides - a case-sensitive match
// silently produced dead links at whichever site seeded the other way.
const ROUTE_BY_TYPE = {
machine: '/machines',
computer: '/pcs',
printer: '/printers',
network_device: '/network',
'network device': '/network',
measuring_tool: '/measuringtools',
'measuring tool': '/measuringtools'
}
export function assetRoute(asset) {
if (!asset) return '#'
const typename = String(asset.assettypename || asset.assettype || '').toLowerCase()
const basePath = ROUTE_BY_TYPE[typename] || '/assets'
const id = asset.pluginid
|| asset.typedata?.networkdeviceid
|| asset.typedata?.machineid
|| asset.assetid
return `${basePath}/${id}`
}
export default assetRoute

View File

@@ -0,0 +1,54 @@
import { describe, it, expect } from 'vitest'
import { assetRoute } from './assetRoute'
describe('assetRoute', () => {
// The bug this file exists to prevent: the triage band linked to
// /assets/<assetid>, and there is no /assets/:id route and no catch-all,
// so every chip was a dead link on five detail pages.
it('routes by asset type, never to /assets', () => {
expect(assetRoute({ assettypename: 'machine', pluginid: 223, assetid: 313 }))
.toBe('/machines/223')
expect(assetRoute({ assettypename: 'computer', pluginid: 279, assetid: 304 }))
.toBe('/pcs/279')
expect(assetRoute({ assettypename: 'printer', pluginid: 1, assetid: 594 }))
.toBe('/printers/1')
})
it('uses pluginid, not assetid - they are different rows', () => {
// /machines/:id keys on machineid. Passing assetid opens the wrong
// machine, or none, and looks like a data problem rather than a link bug.
const route = assetRoute({ assettypename: 'machine', pluginid: 223, assetid: 313 })
expect(route).toBe('/machines/223')
expect(route).not.toContain('313')
})
it('matches the type name whatever case the site seeded it in', () => {
// csvimport.py seeds 'Computer' and 'Machine' capitalised; this site's
// database holds them lowercase. A case-sensitive lookup produced dead
// links at whichever site seeded the other way.
expect(assetRoute({ assettypename: 'Computer', pluginid: 7 })).toBe('/pcs/7')
expect(assetRoute({ assettypename: 'MACHINE', pluginid: 8 })).toBe('/machines/8')
})
it('accepts the older assettype key as well as assettypename', () => {
expect(assetRoute({ assettype: 'printer', pluginid: 4 })).toBe('/printers/4')
})
it('falls back through typedata when pluginid is absent', () => {
expect(assetRoute({ assettypename: 'network_device', typedata: { networkdeviceid: 12 } }))
.toBe('/network/12')
expect(assetRoute({ assettypename: 'machine', typedata: { machineid: 30 } }))
.toBe('/machines/30')
})
it('returns a harmless anchor for nothing', () => {
expect(assetRoute(null)).toBe('#')
expect(assetRoute(undefined)).toBe('#')
})
it('handles both spellings of the multiword type names', () => {
expect(assetRoute({ assettypename: 'network device', pluginid: 2 })).toBe('/network/2')
expect(assetRoute({ assettypename: 'measuring_tool', pluginid: 3 })).toBe('/measuringtools/3')
expect(assetRoute({ assettypename: 'measuring tool', pluginid: 3 })).toBe('/measuringtools/3')
})
})

View File

@@ -70,6 +70,17 @@
<span class="hero-detail-value">{{ machine.locationname }}</span> <span class="hero-detail-value">{{ machine.locationname }}</span>
</div> </div>
</div> </div>
<!-- The 6am band, machine side: which PCs drive this machine. The
same fact as a PC's "Serves machine", read from the other end -
and the end somebody is standing at when the machine has
stopped. It lived only in the relationships card near the
bottom of the page. -->
<AssetTriageBand
:assetid="machine.assetid"
counterparttype="computer"
counterpartlabel="Controlled by"
/>
</div> </div>
</div> </div>
@@ -253,6 +264,7 @@ import { useRoute, useRouter } from 'vue-router'
import { machinesApi } from '@/api' import { machinesApi } from '@/api'
import { withBase } from '@/utils/basePath' import { withBase } from '@/utils/basePath'
import LocationMapTooltip from '@/components/LocationMapTooltip.vue' import LocationMapTooltip from '@/components/LocationMapTooltip.vue'
import AssetTriageBand from '@/components/AssetTriageBand.vue'
import CustomFieldsSection from '@/components/CustomFieldsSection.vue' import CustomFieldsSection from '@/components/CustomFieldsSection.vue'
import PluginAssetPanels from '@/components/PluginAssetPanels.vue' import PluginAssetPanels from '@/components/PluginAssetPanels.vue'
import AssetRelationships from '@/components/AssetRelationships.vue' import AssetRelationships from '@/components/AssetRelationships.vue'

View File

@@ -46,6 +46,15 @@
<span class="hero-detail-value">{{ tool.locationname }}</span> <span class="hero-detail-value">{{ tool.locationname }}</span>
</div> </div>
</div> </div>
<!-- The 6am band. A gage usually has no controls edge at all,
and the band renders nothing when it has nothing to say - it is
here so the five asset pages stay one shape rather than five. -->
<AssetTriageBand
:assetid="tool.assetid"
counterparttype="machine"
counterpartlabel="Used on"
/>
</div> </div>
</div> </div>
@@ -169,6 +178,7 @@ import { ref, onMounted } from 'vue'
import { useRoute, useRouter } from 'vue-router' import { useRoute, useRouter } from 'vue-router'
import { colorStyle } from '@/utils/colorStyle' import { colorStyle } from '@/utils/colorStyle'
import { measuringtoolsApi } from '@/api' import { measuringtoolsApi } from '@/api'
import AssetTriageBand from '@/components/AssetTriageBand.vue'
import CustomFieldsSection from '@/components/CustomFieldsSection.vue' import CustomFieldsSection from '@/components/CustomFieldsSection.vue'
import PluginAssetPanels from '@/components/PluginAssetPanels.vue' import PluginAssetPanels from '@/components/PluginAssetPanels.vue'
import AssetRelationships from '@/components/AssetRelationships.vue' import AssetRelationships from '@/components/AssetRelationships.vue'

View File

@@ -68,6 +68,14 @@
<span class="hero-detail-value">{{ device.businessunitname }}</span> <span class="hero-detail-value">{{ device.businessunitname }}</span>
</div> </div>
</div> </div>
<!-- The 6am band: what stops if this switch or AP goes down. A network
device is rarely interesting in itself; it matters because of what
sits behind it. -->
<AssetTriageBand
:assetid="device.assetid"
counterparttype="machine"
/>
<div class="hero-features" v-if="device.networkdevice"> <div class="hero-features" v-if="device.networkdevice">
<span v-if="device.networkdevice.ispoe" class="feature-badge poe">PoE</span> <span v-if="device.networkdevice.ispoe" class="feature-badge poe">PoE</span>
<span v-if="device.networkdevice.ismanaged" class="feature-badge managed">Managed</span> <span v-if="device.networkdevice.ismanaged" class="feature-badge managed">Managed</span>
@@ -227,6 +235,7 @@ import { useAuthStore } from '@/stores/auth'
import { networkApi } from '@/api' import { networkApi } from '@/api'
import { withBase } from '@/utils/basePath' import { withBase } from '@/utils/basePath'
import AssetRelationships from '@/components/AssetRelationships.vue' import AssetRelationships from '@/components/AssetRelationships.vue'
import AssetTriageBand from '@/components/AssetTriageBand.vue'
import CustomFieldsSection from '@/components/CustomFieldsSection.vue' import CustomFieldsSection from '@/components/CustomFieldsSection.vue'
import PluginAssetPanels from '@/components/PluginAssetPanels.vue' import PluginAssetPanels from '@/components/PluginAssetPanels.vue'
import { useWarrantyBadge } from '@/composables/warrantyBadge' import { useWarrantyBadge } from '@/composables/warrantyBadge'

View File

@@ -66,6 +66,16 @@
<span class="hero-detail-value mono">{{ ipAddress }}</span> <span class="hero-detail-value mono">{{ ipAddress }}</span>
</div> </div>
</div> </div>
<!-- The 6am band: which machine stops if this printer is down.
A shop-floor printer is often a label or tag printer feeding one
machine, and that link was only visible in the relationships
card near the bottom of the page. -->
<AssetTriageBand
:assetid="printer.assetid"
counterparttype="machine"
:relationshiptypes="['usesprinter', 'defaultprinter']"
/>
</div> </div>
</div> </div>
@@ -301,6 +311,7 @@ import { useRoute, useRouter } from 'vue-router'
import { printersApi } from '@/api' import { printersApi } from '@/api'
import { withBase } from '@/utils/basePath' import { withBase } from '@/utils/basePath'
import LocationMapTooltip from '@/components/LocationMapTooltip.vue' import LocationMapTooltip from '@/components/LocationMapTooltip.vue'
import AssetTriageBand from '@/components/AssetTriageBand.vue'
import CustomFieldsSection from '@/components/CustomFieldsSection.vue' import CustomFieldsSection from '@/components/CustomFieldsSection.vue'
import PluginAssetPanels from '@/components/PluginAssetPanels.vue' import PluginAssetPanels from '@/components/PluginAssetPanels.vue'
import AssetRelationships from '@/components/AssetRelationships.vue' import AssetRelationships from '@/components/AssetRelationships.vue'

View File

@@ -104,3 +104,50 @@ def test_relationships_row_carries_isdirectional(client, db, auth_headers):
ctrl_rows = [r for r in data['outgoing'] ctrl_rows = [r for r in data['outgoing']
if r['relationshiptypename'] == 'controls'] if r['relationshiptypename'] == 'controls']
assert ctrl_rows and all(r['isdirectional'] is True for r in ctrl_rows) assert ctrl_rows and all(r['isdirectional'] is True for r in ctrl_rows)
def test_the_controlled_asset_sees_the_link_from_its_own_page(client, db, auth_headers):
"""The target of a directional edge must receive it in `incoming`.
A PC controls a machine, and the row is stored once, source=PC. The PC's
page reads it from `outgoing`; the machine's page can only ever see it in
`incoming`. Nothing asserted that second half, and a UI was shipped that
rendered the link on the PC page and showed the machine page nothing - the
reciprocal-links defect. The endpoint was correct all along, which is
exactly why this assertion is worth having: it pins the contract the
frontend depends on, so the next such bug cannot be blamed on the API.
"""
atype_machine = AssetType(assettype='machine')
atype_computer = AssetType(assettype='computer')
db.session.add_all([atype_machine, atype_computer])
db.session.flush()
controls = RelationshipType(relationshiptype='controls', isdirectional=True)
db.session.add(controls)
db.session.flush()
pc = _make_asset('PC-1', atype_computer.assettypeid)
machine = _make_asset('M-1', atype_machine.assettypeid)
# Stored ONCE, in the canonical direction only.
db.session.add(AssetRelationship(sourceassetid=pc.assetid,
targetassetid=machine.assetid,
relationshiptypeid=controls.relationshiptypeid))
db.session.commit()
resp = client.get(f'/api/assets/{machine.assetid}/relationships',
headers=auth_headers)
assert resp.status_code == 200
data = resp.get_json()['data']
assert data['outgoing'] == [], 'the machine controls nothing'
assert len(data['incoming']) == 1, 'the machine must see the PC that controls it'
row = data['incoming'][0]
assert row['relationshiptypename'] == 'controls'
# The peer has to arrive with the fields the UI routes on. assetid alone
# is not enough: the detail routes key on pluginid, and the type name
# chooses the route base.
peer = row['sourceasset']
assert peer is not None
assert peer['assetid'] == pc.assetid
assert peer['assettypename'] == 'computer'