diff --git a/CHANGELOG.md b/CHANGELOG.md index ebad69b4..e1cd9113 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,6 +105,13 @@ ADR-007 and ADR-002. the wrong code. The page index now travels with the page. - The back label's dotted preview outline outspecified the print reset and drew a dashed rule down the edge of every second card. +- **The warranty list could not be searched by machine number.** The number was + in the payload and rendered in the table as a chip, but the search box looked + only at vendor, service level, service tag and asset number. That made the + identifier the feature was built around - a shopfloor PC is warranted as a PC + but found by the machine it drives - the one thing the box could not find. + Machine number and machine name are now searched too, and the placeholder + names them. - **`flask plugin upgrade-all` failed a plugin whose tables already existed** ("table securesendsecrets already exists" on the tools plugin). Every post-cutover baseline emitted a plain `op.create_table`, which is correct only diff --git a/plugins/warranty/frontend/views/WarrantiesList.vue b/plugins/warranty/frontend/views/WarrantiesList.vue index e208cc04..1913dd92 100644 --- a/plugins/warranty/frontend/views/WarrantiesList.vue +++ b/plugins/warranty/frontend/views/WarrantiesList.vue @@ -24,7 +24,7 @@ - + {{ filteredItems.length }} of {{ items.length }} @@ -186,6 +186,7 @@ import api, { warrantyApi, assetsApi, vendorsApi } from '@/api' import { useToast } from '@/composables/toast' import { apiError } from '@/utils/apiError' import MachineMapChip from '../components/MachineMapChip.vue' +import { matchesWarrantySearch } from './warrantySearch' const toast = useToast() const route = useRoute() @@ -197,16 +198,11 @@ const search = ref('') const page = ref(1) const perPage = 25 -// Client-side search across vendor / service level / tag / linked asset numbers. -const filteredItems = computed(() => { - const term = search.value.trim().toLowerCase() - if (!term) return items.value - return items.value.filter(w => - (w.vendor || '').toLowerCase().includes(term) || - (w.servicelevel || '').toLowerCase().includes(term) || - (w.servicetag || '').toLowerCase().includes(term) || - (w.assets || []).some(a => (a.assetnumber || '').toLowerCase().includes(term))) -}) +// Client-side search. The field list lives in warrantySearch.js, where it is +// covered by tests - the machine number was rendered in this table for a long +// time before anything let you search for it. +const filteredItems = computed(() => + items.value.filter(w => matchesWarrantySearch(w, search.value))) const totalPages = computed(() => Math.max(1, Math.ceil(filteredItems.value.length / perPage))) const paginatedItems = computed(() => filteredItems.value.slice((page.value - 1) * perPage, page.value * perPage)) diff --git a/plugins/warranty/frontend/views/warrantySearch.js b/plugins/warranty/frontend/views/warrantySearch.js new file mode 100644 index 00000000..8579e299 --- /dev/null +++ b/plugins/warranty/frontend/views/warrantySearch.js @@ -0,0 +1,44 @@ +// What the warranty list's search box matches against. +// +// Pulled out of WarrantiesList.vue so the field list is somewhere a test can +// see it. It went wrong the way a field list usually does: silently, by a +// column being added to the table and not to the filter. +// +// THE MACHINE NUMBER IS THE POINT. _related_machine in the warranty API exists +// precisely because "a shopfloor PC is bought, warranted and replaced as a PC, +// but it is FOUND by the machine it drives - nobody walks the floor looking for +// an asset number". The payload carries it, the table renders it in a chip, and +// the filter did not look at it - so the one identifier the feature was built +// around was the one thing the search box could not find. + +/** + * Does this warranty match the search term? + * + * @param {object} warranty - a row from GET /api/warranties + * @param {string} term - the raw search box contents + * @returns {boolean} true when the term is empty or any searched field contains it + */ +export function matchesWarrantySearch(warranty, term) { + const needle = (term || '').trim().toLowerCase() + if (!needle) return true + if (!warranty) return false + + const has = (value) => String(value ?? '').toLowerCase().includes(needle) + + if (has(warranty.vendor) || has(warranty.servicelevel) || has(warranty.servicetag)) { + return true + } + + return (warranty.assets || []).some((asset) => { + if (has(asset?.assetnumber)) return true + // The related machine is optional: a warranty on something that drives no + // machine has none, and a machine that was never linked leaves it null. + const machine = asset?.machine + return !!machine && (has(machine.machinenumber) || has(machine.name)) + }) +} + +/** The fields above, in the order the placeholder should name them. */ +export const WARRANTY_SEARCH_FIELDS = [ + 'vendor', 'service level', 'tag', 'asset number', 'machine', +] diff --git a/plugins/warranty/frontend/views/warrantySearch.spec.js b/plugins/warranty/frontend/views/warrantySearch.spec.js new file mode 100644 index 00000000..fc2e17cd --- /dev/null +++ b/plugins/warranty/frontend/views/warrantySearch.spec.js @@ -0,0 +1,79 @@ +import { describe, it, expect } from 'vitest' + +import { matchesWarrantySearch } from './warrantySearch' + +// Reported: the warranty page could not be searched by machine number. +// +// The number was in the payload (_related_machine puts it there) and rendered +// in the table as a chip, but the filter looked only at vendor, service level, +// service tag and asset number. So the identifier the feature was built around +// - a PC is warranted as a PC but FOUND by the machine it drives - was the one +// thing the box could not find. + +function warranty(overrides = {}) { + return { + vendor: 'Dell', + servicelevel: 'ProSupport', + servicetag: 'ABC1234', + assets: [{ + assetnumber: 'PC-00912', + name: 'Bay 3105 workstation', + machine: { machinenumber: '3016', name: 'Makino a61nx' }, + }], + ...overrides, + } +} + +describe('matchesWarrantySearch', () => { + it('matches on the machine number', () => { + // The reported case. + expect(matchesWarrantySearch(warranty(), '3016')).toBe(true) + }) + + it('matches on the machine name', () => { + expect(matchesWarrantySearch(warranty(), 'makino')).toBe(true) + }) + + it('still matches the fields it always did', () => { + expect(matchesWarrantySearch(warranty(), 'dell')).toBe(true) + expect(matchesWarrantySearch(warranty(), 'prosupport')).toBe(true) + expect(matchesWarrantySearch(warranty(), 'abc1234')).toBe(true) + expect(matchesWarrantySearch(warranty(), 'pc-00912')).toBe(true) + }) + + it('is case insensitive and ignores surrounding space', () => { + expect(matchesWarrantySearch(warranty(), ' MAKINO ')).toBe(true) + }) + + it('does not match an unrelated term', () => { + expect(matchesWarrantySearch(warranty(), '9999')).toBe(false) + }) + + it('keeps every row when the term is empty', () => { + expect(matchesWarrantySearch(warranty(), '')).toBe(true) + expect(matchesWarrantySearch(warranty(), ' ')).toBe(true) + expect(matchesWarrantySearch(warranty(), undefined)).toBe(true) + }) + + it('handles a warranty whose asset drives no machine', () => { + // machine is null whenever the covered asset has no controls link - a + // printer, or a PC nobody linked yet. It must not throw. + const row = warranty({ assets: [{ assetnumber: 'PR-42', machine: null }] }) + expect(matchesWarrantySearch(row, 'PR-42')).toBe(true) + expect(matchesWarrantySearch(row, '3016')).toBe(false) + }) + + it('handles a warranty with no linked assets at all', () => { + const row = warranty({ assets: [] }) + expect(matchesWarrantySearch(row, 'dell')).toBe(true) + expect(matchesWarrantySearch(row, '3016')).toBe(false) + }) + + it('tolerates missing fields rather than matching everything', () => { + // String(undefined) is 'undefined', which would make a search for 'und' + // match every row with an empty field. + const row = { vendor: null, servicelevel: undefined, assets: null } + expect(matchesWarrantySearch(row, 'und')).toBe(false) + expect(matchesWarrantySearch(row, 'null')).toBe(false) + }) +})