diff --git a/frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.html b/frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.html new file mode 100644 index 00000000000..edca1ba47b8 --- /dev/null +++ b/frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.html @@ -0,0 +1,67 @@ + + +
+
+ Jupyter Notebook +
+ + + + +
+
+ +
+ +
+
diff --git a/frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.scss b/frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.scss new file mode 100644 index 00000000000..8d8bb0631f9 --- /dev/null +++ b/frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.scss @@ -0,0 +1,77 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +.draggable-panel { + position: absolute; + top: 50%; + left: 50%; + margin-top: 200px; + width: 660px; + height: 400px; + background-color: #fff; + box-shadow: 0 1px 5px rgba(0, 0, 0, 0.3); + border: 1px solid #ccc; + border-radius: 5px; + z-index: 1000; + resize: both; + overflow: auto; +} + +.panel-header { + display: flex; + justify-content: space-between; + align-items: center; + background-color: #547baa; + color: white; + padding: 5px 10px 5px 10px; + font-weight: bold; + text-align: center; + cursor: move; +} + +.panel-buttons { + display: flex; + align-items: center; + gap: 8px; +} + +.minimize-button, +.delete-button { + background: none; + border: none; + color: white; + font-size: 16px; + cursor: pointer; +} + +.minimize-button:hover, +.delete-button:hover { + color: #bfbfbf; +} + +.iframe-container { + width: 100%; + height: calc(100% - 40px); /* Adjust height for the header */ +} + +.iframe-container iframe { + width: 100%; + height: 100%; + border: none; +} diff --git a/frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.spec.ts b/frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.spec.ts new file mode 100644 index 00000000000..74d2e702983 --- /dev/null +++ b/frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.spec.ts @@ -0,0 +1,279 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { ComponentFixture, fakeAsync, TestBed, tick } from "@angular/core/testing"; +import { JupyterNotebookPanelComponent } from "./jupyter-notebook-panel.component"; +import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; +import { NotebookMigrationService } from "../../service/notebook-migration/notebook-migration.service"; +import { Subject } from "rxjs"; +import { ElementRef } from "@angular/core"; +import { By, DomSanitizer } from "@angular/platform-browser"; + +describe("JupyterNotebookPanelComponent", () => { + let component: JupyterNotebookPanelComponent; + let fixture: ComponentFixture; + + let mockJupyterPanelService: any; + let mockNotebookMigrationService: any; + let bypassSpy: ReturnType; + + beforeEach(async () => { + mockJupyterPanelService = { + jupyterNotebookPanelVisible$: new Subject(), + setIframeRef: vi.fn(), + deleteJupyterNotebook: vi.fn(), + minimizeJupyterNotebookPanel: vi.fn(), + }; + + mockNotebookMigrationService = { + getJupyterIframeURL: vi.fn().mockResolvedValue("http://localhost:8888"), + }; + + await TestBed.configureTestingModule({ + imports: [JupyterNotebookPanelComponent], + providers: [ + { provide: JupyterPanelService, useValue: mockJupyterPanelService }, + { provide: NotebookMigrationService, useValue: mockNotebookMigrationService }, + ], + }).compileComponents(); + }); + + beforeEach(() => { + fixture = TestBed.createComponent(JupyterNotebookPanelComponent); + component = fixture.componentInstance; + // Spy on the shared DomSanitizer so tests can assert the raw URL was trusted, + // without matching Angular's serialized SafeResourceUrl string. + bypassSpy = vi.spyOn(TestBed.inject(DomSanitizer), "bypassSecurityTrustResourceUrl"); + fixture.detectChanges(); + }); + + // Destroy the component so its subscriptions complete, and restore spies so a + // shared spy's call history (e.g. console.error) does not accumulate across + // tests. Mocks are not auto-reset by the Vitest config. + afterEach(() => { + fixture.destroy(); + vi.restoreAllMocks(); + }); + + it("should create", () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + expect(component).toBeTruthy(); + }); + + it("should be hidden by default", () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + expect(component.isVisible).toBe(false); + }); + + it("should update visibility when service emits", async () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + mockJupyterPanelService.jupyterNotebookPanelVisible$.next(true); + + await fixture.whenStable(); + fixture.detectChanges(); + + expect(component.isVisible).toBe(true); + }); + + it("should fetch and sanitize URL when panel becomes visible", async () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + mockJupyterPanelService.jupyterNotebookPanelVisible$.next(true); + + await fixture.whenStable(); + fixture.detectChanges(); + + expect(mockNotebookMigrationService.getJupyterIframeURL).toHaveBeenCalled(); + expect(bypassSpy).toHaveBeenCalledWith("http://localhost:8888"); + expect(component.jupyterUrl).toBe(bypassSpy.mock.results[0].value); + }); + + it("should not render the iframe while visible without a URL", () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + component.isVisible = true; + + // Rendering with an unset jupyterUrl must not throw NG0904 (unsafe resource + // URL); the iframe should simply be absent until a URL is available. + expect(() => fixture.detectChanges()).not.toThrow(); + expect(fixture.nativeElement.querySelector("iframe")).toBeNull(); + }); + + it("should render the iframe once a URL is available", async () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + mockJupyterPanelService.jupyterNotebookPanelVisible$.next(true); + + await fixture.whenStable(); + fixture.detectChanges(); + + const iframe = fixture.nativeElement.querySelector("iframe"); + expect(iframe).not.toBeNull(); + // Don't match the serialized SafeResourceUrl; assert the binding produced a src. + expect(iframe.hasAttribute("src")).toBe(true); + }); + + it("should clear jupyterUrl and remove the iframe when hidden", async () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + mockJupyterPanelService.jupyterNotebookPanelVisible$.next(true); + await fixture.whenStable(); + fixture.detectChanges(); + expect(component.jupyterUrl).not.toBeNull(); + expect(fixture.nativeElement.querySelector("iframe")).not.toBeNull(); + + mockJupyterPanelService.jupyterNotebookPanelVisible$.next(false); + await fixture.whenStable(); + fixture.detectChanges(); + expect(component.jupyterUrl).toBeNull(); + expect(fixture.nativeElement.querySelector("iframe")).toBeNull(); + }); + + it("should not update jupyterUrl when the iframe URL fetch rejects", async () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + vi.spyOn(console, "error").mockImplementation(() => {}); + mockNotebookMigrationService.getJupyterIframeURL.mockRejectedValueOnce(new Error("network error")); + + mockJupyterPanelService.jupyterNotebookPanelVisible$.next(true); + + await fixture.whenStable(); + + expect(mockNotebookMigrationService.getJupyterIframeURL).toHaveBeenCalled(); + expect(component.jupyterUrl).toBeNull(); + }); + + it("should keep handling visibility emissions after a failed fetch", async () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + vi.spyOn(console, "error").mockImplementation(() => {}); + mockNotebookMigrationService.getJupyterIframeURL + .mockRejectedValueOnce(new Error("network error")) + .mockResolvedValueOnce("http://localhost:9999"); + + mockJupyterPanelService.jupyterNotebookPanelVisible$.next(true); + await fixture.whenStable(); + + mockJupyterPanelService.jupyterNotebookPanelVisible$.next(true); + await fixture.whenStable(); + fixture.detectChanges(); + + expect(mockNotebookMigrationService.getJupyterIframeURL).toHaveBeenCalledTimes(2); + expect(bypassSpy).toHaveBeenCalledWith("http://localhost:9999"); + expect(component.jupyterUrl).toBe(bypassSpy.mock.results[0].value); + }); + + it("should call setIframeRef when iframe exists and visible", fakeAsync(() => { + component.isVisible = true; + + const mockIframe = document.createElement("iframe"); + component.iframeRef = new ElementRef(mockIframe); + + component.checkIframeRef(); + tick(0); + + expect(mockJupyterPanelService.setIframeRef).toHaveBeenCalledWith(mockIframe); + })); + + it("should NOT call setIframeRef if not visible", fakeAsync(() => { + component.isVisible = false; + + const mockIframe = document.createElement("iframe"); + component.iframeRef = new ElementRef(mockIframe); + + component.checkIframeRef(); + tick(0); + + expect(mockJupyterPanelService.setIframeRef).not.toHaveBeenCalled(); + })); + + it("should not log an error when checkIframeRef runs while the panel is hidden", fakeAsync(() => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + component.isVisible = false; + + component.checkIframeRef(); + tick(0); + + expect(errorSpy).not.toHaveBeenCalled(); + expect(mockJupyterPanelService.setIframeRef).not.toHaveBeenCalled(); + })); + + it("should log an error when visible but the iframe ref is missing", fakeAsync(() => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + component.isVisible = true; + component.iframeRef = undefined as any; + + component.checkIframeRef(); + tick(0); + + expect(errorSpy).toHaveBeenCalledWith("Jupyter Iframe reference not found."); + expect(mockJupyterPanelService.setIframeRef).not.toHaveBeenCalled(); + })); + + it("should delete notebook via service", () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + component.deletePanel(); + expect(mockJupyterPanelService.deleteJupyterNotebook).toHaveBeenCalled(); + }); + + it("should minimize panel and update visibility", () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + // The real service emits false on minimize; visibility is observable-driven, + // so the mock must emit for isVisible to update. + mockJupyterPanelService.minimizeJupyterNotebookPanel = vi.fn(() => + mockJupyterPanelService.jupyterNotebookPanelVisible$.next(false) + ); + component.isVisible = true; + + component.minimizePanel(); + + expect(component.isVisible).toBe(false); + expect(mockJupyterPanelService.minimizeJupyterNotebookPanel).toHaveBeenCalled(); + }); + + it("should minimize via the header minimize button click", () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + component.isVisible = true; + fixture.detectChanges(); + + const minimizeButton = fixture.debugElement.query(By.css(".minimize-button")); + expect(minimizeButton).not.toBeNull(); + minimizeButton.triggerEventHandler("click", new MouseEvent("click")); + + expect(mockJupyterPanelService.minimizeJupyterNotebookPanel).toHaveBeenCalled(); + }); + + it("should delete via the header delete button popconfirm", () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + component.isVisible = true; + fixture.detectChanges(); + + const deleteButton = fixture.debugElement.query(By.css(".delete-button")); + expect(deleteButton).not.toBeNull(); + // Fire the popconfirm's confirm output, which runs the (nzOnConfirm) binding. + deleteButton.triggerEventHandler("nzOnConfirm", undefined); + + expect(mockJupyterPanelService.deleteJupyterNotebook).toHaveBeenCalled(); + }); + + it("should clean up on destroy", () => { + vi.spyOn(component, "checkIframeRef").mockImplementation(() => {}); + const nextSpy = vi.spyOn(component["destroy$"] as any, "next"); + const completeSpy = vi.spyOn(component["destroy$"] as any, "complete"); + + component.ngOnDestroy(); + + expect(nextSpy).toHaveBeenCalled(); + expect(completeSpy).toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.ts b/frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.ts new file mode 100644 index 00000000000..4f3d957b515 --- /dev/null +++ b/frontend/src/app/workspace/component/jupyter-notebook-panel/jupyter-notebook-panel.component.ts @@ -0,0 +1,115 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Component, ElementRef, OnDestroy, OnInit, ViewChild, AfterViewInit } from "@angular/core"; +import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; +import { from, of, Subject } from "rxjs"; +import { catchError, switchMap, takeUntil } from "rxjs/operators"; +import { DomSanitizer, SafeResourceUrl } from "@angular/platform-browser"; +import { NotebookMigrationService } from "../../service/notebook-migration/notebook-migration.service"; +import { CommonModule } from "@angular/common"; +import { DragDropModule } from "@angular/cdk/drag-drop"; +import { NzButtonModule } from "ng-zorro-antd/button"; +import { NzIconModule } from "ng-zorro-antd/icon"; +import { NzPopconfirmModule } from "ng-zorro-antd/popconfirm"; +import { NzTooltipModule } from "ng-zorro-antd/tooltip"; + +@Component({ + selector: "texera-jupyter-notebook-panel", + templateUrl: "./jupyter-notebook-panel.component.html", + styleUrls: ["./jupyter-notebook-panel.component.scss"], + imports: [CommonModule, DragDropModule, NzButtonModule, NzIconModule, NzPopconfirmModule, NzTooltipModule], +}) +export class JupyterNotebookPanelComponent implements OnInit, AfterViewInit, OnDestroy { + @ViewChild("iframeRef", { static: false }) iframeRef!: ElementRef; // Use static: false + + isVisible: boolean = false; // Initialize to false, meaning the panel is hidden by default + jupyterUrl: SafeResourceUrl | null = null; // Store the notebook URL dynamically; null when no URL is loaded + private destroy$ = new Subject(); + + constructor( + private jupyterPanelService: JupyterPanelService, + private sanitizer: DomSanitizer, + private notebookMigrationService: NotebookMigrationService + ) {} + + ngOnInit(): void { + this.jupyterPanelService.jupyterNotebookPanelVisible$ + .pipe( + switchMap((visible: boolean) => { + this.isVisible = visible; + + if (!visible) { + return of(null); + } + + return from(this.notebookMigrationService.getJupyterIframeURL()).pipe( + catchError(() => { + console.error("Failed to fetch Jupyter iframe URL."); + return of(null); + }) + ); + }), + takeUntil(this.destroy$) + ) + .subscribe(url => { + // Always reflect the latest fetch result. A null url (panel hidden, or a + // failed/empty fetch) clears jupyterUrl so a stale URL is never rendered. + this.jupyterUrl = url ? this.sanitizer.bypassSecurityTrustResourceUrl(url) : null; + if (url) { + this.checkIframeRef(); + } + }); + } + + ngAfterViewInit(): void { + // Ensure iframe is handled after it's available in the DOM + this.checkIframeRef(); + } + + checkIframeRef(): void { + setTimeout(() => { + if (!this.isVisible) { + // Panel hidden; no iframe to register. + return; + } + if (this.iframeRef?.nativeElement) { + this.jupyterPanelService.setIframeRef(this.iframeRef.nativeElement); + } else { + console.error("Jupyter Iframe reference not found."); + } + }, 0); // Small timeout to ensure DOM is updated + } + + ngOnDestroy(): void { + this.destroy$.next(); + this.destroy$.complete(); // Cleanup subscriptions to avoid memory leaks + } + + // Delete the workflow's Jupyter notebook by invoking the service method + deletePanel(): void { + this.jupyterPanelService.deleteJupyterNotebook(); + } + + // Minimize the jupyter notebook by invoking the service method. Visibility is + // driven by jupyterNotebookPanelVisible$, so no local isVisible mutation here. + minimizePanel(): void { + this.jupyterPanelService.minimizeJupyterNotebookPanel(); + } +} diff --git a/frontend/src/app/workspace/component/workspace.component.html b/frontend/src/app/workspace/component/workspace.component.html index c54446fb318..89eeb656f18 100644 --- a/frontend/src/app/workspace/component/workspace.component.html +++ b/frontend/src/app/workspace/component/workspace.component.html @@ -37,4 +37,5 @@ *ngIf="copilotEnabled" [agentIdToActivate]="agentIdToActivate"> + diff --git a/frontend/src/app/workspace/component/workspace.component.ts b/frontend/src/app/workspace/component/workspace.component.ts index 2aa1c43db95..b978107c83f 100644 --- a/frontend/src/app/workspace/component/workspace.component.ts +++ b/frontend/src/app/workspace/component/workspace.component.ts @@ -63,6 +63,7 @@ import { MiniMapComponent } from "./workflow-editor/mini-map/mini-map.component" import { LeftPanelComponent } from "./left-panel/left-panel.component"; import { AgentPanelComponent } from "./agent/agent-panel/agent-panel.component"; import { PropertyEditorComponent } from "./property-editor/property-editor.component"; +import { JupyterNotebookPanelComponent } from "./jupyter-notebook-panel/jupyter-notebook-panel.component"; export const SAVE_DEBOUNCE_TIME_IN_MS = 5000; @@ -85,6 +86,7 @@ export const SAVE_DEBOUNCE_TIME_IN_MS = 5000; NgIf, AgentPanelComponent, PropertyEditorComponent, + JupyterNotebookPanelComponent, ], }) export class WorkspaceComponent implements AfterViewInit, OnInit, OnDestroy { 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 17c8c0d33f2..c60b06d368d 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 @@ -24,7 +24,7 @@ import { HttpClientTestingModule, HttpTestingController } from "@angular/common/ 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"; +import { firstValueFrom, of, throwError } from "rxjs"; describe("JupyterPanelService", () => { let service: JupyterPanelService; @@ -59,6 +59,7 @@ describe("JupyterPanelService", () => { mockNotification = { warning: vi.fn(), + error: vi.fn(), }; mockNotebook = { @@ -72,6 +73,7 @@ describe("JupyterPanelService", () => { deleteMapping: vi.fn(), setMapping: vi.fn(), getJupyterURL: vi.fn().mockResolvedValue("http://jupyter"), + deleteNotebookAndMapping: vi.fn().mockReturnValue(of({ success: true, deleted: 1 })), }; mockGuiConfig = { env: { pythonNotebookMigrationEnabled: true } }; @@ -96,7 +98,7 @@ describe("JupyterPanelService", () => { }); // Panel visibility - it("should open and close panel", () => { + it("should open panel and hide it after deleting the notebook", () => { let state: boolean | null = null; service.jupyterNotebookPanelVisible$.subscribe(v => (state = v)); @@ -104,10 +106,59 @@ describe("JupyterPanelService", () => { service.openPanel("JupyterNotebookPanel"); expect(state).toBe(true); - service.closeJupyterNotebookPanel(); + service.deleteJupyterNotebook(); + expect(mockNotebook.deleteNotebookAndMapping).toHaveBeenCalledWith(1); expect(state).toBe(false); }); + it("deleteJupyterNotebook clears local state and unhighlights on success", () => { + let visible: boolean | null = null; + let exists: boolean | null = null; + service.jupyterNotebookPanelVisible$.subscribe(v => (visible = v)); + service.jupyterNotebookExists$.subscribe(v => (exists = v)); + (service as any).jupyterNotebookPanelVisible.next(true); + (service as any).jupyterNotebookExists.next(true); + + service.deleteJupyterNotebook(); + + expect(mockNotebook.deleteNotebookAndMapping).toHaveBeenCalledWith(1); + expect(mockNotebook.deleteMapping).toHaveBeenCalledWith("mapping_wid_1"); + expect(visible).toBe(false); + expect(exists).toBe(false); + expect(mockWorkflow.unhighlightOperators).toHaveBeenCalled(); + expect(mockWorkflow.unhighlightLinks).toHaveBeenCalled(); + }); + + it("deleteJupyterNotebook keeps the panel open and notifies on failure", () => { + mockNotebook.deleteNotebookAndMapping.mockReturnValueOnce(throwError(() => new Error("boom"))); + let visible: boolean | null = null; + service.jupyterNotebookPanelVisible$.subscribe(v => (visible = v)); + (service as any).jupyterNotebookPanelVisible.next(true); + + service.deleteJupyterNotebook(); + + expect(mockNotification.error).toHaveBeenCalled(); + expect(visible).toBe(true); + expect(mockNotebook.deleteMapping).not.toHaveBeenCalled(); + }); + + it("deleteJupyterNotebook only resets local state for the default wid 0 (no backend call)", () => { + mockWorkflow.getWorkflow.mockReturnValue({ wid: 0 }); + let visible: boolean | null = null; + let exists: boolean | null = null; + service.jupyterNotebookPanelVisible$.subscribe(v => (visible = v)); + service.jupyterNotebookExists$.subscribe(v => (exists = v)); + (service as any).jupyterNotebookPanelVisible.next(true); + (service as any).jupyterNotebookExists.next(true); + + service.deleteJupyterNotebook(); + + // wid 0 is the unsaved default workflow, so no backend delete should fire. + expect(mockNotebook.deleteNotebookAndMapping).not.toHaveBeenCalled(); + expect(visible).toBe(false); + expect(exists).toBe(false); + }); + it("should minimize panel", () => { let state: boolean | null = true; @@ -152,6 +203,20 @@ describe("JupyterPanelService", () => { expect(state).toBe(true); }); + it("openPanel flags jupyterNotebookExists$ so the toolbar expand button appears after an in-place import", () => { + const states: boolean[] = []; + service.jupyterNotebookExists$.subscribe(v => states.push(v)); + expect(states.at(-1)).toBe(false); + + // Wrong panel name does not flip the flag. + service.openPanel("WrongPanel"); + expect(states.at(-1)).toBe(false); + + // Opening the jupyter panel records that the workflow now has a notebook. + service.openPanel("JupyterNotebookPanel"); + expect(states.at(-1)).toBe(true); + }); + // HTTP fetchNotebookAndMapping it("should return 0 when exists=false", async () => { const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(1, 1)); @@ -186,6 +251,9 @@ describe("JupyterPanelService", () => { expect(mockWorkflow.workflowMetaDataChanged).toHaveBeenCalled(); expect(mockNotebook.deleteMapping).toHaveBeenCalledWith("mapping_wid_1"); + // Data-loss guard: switching workflows must never delete a notebook from the + // backend. It only drops the in-memory mapping. + expect(mockNotebook.deleteNotebookAndMapping).not.toHaveBeenCalled(); const req = httpMock.expectOne(r => r.url.includes("/notebook-migration/fetch-notebook-and-mapping")); req.flush({ exists: false }); @@ -391,10 +459,11 @@ describe("JupyterPanelService", () => { 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(); + it("deleteJupyterNotebook does not call the backend or delete the mapping when disabled", () => { + // When the feature is disabled the method returns early, so neither the + // backend delete nor the local mapping drop should run. + service.deleteJupyterNotebook(); + expect(mockNotebook.deleteNotebookAndMapping).not.toHaveBeenCalled(); expect(mockNotebook.deleteMapping).not.toHaveBeenCalled(); }); 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 ceccff995a4..c4f8ffcfbbc 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 @@ -99,11 +99,12 @@ export class JupyterPanelService { distinctUntilChanged() ) .subscribe(wid => { - // 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(); + // On every workflow change, hide the panel and drop the outgoing + // workflow's stale in-memory 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 is local cleanup only: it must never + // delete from the backend, or switching workflows would erase notebooks. + this.hideAndClearLocalState(); this.cellToHighlightMapping = {}; this.jupyterNotebookExists.next(false); // Skip unsaved workflows (wid undefined) and wid 0; both would POST @@ -209,12 +210,46 @@ export class JupyterPanelService { if (!this.enabled) return; if (panelName === "JupyterNotebookPanel") { this.jupyterNotebookPanelVisible.next(true); + // Opening the panel means the current workflow has an associated notebook, so + // surface the toolbar "expand" button (jupyterNotebookExists$) right away. Needed + // after an in-place import where the wid does not change and init() does not re-run + // to detect the notebook. + this.jupyterNotebookExists.next(true); } } - // Close the Jupyter Notebook panel - public closeJupyterNotebookPanel(): void { + // Delete the current workflow's stored notebook from the backend, then hide the + // panel and clear all local notebook state. This is the user-initiated action + // behind the panel's delete button, and is distinct from the workflow-switch + // cleanup (hideAndClearLocalState), which must never touch the backend. + public deleteJupyterNotebook(): void { if (!this.enabled) return; + const wid = this.workflowActionService.getWorkflow().wid; + // Unsaved workflow (wid undefined or the default wid 0): nothing is persisted, + // and a delete POST with such a wid would 500, so just reset local state. + if (!wid) { + this.hideAndClearLocalState(); + this.jupyterNotebookExists.next(false); + this.clearHighlights(); + return; + } + this.notebookMigrationService.deleteNotebookAndMapping(wid).subscribe({ + next: () => { + this.hideAndClearLocalState(); + this.jupyterNotebookExists.next(false); + this.clearHighlights(); + }, + error: (err: unknown) => { + // Keep the panel open on failure so the user sees the notebook wasn't removed. + console.error("Failed to delete Jupyter notebook:", err); + this.notificationService.error("Failed to delete the Jupyter notebook."); + }, + }); + } + + // Hide the panel and drop the current workflow's in-memory mapping. Local only; + // never calls the backend. Used on workflow switch and after a successful delete. + private hideAndClearLocalState(): void { this.jupyterNotebookPanelVisible.next(false); const wid = this.workflowActionService.getWorkflow().wid; if (wid != undefined) { @@ -222,6 +257,24 @@ export class JupyterPanelService { } } + // Unhighlight all operators and links and drop the highlight index, used once + // the notebook is gone so no stale cell-to-operator highlights remain. + private clearHighlights(): void { + this.workflowActionService.unhighlightOperators( + ...this.workflowActionService + .getTexeraGraph() + .getAllOperators() + .map(op => op.operatorID) + ); + this.workflowActionService.unhighlightLinks( + ...this.workflowActionService + .getTexeraGraph() + .getAllLinks() + .map(link => link.linkID) + ); + this.cellToHighlightMapping = {}; + } + // Minimize the Jupyter Notebook panel public minimizeJupyterNotebookPanel(): void { if (!this.enabled) return; diff --git a/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts b/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts index 6de43b41464..6570e47a00d 100644 --- a/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts +++ b/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts @@ -229,6 +229,19 @@ describe("NotebookMigrationService", () => { req.flush({ success: true, message: "stored" }); }); + // deleteNotebookAndMapping + it("should call deleteNotebookAndMapping API with the wid", () => { + let result: any; + service.deleteNotebookAndMapping(7).subscribe(r => (result = r)); + + const req = httpMock.expectOne(req => req.url.includes("/notebook-migration/delete-notebook-and-mapping")); + + expect(req.request.method).toBe("POST"); + expect(req.request.body).toEqual({ wid: 7 }); + req.flush({ success: true, deleted: 1 }); + expect(result).toEqual({ success: true, deleted: 1 }); + }); + // sendToAIGenerateWorkflow (enabled) — drives the NotebookMigrationLLM lifecycle. // The service builds the client through its createMigrationLLM() seam, so stub // that with a plain fake. This keeps the real NotebookMigrationLLM (and its "ai" @@ -323,5 +336,11 @@ describe("NotebookMigrationService", () => { expect(result.success).toBe(false); httpMock.expectNone(req => req.url.includes("/notebook-migration/store-notebook-and-mapping")); }); + + it("deleteNotebookAndMapping emits a disabled result without making an HTTP call", async () => { + const result = await firstValueFrom(service.deleteNotebookAndMapping(7)); + expect(result.success).toBe(false); + httpMock.expectNone(req => req.url.includes("/notebook-migration/delete-notebook-and-mapping")); + }); }); }); diff --git a/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts b/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts index 7bfd3ef541e..3d6600c2a2e 100644 --- a/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts +++ b/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts @@ -48,6 +48,12 @@ interface StoreNotebookResponse { message: string; } +interface DeleteNotebookResponse { + success: boolean; + deleted?: number; + message?: string; +} + @Injectable({ providedIn: "root", }) @@ -209,6 +215,22 @@ export class NotebookMigrationService { return this.http.post(dbAPIUrl, payload, { headers }); } + // Delete the stored notebook and its mapping for a workflow. The backend's + // notebook -> workflow_notebook_mapping FK is ON DELETE CASCADE, and wid alone + // identifies the notebook, so only wid is sent. `deleted` is 1 when a notebook + // was removed, 0 when nothing was stored. + public deleteNotebookAndMapping(wid: number | undefined): Observable { + if (!this.enabled) { + return of({ success: false, message: "Notebook migration feature is disabled" }); + } + const dbAPIUrl = `${AppSettings.getApiEndpoint()}/notebook-migration/delete-notebook-and-mapping`; + const headers = new HttpHeaders({ "Content-Type": "application/json" }); + + const payload = { wid }; + + return this.http.post(dbAPIUrl, payload, { headers }); + } + public hasMapping(id: string): boolean { return id in this.mapping; }