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-8456-7190a8113ca9ec3ae30d7a1dfdbde4c288034e2c in repository https://gitbox.apache.org/repos/asf/texera.git
commit 3c085d652ad8898561a048cb9b6d8def4f7e7d39 Author: yangzhang75 <[email protected]> AuthorDate: Sun Sep 13 23:34:34 2026 +0000 feat(gui): wire the Form View entry points (#8456) ### What changes were proposed in this PR? Closes #8028. Part of the Form View stack (parent issue #8011), on main now that #8516 and #8517 have merged. The review commit is the branch's single commit. Wires the Form View entry points. The flag stays off here; the stack's closing PR, #8528, flips it. - The dashboard opens a workflow in its `default_view` (form or canvas), with a toggle that persists the choice; the deep link goes to the existing `/workflow/:id/form` route. Both renderers of the dashboard, the list row and the card, follow one shared rule (`default-view-landing.ts`: mark, deep link, toggle), so switching the view mode does not lose the entry point. The toggle is offered only with WRITE access, which the endpoint requires, and the handler checks the same rule rather than trusting the template; it is a proper toggle button (constant accessible name, state in `aria-pressed`, the hover title spelling out what a click does); hub links are left untouched. - The canvas menu gains the same Canvas / Form View switch the form already shows, so the two views swap in place. It saves first and hands over only once the save has completed: the switch is a full-page load, which aborts a request still in flight. Two more things the hand-over must not lose: an autosave already in flight when the switch is clicked (`WorkflowPersistService` now sends saves one at a time and in call order, at the one place every save goes through, so the switch's save lands and completes after it; each caller still gets only its own result and a failed save does not hold up the next), and an edit made while the switch's save is out (the page stays editable until the load; `workflowChanged` marks it and the hand-over saves once more before leaving). A reader, who cannot save, goes straight over. A workflow the canvas holds but has never saved (the default id) is created by that save, and the hand-over opens the id the save answered with. The dashboard's hover-revealed controls (the card's cover controls, the row's action group) also appear while a control inside them has the keyboard focus, so the toggle can be reached without a pointer. A second click while the hand-over is in progress is a no-op. A failed save keeps the user on the canvas with the error shown. Every workflow offers both views whenever the flag is on: `default_view` only decides the landing view, and neither view gates the other. - Download/upload round-trips `defaultView` as a sibling key next to the workflow content, in one shared export shape (`exportedWorkflow`) used by the dashboard download and the canvas menu's export alike; an old export without the key imports unchanged. - The computing unit the user picks is remembered per workflow (localStorage) so it survives switching between the two views; a unit selected on load (the remembered one, the last execution's, a running one) is derived rather than chosen and is not stored, or a derived unit would later outrank a fresher last execution. On load the remembered unit is honoured only once the unit list has arrived and still holds it: a unit that has since been terminated is forgotten and the last execution's unit is used instead, and a decision still pending when the workflow changes underneath it is dropped (the remembered-unit check, the last-execution lookup and its running-unit fallback alike). ### Any related issues, documentation, discussions? Closes #8028. Part of the Form View feature (parent issue #8011). ### How was this PR tested? Unit tests (vitest) cover the menu's Canvas / Form View switch through the DOM (absent with the flag off, Canvas pressed, Form View handing over, hidden while an older version is displayed), the row's and the card's default-view behavior (mark and deep link, hub link untouched, flag off leaves the dashboard as today, WRITE-only toggle in the DOM, toggle on / off / failed request / no cached row), the menu switch (navigates only once the save completes, stays on the canvas with the error when it fails, saves once more when an edit lands while its save is out, takes a reader straight over without a save, ignores a second click mid hand-over), the canvas export carrying `defaultView` next to the content and omitting it when unset, the hand-over opening the id the save assigned when the canvas held a never-saved workflow, the persist service sending saves one at a time in order with each caller getting its own result and a failure not holding up the next, the dashboard toggle handlers refusing without WRITE access, the toggle's aria-pressed following the state, the export/import round-trip including a legacy file without `defaultView`, and the computing-unit recall (waits for the first non-empty unit list, forgets a terminated unit and falls back, drops a stale decision after the workflow changed, a late last-execution answer or fallback included, positive-integer validation, storage failures, only an explicit pick remembered). Each new guard was deletion-checked (removing it turns the corresponding test red). eslint, prettier and the production (AOT) build pass; every changed line, template lines included, is statement and function covered. #### Video ##### 1. Default view on the dashboard (row and card toggle, Form View icon, deep link into the form, toggle off again, no toggle without write access) https://github.com/user-attachments/assets/04f71ba5-27ad-489d-a5be-51cdd8d811aa ##### 2. Canvas / Form View switch (save first, then the hand-over; and back) https://github.com/user-attachments/assets/39c3d395-6329-403d-9719-ca7ef179889f ##### 3. Computing unit remembered across the switch https://github.com/user-attachments/assets/6fe70b5f-d936-4333-bcf0-b3fa44f84323 ##### 4. Download / upload keeps the default view https://github.com/user-attachments/assets/0519bfcd-0677-478f-866c-6eb0350d842c ### Was this PR authored or co-authored using generative AI tooling? Yes. Generated-by: Claude Code (Claude Fable 5.1, Anthropic). Co-authored with Claude, reviewed line by line by the author before submission. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY Co-authored-by: Claude Fable 5.1 <[email protected]> --- .../workflow-persist.service.spec.ts | 57 +++++ .../workflow-persist/workflow-persist.service.ts | 50 ++++- frontend/src/app/common/type/workflow.ts | 16 +- .../list-item/card-item/card-item.component.html | 25 ++- .../list-item/card-item/card-item.component.scss | 15 +- .../card-item/card-item.component.spec.ts | 204 +++++++++++++++++- .../list-item/card-item/card-item.component.ts | 59 ++++- .../user/list-item/default-view-landing.ts | 42 ++++ .../user/list-item/list-item.component.html | 23 +- .../user/list-item/list-item.component.scss | 22 +- .../user/list-item/list-item.component.spec.ts | 146 +++++++++++++ .../user/list-item/list-item.component.ts | 60 +++++- .../user-workflow/user-workflow.component.spec.ts | 21 +- .../user/user-workflow/user-workflow.component.ts | 8 +- .../service/user/download/download.service.spec.ts | 30 +++ .../service/user/download/download.service.ts | 8 +- .../workspace/component/menu/menu.component.html | 24 +++ .../workspace/component/menu/menu.component.scss | 62 ++++++ .../component/menu/menu.component.spec.ts | 182 ++++++++++++++++ .../app/workspace/component/menu/menu.component.ts | 101 ++++++++- .../computing-unit-selection.component.html | 2 +- .../computing-unit-selection.component.spec.ts | 237 +++++++++++++++++++++ .../computing-unit-selection.component.ts | 145 +++++++++++-- 23 files changed, 1493 insertions(+), 46 deletions(-) diff --git a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts index 1ae30d331c..a35c9ec106 100644 --- a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts +++ b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts @@ -221,6 +221,45 @@ describe("WorkflowPersistService", () => { expect(result?.isPublished).toBe(1); }); + it("sends saves one at a time, in order, each caller getting its own result", () => { + // Two saves in flight at once can land out of order and the older content would win; the + // autosave and a Save or a view switch are independent callers, so the ordering lives here. + const wf = (name: string) => ({ wid: 9, name, description: "", content: validContent }) as unknown as Workflow; + const seen: string[] = []; + service.persistWorkflow(wf("first")).subscribe(w => seen.push("first:" + w.name)); + service.persistWorkflow(wf("second")).subscribe(w => seen.push("second:" + w.name)); + + // Only the first request has gone out; the second waits for it. + const first = httpTestingController.expectOne(`${API}/${WORKFLOW_PERSIST_URL}`); + expect(first.request.body.name).toBe("first"); + expect(httpTestingController.match(`${API}/${WORKFLOW_PERSIST_URL}`)).toHaveLength(0); + + first.flush({ wid: 9, name: "first", content: "{}" }); + const second = httpTestingController.expectOne(`${API}/${WORKFLOW_PERSIST_URL}`); + expect(second.request.body.name).toBe("second"); + second.flush({ wid: 9, name: "second", content: "{}" }); + + expect(seen).toEqual(["first:first", "second:second"]); + }); + + it("fails only its own caller when a save fails, and still sends the next", () => { + const wf = (name: string) => ({ wid: 9, name, description: "", content: validContent }) as unknown as Workflow; + let firstError: unknown; + let secondName: string | undefined; + service.persistWorkflow(wf("first")).subscribe({ error: (e: unknown) => (firstError = e) }); + service.persistWorkflow(wf("second")).subscribe(w => (secondName = w.name)); + + httpTestingController + .expectOne(`${API}/${WORKFLOW_PERSIST_URL}`) + .flush("boom", { status: 500, statusText: "Server Error" }); + expect(firstError).toBeDefined(); + + httpTestingController + .expectOne(`${API}/${WORKFLOW_PERSIST_URL}`) + .flush({ wid: 9, name: "second", content: "{}" }); + expect(secondName).toBe("second"); + }); + it("persistWorkflow notifies the user when the workflow is broken but still POSTs", () => { const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); const workflow = { @@ -274,6 +313,24 @@ describe("WorkflowPersistService", () => { expect(result).toEqual(created); }); + it("createWorkflow sends the default view when given one, and omits it otherwise", () => { + const content = jsonCast<WorkflowContent>(testContent); + + service.createWorkflow(content, "form default", DefaultView.FORM).subscribe(); + const withView = httpTestingController.expectOne(`${API}/${WORKFLOW_CREATE_URL}`); + expect(withView.request.body).toEqual({ + name: "form default", + content: JSON.stringify(content), + defaultView: DefaultView.FORM, + }); + withView.flush({ workflow: { wid: 1 } } as unknown as DashboardWorkflow); + + service.createWorkflow(content, "no view").subscribe(); + const withoutView = httpTestingController.expectOne(`${API}/${WORKFLOW_CREATE_URL}`); + expect(withoutView.request.body).toEqual({ name: "no view", content: JSON.stringify(content) }); + withoutView.flush({ workflow: { wid: 2 } } as unknown as DashboardWorkflow); + }); + it("createWorkflow filters out a null response so no value is emitted", () => { let emitted = false; service.createWorkflow(jsonCast<WorkflowContent>(testContent)).subscribe(() => (emitted = true)); diff --git a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts index 9b8f4741bd..66d267a673 100644 --- a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts +++ b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts @@ -19,8 +19,8 @@ import { HttpClient, HttpParams } from "@angular/common/http"; import { Injectable } from "@angular/core"; -import { Observable, throwError } from "rxjs"; -import { catchError, filter, map } from "rxjs/operators"; +import { EMPTY, Observable, ReplaySubject, Subject, throwError } from "rxjs"; +import { catchError, concatMap, filter, map, tap } from "rxjs/operators"; import { AppSettings } from "../../app-setting"; import { Workflow, WorkflowContent } from "../../type/workflow"; import { DashboardWorkflow } from "../../../dashboard/type/dashboard-workflow.interface"; @@ -59,13 +59,42 @@ export class WorkflowPersistService { // flag to disable workflow persist when displaying the read only particular version private workflowPersistFlag = true; + /** + * Saves, one at a time and in call order. Two saves in flight at once can reach the backend out + * of order, and then the older content wins: the canvas's autosave (debounced) and a Save or a + * view switch are independent requests, and the Form View's own queue only orders that page's + * saves. Ordering them here, at the one place every save goes through, covers all of them and + * lets a caller that hands over on completion (the view switches) know that everything asked for + * before it has landed too. Each request snapshots its payload when asked for; it is sent when its + * turn comes, and its outcome is relayed to that caller alone. A failed save fails its own caller + * and does not hold up the next. + */ + private readonly persistQueue = new Subject<{ send: Observable<Workflow>; result: Subject<Workflow> }>(); + constructor( private http: HttpClient, private notificationService: NotificationService - ) {} + ) { + this.persistQueue + .pipe( + concatMap(({ send, result }) => + send.pipe( + tap({ + next: updated => result.next(updated), + error: (err: unknown) => result.error(err), + complete: () => result.complete(), + }), + catchError(() => EMPTY) + ) + ) + ) + .subscribe(); + } /** - * persists a workflow to backend database and returns its updated information (e.g., new wid) + * persists a workflow to backend database and returns its updated information (e.g., new wid). + * The request is queued behind any save still in flight (see persistQueue); the returned + * observable completes once this save has come back. * @param workflow */ public persistWorkflow(workflow: Workflow): Observable<Workflow> { @@ -79,7 +108,7 @@ export class WorkflowPersistService { // backend does not read it on this endpoint (publishing goes through /public and /private), // and it is not reliably known here anyway, since the metadata fed back after a save names // it differently (see WorkflowUtilService.parseWorkflowInfo). - return this.http + const send = this.http .post<Workflow>(`${AppSettings.getApiEndpoint()}/${WORKFLOW_PERSIST_URL}`, { wid: workflow.wid, name: workflow.name, @@ -90,6 +119,11 @@ export class WorkflowPersistService { filter((updatedWorkflow: Workflow) => updatedWorkflow != null), map(WorkflowUtilService.parseWorkflowInfo) ); + // Replayed, so a caller that subscribes after the queue has already relayed the outcome (a + // save that was quick, or a synchronous test double) still receives it. + const result = new ReplaySubject<Workflow>(1); + this.persistQueue.next({ send, result }); + return result.asObservable(); } /** @@ -99,12 +133,16 @@ export class WorkflowPersistService { */ public createWorkflow( newWorkflowContent: WorkflowContent, - newWorkflowName: string = DEFAULT_WORKFLOW_NAME + newWorkflowName: string = DEFAULT_WORKFLOW_NAME, + defaultView?: DefaultView ): Observable<DashboardWorkflow> { return this.http .post<DashboardWorkflow>(`${AppSettings.getApiEndpoint()}/${WORKFLOW_CREATE_URL}`, { name: newWorkflowName, content: JSON.stringify(newWorkflowContent), + // Bound onto the workflow row on the server, so an uploaded form-default workflow + // still opens as a form. Omitted (server default CANVAS) when the file carries none. + ...(defaultView === undefined ? {} : { defaultView }), }) .pipe(filter((createdWorkflow: DashboardWorkflow) => createdWorkflow != null)); } diff --git a/frontend/src/app/common/type/workflow.ts b/frontend/src/app/common/type/workflow.ts index 4129346b33..701086e2a7 100644 --- a/frontend/src/app/common/type/workflow.ts +++ b/frontend/src/app/common/type/workflow.ts @@ -17,7 +17,7 @@ * under the License. */ -import { WorkflowMetadata } from "../../dashboard/type/workflow-metadata.interface"; +import { DefaultView, WorkflowMetadata } from "../../dashboard/type/workflow-metadata.interface"; import { CommentBox, OperatorLink, OperatorPredicate, Point } from "../../workspace/types/workflow-common.interface"; export enum ExecutionMode { @@ -104,3 +104,17 @@ export interface WorkflowContent }> {} export type Workflow = { content: WorkflowContent } & WorkflowMetadata; + +/** + * The JSON a workflow is exported as, from the dashboard download and the canvas menu alike: the + * content plus, when the workflow has one, the landing view as one extra top-level key next to the + * content's own (operators/links/...). The importer (upload) destructures it back out onto the + * workflow row, so a download-then-upload keeps a form-default workflow opening as a form; an + * older importer that reads the whole object as content simply ignores the unknown key, and an + * older export without it imports unchanged. + */ +export type ExportedWorkflow = WorkflowContent & { defaultView?: DefaultView }; + +export function exportedWorkflow(content: WorkflowContent, defaultView: DefaultView | undefined): ExportedWorkflow { + return defaultView === undefined ? content : { ...content, defaultView }; +} diff --git a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.html b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.html index 4cd50dc8de..883b3cae69 100644 --- a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.html +++ b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.html @@ -36,12 +36,31 @@ [(ngModel)]="entry.checked" (ngModelChange)="onCheckboxChange(entry)"></label> </div> - <!-- Cover-image controls --> + <!-- Cover controls: the owner's image controls, and the default-view toggle for anyone with + write access (the same toggle the list row offers, so switching to the card view does not + lose it). --> <div class="card-image-controls" - *ngIf="canEditCover" + *ngIf="hasCoverControls" (click)="$event.stopPropagation()"> + <!-- A toggle button: constant name, state in aria-pressed (a name that changed with the state + would announce the opposite of what the state says); the title spells out what a click does. --> <button + *ngIf="canToggleDefaultView" + nz-button + nzType="text" + class="image-control-btn default-view-toggle" + aria-label="Open in the Form View by default" + [attr.aria-pressed]="defaultsToForm" + [class.defaults-to-form-on]="defaultsToForm" + [title]="defaultsToForm ? 'Open on the canvas by default' : 'Open in the Form View by default'" + (click)="onToggleDefaultView()"> + <i + nz-icon + nzType="solution"></i> + </button> + <button + *ngIf="canEditCover" nz-button nzType="text" class="image-control-btn" @@ -52,7 +71,7 @@ nzType="camera"></i> </button> <button - *ngIf="hasCustomImage" + *ngIf="canEditCover && hasCustomImage" nz-button nzType="text" class="image-control-btn" diff --git a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.scss b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.scss index 559e7a70c2..1754434307 100644 --- a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.scss +++ b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.scss @@ -80,7 +80,10 @@ transition: opacity 0.2s; } -.card-item:hover .card-image-controls { +// Revealed on hover, and while a control inside has the keyboard focus: the buttons are always +// in the tab order, so a keyboard user must be able to see the one they have reached. +.card-item:hover .card-image-controls, +.card-item:focus-within .card-image-controls { opacity: 1; } @@ -96,6 +99,16 @@ color: #fff; background: rgba(0, 0, 0, 0.55); } + + /* The default-view toggle shows its state: filled with the accent while the workflow opens + in the Form View, the same cue the list row's toggle gives. */ + &.default-view-toggle.defaults-to-form-on { + background: #1890ff; + + &:hover { + background: #40a9ff; + } + } } .owner-info { diff --git a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts index 1b22b370a0..404d345934 100644 --- a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts @@ -43,6 +43,8 @@ import { } from "../../../../../app-routing.constant"; import { WorkflowCoverService } from "src/app/dashboard/service/user/workflow-cover/workflow-cover.service"; import { NotificationService } from "../../../../../common/service/notification/notification.service"; +import { GuiConfigService } from "../../../../../common/service/gui-config.service"; +import { DefaultView } from "src/app/dashboard/type/workflow-metadata.interface"; import { DatasetService, DEFAULT_DATASET_NAME } from "../../../../service/user/dataset/dataset.service"; import { DownloadService } from "src/app/dashboard/service/user/download/download.service"; @@ -86,7 +88,11 @@ describe("CardItemComponent", () => { let datasetService: Mocked<DatasetService>; beforeEach(async () => { - const workflowPersistServiceSpy = { updateWorkflowName: vi.fn(), updateWorkflowDescription: vi.fn() }; + const workflowPersistServiceSpy = { + updateWorkflowName: vi.fn(), + updateWorkflowDescription: vi.fn(), + setDefaultView: vi.fn(), + }; const workflowCoverServiceSpy = { getCover: vi.fn().mockReturnValue(of(undefined)), setCoverFromFile: vi.fn(), @@ -1165,4 +1171,200 @@ describe("CardItemComponent", () => { expect(getCountsSpy).not.toHaveBeenCalled(); }); }); + + /** + * The card is the second renderer of the same dashboard entries as the list row, so it follows + * the same default-view rule (default-view-landing.ts): mark, deep-link and toggle, all gated on + * the Form View flag. + */ + describe("default view", () => { + const enableFormView = () => + (TestBed.inject(GuiConfigService) as unknown as { setConfig: (c: object) => void }).setConfig({ + formViewEnabled: true, + }); + const formEntry = (defaultView: DefaultView | undefined, accessLevel = "WRITE", accessibleUserIds = [42]) => + makeWorkflowEntry({ + id: 7, + accessibleUserIds, + accessLevel, + workflow: { isOwner: true, workflow: defaultView === undefined ? undefined : { defaultView } }, + } as any); + + beforeEach(() => { + component.currentUid = 42; + component.isPrivateSearch = true; + (workflowPersistService as any).setDefaultView = vi.fn().mockReturnValue(of(undefined)); + }); + + it("opens a form-default workflow in its form and marks it with the Form View icon", () => { + enableFormView(); + component.entry = formEntry(DefaultView.FORM); + component.initializeEntry(); + + expect(component.defaultsToForm).toBe(true); + expect(component.iconType).toBe("solution"); + expect(component.entryLink).toEqual([USER_WORKSPACE, "7", "form"]); + }); + + it("leaves a canvas-default workflow as the descriptor set it", () => { + enableFormView(); + component.entry = formEntry(DefaultView.CANVAS); + component.initializeEntry(); + + expect(component.defaultsToForm).toBe(false); + expect(component.iconType).toBe("project"); + expect(component.entryLink).toEqual([USER_WORKSPACE, "7"]); + }); + + // Behind the flag the dashboard must look exactly as it does today. + it("ignores default_view entirely while the flag is off", () => { + component.entry = formEntry(DefaultView.FORM); + component.initializeEntry(); + + expect(component.defaultsToForm).toBe(false); + expect(component.iconType).toBe("project"); + expect(component.entryLink).toEqual([USER_WORKSPACE, "7"]); + expect(component.canToggleDefaultView).toBe(false); + }); + + // The mark is the owner's entry point; a hub visitor still lands on the detail page. + it("marks a form-default hub card but does not repoint its link", () => { + enableFormView(); + component.entry = formEntry(DefaultView.FORM, "READ", [99]); + component.initializeEntry(); + + expect(component.defaultsToForm).toBe(true); + expect(component.iconType).toBe("solution"); + expect(component.entryLink).toEqual([HUB_WORKFLOW_RESULT_DETAIL, "7"]); + }); + + it("offers the toggle only in private search, for a workflow the user can write", () => { + enableFormView(); + component.entry = formEntry(DefaultView.CANVAS, "WRITE"); + expect(component.canToggleDefaultView).toBe(true); + + component.entry = formEntry(DefaultView.CANVAS, "READ"); + expect(component.canToggleDefaultView).toBe(false); + + component.entry = formEntry(DefaultView.CANVAS, "WRITE"); + component.isPrivateSearch = false; + expect(component.canToggleDefaultView).toBe(false); + + component.isPrivateSearch = true; + component.entry = makeDatasetEntry({ accessLevel: "WRITE" } as any); + expect(component.canToggleDefaultView).toBe(false); + }); + + // A collaborator with write access but no ownership has no image controls, yet still gets the + // toggle, so the hover controls must show for them too. + it("shows the cover controls for a writer who does not own the card", () => { + enableFormView(); + component.entry = makeWorkflowEntry({ + id: 7, + accessibleUserIds: [42], + accessLevel: "WRITE", + workflow: { isOwner: false }, + } as any); + component.initializeEntry(); + + expect(component.canEditCover).toBe(false); + expect(component.canToggleDefaultView).toBe(true); + expect(component.hasCoverControls).toBe(true); + }); + + it("turns the default view on, marking the card and repointing it", () => { + enableFormView(); + component.entry = formEntry(DefaultView.CANVAS); + component.initializeEntry(); + + component.onToggleDefaultView(); + + expect(workflowPersistService.setDefaultView).toHaveBeenCalledWith(7, DefaultView.FORM); + expect(component.entry.workflow?.workflow?.defaultView).toBe(DefaultView.FORM); + expect(component.defaultsToForm).toBe(true); + expect(component.iconType).toBe("solution"); + expect(component.entryLink).toEqual([USER_WORKSPACE, "7", "form"]); + }); + + it("turns it back off, restoring the plain card", () => { + enableFormView(); + component.entry = formEntry(DefaultView.FORM); + component.initializeEntry(); + + component.onToggleDefaultView(); + + expect(workflowPersistService.setDefaultView).toHaveBeenCalledWith(7, DefaultView.CANVAS); + expect(component.defaultsToForm).toBe(false); + expect(component.iconType).toBe("project"); + expect(component.entryLink).toEqual([USER_WORKSPACE, "7"]); + }); + + // A failed call must not leave the card claiming a state the server never took. + it("keeps the previous state and reports it when the request fails", () => { + enableFormView(); + const notify = vi.spyOn(TestBed.inject(NotificationService), "error").mockImplementation(() => {}); + component.entry = formEntry(DefaultView.CANVAS); + component.initializeEntry(); + (workflowPersistService as any).setDefaultView = vi.fn().mockReturnValue(throwError(() => new Error("nope"))); + + component.onToggleDefaultView(); + + expect(component.defaultsToForm).toBe(false); + expect(component.iconType).toBe("project"); + expect(component.entryLink).toEqual([USER_WORKSPACE, "7"]); + expect(notify).toHaveBeenCalledWith("nope"); + }); + + // A card without the cached workflow row must still record the toggle, not crash. + it("still persists the toggle when the entry has no cached workflow row", () => { + enableFormView(); + component.entry = formEntry(undefined); + component.initializeEntry(); + + component.onToggleDefaultView(); + + expect(workflowPersistService.setDefaultView).toHaveBeenCalledWith(7, DefaultView.FORM); + expect(component.defaultsToForm).toBe(false); + }); + + // The permission lives in the method, not only in the button's *ngIf: setting the default view + // writes the workflow row, which needs WRITE access whoever calls. + it("refuses to toggle for a collaborator without write access", () => { + enableFormView(); + component.entry = formEntry(DefaultView.CANVAS, "READ"); + component.initializeEntry(); + + component.onToggleDefaultView(); + + expect(workflowPersistService.setDefaultView).not.toHaveBeenCalled(); + }); + + it("renders the toggle for a writer and routes its click, but not for a reader", () => { + enableFormView(); + const toggle = vi.spyOn(component, "onToggleDefaultView").mockImplementation(() => {}); + component.entry = formEntry(DefaultView.CANVAS, "WRITE"); + component.ngOnChanges({ entry: {} as any }); + fixture.detectChanges(); + + const button = fixture.debugElement.query(By.css("button.default-view-toggle")); + expect(button).not.toBeNull(); + // A toggle: constant name, state in aria-pressed. + expect(button.nativeElement.getAttribute("aria-label")).toBe("Open in the Form View by default"); + expect(button.nativeElement.getAttribute("aria-pressed")).toBe("false"); + button.triggerEventHandler("click", new MouseEvent("click")); + expect(toggle).toHaveBeenCalledTimes(1); + + component.entry = formEntry(DefaultView.FORM, "WRITE"); + component.ngOnChanges({ entry: {} as any }); + fixture.detectChanges(); + expect( + fixture.debugElement.query(By.css("button.default-view-toggle")).nativeElement.getAttribute("aria-pressed") + ).toBe("true"); + + component.entry = formEntry(DefaultView.CANVAS, "READ"); + component.ngOnChanges({ entry: {} as any }); + fixture.detectChanges(); + expect(fixture.debugElement.query(By.css("button.default-view-toggle"))).toBeNull(); + }); + }); }); diff --git a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts index 03cf31147e..e8542799e4 100644 --- a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts +++ b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts @@ -52,6 +52,10 @@ import { extractErrorMessage } from "../../../../../common/util/error"; import { WorkflowCoverService } from "../../../../service/user/workflow-cover/workflow-cover.service"; import { isDefined } from "../../../../../common/util/predicate"; import { ResourceRegistryService } from "../../../../service/user/resource-registry/resource-registry.service"; +import { GuiConfigService } from "../../../../../common/service/gui-config.service"; +import { WorkflowPersistService } from "../../../../../common/service/workflow-persist/workflow-persist.service"; +import { DefaultView } from "../../../../type/workflow-metadata.interface"; +import { defaultsToFormView, landingLink } from "../default-view-landing"; @UntilDestroy() @Component({ @@ -126,7 +130,9 @@ export class CardItemComponent implements OnChanges { private cdr: ChangeDetectorRef, private notificationService: NotificationService, private workflowCoverService: WorkflowCoverService, - private resourceRegistry: ResourceRegistryService + private resourceRegistry: ResourceRegistryService, + private workflowPersistService: WorkflowPersistService, + private config: GuiConfigService ) {} get hasCustomImage(): boolean { @@ -138,6 +144,50 @@ export class CardItemComponent implements OnChanges { return this.isPrivateSearch && this.entry.type === "workflow" && this.entry.workflow.isOwner; } + /** Whether this card is a form-default workflow (Form View icon, deep-links into the form). */ + public defaultsToForm = false; + + /** + * Whether the default-view toggle is offered: the same rule as the list row. Setting the default + * view writes the workflow row, so it needs WRITE access, not just ownership of the card. + */ + get canToggleDefaultView(): boolean { + return ( + this.isPrivateSearch && + this.entry.type === "workflow" && + this.config.env.formViewEnabled && + this.entry.accessLevel === "WRITE" + ); + } + + /** The hover controls over the cover: image controls for the owner, the default-view toggle for a writer. */ + get hasCoverControls(): boolean { + return this.canEditCover || this.canToggleDefaultView; + } + + public onToggleDefaultView(): void { + // The rule the button is gated on, checked here too: the write needs WRITE access whoever calls. + if (!this.canToggleDefaultView) { + return; + } + const next = this.defaultsToForm ? DefaultView.CANVAS : DefaultView.FORM; + this.workflowPersistService + .setDefaultView(this.entry.id as number, next) + .pipe(untilDestroyed(this)) + .subscribe({ + next: () => { + if (this.entry.workflow?.workflow) { + this.entry.workflow.workflow.defaultView = next; + } + // Re-derive from the descriptor so the icon and link fully reset when turning it + // off, not just when turning it on. + this.initializeEntry(); + this.cdr.detectChanges(); + }, + error: (err: unknown) => this.notificationService.error(extractErrorMessage(err)), + }); + } + openImagePicker(): void { this.backgroundInput?.nativeElement.click(); } @@ -191,6 +241,13 @@ export class CardItemComponent implements OnChanges { this.canDownload = descriptor.download !== undefined; this.canShare = descriptor.retrieveOwners !== undefined; this.entryLink = this.resourceRegistry.entryLink(this.entry, this.currentUid); + // Same landing rule as the list row (default-view-landing.ts): a form-default workflow shows + // the Form View icon and opens in its form, so the card and the row never disagree. + this.defaultsToForm = defaultsToFormView(this.entry, this.config.env.formViewEnabled); + if (this.defaultsToForm) { + this.iconType = "solution"; + } + this.entryLink = landingLink(this.entry, this.entryLink, this.config.env.formViewEnabled); if (descriptor.hasSize && typeof this.entry.id === "number") { this.size = this.entry.size; } diff --git a/frontend/src/app/dashboard/component/user/list-item/default-view-landing.ts b/frontend/src/app/dashboard/component/user/list-item/default-view-landing.ts new file mode 100644 index 0000000000..568fe77082 --- /dev/null +++ b/frontend/src/app/dashboard/component/user/list-item/default-view-landing.ts @@ -0,0 +1,42 @@ +/** + * 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 { USER_WORKSPACE } from "../../../../app-routing.constant"; +import { DashboardEntry } from "../../../type/dashboard-entry"; +import { DefaultView } from "../../../type/workflow-metadata.interface"; + +/** + * Where a dashboard entry lands when opened, and whether it is a form-default workflow. Shared by + * the list row and the card so the two renderers of the same dashboard agree: with the Form View + * flag on, a workflow whose default_view is FORM is marked as such and, when its link points into + * the user's own workspace, deep-links straight into its form (the operator canvas stays one click + * away from there). Every other link, hub links included, is left exactly as the descriptor built it. + */ +export function defaultsToFormView(entry: DashboardEntry, formViewEnabled: boolean): boolean { + return formViewEnabled && entry.type === "workflow" && entry.workflow?.workflow?.defaultView === DefaultView.FORM; +} + +export function landingLink(entry: DashboardEntry, descriptorLink: string[], formViewEnabled: boolean): string[] { + if (!formViewEnabled || entry.type !== "workflow" || descriptorLink[0] !== USER_WORKSPACE) { + return descriptorLink; + } + return defaultsToFormView(entry, formViewEnabled) + ? [USER_WORKSPACE, String(entry.id), "form"] + : [USER_WORKSPACE, String(entry.id)]; +} diff --git a/frontend/src/app/dashboard/component/user/list-item/list-item.component.html b/frontend/src/app/dashboard/component/user/list-item/list-item.component.html index 7e67b6438e..205cbe6c8d 100644 --- a/frontend/src/app/dashboard/component/user/list-item/list-item.component.html +++ b/frontend/src/app/dashboard/component/user/list-item/list-item.component.html @@ -45,7 +45,8 @@ <div nz-col nzFlex="0" - class="type-icon"> + class="type-icon" + [class.defaults-to-form]="defaultsToForm"> <i nz-icon [nzType]="iconType"></i> @@ -194,6 +195,26 @@ nz-icon nzType="eye"></i> </button> + <!-- Setting the default view is a write on the workflow row (the endpoint requires WRITE), so a + read-only collaborator is not offered a control that could only fail (the handler checks + the same rule, canToggleDefaultView). --> + <!-- A toggle button: the name stays constant and aria-pressed carries the state (a name that + changed with the state would announce the opposite of what the state says); the title spells + out what a click does. --> + <button + *ngIf="canToggleDefaultView" + nz-button + nzType="text" + class="default-view-toggle" + aria-label="Open in the Form View by default" + [attr.aria-pressed]="defaultsToForm" + [title]="defaultsToForm ? 'Open on the canvas by default' : 'Open in the Form View by default'" + [class.defaults-to-form-on]="defaultsToForm" + (click)="onToggleDefaultView()"> + <i + nz-icon + nzType="solution"></i> + </button> <button nz-button nzType="text" diff --git a/frontend/src/app/dashboard/component/user/list-item/list-item.component.scss b/frontend/src/app/dashboard/component/user/list-item/list-item.component.scss index 97cec2e60c..82ad7c2e56 100644 --- a/frontend/src/app/dashboard/component/user/list-item/list-item.component.scss +++ b/frontend/src/app/dashboard/component/user/list-item/list-item.component.scss @@ -120,7 +120,11 @@ } } -.list-item-card:hover .button-group { +// Shown on hover, and while anything in the row has the keyboard focus: the group is display:none +// otherwise, so a keyboard user reaches it by tabbing into the row (its checkbox, rename, like), +// which reveals the actions and lets the next Tab move into them. +.list-item-card:hover .button-group, +.list-item-card:focus-within .button-group { display: flex; background-color: transparent; } @@ -180,6 +184,22 @@ font-size: 30px; } +/* A form-default workflow carries a little colour so the row reads at a glance, + without shouting: same flask, tinted, in the row icon and in the hover action. */ +.type-icon.defaults-to-form { + color: #1e90ff; +} + +.button-group button.defaults-to-form-on { + background-color: #e6f7ff; + border-color: #1e90ff; + color: #1e90ff; +} + +.button-group button.defaults-to-form-on:hover { + background-color: #cceeff; +} + .workflow-id { padding: 6px; } diff --git a/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts b/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts index e520343ce4..ce4a2310ef 100644 --- a/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts @@ -34,8 +34,10 @@ import { RouterTestingModule } from "@angular/router/testing"; import { StubUserService } from "../../../../common/service/user/stub-user.service"; import { UserService } from "../../../../common/service/user/user.service"; import { commonTestProviders } from "../../../../common/testing/test-utils"; +import { GuiConfigService } from "../../../../common/service/gui-config.service"; import type { Mocked } from "vitest"; import { DashboardEntry } from "src/app/dashboard/type/dashboard-entry"; +import { DefaultView } from "../../../type/workflow-metadata.interface"; import { DatasetService, DEFAULT_DATASET_NAME } from "../../../service/user/dataset/dataset.service"; import { NotificationService } from "../../../../common/service/notification/notification.service"; import { @@ -74,6 +76,12 @@ describe("ListItemComponent", () => { datasetService = TestBed.inject(DatasetService) as unknown as Mocked<DatasetService>; hubService = TestBed.inject(HubService); modalService = TestBed.inject(NzModalService); + // The Form View entry points (deep-link, solution icon, toggle) are gated on the + // form-view-enabled flag, which the stack's closing PR turns on. The shared config mock + // defaults it off (matching the production default), so enable it here for the form-default cases. + (TestBed.inject(GuiConfigService) as unknown as { setConfig: (c: object) => void }).setConfig({ + formViewEnabled: true, + }); // initializeEntry() needs a fully-formed workflow entry to avoid throwing // when the template renders for the first time. Each test below overwrites // component.entry directly, which exercises confirm methods without going @@ -143,6 +151,94 @@ describe("ListItemComponent", () => { expect(component.editingDescription).toBe(false); }); + describe("Form View toggle", () => { + const formEntry = (defaultView: DefaultView, accessLevel = "WRITE") => + ({ + id: 42, + type: "workflow", + workflow: { isOwner: true, workflow: { defaultView } }, + accessibleUserIds: [1], + accessLevel, + likeCount: 0, + viewCount: 0, + isLiked: false, + }) as unknown as DashboardEntry; + + beforeEach(() => { + component.currentUid = 1; + (workflowPersistService as any).setDefaultView = vi.fn().mockReturnValue(of(undefined)); + }); + + it("turns it on, showing the flask and repointing the row", () => { + component.entry = formEntry(DefaultView.CANVAS); + component.initializeEntry(); + expect(component.iconType).toBe("project"); + + component.onToggleDefaultView(); + + expect(workflowPersistService.setDefaultView).toHaveBeenCalledWith(42, DefaultView.FORM); + expect(component.defaultsToForm).toBe(true); + expect(component.iconType).toBe("solution"); + expect(component.entryLink).toEqual([USER_WORKSPACE, "42", "form"]); + }); + + it("turns it back off, restoring the plain row", () => { + component.entry = formEntry(DefaultView.FORM); + component.initializeEntry(); + + component.onToggleDefaultView(); + + expect(workflowPersistService.setDefaultView).toHaveBeenCalledWith(42, DefaultView.CANVAS); + expect(component.defaultsToForm).toBe(false); + expect(component.iconType).toBe("project"); + expect(component.entryLink).toEqual([USER_WORKSPACE, "42"]); + }); + + // A failed call must not leave the row claiming a state the server never took. + it("keeps the previous state when the request fails", () => { + component.entry = formEntry(DefaultView.CANVAS); + component.initializeEntry(); + (workflowPersistService as any).setDefaultView = vi.fn().mockReturnValue(throwError(() => new Error("nope"))); + + component.onToggleDefaultView(); + + expect(component.defaultsToForm).toBe(false); + expect(component.iconType).toBe("project"); + }); + + // A card without the cached workflow row must still record the toggle, not crash. + it("still persists the toggle when the entry has no cached workflow row", () => { + component.entry = { + id: 42, + type: "workflow", + workflow: { isOwner: true }, + accessibleUserIds: [1], + accessLevel: "WRITE", + likeCount: 0, + viewCount: 0, + isLiked: false, + } as unknown as DashboardEntry; + component.initializeEntry(); + + component.onToggleDefaultView(); + + expect(workflowPersistService.setDefaultView).toHaveBeenCalledWith(42, DefaultView.FORM); + expect(component.defaultsToForm).toBe(false); + }); + + // The permission lives in the method, not only in the button's *ngIf: setting the default view + // writes the workflow row, which needs WRITE access whoever calls. + it("refuses to toggle for a collaborator without write access", () => { + component.entry = formEntry(DefaultView.CANVAS, "READ"); + component.initializeEntry(); + + component.onToggleDefaultView(); + + expect(component.canToggleDefaultView).toBe(false); + expect(workflowPersistService.setDefaultView).not.toHaveBeenCalled(); + }); + }); + describe("initializeEntry routes", () => { const baseStats = { likeCount: 0, viewCount: 0, isLiked: false }; @@ -159,6 +255,38 @@ describe("ListItemComponent", () => { expect(component.entryLink).toEqual([USER_WORKSPACE, "100"]); }); + it("sends an owned form-default workflow straight to its form", () => { + component.currentUid = 1; + component.entry = { + id: 100, + type: "workflow", + workflow: { isOwner: true, workflow: { defaultView: DefaultView.FORM } }, + accessibleUserIds: [1], + ...baseStats, + } as unknown as DashboardEntry; + component.initializeEntry(); + + expect(component.entryLink).toEqual([USER_WORKSPACE, "100", "form"]); + expect(component.iconType).toBe("solution"); + expect(component.defaultsToForm).toBe(true); + }); + + // The flask is the owner's entry point; a hub visitor still lands on the detail page. + it("leaves the hub link alone for a form-default workflow the user does not own", () => { + component.currentUid = 1; + component.entry = { + id: 101, + type: "workflow", + workflow: { isOwner: false, workflow: { defaultView: DefaultView.FORM } }, + accessibleUserIds: [2], + ...baseStats, + } as unknown as DashboardEntry; + component.initializeEntry(); + + expect(component.entryLink).toEqual([HUB_WORKFLOW_RESULT_DETAIL, "101"]); + expect(component.iconType).toBe("solution"); + }); + it("routes non-owned workflows to the hub workflow detail page", () => { component.currentUid = 1; component.entry = { @@ -678,6 +806,24 @@ describe("ListItemComponent", () => { expect(edit).toHaveBeenCalledTimes(2); }); + // Setting the default view writes the workflow row, so the control is only offered to a + // collaborator who can write it; a reader would only ever see it fail. + it("offers the default-view toggle only to a collaborator with write access", () => { + const toggle = vi.spyOn(component, "onToggleDefaultView").mockImplementation(() => {}); + render({ accessLevel: "WRITE" }); + + const button = q("button.default-view-toggle"); + expect(button).not.toBeNull(); + // A toggle: constant name, state in aria-pressed. + expect(button.nativeElement.getAttribute("aria-label")).toBe("Open in the Form View by default"); + expect(button.nativeElement.getAttribute("aria-pressed")).toBe("false"); + button.triggerEventHandler("click", new MouseEvent("click")); + expect(toggle).toHaveBeenCalledTimes(1); + + render({ accessLevel: "READ" }); + expect(q("button.default-view-toggle")).toBeNull(); + }); + it("tracks hover over the row", () => { render(); const row = q("div[nz-row]"); diff --git a/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts b/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts index 2226f2e1db..5f35b3d289 100644 --- a/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts +++ b/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts @@ -54,6 +54,10 @@ import { FormsModule } from "@angular/forms"; import { UserAvatarComponent } from "../user-avatar/user-avatar.component"; import { NzWaveDirective } from "ng-zorro-antd/core/wave"; import { NzPopconfirmDirective } from "ng-zorro-antd/popconfirm"; +import { GuiConfigService } from "../../../../common/service/gui-config.service"; +import { DefaultView } from "../../../type/workflow-metadata.interface"; +import { defaultsToFormView, landingLink } from "./default-view-landing"; +import { WorkflowPersistService } from "../../../../common/service/workflow-persist/workflow-persist.service"; @UntilDestroy() @Component({ @@ -96,8 +100,19 @@ export class ListItemComponent implements OnChanges { entryLink: string[] = []; size: number | undefined = 0; public iconType: string = ""; + /** Whether this workflow opens in the Form View by default. */ + public defaultsToForm = false; isLiked: boolean = false; @Input() isPrivateSearch = false; + + /** + * Whether the default-view toggle is offered, and honoured: setting the default view writes the + * workflow row (the endpoint requires WRITE), so a read-only collaborator gets no control that + * could only fail, and the handler itself checks the same rule rather than trusting the template. + */ + get canToggleDefaultView(): boolean { + return this.entry.type === "workflow" && this.config.env.formViewEnabled && this.entry.accessLevel === "WRITE"; + } @Input() editable = false; private _entry?: DashboardEntry; hovering: boolean = false; @@ -125,7 +140,9 @@ export class ListItemComponent implements OnChanges { private hubService: HubService, private cdr: ChangeDetectorRef, private notificationService: NotificationService, - private resourceRegistry: ResourceRegistryService + private resourceRegistry: ResourceRegistryService, + protected config: GuiConfigService, + private workflowPersistService: WorkflowPersistService ) {} initializeEntry() { @@ -138,6 +155,9 @@ export class ListItemComponent implements OnChanges { if (descriptor.hasSize && typeof this.entry.id === "number") { this.size = this.entry.size; } + // A workflow opens in its default view: with the feature flag on, a form-default one shows + // the Form View icon and deep-links straight into the form; a canvas-default one is unchanged. + this.applyDefaultView(); this.likeCount = this.entry.likeCount; this.viewCount = this.entry.viewCount; this.isLiked = this.entry.isLiked; @@ -163,6 +183,44 @@ export class ListItemComponent implements OnChanges { } } + /** + * A workflow opens in its default view. A form-default one is marked with the Form View icon + * and its owner's row deep-links straight into the form; the operator canvas is still one + * click away from there. A canvas-default one is left as the descriptor set it. Hub links are + * untouched; only the owner's own entry point moves. The rule itself lives in + * default-view-landing.ts, shared with the card renderer so both views of the dashboard agree. + */ + private applyDefaultView(): void { + const formViewEnabled = this.config.env.formViewEnabled; + this.defaultsToForm = defaultsToFormView(this.entry, formViewEnabled); + if (this.defaultsToForm) { + this.iconType = "solution"; + } + this.entryLink = landingLink(this.entry, this.entryLink, formViewEnabled); + } + + public onToggleDefaultView(): void { + if (!this.canToggleDefaultView) { + return; + } + const next = this.defaultsToForm ? DefaultView.CANVAS : DefaultView.FORM; + this.workflowPersistService + .setDefaultView(this.entry.id as number, next) + .pipe(untilDestroyed(this)) + .subscribe({ + next: () => { + if (this.entry.workflow?.workflow) { + this.entry.workflow.workflow.defaultView = next; + } + // Re-derive from the descriptor so the icon and link fully reset when turning it + // off, not just when turning it on. + this.initializeEntry(); + this.cdr.detectChanges(); + }, + error: (err: unknown) => this.notificationService.error(extractErrorMessage(err)), + }); + } + onCheckboxChange(entry: DashboardEntry): void { entry.checked = !entry.checked; this.cdr.markForCheck(); diff --git a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts index 6d59455394..0986d60b17 100644 --- a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts @@ -745,12 +745,27 @@ describe("SavedWorkflowSectionComponent", () => { const result = await firstValueFrom(component.onClickUploadExistingWorkflowFromLocal(file as any)); expect(result).toBe(false); - expect(persist.createWorkflow).toHaveBeenCalledWith(content, "wf"); + expect(persist.createWorkflow).toHaveBeenCalledWith(content, "wf", undefined); expect(component.searchResultsComponent.entries.map(e => e.name)).toContain("wf"); expect(searchSpy).toHaveBeenCalledWith(true); expect(successSpy).toHaveBeenCalledWith("Upload Successful"); }); + it("restores a form-default workflow's landing view, keeping it out of the content", async () => { + const persist = TestBed.inject(WorkflowPersistService) as any; + persist.createWorkflow = vi.fn().mockReturnValue(of(makeDashboardWorkflow(43, "form-wf"))); + vi.spyOn(component, "search").mockResolvedValue(undefined); + vi.spyOn(TestBed.inject(NotificationService), "success").mockImplementation(() => undefined as any); + const content = testWorkflowContent([]); + const file = new File([JSON.stringify({ ...content, defaultView: "FORM" })], "form-wf.json"); + setEntries([]); + + await firstValueFrom(component.onClickUploadExistingWorkflowFromLocal(file as any)); + + // defaultView is pulled out and passed on its own; the content stored is unchanged. + expect(persist.createWorkflow).toHaveBeenCalledWith(content, "form-wf", "FORM"); + }); + it("toasts an error and errors the stream when the file is not JSON", async () => { const errorSpy = vi .spyOn(TestBed.inject(NotificationService), "error") @@ -773,7 +788,7 @@ describe("SavedWorkflowSectionComponent", () => { await firstValueFrom(component.onClickUploadExistingWorkflowFromLocal(file as any)); - expect(persist.createWorkflow).toHaveBeenCalledWith(content, DEFAULT_WORKFLOW_NAME); + expect(persist.createWorkflow).toHaveBeenCalledWith(content, DEFAULT_WORKFLOW_NAME, undefined); }); it("imports every workflow file inside an uploaded .zip", async () => { @@ -1105,7 +1120,7 @@ describe("SavedWorkflowSectionComponent", () => { await firstValueFrom(component.onClickUploadExistingWorkflowFromLocal(file as any)); - expect(persist.createWorkflow).toHaveBeenCalledWith(content, "noext"); + expect(persist.createWorkflow).toHaveBeenCalledWith(content, "noext", undefined); }); it("errors the upload stream and does not toast success when createWorkflow fails", async () => { diff --git a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts index 29ebb13afa..5aae5f7943 100644 --- a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts +++ b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts @@ -29,7 +29,7 @@ import { DashboardEntry, UserInfo } from "../../../type/dashboard-entry"; import { UserService } from "../../../../common/service/user/user.service"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; import { NotificationService } from "../../../../common/service/notification/notification.service"; -import { ExecutionMode, WorkflowContent } from "../../../../common/type/workflow"; +import { ExecutionMode, ExportedWorkflow, WorkflowContent } from "../../../../common/type/workflow"; import { NzUploadFile, NzUploadComponent } from "ng-zorro-antd/upload"; import JSZip from "jszip"; import { FiltersComponent } from "../filters/filters.component"; @@ -540,9 +540,11 @@ export class UserWorkflowComponent implements AfterViewInit, OnDestroy { if (typeof result !== "string") { throw new Error("Incorrect format: file is not a string"); } - const workflowContent = JSON.parse(result) as WorkflowContent; + // The landing view rides as a sibling key next to the content (see exportedWorkflow); + // pull it back out so it is stored on the workflow row, not inside content. + const { defaultView, ...workflowContent } = JSON.parse(result) as ExportedWorkflow; this.workflowPersistService - .createWorkflow(workflowContent, this.deriveWorkflowName(name)) + .createWorkflow(workflowContent, this.deriveWorkflowName(name), defaultView) .pipe(untilDestroyed(this)) .subscribe({ next: uploadedWorkflow => { diff --git a/frontend/src/app/dashboard/service/user/download/download.service.spec.ts b/frontend/src/app/dashboard/service/user/download/download.service.spec.ts index b295a16b6d..0d1c3ac734 100644 --- a/frontend/src/app/dashboard/service/user/download/download.service.spec.ts +++ b/frontend/src/app/dashboard/service/user/download/download.service.spec.ts @@ -237,6 +237,36 @@ describe("DownloadService", () => { // produced it. }); + // Blob.text() is missing in jsdom, but FileReader.readAsText works. + const readBlob = (blob: Blob) => + new Promise<string>((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => resolve(reader.result as string); + reader.onerror = () => reject(reader.error); + reader.readAsText(blob); + }); + + it("carries the default view into the downloaded workflow json so it round-trips on upload", async () => { + const content = { operators: [], links: [] }; + workflowPersistServiceSpy.retrieveWorkflow.mockReturnValue(of({ content, defaultView: "FORM" } as any)); + + const result = await firstValueFrom(downloadService.downloadWorkflow(42, "MyWorkflow")); + const parsed = JSON.parse(await readBlob(result.blob)); + + expect(parsed.defaultView).toBe("FORM"); + expect(parsed.operators).toEqual([]); + }); + + it("omits the default view from the json when the workflow has none", async () => { + const content = { operators: [], links: [] }; + workflowPersistServiceSpy.retrieveWorkflow.mockReturnValue(of({ content } as any)); + + const result = await firstValueFrom(downloadService.downloadWorkflow(42, "MyWorkflow")); + const parsed = JSON.parse(await readBlob(result.blob)); + + expect("defaultView" in parsed).toBe(false); + }); + // ─── downloadWorkflowsAsZip ─────────────────────────────────────────────── it("downloads the workflow ZIP and routes through createWorkflowsZip", async () => { diff --git a/frontend/src/app/dashboard/service/user/download/download.service.ts b/frontend/src/app/dashboard/service/user/download/download.service.ts index 65430e5b54..9acdd6ccdb 100644 --- a/frontend/src/app/dashboard/service/user/download/download.service.ts +++ b/frontend/src/app/dashboard/service/user/download/download.service.ts @@ -26,7 +26,7 @@ import { DatasetService } from "../dataset/dataset.service"; import { ModelService } from "../model/model.service"; import { WorkflowPersistService } from "src/app/common/service/workflow-persist/workflow-persist.service"; import JSZip from "jszip"; -import { Workflow } from "../../../../common/type/workflow"; +import { exportedWorkflow, Workflow } from "../../../../common/type/workflow"; import { HttpClient, HttpResponse } from "@angular/common/http"; import { WORKFLOW_EXECUTIONS_API_BASE_URL } from "../workflow-executions/workflow-executions.service"; import { DashboardWorkflowComputingUnit } from "../../../../common/type/workflow-computing-unit"; @@ -315,8 +315,10 @@ export class DownloadService { */ private retrieveWorkflowItem(id: number, name: string): Observable<DownloadableItem> { return this.workflowPersistService.retrieveWorkflow(id).pipe( - map(({ content }) => { - const workflowJson = JSON.stringify(content, null, 2); + map(({ content, defaultView }) => { + // The one export shape (see exportedWorkflow), shared with the canvas menu's export, so + // either file uploads alike. + const workflowJson = JSON.stringify(exportedWorkflow(content, defaultView), null, 2); const fileName = `${name}.json`; const blob = new Blob([workflowJson], { type: "text/plain;charset=utf-8" }); return { blob, fileName }; diff --git a/frontend/src/app/workspace/component/menu/menu.component.html b/frontend/src/app/workspace/component/menu/menu.component.html index cfdeea7710..cf4c053894 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.html +++ b/frontend/src/app/workspace/component/menu/menu.component.html @@ -75,6 +75,30 @@ <span *ngIf="!displayParticularWorkflowVersion"> {{autoSaveState}} </span> </div> + <!-- One workflow, two ways of working on it: the Canvas and the Form View. The + same control sits in the same place on both, so switching never means hunting + for a different affordance; the pressed segment is the view you are looking at. + Every workflow offers both views, so this shows wherever the feature flag is on. --> + <div + class="view-switch" + *ngIf="this.config.env.formViewEnabled && !displayParticularWorkflowVersion"> + <!-- The pressed segment stays a live button with no handler, deliberately: clicking the view + you are already on is a no-op, and a real button keeps the segment focusable and announced + with its pressed state. The Form View page mirrors this exactly, roles swapped. --> + <button + type="button" + class="on" + aria-pressed="true"> + Canvas + </button> + <button + type="button" + aria-pressed="false" + (click)="onClickOpenFormView()"> + Form View + </button> + </div> + <ng-container *ngFor="let user of coeditorPresenceService.coeditors"> <texera-coeditor-user-icon [coeditor]="user"></texera-coeditor-user-icon> </ng-container> diff --git a/frontend/src/app/workspace/component/menu/menu.component.scss b/frontend/src/app/workspace/component/menu/menu.component.scss index deb31c0258..12dafb6a2e 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.scss +++ b/frontend/src/app/workspace/component/menu/menu.component.scss @@ -81,6 +81,7 @@ #user-buttons, #execution-buttons, nz-button-group, +nz-upload, texera-computing-unit-selection { display: inline-flex; align-items: center; @@ -215,3 +216,64 @@ texera-coeditor-user-icon { width: auto; vertical-align: -0.2em; } + +/* One workflow, two ways of working on it. Rendered identically in the operator canvas + and the Form View, in the same slot of the same title row, so the control + never moves when the view does -- that stillness is what makes the two read as two + views of one thing rather than two pages. + + Deliberately quiet: the indicator is a rule sitting on the row's own bottom border, + not a filled button. This is secondary navigation and it shares a screen with Run, + which is the one thing here that should be solid blue. The current view is inert -- + clicking the view you are already in should do nothing. */ +.view-switch { + display: inline-flex; + align-self: stretch; + align-items: stretch; + flex: none; + gap: 20px; + margin-right: 20px; + + button { + appearance: none; + border: 0; + background: none; + cursor: pointer; + font: inherit; + font-size: 13px; + color: rgba(0, 0, 0, 0.45); + padding: 0; + position: relative; + display: inline-flex; + align-items: center; + white-space: nowrap; + transition: color 0.15s; + + /* Sits on the row's bottom rule, so the two read as tabs of the row rather than + as a widget dropped into it. */ + &::after { + content: ""; + position: absolute; + left: -2px; + right: -2px; + bottom: -1px; + height: 2px; + background: transparent; + transition: background 0.15s; + } + + &:hover:not(.on) { + color: rgba(0, 0, 0, 0.85); + } + } + + button.on { + color: rgba(0, 0, 0, 0.85); + font-weight: 500; + cursor: default; + + &::after { + background: #1890ff; + } + } +} diff --git a/frontend/src/app/workspace/component/menu/menu.component.spec.ts b/frontend/src/app/workspace/component/menu/menu.component.spec.ts index da69bf3adf..3e15706cf0 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts @@ -116,6 +116,118 @@ describe("MenuComponent", () => { expect(component).toBeTruthy(); }); + 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; + + component.onClickOpenFormView(); + + expect(window.location.href).toBe(href); + }); + + it("hands over to the id the save assigned when the canvas held a workflow never saved yet", () => { + // After "new workflow" the canvas holds the default workflow (wid 0); the switch's save creates + // it, and the page to open is the created one, not /workflow/0/form. + component.writeAccess = true; + vi.spyOn(component["workflowActionService"], "getWorkflowMetadata").mockReturnValue({ wid: 0 } as any); + vi.spyOn(workflowPersistService, "persistWorkflow").mockReturnValue(of({ wid: 42, name: "created" } as any)); + vi.spyOn(component["workflowActionService"], "setWorkflowMetadata").mockImplementation(() => {}); + const navigate = vi.spyOn(component as any, "openFormViewPage").mockImplementation(() => {}); + + component.onClickOpenFormView(); + + expect(navigate).toHaveBeenCalledWith(42); + }); + + it("saves, then hands over to the Form View only once the save has completed", () => { + component.writeAccess = true; + vi.spyOn(component["workflowActionService"], "getWorkflowMetadata").mockReturnValue({ wid: 7 } as any); + const saved = { wid: 7, name: "saved" } as any; + const persistSpy = vi.spyOn(workflowPersistService, "persistWorkflow").mockReturnValue(of(saved)); + const metadataSpy = vi + .spyOn(component["workflowActionService"], "setWorkflowMetadata") + .mockImplementation(() => {}); + const navigate = vi.spyOn(component as any, "openFormViewPage").mockImplementation(() => {}); + + component.onClickOpenFormView(); + + // The navigation unloads the document and aborts anything still in flight, so it must wait for + // the save's completion rather than be fired right after the request. + expect(persistSpy).toHaveBeenCalled(); + expect(metadataSpy).toHaveBeenCalledWith(saved); + expect(navigate).toHaveBeenCalledWith(7); + expect(component.isSaving).toBe(false); + }); + + it("stays on the canvas and reports the error when the save before the switch fails", () => { + component.writeAccess = true; + vi.spyOn(component["workflowActionService"], "getWorkflowMetadata").mockReturnValue({ wid: 7 } as any); + vi.spyOn(workflowPersistService, "persistWorkflow").mockReturnValue(throwError(() => new Error("nope"))); + const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); + const navigate = vi.spyOn(component as any, "openFormViewPage").mockImplementation(() => {}); + + component.onClickOpenFormView(); + + // Leaving would take the user away from changes that were never stored. + expect(navigate).not.toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalledWith("Could not save. Your latest changes are not stored yet."); + expect(component.isSaving).toBe(false); + }); + + it("saves once more when an edit lands while the switch's save is out, then hands over", () => { + // The page stays editable until the full-page load; an edit made after the click is not in the + // save's snapshot and its own autosave (debounced) would be aborted by the load. workflowChanged + // marks it, and the hand-over saves again before leaving. + const edits = new Subject<unknown>(); + vi.spyOn(component["workflowActionService"], "workflowChanged").mockReturnValue(edits.asObservable()); + component.ngOnInit(); + component.writeAccess = true; + vi.spyOn(component["workflowActionService"], "getWorkflowMetadata").mockReturnValue({ wid: 7 } as any); + vi.spyOn(component["workflowActionService"], "setWorkflowMetadata").mockImplementation(() => {}); + const first$ = new Subject<any>(); + const second$ = new Subject<any>(); + const persistSpy = vi + .spyOn(workflowPersistService, "persistWorkflow") + .mockReturnValueOnce(first$) + .mockReturnValueOnce(second$); + const navigate = vi.spyOn(component as any, "openFormViewPage").mockImplementation(() => {}); + + component.onClickOpenFormView(); + edits.next(undefined); // an edit while the first save is out + first$.complete(); + + expect(persistSpy).toHaveBeenCalledTimes(2); // saved once more + expect(navigate).not.toHaveBeenCalled(); + second$.complete(); + expect(navigate).toHaveBeenCalledWith(7); + expect(component.isSaving).toBe(false); + }); + + it("ignores a second click while the hand-over is already in progress", () => { + component.writeAccess = true; + vi.spyOn(component["workflowActionService"], "getWorkflowMetadata").mockReturnValue({ wid: 7 } as any); + const persistSpy = vi.spyOn(workflowPersistService, "persistWorkflow").mockReturnValue(new Subject<any>()); + vi.spyOn(component as any, "openFormViewPage").mockImplementation(() => {}); + + component.onClickOpenFormView(); + component.onClickOpenFormView(); + + expect(persistSpy).toHaveBeenCalledTimes(1); + }); + + it("takes a reader straight over without a save, which they could not make", () => { + // Every save of a reader's is a 403 that would keep them on the canvas with an error. + component.writeAccess = false; + vi.spyOn(component["workflowActionService"], "getWorkflowMetadata").mockReturnValue({ wid: 7 } as any); + const persistSpy = vi.spyOn(workflowPersistService, "persistWorkflow"); + const navigate = vi.spyOn(component as any, "openFormViewPage").mockImplementation(() => {}); + + component.onClickOpenFormView(); + + expect(persistSpy).not.toHaveBeenCalled(); + expect(navigate).toHaveBeenCalledWith(7); + }); + describe("getRunButtonBehavior", () => { it("returns 'Invalid Workflow' when the workflow is invalid", () => { component.isWorkflowValid = false; @@ -542,6 +654,44 @@ describe("MenuComponent", () => { expect(blobArg).toBeInstanceOf(Blob); expect(blobArg.type).toBe("text/plain;charset=utf-8"); }); + + // Blob.text() is missing in jsdom, but FileReader.readAsText works. + const readBlob = (blob: Blob) => + new Promise<string>((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => resolve(reader.result as string); + reader.onerror = () => reject(reader.error); + reader.readAsText(blob); + }); + + it("carries the workflow's default view next to the content, as the dashboard download does", async () => { + const saveAs = vi.spyOn(TestBed.inject(FileSaverService), "saveAs").mockImplementation(() => {}); + vi.spyOn(workflowActionService, "getWorkflowContent").mockReturnValue({ + operators: [], + links: [], + } as unknown as WorkflowContent); + vi.spyOn(workflowActionService, "getWorkflowMetadata").mockReturnValue({ wid: 7, defaultView: "FORM" } as any); + + component.onClickExportWorkflow(); + + const parsed = JSON.parse(await readBlob(saveAs.mock.calls[0][0] as Blob)); + expect(parsed.defaultView).toBe("FORM"); + expect(parsed.operators).toEqual([]); + }); + + it("leaves the key out when the workflow has no default view", async () => { + const saveAs = vi.spyOn(TestBed.inject(FileSaverService), "saveAs").mockImplementation(() => {}); + vi.spyOn(workflowActionService, "getWorkflowContent").mockReturnValue({ + operators: [], + links: [], + } as unknown as WorkflowContent); + vi.spyOn(workflowActionService, "getWorkflowMetadata").mockReturnValue({ wid: 7 } as any); + + component.onClickExportWorkflow(); + + const parsed = JSON.parse(await readBlob(saveAs.mock.calls[0][0] as Blob)); + expect("defaultView" in parsed).toBe(false); + }); }); describe("version history", () => { @@ -1168,6 +1318,38 @@ describe("MenuComponent", () => { vi.restoreAllMocks(); }); + describe("view switch", () => { + const flag = (formViewEnabled: boolean) => + (TestBed.inject(GuiConfigService) as unknown as MockGuiConfigService).setConfig({ formViewEnabled }); + + it("shows Canvas pressed and hands Form View to onClickOpenFormView, only with the flag on", () => { + flag(false); + fixture.detectChanges(); + expect(q(".view-switch")).toBeNull(); + + flag(true); + fixture.detectChanges(); + const open = vi.spyOn(component, "onClickOpenFormView").mockImplementation(() => {}); + const buttons = fixture.debugElement.queryAll(By.css(".view-switch button")); + expect(buttons.map(b => (b.nativeElement.textContent ?? "").trim())).toEqual(["Canvas", "Form View"]); + // The current view is the pressed segment: announced as such, and a live button on purpose. + expect(buttons[0].nativeElement.getAttribute("aria-pressed")).toBe("true"); + expect(buttons[0].nativeElement.classList.contains("on")).toBe(true); + expect(buttons[1].nativeElement.getAttribute("aria-pressed")).toBe("false"); + buttons[1].triggerEventHandler("click", null); + expect(open).toHaveBeenCalledTimes(1); + }); + + it("hides the switch while an older version is displayed", () => { + flag(true); + component.displayParticularWorkflowVersion = true; + fixture.detectChanges(); + + // A past version has no form to switch to. + expect(q(".view-switch")).toBeNull(); + }); + }); + describe("version display bar", () => { /** Puts the menu into the "viewing an older version" state and renders it. */ function showVersion(versionId: number | null = 7): void { diff --git a/frontend/src/app/workspace/component/menu/menu.component.ts b/frontend/src/app/workspace/component/menu/menu.component.ts index c3ad4aa979..3a46e987f3 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.ts @@ -22,7 +22,7 @@ import { Component, ElementRef, Input, OnDestroy, OnInit, ViewChild } from "@ang import { Router, RouterLink } from "@angular/router"; import { UserService } from "../../../common/service/user/user.service"; import { WorkflowPersistService } from "../../../common/service/workflow-persist/workflow-persist.service"; -import { Workflow, WorkflowContent } from "../../../common/type/workflow"; +import { exportedWorkflow, Workflow, WorkflowContent } from "../../../common/type/workflow"; import { ExecuteWorkflowService } from "../../service/execute-workflow/execute-workflow.service"; import { UndoRedoService } from "../../service/undo-redo/undo-redo.service"; import { ValidationWorkflowService } from "../../service/validation/validation-workflow.service"; @@ -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 } from "../../../app-routing.constant"; +import { USER_WORKFLOW, USER_WORKSPACE } from "../../../app-routing.constant"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; import { ComputingUnitState } from "../../../common/type/computing-unit-connection.interface"; import { ComputingUnitSelectionComponent } from "../power-button/computing-unit-selection.component"; @@ -130,6 +130,10 @@ export class MenuComponent implements OnInit, OnDestroy { public isWorkflowValid: boolean = true; // this will check whether the workflow error or not public isWorkflowEmpty: boolean = false; public isSaving: boolean = false; + /** A Form View hand-over is in progress (saving, then a full-page load); a second click is a no-op. */ + private handingOverToFormView = false; + /** An edit has been reported since the hand-over's last save snapshot (see onClickOpenFormView). */ + private editedSinceSwitchSnapshot = false; public isWorkflowModifiable: boolean = false; public workflowId?: number; public isExportDeactivate: boolean = false; @@ -219,6 +223,13 @@ export class MenuComponent implements OnInit, OnDestroy { } public ngOnInit(): void { + // Marks an edit for the Form View hand-over (see onClickOpenFormView): set the moment an edit is + // reported, before the autosave debounce, cleared when the switch's save snapshots the workflow. + this.workflowActionService + .workflowChanged() + .pipe(untilDestroyed(this)) + .subscribe(() => (this.editedSinceSwitchSnapshot = true)); + this.executeWorkflowService .getExecutionStateStream() .pipe(untilDestroyed(this)) @@ -617,8 +628,13 @@ export class MenuComponent implements OnInit, OnDestroy { } public onClickExportWorkflow(): void { - const workflowContent: WorkflowContent = this.workflowActionService.getWorkflowContent(); - const workflowContentJson = JSON.stringify(workflowContent, null, 2); + // The same shape the dashboard download produces (see exportedWorkflow): the content plus the + // landing view as a sibling key, so a file exported here uploads as a form-default workflow too. + const exported = exportedWorkflow( + this.workflowActionService.getWorkflowContent(), + this.workflowActionService.getWorkflowMetadata().defaultView + ); + const workflowContentJson = JSON.stringify(exported, null, 2); const fileName = this.currentWorkflowName + ".json"; // Through the injectable wrapper (as the dashboard downloads already do), so a spec stubs it // with TestBed instead of module-mocking the CommonJS file-saver package, which the unit-test @@ -626,6 +642,83 @@ export class MenuComponent implements OnInit, OnDestroy { this.fileSaverService.saveAs(new Blob([workflowContentJson], { type: "text/plain;charset=utf-8" }), fileName); } + /** + * Open the Form View -- a full page load, not a route: the two views share root-level + * singletons (graph, Yjs shared model), and routing left the old collaboration client + * alive (you appeared as your own coeditor). A fresh document is the clean handover. + */ + public onClickOpenFormView(): void { + const wid = this.workflowActionService.getWorkflowMetadata().wid; + if (wid === undefined || this.handingOverToFormView) { + return; + } + // A reader has nothing to save, and every save of theirs is a guaranteed 403 that would keep + // them here with an error: straight over, as the form's own switch does for a reader. + if (!this.writeAccess) { + this.openFormViewPage(wid); + return; + } + // Save first, and hand over only once the save has completed. The full-page load that + // follows unloads this document, and a request still in flight at that moment is aborted, so + // navigating right after firing the save could lose the very edit the switch is meant to carry + // across; the workspace's beforeunload save runs into the same unload and is no safety net. A + // save that fails keeps the user here with the error shown, rather than leaving with changes + // that were never stored. The form's own switch (openRegularCanvas) does the same. + // + // Two more things the hand-over must not lose. An autosave already in flight when the switch + // is clicked: WorkflowPersistService sends saves one at a time and in order, so ours lands after + // it and completes after it. And an edit made while our save is out (the page stays editable + // until the load): workflowChanged marks it, and the drain below saves once more before handing + // over rather than letting the full-page load abort that edit's own debounced autosave. + this.handingOverToFormView = true; + this.isSaving = true; + this.saveThenOpenFormView(wid); + } + + private saveThenOpenFormView(wid: number): void { + // The snapshot below carries everything reported up to now. + this.editedSinceSwitchSnapshot = false; + // A workflow the canvas holds but has never saved carries the default id (0); the save creates + // it and answers with the id it was given, which is the one to open -- as the autosave, which + // moves the URL to the answered id, already does. + let target = wid; + this.workflowPersistService + .persistWorkflow(this.workflowActionService.getWorkflow()) + .pipe(untilDestroyed(this)) + .subscribe({ + next: (updatedWorkflow: Workflow) => { + target = updatedWorkflow.wid ?? wid; + this.workflowActionService.setWorkflowMetadata(updatedWorkflow); + }, + error: () => { + this.isSaving = false; + this.handingOverToFormView = false; + // The same wording as the form's own save, so the two switches read alike. + this.notificationService.error("Could not save. Your latest changes are not stored yet."); + }, + complete: () => { + if (this.editedSinceSwitchSnapshot) { + // An edit landed while the save was out; the full-page load would kill its autosave. + this.saveThenOpenFormView(target); + return; + } + this.isSaving = false; + this.openFormViewPage(target); + }, + }); + } + + /** + * 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. + */ + /* v8 ignore start */ + private openFormViewPage(wid: number): void { + window.location.href = `${USER_WORKSPACE}/${wid}/form`; + } + /* v8 ignore stop */ + /** * Calls Markdown Description Component */ diff --git a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html index 6a9a7d9061..066e39c836 100644 --- a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html +++ b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html @@ -113,7 +113,7 @@ 'unit-connecting': unit.status === 'Pending', }" [nz-tooltip]="cannotSelectUnit(unit) ? getUnitStatusTooltip(unit) + '. Cannot select.' : ''" - (click)="selectedComputingUnit = unit; selectComputingUnit(this.workflowId, unit?.computingUnit?.cuid)"> + (click)="onPickComputingUnit(unit)"> <div class="computing-unit-row"> <texera-user-avatar [avatar]="unit.ownerAvatar" diff --git a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts index 3f2b2a06f4..2fca86830e 100644 --- a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts +++ b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts @@ -153,6 +153,12 @@ describe("PowerButtonComponent", () => { ], }).compileComponents(); + // Selecting a unit now remembers it per workflow in localStorage, which survives + // between tests and would let one spec's selection steer another's auto-select. + Object.keys(localStorage) + .filter(key => key.startsWith("computing-unit-of-workflow-")) + .forEach(key => localStorage.removeItem(key)); + fixture = TestBed.createComponent(ComputingUnitSelectionComponent); component = fixture.componentInstance; fixture.detectChanges(); @@ -1357,6 +1363,191 @@ describe("PowerButtonComponent", () => { expect(selectSpy).not.toHaveBeenCalled(); }); + + it("drops a latest-execution answer that arrives after the workflow changed underneath it", () => { + const execService = TestBed.inject(WorkflowExecutionsService); + const late$ = new Subject<WorkflowExecutionsEntry>(); + vi.spyOn(execService, "retrieveLatestWorkflowExecution").mockImplementation((wid: number) => + wid === 100 ? late$ : of({ cuId: 9 } as unknown as WorkflowExecutionsEntry) + ); + const { comp, emit } = bootWithMetaStream(); + const selectSpy = vi.spyOn(comp, "selectComputingUnit").mockImplementation(() => {}); + + emit(100); + emit(101); // decides at once from its own latest execution + late$.next({ cuId: 55 } as unknown as WorkflowExecutionsEntry); + + expect(selectSpy).toHaveBeenCalledWith(101, 9); + expect(selectSpy).not.toHaveBeenCalledWith(100, 55); + }); + + it("drops the running-unit fallback when the failed lookup was for a workflow no longer shown", () => { + const execService = TestBed.inject(WorkflowExecutionsService); + const late$ = new Subject<WorkflowExecutionsEntry>(); + vi.spyOn(execService, "retrieveLatestWorkflowExecution").mockImplementation((wid: number) => + wid === 100 ? late$ : of({ cuId: 9 } as unknown as WorkflowExecutionsEntry) + ); + const { comp, emit } = bootWithMetaStream(); + comp.allComputingUnits = [makeComputingUnit({ cuid: 2, status: "Running" })]; + const selectSpy = vi.spyOn(comp, "selectComputingUnit").mockImplementation(() => {}); + + emit(100); + emit(101); + late$.error(new Error("no execution")); + + expect(selectSpy).not.toHaveBeenCalledWith(100, 2); + expect(selectSpy).toHaveBeenCalledTimes(1); // 101's own decision only + }); + + it("prefers the remembered unit for this workflow over the latest execution", () => { + localStorage.setItem("computing-unit-of-workflow-100", "77"); + const execService = TestBed.inject(WorkflowExecutionsService); + const latestSpy = vi + .spyOn(execService, "retrieveLatestWorkflowExecution") + .mockReturnValue(of({ cuId: 55 } as unknown as WorkflowExecutionsEntry)); + const { comp, emit } = bootWithMetaStream(); + vi.spyOn(TestBed.inject(ComputingUnitStatusService), "getAllComputingUnits").mockReturnValue( + of([makeComputingUnit({ cuid: 77, status: "Running" })]) + ); + const selectSpy = vi.spyOn(comp, "selectComputingUnit").mockImplementation(() => {}); + + emit(100); + + expect(selectSpy).toHaveBeenCalledWith(100, 77); + expect(latestSpy).not.toHaveBeenCalled(); + }); + + // A remembered unit that has since been terminated must not be chased: the status service + // would wait for it to appear forever and the fallbacks would never run. + it("forgets a remembered unit that no longer exists and uses the latest execution", () => { + localStorage.setItem("computing-unit-of-workflow-100", "77"); + const execService = TestBed.inject(WorkflowExecutionsService); + vi.spyOn(execService, "retrieveLatestWorkflowExecution").mockReturnValue( + of({ cuId: 55 } as unknown as WorkflowExecutionsEntry) + ); + const { comp, emit } = bootWithMetaStream(); + vi.spyOn(TestBed.inject(ComputingUnitStatusService), "getAllComputingUnits").mockReturnValue( + of([makeComputingUnit({ cuid: 55, status: "Running" })]) + ); + const selectSpy = vi.spyOn(comp, "selectComputingUnit").mockImplementation(() => {}); + + emit(100); + + expect(selectSpy).toHaveBeenCalledWith(100, 55); + expect(localStorage.getItem("computing-unit-of-workflow-100")).toBeNull(); + }); + + it("still falls back when forgetting the stale unit throws", () => { + localStorage.setItem("computing-unit-of-workflow-100", "77"); + const execService = TestBed.inject(WorkflowExecutionsService); + vi.spyOn(execService, "retrieveLatestWorkflowExecution").mockReturnValue( + of({ cuId: 55 } as unknown as WorkflowExecutionsEntry) + ); + const { comp, emit } = bootWithMetaStream(); + vi.spyOn(TestBed.inject(ComputingUnitStatusService), "getAllComputingUnits").mockReturnValue( + of([makeComputingUnit({ cuid: 55, status: "Running" })]) + ); + // Only the component's one removeItem call throws; the storage keeps working for the + // afterEach clean-up and the tests that follow. + const removeSpy = vi.spyOn(Storage.prototype, "removeItem").mockImplementationOnce(() => { + throw new Error("storage unavailable"); + }); + const selectSpy = vi.spyOn(comp, "selectComputingUnit").mockImplementation(() => {}); + + expect(() => emit(100)).not.toThrow(); + + expect(removeSpy).toHaveBeenCalledWith("computing-unit-of-workflow-100"); + expect(selectSpy).toHaveBeenCalledWith(100, 55); + removeSpy.mockRestore(); + }); + + // Deciding on an empty list would throw the choice away before the list has loaded, so the + // decision waits for the first non-empty list. + it("waits for the unit list before honouring a remembered unit", () => { + localStorage.setItem("computing-unit-of-workflow-100", "77"); + const execService = TestBed.inject(WorkflowExecutionsService); + const latestSpy = vi.spyOn(execService, "retrieveLatestWorkflowExecution"); + const { comp, emit } = bootWithMetaStream(); + const units$ = new Subject<DashboardWorkflowComputingUnit[]>(); + vi.spyOn(TestBed.inject(ComputingUnitStatusService), "getAllComputingUnits").mockReturnValue(units$); + const selectSpy = vi.spyOn(comp, "selectComputingUnit").mockImplementation(() => {}); + + emit(100); + units$.next([]); + + expect(selectSpy).not.toHaveBeenCalled(); + expect(latestSpy).not.toHaveBeenCalled(); + + units$.next([makeComputingUnit({ cuid: 77, status: "Running" })]); + + expect(selectSpy).toHaveBeenCalledWith(100, 77); + }); + + it("drops a pending remembered decision once the workflow has changed underneath it", () => { + localStorage.setItem("computing-unit-of-workflow-100", "77"); + const execService = TestBed.inject(WorkflowExecutionsService); + vi.spyOn(execService, "retrieveLatestWorkflowExecution").mockReturnValue( + of({ cuId: 55 } as unknown as WorkflowExecutionsEntry) + ); + const { comp, emit } = bootWithMetaStream(); + const units$ = new Subject<DashboardWorkflowComputingUnit[]>(); + vi.spyOn(TestBed.inject(ComputingUnitStatusService), "getAllComputingUnits").mockReturnValue(units$); + const selectSpy = vi.spyOn(comp, "selectComputingUnit").mockImplementation(() => {}); + + emit(100); + // Workflow 101 has nothing remembered, so it decides at once from its latest execution. + emit(101); + units$.next([makeComputingUnit({ cuid: 77, status: "Running" })]); + + expect(selectSpy).toHaveBeenCalledWith(101, 55); + expect(selectSpy).not.toHaveBeenCalledWith(100, 77); + }); + + it("does not carry a remembered unit across workflows", () => { + localStorage.setItem("computing-unit-of-workflow-100", "77"); + const execService = TestBed.inject(WorkflowExecutionsService); + vi.spyOn(execService, "retrieveLatestWorkflowExecution").mockReturnValue( + of({ cuId: 55 } as unknown as WorkflowExecutionsEntry) + ); + const { comp, emit } = bootWithMetaStream(); + comp.allComputingUnits = [makeComputingUnit({ cuid: 77, status: "Running" })]; + const selectSpy = vi.spyOn(comp, "selectComputingUnit").mockImplementation(() => {}); + + emit(101); + + expect(selectSpy).toHaveBeenCalledWith(101, 55); + }); + + // Number() is lenient enough to turn several kinds of junk into a "valid" cuid. + ["0", "-3", "1.5", "", " "].forEach(stored => { + it(`ignores a remembered value of ${JSON.stringify(stored)}`, () => { + localStorage.setItem("computing-unit-of-workflow-100", stored); + const execService = TestBed.inject(WorkflowExecutionsService); + vi.spyOn(execService, "retrieveLatestWorkflowExecution").mockReturnValue( + of({ cuId: 55 } as unknown as WorkflowExecutionsEntry) + ); + const { comp, emit } = bootWithMetaStream(); + const selectSpy = vi.spyOn(comp, "selectComputingUnit").mockImplementation(() => {}); + + emit(100); + + expect(selectSpy).toHaveBeenCalledWith(100, 55); + }); + }); + + it("ignores a corrupt remembered value", () => { + localStorage.setItem("computing-unit-of-workflow-100", "not-a-number"); + const execService = TestBed.inject(WorkflowExecutionsService); + vi.spyOn(execService, "retrieveLatestWorkflowExecution").mockReturnValue( + of({ cuId: 55 } as unknown as WorkflowExecutionsEntry) + ); + const { comp, emit } = bootWithMetaStream(); + const selectSpy = vi.spyOn(comp, "selectComputingUnit").mockImplementation(() => {}); + + emit(100); + + expect(selectSpy).toHaveBeenCalledWith(100, 55); + }); }); describe("selectComputingUnit guards", () => { @@ -2200,4 +2391,50 @@ describe("PowerButtonComponent", () => { expect(component.pveModalVisible).toBe(false); }); }); + + describe("remembering the selected unit per workflow", () => { + const unit77 = { computingUnit: { cuid: 77 } } as unknown as DashboardWorkflowComputingUnit; + + it("writes the user's own pick so the other view of the same workflow restores it", () => { + component.workflowId = 100; + const select = vi.spyOn(component, "selectComputingUnit"); + + component.onPickComputingUnit(unit77); + + expect(select).toHaveBeenCalledWith(100, 77); + expect(component.selectedComputingUnit).toBe(unit77); + expect(localStorage.getItem("computing-unit-of-workflow-100")).toBe("77"); + }); + + it("does not remember a unit selected on load, which is derived rather than chosen", () => { + // The remembered unit, the last execution's or a running one are picked FOR the user; storing + // them would let a derived unit outrank a fresher last execution on the next load. + component.selectComputingUnit(100, 77); + expect(localStorage.getItem("computing-unit-of-workflow-100")).toBeNull(); + }); + + it("does not record a pick the component refused to make", () => { + component.workflowId = DEFAULT_WORKFLOW.wid; + component.onPickComputingUnit(unit77); + expect(localStorage.getItem(`computing-unit-of-workflow-${DEFAULT_WORKFLOW.wid}`)).toBeNull(); + component.workflowId = 100; + component.onPickComputingUnit({ computingUnit: {} } as unknown as DashboardWorkflowComputingUnit); + expect(localStorage.getItem("computing-unit-of-workflow-100")).toBeNull(); + }); + + it("skips remembering when there is no workflow to remember it for", () => { + const setItem = vi.spyOn(Storage.prototype, "setItem"); + (component as any).rememberComputingUnit(undefined, 5); + expect(setItem).not.toHaveBeenCalled(); + setItem.mockRestore(); + }); + + it("recalls nothing when storage cannot be read", () => { + const getItem = vi.spyOn(Storage.prototype, "getItem").mockImplementation(() => { + throw new Error("storage blocked"); + }); + expect((component as any).recallComputingUnit(100)).toBeUndefined(); + getItem.mockRestore(); + }); + }); }); diff --git a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts index 06da3e69ab..eb899448f3 100644 --- a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts +++ b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts @@ -18,7 +18,7 @@ */ import { ChangeDetectorRef, Component, OnInit, NgZone, ViewChild } from "@angular/core"; -import { take } from "rxjs/operators"; +import { filter, take } from "rxjs/operators"; import { WorkflowComputingUnitManagingService } from "../../../common/service/computing-unit/workflow-computing-unit/workflow-computing-unit-managing.service"; import { DashboardWorkflowComputingUnit } from "../../../common/type/workflow-computing-unit"; import { NotificationService } from "../../../common/service/notification/notification.service"; @@ -269,25 +269,71 @@ export class ComputingUnitSelectionComponent implements OnInit { if (wid !== this.workflowId) { this.workflowId = wid; if (isDefined(this.workflowId) && this.workflowId !== DEFAULT_WORKFLOW.wid) { - this.workflowExecutionsService - .retrieveLatestWorkflowExecution(this.workflowId) - .pipe(untilDestroyed(this)) - .subscribe({ - next: (latestWorkflowExecution: WorkflowExecutionsEntry) => { - this.selectComputingUnit(this.workflowId, latestWorkflowExecution.cuId); - }, - error: (err: unknown) => { - const runningUnit = this.allComputingUnits.find(unit => unit.status === "Running"); - if (runningUnit) { - this.selectComputingUnit(this.workflowId, runningUnit.computingUnit.cuid); - } - }, - }); + this.selectInitialUnit(this.workflowId); } } }); } + /** + * Pick the unit for a workflow that has just come into view. An explicit choice remembered for + * it is newer than its last run, so it wins -- but only once the unit list has arrived and still + * holds that unit. Deciding on an empty list would either chase a unit that has since been + * terminated (the status service waits for it to appear, forever, and the fallbacks below never + * run) or throw the choice away before the list has loaded. A remembered unit that is gone is + * forgotten, and the fallbacks take over: the last execution's unit, else any running unit. + */ + private selectInitialUnit(wid: number): void { + const remembered = this.recallComputingUnit(wid); + if (!isDefined(remembered)) { + this.selectFromLastExecution(wid); + return; + } + this.computingUnitStatusService + .getAllComputingUnits() + .pipe( + filter(units => units.length > 0), + take(1), + untilDestroyed(this) + ) + .subscribe(units => { + // The workflow can change while the list is still loading; that later change made its own + // decision, so this one is stale. + if (wid !== this.workflowId) { + return; + } + if (units.some(unit => unit.computingUnit.cuid === remembered)) { + this.selectComputingUnit(wid, remembered); + } else { + this.forgetComputingUnit(wid); + this.selectFromLastExecution(wid); + } + }); + } + + /** The unit the workflow last ran on, else any unit that is running. */ + private selectFromLastExecution(wid: number): void { + // The workflow can change while the lookup is out; that later change decided for itself, so an + // answer (or a failure) that arrives for the earlier one is stale. + const stillShown = () => wid === this.workflowId; + this.workflowExecutionsService + .retrieveLatestWorkflowExecution(wid) + .pipe(untilDestroyed(this)) + .subscribe({ + next: (latestWorkflowExecution: WorkflowExecutionsEntry) => { + if (stillShown()) { + this.selectComputingUnit(wid, latestWorkflowExecution.cuId); + } + }, + error: () => { + const runningUnit = this.allComputingUnits.find(unit => unit.status === "Running"); + if (stillShown() && runningUnit) { + this.selectComputingUnit(wid, runningUnit.computingUnit.cuid); + } + }, + }); + } + /** * Called whenever the selected computing unit changes. */ @@ -297,6 +343,72 @@ export class ComputingUnitSelectionComponent implements OnInit { } } + /** + * The user's own pick from the list: select it and remember it for this workflow. Only an explicit + * choice is remembered -- the units selected on load (the remembered one, the last execution's, + * a running one) are derived and must not be stored as if chosen, or a derived unit would later + * outrank a fresher last execution. + */ + public onPickComputingUnit(unit: DashboardWorkflowComputingUnit): void { + this.selectedComputingUnit = unit; + const cuid = unit?.computingUnit?.cuid; + // The same rule as selectComputingUnit: nothing is selected, or remembered, for a workflow that + // has not been saved yet or for a unit without an id. + if (!isDefined(cuid) || this.workflowId === DEFAULT_WORKFLOW.wid) { + return; + } + this.selectComputingUnit(this.workflowId, cuid); + this.rememberComputingUnit(this.workflowId, cuid); + } + + /** + * The live selection lives only in ComputingUnitStatusService, re-derived on load from the + * last execution -- but that only exists once the workflow has run (pick a unit, reload + * before running, and it is gone). Canvas<->Form View switches reload, so we remember the + * last explicit choice per workflow to keep the two views agreeing. One unit per workflow. + */ + private static computingUnitStorageKey(wid: number): string { + return `computing-unit-of-workflow-${wid}`; + } + + private rememberComputingUnit(wid: number | undefined, cuid: number): void { + if (!isDefined(wid)) { + return; + } + try { + localStorage.setItem(ComputingUnitSelectionComponent.computingUnitStorageKey(wid), String(cuid)); + } catch { + // Private browsing or a full quota; remembering is an optimisation, not a + // requirement -- the last-execution lookup still applies on the next load. + } + } + + private recallComputingUnit(wid: number): number | undefined { + let stored: string | null = null; + try { + stored = localStorage.getItem(ComputingUnitSelectionComponent.computingUnitStorageKey(wid)); + } catch { + return undefined; + } + // A cuid is a positive integer. Number() would also accept "0" and "1.5", and handing + // either on would mean chasing a unit that cannot exist. Whether the unit still exists is + // not decided here but against the loaded unit list (selectInitialUnit). + const cuid = Number(stored); + if (!stored || !Number.isInteger(cuid) || cuid <= 0) { + return undefined; + } + return cuid; + } + + /** Drop a remembered unit that no longer exists, so the next load goes straight to the fallbacks. */ + private forgetComputingUnit(wid: number): void { + try { + localStorage.removeItem(ComputingUnitSelectionComponent.computingUnitStorageKey(wid)); + } catch { + // Best effort, like remembering: a stale entry only costs the list check on the next load. + } + } + isComputingUnitRunning(): boolean { return this.selectedComputingUnit != null && this.selectedComputingUnit.status === "Running"; } @@ -348,7 +460,8 @@ export class ComputingUnitSelectionComponent implements OnInit { } onComputingUnitCreated(unit: DashboardWorkflowComputingUnit): void { - this.selectComputingUnit(this.workflowId, unit.computingUnit.cuid); + // Creating a unit from here is as explicit a choice as picking one. + this.onPickComputingUnit(unit); } openComputingUnitMetadataModal(unit: DashboardWorkflowComputingUnit) {
