diff --git a/frontend/src/app/workspace/component/result-exportation/result-exportation.component.spec.ts b/frontend/src/app/workspace/component/result-exportation/result-exportation.component.spec.ts index ed3c62c8fc8..9470b694ddb 100644 --- a/frontend/src/app/workspace/component/result-exportation/result-exportation.component.spec.ts +++ b/frontend/src/app/workspace/component/result-exportation/result-exportation.component.spec.ts @@ -88,7 +88,10 @@ describe("ResultExportationComponent", () => { { provide: WorkflowActionService, useValue: { - getTexeraGraph: vi.fn().mockReturnValue({ getAllOperators: vi.fn().mockReturnValue([]) }), + getTexeraGraph: vi.fn().mockReturnValue({ + getAllOperators: vi.fn().mockReturnValue([]), + getAllOperatorIDs: vi.fn().mockReturnValue([]), + }), getJointGraphWrapper: vi .fn() .mockReturnValue({ getCurrentHighlightedOperatorIDs: vi.fn().mockReturnValue([]) }), @@ -141,8 +144,7 @@ describe("ResultExportationComponent", () => { expect(args[0]).toBe("csv"); // exportType, from modal data expect(args[1]).toBe("my-workflow"); // workflowName expect(args[2]).toEqual([1]); // datasetIds resolved from ds.dataset.did - expect(args[6]).toBe(true); // exportAll, because sourceTriggered === "menu" - expect(args[7]).toBe("dataset"); // destination + expect(args[6]).toBe("dataset"); // destination expect(modalClose).toHaveBeenCalledTimes(1); }); @@ -152,10 +154,45 @@ describe("ResultExportationComponent", () => { expect(exportWorkflowExecutionResult).toHaveBeenCalledTimes(1); const args = exportWorkflowExecutionResult.mock.calls[0]; expect(args[2]).toEqual([]); // local download carries no dataset ids - expect(args[7]).toBe("local"); + expect(args[6]).toBe("local"); expect(modalClose).toHaveBeenCalledTimes(1); }); + // The dialog reports on a scope (what a blocking dataset blocks, what kind of output is on + // offer) and then exports. Those have to be the same scope, so the export is handed the list + // the dialog resolved rather than a flag the service would resolve again, differently. + describe("the scope the dialog exports", () => { + const exportedOperatorIds = (): readonly string[] => { + component.onClickExportResult("local"); + return exportWorkflowExecutionResult.mock.calls[0][8]; + }; + const graph = () => TestBed.inject(WorkflowActionService).getTexeraGraph() as any; + const wrapper = () => TestBed.inject(WorkflowActionService).getJointGraphWrapper() as any; + + it("is the operators the caller named, whatever the canvas has selected", () => { + component.operatorIds = ["opNamed"]; + wrapper().getCurrentHighlightedOperatorIDs.mockReturnValue(["opHighlighted"]); + + expect(exportedOperatorIds()).toEqual(["opNamed"]); + }); + + it("is the whole workflow when the top menu opened the dialog", () => { + component.operatorIds = []; + component.sourceTriggered = "menu"; + graph().getAllOperatorIDs.mockReturnValue(["op1", "op2"]); + + expect(exportedOperatorIds()).toEqual(["op1", "op2"]); + }); + + it("is the canvas selection for anyone else who named nothing", () => { + component.operatorIds = []; + component.sourceTriggered = "context-menu"; + wrapper().getCurrentHighlightedOperatorIDs.mockReturnValue(["opHighlighted"]); + + expect(exportedOperatorIds()).toEqual(["opHighlighted"]); + }); + }); + it("onClickCreateNewDataset opens the dataset-creator modal and adopts the created dataset", () => { const created = { dataset: { did: 9, name: "brand-new" }, @@ -183,6 +220,7 @@ describe("ResultExportationComponent", () => { }; graph.getTexeraGraph.mockReturnValue({ getAllOperators: () => ids.map(id => ({ operatorID: id })), + getAllOperatorIDs: () => ids, }); } @@ -240,6 +278,7 @@ describe("ResultExportationComponent", () => { }; graph.getTexeraGraph.mockReturnValue({ getAllOperators: () => ids.map(id => ({ operatorID: id })), + getAllOperatorIDs: () => ids, }); } @@ -381,6 +420,7 @@ describe("ResultExportationComponent", () => { }; graph.getTexeraGraph.mockReturnValue({ getAllOperators: () => ids.map(id => ({ operatorID: id })), + getAllOperatorIDs: () => ids, }); } @@ -484,7 +524,7 @@ describe("ResultExportationComponent", () => { exportBtn!.triggerEventHandler("click", null); expect(exportWorkflowExecutionResult).toHaveBeenCalledTimes(1); const args = exportWorkflowExecutionResult.mock.calls[0]; - expect(args[7]).toBe("local"); + expect(args[6]).toBe("local"); }); it("renders the dataset destination with its list and create button", () => { @@ -601,7 +641,7 @@ describe("ResultExportationComponent", () => { expect(exportWorkflowExecutionResult).toHaveBeenCalledTimes(1); const args = exportWorkflowExecutionResult.mock.calls[0]; expect(args[2]).toEqual([1]); // the clicked dataset's did - expect(args[7]).toBe("dataset"); + expect(args[6]).toBe("dataset"); expect(modalClose).toHaveBeenCalledTimes(1); }); @@ -780,7 +820,10 @@ describe("ResultExportationComponent (context-menu source with default modal dat { provide: WorkflowActionService, useValue: { - getTexeraGraph: vi.fn().mockReturnValue({ getAllOperators: vi.fn().mockReturnValue([]) }), + getTexeraGraph: vi.fn().mockReturnValue({ + getAllOperators: vi.fn().mockReturnValue([]), + getAllOperatorIDs: vi.fn().mockReturnValue([]), + }), getJointGraphWrapper: vi.fn().mockReturnValue({ getCurrentHighlightedOperatorIDs: vi.fn().mockReturnValue(["hl-1", "hl-2"]), }), @@ -826,7 +869,7 @@ describe("ResultExportationComponent (context-menu source with default modal dat expect(component.blockedOperatorIds).toEqual([]); }); - it("exports highlighted operators only (exportAll === false) for a context-menu trigger", () => { + it("exports to the destination it was given for a context-menu trigger", () => { const exportService = TestBed.inject(WorkflowResultExportService) .exportWorkflowExecutionResult as unknown as ReturnType; @@ -834,7 +877,117 @@ describe("ResultExportationComponent (context-menu source with default modal dat expect(exportService).toHaveBeenCalledTimes(1); const args = exportService.mock.calls[0]; - expect(args[6]).toBe(false); // exportAll is false because sourceTriggered !== "menu" - expect(args[7]).toBe("local"); + expect(args[6]).toBe("local"); // destination + }); + + // The context menu names no operators of its own, so the dialog resolves the selection and + // hands that over. It does not pass nothing and leave the export to read the canvas a second + // time, which would let what is exported differ from what the dialog reported on. + it("hands over the selection it resolved, not an empty scope", () => { + const exportService = TestBed.inject(WorkflowResultExportService) + .exportWorkflowExecutionResult as unknown as ReturnType; + + component.onClickExportResult("local"); + + expect(exportService.mock.calls[0][8]).toEqual(["hl-1", "hl-2"]); + }); +}); + +// A result cell opens this dialog naming the one operator whose results it shows. Both the +// dialog's own checks and the export it triggers have to use that operator: the cell is also +// mounted on the Form View, where nothing is selected until the user clicks a step, so reading +// the selection there answered "no operators" and the export sent nothing. +describe("ResultExportationComponent (a caller that names its operators)", () => { + let component: ResultExportationComponent; + let fixture: ComponentFixture; + + // Exactly what result-table-frame.component.ts puts in nzData, including the absence of + // sourceTriggered: a result cell names its operator instead of naming a trigger. + const CELL_DATA = { + exportType: "data", + workflowName: "cell-workflow", + defaultFileName: "content_3", + rowIndex: 3, + columnIndex: 1, + operatorIds: ["op-named"], + }; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [ResultExportationComponent], + providers: [ + { provide: NZ_MODAL_DATA, useValue: CELL_DATA }, + { provide: NzModalRef, useValue: { close: vi.fn(), getConfig: () => ({}) } }, + { provide: NzModalService, useValue: { create: vi.fn().mockReturnValue({ afterClose: of(null) }) } }, + { + provide: WorkflowResultExportService, + useValue: { + computeRestrictionAnalysis: vi.fn().mockReturnValue(of(new WorkflowResultDownloadability(new Map()))), + exportWorkflowExecutionResult: vi.fn(), + }, + }, + { + provide: DatasetService, + useValue: { retrieveAccessibleDatasets: vi.fn().mockReturnValue(of([])) }, + }, + { + provide: WorkflowActionService, + useValue: { + // Both fallbacks answer with something else, so a passing test can only be reading + // the operator the caller named. + getTexeraGraph: vi.fn().mockReturnValue({ + getAllOperators: vi.fn().mockReturnValue([{ operatorID: "op-all" }]), + getAllOperatorIDs: vi.fn().mockReturnValue(["op-all"]), + }), + getJointGraphWrapper: vi.fn().mockReturnValue({ + getCurrentHighlightedOperatorIDs: vi.fn().mockReturnValue(["op-highlighted"]), + }), + }, + }, + { + provide: WorkflowResultService, + useValue: { + determineOutputTypes: vi.fn().mockReturnValue({ + hasAnyResult: true, + isTableOutput: true, + isVisualizationOutput: false, + containsBinaryData: true, + }), + }, + }, + { + provide: ComputingUnitStatusService, + useValue: { getSelectedComputingUnit: vi.fn().mockReturnValue(of(null)) }, + }, + ], + }).compileComponents(); + fixture = TestBed.createComponent(ResultExportationComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + afterEach(() => { + fixture?.destroy(); + }); + + // A result cell names its operator and sends no trigger of its own, so this field arrives + // absent and must read as a string rather than as undefined wearing a string's type. + it("reads an absent source trigger as an empty string", () => { + expect(component.sourceTriggered).toBe(""); + }); + + it("checks the named operator rather than the canvas selection", () => { + expect(component.exportableOperatorIds).toEqual(["op-named"]); + expect(component.blockedOperatorIds).toEqual([]); + }); + + it("hands the named operator to the export", () => { + const exportService = TestBed.inject(WorkflowResultExportService) + .exportWorkflowExecutionResult as unknown as ReturnType; + + component.onClickExportResult("local"); + + expect(exportService).toHaveBeenCalledTimes(1); + expect(exportService.mock.calls[0][8]).toEqual(["op-named"]); }); }); diff --git a/frontend/src/app/workspace/component/result-exportation/result-exportation.component.ts b/frontend/src/app/workspace/component/result-exportation/result-exportation.component.ts index 33abb625245..96fafe7ca7b 100644 --- a/frontend/src/app/workspace/component/result-exportation/result-exportation.component.ts +++ b/frontend/src/app/workspace/component/result-exportation/result-exportation.component.ts @@ -81,17 +81,20 @@ import { NzIconDirective } from "ng-zorro-antd/icon"; ], }) export class ResultExportationComponent implements OnInit { - /* Two sources can trigger this dialog, one from context-menu - which only export highlighted operators - and second is menu which wants to export all operators + /* Three sources can trigger this dialog: the context-menu, which exports the highlighted + operators; the menu, which wants to export all of them; and a result cell, which names the + one operator whose results it shows in operatorIds below and sends no trigger of its own. */ - sourceTriggered: string = inject(NZ_MODAL_DATA).sourceTriggered; + sourceTriggered: string = inject(NZ_MODAL_DATA).sourceTriggered ?? ""; workflowName: string = inject(NZ_MODAL_DATA).workflowName; inputFileName: string = inject(NZ_MODAL_DATA).defaultFileName ?? ""; rowIndex: number = inject(NZ_MODAL_DATA).rowIndex ?? -1; columnIndex: number = inject(NZ_MODAL_DATA).columnIndex ?? -1; destination: string = ""; exportType: string = inject(NZ_MODAL_DATA).exportType ?? ""; + // The operators this export covers, when the caller knows them. Empty leaves the scope to + // the rule below: the whole workflow for the menu, the canvas selection for the rest. + operatorIds: readonly string[] = inject(NZ_MODAL_DATA).operatorIds ?? []; isTableOutput: boolean = false; isVisualizationOutput: boolean = false; containsBinaryData: boolean = false; @@ -103,15 +106,21 @@ export class ResultExportationComponent implements OnInit { filteredUserAccessibleDatasets: DashboardDataset[] = []; /** - * Gets the operator IDs to check for restrictions based on the source trigger. - * Menu: all operators, Context menu: highlighted operators only + * The operators this export covers, which everything below reads: what may be exported, what + * a blocking dataset blocks, and what kind of output the dialog is offering. + * + * A caller that named them wins. The two fallbacks each belong to a caller reading the + * canvas -- the menu exports the whole workflow, the context menu exports the selection -- + * and a result cell is neither: it belongs to one operator, whoever is selected. On the Form + * View nothing is selected until the user clicks a step, so this answered "no operators", and + * with nothing in scope a blocked operator went unreported. */ private getOperatorIdsToCheck(): readonly string[] { + if (this.operatorIds.length > 0) { + return this.operatorIds; + } if (this.sourceTriggered === "menu") { - return this.workflowActionService - .getTexeraGraph() - .getAllOperators() - .map(op => op.operatorID); + return this.workflowActionService.getTexeraGraph().getAllOperatorIDs(); } else { return this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedOperatorIDs(); } @@ -199,7 +208,7 @@ export class ResultExportationComponent implements OnInit { const operatorIds = this.getOperatorIdsToCheck(); if (operatorIds.length === 0) { - // No operators highlighted + // No operators in scope this.isTableOutput = false; this.isVisualizationOutput = false; this.containsBinaryData = false; @@ -261,9 +270,11 @@ export class ResultExportationComponent implements OnInit { this.rowIndex, this.columnIndex, this.inputFileName, - this.sourceTriggered === "menu", destination, - this.selectedComputingUnit + this.selectedComputingUnit, + // The same scope the dialog reported on, so what is exported is what the dialog said it + // would export. + this.getOperatorIdsToCheck() ); this.modalRef.close(); } diff --git a/frontend/src/app/workspace/component/result-panel/result-table-frame/result-table-frame.component.html b/frontend/src/app/workspace/component/result-panel/result-table-frame/result-table-frame.component.html index e9c5f83ee4d..be6b1c06b10 100644 --- a/frontend/src/app/workspace/component/result-panel/result-table-frame/result-table-frame.component.html +++ b/frontend/src/app/workspace/component/result-panel/result-table-frame/result-table-frame.component.html @@ -193,7 +193,11 @@
{{ column.getCell(row) }} +