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-5265-52584a80cef1c9b9ee67b8083b617bf654be806e in repository https://gitbox.apache.org/repos/asf/texera.git
commit 81d4612dc796ed3ea5dc6d98bd5bcc5f7fce0b41 Author: Ryan Zhang <[email protected]> AuthorDate: Sat Jul 25 00:36:36 2026 -0700 feat(python-notebook-migration, frontend): add Jupyter panel service (visibility surface and notebook-exists signal) (#5265) ### What changes were proposed in this PR? Extends `JupyterPanelService` with the panel-visibility surface (a `BehaviorSubject` plus four open/close/minimize methods) and a reactive `jupyterNotebookExists$` signal. This PR is the service layer only. The button that consumes it lives in the workspace top menu bar via #5273; #5265 provides the service, #5273 consumes it. **`jupyter-panel.service.ts`:** - Restores `BehaviorSubject` to the `rxjs` import and re-adds the `NotificationService` injection. - Panel-visibility stream: `private jupyterNotebookPanelVisible = new BehaviorSubject<boolean>(false)` with public `jupyterNotebookPanelVisible$ = this.jupyterNotebookPanelVisible.asObservable()`. - Notebook-exists signal: `private jupyterNotebookExists = new BehaviorSubject<boolean>(false)` with public `jupyterNotebookExists$`. `init()` resets it to `false` on every workflow change and sets it to `true` in the `fetchNotebookAndMapping` success branch (`result === 1`). It stays `true` through minimize and resets when switching workflows. The menu button in #5273 binds to this to decide whether to show. - Four `if (!this.enabled) return;`-gated methods: - `openPanel(panelName: string)` — flips visibility to `true` when `panelName === "JupyterNotebookPanel"` (the existing workspace panel-system convention). - `closeJupyterNotebookPanel()` — flips visibility to `false`, then `notebookMigrationService.deleteMapping("mapping_wid_" + workflowActionService.getWorkflow().wid)`. - `minimizeJupyterNotebookPanel()` — flips visibility to `false`. - `openJupyterNotebookPanel()` — checks `notebookMigrationService.hasMapping(...)`; warns via `notificationService.warning("No Jupyter notebook associated with this workflow.")` and returns if there is no cached mapping, otherwise flips visibility to `true`. - `init()`'s subscribe handler: on every workflow change it calls `closeJupyterNotebookPanel()` (which also does the stale-mapping cleanup), clears the highlight index, and resets `jupyterNotebookExists` to `false`; on a successful fetch it sets `jupyterNotebookExists` to `true`, precomputes the highlight index, and auto-opens via `openJupyterNotebookPanel()`. No mini-map changes: the expand control moved out of the mini-map and into the top menu bar (see #5273), so `mini-map.component.*` is back to its `main` version here. ### Any related issues, documentation, discussions? Closes #5264 Parent issue #4301 Builds on #5263 (`migration-tool-mapping-highlighting`), merged. The consuming menu button lives in #5273 (`migration-tool-modal`): #5265 provides `openJupyterNotebookPanel` and `jupyterNotebookExists$`, #5273 consumes them. ### How was this PR tested? `jupyter-panel.service.spec.ts` adds visibility coverage (enabled-flag cases for open/close, minimize, and the warn-vs-open branch of `openJupyterNotebookPanel`; disabled-flag short-circuit cases per method) and a `jupyterNotebookExists$` case (starts `false`, flips `true` after a successful fetch). ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) --------- Co-authored-by: Meng Wang <[email protected]> --- .../jupyter-panel/jupyter-panel.service.spec.ts | 111 +++++++++++++++++++++ .../service/jupyter-panel/jupyter-panel.service.ts | 75 +++++++++++--- 2 files changed, 171 insertions(+), 15 deletions(-) diff --git a/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts b/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts index a90714c041..17c8c0d33f 100644 --- a/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts +++ b/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts @@ -21,6 +21,7 @@ import { TestBed } from "@angular/core/testing"; import { JupyterPanelService } from "./jupyter-panel.service"; import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; +import { NotificationService } from "src/app/common/service/notification/notification.service"; import { NotebookMigrationService } from "../notebook-migration/notebook-migration.service"; import { GuiConfigService } from "src/app/common/service/gui-config.service"; import { firstValueFrom, of } from "rxjs"; @@ -30,6 +31,7 @@ describe("JupyterPanelService", () => { let httpMock: HttpTestingController; let mockWorkflow: any; + let mockNotification: any; let mockNotebook: any; // Mutable so individual describe blocks can flip the flag mid-spec; the // service stores a reference, so mutations are observed on the next read. @@ -55,6 +57,10 @@ describe("JupyterPanelService", () => { unhighlightLinks: vi.fn(), }; + mockNotification = { + warning: vi.fn(), + }; + mockNotebook = { hasMapping: vi.fn().mockReturnValue(true), getMapping: vi.fn().mockReturnValue({ @@ -75,6 +81,7 @@ describe("JupyterPanelService", () => { providers: [ JupyterPanelService, { provide: WorkflowActionService, useValue: mockWorkflow }, + { provide: NotificationService, useValue: mockNotification }, { provide: NotebookMigrationService, useValue: mockNotebook }, { provide: GuiConfigService, useValue: mockGuiConfig }, ], @@ -88,6 +95,63 @@ describe("JupyterPanelService", () => { httpMock.verify(); }); + // Panel visibility + it("should open and close panel", () => { + let state: boolean | null = null; + + service.jupyterNotebookPanelVisible$.subscribe(v => (state = v)); + + service.openPanel("JupyterNotebookPanel"); + expect(state).toBe(true); + + service.closeJupyterNotebookPanel(); + expect(state).toBe(false); + }); + + it("should minimize panel", () => { + let state: boolean | null = true; + + service.jupyterNotebookPanelVisible$.subscribe(v => (state = v)); + + service.minimizeJupyterNotebookPanel(); + + expect(state).toBe(false); + }); + + // openJupyterNotebookPanel + it("should warn if no mapping exists", () => { + mockNotebook.hasMapping.mockReturnValue(false); + + service.openJupyterNotebookPanel(); + + expect(mockNotification.warning).toHaveBeenCalled(); + }); + + it("should open panel if mapping exists", () => { + mockNotebook.hasMapping.mockReturnValue(true); + + let state: boolean | null = false; + + service.jupyterNotebookPanelVisible$.subscribe(v => (state = v)); + + service.openJupyterNotebookPanel(); + + expect(state).toBe(true); + }); + + // openPanel + it("should open panel only for correct name", () => { + let state: boolean | null = false; + + service.jupyterNotebookPanelVisible$.subscribe(v => (state = v)); + + service.openPanel("WrongPanel"); + expect(state).toBe(false); + + service.openPanel("JupyterNotebookPanel"); + expect(state).toBe(true); + }); + // HTTP fetchNotebookAndMapping it("should return 0 when exists=false", async () => { const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(1, 1)); @@ -98,6 +162,23 @@ describe("JupyterPanelService", () => { expect(await resultPromise).toBe(0); }); + // jupyterNotebookExists$ starts false and flips true once init()'s fetch finds + // a notebook for the workflow; the toolbar's expand button binds to this. + it("sets jupyterNotebookExists$ true after a workflow's notebook is fetched", async () => { + mockNotebook.sendNotebookToJupyter = vi.fn().mockResolvedValue(1); + const states: boolean[] = []; + service.jupyterNotebookExists$.subscribe(v => states.push(v)); + + service.init(); + httpMock + .expectOne(r => r.url.includes("/notebook-migration/fetch-notebook-and-mapping")) + .flush({ exists: true, mapping: { cell_to_operator: {}, operator_to_cell: {} }, notebook: {} }); + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(states[0]).toBe(false); // starts false + expect(states.at(-1)).toBe(true); // true once the notebook is found + }); + // init(): subscribes to workflow changes, drops the stale mapping for the // current workflow, and fetches the incoming workflow's notebook + mapping. it("init subscribes, drops the stale mapping, and fetches for the new workflow", () => { @@ -303,6 +384,36 @@ describe("JupyterPanelService", () => { expect(mockWorkflow.workflowMetaDataChanged).not.toHaveBeenCalled(); }); + it("openPanel does not flip the visibility stream", () => { + let state: boolean | null = false; + service.jupyterNotebookPanelVisible$.subscribe(v => (state = v)); + service.openPanel("JupyterNotebookPanel"); + expect(state).toBe(false); + }); + + it("closeJupyterNotebookPanel does not flip visibility or delete the mapping", () => { + // BehaviorSubject's initial value is false; the meaningful assertion is + // that the side effect (deleteMapping) was never called. + service.closeJupyterNotebookPanel(); + expect(mockNotebook.deleteMapping).not.toHaveBeenCalled(); + }); + + it("minimizeJupyterNotebookPanel does not flip visibility", () => { + const visibleSubject = (service as any).jupyterNotebookPanelVisible; + visibleSubject.next(true); + service.minimizeJupyterNotebookPanel(); + expect(visibleSubject.value).toBe(true); + }); + + it("openJupyterNotebookPanel does not warn or flip visibility", () => { + mockNotebook.hasMapping.mockReturnValue(false); + let state: boolean | null = false; + service.jupyterNotebookPanelVisible$.subscribe(v => (state = v)); + service.openJupyterNotebookPanel(); + expect(state).toBe(false); + expect(mockNotification.warning).not.toHaveBeenCalled(); + }); + it("onWorkflowComponentClick does not postMessage to the iframe", async () => { const mockIframe = { contentWindow: { postMessage: vi.fn() }, diff --git a/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts b/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts index f2083e5c5d..ceccff995a 100644 --- a/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts +++ b/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts @@ -18,10 +18,11 @@ */ import { Injectable } from "@angular/core"; -import { catchError, map, of } from "rxjs"; +import { BehaviorSubject, catchError, map, of } from "rxjs"; import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; import { OperatorLink } from "../../types/workflow-common.interface"; import { HttpClient, HttpHeaders } from "@angular/common/http"; +import { NotificationService } from "src/app/common/service/notification/notification.service"; import { distinctUntilChanged, switchMap } from "rxjs/operators"; import { AppSettings } from "../../../common/app-setting"; import { NotebookMigrationService } from "../notebook-migration/notebook-migration.service"; @@ -31,6 +32,16 @@ import { GuiConfigService } from "../../../common/service/gui-config.service"; providedIn: "root", }) export class JupyterPanelService { + private jupyterNotebookPanelVisible = new BehaviorSubject<boolean>(false); + public jupyterNotebookPanelVisible$ = this.jupyterNotebookPanelVisible.asObservable(); + + // Whether the current workflow has an associated notebook in the migration DB. + // Driven by the per-workflow fetch in init(): reset on every workflow change, + // set true only when a notebook/mapping is found. Used to gate the toolbar's + // expand button so it appears only for workflows that actually have a notebook. + private jupyterNotebookExists = new BehaviorSubject<boolean>(false); + public jupyterNotebookExists$ = this.jupyterNotebookExists.asObservable(); + private iframeRef: HTMLIFrameElement | null = null; // Store reference to iframe element // Precomputed dictionary for cell to highlight mapping @@ -42,6 +53,7 @@ export class JupyterPanelService { constructor( private workflowActionService: WorkflowActionService, private http: HttpClient, + private notificationService: NotificationService, private notebookMigrationService: NotebookMigrationService, private config: GuiConfigService ) { @@ -87,27 +99,21 @@ export class JupyterPanelService { distinctUntilChanged() ) .subscribe(wid => { - // On every workflow change, drop the outgoing workflow's stale mapping - // and clear the highlight index. Clearing here (not only inside - // precomputeHighlightMapping, which runs only on a successful fetch) - // ensures switching to a workflow without a stored notebook can't leave - // the previous workflow's highlights active. This cleanup previously - // happened inside closeJupyterNotebookPanel; the panel-visibility - // surface lives with the iframe component in - // `migration-tool-jupyter-panel` now, so it is inlined. - const currentWid = this.workflowActionService.getWorkflow().wid; - if (currentWid !== undefined) { - this.notebookMigrationService.deleteMapping("mapping_wid_" + currentWid); - } + // On every workflow change, close the panel (which also drops the + // outgoing workflow's stale mapping) and clear the highlight index, so a + // switch to a workflow without a stored notebook can't leave the + // previous workflow's highlights active. + this.closeJupyterNotebookPanel(); this.cellToHighlightMapping = {}; + this.jupyterNotebookExists.next(false); // Skip unsaved workflows (wid undefined) and wid 0; both would POST // without a usable wid and 500 on the backend. if (wid) { this.fetchNotebookAndMapping(wid).subscribe(result => { if (result == 1) { + this.jupyterNotebookExists.next(true); this.precomputeHighlightMapping(); - // Panel auto-open on workflow restore is wired in - // `migration-tool-jupyter-panel` once the visibility API exists. + this.openJupyterNotebookPanel(); } }); } @@ -198,6 +204,45 @@ export class JupyterPanelService { this.iframeRef = iframe; } + // Open the Jupyter Notebook panel + public openPanel(panelName: string): void { + if (!this.enabled) return; + if (panelName === "JupyterNotebookPanel") { + this.jupyterNotebookPanelVisible.next(true); + } + } + + // Close the Jupyter Notebook panel + public closeJupyterNotebookPanel(): void { + if (!this.enabled) return; + this.jupyterNotebookPanelVisible.next(false); + const wid = this.workflowActionService.getWorkflow().wid; + if (wid != undefined) { + this.notebookMigrationService.deleteMapping("mapping_wid_" + wid); + } + } + + // Minimize the Jupyter Notebook panel + public minimizeJupyterNotebookPanel(): void { + if (!this.enabled) return; + this.jupyterNotebookPanelVisible.next(false); + } + + // Expand the Jupyter Notebook panel + public openJupyterNotebookPanel(): void { + if (!this.enabled) return; + const wid = this.workflowActionService.getWorkflow().wid; + const mappingKey = "mapping_wid_" + wid; + // Check if there is corresponding mapping data + if (wid === undefined || !this.notebookMigrationService.hasMapping(mappingKey)) { + this.notificationService.warning("No Jupyter notebook associated with this workflow."); + return; + } + + // Expand only if the mapping exists + this.jupyterNotebookPanelVisible.next(true); + } + // Handle messages from the Jupyter notebook iframe private handleNotebookMessage = async (event: MessageEvent) => { if (!this.enabled) return;
