Stop re-creating a controlling-PC link that was already there

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.

plugins/machines/frontend/views/MachineForm.vue

@@ -379,6 +379,7 @@ import PrinterAssignmentPicker from '@/components/PrinterAssignmentPicker.vue'
 import { currentTheme } from '@/stores/theme'
 import { useIdentifierFlags } from '@/composables/identifierSettings'
 import { apiError } from '@/utils/apiError'
+import { planControllingPcChange } from './controllingPc'
 
 const { isEnabled } = useIdentifierFlags()
 
@@ -440,6 +441,9 @@ const relationshipTypes = ref([])
 const controllingPcId = ref(null)
 const relationshipTypeId = ref(null)
 const existingRelationshipId = ref(null)
+// Which PC the link named when the form loaded, so an unchanged one can be left
+// alone. Editing a machine is mostly editing something else about it.
+const loadedControllingPcId = ref(null)
 const currentMachine = ref(null)
 
 // Filter models by selected machine vendor
@@ -575,7 +579,16 @@ onMounted(async () => {
             if (rel.sourceasset?.assettypename === 'computer' && (rel.relationshiptypename || '').toLowerCase() === 'controls') {
               controllingPcId.value = rel.sourceasset.assetid
               relationshipTypeId.value = rel.relationshiptypeid
-              existingRelationshipId.value = rel.assetrelationshipid
+              // relationshipid. `assetrelationshipid` is not a key the API has
+              // ever emitted - to_dict serialises the column names and the
+              // column is relationshipid - so this read undefined every time,
+              // and saveRelationship then took the "there is nothing to
+              // replace" path on a machine that had a controlling PC. Saving
+              // ANY change, a map position included, re-POSTed the link that
+              // was already there and the server correctly answered 409 "this
+              // relationship already exists".
+              existingRelationshipId.value = rel.relationshipid
+              loadedControllingPcId.value = rel.sourceasset.assetid
               break
             }
           }
@@ -693,31 +706,40 @@ async function saveMachine() {
 }
 
 async function saveRelationship(assetId) {
-  // If no PC selected and no existing relationship, nothing to do
-  if (!controllingPcId.value && !existingRelationshipId.value) {
+  // The decision lives in controllingPc.js, where it is covered by tests.
+  // See that file for why "is there a link" and "do I know its id" are two
+  // different questions and what happened when they were treated as one.
+  const plan = planControllingPcChange({
+    selected: controllingPcId.value,
+    loaded: loadedControllingPcId.value,
+    existingRelationshipId: existingRelationshipId.value,
+  })
+
+  if (plan.action === 'none') {
     return
   }
 
-  // If clearing the relationship
-  if (!controllingPcId.value && existingRelationshipId.value) {
-    await assetsApi.deleteRelationship(existingRelationshipId.value)
+  if (plan.deleteId !== null) {
+    await assetsApi.deleteRelationship(plan.deleteId)
+    existingRelationshipId.value = null
+    loadedControllingPcId.value = null
+  }
+
+  if (plan.action === 'unlink') {
     return
   }
 
-  // If PC is selected
-  if (controllingPcId.value) {
-    // If there's an existing relationship, delete it first
-    if (existingRelationshipId.value) {
-      await assetsApi.deleteRelationship(existingRelationshipId.value)
-    }
+  // PC controls Machine, so the PC is the source and the machine the target.
+  const created = await assetsApi.createRelationship({
+    sourceassetid: controllingPcId.value,
+    targetassetid: assetId,
+    relationshiptypeid: relationshipTypeId.value
+  })
 
-    // Create new relationship (PC controls Machine, so PC is source, Machine is target)
-    await assetsApi.createRelationship({
-      sourceassetid: controllingPcId.value,
-      targetassetid: assetId,
-      relationshiptypeid: relationshipTypeId.value
-    })
-  }
+  // Keep the form honest about what is now stored, so a second save without a
+  // reload does not repeat the create it just did.
+  existingRelationshipId.value = created?.data?.data?.relationshipid ?? null
+  loadedControllingPcId.value = controllingPcId.value
 }
 </script>