aglinxinyuan commented on code in PR #6359:
URL: https://github.com/apache/texera/pull/6359#discussion_r3565440671
##########
frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts:
##########
@@ -357,4 +366,343 @@ describe("SavedWorkflowSectionComponent", () => {
expect(testWorkflowFileNameConflictEntries[0].checked).toBe(true);
expect(testWorkflowFileNameConflictEntries[2].checked).toBe(true);
});
+
+ describe("additional UserWorkflowComponent behaviors", () => {
+ const VIEW_MODE_KEY = "texera.userWorkflow.viewMode";
+
+ const makeDashboardWorkflow = (wid: number | undefined, name: string):
DashboardWorkflow => ({
+ workflow: {
+ wid,
+ name,
+ description: "desc",
+ content: testWorkflowContent([]),
+ creationTime: 0,
+ lastModifiedTime: 0,
+ isPublished: 0,
+ readonly: false,
+ },
+ isOwner: true,
+ ownerName: "Texera",
+ accessLevel: "Write",
+ projectIDs: [],
+ ownerId: 1,
+ });
+
+ const makeEntry = (wid: number | undefined, name: string, checked =
false): DashboardEntry => {
+ const entry = new DashboardEntry(makeDashboardWorkflow(wid, name));
+ entry.checked = checked;
+ return entry;
+ };
+
+ const setEntries = (entries: DashboardEntry[]): void => {
+ component.searchResultsComponent.entries = entries;
+ };
+
+ afterEach(() => {
+ vi.restoreAllMocks();
+ localStorage.removeItem(VIEW_MODE_KEY);
+ });
+
+ describe("deleteWorkflow", () => {
+ it("deletes an entry with a wid and removes it from the results", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.deleteWorkflow = vi.fn().mockReturnValue(of(null));
+ const target = makeEntry(5, "to delete");
+ setEntries([target, makeEntry(6, "keep")]);
+
+ component.deleteWorkflow(target);
+
+ expect(persist.deleteWorkflow).toHaveBeenCalledWith([5]);
+ expect(component.searchResultsComponent.entries.map(e =>
e.name)).toEqual(["keep"]);
+ });
+
+ it("does nothing when the entry has no wid", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.deleteWorkflow = vi.fn();
+
+ component.deleteWorkflow(makeEntry(undefined, "no wid"));
+
+ expect(persist.deleteWorkflow).not.toHaveBeenCalled();
+ });
+ });
+
+ describe("onClickDuplicateSelectedWorkflows", () => {
+ it("duplicates checked wids without a pid and prepends the new entries",
() => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.duplicateWorkflow = vi
+ .fn()
+ .mockReturnValue(of([makeDashboardWorkflow(101, "dup a"),
makeDashboardWorkflow(102, "dup b")]));
+ component.pid = undefined;
+ setEntries([makeEntry(1, "wf 1", true), makeEntry(2, "wf 2", true),
makeEntry(3, "wf 3", false)]);
+
+ component.onClickDuplicateSelectedWorkflows();
+
+ expect(persist.duplicateWorkflow).toHaveBeenCalledWith([1, 2]);
+ expect(component.searchResultsComponent.entries.map(e =>
e.name)).toEqual([
+ "dup a",
+ "dup b",
+ "wf 1",
+ "wf 2",
+ "wf 3",
+ ]);
+ });
+
+ it("passes the pid to duplicateWorkflow when the section belongs to a
project", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.duplicateWorkflow =
vi.fn().mockReturnValue(of([makeDashboardWorkflow(101, "dup a")]));
+ component.pid = 9;
+ setEntries([makeEntry(1, "wf 1", true), makeEntry(2, "wf 2", true)]);
+
+ component.onClickDuplicateSelectedWorkflows();
+
+ expect(persist.duplicateWorkflow).toHaveBeenCalledWith([1, 2], 9);
+ });
+
+ it("early-returns without calling the service when a checked entry has
no wid", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.duplicateWorkflow = vi.fn();
+ setEntries([makeEntry(1, "wf 1", true), makeEntry(undefined, "wf 2",
true)]);
+
+ component.onClickDuplicateSelectedWorkflows();
+
+ expect(persist.duplicateWorkflow).not.toHaveBeenCalled();
+ });
+
+ it("alerts on a duplication error", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.duplicateWorkflow = vi.fn().mockReturnValue(throwError(() =>
"boom"));
+ const alertSpy = vi.spyOn(window, "alert").mockImplementation(() =>
{});
+ component.pid = undefined;
+ setEntries([makeEntry(1, "wf 1", true)]);
+
+ component.onClickDuplicateSelectedWorkflows();
+
+ expect(alertSpy).toHaveBeenCalledWith("boom");
+ });
+ });
+
+ describe("handleConfirmDeleteSelectedWorkflows", () => {
+ it("deletes checked wids and keeps undefined-wid entries", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.deleteWorkflow = vi.fn().mockReturnValue(of(null));
+ setEntries([
+ makeEntry(1, "a", true),
+ makeEntry(2, "b", true),
+ makeEntry(undefined, "c", false),
+ makeEntry(3, "d", false),
+ ]);
+
+ component.handleConfirmDeleteSelectedWorkflows();
+
+ expect(persist.deleteWorkflow).toHaveBeenCalledWith([1, 2]);
+ expect(component.searchResultsComponent.entries.map(e =>
e.name)).toEqual(["c", "d"]);
+ });
+
+ it("early-returns when a checked entry has no wid", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.deleteWorkflow = vi.fn();
+ setEntries([makeEntry(undefined, "a", true)]);
+
+ component.handleConfirmDeleteSelectedWorkflows();
+
+ expect(persist.deleteWorkflow).not.toHaveBeenCalled();
+ });
+
+ it("alerts on a deletion error", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.deleteWorkflow = vi.fn().mockReturnValue(throwError(() =>
"delfail"));
+ const alertSpy = vi.spyOn(window, "alert").mockImplementation(() =>
{});
+ setEntries([makeEntry(1, "a", true)]);
+
+ component.handleConfirmDeleteSelectedWorkflows();
+
+ expect(alertSpy).toHaveBeenCalledWith("delfail");
+ });
+ });
+
+ describe("onClickOpenDownloadZip", () => {
+ it("does not call the download service when nothing is checked", () => {
+ setEntries([makeEntry(1, "a", false)]);
+
+ component.onClickOpenDownloadZip();
+
+
expect(downloadServiceSpy.downloadWorkflowsAsZip).not.toHaveBeenCalled();
+ });
+
+ it("logs an error when the download fails", () => {
+ setEntries([makeEntry(1, "a", true)]);
+
downloadServiceSpy.downloadWorkflowsAsZip.mockReturnValue(throwError(() =>
"dlfail"));
+ const errorSpy = vi.spyOn(console, "error").mockImplementation(() =>
{});
+
+ component.onClickOpenDownloadZip();
+
+
expect(downloadServiceSpy.downloadWorkflowsAsZip).toHaveBeenCalledWith([{ id:
1, name: "a" }]);
+ expect(errorSpy).toHaveBeenCalledWith("Error downloading workflows:",
"dlfail");
+ });
+ });
+
+ describe("workflow uploads", () => {
+ it("imports a valid .json workflow, appending an entry and toasting
success", async () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.createWorkflow =
vi.fn().mockReturnValue(of(makeDashboardWorkflow(42, "wf")));
+ const searchSpy = vi.spyOn(component,
"search").mockResolvedValue(undefined);
+ const successSpy = vi
+ .spyOn(TestBed.inject(NotificationService), "success")
+ .mockImplementation(() => undefined as any);
+ const content = testWorkflowContent([]);
+ const file = new File([JSON.stringify(content)], "wf.json");
+ setEntries([]);
+
+ const result = await
firstValueFrom(component.onClickUploadExistingWorkflowFromLocal(file as any));
+
+ expect(result).toBe(false);
+ expect(persist.createWorkflow).toHaveBeenCalledWith(content, "wf");
+ expect(component.searchResultsComponent.entries.map(e =>
e.name)).toContain("wf");
+ expect(searchSpy).toHaveBeenCalledWith(true);
+ expect(successSpy).toHaveBeenCalledWith("Upload Successful");
+ });
+
+ it("toasts an error and errors the stream when the file is not JSON",
async () => {
+ const errorSpy = vi
+ .spyOn(TestBed.inject(NotificationService), "error")
+ .mockImplementation(() => undefined as any);
+ const file = new File(["this is not json"], "bad.json");
+
+ await
expect(firstValueFrom(component.onClickUploadExistingWorkflowFromLocal(file as
any))).rejects.toThrow();
+
+ expect(errorSpy).toHaveBeenCalledWith(
+ "An error occurred when importing the workflow. Please import a
workflow json file."
+ );
+ });
+
+ it("falls back to DEFAULT_WORKFLOW_NAME when the stripped name is
empty", async () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.createWorkflow =
vi.fn().mockReturnValue(of(makeDashboardWorkflow(1, "x")));
+ vi.spyOn(component, "search").mockResolvedValue(undefined);
+ const content = testWorkflowContent([]);
+ const file = new File([JSON.stringify(content)], ".json");
+
+ await
firstValueFrom(component.onClickUploadExistingWorkflowFromLocal(file as any));
+
+ expect(persist.createWorkflow).toHaveBeenCalledWith(content,
DEFAULT_WORKFLOW_NAME);
+ });
+
+ // handleZipUploads is intentionally not covered here: its first
statement is
+ // `new JSZip()` against the `import * as JSZip from "jszip"` namespace,
which
+ // crashes under Vitest ("... is not a constructor"). The same
limitation is
+ // documented and skipped in download.service.spec.ts. nameWorkflow's
dedup
+ // logic is exercised below with a lightweight fake that mimics
`zip.files`.
Review Comment:
Went with option (1): covered the zip path for real. The `import * as JSZip;
new JSZip()` pattern only worked under webpack interop and throws under the
esbuild/Vitest runner, so I switched the component to `import JSZip from
"jszip"` (default import — works under both) and added a test that builds a
real 2-file JSZip archive and asserts `createWorkflow` is invoked once per
entry (c0add7f). #6357's zip-import goal is now genuinely met.
(`download.service.ts` still has the same `import *` pattern — pre-existing,
out of scope for this PR, and only a warning.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
