This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8438-357d9013e14bac3bcfc7a8bf6ddfdb13937bd55a in repository https://gitbox.apache.org/repos/asf/texera.git
commit d0ab10d7d63b4e40bbea15a0ea1a285c886fd9ef Author: yangzhang75 <[email protected]> AuthorDate: Sun Sep 6 23:42:08 2026 +0000 feat(workflow-form): render nested and array sub-field inputs (#8438) ### What changes were proposed in this PR? Closes #8022. Part of the Form View stack (parent issue #8011), stacked on #8437 (PR10) and #8436 (PR9). The render PR (#8437) shows the top-level exposed inputs. This one handles a property that carries sub-fields: an object (a nested group) or a repeated section (an array). It renders each sub-field and applies the author's per-sub-field setup. - A walk over the built formly field and its sub-fields drops the operator schema's own per-field descriptions (author notes about the operator, not guidance to a form reader, and shown once per field by formly), and applies the author's stored overrides -- rename and hide -- keyed by field path, with array indices dropped so one entry covers every row. - A repeated section builds its row template on demand, so the walk wraps the builder rather than the single object it returns, decorating every row formly ever creates. - Two small statics, `childPath` (the override path for a child, indices dropped) and `arrayItemOf` (a repeated section's row template, whether formly gives it as a value or a builder), back the keying and are unit tested directly. Writing a filled-in value back to its operator is already handled by #8437: formly's model carries the nested value, so a sub-field edit persists through the same write-back. This slice is the sub-field rendering and overrides only. ### Any related issues, documentation, discussions? Closes #8022. Part of the Form View feature (parent issue #8011). ### How was this PR tested? Unit tests (vitest). `sub-fields.spec.ts` covers the two statics directly (path joining, index dropping, non-name keys; the value/builder/throwing/none row-template cases). `workflow-form.component.spec.ts` covers the walk against the component: renaming and hiding an overridden sub-field of both an object property and a repeated section (per row, through the wrapped builder), dropping the schema descriptions, leaving an un-overridden sub-field untouched, and a scalar array's leaf row template. 100% statement and function coverage on the changed source; the remaining uncovered branches are defensive `??`/`||` fallbacks. `ng build gui`, eslint and prettier are clean, and the full workflow-form suite (73 tests) is green in both the single-user and collaboration paths. The visible effect (with the flag on) is that an exposed nested or array property renders its sub-fields, each renamed or hidden as the author set up. A screenshot can be added on request. #### Screenshot An exposed nested or array property rendered with its **sub-fields**, each renamed or hidden as the author set up. <img width="1271" height="785" alt="Screenshot 2026-09-05 at 11 42 53 AM" src="https://github.com/user-attachments/assets/62233d72-c64b-421d-b637-2dce4e742405"; /> ### Was this PR authored or co-authored using generative AI tooling? Yes. Co-authored with Claude (Anthropic), reviewed line by line by the author before submission. Co-authored-by: Claude Opus 4.8 <[email protected]> --- .../component/workflow-form/sub-fields.spec.ts | 65 +++++++++++ .../workflow-form/workflow-form.component.spec.ts | 120 ++++++++++++++++++++ .../workflow-form/workflow-form.component.ts | 124 +++++++++++++++++++-- .../workflow-form/workflow-form.spec-harness.ts | 42 +++++++ 4 files changed, 344 insertions(+), 7 deletions(-) diff --git a/frontend/src/app/workspace/component/workflow-form/sub-fields.spec.ts b/frontend/src/app/workspace/component/workflow-form/sub-fields.spec.ts new file mode 100644 index 0000000000..f318ad94df --- /dev/null +++ b/frontend/src/app/workspace/component/workflow-form/sub-fields.spec.ts @@ -0,0 +1,65 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { WorkflowFormComponent } from "./workflow-form.component"; + +// The two statics that back per-sub-field rename/hide: locating a sub-field means walking formly's +// output, which is a plain box, an object with a fieldGroup, or a repeated section whose row +// template is either a value or a builder function. +describe("sub-fields of an input", () => { + describe("override paths", () => { + it("joins nested keys", () => { + expect(WorkflowFormComponent.childPath("pairs", "key")).toBe("pairs.key"); + expect(WorkflowFormComponent.childPath("", "value")).toBe("value"); + }); + + it("drops array indices so every row shares one override", () => { + expect(WorkflowFormComponent.childPath("pairs", "0")).toBe("pairs"); + expect(WorkflowFormComponent.childPath("pairs", "12")).toBe("pairs"); + }); + + it("ignores keys that are not names", () => { + expect(WorkflowFormComponent.childPath("pairs", undefined)).toBe("pairs"); + expect(WorkflowFormComponent.childPath("", "key")).toBe("key"); + }); + }); + + describe("finding the row template of a repeated section", () => { + it("takes it directly when it is a value", () => { + const item = { key: "row" }; + expect(WorkflowFormComponent.arrayItemOf({ fieldArray: item })).toBe(item); + }); + + it("calls it when formly supplies a builder", () => { + const item = { key: "row" }; + expect(WorkflowFormComponent.arrayItemOf({ fieldArray: () => item })).toBe(item); + }); + + it("gives up quietly on a builder it cannot call", () => { + const throwing = () => { + throw new Error("needs a real field"); + }; + expect(WorkflowFormComponent.arrayItemOf({ fieldArray: throwing as never })).toBeUndefined(); + }); + + it("has nothing to offer when there is no array", () => { + expect(WorkflowFormComponent.arrayItemOf({ key: "plain" })).toBeUndefined(); + }); + }); +}); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts index 3c747ae58b..543eb94005 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts @@ -685,6 +685,126 @@ describe("WorkflowFormComponent", () => { }); }); + // A nested (object) or repeated (array) property carries sub-fields; the author can rename and + // hide each one, and the schema's own per-field notes are dropped so only the author's help text + // guides a reader. Overrides are keyed by field path, array indices dropped. + describe("nested and array sub-fields", () => { + // Expose one property of op-1 with the given binding, then read the config. + const expose = (bindingExtra: any) => { + h.hasOperatorIds.add("op-1"); + formBindingService.resolveFields.mockReturnValue([resolved("x", "x", { binding: bindingExtra })]); + (component as any).readConfig(); + return component.rendered[0].fields[0] as any; + }; + + it("renames and hides an overridden sub-field of an object property", () => { + build(formViewWorkflow).ngOnInit(); + + const field = expose({ + id: "n", + operatorID: "op-1", + propertyKey: "nested", + displayName: "Nested", + overrides: { sub: { displayName: "Renamed sub", hidden: true } }, + }); + + const sub = field.fieldGroup[0]; + expect(sub.key).toBe("sub"); + expect(sub.props.label).toBe("Renamed sub"); + expect(sub.hide).toBe(true); + // Hidden must not strip the value: formly's resetFieldOnHide default would otherwise clear it + // from the model on render, and the card writes the whole nested object back -- deleting the + // author's pinned value. resetOnHide=false keeps it. + expect(sub.resetOnHide).toBe(false); + }); + + it("renames and hides an overridden sub-field of a repeated section, per row", () => { + build(formViewWorkflow).ngOnInit(); + + const field = expose({ + id: "p", + operatorID: "op-1", + propertyKey: "predicates", + displayName: "Predicates", + overrides: { alias: { displayName: "Renamed", hidden: true } }, + }); + + // Formly builds a repeated section's rows on demand; invoke the wrapped builder so the walk + // decorates the row's sub-fields (every row formly ever makes comes out decorated). + const row = field.fieldArray({}); + const alias = row.fieldGroup[0]; + expect(alias.key).toBe("alias"); + expect(alias.props.label).toBe("Renamed"); + expect(alias.hide).toBe(true); + expect(alias.resetOnHide).toBe(false); + }); + + it("drops the schema's own descriptions on the field and its sub-fields", () => { + build(formViewWorkflow).ngOnInit(); + + const field = expose({ id: "n", operatorID: "op-1", propertyKey: "nested", displayName: "Nested" }); + + expect(field.props.description).toBe(""); + expect(field.fieldGroup[0].props.description).toBe(""); + }); + + it("leaves a sub-field untouched when the author set no override for it", () => { + build(formViewWorkflow).ngOnInit(); + + const field = expose({ id: "n", operatorID: "op-1", propertyKey: "nested", displayName: "Nested" }); + + const sub = field.fieldGroup[0]; + // No override: keeps the schema label and stays visible. + expect(sub.props.label).toBe("Sub"); + expect(sub.hide).toBeUndefined(); + // A visible field is never opted out of reset-on-hide -- the switch rides with the hide. + expect(sub.resetOnHide).toBeUndefined(); + }); + + it("drops the description on a scalar array's row template", () => { + build(formViewWorkflow).ngOnInit(); + + const field = expose({ id: "t", operatorID: "op-1", propertyKey: "tags", displayName: "Tags" }); + + // The row template is a leaf (no sub-fields); its schema description is dropped like the rest. + expect(field.fieldArray.props.description).toBe(""); + }); + + it("drops the description on a builder-backed scalar array's leaf row", () => { + build(formViewWorkflow).ngOnInit(); + + const field = expose({ id: "tf", operatorID: "op-1", propertyKey: "tagsFn", displayName: "Tags" }); + // Invoke the wrapped builder: it returns a leaf row (no fieldGroup), which the walk decorates. + const row = field.fieldArray({}); + + expect(row.props.description).toBe(""); + }); + + it("drops the description on a builder-backed object row without reprinting its title", () => { + build(formViewWorkflow).ngOnInit(); + + const field = expose({ id: "p", operatorID: "op-1", propertyKey: "predicates", displayName: "Predicates" }); + // An object row (fieldGroup): its container is not walked as a root (that would reprint the + // array's title), but its own items.description would still render once per row, so it is + // dropped; the row's sub-field is walked as before. + const row = field.fieldArray({}); + + expect(row.props.description).toBe(""); + expect(row.fieldGroup[0].props.description).toBe(""); + }); + + it("drops the description on a static object-array's row template", () => { + build(formViewWorkflow).ngOnInit(); + + const field = expose({ id: "r", operatorID: "op-1", propertyKey: "rules", displayName: "Rules" }); + + // The template container (fieldArray with a fieldGroup) carries items.description; it is + // dropped, and its sub-fields are still walked (their descriptions dropped too). + expect(field.fieldArray.props.description).toBe(""); + expect(field.fieldArray.fieldGroup[0].props.description).toBe(""); + }); + }); + describe("keeping the inputs in step with the workflow", () => { it("rebuilds the inputs when compilation reports a new state", async () => { build(formViewWorkflow).ngOnInit(); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts index 02eed5be3f..260f9c2303 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts @@ -32,7 +32,7 @@ import { forkJoin, Subject } from "rxjs"; import { debounceTime, takeUntil } from "rxjs/operators"; import { USER_WORKFLOW, USER_WORKSPACE } from "../../../app-routing.constant"; -import { Workflow, WorkflowContent } from "../../../common/type/workflow"; +import { FormFieldBinding, Workflow, WorkflowContent } from "../../../common/type/workflow"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; import { WorkflowPersistService } from "../../../common/service/workflow-persist/workflow-persist.service"; import { NotificationService } from "../../../common/service/notification/notification.service"; @@ -71,8 +71,9 @@ interface RenderedField { * read-only workflow preview, this PR renders the inputs an author exposed -- each as its * operator's own formly field, so a file property gets the real picker and an attribute a column * dropdown -- and writes a filled-in value straight back to its operator, the same edit the canvas - * makes. Nested sub-field overrides, running and results are added by later PRs. A view, not a new - * object: it opens the same workflow the canvas does. + * makes, with each sub-field of a nested or repeated property renamed and hidden as the author set + * it up. Running the workflow and showing results are added by later PRs. A view, not a new object: + * it opens the same workflow the canvas does. */ @UntilDestroy() @Component({ @@ -316,10 +317,6 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { field.props = { ...(field.props ?? {}), label: binding.displayName || schemaLabel, - // The schema's own description is the operator author's note to whoever wired the operator - // up; it is not guidance to a form reader, and formly shows it once per scalar field. Drop it - // here so it does not appear unbidden under the input. - description: "", }; const form = new FormGroup({}); @@ -367,9 +364,122 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { field.props = { ...(field.props ?? {}), disabled: true }; } + this.applyFieldOverrides(field, binding); return { resolved, fields: [field], form, model }; } + /** + * The template for one row of a repeated section. formly's `fieldArray` may be the template + * object or a function that builds one per row; resolve both so an array property's sub-fields + * are reachable (treating the function case as a leaf hid them). @internal, exported for tests. + */ + public static arrayItemOf(node: FormlyFieldConfig): FormlyFieldConfig | undefined { + const fa = node.fieldArray; + if (!fa) { + return undefined; + } + if (typeof fa !== "function") { + return fa; + } + try { + return fa(node); + } catch { + // A builder that needs more context than we can give it tells us nothing about the row's + // shape; better to list no sub-fields than to guess at them. + return undefined; + } + } + + /** + * The override path for a child field: the parent path joined with the child's key, but array + * indices are dropped so one override entry covers every row of a repeated section. @internal, + * exported for tests. + */ + public static childPath(parent: string, key: unknown): string { + if (typeof key !== "string" || key === "" || /^\d+$/.test(key)) { + return parent; + } + return parent ? parent + "." + key : key; + } + + /** + * Walk the field and its sub-fields, dropping the operator schema's own per-field descriptions + * (author notes about the operator, not guidance to a form reader) and applying the author's + * stored per-sub-field overrides (rename, hide), keyed by field path. A repeated section builds + * its row template on demand, so its builder is wrapped to decorate every row formly ever makes. + */ + private applyFieldOverrides(field: FormlyFieldConfig, binding: FormFieldBinding): void { + const walk = (node: FormlyFieldConfig, path: string): void => { + // Drop the schema's own description on every field, nested ones included: on this page the + // one piece of guidance is the help text the form's author writes, rendered once by the card. + node.props = { ...(node.props ?? {}), description: "" }; + // Apply the author's stored overrides so a reader sees each sub-field renamed and hidden as + // set up. The root (path "") carries the binding's own displayName, set in renderField. + if (path) { + const override = binding.overrides?.[path] ?? {}; + if (override.displayName) { + node.props = { ...(node.props ?? {}), label: override.displayName }; + } + if (override.hidden) { + node.hide = true; + // Hidden means "not shown", not "cleared". Formly 7's resetFieldOnHide extra defaults to + // true, so a field that renders hidden has its value stripped from the model -- and this + // card writes the whole nested object back, so that strip would delete the author's pinned + // value for the hidden sub-field the moment a writer opens the form. Opt this field out so + // its value survives, matching FormFieldOverride.hidden's contract (the value still + // applies; it is only hidden). + node.resetOnHide = false; + } + } + // A repeated section may build its row template on demand, once per row. Decorating the + // object it returns is pointless -- the next row gets a fresh one. Wrap the builder instead, + // so every row formly ever creates comes out decorated. + if (typeof node.fieldArray === "function") { + const build = node.fieldArray; + node.fieldArray = (f: FormlyFieldConfig) => { + const row = build(f); + // Walk what is INSIDE each row, never the row container itself: the container carries the + // array property's own name, so decorating it as a root (path "") printed the group title + // a second time above the rows. Its sub-fields keep their own key paths, the same ones + // their overrides are stored under. + const children = row.fieldGroup ?? []; + if (children.length === 0) { + // A scalar array (a list of strings): the builder returns a leaf row with no sub-fields, + // so decorate the row itself, mirroring the leaf case of the non-function branch below. + walk(row, path); + } else { + // An object row: not walked as a root (that reprints the array's group title), but its + // own schema description (the items.description) still renders once per row via the + // field wrapper's nzExtra, so drop just that -- the description-removal the walk does for + // every other field, minus the title-reprinting root treatment. + row.props = { ...(row.props ?? {}), description: "" }; + } + for (const child of children) { + walk(child, WorkflowFormComponent.childPath(path, child.key)); + } + return row; + }; + return; + } + const arrayItem = WorkflowFormComponent.arrayItemOf(node); + const children = node.fieldGroup ?? arrayItem?.fieldGroup ?? []; + for (const child of children) { + walk(child, WorkflowFormComponent.childPath(path, child.key)); + } + // A scalar array (e.g. a list of strings) has a row template with no sub-fields of its own; + // decorate it directly so its schema description is dropped like every other field's. + if (arrayItem && !arrayItem.fieldGroup) { + walk(arrayItem, path); + } else if (arrayItem) { + // A static object-array template: its sub-fields are walked above, but the template + // container's own items.description still renders once per row, so drop just that (not + // walking it as a root, which would reprint the array's group title). + arrayItem.props = { ...(arrayItem.props ?? {}), description: "" }; + } + }; + walk(field, ""); + } + private operatorSchemaFor(operatorID: string): object | undefined { const graph = this.workflowActionService.getTexeraGraph(); if (!graph.hasOperator(operatorID)) { diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts index b3a210ab54..842cbea78d 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts @@ -101,6 +101,48 @@ export function setupHarness() { { key: "fileName", props: { label: "File" } }, { key: "modelId", props: { label: "Model" } }, { key: "datasetVersionPath", props: { label: "Dataset" } }, + // An object property with sub-fields (drives the override walk over a fieldGroup). The + // schema descriptions are here so a test can assert the walk drops them. + { + key: "nested", + props: { label: "Nested", description: "obj note" }, + fieldGroup: [{ key: "sub", props: { label: "Sub", description: "sub note" } }], + }, + // A repeated section whose row template is a builder (drives the fieldArray-wrapping path). + // The returned row carries its own description (the schema's items.description), so a test + // can assert the walk drops it -- it would otherwise render once per row. + { + key: "predicates", + props: { label: "Predicates" }, + fieldArray: () => ({ + props: { description: "row note" }, + fieldGroup: [{ key: "alias", props: { label: "Alias" } }], + }), + }, + // A scalar array: its row template is a leaf (no sub-fields), drives the leaf-item branch. + { + key: "tags", + props: { label: "Tags" }, + fieldArray: { key: "item", props: { label: "Tag", description: "item note" } }, + }, + // A static object-array template (fieldArray is an object WITH sub-fields, not a builder): + // drives the object-array-template branch, where the container's own items.description must + // be dropped even though the container itself is not walked as a root. + { + key: "rules", + props: { label: "Rules" }, + fieldArray: { + props: { description: "rules note" }, + fieldGroup: [{ key: "field", props: { label: "Field", description: "field note" } }], + }, + }, + // A scalar array whose row template is a BUILDER returning a leaf (no fieldGroup): drives + // the leaf case inside the fieldArray-function wrapper. + { + key: "tagsFn", + props: { label: "Tags (fn)" }, + fieldArray: () => ({ key: "item", props: { label: "Tag", description: "fn note" } }), + }, ]; return { fieldGroup: opts?.map ? fields.map(opts.map) : fields }; },
