Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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" };
Expand Down
19 changes: 16 additions & 3 deletions frontend/src/app/workspace/types/shared-editing.interface.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));

Expand All @@ -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", () => {
Expand Down
20 changes: 15 additions & 5 deletions frontend/src/app/workspace/types/shared-editing.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
});
Expand Down
Loading