mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-23 03:27:50 +09:00
fix(dashboard): allow editing free-form objects in application forms (#4682)
## What this PR does Free-form object fields, such as NATS `config.merge` and `config.resolver`, get a JSON editor. The form keeps nested objects, arrays and primitive values. Input that is not a JSON object stays visible, and schema validation blocks the create or update request until it is fixed. A JSON object that fails the schema marks the field invalid and points it at the error list. Typed maps keep their key and value editor, and read-only and immutable fields stay protected. Fixes #4676. ### Validation - All 463 console tests in 63 files pass, and so does the TypeScript check. - Tests cover typing into the editor, nested NATS objects next to the typed users editor, input that is not a JSON object, schema errors on a valid object, existing values, immutable fields and free-form objects inside arrays. On the order page they cover a create after a form and YAML round trip, saving an edited object, and blocking input that is not JSON in create and edit mode. - The two schema-error tests fail on the component without the change. ### Screenshots   ### Downstream repositories This change preserves the public resource schema and client contract. The file-level trigger map was reviewed; no companion repository update is required. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(dashboard): allow editing free-form objects in application forms ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a JSON editor for configuration fields that accept objects with arbitrary properties. Edit nested values directly, with changes retained when switching between form and YAML views. * **Bug Fixes** * Invalid JSON is flagged and prevents create or update submissions until corrected. The editor supports empty objects, read-only fields, and objects nested in arrays, and makes validation errors accessible. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
4 files changed
+288
No files matched your search
+122
@@ -0,0 +1,122 @@
|
||||
import { createRef, useState } from "react"
|
||||
import { describe, it, expect, vi } from "vitest"
|
||||
import { act, fireEvent, render, screen } from "@testing-library/react"
|
||||
import type { FieldProps } from "@rjsf/utils"
|
||||
import { SchemaForm, type SchemaFormHandle } from "./SchemaForm.tsx"
|
||||
import { FreeformObjectField } from "./FreeformObjectField.tsx"
|
||||
import { IMMUTABLE_HELP_TEXT } from "../lib/immutable-paths.ts"
|
||||
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
users: { type: "object", additionalProperties: { type: "object", properties: { password: { type: "string" } } } },
|
||||
config: { type: "object", properties: {
|
||||
merge: { type: "object", default: {}, "x-kubernetes-preserve-unknown-fields": true },
|
||||
resolver: { type: "object", additionalProperties: true },
|
||||
} },
|
||||
},
|
||||
}
|
||||
|
||||
function setup(initial: unknown = { users: {}, config: { merge: {}, resolver: {} } }, openAPISchema: unknown = schema, immutableMode?: "enforce") {
|
||||
const ref = createRef<SchemaFormHandle>()
|
||||
const changed = vi.fn()
|
||||
function Controlled() {
|
||||
const [data, setData] = useState(initial)
|
||||
return <SchemaForm ref={ref} openAPISchema={JSON.stringify(openAPISchema)} formData={data} onChange={(next) => { changed(next); setData(next) }} immutableMode={immutableMode} />
|
||||
}
|
||||
render(<Controlled />)
|
||||
const validate = () => {
|
||||
let valid = false
|
||||
act(() => { valid = ref.current!.validate() })
|
||||
return valid
|
||||
}
|
||||
return { changed, validate }
|
||||
}
|
||||
|
||||
describe("free-form object editing", () => {
|
||||
it("preserves typed whitespace instead of reformatting each valid keystroke", () => {
|
||||
const { changed, validate } = setup()
|
||||
const text = '{ "jetstream": true, "subjects": ["one"] }\n'
|
||||
fireEvent.change(screen.getByLabelText("merge"), { target: { value: text } })
|
||||
expect(screen.getByLabelText("merge")).toHaveValue(text)
|
||||
expect(changed.mock.lastCall![0].config.merge).toEqual({ jetstream: true, subjects: ["one"] })
|
||||
expect(validate()).toBe(true)
|
||||
})
|
||||
|
||||
it("edits both NATS objects and preserves nested types and the typed users editor", () => {
|
||||
const { changed, validate } = setup()
|
||||
const merge = { accounts: { A: { jetstream: { max_streams: 2 }, enabled: false, subjects: ["a", "b"], extra: null } } }
|
||||
fireEvent.change(screen.getByLabelText("merge"), { target: { value: JSON.stringify(merge) } })
|
||||
fireEvent.change(screen.getByLabelText("resolver"), { target: { value: '{"type":"full","dir":"/data"}' } })
|
||||
expect(changed.mock.lastCall![0].config).toEqual({ merge, resolver: { type: "full", dir: "/data" } })
|
||||
expect(screen.getByPlaceholderText("Enter key name...")).toBeInTheDocument()
|
||||
expect(validate()).toBe(true)
|
||||
})
|
||||
|
||||
it.each(["{", "", "[]", "null", "12", '"text"'])("blocks submission for %j and recovers after correction", (input) => {
|
||||
const { validate } = setup()
|
||||
fireEvent.change(screen.getByLabelText("merge"), { target: { value: input } })
|
||||
expect(screen.getByLabelText("merge")).toHaveValue(input)
|
||||
expect(screen.getByText("Enter a valid JSON object.")).toBeInTheDocument()
|
||||
expect(validate()).toBe(false)
|
||||
fireEvent.change(screen.getByLabelText("merge"), { target: { value: '{"jetstream":true}' } })
|
||||
expect(validate()).toBe(true)
|
||||
})
|
||||
|
||||
it("renders existing values and allows replacing them with an empty object", () => {
|
||||
const { changed, validate } = setup({ config: { merge: { accounts: { A: { jetstream: true } } }, resolver: {} } })
|
||||
expect(JSON.parse((screen.getByLabelText("merge") as HTMLTextAreaElement).value)).toEqual({ accounts: { A: { jetstream: true } } })
|
||||
fireEvent.change(screen.getByLabelText("merge"), { target: { value: "{}" } })
|
||||
expect(changed.mock.lastCall![0].config.merge).toEqual({})
|
||||
expect(validate()).toBe(true)
|
||||
})
|
||||
|
||||
it("keeps immutable objects disabled with their explanation", () => {
|
||||
setup({ settings: { enabled: true } }, { type: "object", properties: { settings: { type: "object", additionalProperties: true, "x-kubernetes-validations": [{ rule: "self == oldSelf" }] } } }, "enforce")
|
||||
expect(screen.getByLabelText("settings")).toBeDisabled()
|
||||
expect(screen.getByText(IMMUTABLE_HELP_TEXT)).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it("marks a valid JSON object that fails the schema as invalid and points at its errors", () => {
|
||||
const { validate } = setup({ settings: {} }, { type: "object", properties: { settings: { type: "object", additionalProperties: true, minProperties: 1 } } })
|
||||
const field = screen.getByLabelText("settings")
|
||||
expect(field).toHaveAttribute("aria-invalid", "false")
|
||||
expect(validate()).toBe(false)
|
||||
expect(field).toHaveAttribute("aria-invalid", "true")
|
||||
expect(field.getAttribute("aria-describedby")!.split(" ")).toContain("root_settings__error")
|
||||
expect(document.getElementById("root_settings__error")).toBeInTheDocument()
|
||||
expect(screen.queryByText("Enter a valid JSON object.")).not.toBeInTheDocument()
|
||||
fireEvent.change(field, { target: { value: '{"enabled":true}' } })
|
||||
expect(validate()).toBe(true)
|
||||
expect(field).toHaveAttribute("aria-invalid", "false")
|
||||
expect(field.getAttribute("aria-describedby")!.split(" ")).not.toContain("root_settings__error")
|
||||
})
|
||||
|
||||
it("does not point at an error list that hideError keeps from rendering", () => {
|
||||
const props = {
|
||||
schema: { type: "object", additionalProperties: true },
|
||||
formData: {},
|
||||
onChange: vi.fn(),
|
||||
idSchema: { $id: "root_settings" },
|
||||
name: "settings",
|
||||
required: false,
|
||||
readonly: false,
|
||||
disabled: false,
|
||||
uiSchema: {},
|
||||
rawErrors: ["must NOT have fewer than 1 properties"],
|
||||
hideError: true,
|
||||
registry: {},
|
||||
} as unknown as FieldProps
|
||||
render(<FreeformObjectField {...props} />)
|
||||
const field = screen.getByLabelText("settings")
|
||||
expect(field).toHaveAttribute("aria-invalid", "true")
|
||||
expect(field.getAttribute("aria-describedby")!.split(" ")).not.toContain("root_settings__error")
|
||||
})
|
||||
|
||||
it("reaches free-form objects nested in arrays", () => {
|
||||
const { changed, validate } = setup({ items: [{ options: {} }] }, { type: "object", properties: { items: { type: "array", items: { type: "object", properties: { options: { type: "object", additionalProperties: true } } } } } })
|
||||
fireEvent.change(screen.getByLabelText("options"), { target: { value: '{"nested":[1,true]}' } })
|
||||
expect(changed.mock.lastCall![0].items[0].options).toEqual({ nested: [1, true] })
|
||||
expect(validate()).toBe(true)
|
||||
})
|
||||
})
|
||||
+66
@@ -0,0 +1,66 @@
|
||||
import { useState } from "react"
|
||||
import { errorId } from "@rjsf/utils"
|
||||
import type { FieldProps } from "@rjsf/utils"
|
||||
|
||||
function displayValue(value: unknown) {
|
||||
return typeof value === "string" ? value : JSON.stringify(value ?? {}, null, 2)
|
||||
}
|
||||
|
||||
export function FreeformObjectField({
|
||||
schema, formData, onChange, idSchema, name, required, readonly, disabled, uiSchema, rawErrors, hideError,
|
||||
}: FieldProps) {
|
||||
const source = displayValue(formData)
|
||||
const [draft, setDraft] = useState<{ source: string, text: string } | null>(null)
|
||||
const value = draft?.source === source ? draft.text : source
|
||||
const invalid = typeof formData === "string"
|
||||
// A parsed object can still fail the schema, and RJSF lists those errors
|
||||
// under the field template rather than here.
|
||||
const hasSchemaErrors = Boolean(rawErrors?.length)
|
||||
const helpId = `${idSchema.$id}__json_help`
|
||||
const describedBy = [
|
||||
helpId,
|
||||
...(uiSchema?.["ui:help"] ? [`${idSchema.$id}__help`] : []),
|
||||
...(hasSchemaErrors && !hideError ? [errorId(idSchema)] : []),
|
||||
].join(" ")
|
||||
|
||||
return (
|
||||
<div className="form-group field">
|
||||
<label htmlFor={idSchema.$id} className="control-label mb-2 block text-sm font-medium text-slate-700">
|
||||
{schema.title ?? name}
|
||||
{required && <span className="required ml-1 text-red-500">*</span>}
|
||||
</label>
|
||||
{schema.description && <p className="mb-3 text-xs text-slate-500">{schema.description}</p>}
|
||||
<textarea
|
||||
id={idSchema.$id}
|
||||
value={value}
|
||||
rows={8}
|
||||
readOnly={readonly}
|
||||
disabled={disabled}
|
||||
spellCheck={false}
|
||||
aria-invalid={invalid || hasSchemaErrors}
|
||||
aria-describedby={describedBy}
|
||||
className="w-full rounded-lg border border-slate-300 bg-white p-3 font-mono text-sm text-slate-900 outline-none focus:border-blue-400 focus:ring-1 focus:ring-blue-400 disabled:opacity-50"
|
||||
onChange={(event) => {
|
||||
const text = event.target.value
|
||||
const change = (next: unknown) => {
|
||||
setDraft({ source: displayValue(next), text })
|
||||
onChange(next)
|
||||
}
|
||||
try {
|
||||
const parsed: unknown = JSON.parse(text)
|
||||
if (parsed !== null && typeof parsed === "object" && !Array.isArray(parsed)) {
|
||||
change(parsed)
|
||||
return
|
||||
}
|
||||
} catch {
|
||||
// Keep incomplete input in formData so schema validation blocks submission.
|
||||
}
|
||||
change(text)
|
||||
}}
|
||||
/>
|
||||
<p id={helpId} className={invalid ? "mt-1 text-xs text-red-600" : "mt-1 text-xs text-slate-500"}>
|
||||
{invalid ? "Enter a valid JSON object." : "Enter a JSON object. Nested objects and arrays are supported."}
|
||||
</p>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
import { customTemplates, customWidgets } from "./rjsf-registry.ts"
|
||||
import { addDynamicOptionWidgets } from "../lib/dynamic-options.ts"
|
||||
import { AdditionalPropertiesField } from "./AdditionalPropertiesField.tsx"
|
||||
import { FreeformObjectField } from "./FreeformObjectField.tsx"
|
||||
import { ResourceQuotasField } from "./ResourceQuotasField.tsx"
|
||||
import { SourceField } from "./SourceField.tsx"
|
||||
import "./schema-form.css"
|
||||
@@ -66,6 +67,14 @@ function bindAdditionalProperties(
|
||||
): UiSchema | undefined {
|
||||
const node: SchemaNode = fieldSchema
|
||||
|
||||
if (
|
||||
node.type === "object" &&
|
||||
(!node.properties || Object.keys(node.properties).length === 0) &&
|
||||
node.additionalProperties === true
|
||||
) {
|
||||
return { ...uiNode, "ui:field": "FreeformObjectField" }
|
||||
}
|
||||
|
||||
const isAdditionalPropertiesMap =
|
||||
node.type === "object" &&
|
||||
(!node.properties || Object.keys(node.properties).length === 0) &&
|
||||
@@ -315,6 +324,7 @@ export const SchemaForm = forwardRef<SchemaFormHandle, SchemaFormProps>(function
|
||||
const customFields = useMemo(
|
||||
() => ({
|
||||
AdditionalPropertiesField: AdditionalPropertiesField,
|
||||
FreeformObjectField: FreeformObjectField,
|
||||
ResourceQuotasField: ResourceQuotasField,
|
||||
SourceField: SourceField,
|
||||
}),
|
||||
|
||||
+90
@@ -0,0 +1,90 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest"
|
||||
import { fireEvent, render, screen } from "@testing-library/react"
|
||||
import userEvent from "@testing-library/user-event"
|
||||
import { MemoryRouter } from "react-router"
|
||||
|
||||
const h = vi.hoisted(() => ({
|
||||
create: vi.fn(),
|
||||
update: vi.fn(),
|
||||
ad: {
|
||||
spec: {
|
||||
application: {
|
||||
kind: "NATS", plural: "natses", singular: "nats",
|
||||
openAPISchema: JSON.stringify({
|
||||
type: "object",
|
||||
properties: {
|
||||
config: {
|
||||
type: "object",
|
||||
properties: {
|
||||
merge: { type: "object", "x-kubernetes-preserve-unknown-fields": true },
|
||||
resolver: { type: "object", additionalProperties: true },
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
},
|
||||
dashboard: {},
|
||||
},
|
||||
},
|
||||
}))
|
||||
|
||||
vi.mock("../lib/tenant-context.tsx", () => ({
|
||||
useTenantContext: () => ({ tenantNamespace: "tenant-test" }),
|
||||
}))
|
||||
vi.mock("../lib/app-definitions.ts", () => ({
|
||||
useApplicationDefinition: () => ({ data: h.ad, isLoading: false, error: null }),
|
||||
appDisplayName: () => "NATS", iconDataUrl: () => null,
|
||||
}))
|
||||
vi.mock("@cozystack/k8s-client", () => ({
|
||||
useK8sCreate: () => ({ mutateAsync: h.create, isPending: false }),
|
||||
useK8sUpdate: () => ({ mutateAsync: h.update, isPending: false }),
|
||||
K8sApiError: class extends Error {},
|
||||
}))
|
||||
vi.mock("../components/YamlEditor.tsx", () => ({
|
||||
YamlEditor: ({ value }: { value: string }) => <textarea data-testid="yaml" readOnly value={value} />,
|
||||
}))
|
||||
|
||||
const { ApplicationOrderPage } = await import("./ApplicationOrderPage.tsx")
|
||||
const merge = { accounts: { A: { jetstream: { max_streams: 2 }, subjects: ["one"], enabled: false } } }
|
||||
|
||||
describe("ApplicationOrderPage free-form objects", () => {
|
||||
beforeEach(() => {
|
||||
h.create.mockReset().mockResolvedValue({})
|
||||
h.update.mockReset().mockResolvedValue({})
|
||||
})
|
||||
|
||||
it("sends nested objects on create after a form/YAML round trip", async () => {
|
||||
const user = userEvent.setup()
|
||||
render(<MemoryRouter><ApplicationOrderPage appNameOverride="nats" /></MemoryRouter>)
|
||||
await user.type(screen.getByPlaceholderText("nats"), "demo")
|
||||
fireEvent.change(screen.getByLabelText("merge"), { target: { value: JSON.stringify(merge) } })
|
||||
fireEvent.change(screen.getByLabelText("resolver"), { target: { value: '{"type":"full"}' } })
|
||||
await user.click(screen.getByRole("button", { name: /^yaml$/i }))
|
||||
await screen.findByTestId("yaml")
|
||||
await user.click(screen.getByRole("button", { name: /^form$/i }))
|
||||
expect(JSON.parse((screen.getByLabelText("merge") as HTMLTextAreaElement).value)).toEqual(merge)
|
||||
await user.click(screen.getByRole("button", { name: /^deploy$/i }))
|
||||
expect(h.create).toHaveBeenCalledOnce()
|
||||
expect(h.create.mock.calls[0][0].spec.config).toEqual({ merge, resolver: { type: "full" } })
|
||||
})
|
||||
|
||||
it("preserves existing nested settings and saves an edited object", async () => {
|
||||
const user = userEvent.setup()
|
||||
render(<MemoryRouter><ApplicationOrderPage appNameOverride="nats" editMode={{ name: "demo", initialSpec: { config: { merge, resolver: { type: "full" } } } }} /></MemoryRouter>)
|
||||
expect(JSON.parse((screen.getByLabelText("merge") as HTMLTextAreaElement).value)).toEqual(merge)
|
||||
fireEvent.change(screen.getByLabelText("resolver"), { target: { value: "{}" } })
|
||||
await user.click(screen.getByRole("button", { name: /^save$/i }))
|
||||
expect(h.update).toHaveBeenCalledOnce()
|
||||
expect(h.update.mock.calls[0][0].spec.config).toEqual({ merge, resolver: {} })
|
||||
})
|
||||
|
||||
it.each([false, true])("blocks invalid JSON from the API in edit mode %s", async (edit) => {
|
||||
const user = userEvent.setup()
|
||||
render(<MemoryRouter><ApplicationOrderPage appNameOverride="nats" editMode={edit ? { name: "demo", initialSpec: { config: { merge } } } : undefined} /></MemoryRouter>)
|
||||
if (!edit) await user.type(screen.getByPlaceholderText("nats"), "demo")
|
||||
fireEvent.change(screen.getByLabelText("merge"), { target: { value: "{" } })
|
||||
await user.click(screen.getByRole("button", { name: edit ? /^save$/i : /^deploy$/i }))
|
||||
expect(h.create).not.toHaveBeenCalled()
|
||||
expect(h.update).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
Reference in new issue
Block a user