(texera) branch gh-readonly-queue/main/pr-7584-22a66d325c380e7d11d4c2240c9a53ffc7e31aa9 created (now 7db229c41f)

[gi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-7584-22a66d325c380e7d11d4c2240c9a53ffc7e31aa9
in repository https://gitbox.apache.org/repos/asf/texera.git


      at 7db229c41f feat(frontend): remove an operator property when its field is cleared (#7584)

This branch includes the following new commits:

     new 7db229c41f feat(frontend): remove an operator property when its field is cleared (#7584)

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

(texera) 01/01: feat(frontend): remove an operator property when its field is cleared (#7584)

[gi...@apache.org]

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-7584-22a66d325c380e7d11d4c2240c9a53ffc7e31aa9
in repository https://gitbox.apache.org/repos/asf/texera.git

commit 7db229c41f9a28b8e6ca6c107dd21e820e03dbb3
Author: Kary Zheng <15...@users.noreply.github.com>
AuthorDate: Wed Sep 2 00:53:52 2026 +0000

    feat(frontend): remove an operator property when its field is cleared (#7584)
    
    ### What changes were proposed in this PR?
    
    Clearing a text or number field in the property panel left the box empty
    while the operator kept its previous value, so the workflow ran on a
    value the user had removed and could no longer see. Setting CSV File
    Scan's Limit to 5 and clearing it left the scan reading 5 rows, and the
    value came back on the next reload.
    
    A cleared field is simply not among the properties the panel sends:
    formly drops a cleared text field from the model, and a cleared number
    arrives as `null`, which `onFormChanges` filters out.
    `updateYTypeFromObject` updates a value in place or sets a new one, and
    for a key the new object no longer carries it did neither, so the old
    entry stayed in the Y.Map. It now deletes such a key, deciding on
    membership rather than on the value so that an explicit `undefined`
    still takes the update path.
    
    Clearing a property that has a schema default now shows that default
    when the panel is next rendered; one without a default stays empty.
    
    ### Any related issues, documentation, discussions?
    
    Closes #7394
    
    The issue also read this as evidence that an empty string cannot be
    stored from the UI. That part has been withdrawn: an absent property
    deserializes to its Scala field initializer, which already gives an
    empty value where one is meaningful.
    
    ### How was this PR tested?
    
    The test that pinned the old behaviour is updated, since it described
    what the function did rather than what it should do, and
    `setOperatorProperty` gains a case that fails without this change. The
    specs that write operator properties pass together, 465 cases.
    
    The issue's reproduction was run before and after. Delimiter set to `X`
    and cleared came back as `X`; it now leaves `,`, and Limit set to 5 and
    cleared stays empty.
    
    ### Was this PR authored or co-authored using generative AI tooling?
    
    Generated-by: Claude Code (Claude Opus 5)
    
    Co-authored-by: Claude Opus 5 (1M context) <no...@anthropic.com>
---
 .../workflow-graph/model/workflow-graph.spec.ts      | 14 ++++++++++++++
 .../workspace/types/shared-editing.interface.spec.ts | 19 ++++++++++++++++---
 .../app/workspace/types/shared-editing.interface.ts  | 20 +++++++++++++++-----
 3 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/frontend/src/app/workspace/service/workflow-graph/model/workflow-graph.spec.ts b/frontend/src/app/workspace/service/workflow-graph/model/workflow-graph.spec.ts
index 2e1e6b2ef2..41942cd7a5 100644
--- a/frontend/src/app/workspace/service/workflow-graph/model/workflow-graph.spec.ts
+++ b/frontend/src/app/workspace/service/workflow-graph/model/workflow-graph.spec.ts
@@ -199,6 +199,20 @@ describe("WorkflowGraph", () => {
     expect(operator.operatorProperties).toEqual(testProperty);
   });
 
+  // The property panel clears a field by sending the properties without it, so a
+  // key left out has to be dropped here. Retaining it leaves the operator running
+  // on a value the user removed and the panel no longer shows.
+  it("should drop a property the new object leaves out", () => {
+    workflowGraph.addOperator(mockScanPredicate);
+    workflowGraph.setOperatorProperty(mockScanPredicate.operatorID, { tableName: "testTable", limit: 5 });
+
+    workflowGraph.setOperatorProperty(mockScanPredicate.operatorID, { tableName: "testTable" });
+
+    expect(workflowGraph.getOperator(mockScanPredicate.operatorID).operatorProperties).toEqual({
+      tableName: "testTable",
+    });
+  });
+
   it("should throw an error when trying to set the property of an nonexist operator", () => {
     expect(() => {
       const testProperty = { tableName: "testTable" };
diff --git a/frontend/src/app/workspace/types/shared-editing.interface.spec.ts b/frontend/src/app/workspace/types/shared-editing.interface.spec.ts
index 40141d0006..83af7d2fda 100644
--- a/frontend/src/app/workspace/types/shared-editing.interface.spec.ts
+++ b/frontend/src/app/workspace/types/shared-editing.interface.spec.ts
@@ -142,7 +142,7 @@ describe("updateYTypeFromObject", () => {
     expect(originalText.toJSON()).toBe("world");
   });
 
-  it("adds new object keys but leaves keys absent from the new object in place", () => {
+  it("adds new object keys and removes the ones the new object no longer carries", () => {
     const doc = new Y.Doc();
     const yMap = attach(doc, "m", createYTypeFromObject({ keep: 1, drop: 2 }));
 
@@ -153,8 +153,21 @@ describe("updateYTypeFromObject", () => {
     // A key present only in the new object is added.
     expect(asMap.has("add")).toBe(true);
     expect(asMap.get("add")).toBe(3);
-    // A key omitted from the new object is retained (an undefined new value never deletes).
-    expect(asMap.get("drop")).toBe(2);
+    // A key the new object has dropped is deleted rather than left behind. Clearing
+    // an operator property is exactly this: the editor sends the properties without
+    // it, and anything retained here is a value the user removed and can no longer see.
+    expect(asMap.has("drop")).toBe(false);
+  });
+
+  it("keeps a key whose new value is explicitly undefined", () => {
+    const doc = new Y.Doc();
+    const yMap = attach(doc, "m", createYTypeFromObject({ a: 1 }));
+
+    updateYTypeFromObject(yMap, { a: undefined } as any);
+
+    // Carrying the key with an undefined value is not the same as dropping it, and
+    // only the latter means removal.
+    expect((yMap as unknown as Y.Map<any>).get("a")).toBe(1);
   });
 
   it("appends new items to a Y.Array in place", () => {
diff --git a/frontend/src/app/workspace/types/shared-editing.interface.ts b/frontend/src/app/workspace/types/shared-editing.interface.ts
index fc7843992d..f68beef1e4 100644
--- a/frontend/src/app/workspace/types/shared-editing.interface.ts
+++ b/frontend/src/app/workspace/types/shared-editing.interface.ts
@@ -246,11 +246,21 @@ export function updateYTypeFromObject<T extends object>(oldYObj: YType<T>, newOb
     const keySet = new Set([...Object.keys(oldObj), ...Object.keys(newObj)]);
     keySet.forEach((k: string) => {
       const newValue = newObj[k as keyof T] as any;
-      if (!_.isEqual(oldObj[k as keyof T], newValue)) {
-        if (!updateYTypeFromObject(oldYObjAsYMap.get(k), newValue)) {
-          if (newValue !== undefined) {
-            oldYObjAsYMap.set(k, createYTypeFromObject(newValue));
-          }
+      if (_.isEqual(oldObj[k as keyof T], newValue)) {
+        return;
+      }
+      // A key the new object does not carry is one the caller removed, and deleting
+      // it is the only way that removal can reach the shared type: the update below
+      // changes a value or adds one, and for a key that is simply gone it did
+      // neither, leaving the old value in place. `in` rather than an undefined check,
+      // so a key explicitly carrying undefined still takes the path below.
+      if (!(k in newObj)) {
+        oldYObjAsYMap.delete(k);
+        return;
+      }
+      if (!updateYTypeFromObject(oldYObjAsYMap.get(k), newValue)) {
+        if (newValue !== undefined) {
+          oldYObjAsYMap.set(k, createYTypeFromObject(newValue));
         }
       }
     });