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-8551-af6e0fdd96f44c1d575278b9fd12ed61ce6a8640 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 9264975c2d05d2520593f84060b39db23ab8eae0 Author: Meng Wang <[email protected]> AuthorDate: Fri Sep 18 04:13:10 2026 +0000 feat(frontend): add the on-canvas per-execution warehouse picker (#8551) ### What changes were proposed in this PR? The workspace-side half of per-user warehouses (#6870): a warehouse picker on the canvas, beside the computing-unit selector it mirrors, choosing which warehouse the next execution writes to. - **The picker** (inside `ComputingUnitSelectionComponent`, like the computing-unit dropdown it sits beside) — shown only while the deployment enables the feature; each entry carries the owner avatar and a per-row delete, plus a create entry opening the shared create dialog. The list refreshes on every dropdown open, and preselection picks the latest execution's warehouse, falling back to the user's first one — so a run needs no explicit pick. A status failure clears the pick rather than letting a stale id ride the next request. - **Run gating** — with the feature enabled, every execution must have a warehouse: while none is selected the Run button becomes "Create Warehouse" and leads to the create dialog, the same shape as the Connect flow. (The backend-side requirement follows separately — #7751 stays the tracker.) - **The request** — the picked `warehouseId` rides the execute request (`ExecuteWorkflowService` reads the pick from `WarehouseService`, where it lives); executions expose `whId` so the preselect can read the latest run's warehouse. Flag off (the default): the picker never renders, the Run button is untouched, and requests carry no warehouseId — no user-visible change. ### demo https://github.com/user-attachments/assets/20e81213-ba36-450c-861e-594112515b50 ### Any related issues, documentation, discussions? Closes #7817. Part of #6870, on top of the dashboard tab (#8005); the backend enforcement (#7751) follows. ### How was this PR tested? - 20 new Vitest tests: 15 on the picker (preselect from the latest execution, first-warehouse fallback, disabled/failed states clearing the pick, manual pick surviving refreshes, create/delete flows, dropdown rendering), 3 on the Run gating, 1 pinning `warehouseId` on the execute request, 1 on the selection state. One pre-existing assertion modernized: the remembered-unit test asserted the latest-execution lookup's absence, which the warehouse preselect now legitimately performs — it asserts the unit choice directly instead. - The affected suites pass in full: 5083 tests across the workspace and dashboard trees. - Failure paths verified rather than assumed: the preselect, the Run gating, and the request field were each broken on purpose and the suite confirmed to fail for the expected reason before being restored. - Screenshots/video from a local flag-on deployment follow in the comments. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-5, claude-fable-5) --- .../service/warehouse/warehouse.service.spec.ts | 15 + .../common/service/warehouse/warehouse.service.ts | 20 +- .../workflow-execution-history.component.spec.ts | 1 + .../dashboard/type/workflow-executions-entry.ts | 2 + .../component/menu/menu.component.spec.ts | 80 ++++ .../app/workspace/component/menu/menu.component.ts | 44 +++ .../computing-unit-selection.component.html | 93 ++++- .../computing-unit-selection.component.scss | 85 +++++ .../computing-unit-selection.component.spec.ts | 406 ++++++++++++++++++++- .../computing-unit-selection.component.ts | 190 +++++++++- .../execute-workflow.service.spec.ts | 62 ++++ .../execute-workflow/execute-workflow.service.ts | 33 +- 12 files changed, 1021 insertions(+), 10 deletions(-) diff --git a/frontend/src/app/common/service/warehouse/warehouse.service.spec.ts b/frontend/src/app/common/service/warehouse/warehouse.service.spec.ts index 093aadcaaa..aea048c995 100644 --- a/frontend/src/app/common/service/warehouse/warehouse.service.spec.ts +++ b/frontend/src/app/common/service/warehouse/warehouse.service.spec.ts @@ -83,4 +83,19 @@ describe("WarehouseService", () => { expect(completed).toBe(true); }); + + it("holds the per-execution warehouse pick; undefined means no pick", () => { + expect(service.getSelectedWarehouseIdValue()).toBeUndefined(); + + const seen: (number | undefined)[] = []; + service.getSelectedWarehouseId().subscribe(whid => seen.push(whid)); + + service.selectWarehouse(7); + expect(service.getSelectedWarehouseIdValue()).toBe(7); + + service.selectWarehouse(undefined); + expect(service.getSelectedWarehouseIdValue()).toBeUndefined(); + + expect(seen).toEqual([undefined, 7, undefined]); + }); }); diff --git a/frontend/src/app/common/service/warehouse/warehouse.service.ts b/frontend/src/app/common/service/warehouse/warehouse.service.ts index facc7879b3..dc78a339cf 100644 --- a/frontend/src/app/common/service/warehouse/warehouse.service.ts +++ b/frontend/src/app/common/service/warehouse/warehouse.service.ts @@ -19,7 +19,7 @@ import { Injectable } from "@angular/core"; import { HttpClient } from "@angular/common/http"; -import { Observable } from "rxjs"; +import { BehaviorSubject, Observable } from "rxjs"; import { AppSettings } from "../../app-setting"; import { DashboardWarehouse, WarehouseStatus } from "../../type/warehouse"; @@ -34,6 +34,12 @@ export const WAREHOUSE_STATUS_URL = `${WAREHOUSE_BASE_URL}/status`; providedIn: "root", }) export class WarehouseService { + // The warehouse the next execution writes to; undefined = no pick. The + // backend still treats an absent warehouseId as the shared default storage; + // #7751 tightens that to a rejection while the feature is enabled — the + // picker's preselect and the Run gate keep it defined there. + private selectedWarehouseIdSubject = new BehaviorSubject<number | undefined>(undefined); + constructor(private http: HttpClient) {} public getStatus(): Observable<WarehouseStatus> { @@ -47,4 +53,16 @@ export class WarehouseService { public deleteWarehouse(whid: number): Observable<void> { return this.http.delete<void>(`${AppSettings.getApiEndpoint()}/${WAREHOUSE_BASE_URL}/${whid}`); } + + public selectWarehouse(whid: number | undefined): void { + this.selectedWarehouseIdSubject.next(whid); + } + + public getSelectedWarehouseId(): Observable<number | undefined> { + return this.selectedWarehouseIdSubject.asObservable(); + } + + public getSelectedWarehouseIdValue(): number | undefined { + return this.selectedWarehouseIdSubject.value; + } } diff --git a/frontend/src/app/dashboard/component/user/user-workflow/ngbd-modal-workflow-executions/workflow-execution-history.component.spec.ts b/frontend/src/app/dashboard/component/user/user-workflow/ngbd-modal-workflow-executions/workflow-execution-history.component.spec.ts index 8605d07a6d..8fe249275d 100644 --- a/frontend/src/app/dashboard/component/user/user-workflow/ngbd-modal-workflow-executions/workflow-execution-history.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-workflow/ngbd-modal-workflow-executions/workflow-execution-history.component.spec.ts @@ -47,6 +47,7 @@ function makeEntry(overrides: Partial<WorkflowExecutionsEntry> = {}): WorkflowEx eId: 1, vId: 1, cuId: 1, + whId: 1, sId: 0, userName: "alice", avatar: "", diff --git a/frontend/src/app/dashboard/type/workflow-executions-entry.ts b/frontend/src/app/dashboard/type/workflow-executions-entry.ts index 8687416d83..9a9136c39d 100644 --- a/frontend/src/app/dashboard/type/workflow-executions-entry.ts +++ b/frontend/src/app/dashboard/type/workflow-executions-entry.ts @@ -22,6 +22,8 @@ export interface WorkflowExecutionsEntry { eId: number; vId: number; cuId: number; + /** null for runs on the shared default storage, and after the warehouse is deleted. */ + whId: number | null; sId: number; userName: string; avatar: string; 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 3e15706cf0..5996762420 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts @@ -56,6 +56,7 @@ import { GuiConfigService } from "../../../common/service/gui-config.service"; import { MockGuiConfigService } from "../../../common/service/gui-config.service.mock"; import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; import type { Mocked } from "vitest"; +import { WarehouseService } from "../../../common/service/warehouse/warehouse.service"; describe("MenuComponent", () => { let component: MenuComponent; @@ -410,6 +411,40 @@ describe("MenuComponent", () => { expect(behavior.disable).toBe(false); expect(runSpy).toHaveBeenCalledTimes(1); }); + + it("keeps Pause in control of a running execution even when the warehouse disappears", () => { + // Deleting the last warehouse mid-run flips warehouseRequiredButMissing; + // the primary button must stay Pause/Kill, not become "Create Warehouse". + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.computingUnitStatus = ComputingUnitState.Running; + Object.defineProperty(component.workflowWebsocketService, "isConnected", { get: () => true, configurable: true }); + component.executionState = ExecutionState.Running; + component.computingUnitSelectionComponent = { + warehouseRequiredButMissing: true, + } as unknown as Mocked<ComputingUnitSelectionComponent>; + + const behavior = component.getRunButtonBehavior(); + + expect(behavior.text).toBe("Pause"); + }); + + it("offers to create a warehouse when one is required but missing", () => { + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.computingUnitStatus = ComputingUnitState.Running; + Object.defineProperty(component.workflowWebsocketService, "isConnected", { get: () => true, configurable: true }); + component.executionState = ExecutionState.Uninitialized; + component.computingUnitSelectionComponent = { + warehouseRequiredButMissing: true, + } as unknown as Mocked<ComputingUnitSelectionComponent>; + + const behavior = component.getRunButtonBehavior(); + + expect(behavior.text).toBe("Create Warehouse"); + expect(behavior.icon).toBe("plus-circle"); + expect(behavior.disable).toBe(false); + }); }); it("applyRunButtonBehavior copies the behavior onto the bound fields", () => { @@ -591,6 +626,51 @@ describe("MenuComponent", () => { expect(executeSpy).toHaveBeenCalledWith("Untitled Execution", false); }); + + it("leads to the create-warehouse modal when a warehouse is required but missing", () => { + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.computingUnitStatus = ComputingUnitState.Running; + component.computingUnitSelectionComponent = { + showAddComputeUnitModalVisible: vi.fn(), + showAddWarehouseModalVisible: vi.fn(), + warehouseRequiredButMissing: true, + } as unknown as Mocked<ComputingUnitSelectionComponent>; + const executeSpy = vi.spyOn(executeWorkflowService, "executeWorkflowWithEmailNotification"); + + component.runWorkflow(); + + expect(component.computingUnitSelectionComponent.showAddWarehouseModalVisible).toHaveBeenCalledTimes(1); + expect(executeSpy).not.toHaveBeenCalled(); + }); + + it("recomputes the Run button snapshot when the warehouse pick changes", () => { + // The button text is a stored snapshot; without the subscription it + // would keep saying "Run" after the warehouse load leaves none. + const applySpy = vi.spyOn(component, "applyRunButtonBehavior"); + + TestBed.inject(WarehouseService).selectWarehouse(7); + + expect(applySpy).toHaveBeenCalled(); + }); + + it("submits the execution when a warehouse is selected", () => { + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.computingUnitStatus = ComputingUnitState.Running; + component.computingUnitSelectionComponent = { + showAddWarehouseModalVisible: vi.fn(), + warehouseRequiredButMissing: false, + } as unknown as Mocked<ComputingUnitSelectionComponent>; + const executeSpy = vi + .spyOn(executeWorkflowService, "executeWorkflowWithEmailNotification") + .mockImplementation(() => {}); + + component.runWorkflow(); + + expect(executeSpy).toHaveBeenCalledTimes(1); + expect(component.computingUnitSelectionComponent.showAddWarehouseModalVisible).not.toHaveBeenCalled(); + }); }); it("onWorkflowNameChange forwards the new name to the workflow action service", () => { diff --git a/frontend/src/app/workspace/component/menu/menu.component.ts b/frontend/src/app/workspace/component/menu/menu.component.ts index 3a46e987f3..04da8e2b08 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.ts @@ -47,6 +47,7 @@ import { ShareAccessComponent } from "src/app/dashboard/component/user/share-acc import { PanelService } from "../../service/panel/panel.service"; import { USER_WORKFLOW, USER_WORKSPACE } from "../../../app-routing.constant"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; +import { WarehouseService } from "../../../common/service/warehouse/warehouse.service"; import { ComputingUnitState } from "../../../common/type/computing-unit-connection.interface"; import { ComputingUnitSelectionComponent } from "../power-button/computing-unit-selection.component"; import { GuiConfigService } from "../../../common/service/gui-config.service"; @@ -190,6 +191,7 @@ export class MenuComponent implements OnInit, OnDestroy { private reportGenerationService: ReportGenerationService, private panelService: PanelService, private computingUnitStatusService: ComputingUnitStatusService, + private warehouseService: WarehouseService, protected config: GuiConfigService, private router: Router, private jupyterPanelService: JupyterPanelService, @@ -285,6 +287,17 @@ export class MenuComponent implements OnInit, OnDestroy { this.computingUnitStatus = status; this.applyRunButtonBehavior(this.getRunButtonBehavior()); }); + + // The warehouse pick also feeds getRunButtonBehavior (#7817); without this + // the snapshot keeps saying "Run" after the load leaves no warehouse, and + // "Create Warehouse" after one is created. Every relevant transition ends + // in a selectWarehouse call, so the pick stream covers them all. + this.warehouseService + .getSelectedWarehouseId() + .pipe(untilDestroyed(this)) + .subscribe(() => { + this.applyRunButtonBehavior(this.getRunButtonBehavior()); + }); } /** @@ -408,6 +421,29 @@ export class MenuComponent implements OnInit, OnDestroy { }; } + // Per-user warehouses enabled but none to write to (#7817): mirror the + // Connect state above — name the fixing action, and runWorkflow() routes + // the click into the create-warehouse modal. Only in the states whose + // button would start a run: mid-execution the button is Pause/Resume/Kill, + // and losing the last warehouse must not take that control away. + if ( + this.computingUnitSelectionComponent?.warehouseRequiredButMissing && + [ + ExecutionState.Uninitialized, + ExecutionState.Completed, + ExecutionState.Terminated, + ExecutionState.Killed, + ExecutionState.Failed, + ].includes(this.executionState) + ) { + return { + text: "Create Warehouse", + icon: "plus-circle", + disable: false, + onClick: () => this.runWorkflow(), + }; + } + // Handle execution states when connected to a running computing unit switch (this.executionState) { case ExecutionState.Uninitialized: @@ -910,6 +946,14 @@ export class MenuComponent implements OnInit, OnDestroy { return; } + // Per-user warehouses enabled but none to write to (#7817): an execution + // must have a warehouse, so lead to the create-warehouse modal instead of + // running — the same shape as the Connect flow above. + if (this.computingUnitSelectionComponent.warehouseRequiredButMissing) { + this.computingUnitSelectionComponent.showAddWarehouseModalVisible(); + return; + } + // Regular workflow execution - already connected this.executeWorkflowService.executeWorkflowWithEmailNotification( this.currentExecutionName || "Untitled Execution", 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 066e39c836..ddbd2249a4 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 @@ -19,7 +19,7 @@ <div class="computing-units-selection" - [ngClass]="{ 'metrics-visible': isComputingUnitRunning() }"> + [ngClass]="{ 'metrics-visible': isComputingUnitRunning(), 'warehouse-visible': warehouseEnabled }"> <div *ngIf="isComputingUnitRunning() && selectedComputingUnit && selectedComputingUnit.computingUnit.type !== 'local'" nz-button @@ -58,6 +58,92 @@ </div> </div> + <button + *ngIf="warehouseEnabled" + nz-button + nz-dropdown + nzTrigger="click" + [nzDropdownMenu]="warehouseMenu" + [nzPlacement]="'bottomRight'" + (nzVisibleChange)="onWarehouseDropdownVisibilityChange($event)" + class="warehouse-dropdown-button" + nz-tooltip + nzTooltipTitle="Warehouse this execution writes to"> + <div class="button-content"> + <texera-user-avatar + *ngIf="selectedWarehouse as selected" + [avatar]="selected.ownerAvatar || ''" + userColor="grey" + [userName]="selected.ownerName || ''" + [style.transform]="'scale(0.65)'" + [style.opacity]="0.7" + [style.padding-right.px]="2"> + </texera-user-avatar> + <i + nz-icon + nzType="cloud-server"></i> + <span class="warehouse-name-text">{{ getWarehouseButtonText() }}</span> + <i + nz-icon + nzType="down"></i> + </div> + </button> + + <nz-dropdown-menu #warehouseMenu="nzDropdownMenu"> + <ul + nz-menu + class="warehouses-dropdown"> + <li + nz-menu-item + *ngFor="let warehouse of warehouses; trackBy: trackByWhid" + [id]="'warehouse-option-' + warehouse.whid" + class="warehouse-option" + [ngClass]="{ 'warehouse-selected': warehouse.whid === selectedWarehouseId }" + (click)="onWarehouseSelected(warehouse.whid)"> + <div class="warehouse-row"> + <texera-user-avatar + [avatar]="warehouse.ownerAvatar || ''" + userColor="grey" + [userName]="warehouse.ownerName || ''" + [style.transform]="'scale(0.65)'" + [style.opacity]="0.7"> + </texera-user-avatar> + <div class="warehouse-name"> + <span + nz-tooltip + [nzTooltipTitle]="warehouse.name"> + {{ warehouse.name }} + </span> + </div> + <i + nz-icon + nzType="delete" + class="warehouse-delete-icon" + nz-tooltip + [nzTooltipTitle]="'Delete warehouse and all data stored in it'" + (click)="confirmDeleteWarehouse(warehouse); $event.stopPropagation()" + role="button" + aria-label="Delete warehouse"> + </i> + </div> + </li> + + <li + *ngIf="warehouses.length > 0" + nz-menu-divider></li> + <li + nz-menu-item + (click)="showAddWarehouseModalVisible()"> + <div class="create-warehouse"> + <i + nz-icon + nzType="plus"></i> + <span> Warehouse</span> + </div> + </li> + </ul> + </nz-dropdown-menu> + <button nz-button nz-dropdown @@ -223,6 +309,11 @@ [(visible)]="addComputeUnitModalVisible" (unitCreated)="onComputingUnitCreated($event)"></texera-computing-unit-create-modal> +<!-- Panel for creating the warehouse --> +<texera-warehouse-create-modal + [(visible)]="addWarehouseModalVisible" + (warehouseCreated)="onWarehouseCreated($event)"></texera-warehouse-create-modal> + <ng-template #metricsTemplate> <div class="resource-metrics"> <div class="cpu-metric general-metric"> diff --git a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.scss b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.scss index 1bae9769d7..ce27d35674 100644 --- a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.scss +++ b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.scss @@ -29,6 +29,10 @@ min-width: 220px; max-width: 280px; + &.warehouse-visible { + max-width: none; + } + &.metrics-visible { min-width: 290px; max-width: none; @@ -139,6 +143,87 @@ justify-content: flex-start; } +.warehouses-dropdown { + width: 350px; + max-height: 50vh; + overflow-y: auto; +} + +.warehouse-option { + display: block; + width: 100%; + padding: 0 !important; +} + +.warehouse-row, +.warehouse-name, +.create-warehouse { + display: flex; + align-items: center; +} + +.warehouse-row { + justify-content: space-between; + width: 100%; + gap: 10px; + padding: 5px 12px; + box-sizing: border-box; +} + +.warehouse-name { + flex-grow: 1; + gap: 8px; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.warehouse-name span { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.warehouse-delete-icon { + margin-left: auto; + flex-shrink: 0; + opacity: 0.85; + color: #ff4d4f; + cursor: pointer; + + &:hover { + opacity: 1; + transform: scale(1.1); + } +} + +.create-warehouse { + gap: 10px; + justify-content: flex-start; +} + +.warehouse-dropdown-button { + display: inline-flex; + align-items: center; + min-width: 220px; + max-width: 280px; + margin-right: 4px; + padding: 0 8px; + overflow: hidden; + white-space: nowrap; +} + +.warehouse-name-text { + display: inline-block; + flex: 1 1 auto; + min-width: 0; + max-width: 220px; + margin: 0 4px; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + .resource-metrics { display: grid; } 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 2fca86830e..9f861e0d37 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 @@ -56,6 +56,9 @@ import { ComputingUnitActionsService } from "../../../common/service/computing-u import { ComputingUnitMetadataComponent } from "../../../common/util/computing-unit.util"; import { GuiConfigService } from "../../../common/service/gui-config.service"; import { NzPopoverDirective } from "ng-zorro-antd/popover"; +import { WarehouseService } from "../../../common/service/warehouse/warehouse.service"; +import { WarehouseActionsService } from "../../../common/service/warehouse/warehouse-actions.service"; +import { DashboardWarehouse } from "../../../common/type/warehouse"; /** * Builds a fully-populated DashboardWorkflowComputingUnit for driving the @@ -1414,7 +1417,11 @@ describe("PowerButtonComponent", () => { emit(100); expect(selectSpy).toHaveBeenCalledWith(100, 77); - expect(latestSpy).not.toHaveBeenCalled(); + // The warehouse preselect (#7817) legitimately reads the latest execution + // even on this path, so assert the unit choice directly instead of the + // lookup's absence: the latest execution's unit must not win. + expect(latestSpy).toHaveBeenCalled(); + expect(selectSpy).not.toHaveBeenCalledWith(100, 55); }); // A remembered unit that has since been terminated must not be chased: the status service @@ -2437,4 +2444,401 @@ describe("PowerButtonComponent", () => { getItem.mockRestore(); }); }); + + describe("warehouse picker (#7817)", () => { + function makeWarehouse(whid: number, name: string): DashboardWarehouse { + return { + whid, + name, + lakekeeperWarehouseName: `user-1-${name}`, + flavor: "local", + createdAtMillis: 0, + ownerName: "Alice", + ownerAvatar: "", + }; + } + + // Mirrors bootWithMetaStream, additionally pinning the warehouse status and + // the latest-execution response the preselection logic consumes. + function bootPicker(opts: { + enabled: boolean; + warehouses: DashboardWarehouse[]; + latest?: Partial<WorkflowExecutionsEntry> | "error"; + }): { + comp: ComputingUnitSelectionComponent; + pickerFixture: ComponentFixture<ComputingUnitSelectionComponent>; + emit: (wid: number) => void; + } { + vi.spyOn(TestBed.inject(WarehouseService), "getStatus").mockReturnValue( + of({ enabled: opts.enabled, warehouses: opts.warehouses }) + ); + const execService = TestBed.inject(WorkflowExecutionsService); + if (opts.latest === "error") { + vi.spyOn(execService, "retrieveLatestWorkflowExecution").mockReturnValue( + throwError(() => new Error("no execution")) + ); + } else if (opts.latest !== undefined) { + vi.spyOn(execService, "retrieveLatestWorkflowExecution").mockReturnValue( + of(opts.latest as WorkflowExecutionsEntry) + ); + } + const actionService = TestBed.inject(WorkflowActionService); + const meta$ = new Subject<WorkflowMetadata>(); + vi.spyOn(actionService, "workflowMetaDataChanged").mockReturnValue(meta$.asObservable()); + let currentMeta: WorkflowMetadata = { ...DEFAULT_WORKFLOW }; + vi.spyOn(actionService, "getWorkflowMetadata").mockImplementation(() => currentMeta); + const pickerFixture = TestBed.createComponent(ComputingUnitSelectionComponent); + pickerFixture.detectChanges(); + const comp = pickerFixture.componentInstance; + vi.spyOn(comp, "selectComputingUnit").mockImplementation(() => {}); + const emit = (wid: number) => { + currentMeta = { ...DEFAULT_WORKFLOW, wid }; + meta$.next(currentMeta); + }; + return { comp, pickerFixture, emit }; + } + + it("preselects the latest execution's warehouse when it still exists", () => { + const { emit } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first"), makeWarehouse(2, "second")], + latest: { cuId: 55, whId: 2 }, + }); + + emit(100); + + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(2); + }); + + it("falls back to the first warehouse when the latest execution used none", () => { + const { emit } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first"), makeWarehouse(2, "second")], + latest: { cuId: 55, whId: null }, + }); + + emit(100); + + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(1); + }); + + it("still preselects the first warehouse when there is no execution history", () => { + const { emit } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first")], + latest: "error", + }); + + emit(100); + + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(1); + }); + + it("never picks a warehouse while the feature is disabled, and hides the picker", () => { + // Warehouses alongside enabled=false cannot come from the real backend; the + // artificial combination pins that the flag alone suppresses preselection. + const { pickerFixture, emit } = bootPicker({ + enabled: false, + warehouses: [makeWarehouse(1, "first")], + latest: { cuId: 55, whId: 1 }, + }); + + emit(100); + + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBeUndefined(); + expect(pickerFixture.nativeElement.querySelector(".warehouse-dropdown-button")).toBeNull(); + }); + + it("a status failure keeps the last known list and pick, and reports the error", () => { + // A transport failure is not an answer; only an authoritative response + // (enabled:false, or a list without the pick) may clear the pick. + const { comp, emit } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first"), makeWarehouse(2, "second")], + latest: { cuId: 55, whId: 2 }, + }); + emit(100); + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(2); + const errorSpy = vi.spyOn(TestBed.inject(NotificationService), "error").mockImplementation(() => {}); + vi.spyOn(TestBed.inject(WarehouseService), "getStatus").mockReturnValue( + throwError(() => new Error("status unavailable")) + ); + + comp.onWarehouseDropdownVisibilityChange(true); + + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(2); + expect(comp.warehouses.map(w => w.whid)).toEqual([1, 2]); + expect(errorSpy).toHaveBeenCalledWith("Failed to fetch warehouses: status unavailable"); + }); + + it("clears any stale pick when the feature is disabled or no warehouse exists", () => { + TestBed.inject(WarehouseService).selectWarehouse(9); + + const { emit } = bootPicker({ enabled: true, warehouses: [], latest: { cuId: 55, whId: 9 } }); + emit(100); + + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBeUndefined(); + }); + + it("renders the dropdown trigger when enabled, and a manual pick writes through to the service", () => { + const { comp, pickerFixture } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first"), makeWarehouse(2, "second")], + }); + + pickerFixture.detectChanges(); + expect(pickerFixture.nativeElement.querySelector(".warehouse-dropdown-button")).toBeTruthy(); + + comp.onWarehouseSelected(2); + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(2); + expect(comp.trackByWhid(0, makeWarehouse(2, "second"))).toBe(2); + }); + + it("shows the trigger with the generic label when enabled with zero warehouses", () => { + const { comp, pickerFixture } = bootPicker({ enabled: true, warehouses: [] }); + + pickerFixture.detectChanges(); + + expect(pickerFixture.nativeElement.querySelector(".warehouse-dropdown-button")).toBeTruthy(); + expect(comp.getWarehouseButtonText()).toBe("Warehouse"); + expect(comp.warehouseRequiredButMissing).toBe(true); + }); + + it("reports no missing warehouse once the preselect has picked one", () => { + const { comp } = bootPicker({ enabled: true, warehouses: [makeWarehouse(1, "first")] }); + + expect(comp.warehouseRequiredButMissing).toBe(false); + }); + + it("shows the selected warehouse's name on the dropdown trigger", () => { + const { comp, emit } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first"), makeWarehouse(2, "second")], + latest: { cuId: 55, whId: 2 }, + }); + + emit(100); + expect(comp.getWarehouseButtonText()).toBe("second"); + + // An id that matches no warehouse falls back to the generic label. + TestBed.inject(WarehouseService).selectWarehouse(999); + expect(comp.getWarehouseButtonText()).toBe("Warehouse"); + }); + + it("refreshes the warehouse list when the dropdown opens, not when it closes", () => { + const { comp } = bootPicker({ enabled: true, warehouses: [makeWarehouse(1, "first")] }); + const statusSpy = vi.spyOn(TestBed.inject(WarehouseService), "getStatus"); + statusSpy.mockClear(); + + comp.onWarehouseDropdownVisibilityChange(true); + expect(statusSpy).toHaveBeenCalledTimes(1); + + comp.onWarehouseDropdownVisibilityChange(false); + expect(statusSpy).toHaveBeenCalledTimes(1); + }); + + it("keeps a manual pick across a dropdown-open refresh", () => { + const { comp } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first"), makeWarehouse(2, "second")], + }); + comp.onWarehouseSelected(2); + + comp.onWarehouseDropdownVisibilityChange(true); + + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(2); + }); + + it("re-preselects when the picked warehouse no longer exists", () => { + const { comp } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first"), makeWarehouse(2, "second")], + }); + comp.onWarehouseSelected(2); + vi.spyOn(TestBed.inject(WarehouseService), "getStatus").mockReturnValue( + of({ enabled: true, warehouses: [makeWarehouse(1, "first")] }) + ); + + comp.onWarehouseDropdownVisibilityChange(true); + + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(1); + }); + + it("opens the create modal from the menu, and selects a warehouse created there", () => { + const { comp } = bootPicker({ enabled: true, warehouses: [makeWarehouse(1, "first")] }); + + expect(comp.addWarehouseModalVisible).toBe(false); + comp.showAddWarehouseModalVisible(); + expect(comp.addWarehouseModalVisible).toBe(true); + + vi.spyOn(TestBed.inject(WarehouseService), "getStatus").mockReturnValue( + of({ enabled: true, warehouses: [makeWarehouse(1, "first"), makeWarehouse(9, "fresh")] }) + ); + comp.onWarehouseCreated(makeWarehouse(9, "fresh")); + + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(9); + }); + + it("hands the warehouse to the actions service, and re-preselects after the delete", () => { + const { comp } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first"), makeWarehouse(2, "second")], + }); + const actionsService = TestBed.inject(WarehouseActionsService); + const confirmAndDeleteSpy = vi.spyOn(actionsService, "confirmAndDelete").mockImplementation(() => {}); + const doomed = makeWarehouse(1, "first"); + + comp.confirmDeleteWarehouse(doomed); + + expect(confirmAndDeleteSpy).toHaveBeenCalledTimes(1); + expect(confirmAndDeleteSpy.mock.calls[0][0]).toEqual(doomed); + + vi.spyOn(TestBed.inject(WarehouseService), "getStatus").mockReturnValue( + of({ enabled: true, warehouses: [makeWarehouse(2, "second")] }) + ); + (confirmAndDeleteSpy.mock.calls[0][1] as () => void)(); + + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(2); + }); + + it("a stale last-execution warehouse never steers the next workflow's fallback", () => { + const { comp, emit } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first"), makeWarehouse(2, "second")], + latest: { cuId: 55, whId: 2 }, + }); + emit(100); + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(2); + + // The next workflow has no history: its fallback must be the FIRST + // warehouse, not the previous workflow's. + const execService = TestBed.inject(WorkflowExecutionsService); + vi.spyOn(execService, "retrieveLatestWorkflowExecution").mockReturnValue( + throwError(() => new Error("no execution")) + ); + emit(200); + + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(1); + }); + + it("a late response from a superseded refresh cannot restore stale state", () => { + const first = new Subject<{ enabled: boolean; warehouses: DashboardWarehouse[] }>(); + const second = new Subject<{ enabled: boolean; warehouses: DashboardWarehouse[] }>(); + vi.spyOn(TestBed.inject(WarehouseService), "getStatus") + .mockReturnValueOnce(first.asObservable()) + .mockReturnValueOnce(second.asObservable()); + const execService = TestBed.inject(WorkflowExecutionsService); + vi.spyOn(execService, "retrieveLatestWorkflowExecution").mockReturnValue( + throwError(() => new Error("no execution")) + ); + const pickerFixture = TestBed.createComponent(ComputingUnitSelectionComponent); + pickerFixture.detectChanges(); + const comp = pickerFixture.componentInstance; + + comp.onWarehouseDropdownVisibilityChange(true); + second.next({ enabled: true, warehouses: [makeWarehouse(2, "kept")] }); + second.complete(); + // The older request settles last; switchMap must already have dropped it. + first.next({ enabled: true, warehouses: [makeWarehouse(1, "stale"), makeWarehouse(9, "gone")] }); + first.complete(); + + expect(comp.warehouses.map(w => w.name)).toEqual(["kept"]); + }); + + it("a status failure keeps the gate closed on an enabled deployment", () => { + // Failing open would un-gate Run and let the execution write to the + // shared default storage with no warehouseId. + TestBed.inject(GuiConfigService).env.warehouseEnabled = true; + vi.spyOn(TestBed.inject(WarehouseService), "getStatus").mockReturnValue( + throwError(() => new Error("status unavailable")) + ); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + const failedFixture = TestBed.createComponent(ComputingUnitSelectionComponent); + failedFixture.detectChanges(); + + expect(failedFixture.componentInstance.warehouseRequiredButMissing).toBe(true); + errorSpy.mockRestore(); + TestBed.inject(GuiConfigService).env.warehouseEnabled = false; + }); + + it("a manual pick survives a late latest-execution answer", () => { + const inFlight = new Subject<WorkflowExecutionsEntry>(); + vi.spyOn(TestBed.inject(WorkflowExecutionsService), "retrieveLatestWorkflowExecution").mockReturnValue( + inFlight.asObservable() + ); + const { comp, emit } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first"), makeWarehouse(2, "second")], + }); + emit(100); + + comp.onWarehouseSelected(2); + inFlight.next({ cuId: 55, whId: 1 } as unknown as WorkflowExecutionsEntry); + inFlight.complete(); + + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(2); + }); + + it("switching workflows clears the pick at once, before the new preselect answers", () => { + const { comp, emit } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first"), makeWarehouse(2, "second")], + latest: { cuId: 55, whId: 2 }, + }); + emit(100); + comp.onWarehouseSelected(2); + + // The new workflow's lookup stays pending: in that window nothing of the + // old workflow's pick may ride an execution. + const pending = new Subject<WorkflowExecutionsEntry>(); + vi.spyOn(TestBed.inject(WorkflowExecutionsService), "retrieveLatestWorkflowExecution").mockReturnValue( + pending.asObservable() + ); + emit(200); + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBeUndefined(); + + pending.error(new Error("no execution")); + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(1); + }); + + it("deleting the picked warehouse removes it locally and re-preselects, with no refetch", () => { + const { comp, emit } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first"), makeWarehouse(2, "second")], + latest: { cuId: 55, whId: 2 }, + }); + emit(100); + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(2); + const statusSpy = vi.spyOn(TestBed.inject(WarehouseService), "getStatus"); + statusSpy.mockClear(); // drop the boot-time call; only the action below counts + const deleteSpy = vi + .spyOn(TestBed.inject(WarehouseActionsService), "confirmAndDelete") + .mockImplementation((_warehouse, onDeleted) => onDeleted()); + + comp.confirmDeleteWarehouse(makeWarehouse(2, "second")); + + expect(deleteSpy).toHaveBeenCalledTimes(1); + expect(comp.warehouses.map(w => w.whid)).toEqual([1]); + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(1); + expect(statusSpy).not.toHaveBeenCalled(); + }); + + it("a created warehouse is appended locally and becomes the pick, with no refetch", () => { + const { comp, emit } = bootPicker({ + enabled: true, + warehouses: [makeWarehouse(1, "first")], + latest: "error", + }); + emit(100); + const statusSpy = vi.spyOn(TestBed.inject(WarehouseService), "getStatus"); + statusSpy.mockClear(); // drop the boot-time call; only the action below counts + + comp.onWarehouseCreated(makeWarehouse(9, "fresh")); + + expect(comp.warehouses.map(w => w.whid)).toEqual([1, 9]); + expect(TestBed.inject(WarehouseService).getSelectedWarehouseIdValue()).toBe(9); + expect(statusSpy).not.toHaveBeenCalled(); + }); + }); }); 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 eb899448f3..566e1d9bed 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,8 @@ */ import { ChangeDetectorRef, Component, OnInit, NgZone, ViewChild } from "@angular/core"; -import { filter, take } from "rxjs/operators"; +import { catchError, filter, switchMap, take } from "rxjs/operators"; +import { EMPTY, Subject } from "rxjs"; 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"; @@ -27,6 +28,9 @@ import { isDefined } from "../../../common/util/predicate"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; import { extractErrorMessage } from "../../../common/util/error"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; +import { WarehouseService } from "../../../common/service/warehouse/warehouse.service"; +import { WarehouseActionsService } from "../../../common/service/warehouse/warehouse-actions.service"; +import { DashboardWarehouse } from "../../../common/type/warehouse"; import { NzModalService, NzModalComponent, NzModalContentDirective } from "ng-zorro-antd/modal"; import { WorkflowExecutionsService } from "../../../dashboard/service/user/workflow-executions/workflow-executions.service"; import { WorkflowExecutionsEntry } from "../../../dashboard/type/workflow-executions-entry"; @@ -73,6 +77,7 @@ import { NzSelectComponent, NzOptionComponent } from "ng-zorro-antd/select"; import { FormsModule } from "@angular/forms"; import { NzCollapseComponent, NzCollapsePanelComponent } from "ng-zorro-antd/collapse"; import { ComputingUnitCreateModalComponent } from "../../../common/component/computing-unit-create-modal/computing-unit-create-modal.component"; +import { WarehouseCreateModalComponent } from "../../../common/component/warehouse-create-modal/warehouse-create-modal.component"; type PveUserPackageRow = { name: string; @@ -128,6 +133,7 @@ type PveDraft = { NzCollapsePanelComponent, DecimalPipe, ComputingUnitCreateModalComponent, + WarehouseCreateModalComponent, ], }) export class ComputingUnitSelectionComponent implements OnInit { @@ -153,9 +159,31 @@ export class ComputingUnitSelectionComponent implements OnInit { selectedComputingUnit: DashboardWorkflowComputingUnit | null = null; allComputingUnits: DashboardWorkflowComputingUnit[] = []; + // Per-user warehouse picker (#7817): shown whenever the deployment reports + // the feature enabled — with zero warehouses it still offers the create + // entry, and the Run button leads there too. + warehouseEnabled: boolean = false; + warehouses: DashboardWarehouse[] = []; + selectedWarehouseId?: number; + // An explicit pick from the dropdown (or a create). Preselection never + // overrides it: a late latest-execution answer must not undo what the user + // chose in the meantime. Reset when the workflow changes. + private manualWarehousePick = false; + // The latest execution's warehouse; the warehouse list and the latest + // execution are fetched concurrently, so preselection re-runs after + // whichever response lands last. + private lastExecutionWhid?: number; + // All warehouse refreshes flow through one switchMap'd stream (like + // UserWarehouseComponent): a new request cancels the in-flight one, so an + // older response can never restore a deleted row or clobber a newer answer. + private readonly warehouseRefreshRequested$ = new Subject<void>(); + // visibility of the shared create-computing-unit modal addComputeUnitModalVisible = false; + // visibility of the shared create-warehouse modal + addWarehouseModalVisible = false; + @ViewChild(ComputingUnitCreateModalComponent) private computingUnitCreateModal?: ComputingUnitCreateModalComponent; @@ -180,7 +208,9 @@ export class ComputingUnitSelectionComponent implements OnInit { private cdr: ChangeDetectorRef, private computingUnitActionsService: ComputingUnitActionsService, private workflowPveService: WorkflowPveService, - private ngZone: NgZone + private ngZone: NgZone, + private warehouseService: WarehouseService, + private warehouseActionsService: WarehouseActionsService ) {} ngOnInit(): void { @@ -226,6 +256,43 @@ export class ComputingUnitSelectionComponent implements OnInit { this.allComputingUnits = units; }); + // Warehouse picker state (#7817). The pick itself lives in WarehouseService, + // where ExecuteWorkflowService reads it at execution time. + this.warehouseRefreshRequested$ + .pipe( + switchMap(() => + this.warehouseService.getStatus().pipe( + // Caught inside the switchMap so a failure ends only this request, + // not the stream. + catchError((err: unknown) => { + // A transport failure is not an answer: the last known list and + // pick stay (clearing them is reserved for an authoritative + // response — enabled:false, or a list without the pick). The flag + // only falls back to the boot-time config, never to false: failing + // open would un-gate Run on an enabled deployment just because one + // status request failed. + this.warehouseEnabled = this.config.env.warehouseEnabled; + this.notificationService.error(`Failed to fetch warehouses: ${extractErrorMessage(err)}`); + return EMPTY; + }) + ) + ), + untilDestroyed(this) + ) + .subscribe(status => { + this.warehouseEnabled = status.enabled; + this.warehouses = [...status.warehouses]; + this.applyWarehousePreselect(); + }); + this.refreshWarehouses(); + + this.warehouseService + .getSelectedWarehouseId() + .pipe(untilDestroyed(this)) + .subscribe(whid => { + this.selectedWarehouseId = whid; + }); + this.registerWorkflowMetadataSubscription(); } @@ -268,6 +335,12 @@ export class ComputingUnitSelectionComponent implements OnInit { const wid = this.workflowActionService.getWorkflowMetadata()?.wid; if (wid !== this.workflowId) { this.workflowId = wid; + // The previous workflow's execution — and pick — must not steer this + // one: with the stale value, a workflow without history would fall + // back to the OLD workflow's warehouse instead of the first one. + this.lastExecutionWhid = undefined; + this.manualWarehousePick = false; + this.warehouseService.selectWarehouse(undefined); if (isDefined(this.workflowId) && this.workflowId !== DEFAULT_WORKFLOW.wid) { this.selectInitialUnit(this.workflowId); } @@ -304,6 +377,9 @@ export class ComputingUnitSelectionComponent implements OnInit { } if (units.some(unit => unit.computingUnit.cuid === remembered)) { this.selectComputingUnit(wid, remembered); + // The remembered shortcut skips the execution lookup the warehouse + // preselect rides on, so run the lookup for the warehouse alone. + this.selectFromLastExecution(wid, false); } else { this.forgetComputingUnit(wid); this.selectFromLastExecution(wid); @@ -311,8 +387,12 @@ export class ComputingUnitSelectionComponent implements OnInit { }); } - /** The unit the workflow last ran on, else any unit that is running. */ - private selectFromLastExecution(wid: number): void { + /** + * The unit the workflow last ran on, else any unit that is running — and the warehouse it last + * wrote to, for the preselect (#7817). `selectUnit` is false when the unit was already settled by + * a remembered choice and only the warehouse still needs the lookup. + */ + private selectFromLastExecution(wid: number, selectUnit: boolean = true): 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; @@ -322,14 +402,22 @@ export class ComputingUnitSelectionComponent implements OnInit { .subscribe({ next: (latestWorkflowExecution: WorkflowExecutionsEntry) => { if (stillShown()) { - this.selectComputingUnit(wid, latestWorkflowExecution.cuId); + if (selectUnit) { + this.selectComputingUnit(wid, latestWorkflowExecution.cuId); + } + this.lastExecutionWhid = latestWorkflowExecution.whId ?? undefined; + this.applyWarehousePreselect(); } }, error: () => { const runningUnit = this.allComputingUnits.find(unit => unit.status === "Running"); - if (stillShown() && runningUnit) { + if (selectUnit && stillShown() && runningUnit) { this.selectComputingUnit(wid, runningUnit.computingUnit.cuid); } + // No execution history: still preselect a warehouse (the first one). + if (stillShown()) { + this.applyWarehousePreselect(); + } }, }); } @@ -409,6 +497,96 @@ export class ComputingUnitSelectionComponent implements OnInit { } } + /** + * Fetches the warehouse list, on init and on every dropdown open (mirroring + * onDropdownVisibilityChange). Every answer re-runs the preselect, whose + * manual-pick guard keeps a routine refresh from overriding a user's choice. + */ + private refreshWarehouses(): void { + this.warehouseRefreshRequested$.next(); + } + + /** + * Mirrors the CU preselection for warehouses (#7817): pick the latest + * execution's warehouse when it still exists, else the user's first + * warehouse — so a run needs no explicit pick. + */ + private applyWarehousePreselect(): void { + if ( + this.manualWarehousePick && + this.selectedWarehouseId !== undefined && + this.warehouses.some(warehouse => warehouse.whid === this.selectedWarehouseId) + ) { + return; + } + if (!this.warehouseEnabled || this.warehouses.length === 0) { + // Nothing selectable: drop any pick the root-scoped service still holds, so a + // stale id cannot ride the next execution while the picker stays hidden. + this.warehouseService.selectWarehouse(undefined); + return; + } + const lastUsed = this.warehouses.find(warehouse => warehouse.whid === this.lastExecutionWhid); + this.warehouseService.selectWarehouse((lastUsed ?? this.warehouses[0]).whid); + } + + onWarehouseSelected(whid: number): void { + this.manualWarehousePick = true; + this.warehouseService.selectWarehouse(whid); + } + + public trackByWhid(_idx: number, warehouse: DashboardWarehouse): number { + return warehouse.whid; + } + + onWarehouseDropdownVisibilityChange(visible: boolean): void { + if (visible) { + this.refreshWarehouses(); + } + } + + get selectedWarehouse(): DashboardWarehouse | undefined { + return this.warehouses.find(warehouse => warehouse.whid === this.selectedWarehouseId); + } + + /** + * True when the deployment enables per-user warehouses but none is selected. + * The menu's Run button redirects to the create-warehouse modal in this + * state, mirroring the computing-unit Connect flow: with the feature on, + * every execution must have a warehouse to write to. + */ + get warehouseRequiredButMissing(): boolean { + return this.warehouseEnabled && this.selectedWarehouseId === undefined; + } + + getWarehouseButtonText(): string { + return this.selectedWarehouse?.name ?? "Warehouse"; + } + + showAddWarehouseModalVisible(): void { + this.addWarehouseModalVisible = true; + } + + onWarehouseCreated(warehouse: DashboardWarehouse): void { + // Mirrors onComputingUnitCreated: a warehouse created from the workspace is + // what the next execution should write to — as explicit a choice as a pick. + this.manualWarehousePick = true; + // Appended locally, as the dashboard tab does: the backend lists by + // created_at ascending, so no round trip is needed to keep the order. + this.warehouses = [...this.warehouses, warehouse]; + this.warehouseService.selectWarehouse(warehouse.whid); + } + + confirmDeleteWarehouse(warehouse: DashboardWarehouse): void { + this.warehouseActionsService.confirmAndDelete(warehouse, () => { + this.warehouses = this.warehouses.filter(entry => entry.whid !== warehouse.whid); + // Deleting the picked warehouse leaves the pick dangling; the preselect + // moves it (its manual-pick guard no longer holds for a gone id). + if (this.selectedWarehouseId === warehouse.whid) { + this.applyWarehousePreselect(); + } + }); + } + isComputingUnitRunning(): boolean { return this.selectedComputingUnit != null && this.selectedComputingUnit.status === "Running"; } diff --git a/frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.spec.ts b/frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.spec.ts index 6b1b510634..c097fbf5a1 100644 --- a/frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.spec.ts +++ b/frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.spec.ts @@ -32,6 +32,9 @@ import { StubOperatorMetadataService } from "../operator-metadata/stub-operator- import { JointUIService } from "../joint-ui/joint-ui.service"; import { of, Subject } from "rxjs"; import { WorkflowWebsocketService } from "../workflow-websocket/workflow-websocket.service"; +import { WorkflowStatusService } from "../workflow-status/workflow-status.service"; +import { NotificationService } from "../../../common/service/notification/notification.service"; +import { GuiConfigService } from "../../../common/service/gui-config.service"; import { mockLogicalPlan_scan_result, mockWorkflowPlan_scan_result } from "./mock-workflow-plan"; import { HttpClientTestingModule } from "@angular/common/http/testing"; @@ -39,6 +42,7 @@ import { WorkflowUtilService } from "../workflow-graph/util/workflow-util.servic import { WorkflowSettings } from "src/app/common/type/workflow"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; +import { WarehouseService } from "../../../common/service/warehouse/warehouse.service"; import { AuthService } from "src/app/common/service/user/auth.service"; import { StubAuthService } from "src/app/common/service/user/stub-auth.service"; import { UserService } from "src/app/common/service/user/user.service"; @@ -397,6 +401,64 @@ describe("ExecuteWorkflowService", () => { ); })); + it("a refused run leaves the previous execution's state untouched (#7817)", () => { + TestBed.inject(GuiConfigService).env.warehouseEnabled = true; + try { + TestBed.inject(WarehouseService).selectWarehouse(undefined); + const resetSpy = vi.spyOn(service, "resetExecutionState"); + const statusResetSpy = vi.spyOn(TestBed.inject(WorkflowStatusService), "resetStatus"); + vi.spyOn(TestBed.inject(NotificationService), "error").mockReturnValue(undefined as never); + + service.executeWorkflowWithEmailNotification("exec", false); + + expect(resetSpy).not.toHaveBeenCalled(); + expect(statusResetSpy).not.toHaveBeenCalled(); + } finally { + TestBed.inject(GuiConfigService).env.warehouseEnabled = false; + } + }); + + it("refuses to run without a warehouse while the deployment requires one (#7817)", fakeAsync(() => { + // Paths that bypass the menu gate (form view, run-up-to, replay) all funnel + // through sendExecutionRequest; the shared storage must not catch them. + TestBed.inject(GuiConfigService).env.warehouseEnabled = true; + try { + TestBed.inject(WarehouseService).selectWarehouse(undefined); + const wsSendSpy = vi.spyOn(service["workflowWebsocketService"], "send"); + const errorSpy = vi.spyOn(TestBed.inject(NotificationService), "error").mockReturnValue(undefined as never); + + service.executeWorkflowWithEmailNotification("exec", false); + tick(FORM_DEBOUNCE_TIME_MS + 1); + flush(); + + expect(wsSendSpy).not.toHaveBeenCalledWith("WorkflowExecuteRequest", expect.anything()); + expect(errorSpy).toHaveBeenCalledWith("Create or select a warehouse before running."); + } finally { + TestBed.inject(GuiConfigService).env.warehouseEnabled = false; + } + })); + + it("sendExecutionRequest carries the picked warehouse id, and none when unset (#7817)", fakeAsync(() => { + const warehouseService = TestBed.inject(WarehouseService); + const wsSendSpy = vi.spyOn(service["workflowWebsocketService"], "send"); + const settings = service["workflowActionService"].getWorkflowSettings(); + + warehouseService.selectWarehouse(7); + service.sendExecutionRequest("exec", {} as LogicalPlan, settings, false, undefined); + tick(FORM_DEBOUNCE_TIME_MS + 1); + flush(); + expect(wsSendSpy).toHaveBeenLastCalledWith("WorkflowExecuteRequest", expect.objectContaining({ warehouseId: 7 })); + + warehouseService.selectWarehouse(undefined); + service.sendExecutionRequest("exec", {} as LogicalPlan, settings, false, undefined); + tick(FORM_DEBOUNCE_TIME_MS + 1); + flush(); + expect(wsSendSpy).toHaveBeenLastCalledWith( + "WorkflowExecuteRequest", + expect.objectContaining({ warehouseId: undefined }) + ); + })); + it("sendExecutionRequest flags stored pagination info as belonging to a new execution", fakeAsync(() => { sessionSetObject(PAGINATION_INFO_STORAGE_KEY, { newWorkflowExecuted: false }); const settings = service["workflowActionService"].getWorkflowSettings(); diff --git a/frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.ts b/frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.ts index c2ab3eac0d..3f9f5c53a1 100644 --- a/frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.ts +++ b/frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.ts @@ -48,6 +48,8 @@ import { intersection } from "../../../common/util/set"; import { WorkflowSettings } from "../../../common/type/workflow"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; +import { WarehouseService } from "../../../common/service/warehouse/warehouse.service"; +import { GuiConfigService } from "../../../common/service/gui-config.service"; // TODO: change this declaration export const FORM_DEBOUNCE_TIME_MS = 150; @@ -100,7 +102,9 @@ export class ExecuteWorkflowService { private workflowStatusService: WorkflowStatusService, private notificationService: NotificationService, @Inject(DOCUMENT) private document: Document, - private computingUnitStatusService: ComputingUnitStatusService + private computingUnitStatusService: ComputingUnitStatusService, + private warehouseService: WarehouseService, + private config: GuiConfigService ) { workflowWebsocketService.websocketEvent().subscribe(event => { switch (event.type) { @@ -207,6 +211,9 @@ export class ExecuteWorkflowService { targetOperatorId ); const settings = this.workflowActionService.getWorkflowSettings(); + if (this.refuseToRunWithoutWarehouse()) { + return; + } this.resetExecutionState(); this.workflowStatusService.resetStatus(); this.sendExecutionRequest(executionName, logicalPlan, settings, emailNotificationEnabled); @@ -219,6 +226,9 @@ export class ExecuteWorkflowService { public executeWorkflowWithReplay(replayExecutionInfo: ReplayExecutionInfo): void { const logicalPlan = ExecuteWorkflowService.getLogicalPlanRequest(this.workflowActionService.getTexeraGraph()); const settings = this.workflowActionService.getWorkflowSettings(); + if (this.refuseToRunWithoutWarehouse()) { + return; + } this.resetExecutionState(); this.workflowStatusService.resetStatus(); this.sendExecutionRequest( @@ -230,6 +240,21 @@ export class ExecuteWorkflowService { ); } + /** + * While the deployment requires a warehouse (#7817) and none is picked, + * refuses with a toast and returns true. Checked at every public entry + * point before it resets the previous execution's state — a refused click + * must not wipe the results already on screen (#7751 adds the backend-side + * rejection). + */ + private refuseToRunWithoutWarehouse(): boolean { + if (!this.config.env.warehouseEnabled || this.warehouseService.getSelectedWarehouseIdValue() !== undefined) { + return false; + } + this.notificationService.error("Create or select a warehouse before running."); + return true; + } + public sendExecutionRequest( executionName: string, logicalPlan: LogicalPlan, @@ -241,6 +266,11 @@ export class ExecuteWorkflowService { const selectedUnit = this.computingUnitStatusService.getSelectedComputingUnitValue(); const computingUnitId = selectedUnit?.computingUnit.cuid; + // The warehouse this execution writes to (#7817); undefined serializes away, + // which the backend today reads as the shared default storage (#7751 + // tightens that to a rejection while the feature is enabled). + const warehouseId = this.warehouseService.getSelectedWarehouseIdValue(); + // Log a warning if no computing unit is selected if (computingUnitId === undefined) { console.warn("No computing unit selected for workflow execution"); @@ -254,6 +284,7 @@ export class ExecuteWorkflowService { workflowSettings: workflowSettings, emailNotificationEnabled: emailNotificationEnabled, computingUnitId: computingUnitId, // Include the computing unit ID + warehouseId: warehouseId, }; // wait for the form debounce to complete, then send window.setTimeout(() => {
