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-6830-8cf4894663f5a8cecf895e6be008b96c06672b2c in repository https://gitbox.apache.org/repos/asf/texera.git
commit 3d38b2bdcddeb494b71bf9e3e7b01395ef2780ec Author: Meng Wang <[email protected]> AuthorDate: Thu Jul 23 20:46:58 2026 -0700 test(frontend): cover JointGraphWrapper highlighting, flags, zoom and coeditor paths (#6830) ### What changes were proposed in this PR? Extends the existing `JointGraphWrapper` spec to cover the remaining unit-testable (non-rendering) methods (`frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts`). The wrapper is built over a headless `new joint.dia.Graph()` with no `dia.Paper`, matching the spec's existing setup. No production code was changed. 19 tests, grouped by the families the issue lists: **Highlighting** — `highlightLinks` / `unhighlightLinks`, `highlightCommentBoxes` / `unhighlightCommentBoxes`, `highlightPorts` / `unhighlightPorts`, each asserted through the matching `getCurrentHighlighted*IDs` getter *and* its highlight/unhighlight stream; single-select (the default) dropping the previous highlight vs `setMultiSelectMode(true)` keeping it; `getCurrentHighlights` / `getCurrentHighlightedIDs` aggregation (ports are deliberately excluded from the latter); `unhighlightElements` clearing all four families; and the group highlight streams, which operator highlighting does not touch. **Flags** — `setReloadingWorkflow` / `getReloadingWorkflow` and `setListenPositionChange` / `getListenPositionChange` round-trips. **Zoom** — `setZoomProperty` updating the ratio and emitting on the zoom stream, `isZoomRatioMin` / `isZoomRatioMax` at the configured bounds, and `restoreDefaultZoomAndOffset` resetting to `INIT_ZOOM_VALUE` while signalling a paper-offset restore. **Cell layer / breakpoints / position** — `getCellLayer` (z index, and the throw for an unknown cell), `getLinkIDsWithBreakpoint`, and `getElementPositionChangeEvent` reporting a moved element's old/new position. **Coeditor presence** — `addCoeditorOperatorHighlight` / `deleteCoeditorOperatorHighlight`, `setCurrentEditing` / `removeCurrentEditing`, and `setPropertyChanged` / `removePropertyChanged`. These reach the canvas through `getMainJointPaper()?.`, so with no paper attached they must degrade to a safe no-op; `setCurrentEditing` additionally returns a 300 ms interval, so the test drives it under `vi.useFakeTimers()` and asserts `removeCurrentEditing` clears it (real timers restored in a `finally`). Two behaviours are asserted as the code actually behaves rather than as the issue describes them, and are commented in the spec: - `setZoomProperty` does **not** clamp — `isZoomRatioMin/Max` only *report* where the ratio sits relative to `ZOOM_MINIMUM` / `ZOOM_MAXIMUM`; clamping is the caller's job. - the port streams emit **unconditionally**, so single-select's "clear the previous ports first" step surfaces as an extra empty batch on the unhighlight stream. Out of scope per the issue (they need a real `dia.Paper` / geometry): `attachMainJointPaper` / `getMainJointPaper`, `autoLayoutJoint`, `getElementPosition` / `setElementPosition` / `setAbsolutePosition`, `restorePaperOffset`. ### Any related issues, documentation, discussions? Closes #6825 ### How was this PR tested? Extended unit tests, run locally in `frontend/` (all green; the failure path was verified by breaking an assertion to confirm the suite goes red): ``` ng test --watch=false --include src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts # Test Files 1 passed (1) | Tests 47 passed (47) prettier --write <spec> # clean eslint <spec> # clean ``` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --------- Co-authored-by: Xinyuan Lin <[email protected]> --- .../model/joint-graph-wrapper.spec.ts | 321 +++++++++++++++++++++ 1 file changed, 321 insertions(+) diff --git a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts index c212722314..c4bbd8b4f3 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts @@ -25,6 +25,7 @@ import { JointGraphWrapper } from "./joint-graph-wrapper"; import { TestBed } from "@angular/core/testing"; import { marbles } from "rxjs-marbles"; import { + mockCommentBox, mockPoint, mockResultPredicate, mockScanPredicate, @@ -33,6 +34,7 @@ import { mockSentimentPredicate, mockSentimentResultLink, } from "./mock-workflow-data"; +import { Coeditor, Role } from "../../../../common/type/user"; import * as joint from "jointjs"; import { StubOperatorMetadataService } from "../../operator-metadata/stub-operator-metadata.service"; import { WorkflowUtilService } from "../util/workflow-util.service"; @@ -818,4 +820,323 @@ describe("JointGraphWrapperService", () => { expect(emitted).toEqual([false, true, false]); }); }); + + describe("highlighting links, comment boxes and ports", () => { + // highlightElement() throws unless the cell is actually in the graph, so seed + // the cells first. Ports are tracked purely in the wrapper and need no cell. + const addOperators = (): void => { + jointGraph.addCell(jointUIService.getJointOperatorElement(mockScanPredicate, mockPoint)); + jointGraph.addCell(jointUIService.getJointOperatorElement(mockResultPredicate, mockPoint)); + }; + const addLink = (): void => { + jointGraph.addCell(JointUIService.getJointLinkCell(mockScanResultLink)); + }; + const addCommentBox = (): void => { + jointGraph.addCell(jointUIService.getCommentElement(mockCommentBox)); + }; + const port = (operatorID: string, portID: string) => ({ operatorID, portID }); + + it("highlightLinks and unhighlightLinks track the current link ids and emit", () => { + addOperators(); + addLink(); + const highlighted: string[][] = []; + const unhighlighted: string[][] = []; + const onHighlight = jointGraphWrapper.getLinkHighlightStream().subscribe(ids => highlighted.push([...ids])); + const onUnhighlight = jointGraphWrapper.getLinkUnhighlightStream().subscribe(ids => unhighlighted.push([...ids])); + + jointGraphWrapper.highlightLinks(mockScanResultLink.linkID); + expect(jointGraphWrapper.getCurrentHighlightedLinkIDs()).toEqual([mockScanResultLink.linkID]); + expect(highlighted).toEqual([[mockScanResultLink.linkID]]); + + // re-highlighting an already-highlighted link is a no-op (no second emission) + jointGraphWrapper.highlightLinks(mockScanResultLink.linkID); + expect(highlighted).toHaveLength(1); + + jointGraphWrapper.unhighlightLinks(mockScanResultLink.linkID); + expect(jointGraphWrapper.getCurrentHighlightedLinkIDs()).toEqual([]); + expect(unhighlighted).toEqual([[mockScanResultLink.linkID]]); + + onHighlight.unsubscribe(); + onUnhighlight.unsubscribe(); + }); + + it("highlightCommentBoxes and unhighlightCommentBoxes track the ids and emit", () => { + addCommentBox(); + const highlighted: string[][] = []; + const unhighlighted: string[][] = []; + const onHighlight = jointGraphWrapper + .getJointCommentBoxHighlightStream() + .subscribe(ids => highlighted.push([...ids])); + const onUnhighlight = jointGraphWrapper + .getJointCommentBoxUnhighlightStream() + .subscribe(ids => unhighlighted.push([...ids])); + + jointGraphWrapper.highlightCommentBoxes(mockCommentBox.commentBoxID); + expect(jointGraphWrapper.getCurrentHighlightedCommentBoxIDs()).toEqual([mockCommentBox.commentBoxID]); + expect(highlighted).toEqual([[mockCommentBox.commentBoxID]]); + + jointGraphWrapper.unhighlightCommentBoxes(mockCommentBox.commentBoxID); + expect(jointGraphWrapper.getCurrentHighlightedCommentBoxIDs()).toEqual([]); + expect(unhighlighted).toEqual([[mockCommentBox.commentBoxID]]); + + onHighlight.unsubscribe(); + onUnhighlight.unsubscribe(); + }); + + it("highlightPorts and unhighlightPorts track the current ports and emit", () => { + const highlighted: unknown[][] = []; + const unhighlighted: unknown[][] = []; + const onHighlight = jointGraphWrapper.getJointPortHighlightStream().subscribe(ids => highlighted.push([...ids])); + const onUnhighlight = jointGraphWrapper + .getJointPortUnhighlightStream() + .subscribe(ids => unhighlighted.push([...ids])); + + const portA = port(mockScanPredicate.operatorID, "output-0"); + jointGraphWrapper.highlightPorts(portA); + expect(jointGraphWrapper.getCurrentHighlightedPortIDs()).toEqual([portA]); + expect(highlighted).toEqual([[portA]]); + // With multi-select off, highlightPorts clears the previous ports first. The + // port streams emit unconditionally (unlike the operator/link ones), so that + // pre-clear surfaces as an empty batch even though nothing was highlighted. + expect(unhighlighted).toEqual([[]]); + + jointGraphWrapper.unhighlightPorts(portA); + expect(jointGraphWrapper.getCurrentHighlightedPortIDs()).toEqual([]); + expect(unhighlighted).toEqual([[], [portA]]); + + onHighlight.unsubscribe(); + onUnhighlight.unsubscribe(); + }); + + it("single-select mode (the default) unhighlights the previous element", () => { + addOperators(); + addLink(); + + jointGraphWrapper.highlightOperators(mockScanPredicate.operatorID); + expect(jointGraphWrapper.getCurrentHighlightedOperatorIDs()).toEqual([mockScanPredicate.operatorID]); + + // highlighting a link with multi-select off drops the operator highlight + jointGraphWrapper.highlightLinks(mockScanResultLink.linkID); + expect(jointGraphWrapper.getCurrentHighlightedOperatorIDs()).toEqual([]); + expect(jointGraphWrapper.getCurrentHighlightedLinkIDs()).toEqual([mockScanResultLink.linkID]); + }); + + it("multi-select mode keeps previously highlighted elements", () => { + addOperators(); + addLink(); + jointGraphWrapper.setMultiSelectMode(true); + + jointGraphWrapper.highlightOperators(mockScanPredicate.operatorID); + jointGraphWrapper.highlightLinks(mockScanResultLink.linkID); + + expect(jointGraphWrapper.getCurrentHighlightedOperatorIDs()).toEqual([mockScanPredicate.operatorID]); + expect(jointGraphWrapper.getCurrentHighlightedLinkIDs()).toEqual([mockScanResultLink.linkID]); + }); + + it("getCurrentHighlights and getCurrentHighlightedIDs aggregate every family", () => { + addOperators(); + addLink(); + addCommentBox(); + jointGraphWrapper.setMultiSelectMode(true); + + const portA = port(mockScanPredicate.operatorID, "output-0"); + jointGraphWrapper.highlightOperators(mockScanPredicate.operatorID); + jointGraphWrapper.highlightLinks(mockScanResultLink.linkID); + jointGraphWrapper.highlightCommentBoxes(mockCommentBox.commentBoxID); + jointGraphWrapper.highlightPorts(portA); + + expect(jointGraphWrapper.getCurrentHighlights()).toEqual({ + operators: [mockScanPredicate.operatorID], + links: [mockScanResultLink.linkID], + commentBoxes: [mockCommentBox.commentBoxID], + ports: [portA], + }); + // getCurrentHighlightedIDs concatenates the id-based families only (ports excluded) + expect(jointGraphWrapper.getCurrentHighlightedIDs()).toEqual([ + mockScanPredicate.operatorID, + mockScanResultLink.linkID, + mockCommentBox.commentBoxID, + ]); + }); + + it("unhighlightElements clears operators, links, comment boxes and ports together", () => { + addOperators(); + addLink(); + addCommentBox(); + jointGraphWrapper.setMultiSelectMode(true); + + const portA = port(mockScanPredicate.operatorID, "output-0"); + jointGraphWrapper.highlightOperators(mockScanPredicate.operatorID); + jointGraphWrapper.highlightLinks(mockScanResultLink.linkID); + jointGraphWrapper.highlightCommentBoxes(mockCommentBox.commentBoxID); + jointGraphWrapper.highlightPorts(portA); + + // copy the lists so the call does not iterate the arrays it is mutating + jointGraphWrapper.unhighlightElements({ + operators: [...jointGraphWrapper.getCurrentHighlightedOperatorIDs()], + links: [...jointGraphWrapper.getCurrentHighlightedLinkIDs()], + commentBoxes: [...jointGraphWrapper.getCurrentHighlightedCommentBoxIDs()], + ports: [...jointGraphWrapper.getCurrentHighlightedPortIDs()], + }); + + expect(jointGraphWrapper.getCurrentHighlights()).toEqual({ + operators: [], + links: [], + commentBoxes: [], + ports: [], + }); + }); + + it("exposes the group highlight streams, which operator highlighting does not touch", () => { + addOperators(); + const emitted: unknown[] = []; + const onHighlight = jointGraphWrapper.getJointGroupHighlightStream().subscribe(ids => emitted.push(ids)); + const onUnhighlight = jointGraphWrapper.getJointGroupUnhighlightStream().subscribe(ids => emitted.push(ids)); + + jointGraphWrapper.highlightOperators(mockScanPredicate.operatorID); + jointGraphWrapper.unhighlightOperators(mockScanPredicate.operatorID); + + expect(emitted).toEqual([]); + onHighlight.unsubscribe(); + onUnhighlight.unsubscribe(); + }); + }); + + describe("workflow flags", () => { + it("setReloadingWorkflow round-trips through getReloadingWorkflow", () => { + expect(jointGraphWrapper.getReloadingWorkflow()).toBe(false); + + jointGraphWrapper.setReloadingWorkflow(true); + expect(jointGraphWrapper.getReloadingWorkflow()).toBe(true); + + jointGraphWrapper.setReloadingWorkflow(false); + expect(jointGraphWrapper.getReloadingWorkflow()).toBe(false); + }); + + it("setListenPositionChange round-trips through getListenPositionChange", () => { + expect(jointGraphWrapper.getListenPositionChange()).toBe(true); + + jointGraphWrapper.setListenPositionChange(false); + expect(jointGraphWrapper.getListenPositionChange()).toBe(false); + + jointGraphWrapper.setListenPositionChange(true); + expect(jointGraphWrapper.getListenPositionChange()).toBe(true); + }); + }); + + describe("zoom ratio bounds", () => { + it("setZoomProperty updates the ratio and emits it on the zoom stream", () => { + const emitted: number[] = []; + const subscription = jointGraphWrapper.getWorkflowEditorZoomStream().subscribe(ratio => emitted.push(ratio)); + + jointGraphWrapper.setZoomProperty(1.2); + + expect(jointGraphWrapper.getZoomRatio()).toBe(1.2); + expect(emitted).toEqual([1.2]); + subscription.unsubscribe(); + }); + + it("isZoomRatioMin and isZoomRatioMax report the configured bounds", () => { + // the wrapper itself does not clamp; it only reports where the ratio sits + jointGraphWrapper.setZoomProperty(JointGraphWrapper.ZOOM_MINIMUM); + expect(jointGraphWrapper.isZoomRatioMin()).toBe(true); + expect(jointGraphWrapper.isZoomRatioMax()).toBe(false); + + jointGraphWrapper.setZoomProperty(JointGraphWrapper.ZOOM_MAXIMUM); + expect(jointGraphWrapper.isZoomRatioMax()).toBe(true); + expect(jointGraphWrapper.isZoomRatioMin()).toBe(false); + + jointGraphWrapper.setZoomProperty(JointGraphWrapper.INIT_ZOOM_VALUE); + expect(jointGraphWrapper.isZoomRatioMin()).toBe(false); + expect(jointGraphWrapper.isZoomRatioMax()).toBe(false); + }); + + it("restoreDefaultZoomAndOffset resets the ratio and signals a paper-offset restore", () => { + const restores: number[] = []; + const subscription = jointGraphWrapper.getRestorePaperOffsetStream().subscribe(() => restores.push(1)); + jointGraphWrapper.setZoomProperty(1.25); + + jointGraphWrapper.restoreDefaultZoomAndOffset(); + + expect(jointGraphWrapper.getZoomRatio()).toBe(JointGraphWrapper.INIT_ZOOM_VALUE); + expect(restores).toHaveLength(1); + subscription.unsubscribe(); + }); + }); + + describe("cell layer, breakpoints and position changes", () => { + it("getCellLayer returns the cell's z index and throws for an unknown cell", () => { + jointGraph.addCell(jointUIService.getJointOperatorElement(mockScanPredicate, mockPoint)); + + expect(jointGraphWrapper.getCellLayer(mockScanPredicate.operatorID)).toBeGreaterThanOrEqual(0); + expect(() => jointGraphWrapper.getCellLayer("no-such-cell")).toThrowError( + "cell with ID no-such-cell doesn't exist" + ); + }); + + it("getLinkIDsWithBreakpoint starts empty", () => { + expect(jointGraphWrapper.getLinkIDsWithBreakpoint()).toEqual([]); + }); + + it("getElementPositionChangeEvent reports the old and new position of a moved element", () => { + jointGraph.addCell(jointUIService.getJointOperatorElement(mockScanPredicate, mockPoint)); + + const moves: { elementID: string; newPosition: { x: number; y: number } }[] = []; + const subscription = jointGraphWrapper + .getElementPositionChangeEvent() + .subscribe(event => moves.push({ elementID: event.elementID, newPosition: event.newPosition })); + + (jointGraph.getCell(mockScanPredicate.operatorID) as joint.dia.Element).position(100, 200); + + expect(moves).toEqual([{ elementID: mockScanPredicate.operatorID, newPosition: { x: 100, y: 200 } }]); + subscription.unsubscribe(); + }); + }); + + describe("coeditor presence without an attached paper", () => { + // Every coeditor method reaches the canvas through `getMainJointPaper()?.`, + // so with no paper attached they must degrade to a safe no-op. + const coeditor: Coeditor = { + clientId: "client-1", + uid: 1, + name: "Alice", + email: "[email protected]", + role: Role.REGULAR, + comment: "", + joiningReason: "", + color: "#ff0000", + }; + + it("add/deleteCoeditorOperatorHighlight are no-ops when no paper is attached", () => { + expect(() => + jointGraphWrapper.addCoeditorOperatorHighlight(coeditor, mockScanPredicate.operatorID) + ).not.toThrow(); + expect(() => + jointGraphWrapper.deleteCoeditorOperatorHighlight(coeditor, mockScanPredicate.operatorID) + ).not.toThrow(); + }); + + it("setCurrentEditing returns an interval that removeCurrentEditing clears", () => { + // Deliberately NOT using fake timers: zone.js patches setInterval/clearInterval + // too, and driving vitest's fake clock through that patched pair behaves + // differently across Node versions. This body is fully synchronous, so the + // 300ms animation callback can never be reached before it is cleared. + const clearIntervalSpy = vi.spyOn(globalThis, "clearInterval"); + const intervalId = jointGraphWrapper.setCurrentEditing(coeditor, mockScanPredicate.operatorID); + try { + expect(intervalId).toBeDefined(); + + jointGraphWrapper.removeCurrentEditing(coeditor, mockScanPredicate.operatorID, intervalId); + expect(clearIntervalSpy).toHaveBeenCalledWith(intervalId); + } finally { + clearInterval(intervalId); + clearIntervalSpy.mockRestore(); + } + }); + + it("set/removePropertyChanged are no-ops when no paper is attached", () => { + expect(() => jointGraphWrapper.setPropertyChanged(coeditor, mockScanPredicate.operatorID)).not.toThrow(); + expect(() => jointGraphWrapper.removePropertyChanged(coeditor, mockScanPredicate.operatorID)).not.toThrow(); + }); + }); });
