de3c5aa562
Reported: moving machine 3016 on the map and saving answered "this relationship already exists" and refused the save. The map had nothing to do with it. Every save of the machine form re-POSTed the controlling-PC link, and the server was right to refuse it - the row was already active, and assetrelationships is unique on (source, target, type). Any edit to a machine that has a controlling PC hit this; a position change is just the one somebody happened to be making. The form asked the wrong question. It used "do I know the existing link's id" as a stand-in for "is there an existing link", and read that id from rel.assetrelationshipid - a key the API has never emitted. to_dict serialises column names and the column is relationshipid, so the read was undefined on every load while controllingPcId was populated from the same relationship. The two answers disagreed, and the disagreement was invisible: undefined is not an error, it is just a falsy id. That single conflation caused both failures. Saving anything re-created the link, and CLEARING the controlling PC did nothing at all, because "no selection and no known id" reads as "nothing to do" - so removing a machine's controlling PC silently kept it. Only the first of those was reported, and the second is the one somebody would have trusted. Fixed by making the LOADED PC the authority on whether a link exists, with the relationship id used only to delete a row. If that id ever goes missing again the worst case is a link that cannot be removed, never a duplicate create. The decision moved to controllingPc.js as a pure function so it could be pinned by tests, following labelPages.js. Unchanged is now explicitly no-op rather than delete-then-create: re-linking a machine to the same PC opens a window where it has no controlling PC at all, and resolve_asset_position walks that edge, so a failure between the two calls would drop the machine's inherited position along with its link. The specs also cover a select handing back '42' against an API that gave 42, which would make an untouched link look like a swap and reach the same duplicate POST by another route. Not the printer assignment, which was the first guess: PrinterAssignmentPicker makes no relationship calls, and its save is wrapped in a catch that only logs, so it cannot surface an error from this form at all.
82 lines
3.4 KiB
JavaScript
82 lines
3.4 KiB
JavaScript
import { describe, it, expect } from 'vitest'
|
|
|
|
import { planControllingPcChange } from './controllingPc'
|
|
|
|
// Reported: moving machine 3016 on the map, then saving, answered "this
|
|
// relationship already exists" and refused the save. The machine had a
|
|
// controlling PC and the form re-created that link on every save, because it
|
|
// asked "do I know the existing link's id" instead of "is there an existing
|
|
// link" - and the id was read from a key the API does not emit, so it was
|
|
// always undefined.
|
|
//
|
|
// The first two specs are that bug. The rest pin the behaviour it was hiding.
|
|
|
|
describe('planControllingPcChange', () => {
|
|
it('does nothing when the controlling PC is unchanged', () => {
|
|
// The reported case: only the map position moved.
|
|
expect(planControllingPcChange({
|
|
selected: 42, loaded: 42, existingRelationshipId: 7,
|
|
})).toEqual({ action: 'none', deleteId: null })
|
|
})
|
|
|
|
it('still does nothing when the existing row id was never learned', () => {
|
|
// The exact shape of the bug: the link is there, its id is not known. The
|
|
// answer must be "leave it alone", never "create it again" - a create here
|
|
// is the 409.
|
|
expect(planControllingPcChange({
|
|
selected: 42, loaded: 42, existingRelationshipId: undefined,
|
|
})).toEqual({ action: 'none', deleteId: null })
|
|
})
|
|
|
|
it('removes the link when the PC is cleared', () => {
|
|
// This never fired before either: no selection and no known id read as
|
|
// "nothing to do", so clearing a controlling PC silently kept it.
|
|
expect(planControllingPcChange({
|
|
selected: null, loaded: 42, existingRelationshipId: 7,
|
|
})).toEqual({ action: 'unlink', deleteId: 7 })
|
|
})
|
|
|
|
it('creates a link when there was none', () => {
|
|
expect(planControllingPcChange({
|
|
selected: 42, loaded: null, existingRelationshipId: null,
|
|
})).toEqual({ action: 'link', deleteId: null })
|
|
})
|
|
|
|
it('replaces the old row when the PC is swapped', () => {
|
|
// controls is singular per SOURCE, not per target, so the old row has to go
|
|
// or the machine ends up with two controlling PCs.
|
|
expect(planControllingPcChange({
|
|
selected: 99, loaded: 42, existingRelationshipId: 7,
|
|
})).toEqual({ action: 'relink', deleteId: 7 })
|
|
})
|
|
|
|
it('treats a select string and an API number as the same PC', () => {
|
|
// A <select> hands back '42' while the relationship carried 42. Comparing
|
|
// those raw makes an untouched link look like a swap, which is the same
|
|
// duplicate POST by another route.
|
|
expect(planControllingPcChange({
|
|
selected: '42', loaded: 42, existingRelationshipId: 7,
|
|
})).toEqual({ action: 'none', deleteId: null })
|
|
})
|
|
|
|
it('treats an empty selection as cleared, not as a PC named ""', () => {
|
|
expect(planControllingPcChange({
|
|
selected: '', loaded: 42, existingRelationshipId: 7,
|
|
})).toEqual({ action: 'unlink', deleteId: 7 })
|
|
})
|
|
|
|
it('does nothing when there was no PC and none was chosen', () => {
|
|
expect(planControllingPcChange({
|
|
selected: null, loaded: null, existingRelationshipId: null,
|
|
})).toEqual({ action: 'none', deleteId: null })
|
|
})
|
|
|
|
it('does not attempt a delete it cannot perform', () => {
|
|
// Cleared, but the row id was never learned. Asking the API to delete
|
|
// `undefined` produced a request against /relationships/undefined.
|
|
expect(planControllingPcChange({
|
|
selected: null, loaded: 42, existingRelationshipId: undefined,
|
|
})).toEqual({ action: 'none', deleteId: null })
|
|
})
|
|
})
|