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-8391-dcb25efa9e1d4321b7b99459db4ee777f37cbaff in repository https://gitbox.apache.org/repos/asf/texera.git
commit 873c91ce70f15266383e82c2e0f74050692c2b84 Author: yangzhang75 <[email protected]> AuthorDate: Sat Sep 5 05:32:33 2026 +0000 feat(workflow-form): Form View title bar, save, and modification lock (#8391) ### What changes were proposed in this PR? Building on the Form View page shell (#8376), this PR adds the title bar and the save it needs (parent issue #8011). Frontend only. * The workflow name, renamable inline exactly as on the operator canvas, with the same "Saved at ..." state beside it. Both follow the workflow's metadata live through the same `workflowMetaDataChanged` subscription the canvas uses, so a rename or save (this page's own, or a co-editor's) refreshes the name, its width, and the saved-at state from one place, and the two views never drift apart. * The debounced save both views share, so an edit in one view is not lost in the other. It saves only the workflow this page opened (never spawning a stray "Untitled workflow" from a payload with no id), preserves a position for every operator (a partial map makes a workflow unopenable), saves once more on the way out and before handing back to the canvas, and reports a failed save rather than losing it silently. * Locks the workflow to read-only on this page (`disableWorkflowModification`): the graph and its properties are shown, not edited. A later PR's authoring mode makes them editable with write access. The read-only preview, the inputs, running and results are added by later PRs. #### Screenshot The title bar (renamable name, "Saved at ..." state, the Canvas / Form View switch) on the read-only page: <img width="1280" height="86" alt="Screenshot 2026-09-03 at 11 18 10 PM" src="https://github.com/user-attachments/assets/b1fa29a8-8b08-4b0c-bfda-51e62d8628f6"; /> ### Any related issues, documentation, discussions? Part of the Form View feature (parent issue #8011); stacked on #8376 (the page shell). Until #8376 merges, this PR's diff includes that PR's commit as well. close #8019 ### How was this PR tested? Unit tests (vitest): `workflow-form.component.spec.ts` (21 tests). As with the shell, the component injects heavy services and child components, so its spec constructs it directly with the shared harness's mocks rather than through TestBed; template rendering is therefore not exercised, but the component and the harness are at 100% of the TypeScript lines this PR adds. The template compiles cleanly under `ng build`. ### Was this PR authored or co-authored using generative AI tooling? Yes. Co-authored with Claude (Anthropic), reviewed line by line by the author before submission. Co-authored-by: Claude Opus 4.8 <[email protected]> --- .../workflow-form/workflow-form.component.html | 16 +- .../workflow-form/workflow-form.component.scss | 23 ++- .../workflow-form/workflow-form.component.spec.ts | 206 ++++++++++++++++++++- .../workflow-form/workflow-form.component.ts | 184 +++++++++++++++++- .../workflow-form/workflow-form.rendered.spec.ts | 40 +++- .../workflow-form/workflow-form.spec-harness.ts | 25 ++- 6 files changed, 468 insertions(+), 26 deletions(-) diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html index 618e8c0e1c..a1402b04f2 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html @@ -19,14 +19,24 @@ <!-- The operator canvas's title row, rebuilt here to the same measurements: same height, same rule underneath, same full-bleed width, and the view switch pinned to - the same right inset ahead of the same user icon. The name is shown here; making it - editable (with the same rename/save the canvas gives) is the next PR. --> + the same right inset ahead of the same user icon. The content below is padded and + centred, which is why the switch cannot live down there -- it would stop at the + column edge and never line up with the other view. --> <div class="pc-topbar"> <nz-avatar *ngIf="wid" class="wid" [nzText]="wid.toString()"></nz-avatar> - <span class="wf-name">{{ workflowName || "Untitled workflow" }}</span> + <!-- An input, not a label: the workflow can be renamed from either view, with the same + control in the same place. --> + <input + class="wf-name" + [(ngModel)]="workflowName" + (change)="onRenameWorkflow()" + placeholder="Untitled workflow" /> + <!-- Editing here writes to the same workflow, so it earns the same reassurance the + operator canvas gives: the moment it was last saved. --> + <span class="saved-state">{{ autoSaveState }}</span> <span class="topbar-spacer"></span> <div class="view-switch"> <button diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss index 29960a1972..c166254975 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss @@ -22,6 +22,7 @@ $blue: #1890ff; $text: rgba(0, 0, 0, 0.85); $text-2: rgba(0, 0, 0, 0.45); $divider: #f0f0f0; +$shell: #fafafa; :host { display: block; @@ -64,16 +65,32 @@ $divider: #f0f0f0; padding; matching that is what lines the two titles up. */ gap: 0; - /* Every value here is the operator canvas's .workflow-name, so the two title rows read - as the same row: the same size, weight and padding. Shown here; the editable input is - the next PR. */ + /* An input, not a label: the workflow can be renamed from either view. Every value here is + the operator canvas's .workflow-name, so the two title rows read as the same row -- + content-box sizing with the same padding, no border, chromeless until you reach for it, + and a width set from the text by the same measure-in-a-span trick. */ .wf-name { min-width: 100px; max-width: 800px; font-size: 18px; font-weight: 400; padding: 2px 8px; + border: none; + box-sizing: content-box; + background: none; color: $text; + outline: none; + + &:hover { + background: $shell; + } + } + + /* Immediately after the name, in the body colour and size of ordinary text, exactly as on + the operator canvas; the name field's own padding makes the gap. */ + .saved-state { + color: $text; + white-space: nowrap; } .topbar-spacer { diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts index 76732c814d..ebd55cf3f0 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts @@ -18,7 +18,7 @@ */ import { Router } from "@angular/router"; -import { throwError } from "rxjs"; +import { of, throwError } from "rxjs"; import { WorkflowFormComponent } from "./workflow-form.component"; import { setupHarness, formViewWorkflow } from "./workflow-form.spec-harness"; @@ -26,10 +26,11 @@ import { USER_WORKFLOW, USER_WORKSPACE } from "../../../app-routing.constant"; import { DefaultView } from "../../../dashboard/type/workflow-metadata.interface"; /** - * These exercise the shell's own decisions -- what a reader is shown and where an ordinary - * workflow is sent -- without standing up the JointJS canvas. The component is built directly - * (not through TestBed) with the shared spec harness's mocks; the title bar, save and preview - * are added, with their own tests, by later PRs. + * These exercise the page's own decisions -- what a reader is shown, where an ordinary + * workflow is sent, and how the title bar renames and saves -- without standing up the JointJS + * canvas. The component is built directly (not through TestBed) with the shared spec harness's + * mocks; the read-only preview, inputs, running and results are added, with their own tests, by + * later PRs. */ describe("WorkflowFormComponent", () => { let component: WorkflowFormComponent; @@ -54,6 +55,8 @@ describe("WorkflowFormComponent", () => { h.cdr as any, h.computingUnitStatusService as any, h.workflowConsoleService as any, + h.host as any, + h.datePipe as any, h.config as any ); return component; @@ -145,4 +148,197 @@ describe("WorkflowFormComponent", () => { expect(h.workflowResultService.clearResults).toHaveBeenCalled(); }); }); + + describe("title bar and saving", () => { + const enableSave = () => { + h.userService.isLogin.mockReturnValue(true); + h.workflowPersistService.isWorkflowPersistEnabled.mockReturnValue(true); + }; + + it("shows the last-saved time from the workflow's metadata", () => { + build(formViewWorkflow).ngOnInit(); + + expect(component.autoSaveState).toBe("Saved at 01/01/2026 00:00:00"); + }); + + it("shows no saved state when the workflow has never been saved", () => { + workflowActionService.getWorkflowMetadata = () => ({ name: "x", lastModifiedTime: undefined }); + + build(formViewWorkflow).ngOnInit(); + + expect(component.autoSaveState).toBe(""); + }); + + it("renames through the workflow action service and saves", () => { + enableSave(); + build(formViewWorkflow).ngOnInit(); + component.workflowName = "New name"; + + component.onRenameWorkflow(); + + expect(workflowActionService.setWorkflowName).toHaveBeenCalledWith("New name"); + expect(workflowPersistService.persistWorkflow).toHaveBeenCalled(); + }); + + // The title bar is refreshed from one place: a rename or save -- here or by a co-editor -- + // updates the shown name and the saved-at state, so the two views never drift apart. This + // is also where onRenameWorkflow's normalised name is read back. + it("follows the workflow metadata: refreshes the name and saved state when it changes", () => { + vi.useFakeTimers(); + build(formViewWorkflow).ngOnInit(); + component.workflowName = "stale"; + workflowActionService.getWorkflowMetadata = () => ({ name: "Renamed", lastModifiedTime: 1767225600000 }); + + h.workflowMetaDataChangedStream.next(undefined); + vi.runAllTimers(); + + expect(component.workflowName).toBe("Renamed"); + expect(component.autoSaveState).toBe("Saved at 01/01/2026 00:00:00"); + vi.useRealTimers(); + }); + + it("persists the workflow, filling in a position for every operator", () => { + enableSave(); + workflowActionService.getWorkflow.mockReturnValue({ + wid: 7, + content: { + operators: [{ operatorID: "op-1" }, { operatorID: "op-2" }], + operatorPositions: { "op-1": { x: 5, y: 6 } }, + }, + }); + build(formViewWorkflow).ngOnInit(); + + (component as any).save(); + + const saved = workflowPersistService.persistWorkflow.mock.calls.at(-1)[0]; + expect(saved.content.operatorPositions).toEqual({ "op-1": { x: 5, y: 6 }, "op-2": { x: 0, y: 0 } }); + }); + + // The graph is read-only here, but a co-editor can still move operators on the canvas; a save + // must carry those live positions, not revert them to where they sat when this page opened. + it("saves the live positions, not the load-time snapshot", () => { + enableSave(); + build({ ...formViewWorkflow, content: { operatorPositions: { "op-1": { x: 1, y: 1 } } } }).ngOnInit(); + // a co-editor has since dragged op-1; the shared graph reflects the new spot + workflowActionService.getWorkflow.mockReturnValue({ + wid: 7, + content: { operators: [{ operatorID: "op-1" }], operatorPositions: { "op-1": { x: 9, y: 9 } } }, + }); + + (component as any).save(); + + const saved = workflowPersistService.persistWorkflow.mock.calls.at(-1)[0]; + expect(saved.content.operatorPositions).toEqual({ "op-1": { x: 9, y: 9 } }); + }); + + // The canvas advances "Saved at ..." by feeding the persist response back into the metadata; + // the form must do the same, or the saved-at state never moves past the moment it opened. + it("feeds the persist response back into the workflow metadata", () => { + enableSave(); + build(formViewWorkflow).ngOnInit(); + const updated = { wid: 7, name: "scGPT", lastModifiedTime: 999, content: {} }; + workflowPersistService.persistWorkflow.mockReturnValue(of(updated)); + + (component as any).save(); + + expect(workflowActionService.setWorkflowMetadata).toHaveBeenCalledWith(updated); + }); + + it("does not save when the user is not logged in", () => { + build(formViewWorkflow).ngOnInit(); + workflowPersistService.persistWorkflow.mockClear(); + + (component as any).save(); + + expect(workflowPersistService.persistWorkflow).not.toHaveBeenCalled(); + }); + + it("does not save when persistence is disabled", () => { + h.userService.isLogin.mockReturnValue(true); + build(formViewWorkflow).ngOnInit(); + workflowPersistService.persistWorkflow.mockClear(); + + (component as any).save(); + + expect(workflowPersistService.persistWorkflow).not.toHaveBeenCalled(); + }); + + it("does not save a workflow that is not the one this page opened", () => { + enableSave(); + workflowActionService.getWorkflow.mockReturnValue({ wid: 99, content: { operators: [], operatorPositions: {} } }); + build(formViewWorkflow).ngOnInit(); + workflowPersistService.persistWorkflow.mockClear(); + + (component as any).save(); + + expect(workflowPersistService.persistWorkflow).not.toHaveBeenCalled(); + }); + + it("reports a failed save so a lost edit is not silent", () => { + enableSave(); + build(formViewWorkflow).ngOnInit(); + // set after build(): build()'s useWorkflow() resets the persist mock + workflowPersistService.persistWorkflow.mockReturnValue(throwError(() => new Error("no"))); + + (component as any).save(); + + expect(h.notificationService.error).toHaveBeenCalled(); + }); + + it("saves on any workflow change, debounced", () => { + vi.useFakeTimers(); + enableSave(); + build(formViewWorkflow).ngOnInit(); + workflowPersistService.persistWorkflow.mockClear(); + + h.workflowChangedStream.next(undefined); + vi.runAllTimers(); + + expect(workflowPersistService.persistWorkflow).toHaveBeenCalled(); + vi.useRealTimers(); + }); + + it("saves before handing over to the operator canvas", () => { + enableSave(); + build(formViewWorkflow).ngOnInit(); + workflowPersistService.persistWorkflow.mockClear(); + + component.openRegularCanvas(); + + expect(workflowPersistService.persistWorkflow).toHaveBeenCalled(); + }); + + it("saves once more on the way out", () => { + enableSave(); + build(formViewWorkflow).ngOnInit(); + workflowPersistService.persistWorkflow.mockClear(); + + component.ngOnDestroy(); + + expect(workflowPersistService.persistWorkflow).toHaveBeenCalled(); + }); + + it("measures the name field after load, and no-ops when it is not in the DOM", () => { + vi.useFakeTimers(); + const query = vi.spyOn(h.host.nativeElement, "querySelector"); + build(formViewWorkflow).ngOnInit(); + + vi.runAllTimers(); + + expect(query).toHaveBeenCalledWith("input.wf-name"); + vi.useRealTimers(); + }); + + it("stops a deferred name measurement once the page is gone", () => { + vi.useFakeTimers(); + build(formViewWorkflow).ngOnInit(); + const query = vi.spyOn(h.host.nativeElement, "querySelector"); + component.ngOnDestroy(); + + vi.runAllTimers(); + + expect(query).not.toHaveBeenCalled(); + vi.useRealTimers(); + }); + }); }); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts index 3d8f2b8f8c..c1db653f03 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts @@ -17,15 +17,18 @@ * under the License. */ -import { ChangeDetectorRef, Component, HostListener, OnDestroy, OnInit } from "@angular/core"; -import { CommonModule } from "@angular/common"; +import { ChangeDetectorRef, Component, ElementRef, HostListener, OnDestroy, OnInit } from "@angular/core"; +import { CommonModule, DatePipe } from "@angular/common"; +import { FormsModule } from "@angular/forms"; import { ActivatedRoute, Router } from "@angular/router"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; import { NzAvatarModule } from "ng-zorro-antd/avatar"; import { UserIconComponent } from "../../../dashboard/component/user/user-icon/user-icon.component"; import { forkJoin } from "rxjs"; +import { debounceTime } from "rxjs/operators"; import { USER_WORKFLOW, USER_WORKSPACE } from "../../../app-routing.constant"; +import { Workflow, WorkflowContent } from "../../../common/type/workflow"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; import { WorkflowPersistService } from "../../../common/service/workflow-persist/workflow-persist.service"; import { NotificationService } from "../../../common/service/notification/notification.service"; @@ -36,27 +39,42 @@ import { WorkflowActionService } from "../../service/workflow-graph/model/workfl import { GuiConfigService } from "../../../common/service/gui-config.service"; import { WorkflowConsoleService } from "../../service/workflow-console/workflow-console.service"; import { WorkflowResultService } from "../../service/workflow-result/workflow-result.service"; +import { Point } from "../../types/workflow-common.interface"; import { CoeditorUserIconComponent } from "../menu/coeditor-user-icon/coeditor-user-icon.component"; import { CoeditorPresenceService } from "../../service/workflow-graph/model/coeditor-presence.service"; +import { SAVE_DEBOUNCE_TIME_IN_MS } from "../workspace.component"; /** - * The Form View: a second way to use a workflow. This PR lays down the page shell -- behind - * the feature flag it loads the workflow the URL names, shows it read-only, and hands back to - * the operator canvas. The title bar's rename/save, the read-only preview, the inputs, running - * and results are added on top by later PRs. A view, not a new object: it opens the same - * workflow the canvas does. + * The Form View: a second way to use a workflow. Building on the page shell, this PR adds the + * title bar -- the workflow name (renamable, exactly as on the operator canvas), its "Saved + * at ..." state, and the debounced save that both views share so an edit in one view is not + * lost in the other. The read-only preview, the inputs, running and results are added by later + * PRs. A view, not a new object: it opens the same workflow the canvas does. */ @UntilDestroy() @Component({ selector: "texera-workflow-form", templateUrl: "./workflow-form.component.html", styleUrls: ["./workflow-form.component.scss"], - imports: [CommonModule, NzAvatarModule, UserIconComponent, CoeditorUserIconComponent], + imports: [CommonModule, FormsModule, NzAvatarModule, UserIconComponent, CoeditorUserIconComponent], }) export class WorkflowFormComponent implements OnInit, OnDestroy { public wid?: number; public workflowName = ""; public loading = true; + /** "Saved at …", worded and formatted exactly as on the operator canvas. */ + public autoSaveState = ""; + + /** Set on teardown so deferred callbacks stop touching a view that is gone. */ + private destroyed = false; + + /** + * Operator positions as loaded, kept only as a fallback: a save writes the live positions + * from the shared model (a co-editor's drags included), and falls back to this snapshot, then + * origin, if the live map is ever missing an operator -- so a save never drops an operator's + * position, and never overwrites a co-editor's move with a stale one. + */ + private storedPositions: { [operatorID: string]: Point } = {}; constructor( // Public for the template: shows the same live collaborator avatars as the canvas. @@ -73,6 +91,8 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { private cdr: ChangeDetectorRef, private computingUnitStatusService: ComputingUnitStatusService, private workflowConsoleService: WorkflowConsoleService, + private host: ElementRef<HTMLElement>, + private datePipe: DatePipe, private config: GuiConfigService ) {} @@ -108,11 +128,16 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { // a per-workflow switch -- and bounce a later PR's canvas-to-form switch straight // back for any canvas-default workflow. this.workflowName = workflow.name; + this.storedPositions = { ...(workflow.content?.operatorPositions ?? {}) }; this.workflowActionService.setNewSharedModel(wid, this.userService.getCurrentUser()); this.workflowActionService.reloadWorkflow(workflow); // The workflow is shown, not edited, from here: dragging operators around or // deleting them belongs to the operator canvas. this.applyEditability(); + this.refreshSavedState(); + this.later(() => this.adjustWorkflowNameWidth(), 0); + this.registerMetadataRefresh(); + this.registerAutoPersist(); this.loading = false; this.cdr.detectChanges(); }, @@ -133,6 +158,70 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { this.workflowActionService.disableWorkflowModification(); } + /** + * Size the name field to its text, the way the operator canvas does, so what follows + * it starts at the same place in both views instead of after a fixed-width box. + */ + private adjustWorkflowNameWidth(): void { + const input = this.host.nativeElement.querySelector<HTMLInputElement>("input.wf-name"); + if (!input) { + return; + } + /* v8 ignore start -- font-metrics DOM measuring; jsdom has no layout */ + const probe = document.createElement("span"); + probe.style.visibility = "hidden"; + probe.style.position = "absolute"; + probe.style.whiteSpace = "pre"; + probe.style.font = getComputedStyle(input).font; + probe.textContent = input.value || input.placeholder; + document.body.appendChild(probe); + input.style.width = `${Math.min(probe.offsetWidth + 20, 800)}px`; + document.body.removeChild(probe); + /* v8 ignore stop */ + } + + private refreshSavedState(): void { + const lastModified = this.workflowActionService.getWorkflowMetadata()?.lastModifiedTime; + this.autoSaveState = + lastModified === undefined + ? "" + : "Saved at " + + (this.datePipe.transform( + lastModified, + "MM/dd/yyyy HH:mm:ss", + Intl.DateTimeFormat().resolvedOptions().timeZone, + "en" + ) ?? ""); + } + + /** + * Renaming here is the same edit as renaming on the operator canvas: commit the name and + * save. The title bar itself -- the read-back (normalised) name and its width -- is + * refreshed from the metadata subscription below, the single place this page's own rename + * and a co-editor's both flow through. + */ + public onRenameWorkflow(): void { + this.workflowActionService.setWorkflowName(this.workflowName); + this.save(); + } + + /** + * Keep the title bar in step with the workflow's metadata, exactly as the operator canvas + * does: a rename or a save -- this page's own or a co-editor's -- refreshes the name, its + * width, and the "Saved at ..." state from one place, so the two views never drift apart. + */ + private registerMetadataRefresh(): void { + this.workflowActionService + .workflowMetaDataChanged() + // The same 100ms the operator canvas debounces its title-bar refresh by. + .pipe(debounceTime(100), untilDestroyed(this)) + .subscribe(() => { + this.workflowName = this.workflowActionService.getWorkflowMetadata()?.name ?? ""; + this.later(() => this.adjustWorkflowNameWidth(), 0); + this.refreshSavedState(); + }); + } + /** * Switch to the operator canvas with a full page load, not a route. The two views share * root-level singletons (the graph, the Yjs shared model, the CU connection); handing @@ -140,18 +229,97 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { * of yourself, broken runs. A fresh document is the reliable handover. */ public openRegularCanvas(): void { + this.save(); /* v8 ignore start -- full-document navigation; jsdom cannot navigate */ window.location.href = `${USER_WORKSPACE}/${this.wid}`; /* v8 ignore stop */ } + /** + * Save the same way the operator canvas does. Both views edit one workflow, so the + * form has to write through the same debounced persist -- otherwise an author's + * setup, or a value someone filled in, would be gone on the next visit. + */ + private registerAutoPersist(): void { + this.workflowActionService + .workflowChanged() + .pipe(debounceTime(SAVE_DEBOUNCE_TIME_IN_MS), untilDestroyed(this)) + .subscribe(() => this.save()); + } + + /** + * Save the workflow this page opened, and only that one. The persist endpoint creates a + * workflow when the payload has no id, so saving whatever the graph holds would spawn + * stray "Untitled workflow" rows when the page is left before its workflow loaded. + */ + private save(): void { + if (!this.userService.isLogin() || !this.workflowPersistService.isWorkflowPersistEnabled()) { + return; + } + const workflow = this.workflowActionService.getWorkflow(); + if (workflow.wid === undefined || workflow.wid !== this.wid) { + return; + } + const preserved: Workflow = { + ...workflow, + content: { ...workflow.content, operatorPositions: this.positionsToSave(workflow.content) }, + }; + // On the way out the subscription must NOT be tied to this component: ngOnDestroy + // calls save(), and untilDestroyed would tear the subscription down as part of the + // very same destroy sequence, aborting the request that was the point of the call. + const persist = this.workflowPersistService.persistWorkflow(preserved); + // The `destroyed` branch deliberately omits untilDestroyed (see above); the persist call + // is a one-shot HTTP request that completes on its own, so it needs no teardown operator. + // eslint-disable-next-line rxjs-angular/prefer-takeuntil + (this.destroyed ? persist : persist.pipe(untilDestroyed(this))).subscribe({ + // Feed the saved workflow back, exactly as the operator canvas does: this advances + // lastModifiedTime (and the normalised name), and the metadata subscription then repaints + // the title bar -- so "Saved at ..." moves past the moment the page opened. + next: updatedWorkflow => this.workflowActionService.setWorkflowMetadata(updatedWorkflow), + // A save that fails silently is the worst thing this page can do: the author walks + // away believing the form they just built is stored. + error: () => this.notificationService.error("Could not save. Your latest changes are not stored yet."), + }); + } + + /** + * A position for every operator: the live one from the shared model (what a co-editor's drag + * has set), else the load-time snapshot, else origin. Preferring live saves what is current + * rather than overwriting moves with a stale copy; the fallbacks keep the guarantee that every + * operator has a position, since loading throws on one that does not. + */ + private positionsToSave(content: WorkflowContent): { [operatorID: string]: Point } { + const positions: { [operatorID: string]: Point } = {}; + for (const operator of content.operators) { + positions[operator.operatorID] = content.operatorPositions?.[operator.operatorID] ?? + this.storedPositions[operator.operatorID] ?? { x: 0, y: 0 }; + } + return positions; + } + + /** + * Run after a short delay, unless the page is gone by then: the callback touches the view, + * and detectChanges on a destroyed view throws -- reachable by navigating away while the + * name field is still waiting to be measured. + */ + private later(fn: () => void, delayMs = 0): void { + setTimeout(() => { + if (!this.destroyed) { + fn(); + } + }, delayMs); + } + /** * Tear down exactly what the operator canvas tears down: both views drive the same * singleton services, so anything left bound here follows the user to the next page * (the symptom was a frozen canvas after a visit -- the old shared model still attached). + * On the way out, save once more so a last edit is not lost. */ @HostListener("window:beforeunload") ngOnDestroy(): void { + this.destroyed = true; + this.save(); this.workflowActionService.clearWorkflow(); this.computingUnitStatusService.disconnect(); this.executeWorkflowService.resetExecutionAndWorkers(); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts index 90285492d4..af7ba64f07 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts @@ -17,9 +17,10 @@ * under the License. */ +import { DatePipe } from "@angular/common"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { ActivatedRoute, Router } from "@angular/router"; -import { of, Subject } from "rxjs"; +import { EMPTY, of, Subject } from "rxjs"; import { WorkflowFormComponent } from "./workflow-form.component"; import { UserIconComponent } from "../../../dashboard/component/user/user-icon/user-icon.component"; @@ -78,17 +79,29 @@ describe("WorkflowFormComponent (rendered template)", () => { disableWorkflowModification: vi.fn(), clearWorkflow: vi.fn(), getWorkflowMetadata: () => ({ name: "scGPT", lastModifiedTime: undefined }), + getWorkflow: () => ({ wid: 7, content: { operators: [], operatorPositions: {} } }), + setWorkflowName: vi.fn(), + workflowChanged: () => EMPTY, + workflowMetaDataChanged: () => EMPTY, + }, + }, + { + provide: WorkflowPersistService, + useValue: { + retrieveWorkflow: () => workflow$, + isWorkflowPersistEnabled: () => false, + persistWorkflow: () => of({}), }, }, - { provide: WorkflowPersistService, useValue: { retrieveWorkflow: () => workflow$ } }, { provide: OperatorMetadataService, useValue: { getOperatorMetadata: () => of({}) } }, { provide: ExecuteWorkflowService, useValue: { resetExecutionAndWorkers: vi.fn() } }, { provide: WorkflowResultService, useValue: { clearResults: vi.fn() } }, { provide: NotificationService, useValue: { error: vi.fn() } }, - { provide: UserService, useValue: { getCurrentUser: () => undefined } }, + { provide: UserService, useValue: { getCurrentUser: () => undefined, isLogin: () => false } }, { provide: ComputingUnitStatusService, useValue: { disconnect: vi.fn() } }, { provide: WorkflowConsoleService, useValue: { clearConsoleMessages: vi.fn() } }, { provide: GuiConfigService, useValue: { env: { formViewEnabled: true } } }, + DatePipe, ], }).compileComponents(); fixture = TestBed.createComponent(WorkflowFormComponent); @@ -103,13 +116,30 @@ describe("WorkflowFormComponent (rendered template)", () => { beforeEach(configure); - it("renders the workflow's avatar and name in the title row", () => { + it("renders the workflow's avatar and name in the title row", async () => { fixture.detectChanges(); // ngOnInit -> load() finishLoad(); + // ngModel writes the name into the input on a microtask; let it flush before reading. + await fixture.whenStable(); + fixture.detectChanges(); expect(el(".pc-topbar")).not.toBeNull(); expect(el("nz-avatar.wid")).not.toBeNull(); - expect(el(".wf-name")?.textContent?.trim()).toBe("scGPT"); + // The name is an editable input on this slice; its value is the workflow name. + expect((el("input.wf-name") as HTMLInputElement | null)?.value).toBe("scGPT"); + }); + + it("renames the workflow when the name input fires a change", async () => { + fixture.detectChanges(); + finishLoad(); + await fixture.whenStable(); + const spy = vi.spyOn(fixture.componentInstance, "onRenameWorkflow"); + const input = el("input.wf-name") as HTMLInputElement; + + input.value = "Renamed"; + input.dispatchEvent(new Event("change")); + + expect(spy).toHaveBeenCalled(); }); it("switches to the operator canvas when the Canvas control is clicked", () => { diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts index b047361535..2fc33a6a19 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts @@ -17,7 +17,7 @@ * under the License. */ -import { of } from "rxjs"; +import { of, Subject } from "rxjs"; import { vi } from "vitest"; import { DefaultView } from "../../../dashboard/type/workflow-metadata.interface"; @@ -34,6 +34,8 @@ export const formViewWorkflow = { name: "scGPT", defaultView: DefaultView.FORM, */ export function setupHarness() { const router = { navigate: vi.fn() }; + const workflowChangedStream = new Subject<unknown>(); + const workflowMetaDataChangedStream = new Subject<unknown>(); const workflowActionService = { resetAsNewWorkflow: vi.fn(), @@ -42,9 +44,18 @@ export function setupHarness() { enableWorkflowModification: vi.fn(), disableWorkflowModification: vi.fn(), clearWorkflow: vi.fn(), + workflowChanged: () => workflowChangedStream.asObservable(), + workflowMetaDataChanged: () => workflowMetaDataChangedStream.asObservable(), + getWorkflow: vi.fn().mockReturnValue({ wid: 7, content: { operators: [], operatorPositions: {} } }), + getWorkflowMetadata: () => ({ name: "scGPT", lastModifiedTime: 1767225600000 }), + setWorkflowName: vi.fn(), + setWorkflowMetadata: vi.fn(), }; const workflowPersistService = { retrieveWorkflow: vi.fn().mockReturnValue(of(formViewWorkflow)), + // Off by default so opening a workflow does not save; the save tests turn it on. + isWorkflowPersistEnabled: vi.fn().mockReturnValue(false), + persistWorkflow: vi.fn().mockReturnValue(of(formViewWorkflow)), }; const coeditorPresenceService = { coeditors: [] }; const route = { snapshot: { params: { id: "7" } } }; @@ -52,16 +63,22 @@ export function setupHarness() { const executeWorkflowService = { resetExecutionAndWorkers: vi.fn() }; const workflowResultService = { clearResults: vi.fn() }; const notificationService = { error: vi.fn() }; - const userService = { getCurrentUser: () => undefined }; + // Not logged in by default so opening a workflow does not save; the save tests log in. + const userService = { getCurrentUser: () => undefined, isLogin: vi.fn().mockReturnValue(false) }; const cdr = { detectChanges: vi.fn() }; const computingUnitStatusService = { disconnect: vi.fn() }; const workflowConsoleService = { clearConsoleMessages: vi.fn() }; + // The name field is measured off the host; querySelector returns null so the measuring + // (DOM-layout, jsdom has none) short-circuits. + const host = { nativeElement: { querySelector: () => null } }; + const datePipe = { transform: () => "01/01/2026 00:00:00" }; const config = { env: { formViewEnabled: true } }; // Point the persist mock at `workflow`; each spec supplies the remaining constructor // arguments in its own order via the named mocks above. const useWorkflow = (workflow: any) => { workflowPersistService.retrieveWorkflow.mockReturnValue(of(workflow)); + workflowPersistService.persistWorkflow.mockReturnValue(of(workflow)); }; return { @@ -79,6 +96,10 @@ export function setupHarness() { cdr, computingUnitStatusService, workflowConsoleService, + host, + datePipe, config, + workflowChangedStream, + workflowMetaDataChangedStream, }; }
