fix: harden composite input authoring
This commit is contained in:
@@ -77,14 +77,9 @@ const mergeInputBindings = (
|
||||
bindingKind(candidate) === kind &&
|
||||
bindingTargetKey(candidate) === target,
|
||||
);
|
||||
const fallbackIndex = serialized.findIndex(
|
||||
(candidate, candidateIndex) =>
|
||||
!used.has(candidateIndex) && bindingKind(candidate) === kind,
|
||||
);
|
||||
const selectedIndex = index === -1 ? fallbackIndex : index;
|
||||
if (selectedIndex !== -1) {
|
||||
used.add(selectedIndex);
|
||||
merged.push(serialized[selectedIndex]!);
|
||||
if (index !== -1) {
|
||||
used.add(index);
|
||||
merged.push(serialized[index]!);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -6,9 +6,95 @@ import { normalizeSchema } from "../schema-form/schema-field.js";
|
||||
import type { ExpressionEditorState } from "./input-expression-editor.js";
|
||||
import { InputExpressionControl } from "./InputExpressionControl.js";
|
||||
|
||||
afterEach(() => cleanup());
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("InputExpressionControl", () => {
|
||||
it("offers construct mode for an unconstrained target", async () => {
|
||||
const user = userEvent.setup();
|
||||
const Harness = () => {
|
||||
const [state, setState] = useState<ExpressionEditorState>({
|
||||
kind: "literal",
|
||||
value: null,
|
||||
touched: false,
|
||||
});
|
||||
return (
|
||||
<InputExpressionControl
|
||||
field={null}
|
||||
label="payload"
|
||||
onChange={setState}
|
||||
state={state}
|
||||
/>
|
||||
);
|
||||
};
|
||||
|
||||
render(<Harness />);
|
||||
await user.selectOptions(
|
||||
screen.getByRole("combobox", { name: "Value source for payload" }),
|
||||
"construct",
|
||||
);
|
||||
|
||||
expect(screen.getByRole("group", { name: "payload" })).toBeInTheDocument();
|
||||
expect(screen.getByRole("button", { name: "Add item to payload" })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("removes and restores an optional declared property", async () => {
|
||||
const user = userEvent.setup();
|
||||
const Harness = () => {
|
||||
const [state, setState] = useState<ExpressionEditorState>({
|
||||
kind: "object",
|
||||
fields: [{ name: "note", value: { kind: "literal", value: "", touched: false } }],
|
||||
});
|
||||
return (
|
||||
<InputExpressionControl
|
||||
field={normalizeSchema({
|
||||
type: "object",
|
||||
properties: { note: { type: "string" } },
|
||||
})}
|
||||
label="payload"
|
||||
onChange={setState}
|
||||
state={state}
|
||||
/>
|
||||
);
|
||||
};
|
||||
|
||||
render(<Harness />);
|
||||
await user.click(screen.getByRole("button", { name: "Remove payload.note" }));
|
||||
expect(screen.queryByRole("group", { name: "payload.note" })).toBeNull();
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Add optional property note to payload" }));
|
||||
expect(screen.getByRole("group", { name: "payload.note" })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("repairs one duplicate object field without changing the other", async () => {
|
||||
const user = userEvent.setup();
|
||||
const Harness = () => {
|
||||
const [state, setState] = useState<ExpressionEditorState>({
|
||||
kind: "object",
|
||||
fields: [
|
||||
{ name: "duplicate", value: { kind: "literal", value: "first", touched: false } },
|
||||
{ name: "duplicate", value: { kind: "literal", value: "second", touched: false } },
|
||||
],
|
||||
});
|
||||
return (
|
||||
<InputExpressionControl
|
||||
field={normalizeSchema({ type: "object", additionalProperties: true })}
|
||||
label="payload"
|
||||
onChange={setState}
|
||||
state={state}
|
||||
/>
|
||||
);
|
||||
};
|
||||
|
||||
render(<Harness />);
|
||||
const removeButtons = screen.getAllByRole("button", { name: "Remove payload field duplicate" });
|
||||
await user.click(removeButtons[0]!);
|
||||
|
||||
expect(screen.getAllByRole("group", { name: "payload field duplicate" })).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("adds, removes, and reorders array expressions with accessible controls", async () => {
|
||||
const user = userEvent.setup();
|
||||
let state: ExpressionEditorState = { kind: "array", items: [] };
|
||||
@@ -314,7 +400,6 @@ describe("InputExpressionControl", () => {
|
||||
await user.click(screen.getByRole("button", { name: "External replace alias" }));
|
||||
|
||||
expect(consoleError.mock.calls.flat().join(" ")).not.toContain("same key");
|
||||
consoleError.mockRestore();
|
||||
});
|
||||
|
||||
it("transfers the edited occurrence identity when aliased children diverge", async () => {
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { useId, useState } from "react";
|
||||
import { SchemaFieldControl } from "../schema-form/SchemaFieldControl.js";
|
||||
import { rebaseSchemaField, type SchemaField } from "../schema-form/schema-field.js";
|
||||
import {
|
||||
rebaseSchemaField,
|
||||
UNCONSTRAINED_SCHEMA_REASON,
|
||||
type SchemaField,
|
||||
} from "../schema-form/schema-field.js";
|
||||
import type { FieldSources } from "../schema-form/schema-values.js";
|
||||
import {
|
||||
defaultExpressionEditorState,
|
||||
@@ -17,7 +21,10 @@ export type InputExpressionControlProps = {
|
||||
};
|
||||
|
||||
const isConstructField = (field: SchemaField | null): boolean =>
|
||||
field?.kind === "array" || field?.kind === "object";
|
||||
field === null ||
|
||||
field.kind === "array" ||
|
||||
field.kind === "object" ||
|
||||
field.fallbackReason === UNCONSTRAINED_SCHEMA_REASON;
|
||||
|
||||
const valueSourceFor = (state: ExpressionEditorState): "path" | "literal" | "construct" =>
|
||||
state.kind === "array" || state.kind === "object" ? "construct" : state.kind;
|
||||
@@ -44,7 +51,13 @@ const stateForSource = (
|
||||
};
|
||||
})();
|
||||
}
|
||||
return isConstructField(field) ? defaultExpressionEditorState(field) : current;
|
||||
if (!isConstructField(field)) return current;
|
||||
if (field?.kind === "array" || field?.kind === "object") {
|
||||
return defaultExpressionEditorState(field);
|
||||
}
|
||||
return current.kind === "array" || current.kind === "object"
|
||||
? current
|
||||
: { kind: "array", items: [] };
|
||||
};
|
||||
|
||||
const fieldForName = (field: SchemaField | null, name: string): SchemaField | null => {
|
||||
@@ -53,7 +66,7 @@ const fieldForName = (field: SchemaField | null, name: string): SchemaField | nu
|
||||
(field.additionalPropertiesKind === "schema" ? field.additionalProperty : null);
|
||||
};
|
||||
|
||||
const missingRequiredProperties = (
|
||||
const missingDeclaredProperties = (
|
||||
field: SchemaField | null,
|
||||
fields: ReadonlyArray<{ readonly name: string }>,
|
||||
): ReadonlyArray<SchemaField> => {
|
||||
@@ -62,7 +75,7 @@ const missingRequiredProperties = (
|
||||
for (const entry of fields) presentNames.add(entry.name);
|
||||
const missing: SchemaField[] = [];
|
||||
for (const child of field.children) {
|
||||
if (child.required && !presentNames.has(child.key)) missing.push(child);
|
||||
if (!presentNames.has(child.key)) missing.push(child);
|
||||
}
|
||||
return missing;
|
||||
};
|
||||
@@ -73,7 +86,7 @@ const labelForName = (label: string, name: string): string =>
|
||||
const pathNeedsDeferredValidation = (field: SchemaField | null, path: string): boolean =>
|
||||
path.startsWith("context.") ||
|
||||
field === null ||
|
||||
field.fallbackReason === "The schema is unconstrained; edit JSON directly.";
|
||||
field.fallbackReason === UNCONSTRAINED_SCHEMA_REASON;
|
||||
|
||||
const safeIdSuffix = (value: string): string =>
|
||||
value.replaceAll(/[^a-zA-Z0-9_-]/g, "-");
|
||||
@@ -362,9 +375,10 @@ export const InputExpressionControl = ({
|
||||
<fieldset className="input-expression-control__construct" aria-label={label}>
|
||||
<legend>{label}</legend>
|
||||
{field?.description && <p>{field.description}</p>}
|
||||
{state.fields.map((entry) => {
|
||||
{state.fields.map((entry, fieldIndex) => {
|
||||
const childField = fieldForName(field, entry.name);
|
||||
const declaredField = field?.kind === "object" && field.children.some((child) => child.key === entry.name);
|
||||
const requiredField = field?.kind === "object" &&
|
||||
field.children.some((child) => child.key === entry.name && child.required);
|
||||
const childLabel = childField === null && field?.additionalPropertiesKind !== "schema"
|
||||
? labelForName(label, entry.name)
|
||||
: `${label}.${entry.name}`;
|
||||
@@ -372,28 +386,28 @@ export const InputExpressionControl = ({
|
||||
<fieldset
|
||||
aria-label={childLabel}
|
||||
className="input-expression-control__field"
|
||||
key={entry.name}
|
||||
key={`${fieldIndex}-${entry.name}`}
|
||||
>
|
||||
<InputExpressionControl
|
||||
field={childField}
|
||||
label={childLabel}
|
||||
onChange={(next) => onChange({
|
||||
kind: "object",
|
||||
fields: state.fields.map((candidate) => candidate.name === entry.name
|
||||
fields: state.fields.map((candidate, candidateIndex) => candidateIndex === fieldIndex
|
||||
? { ...candidate, value: next }
|
||||
: candidate),
|
||||
})}
|
||||
sourceSuggestions={sourceSuggestions}
|
||||
state={entry.value}
|
||||
/>
|
||||
{!declaredField && (
|
||||
{!requiredField && (
|
||||
<div className="input-expression-control__item-actions">
|
||||
<button
|
||||
aria-label={`Remove ${childLabel}`}
|
||||
className="schema-form__secondary-action"
|
||||
onClick={() => onChange({
|
||||
kind: "object",
|
||||
fields: state.fields.filter((candidate) => candidate.name !== entry.name),
|
||||
fields: state.fields.filter((_, candidateIndex) => candidateIndex !== fieldIndex),
|
||||
})}
|
||||
type="button"
|
||||
>
|
||||
@@ -404,9 +418,9 @@ export const InputExpressionControl = ({
|
||||
</fieldset>
|
||||
);
|
||||
})}
|
||||
{missingRequiredProperties(field, state.fields).map((child) => (
|
||||
{missingDeclaredProperties(field, state.fields).map((child) => (
|
||||
<button
|
||||
aria-label={`Add required property ${child.key} to ${label}`}
|
||||
aria-label={`Add ${child.required ? "required" : "optional"} property ${child.key} to ${label}`}
|
||||
className="schema-form__secondary-action"
|
||||
key={`required-${child.key}`}
|
||||
onClick={() => onChange({
|
||||
@@ -418,7 +432,7 @@ export const InputExpressionControl = ({
|
||||
})}
|
||||
type="button"
|
||||
>
|
||||
Add required property {child.key}
|
||||
Add {child.required ? "required" : "optional"} property {child.key}
|
||||
</button>
|
||||
))}
|
||||
{(field?.additionalPropertiesKind === "allowed" || field?.additionalPropertiesKind === "schema") && (
|
||||
|
||||
@@ -11,7 +11,11 @@ import {
|
||||
hasBoundedInputExpressionLiteralValue,
|
||||
hasBoundedInputExpressionNodeBudget,
|
||||
} from "@lda/workflow-rpc/input-expression-limits";
|
||||
import { normalizeSchema, type SchemaField } from "../schema-form/schema-field.js";
|
||||
import {
|
||||
normalizeSchema,
|
||||
UNCONSTRAINED_SCHEMA_REASON,
|
||||
type SchemaField,
|
||||
} from "../schema-form/schema-field.js";
|
||||
import { formatTOMLPath, parseGraphSourcePath, parseTOMLPath } from "../schema-form/schema-paths.js";
|
||||
|
||||
export type ExpressionEditorState =
|
||||
@@ -137,7 +141,7 @@ const unsupported = (raw: InputExpression, reason: string): ExpressionProjection
|
||||
|
||||
const schemaReason = (field: SchemaField | null): string | null => {
|
||||
if (field?.fallbackReason === null || field?.fallbackReason === undefined) return null;
|
||||
if (field.fallbackReason === "The schema is unconstrained; edit JSON directly.") return null;
|
||||
if (field.fallbackReason === UNCONSTRAINED_SCHEMA_REASON) return null;
|
||||
return field.fallbackReason;
|
||||
};
|
||||
|
||||
@@ -236,7 +240,7 @@ export const serializeExpressionEditorState = (
|
||||
const issue = (path: ReadonlyArray<string | number>, message: string): ExpressionValidationIssue => ({ path, message });
|
||||
|
||||
const unconstrained = (field: SchemaField | null): boolean =>
|
||||
field === null || field.fallbackReason === "The schema is unconstrained; edit JSON directly.";
|
||||
field === null || field.fallbackReason === UNCONSTRAINED_SCHEMA_REASON;
|
||||
|
||||
const literalIssues = (
|
||||
value: unknown,
|
||||
|
||||
@@ -19,6 +19,8 @@ export type SchemaField = {
|
||||
readonly fallbackReason: string | null;
|
||||
};
|
||||
|
||||
export const UNCONSTRAINED_SCHEMA_REASON = "The schema is unconstrained; edit JSON directly.";
|
||||
|
||||
export type FieldSource =
|
||||
| { readonly mode: "literal"; readonly value: unknown }
|
||||
| { readonly mode: "bind"; readonly sourcePath: string };
|
||||
@@ -222,7 +224,7 @@ const normalizeField = (
|
||||
|
||||
const type = resolvedSchema.type;
|
||||
if (type === undefined) {
|
||||
return fallback(resolvedSchema, path, key, required, title, "The schema is unconstrained; edit JSON directly.");
|
||||
return fallback(resolvedSchema, path, key, required, title, UNCONSTRAINED_SCHEMA_REASON);
|
||||
}
|
||||
|
||||
if (type === "object") {
|
||||
|
||||
Reference in New Issue
Block a user