diff --git a/frontend/src/composables/assetTypeIds.js b/frontend/src/composables/assetTypeIds.js new file mode 100644 index 00000000..eb013aef --- /dev/null +++ b/frontend/src/composables/assetTypeIds.js @@ -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 } diff --git a/frontend/src/composables/assetTypeIds.spec.js b/frontend/src/composables/assetTypeIds.spec.js new file mode 100644 index 00000000..995d01cd --- /dev/null +++ b/frontend/src/composables/assetTypeIds.spec.js @@ -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() + }) +}) diff --git a/plugins/computers/frontend/views/PCForm.vue b/plugins/computers/frontend/views/PCForm.vue index 6924908f..13b64e20 100644 --- a/plugins/computers/frontend/views/PCForm.vue +++ b/plugins/computers/frontend/views/PCForm.vue @@ -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) { diff --git a/plugins/machines/frontend/views/MachineForm.vue b/plugins/machines/frontend/views/MachineForm.vue index 4f159a5c..6350c449 100644 --- a/plugins/machines/frontend/views/MachineForm.vue +++ b/plugins/machines/frontend/views/MachineForm.vue @@ -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 diff --git a/plugins/network/frontend/views/NetworkDeviceForm.vue b/plugins/network/frontend/views/NetworkDeviceForm.vue index ff6c6ef1..e3b6cf4d 100644 --- a/plugins/network/frontend/views/NetworkDeviceForm.vue +++ b/plugins/network/frontend/views/NetworkDeviceForm.vue @@ -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(), diff --git a/plugins/printers/frontend/views/PrinterForm.vue b/plugins/printers/frontend/views/PrinterForm.vue index 9a92f7d8..b5a2932d 100644 --- a/plugins/printers/frontend/views/PrinterForm.vue +++ b/plugins/printers/frontend/views/PrinterForm.vue @@ -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) { diff --git a/scripts/check-naming-and-style.sh b/scripts/check-naming-and-style.sh index b251c412..d6743f82 100755 --- a/scripts/check-naming-and-style.sh +++ b/scripts/check-naming-and-style.sh @@ -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 </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"