Resolve an asset type by name, and send a map position with its level

Wave 5, the asset forms. Both defects are the same mistake in different
clothes: a form deciding something it should have been told.

Four forms carried a hardcoded asset-type id - const COMPUTER_ASSETTYPEID = 2,
and the same for machines, printers and network devices - to tell
CustomFieldsInputs which type's fields to render. Those ids come from seed
order. On the instance they were written against, computer was 2; on a freshly
seeded database computer is 1 and 2 is network_device. So the PC form asked for
another type's custom fields, rendered them, and dropped everything typed into
them behind a success toast, because the values were keyed to a type the record
does not have. Verified in a browser against a correctly built dist: the form
now requests assettypeid=1 on an instance where computer is 1.

The name is the stable thing - assettype is a fixed vocabulary and the API
already publishes the mapping - so composables/assetTypeIds.js resolves it once
per page load. A failed load leaves it undefined, which renders no custom-field
panel: a missing panel is honest, a guessed id is what this replaces.

Three of the five forms that emit a map position sent mapx and mapy without
levelid. ADR-017 is explicit that a position without its level cannot be drawn
on the right floor plan, so a picked pin saved a null level and rendered
nowhere - while the form displayed "Position: x, y" back, because it kept the
value it had never sent.

The ADR-017 gate should have caught that and did not: it globbed '*.py' only,
so it policed what the server emits and never what the browser sends. It now
covers .vue and .js too, and accepts both quoting styles. Verified by planting:
drop levelid from PCForm again and the gate names that exact line.

Five composable tests, including a fixture with a DIFFERENT seed order - the
case the constants got wrong.

Still open from this area, and deliberately not in this commit: the PC form
writes an INHERITED location back as the record's own. Fixing it needs the API
to distinguish an owned locationid from a derived one, which is a contract
change rather than a form fix.

Scope: product
This commit is contained in:
cproudlock
2026-09-03 14:42:24 -04:00
parent ffced2f822
commit 45acaf0b8f
7 changed files with 208 additions and 13 deletions

View File

@@ -0,0 +1,56 @@
// Asset-type ids resolved by NAME, once per page load.
//
// Four asset forms carried a hardcoded id - `const COMPUTER_ASSETTYPEID = 2`,
// and the same for machines, printers and network devices - to tell
// CustomFieldsInputs which type's fields to render. The ids come from seed
// order, not from anything stable: on the instance those constants were written
// against, computer was 2; on a freshly seeded one it is 1 and 2 is
// network_device. So the PC form asked for another type's custom fields,
// rendered them, and dropped whatever the operator typed - with a success
// toast, because the values were keyed to a type the record does not have.
//
// A name is the stable thing. `assettype` is a fixed vocabulary (ADR-001) and
// the API already publishes the mapping; nothing else should hardcode a number.
import { reactive } from 'vue'
import { assetsApi } from '../api'
const state = reactive({
byname: {},
loaded: false
})
let inflight = null
export async function loadAssetTypeIds() {
if (state.loaded) return state
if (inflight) return inflight
inflight = assetsApi.types.list()
.then((response) => {
const rows = response?.data?.data || response?.data || []
const byname = {}
rows.forEach((row) => {
if (row && row.assettype) byname[row.assettype] = row.assettypeid
})
state.byname = byname
state.loaded = true
return state
})
.catch(() => {
// Leave it unloaded rather than caching a failure: a form that could not
// resolve its type renders no custom fields, which is honest. Guessing an
// id is what caused the defect this replaces.
state.loaded = false
return state
})
.finally(() => { inflight = null })
return inflight
}
// Undefined until loaded, and undefined for a name the server does not know.
// CustomFieldsInputs treats an undefined assettypeid as "nothing to render",
// which is the safe reading: better a missing panel than the wrong type's.
export function assetTypeId(name) {
return state.byname[name]
}
export { state }

View File

@@ -0,0 +1,94 @@
// Asset-type ids must come from the server by name.
//
// Four forms hardcoded the id. Those constants were written against one
// instance's seed order and were wrong on another: PCForm said 2, and on a
// freshly seeded database 2 is network_device. The form asked
// CustomFieldsInputs for that type's fields, rendered them, and dropped what
// the operator typed - with a success toast, because the values were keyed to
// a type the record does not have.
import { describe, it, expect, vi, beforeEach } from 'vitest'
vi.mock('../api', () => ({
assetsApi: {
types: {
list: vi.fn()
}
}
}))
import { assetsApi } from '../api'
import { loadAssetTypeIds, assetTypeId, state } from './assetTypeIds'
function reset() {
state.byname = {}
state.loaded = false
assetsApi.types.list.mockReset()
}
describe('assetTypeIds', () => {
beforeEach(reset)
it('resolves an id by name from what the server actually has', async () => {
assetsApi.types.list.mockResolvedValue({
data: { data: [
{ assettypeid: 1, assettype: 'computer' },
{ assettypeid: 2, assettype: 'network_device' },
{ assettypeid: 4, assettype: 'machine' }
] }
})
await loadAssetTypeIds()
expect(assetTypeId('computer')).toBe(1)
expect(assetTypeId('network_device')).toBe(2)
expect(assetTypeId('machine')).toBe(4)
})
it('survives a differently seeded site, which is the whole point', async () => {
// The same names, different ids. A hardcoded 2 would be network_device
// here and computer in the other fixture.
assetsApi.types.list.mockResolvedValue({
data: { data: [
{ assettypeid: 2, assettype: 'computer' },
{ assettypeid: 3, assettype: 'network_device' }
] }
})
await loadAssetTypeIds()
expect(assetTypeId('computer')).toBe(2)
expect(assetTypeId('network_device')).toBe(3)
})
it('fetches once even when several forms ask at the same time', async () => {
assetsApi.types.list.mockResolvedValue({
data: { data: [{ assettypeid: 1, assettype: 'computer' }] }
})
await Promise.all([loadAssetTypeIds(), loadAssetTypeIds(), loadAssetTypeIds()])
await loadAssetTypeIds()
expect(assetsApi.types.list).toHaveBeenCalledTimes(1)
})
it('returns undefined rather than a guess when the load fails', async () => {
assetsApi.types.list.mockRejectedValue(new Error('network down'))
await loadAssetTypeIds()
// Undefined means CustomFieldsInputs renders nothing. A missing panel is
// honest; the wrong type's panel is what this replaces.
expect(assetTypeId('computer')).toBeUndefined()
expect(state.loaded).toBe(false)
})
it('returns undefined for a type the server does not know', async () => {
assetsApi.types.list.mockResolvedValue({
data: { data: [{ assettypeid: 1, assettype: 'computer' }] }
})
await loadAssetTypeIds()
expect(assetTypeId('forklift')).toBeUndefined()
})
})

View File

@@ -373,6 +373,7 @@ import CustomFieldsInputs from '@/components/CustomFieldsInputs.vue'
import PrinterAssignmentPicker from '@/components/PrinterAssignmentPicker.vue'
import { currentTheme } from '@/stores/theme'
import { useIdentifierFlags } from '@/composables/identifierSettings'
import { loadAssetTypeIds, assetTypeId } from '@/composables/assetTypeIds'
import { isPluginEnabled, loadEnabledPlugins } from '@/composables/enabledPlugins'
import { useToast } from '@/composables/toast'
import { apiError } from '@/utils/apiError'
@@ -390,9 +391,11 @@ const isEdit = computed(() => !!route.params.id)
// falls back to the PCs list.
const returnTo = computed(() => (route.query.from === 'fleet' ? '/endpoints' : '/pcs'))
// Seeded asset-type id for computers (see /api/assets/types). Custom-field
// values are keyed by the underlying asset id, captured on load / create.
const COMPUTER_ASSETTYPEID = 2
// Resolved by NAME at load, never hardcoded: the id comes from seed
// order, so the constant that used to live here pointed at a different
// asset type on a differently-seeded site - the form then rendered THAT
// type's custom fields and dropped whatever was typed into them.
const COMPUTER_ASSETTYPEID = computed(() => assetTypeId('computer'))
const customFieldsRef = ref(null)
const printerPickerRef = ref(null)
const currentAssetId = ref(null)
@@ -519,6 +522,7 @@ const filteredVendors = computed(() => {
})
onMounted(async () => {
await loadAssetTypeIds()
try {
// Load reference data
// perpage 100 so dropdowns aren't truncated to the default 20-row page
@@ -649,7 +653,13 @@ async function savePC() {
notes: form.value.notes || null,
ipaddress: form.value.ipaddress || null,
mapx: form.value.mapx,
mapy: form.value.mapy
mapy: form.value.mapy,
// ADR-017: a position without its level cannot be drawn on the right
// floor plan. The picker sets all three and this payload sent two, so a
// picked pin saved a levelid of null and then rendered nowhere - the
// form even showed "Position: x, y" back, because it kept the value it
// had never sent.
levelid: form.value.levelid
}
// only set display name when an alias is given, so we don't clobber it
if (form.value.alias) {

View File

@@ -416,6 +416,7 @@ import CustomFieldsInputs from '@/components/CustomFieldsInputs.vue'
import PrinterAssignmentPicker from '@/components/PrinterAssignmentPicker.vue'
import { currentTheme } from '@/stores/theme'
import { useIdentifierFlags } from '@/composables/identifierSettings'
import { loadAssetTypeIds, assetTypeId } from '@/composables/assetTypeIds'
import { apiError } from '@/utils/apiError'
import { planControllingPcChange } from './controllingPc'
@@ -426,8 +427,11 @@ const router = useRouter()
const isEdit = computed(() => !!route.params.id)
// Seeded asset-type id for machines (see /api/assets/types).
const MACHINE_ASSETTYPEID = 1
// Resolved by NAME at load, never hardcoded: the id comes from seed
// order, so the constant that used to live here pointed at a different
// asset type on a differently-seeded site - the form then rendered THAT
// type's custom fields and dropped whatever was typed into them.
const MACHINE_ASSETTYPEID = computed(() => assetTypeId('machine'))
const customFieldsRef = ref(null)
const printerPickerRef = ref(null)
const currentAssetId = ref(null)
@@ -571,6 +575,7 @@ watch(() => form.value.controllervendorid, (newVal, oldVal) => {
})
onMounted(async () => {
await loadAssetTypeIds()
try {
// Load reference data in parallel
const [typesRes, statusRes, vendorRes, locRes, allModels, buRes, pcsRes, relTypesRes] = await Promise.all([
@@ -763,7 +768,13 @@ async function saveMachine() {
maintenanceintervaldays: form.value.maintenanceintervaldays || null,
notes: form.value.notes || null,
mapx: form.value.mapx,
mapy: form.value.mapy
mapy: form.value.mapy,
// ADR-017: a position without its level cannot be drawn on the right
// floor plan. The picker sets all three and this payload sent two, so a
// picked pin saved a levelid of null and then rendered nowhere - the
// form even showed "Position: x, y" back, because it kept the value it
// had never sent.
levelid: form.value.levelid
}
let savedMachine

View File

@@ -334,6 +334,7 @@ import { loadMapConfig, levelOptions, levelName, state as mapConfig }
import Modal from '@/components/Modal.vue'
import { currentTheme } from '@/stores/theme'
import { useIdentifierFlags } from '@/composables/identifierSettings'
import { loadAssetTypeIds, assetTypeId } from '@/composables/assetTypeIds'
import { apiError } from '@/utils/apiError'
const { isEnabled } = useIdentifierFlags()
@@ -350,8 +351,11 @@ const router = useRouter()
const deviceId = route.params.id
const isEdit = computed(() => !!deviceId)
// Seeded asset-type id for network devices (see /api/assets/types).
const NETWORK_ASSETTYPEID = 3
// Resolved by NAME at load, never hardcoded: the id comes from seed
// order, so the constant that used to live here pointed at a different
// asset type on a differently-seeded site - the form then rendered THAT
// type's custom fields and dropped whatever was typed into them.
const NETWORK_ASSETTYPEID = computed(() => assetTypeId('network_device'))
const customFieldsRef = ref(null)
const currentAssetId = ref(null)
@@ -424,6 +428,7 @@ const saving = ref(false)
const error = ref('')
onMounted(async () => {
await loadAssetTypeIds()
await Promise.all([
loadDeviceTypes(),
loadVendors(),

View File

@@ -336,6 +336,7 @@ import Modal from '@/components/Modal.vue'
import CustomFieldsInputs from '@/components/CustomFieldsInputs.vue'
import { currentTheme } from '@/stores/theme'
import { useIdentifierFlags } from '@/composables/identifierSettings'
import { loadAssetTypeIds, assetTypeId } from '@/composables/assetTypeIds'
import { apiError } from '@/utils/apiError'
import { getPrinterHostnameTemplate } from '@/utils/siteSettings'
@@ -346,8 +347,11 @@ const router = useRouter()
const isEdit = computed(() => !!route.params.id)
// Seeded asset-type id for printers (see /api/assets/types).
const PRINTER_ASSETTYPEID = 4
// Resolved by NAME at load, never hardcoded: the id comes from seed
// order, so the constant that used to live here pointed at a different
// asset type on a differently-seeded site - the form then rendered THAT
// type's custom fields and dropped whatever was typed into them.
const PRINTER_ASSETTYPEID = computed(() => assetTypeId('printer'))
const customFieldsRef = ref(null)
const currentAssetId = ref(null)
const manualHostname = ref(false)
@@ -542,6 +546,7 @@ function onWindowsNameInput() {
}
onMounted(async () => {
await loadAssetTypeIds()
try {
// Load reference data (models paged in full via listAll, see api/index.js)
// perpage 100 so dropdowns aren't truncated to the default 20-row page
@@ -686,7 +691,13 @@ async function savePrinter() {
isduplex: !!form.value.isduplex,
isnetwork: !!form.value.isnetwork,
mapx: form.value.mapx,
mapy: form.value.mapy
mapy: form.value.mapy,
// ADR-017: a position without its level cannot be drawn on the right
// floor plan. The picker sets all three and this payload sent two, so a
// picked pin saved a levelid of null and then rendered nowhere - the
// form even showed "Position: x, y" back, because it kept the value it
// had never sent.
levelid: form.value.levelid
}
// only set the display name when an alias is given, so we don't clobber it
if (form.value.alias) {

View File

@@ -191,11 +191,19 @@ while IFS= read -r hit; do
[ -z "$hit" ] && continue
file=${hit%%:*}
line=${hit#*:}; line=${line%%:*}
if ! sed -n "${line},$((line + 8))p" "$file" | grep -q "'levelid'"; then
# Python writes 'levelid', Vue writes levelid: - accept either, so the
# check covers the payloads a browser sends as well as the ones the server
# emits. It was Python-only, which is how three asset forms shipped a
# position with no level.
if ! sed -n "${line},$((line + 8))p" "$file" | grep -qE "'levelid'|levelid:"; then
MISSING_LEVEL="$MISSING_LEVEL$file:$line"$'\n'
fi
done <<EOF
$(grep -rn "'mapx':" --include='*.py' shopdb/ plugins/ scripts/ 2>/dev/null | grep -v '/tests\?/' || true)
$(grep -rn "mapx: " --include='*.vue' --include='*.js' \
frontend/src plugins/*/frontend 2>/dev/null \
| grep -v '.plugins-staged' | grep -v '\.spec\.js' \
| grep -vE "mapx: (null|0)," || true)
EOF
if [ -n "$MISSING_LEVEL" ]; then
echo "FAIL: these emit 'mapx' with no 'levelid' beside it - a position with no"