This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new 81d4612dc7 feat(python-notebook-migration, frontend): add Jupyter
panel service (visibility surface and notebook-exists signal) (#5265)
81d4612dc7 is described below
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;