From 4bf4531fba65da92ae7acbea22b78187e24d7d75 Mon Sep 17 00:00:00 2001
From: Ruben Fiszel <ruben@windmill.dev>
Date: Thu, 14 Aug 2025 13:51:25 +0000
Subject: [PATCH] fix: improve schemaeditor for nested oneOfs

---
 .../lib/components/EditableSchemaForm.svelte  |  7 +-
 .../lib/components/schema/AddProperty.svelte  | 88 +++++++++----------
 .../components/schema/AddPropertyV2.svelte    | 88 +++++++++----------
 .../schema/FlowPropertyEditor.svelte          | 14 +--
 4 files changed, 98 insertions(+), 99 deletions(-)

diff --git a/frontend/src/lib/components/EditableSchemaForm.svelte b/frontend/src/lib/components/EditableSchemaForm.svelte
index 555b78a888..d91cc5c2d6 100644
--- a/frontend/src/lib/components/EditableSchemaForm.svelte
+++ b/frontend/src/lib/components/EditableSchemaForm.svelte
@@ -186,6 +186,7 @@
 	function onSchemaChange() {
 		let editSchema = false
 		if (alignOrderWithProperties(schema)) {
+			console.log('alignOrderWithProperties', JSON.stringify(schema, null, 2))
 			editSchema = true
 		}
 		let lkeys = schema?.order ?? Object.keys(schema?.properties ?? {})
@@ -704,7 +705,8 @@
 																										property_1: {
 																											type: 'string'
 																										}
-																									}
+																									},
+																									order: ['property_1']
 																								},
 																								{
 																									title: 'Option 2',
@@ -717,7 +719,8 @@
 																										property_2: {
 																											type: 'string'
 																										}
-																									}
+																									},
+																									order: ['property_2']
 																								}
 																							]
 																						}
diff --git a/frontend/src/lib/components/schema/AddProperty.svelte b/frontend/src/lib/components/schema/AddProperty.svelte
index eccf6a375d..f5a7b2b8b7 100644
--- a/frontend/src/lib/components/schema/AddProperty.svelte
+++ b/frontend/src/lib/components/schema/AddProperty.svelte
@@ -1,10 +1,5 @@
 <script lang="ts">
-	import {
-		type Schema,
-		type SchemaProperty,
-		modalToSchema,
-		type ModalSchemaProperty
-	} from '$lib/common'
+	import { type Schema, modalToSchema, type ModalSchemaProperty } from '$lib/common'
 	import { emptySchema, sendUserToast } from '$lib/utils'
 	import { createEventDispatcher } from 'svelte'
 	import SimpleEditor from '../SimpleEditor.svelte'
@@ -33,30 +28,30 @@
 	let oldArgName: string | undefined // when editing argument and changing name
 	let jsonEditor: SimpleEditor | undefined
 
-	reorder()
+	reorder(schema)
 
-	function reorder() {
-		if (schema.order && Array.isArray(schema.order)) {
+	function reorder(s: Schema) {
+		if (s.order && Array.isArray(s.order)) {
 			const n = {}
 
-			;(schema.order as string[]).forEach((x) => {
-				if (schema.properties && schema.properties[x] != undefined) {
-					n[x] = schema.properties[x]
+			;(s.order as string[]).forEach((x) => {
+				if (s.properties && s.properties[x] != undefined) {
+					n[x] = s.properties[x]
 				}
 			})
 
-			Object.keys(schema.properties ?? {})
-				.filter((x) => !schema.order?.includes(x))
+			Object.keys(s.properties ?? {})
+				.filter((x) => !s.order?.includes(x))
 				.forEach((x) => {
-					n[x] = schema.properties[x]
+					n[x] = s.properties[x]
 				})
 			schema.properties = n
 		}
 	}
 
-	function syncOrders() {
-		if (schema) {
-			schema.order = Object.keys(schema.properties ?? {})
+	function syncOrders(s: Schema) {
+		if (s) {
+			s.order = Object.keys(s.properties ?? {})
 		}
 	}
 
@@ -64,7 +59,6 @@
 		// If editing the arg's name, oldName containing the old argument name must be provided
 		argError = ''
 		modalProperty.name = modalProperty.name.trim()
-
 		if (modalProperty.name.length === 0) {
 			argError = 'Arguments need to have a name'
 		} else if (
@@ -73,56 +67,59 @@
 		) {
 			argError = 'There is already an argument with this name'
 		} else {
-			if (!schema.properties) {
-				schema.properties = {}
+			let newSchema = { ...schema }
+			if (!newSchema.properties) {
+				newSchema.properties = {}
 			}
-			if (!schema.required) {
-				schema.required = []
+			if (!newSchema.required) {
+				newSchema.required = []
 			}
-			if (!schema.order || !Array.isArray(schema.order)) {
-				syncOrders()
+			if (!newSchema.order || !Array.isArray(newSchema.order)) {
+				syncOrders(newSchema)
+			}
+			newSchema.properties = {
+				...newSchema.properties,
+				[modalProperty.name]: modalToSchema(modalProperty)
 			}
-			schema.properties[modalProperty.name] = modalToSchema(modalProperty)
 			if (modalProperty.required) {
-				if (!schema.required.includes(modalProperty.name)) {
-					schema.required.push(modalProperty.name)
+				if (!newSchema.required.includes(modalProperty.name)) {
+					newSchema.required.push(modalProperty.name)
 				}
-			} else if (schema.required.includes(modalProperty.name)) {
-				const index = schema.required.indexOf(modalProperty.name, 0)
+			} else if (newSchema.required.includes(modalProperty.name)) {
+				const index = newSchema.required.indexOf(modalProperty.name, 0)
 				if (index > -1) {
-					schema.required.splice(index, 1)
+					newSchema.required.splice(index, 1)
 				}
 			}
 
 			if (editing && oldArgName && oldArgName !== modalProperty.name) {
-				let oldPosition = schema.order.indexOf(oldArgName)
-				schema.order[oldPosition] = modalProperty.name
-				reorder()
-				handleDeleteArgument([oldArgName])
+				let oldPosition = newSchema.order.indexOf(oldArgName)
+				newSchema.order[oldPosition] = modalProperty.name
+				reorder(newSchema)
+				handleDeleteArgument([oldArgName], newSchema)
 			}
 
-			if (!schema.order?.includes(modalProperty.name)) {
-				schema.order.push(modalProperty.name)
+			if (!newSchema.order?.includes(modalProperty.name)) {
+				newSchema.order.push(modalProperty.name)
 			}
 			modalProperty = Object.assign({}, DEFAULT_PROPERTY)
 			editing = false
 			oldArgName = undefined
-		}
 
-		schema = schema
-		schemaString = JSON.stringify(schema, null, '\t')
-		jsonEditor?.setCode(schemaString)
+			schema = newSchema
+			schemaString = JSON.stringify(schema, null, '\t')
+			jsonEditor?.setCode(schemaString)
+		}
 
 		if (argError !== '') {
 			sendUserToast(argError, true)
 		}
-
 		dispatch('change', schema)
 	}
 
-	export function handleDeleteArgument(argPath: string[]): void {
+	export function handleDeleteArgument(argPath: string[], nschema?: Schema): void {
 		try {
-			let modifiedObject: Schema | SchemaProperty = schema
+			let modifiedObject: Schema = { ...(nschema ?? schema) }
 			let modifiedProperties = modifiedObject.properties as object
 			let argName = argPath.pop() as string
 
@@ -144,7 +141,7 @@
 				if (modifiedObject.order) {
 					modifiedObject.order = modifiedObject.order.filter((arg) => arg !== argName)
 				}
-				schema = schema
+				schema = modifiedObject
 				schemaString = JSON.stringify(schema, null, '\t')
 				dispatch('change', schema)
 			} else {
@@ -168,6 +165,7 @@
 				})
 			} catch (err) {
 				sendUserToast(`Could not add argument: ${err}`, true)
+				console.log('Could not add argument', err)
 			}
 		}}
 	/>
diff --git a/frontend/src/lib/components/schema/AddPropertyV2.svelte b/frontend/src/lib/components/schema/AddPropertyV2.svelte
index c371c4fe1f..47b3dd4661 100644
--- a/frontend/src/lib/components/schema/AddPropertyV2.svelte
+++ b/frontend/src/lib/components/schema/AddPropertyV2.svelte
@@ -1,10 +1,5 @@
 <script lang="ts">
-	import {
-		type Schema,
-		type SchemaProperty,
-		modalToSchema,
-		type ModalSchemaProperty
-	} from '$lib/common'
+	import { type Schema, modalToSchema, type ModalSchemaProperty } from '$lib/common'
 	import { emptySchema, sendUserToast } from '$lib/utils'
 	import { createEventDispatcher } from 'svelte'
 	import AddPropertyFormV2 from './AddPropertyFormV2.svelte'
@@ -34,30 +29,30 @@
 	let editing = false
 	let oldArgName: string | undefined // when editing argument and changing name
 
-	reorder()
+	reorder(schema)
 
-	function reorder() {
-		if (schema.order && Array.isArray(schema.order)) {
+	function reorder(s: Schema) {
+		if (s.order && Array.isArray(s.order)) {
 			const n = {}
 
-			;(schema.order as string[]).forEach((x) => {
-				if (schema.properties && schema.properties[x] != undefined) {
-					n[x] = schema.properties[x]
+			;(s.order as string[]).forEach((x) => {
+				if (s.properties && s.properties[x] != undefined) {
+					n[x] = s.properties[x]
 				}
 			})
 
-			Object.keys(schema.properties ?? {})
-				.filter((x) => !schema.order?.includes(x))
+			Object.keys(s.properties ?? {})
+				.filter((x) => !s.order?.includes(x))
 				.forEach((x) => {
-					n[x] = schema.properties[x]
+					n[x] = s.properties[x]
 				})
-			schema.properties = n
+			s.properties = n
 		}
 	}
 
-	function syncOrders() {
-		if (schema) {
-			schema.order = Object.keys(schema.properties ?? {})
+	function syncOrders(s: Schema) {
+		if (s) {
+			s.order = Object.keys(s.properties ?? {})
 		}
 	}
 
@@ -65,7 +60,6 @@
 		// If editing the arg's name, oldName containing the old argument name must be provided
 		argError = ''
 		modalProperty.name = modalProperty.name.trim()
-
 		if (modalProperty.name.length === 0) {
 			argError = 'Arguments need to have a name'
 		} else if (
@@ -74,54 +68,56 @@
 		) {
 			argError = 'There is already an argument with this name'
 		} else {
-			if (!schema.properties) {
-				schema.properties = {}
+			let newSchema = { ...schema }
+			if (!newSchema.properties) {
+				newSchema.properties = {}
 			}
-			if (!schema.required) {
-				schema.required = []
+			if (!newSchema.required) {
+				newSchema.required = []
 			}
-			if (!schema.order || !Array.isArray(schema.order)) {
-				syncOrders()
+			if (!newSchema.order || !Array.isArray(newSchema.order)) {
+				syncOrders(newSchema)
+			}
+			newSchema.properties = {
+				...newSchema.properties,
+				[modalProperty.name]: modalToSchema(modalProperty)
 			}
-			schema.properties[modalProperty.name] = modalToSchema(modalProperty)
 			if (modalProperty.required) {
-				if (!schema.required.includes(modalProperty.name)) {
-					schema.required.push(modalProperty.name)
+				if (!newSchema.required.includes(modalProperty.name)) {
+					newSchema.required.push(modalProperty.name)
 				}
-			} else if (schema.required.includes(modalProperty.name)) {
-				const index = schema.required.indexOf(modalProperty.name, 0)
+			} else if (newSchema.required.includes(modalProperty.name)) {
+				const index = newSchema.required.indexOf(modalProperty.name, 0)
 				if (index > -1) {
-					schema.required.splice(index, 1)
+					newSchema.required.splice(index, 1)
 				}
 			}
 
 			if (editing && oldArgName && oldArgName !== modalProperty.name) {
-				let oldPosition = schema.order.indexOf(oldArgName)
-				schema.order[oldPosition] = modalProperty.name
-				reorder()
-				handleDeleteArgument([oldArgName])
+				let oldPosition = newSchema.order.indexOf(oldArgName)
+				newSchema.order[oldPosition] = modalProperty.name
+				reorder(newSchema)
+				handleDeleteArgument([oldArgName], newSchema)
 			}
 
-			if (!schema.order?.includes(modalProperty.name)) {
-				schema.order.push(modalProperty.name)
+			if (!newSchema.order?.includes(modalProperty.name)) {
+				newSchema.order.push(modalProperty.name)
 			}
 			modalProperty = Object.assign({}, DEFAULT_PROPERTY)
 			editing = false
 			oldArgName = undefined
+			schema = $state.snapshot(newSchema)
+			dispatch('change', schema)
 		}
 
-		schema = $state.snapshot(schema)
-
 		if (argError !== '') {
 			sendUserToast(argError, true)
 		}
-
-		dispatch('change', schema)
 	}
 
-	export function handleDeleteArgument(argPath: string[]): void {
+	export function handleDeleteArgument(argPath: string[], nschema?: Schema): void {
 		try {
-			let modifiedObject: Schema | SchemaProperty = schema
+			let modifiedObject: Schema = { ...(nschema ?? schema) }
 			let modifiedProperties = modifiedObject.properties as object
 			let argName = argPath.pop() as string
 
@@ -147,8 +143,8 @@
 			} else {
 				throw Error('Argument not found!')
 			}
-			syncOrders()
-			schema = $state.snapshot(schema)
+			syncOrders(modifiedObject)
+			schema = $state.snapshot(modifiedObject)
 
 			dispatch('change', schema)
 		} catch (err) {
diff --git a/frontend/src/lib/components/schema/FlowPropertyEditor.svelte b/frontend/src/lib/components/schema/FlowPropertyEditor.svelte
index 19342ad49c..631212c974 100644
--- a/frontend/src/lib/components/schema/FlowPropertyEditor.svelte
+++ b/frontend/src/lib/components/schema/FlowPropertyEditor.svelte
@@ -250,13 +250,16 @@
 				bind:schema={
 					() => {
 						if (oneOf?.[idx]) {
+							let properties = Object.fromEntries(
+								Object.entries(oneOf[idx].properties ?? {}).filter(
+									([k]) => k !== 'label' && k !== 'kind'
+								)
+							)
 							return {
 								...oneOf[idx],
-								properties: Object.fromEntries(
-									Object.entries(oneOf[idx].properties ?? {}).filter(
-										([k]) => k !== 'label' && k !== 'kind'
-									)
-								)
+								properties: properties,
+								order: Object.keys(properties),
+								required: oneOf[idx].required ?? []
 							}
 						}
 					},
@@ -265,7 +268,6 @@
 							const tagKey = oneOf?.find((o) => Object.keys(o.properties ?? {}).includes('kind'))
 								? 'kind'
 								: 'label'
-
 							oneOf[idx] = {
 								...(v ?? {}),
 								type: 'object',