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 a90714c0417..17c8c0d33f2 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 f2083e5c5d6..ceccff995a4 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(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(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;