perf(workflow): switch between a workflow's two views without reloading the page

frontend/src/app/app-routing.constant.ts

@@ -17,6 +17,9 @@
  * under the License.
  */
 
+// Type-only: this module is imported all over the app and must not pull the router in at runtime.
+import type { Router } from "@angular/router";
+
 export const HOME = "/home";
 export const ABOUT = "/about";
 export const LOGIN = "/login";
@@ -35,6 +38,44 @@ export const HUB_MODEL_RESULT_DETAIL = `${HUB_MODEL_RESULT}/detail`;
 export const USER = "/user";
 export const USER_WORKSPACE = `${USER}/workflow`;
 export const USER_WORKFLOW = `${USER}/workflow`;
+
+/** One workflow is open in the workspace under two views: the operator canvas and the Form View. */
+export const workspaceCanvasUrl = (wid: number): string => `${USER_WORKSPACE}/${wid}`;
+export const workspaceFormUrl = (wid: number): string => `${workspaceCanvasUrl(wid)}/form`;
+
+/**
+ * Whether `url` is one of the two views workflow `wid` is open under. A falsy `wid` is no
+ * workflow: `DEFAULT_WORKFLOW` carries 0 until the first save gives it an id.
+ */
+function isWorkspaceViewOf(url: string, wid: number | undefined): boolean {
+  if (!wid) {
+    return false;
+  }
+  const path = url.split(/[?#]/)[0];
+  return path === workspaceCanvasUrl(wid) || path === workspaceFormUrl(wid);
+}
+
+/**
+ * Whether the navigation now in flight is leaving workflow `wid`, rather than moving between the
+ * two views it is open under.
+ *
+ * Both views ask this as they are destroyed, and both must answer it the same way: the session
+ * below them -- the shared document and the co-editing room it holds, the computing unit
+ * connection, the execution state -- belongs to the workflow, not to either view. Moving between
+ * the views hands it over; leaving the pair drops it. No navigation in flight means the view is
+ * being destroyed for some reason other than routing, and so has no successor to hand to, which
+ * counts as leaving. Unloading the page is not one of those: since #8600 neither view tears down
+ * on `beforeunload` at all, precisely so a document restored from the back/forward cache still
+ * has the session it was left with.
+ *
+ * `wid` is the workflow the caller is actually holding open, not the one in its route: a workflow
+ * created by the first autosave has no id in the route it was opened with.
+ */
+export function isLeavingWorkspace(router: Router, wid: number | undefined): boolean {
+  const target = router.getCurrentNavigation()?.finalUrl;
+  return !target || !isWorkspaceViewOf(router.serializeUrl(target), wid);
+}
+
 export const USER_DATASET = `${USER}/dataset`;
 export const USER_DATASET_CREATE = `${USER_DATASET}/create`;
 export const USER_MODEL = `${USER}/model`;

frontend/src/app/workspace/component/menu/menu.component.spec.ts

@@ -51,7 +51,7 @@ import type { ComputingUnitSelectionComponent } from "../power-button/computing-
 import { WorkflowContent } from "../../../common/type/workflow";
 import { Router } from "@angular/router";
 import { ReportGenerationService } from "../../service/report-generation/report-generation.service";
-import { USER_WORKFLOW } from "../../../app-routing.constant";
+import { USER_WORKFLOW, workspaceFormUrl } from "../../../app-routing.constant";
 import { GuiConfigService } from "../../../common/service/gui-config.service";
 import { MockGuiConfigService } from "../../../common/service/gui-config.service.mock";
 import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service";
@@ -121,11 +121,21 @@ describe("MenuComponent", () => {
 
   it("does not open the Form View for a workflow that has not been saved yet", () => {
     vi.spyOn(component["workflowActionService"], "getWorkflowMetadata").mockReturnValue({ wid: undefined } as any);
-    const href = window.location.href;
+    const navigate = vi.spyOn(component as any, "openFormViewPage");
 
     component.onClickOpenFormView();
 
-    expect(window.location.href).toBe(href);
+    expect(navigate).not.toHaveBeenCalled();
+  });
+
+  // A route, not a page load: the workflow stays open across the switch, so the shared document,
+  // the computing unit connection and a running execution are handed over rather than rebuilt.
+  it("routes to the Form View rather than reloading the page", () => {
+    const navigateByUrl = vi.spyOn(TestBed.inject(Router), "navigateByUrl").mockResolvedValue(true);
+
+    (component as any).openFormViewPage(42);
+
+    expect(navigateByUrl).toHaveBeenCalledWith(workspaceFormUrl(42));
   });
 
   it("hands over to the id the save assigned when the canvas held a workflow never saved yet", () => {
@@ -1131,104 +1141,24 @@ describe("MenuComponent", () => {
   // (base-duration updates, 1s cadence, restart-on-event, stop-when-idle) and,
   // crucially, that the timer is torn down with the component so it cannot keep
   // firing or leak after destroy.
-  describe("execution duration timer", () => {
-    let durationEvents$: Subject<{ type: "ExecutionDurationUpdateEvent" } & ExecutionDurationUpdateEvent>;
-    let timerFixture: ComponentFixture<MenuComponent>;
-    let timerComponent: MenuComponent;
-
-    function emitDuration(duration: number, isRunning: boolean): void {
-      durationEvents$.next({ type: "ExecutionDurationUpdateEvent", duration, isRunning });
-    }
-
-    beforeEach(() => {
-      vi.useFakeTimers();
-      durationEvents$ = new Subject();
-      const websocket = TestBed.inject(WorkflowWebsocketService);
-      const original = websocket.subscribeToEvent.bind(websocket);
-      // Only intercept the duration event; defer every other event type to the
-      // real implementation so unrelated subscriptions keep working.
-      vi.spyOn(websocket, "subscribeToEvent").mockImplementation((type: any) =>
-        type === "ExecutionDurationUpdateEvent" ? (durationEvents$.asObservable() as any) : original(type)
-      );
-
-      timerFixture = TestBed.createComponent(MenuComponent);
-      timerComponent = timerFixture.componentInstance;
-      timerFixture.detectChanges();
-    });
-
-    afterEach(() => {
-      vi.useRealTimers();
-    });
-
-    it("sets executionDuration to the event's base duration on each event", () => {
-      emitDuration(5000, false);
-      expect(timerComponent.executionDuration).toBe(5000);
-
-      emitDuration(8000, false);
-      expect(timerComponent.executionDuration).toBe(8000);
-    });
-
-    it("advances the duration by 1s every second while running", () => {
-      emitDuration(0, true);
-      expect(timerComponent.executionDuration).toBe(0);
-
-      vi.advanceTimersByTime(1000);
-      expect(timerComponent.executionDuration).toBe(1000);
-
-      vi.advanceTimersByTime(2000);
-      expect(timerComponent.executionDuration).toBe(3000);
-    });
-
-    it("does not start a timer when the execution is not running", () => {
-      emitDuration(7000, false);
-
-      vi.advanceTimersByTime(5000);
-
-      expect(timerComponent.executionDuration).toBe(7000);
-    });
-
-    it("restarts the 1s timer on each new running event, cancelling the previous one", () => {
-      emitDuration(0, true);
-      vi.advanceTimersByTime(1000);
-      expect(timerComponent.executionDuration).toBe(1000);
-
-      // A new event resets the base duration and restarts the cadence; the
-      // previous timer must be cancelled (switchMap) so it cannot double-count.
-      emitDuration(10000, true);
-      expect(timerComponent.executionDuration).toBe(10000);
-
-      vi.advanceTimersByTime(500);
-      expect(timerComponent.executionDuration).toBe(10000);
-
-      vi.advanceTimersByTime(500);
-      expect(timerComponent.executionDuration).toBe(11000);
-    });
-
-    it("stops the timer when a running execution transitions to not running", () => {
-      emitDuration(0, true);
-      vi.advanceTimersByTime(1000);
-      expect(timerComponent.executionDuration).toBe(1000);
-
-      emitDuration(2000, false);
-      vi.advanceTimersByTime(5000);
-      expect(timerComponent.executionDuration).toBe(2000);
-    });
-
-    it("tears down the timer on destroy so the duration stops advancing", () => {
-      emitDuration(0, true);
-      vi.advanceTimersByTime(1000);
-      expect(timerComponent.executionDuration).toBe(1000);
-
-      timerFixture.destroy();
-
-      // The previously running timer must not keep firing after destroy...
-      vi.advanceTimersByTime(5000);
-      expect(timerComponent.executionDuration).toBe(1000);
-
-      // ...nor should late events revive it (the source subscription is closed).
-      emitDuration(9999, true);
-      vi.advanceTimersByTime(5000);
-      expect(timerComponent.executionDuration).toBe(1000);
+  // The clock itself lives in ExecuteWorkflowService now -- anchored and ticked there, so a menu
+  // that mounts mid-run gets where the run has got to instead of starting from zero, which is what
+  // a routed switch between the canvas and the Form View makes. What is left here is that the menu
+  // shows what the service says.
+  describe("execution duration", () => {
+    it("shows the run clock the service reports", () => {
+      const ticks = new BehaviorSubject<number>(7000);
+      vi.spyOn(executeWorkflowService, "getExecutionDurationStream").mockReturnValue(ticks.asObservable());
+
+      const f = TestBed.createComponent(MenuComponent);
+      f.detectChanges();
+
+      // Replayed on subscribe: the value the run was already at when this menu mounted.
+      expect(f.componentInstance.executionDuration).toBe(7000);
+
+      ticks.next(8000);
+      expect(f.componentInstance.executionDuration).toBe(8000);
+      f.destroy();
     });
   });
 

frontend/src/app/workspace/component/menu/menu.component.ts

@@ -45,7 +45,7 @@ import { ResultExportationComponent } from "../result-exportation/result-exporta
 import { ReportGenerationService } from "../../service/report-generation/report-generation.service";
 import { ShareAccessComponent } from "src/app/dashboard/component/user/share-access/share-access.component";
 import { PanelService } from "../../service/panel/panel.service";
-import { USER_WORKFLOW, USER_WORKSPACE } from "../../../app-routing.constant";
+import { USER_WORKFLOW, workspaceFormUrl } from "../../../app-routing.constant";
 import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service";
 import { WarehouseService } from "../../../common/service/warehouse/warehouse.service";
 import { ComputingUnitState } from "../../../common/type/computing-unit-connection.interface";
@@ -197,17 +197,14 @@ export class MenuComponent implements OnInit, OnDestroy {
     private jupyterPanelService: JupyterPanelService,
     private fileSaverService: FileSaverService
   ) {
-    workflowWebsocketService
-      .subscribeToEvent("ExecutionDurationUpdateEvent")
-      .pipe(
-        tap(event => (this.executionDuration = event.duration)),
-        // restart the 1s timer on each event, only while running
-        switchMap(event => (event.isRunning ? timer(1000, 1000) : EMPTY)),
-        untilDestroyed(this)
-      )
-      .subscribe(() => {
-        this.executionDuration += 1000;
-      });
+    // From the service, not from the engine's event directly: that event arrives twice in a whole
+    // run, so a timer hung off it never started for a menu that mounted in between -- which is what
+    // a routed switch between the canvas and the Form View makes. The service anchors the clock and
+    // ticks it, and replays the current value to whoever subscribes.
+    executeWorkflowService
+      .getExecutionDurationStream()
+      .pipe(untilDestroyed(this))
+      .subscribe(duration => (this.executionDuration = duration));
     this.executionState = executeWorkflowService.getExecutionState().state;
     // return the run button after the execution is finished, either
     //  when the value is valid or invalid
@@ -756,15 +753,16 @@ export class MenuComponent implements OnInit, OnDestroy {
   }
 
   /**
-   * The full-page handover to the Form View, apart from the save so the order is testable.
-   * Excluded from coverage as a whole: jsdom cannot navigate, so the specs stub this method and
-   * assert when it is called rather than what it does.
+   * The hand-over to the Form View, apart from the save so the order is testable.
+   *
+   * A route, not a page load: the two views are views of one open workflow, and reloading threw
+   * away everything that made the workflow live -- the shared document, the computing unit
+   * connection, the execution state -- only to rebuild it on the other side. The canvas keeps
+   * the session on its way out (see its ngOnDestroy) and the Form View attaches to it.
    */
-  /* v8 ignore start */
   private openFormViewPage(wid: number): void {
-    window.location.href = `${USER_WORKSPACE}/${wid}/form`;
+    void this.router.navigateByUrl(workspaceFormUrl(wid));
   }
-  /* v8 ignore stop */
 
   /**
    * Calls Markdown Description Component

frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts

@@ -49,6 +49,7 @@ import { ComputingUnitCreateModalComponent } from "../../../common/component/com
 import { NoopAnimationsModule } from "@angular/platform-browser/animations";
 import { DEFAULT_WORKFLOW, WorkflowActionService } from "../../service/workflow-graph/model/workflow-action.service";
 import { WorkflowExecutionsService } from "../../../dashboard/service/user/workflow-executions/workflow-executions.service";
+import { ExecuteWorkflowService } from "../../service/execute-workflow/execute-workflow.service";
 import { WorkflowExecutionsEntry } from "../../../dashboard/type/workflow-executions-entry";
 import { WorkflowMetadata } from "../../../dashboard/type/workflow-metadata.interface";
 import { ExecutionState } from "../../types/execute-workflow.interface";
@@ -1242,6 +1243,28 @@ describe("PowerButtonComponent", () => {
       expect(enableSpy).not.toHaveBeenCalled();
     });
 
+    // "No execution someone else started" is not "nothing is running here": this query asks only
+    // about Running and Initializing, and its answer lands asynchronously, after whatever the page
+    // set on arrival. A view handed a session with a run in flight had just been given the lock;
+    // enabling outright took it away again, leaving a running workflow editable.
+    it("leaves a run already in flight locked, even when the backend reports no executions", () => {
+      const actionService = TestBed.inject(WorkflowActionService);
+      vi.spyOn(actionService, "getWorkflowMetadata").mockReturnValue({ ...DEFAULT_WORKFLOW, wid: 42 });
+      const disableSpy = vi.spyOn(actionService, "disableWorkflowModification").mockImplementation(() => {});
+      const enableSpy = vi.spyOn(actionService, "enableWorkflowModification").mockImplementation(() => {});
+      vi.spyOn(TestBed.inject(WorkflowExecutionsService), "retrieveWorkflowExecutions").mockReturnValue(
+        of([] as WorkflowExecutionsEntry[])
+      );
+      // This page's own execute service is holding a run, as it would after a hand-over.
+      (TestBed.inject(ExecuteWorkflowService) as any).currentState = { state: ExecutionState.Running };
+
+      const { selected$ } = bootWithSelectedStream();
+      selected$.next(makeComputingUnit({ cuid: 7 }));
+
+      expect(disableSpy).toHaveBeenCalled();
+      expect(enableSpy).not.toHaveBeenCalled();
+    });
+
     it("enables workflow modification when there are no ongoing executions", () => {
       const actionService = TestBed.inject(WorkflowActionService);
       vi.spyOn(actionService, "getWorkflowMetadata").mockReturnValue({ ...DEFAULT_WORKFLOW, wid: 42 });

frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts

@@ -33,6 +33,7 @@ import { WarehouseActionsService } from "../../../common/service/warehouse/wareh
 import { DashboardWarehouse } from "../../../common/type/warehouse";
 import { NzModalService, NzModalComponent, NzModalContentDirective } from "ng-zorro-antd/modal";
 import { WorkflowExecutionsService } from "../../../dashboard/service/user/workflow-executions/workflow-executions.service";
+import { ExecuteWorkflowService } from "../../service/execute-workflow/execute-workflow.service";
 import { WorkflowExecutionsEntry } from "../../../dashboard/type/workflow-executions-entry";
 import { ExecutionState } from "../../types/execute-workflow.interface";
 import { ShareAccessComponent } from "../../../dashboard/component/user/share-access/share-access.component";
@@ -210,7 +211,8 @@ export class ComputingUnitSelectionComponent implements OnInit {
     private workflowPveService: WorkflowPveService,
     private ngZone: NgZone,
     private warehouseService: WarehouseService,
-    private warehouseActionsService: WarehouseActionsService
+    private warehouseActionsService: WarehouseActionsService,
+    private executeWorkflowService: ExecuteWorkflowService
   ) {}
 
   ngOnInit(): void {
@@ -310,7 +312,12 @@ export class ComputingUnitSelectionComponent implements OnInit {
           );
           this.workflowActionService.disableWorkflowModification();
         } else {
-          this.workflowActionService.enableWorkflowModification();
+          // "No execution someone else started" is not "nothing is running here". This asks only
+          // about Running and Initializing, and the answer arrives asynchronously, after whatever
+          // the page set on arrival, so unlocking outright undid the lock a run in flight had just
+          // been given -- a handed-over Paused or Recovering run, say, which this query does not
+          // look for. Defer to the state-to-lock rule the execute service owns.
+          this.executeWorkflowService.reapplyExecutionLock();
         }
       });
   }