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 f9f2994fc06..f7ba00a182b 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts @@ -55,6 +55,8 @@ 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 { UserProjectService } from "../../../dashboard/service/user/project/user-project.service"; +import { WorkflowCompilingService } from "../../service/compile-workflow/workflow-compiling.service"; +import { CompilationState } from "../../types/workflow-compiling.interface"; import type { Mocked } from "vitest"; vi.mock("file-saver", () => ({ saveAs: vi.fn() })); @@ -72,8 +74,10 @@ describe("MenuComponent", () => { let notificationService: NotificationService; let location: Location; let validationStream$: BehaviorSubject; + let compilationStream$: Subject; beforeEach(async () => { + compilationStream$ = new Subject(); await TestBed.configureTestingModule({ imports: [MenuComponent, HttpClientTestingModule, RouterTestingModule.withRoutes([]), NzModalModule], providers: [ @@ -90,6 +94,11 @@ describe("MenuComponent", () => { }, }, { provide: UserService, useClass: StubUserService }, + { + // stubbed so the debounced compile request of the real service does not outlive the test injector + provide: WorkflowCompilingService, + useValue: { getCompilationStateInfoChangedStream: () => compilationStream$.asObservable() }, + }, ...commonTestProviders, ], }).compileComponents(); @@ -131,6 +140,18 @@ describe("MenuComponent", () => { expect(behavior.disable).toBe(true); }); + it("returns 'Invalid Workflow' when the workflow does not compile", () => { + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.isWorkflowCompilable = false; + + const behavior = component.getRunButtonBehavior(); + + expect(behavior.text).toBe("Invalid Workflow"); + expect(behavior.icon).toBe("warning"); + expect(behavior.disable).toBe(true); + }); + it("returns 'Empty Workflow' when the workflow has no operators", () => { component.isWorkflowValid = true; component.isWorkflowEmpty = true; @@ -416,6 +437,19 @@ describe("MenuComponent", () => { expect(component.computingUnitSelectionComponent.showAddComputeUnitModalVisible).not.toHaveBeenCalled(); }); + it("does nothing when the workflow does not compile", () => { + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.isWorkflowCompilable = false; + component.computingUnitStatus = ComputingUnitState.Running; + const executeSpy = vi.spyOn(executeWorkflowService, "executeWorkflowWithEmailNotification"); + + component.runWorkflow(); + + expect(executeSpy).not.toHaveBeenCalled(); + expect(component.computingUnitSelectionComponent.showAddComputeUnitModalVisible).not.toHaveBeenCalled(); + }); + it("does nothing when the workflow is empty", () => { component.isWorkflowValid = true; component.isWorkflowEmpty = true; @@ -1393,6 +1427,27 @@ describe("MenuComponent", () => { } }); + it("re-applies the run button behavior on every compilation state event", () => { + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.computingUnitStatus = ComputingUnitState.Running; + component.executionState = ExecutionState.Uninitialized; + Object.defineProperty(component.workflowWebsocketService, "isConnected", { + get: () => true, + configurable: true, + }); + + compilationStream$.next(CompilationState.Failed); + expect(component.isWorkflowCompilable).toBe(false); + expect(component.runButtonText).toBe("Invalid Workflow"); + expect(component.runDisable).toBe(true); + + compilationStream$.next(CompilationState.Succeeded); + expect(component.isWorkflowCompilable).toBe(true); + expect(component.runButtonText).toBe("Run"); + expect(component.runDisable).toBe(false); + }); + it("deactivates the export button unless the feature is on and results exist", () => { const guiConfig = TestBed.inject(GuiConfigService); const results$ = component.workflowResultExportService.hasResultToExportOnAllOperators; diff --git a/frontend/src/app/workspace/component/menu/menu.component.ts b/frontend/src/app/workspace/component/menu/menu.component.ts index b3e44057da9..662440b788f 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.ts @@ -71,6 +71,8 @@ import { NzSwitchComponent } from "ng-zorro-antd/switch"; import { NzBadgeComponent } from "ng-zorro-antd/badge"; import { NzTooltipDirective } from "ng-zorro-antd/tooltip"; import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; +import { WorkflowCompilingService } from "../../service/compile-workflow/workflow-compiling.service"; +import { CompilationState } from "../../types/workflow-compiling.interface"; /** * MenuComponent is the top level menu bar that shows @@ -127,6 +129,9 @@ export class MenuComponent implements OnInit, OnDestroy { public ComputingUnitState = ComputingUnitState; // make Angular HTML access enum definition public isWorkflowValid: boolean = true; // this will check whether the workflow error or not public isWorkflowEmpty: boolean = false; + // whether the last compilation of the workflow failed. A workflow that cannot compile cannot be executed, + // so it is treated the same way as one that fails the schema / port validation. + public isWorkflowCompilable: boolean = true; public isSaving: boolean = false; public isWorkflowModifiable: boolean = false; public workflowId?: number; @@ -169,6 +174,7 @@ export class MenuComponent implements OnInit, OnDestroy { private location: Location, public undoRedoService: UndoRedoService, public validationWorkflowService: ValidationWorkflowService, + private workflowCompilingService: WorkflowCompilingService, public workflowPersistService: WorkflowPersistService, public workflowVersionService: WorkflowVersionService, public userService: UserService, @@ -233,6 +239,16 @@ export class MenuComponent implements OnInit, OnDestroy { this.applyRunButtonBehavior(this.getRunButtonBehavior()); }); + // the compilation errors are reported per operator on the canvas and in the result panel, but a workflow + // that cannot compile cannot be executed either, so the run button has to reflect the compilation state too + this.workflowCompilingService + .getCompilationStateInfoChangedStream() + .pipe(untilDestroyed(this)) + .subscribe(state => { + this.isWorkflowCompilable = state !== CompilationState.Failed; + this.applyRunButtonBehavior(this.getRunButtonBehavior()); + }); + // Subscribe to WorkflowResultExportService observable this.workflowResultExportService .getExportOnAllOperatorsStatusStream() @@ -353,8 +369,8 @@ export class MenuComponent implements OnInit, OnDestroy { disable: boolean; onClick: () => void; } { - // If workflow is invalid, always disable and show "Invalid Workflow" - if (!this.isWorkflowValid) { + // If workflow is invalid or does not compile, always disable and show "Invalid Workflow" + if (!this.isWorkflowValid || !this.isWorkflowCompilable) { return { text: "Invalid Workflow", icon: "warning", @@ -784,7 +800,7 @@ export class MenuComponent implements OnInit, OnDestroy { */ runWorkflow(): void { // Use the existing flags that were already updated via subscriptions - if (!this.isWorkflowValid || this.isWorkflowEmpty) { + if (!this.isWorkflowValid || !this.isWorkflowCompilable || this.isWorkflowEmpty) { return; } diff --git a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts index 2903abc1fb7..e180ae358c1 100644 --- a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts @@ -33,6 +33,7 @@ import { ReactiveFormsModule } from "@angular/forms"; import { BrowserAnimationsModule } from "@angular/platform-browser/animations"; import { NzDropDownModule } from "ng-zorro-antd/dropdown"; import { ValidationWorkflowService } from "src/app/workspace/service/validation/validation-workflow.service"; +import { WorkflowCompilingService } from "src/app/workspace/service/compile-workflow/workflow-compiling.service"; import { NzModalModule, NzModalService } from "ng-zorro-antd/modal"; import { commonTestProviders } from "../../../../../common/testing/test-utils"; // Import NzModalModule and NzModalService import type { Mocked } from "vitest"; @@ -56,6 +57,7 @@ describe("ContextMenuComponent", () => { let operatorMenuService: Mocked; let jointGraphWrapperSpy: Mocked; let validationWorkflowService: Mocked; + let workflowCompilingService: Mocked; let highlightedOperatorsSubject: BehaviorSubject; let highlightedCommentBoxesSubject: BehaviorSubject; @@ -71,7 +73,12 @@ describe("ContextMenuComponent", () => { jointGraphWrapperSpy.getCurrentHighlightedCommentBoxIDs.mockReturnValue([]); jointGraphWrapperSpy.getCurrentHighlightedLinkIDs.mockReturnValue([]); - const texeraGraphSpy = { isOperatorDisabled: vi.fn(), hasLinkWithID: vi.fn(), bundleActions: vi.fn() }; + const texeraGraphSpy = { + isOperatorDisabled: vi.fn(), + hasLinkWithID: vi.fn(), + bundleActions: vi.fn(), + getSubDAG: vi.fn(), + }; const workflowActionServiceSpy = { getJointGraphWrapper: vi.fn(), @@ -92,6 +99,7 @@ describe("ContextMenuComponent", () => { // Set up TexeraGraph spy return values texeraGraphSpy.hasLinkWithID.mockReturnValue(false); + texeraGraphSpy.getSubDAG.mockReturnValue({ operators: [], links: [] }); texeraGraphSpy.bundleActions.mockImplementation((callback: Function) => callback()); const workflowResultServiceSpy = { getResultService: vi.fn(), hasAnyResult: vi.fn() }; @@ -121,6 +129,9 @@ describe("ContextMenuComponent", () => { const validationWorkflowServiceSpy = { validateOperator: vi.fn() }; + const workflowCompilingServiceSpy = { getWorkflowCompilationErrors: vi.fn() }; + workflowCompilingServiceSpy.getWorkflowCompilationErrors.mockReturnValue({}); + await TestBed.configureTestingModule({ providers: [ { provide: OperatorMetadataService, useClass: StubOperatorMetadataService }, @@ -129,6 +140,7 @@ describe("ContextMenuComponent", () => { { provide: WorkflowResultExportService, useValue: workflowResultExportServiceSpy }, { provide: OperatorMenuService, useValue: operatorMenuService }, { provide: ValidationWorkflowService, useValue: validationWorkflowServiceSpy }, + { provide: WorkflowCompilingService, useValue: workflowCompilingServiceSpy }, NzModalService, // Provide NzModalService ...commonTestProviders, ], @@ -151,6 +163,7 @@ describe("ContextMenuComponent", () => { validationWorkflowService = TestBed.inject( ValidationWorkflowService ) as unknown as Mocked; + workflowCompilingService = TestBed.inject(WorkflowCompilingService) as unknown as Mocked; fixture = TestBed.createComponent(ContextMenuComponent); component = fixture.componentInstance; @@ -263,6 +276,31 @@ describe("ContextMenuComponent", () => { expect(texeraGraphSpy.isOperatorDisabled).toHaveBeenCalledWith("op1"); }); + it("should return false when the target operator failed to compile", () => { + texeraGraphSpy.getSubDAG.mockReturnValue({ operators: [{ operatorID: "op1" }], links: [] } as any); + workflowCompilingService.getWorkflowCompilationErrors.mockReturnValue({ op1: {} as any }); + + expect(component.canExecuteOperator()).toBe(false); + expect(texeraGraphSpy.getSubDAG).toHaveBeenCalledWith("op1"); + }); + + it("should return false when an upstream operator failed to compile", () => { + texeraGraphSpy.getSubDAG.mockReturnValue({ + operators: [{ operatorID: "op1" }, { operatorID: "upstream" }], + links: [], + } as any); + workflowCompilingService.getWorkflowCompilationErrors.mockReturnValue({ upstream: {} as any }); + + expect(component.canExecuteOperator()).toBe(false); + }); + + it("should return true when the compilation error is outside the target's sub-DAG", () => { + texeraGraphSpy.getSubDAG.mockReturnValue({ operators: [{ operatorID: "op1" }], links: [] } as any); + workflowCompilingService.getWorkflowCompilationErrors.mockReturnValue({ unrelated: {} as any }); + + expect(component.canExecuteOperator()).toBe(true); + }); + it("should check disabled status only for valid operators", () => { // First test with invalid operator validationWorkflowService.validateOperator.mockReturnValue({ isValid: false, messages: {} }); diff --git a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts index 019f77f7afa..8f0f6321bca 100644 --- a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts +++ b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts @@ -26,6 +26,7 @@ import { WorkflowResultExportService } from "src/app/workspace/service/workflow- import { NzModalService } from "ng-zorro-antd/modal"; import { ResultExportationComponent } from "../../../result-exportation/result-exportation.component"; import { ValidationWorkflowService } from "src/app/workspace/service/validation/validation-workflow.service"; +import { WorkflowCompilingService } from "src/app/workspace/service/compile-workflow/workflow-compiling.service"; import { GuiConfigService } from "../../../../../common/service/gui-config.service"; import { NzMenuDirective, NzMenuItemComponent } from "ng-zorro-antd/menu"; import { NgIf } from "@angular/common"; @@ -51,7 +52,8 @@ export class ContextMenuComponent { protected config: GuiConfigService, private workflowResultService: WorkflowResultService, private modalService: NzModalService, - private validationWorkflowService: ValidationWorkflowService + private validationWorkflowService: ValidationWorkflowService, + private workflowCompilingService: WorkflowCompilingService ) { this.registerWorkflowModifiableChangedHandler(); this.operatorMenuService.highlightedOperators$ @@ -82,10 +84,23 @@ export class ContextMenuComponent { private isOperatorExecutable(operatorID: string): boolean { return ( this.validationWorkflowService.validateOperator(operatorID).isValid && - !this.workflowActionService.getTexeraGraph().isOperatorDisabled(operatorID) + !this.workflowActionService.getTexeraGraph().isOperatorDisabled(operatorID) && + this.isSubDAGCompilable(operatorID) ); } + /** + * Executing to an operator runs the operator together with everything upstream of it, so the entry has to be + * disabled when any operator in that sub-DAG failed to compile, not only when the target operator itself did. + */ + private isSubDAGCompilable(operatorID: string): boolean { + const compilationErrors = this.workflowCompilingService.getWorkflowCompilationErrors(); + return this.workflowActionService + .getTexeraGraph() + .getSubDAG(operatorID) + .operators.every(operator => !(operator.operatorID in compilationErrors)); + } + public hasHighlightedLinks(): boolean { return this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedLinkIDs().length > 0; }