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-8133-eba468720500ab7a6d786dc4f59282e2c7a6f65f in repository https://gitbox.apache.org/repos/asf/texera.git
commit 7ba7b9865f741c652ec9af9eae80645387a377fd Author: Xinyuan Lin <[email protected]> AuthorDate: Mon Aug 31 19:16:18 2026 +0000 test(frontend): cover the share-access, card-item and list-item components (#8133) ### What changes were proposed in this PR? Five existing dashboard/workspace specs extended, 202 tests to 236. | File | Codecov lines | Branch arms | |---|---|---| | `share-access.component.ts` | 159/169 → **169/169** | 88/98 → **98/98** | | `share-access.component.html` | 66/73 → **73/73** | 10/10 | | `user-model.component.ts` | 79/82 → **82/82** | 38/42 → 40/42 | | `user-model.component.html` | 30/34 → **34/34** | 6/6 | | `user-workflow-list-item.component.ts` | 36/41 → **41/41** | 22/24 → **24/24** | | `user-workflow-list-item.component.html` | 68/71 → **71/71** | 4/4 | | `card-item.component.ts` | 181/190 → 188/190 | 95/102 → 100/102 | | `card-item.component.html` | 103/106 → 105/106 | 10/14 → 12/14 | | `workspace.component.ts` | 129/133 → 130/133 | 35/42 → 36/42 | **+42 fully-covered lines and +22 branch arms** (851/913 → 893/913). Six files reach 100%. The plain line-hit metric moves +21 — half the Codecov gain — because many of these lines were already executed and flip only by completing a branch arm; the two numbers are not interchangeable and both are given. ### The headline target contributes nothing, and that is the main finding `workspace.component.html` shows **0 of 14 lines** on Codecov and was the reason this bundle was picked. It stays at 0/14. `workspace.component.spec.ts:164` calls `TestBed.overrideComponent(WorkspaceComponent, { set: { imports: [], providers: [], schemas: [CUSTOM_ELEMENTS_SCHEMA] } })`. That forces a JIT recompile, so the istanbul-instrumented AOT template function is never executed — measured, not inferred: the spec runs 24 tests green and every one of the 15 statements in the template reads `hits=0`, along with the `@ViewChild` view-query and the `beforeunload` host-binding handler at `FNDA:0`. Unblocking it means providing each child's transitive dependencies. The eight child components declare **62 constructor dependencies** between them (workflow-editor 19, menu 22, result-panel 9), before their `providedIn: 'root'` services — JointJS paper init, Monaco, the websocket services. Overriding each *child* instead would keep the parent template instrumented but still construct each child class and run its `ngAfterViewInit` against DOM an emptied template does not have. `vitest.config.ts` already documents this suite OOM-ing workers at 2 GB (#7975). Refused as an accepted limitation rather than attempted. ### A trap that would have wasted the work Angular's generated listener source-spans **chain from the end of the previous listener attribute**, so a zero-hit line in an `.html` file is frequently not the handler written on that line. `card-item.component.html:105` reads as `(keydown.enter)="confirmUpdateCustomName(entry.name)"`, and an existing passing test already dispatches exactly that key event — yet the line showed zero. The `statementMap` explains it: the `keydown.enter` function is declared at `104:56` and is already covered; line 105's zero statement is the `(click)="$event.stopPropagation()"` handler. Every `.html` line in this PR was planned from `--coverage-reporters=json` and the `statementMap`/`fnMap`, never from the template source. ### Verification **50 semantic mutants, all 50 killed.** Each applied one at a time, with a sha256 snapshot check, an empty-production-diff check and a "no non-spec file modified" check before every compile, reverted from the scratch snapshot and re-verified after. The builder's 31-mutant table was re-derived from scratch rather than carried over, with every kill credit re-read from the run log. Two reviewers returned ten findings; all repaired. **The repair pass moved coverage by exactly zero** — its three new tests and ~15 new assertions exercise lines the bundle already reached. What changed is that those lines are now *pinned* rather than merely executed, and that is stated rather than sold as extra coverage. One further trap confirmed and worth carrying: the `DA` line set for a file depends on which specs are in the run, so the before and after measurements must use an identical `--include` filter. Both runs here used the same five spec files. `card-item.component.ts:310` sits inside a `setTimeout` arrow body — istanbul instruments arrow bodies, so unlike the Scala modules' `$anonfun` rule it does count, but the test must not call `detectChanges` after `onEditDescription()` or the `@ViewChild` resolves and the false arm is lost. It uses `fakeAsync` + `tick(0)`. `yarn format:ci` passes. `frontend/junit.xml` is regenerated by every run, is **not** gitignored, and is not committed. No production file is touched. ### Any related issues, documentation, discussions? Closes #8131 ### How was this PR tested? ``` npx ng test --watch=false --include="**/share-access.component.spec.ts" --include="**/card-item.component.spec.ts" --include="**/user-model.component.spec.ts" --include="**/user-workflow-list-item.component.spec.ts" --include="**/workspace.component.spec.ts" ``` ``` Test Files 5 passed (5) ``` Re-run after rebasing onto current `main`. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --- .../card-item/card-item.component.spec.ts | 121 ++++++++ .../share-access/share-access.component.spec.ts | 309 ++++++++++++++++++++- .../user/user-model/user-model.component.spec.ts | 80 +++++- .../user-workflow-list-item.component.spec.ts | 101 +++++++ .../component/workspace.component.spec.ts | 42 +++ 5 files changed, 649 insertions(+), 4 deletions(-) diff --git a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts index 65376a7909..e1410a6b0b 100644 --- a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts @@ -1032,5 +1032,126 @@ describe("CardItemComponent", () => { fire(".card-preview-image", "error", {}); expect(errorSpy).toHaveBeenCalled(); }); + + it("writes what was typed in the name editor back onto the entry", () => { + // The editor is seeded from entry.name; with a one-way binding it would look right on screen + // while the confirmed rename kept sending the name the card started with. + const entry = makeWorkflowEntry({ name: "before" }); + component.entry = entry; + component.isPrivateSearch = true; + component.editingName = true; + fixture.detectChanges(); + + const input = fixture.debugElement.query(By.css(".resource-name-edit-input")); + expect(input).toBeTruthy(); + input.triggerEventHandler("ngModelChange", "after"); + + expect(entry.name).toBe("after"); + }); + + it("keeps a click inside the name editor from opening the card", () => { + // The whole header is a routerLink, so without stopPropagation every click meant for the + // caret would navigate away mid-rename. + component.entry = makeWorkflowEntry(); + component.isPrivateSearch = true; + component.editingName = true; + fixture.detectChanges(); + + const header = fixture.debugElement.query(By.css(".card-header")).nativeElement as HTMLElement; + const reachedHeader = vi.fn(); + header.addEventListener("click", reachedHeader); + + const input = fixture.debugElement.query(By.css(".resource-name-edit-input")).nativeElement as HTMLInputElement; + input.dispatchEvent(new MouseEvent("click", { bubbles: true })); + + expect(reachedHeader).not.toHaveBeenCalled(); + }); + }); + + describe("guard paths", () => { + /** Files are the one registered kind with neither a rename nor a description endpoint. */ + function makeFileEntry(overrides: Partial<DashboardEntry> = {}): DashboardEntry { + return { + id: 3, + name: "notes.txt", + description: "", + type: EntityType.File, + accessibleUserIds: [], + likeCount: 0, + viewCount: 0, + isLiked: false, + size: 0, + ...overrides, + } as unknown as DashboardEntry; + } + + it("ngOnChanges ignores a change set that does not carry the entry", () => { + // initializeEntry resets the cover and the counters; re-running it on an unrelated input + // change would discard a cover that had just finished loading. + const initialize = vi.spyOn(component, "initializeEntry"); + + component.ngOnChanges({ currentUid: { currentValue: 3 } as any }); + + expect(initialize).not.toHaveBeenCalled(); + }); + + it("onEditDescription tolerates a textarea that has not rendered yet", fakeAsync(() => { + // The caret is placed in a timer callback, which can outlive the element it was queued for. + component.entry = makeWorkflowEntry({ description: "some text" }); + component.descriptionInput = undefined as any; + + component.onEditDescription(); + + expect(component.editingDescription).toBe(true); + expect(() => tick(0)).not.toThrow(); + })); + + it("does not attempt a rename for a kind that has no rename endpoint", () => { + // Whatever the fixture types stays typed no matter which branch runs, so the editor state is + // what separates this early return from the two that close the editor. The rename endpoint is + // mocked so a regression fails on the assertion rather than on a TypeError out of + // updateProperty. + workflowPersistService.updateWorkflowName.mockReturnValue(of({} as Response)); + component.entry = makeFileEntry({ name: "typed" }); + component.originalName = "notes.txt"; + component.editingName = true; + + component.confirmUpdateCustomName("typed"); + + expect(workflowPersistService.updateWorkflowName).not.toHaveBeenCalled(); + expect(datasetService.updateDatasetName).not.toHaveBeenCalled(); + expect(component.editingName).toBe(true); + }); + + it("does not attempt a description update for a kind that has no description endpoint", () => { + // Mocked so that a regression here fails on the assertion below rather than on a TypeError + // thrown out of updateProperty. + workflowPersistService.updateWorkflowDescription.mockReturnValue(of({} as Response)); + component.entry = makeFileEntry({ description: "typed" }); + component.originalDescription = ""; + component.editingDescription = true; + + component.confirmUpdateCustomDescription("typed"); + + expect(workflowPersistService.updateWorkflowDescription).not.toHaveBeenCalled(); + expect(component.editingDescription).toBe(true); + }); + + it("toggleLike leaves the liked state and the count alone when the unlike reports failure", () => { + const hubService = TestBed.inject(HubService); + vi.spyOn(hubService, "postUnlike").mockReturnValue(of(false)); + const getCountsSpy = vi.spyOn(hubService, "getCounts"); + + component.currentUid = 42; + component.entry = makeWorkflowEntry({ id: 7 }); + component.isLiked = true; + component.likeCount = 5; + + component.toggleLike(); + + expect(component.isLiked).toBe(true); + expect(component.likeCount).toBe(5); + expect(getCountsSpy).not.toHaveBeenCalled(); + }); }); }); diff --git a/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts b/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts index d23510bb6d..d11be498a2 100644 --- a/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts @@ -17,9 +17,10 @@ * under the License. */ -import { TestBed } from "@angular/core/testing"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; import { HttpClientTestingModule } from "@angular/common/http/testing"; import { NoopAnimationsModule } from "@angular/platform-browser/animations"; +import { By } from "@angular/platform-browser"; import { HttpErrorResponse } from "@angular/common/http"; import { of, throwError } from "rxjs"; @@ -66,6 +67,14 @@ describe("ShareAccessComponent", () => { let workflowActionSpy: { setWorkflowIsPublished: ReturnType<typeof vi.fn> }; let userServiceCurrentEmail: string | undefined; let capturedModalConfigs: any[]; + /** The NzModalRef stubs handed back by modalService.create, in creation order. */ + let capturedModalRefs: { close: ReturnType<typeof vi.fn> }[]; + /** + * The fixture built by the most recent setupComponent() call, for the template-level tests. + * Cleared in beforeEach: a test that reads it without having built one then fails on the spot + * rather than silently querying the previous test's detached DOM. + */ + let fixture: ComponentFixture<ShareAccessComponent>; function setupComponent(opts: SetupOptions = {}): ShareAccessComponent { const { type = "workflow", id = 1, inWorkspace = false, currentEmail = "[email protected]" } = opts; @@ -92,14 +101,16 @@ describe("ShareAccessComponent", () => { { provide: WorkflowActionService, useValue: workflowActionSpy }, ], }); - const fixture = TestBed.createComponent(ShareAccessComponent); + fixture = TestBed.createComponent(ShareAccessComponent); fixture.detectChanges(); return fixture.componentInstance; } beforeEach(() => { TestBed.resetTestingModule(); + fixture = undefined as unknown as ComponentFixture<ShareAccessComponent>; capturedModalConfigs = []; + capturedModalRefs = []; gmailSpy = { sendEmail: vi.fn() }; accessServiceSpy = { grantAccess: vi.fn().mockReturnValue(of(null)), @@ -113,7 +124,9 @@ describe("ShareAccessComponent", () => { modalServiceSpy = { create: vi.fn().mockImplementation((config: any) => { capturedModalConfigs.push(config); - return { close: vi.fn() }; + const ref = { close: vi.fn() }; + capturedModalRefs.push(ref); + return ref; }), }; workflowPersistSpy = { @@ -711,4 +724,294 @@ describe("ShareAccessComponent", () => { expect(datasetServiceSpy.updateDatasetPublicity).not.toHaveBeenCalled(); }); }); + + /** + * Everything above drives the component's methods directly. The template decides which control + * reaches which of those methods, and with what argument — the publish pair and the per-row + * access controls are near-symmetric, so a crossed binding would look right on screen and do the + * opposite thing. + */ + describe("template wiring", () => { + /** Makes the current user the owner, which is what enables the write-gated controls. */ + function asOwner(): void { + accessServiceSpy.getOwner.mockReturnValue(of("[email protected]")); + } + + /** + * The publish pair, addressed by the label the user reads rather than by DOM position. That is + * the mapping under test: a crossed (click) handler still fails, while reordering the two + * buttons no longer fails *this* test. Document order is a separate contract and is pinned by + * its own test below, so addressing by label gives up nothing. The uniqueness check is what + * keeps a dropped or duplicated button from reading as a pass. + */ + function accessButton(label: "Private" | "Public"): HTMLButtonElement { + const matches = fixture.debugElement + .queryAll(By.css("button.access-button")) + .filter( + button => + (button.nativeElement as HTMLElement).querySelector(".button-text-header")?.textContent?.trim() === label + ); + expect(matches).toHaveLength(1); + return matches[0].nativeElement as HTMLButtonElement; + } + + it("offers the restrictive option first: Private, then Public", () => { + asOwner(); + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Private")); + setupComponent({ type: "workflow" }); + + // accessButton() is deliberately blind to order so the two mapping tests below fail only + // for a crossed binding. Order is still a contract of its own — this pair is how the user + // is asked to think about visibility, and the narrower choice is presented first — so it is + // pinned here explicitly rather than riding along as a side effect of a positional + // destructure, where a reorder and a crossed handler were indistinguishable. + const labels = fixture.debugElement + .queryAll(By.css("button.access-button .button-text-header")) + .map(header => (header.nativeElement as HTMLElement).textContent?.trim()); + expect(labels).toEqual(["Private", "Public"]); + }); + + it("puts the unpublish confirmation behind Private and the publish confirmation behind Public", () => { + asOwner(); + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Private")); + setupComponent({ type: "workflow" }); + + // Already private, so Private has nothing to confirm. + accessButton("Private").click(); + expect(modalServiceSpy.create).not.toHaveBeenCalled(); + + accessButton("Public").click(); + expect(capturedModalConfigs).toHaveLength(1); + expect(capturedModalConfigs[0].nzContent).toContain("Publishing your workflow"); + }); + + it("keeps that pairing when the workflow is already public", () => { + asOwner(); + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + setupComponent({ type: "workflow" }); + + // Already public, so Public has nothing to confirm. + accessButton("Public").click(); + expect(modalServiceSpy.create).not.toHaveBeenCalled(); + + accessButton("Private").click(); + expect(capturedModalConfigs).toHaveLength(1); + expect(capturedModalConfigs[0].nzContent).toContain("lose access to your workflow"); + }); + + it("renders one closable tag per queued email and drops only the tag that was closed", () => { + const c = setupComponent(); + c.emailTags = ["[email protected]", "[email protected]"]; + fixture.detectChanges(); + + // Scoped to the form: the access list below it carries an OWNER tag of its own. + const tags = fixture.debugElement.queryAll(By.css("form nz-tag")); + expect(tags.map(tag => (tag.nativeElement as HTMLElement).textContent?.trim())).toEqual([ + "[email protected]", + "[email protected]", + ]); + + // nzMode drives whether nz-tag emits a close control at all; firing nzOnClose by hand would + // pass just as well against a tag the user can never dismiss. + tags.forEach(tag => expect((tag.nativeElement as HTMLElement).querySelector(".ant-tag-close-icon")).toBeTruthy()); + + // Closing the second tag must not take the first one with it. + tags[1].triggerEventHandler("nzOnClose", new MouseEvent("click")); + + expect(c.emailTags).toEqual(["[email protected]"]); + }); + + it("applies a level change and a revoke to the row they were issued from", () => { + asOwner(); + accessServiceSpy.getAccessList.mockReturnValue( + of([{ email: "[email protected]", name: "Other", privilege: Privilege.READ }]) + ); + setupComponent({ type: "workflow", id: 3 }); + accessServiceSpy.grantAccess.mockClear(); + + const select = fixture.debugElement.query(By.css("ul.current-share select")).nativeElement as HTMLSelectElement; + expect(select.value).toBe("READ"); + // dispatchEvent fires listeners on a disabled control too, so the gate has to be read off + // the element rather than inferred from the call going through. + expect(select.disabled).toBe(false); + select.value = "WRITE"; + select.dispatchEvent(new Event("change")); + + // The email and the new privilege must not swap places. + expect(accessServiceSpy.grantAccess).toHaveBeenCalledWith("workflow", 3, "[email protected]", "WRITE"); + + const revoke = fixture.debugElement.query(By.css("ul.current-share li button")) + .nativeElement as HTMLButtonElement; + revoke.click(); + + expect(capturedModalConfigs).toHaveLength(1); + expect(capturedModalConfigs[0].nzContent).toContain("revoke [email protected]'s access"); + }); + + it("leaves every write-gated control live for the owner", () => { + asOwner(); + setupComponent({ type: "workflow" }); + + const buttons = fixture.debugElement.queryAll(By.css("button.access-button")); + expect(buttons.map(b => (b.nativeElement as HTMLButtonElement).disabled)).toEqual([false, false]); + const submit = fixture.debugElement.query(By.css('form button[type="submit"]')) + .nativeElement as HTMLButtonElement; + expect(submit.disabled).toBe(false); + }); + + it("locks the write-gated controls for a read-only viewer, except the one that drops their own access", () => { + accessServiceSpy.getOwner.mockReturnValue(of("[email protected]")); + accessServiceSpy.getAccessList.mockReturnValue( + of([ + { email: "[email protected]", name: "Me", privilege: Privilege.READ }, + { email: "[email protected]", name: "Other", privilege: Privilege.READ }, + ]) + ); + const c = setupComponent({ type: "workflow", currentEmail: "[email protected]" }); + expect(c.hasWriteAccess).toBe(false); + + const buttons = fixture.debugElement.queryAll(By.css("button.access-button")); + expect(buttons.map(b => (b.nativeElement as HTMLButtonElement).disabled)).toEqual([true, true]); + const submit = fixture.debugElement.query(By.css('form button[type="submit"]')) + .nativeElement as HTMLButtonElement; + expect(submit.disabled).toBe(true); + + const selects = fixture.debugElement.queryAll(By.css("ul.current-share li select")); + expect(selects.map(s => (s.nativeElement as HTMLSelectElement).disabled)).toEqual([true, true]); + + // Leaving a resource you can only read is still yours to do, so your own row keeps its + // revoke button live while everybody else's is locked. + const revokes = fixture.debugElement.queryAll(By.css("ul.current-share li button")); + expect(revokes.map(b => (b.nativeElement as HTMLButtonElement).disabled)).toEqual([false, true]); + }); + }); + + /** + * Only HttpErrorResponse carries the `error.message` these handlers read, so every subscription + * narrows before touching it. A transport-level failure therefore reaches the user as nothing at + * all — pinned here so the guard cannot be dropped, which would turn each of these into a + * TypeError thrown out of the error callback. + */ + describe("non-HTTP failures", () => { + const offline = () => throwError(() => new Error("offline")); + + it("sharing: no notification either way", () => { + accessServiceSpy.grantAccess.mockReturnValue(offline()); + const c = setupComponent({ type: "workflow", id: 5 }); + c.emailTags = ["[email protected]"]; + + c.grantAccess(); + + expect(notificationSpy.error).not.toHaveBeenCalled(); + expect(notificationSpy.success).not.toHaveBeenCalled(); + }); + + it("revoking: no notification, and the modal still closes", () => { + accessServiceSpy.revokeAccess.mockReturnValue(offline()); + const c = setupComponent({ currentEmail: "[email protected]" }); + + c.verifyRevokeAccess("[email protected]"); + getFooterButton(capturedModalConfigs[0], "Revoke").onClick(); + + expect(accessServiceSpy.revokeAccess).toHaveBeenCalled(); + expect(notificationSpy.error).not.toHaveBeenCalled(); + // The confirmation is dismissed by the button itself, not by the response, so a failed + // revoke must not leave the dialog open over the list. + expect(capturedModalRefs[0].close).toHaveBeenCalled(); + }); + + it("changing a level: no notification, but the list is still reloaded", () => { + accessServiceSpy.grantAccess.mockReturnValue(offline()); + const c = setupComponent({ currentEmail: "[email protected]", type: "workflow", id: 3 }); + accessServiceSpy.getAccessList.mockClear(); + + c.changeAccessLevel("[email protected]", "READ"); + + expect(notificationSpy.error).not.toHaveBeenCalled(); + expect(accessServiceSpy.getAccessList).toHaveBeenCalledWith("workflow", 3); + }); + + it("publishing a workflow: no notification, and it stays private", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Private")); + workflowPersistSpy.updateWorkflowIsPublished.mockReturnValue(offline()); + const c = setupComponent({ type: "workflow" }); + + c.publishWorkflow(); + + expect(workflowPersistSpy.updateWorkflowIsPublished).toHaveBeenCalledWith(1, true); + expect(notificationSpy.error).not.toHaveBeenCalled(); + expect(c.isPublic).toBe(false); + }); + + it("unpublishing a workflow: no notification, and it stays public", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.updateWorkflowIsPublished.mockReturnValue(offline()); + const c = setupComponent({ type: "workflow" }); + + c.unpublishWorkflow(); + + expect(workflowPersistSpy.updateWorkflowIsPublished).toHaveBeenCalledWith(1, false); + expect(notificationSpy.error).not.toHaveBeenCalled(); + expect(c.isPublic).toBe(true); + }); + + it("publishing a dataset: no notification, and it stays private", () => { + datasetServiceSpy.getDataset.mockReturnValue(of({ dataset: { isPublic: false } })); + datasetServiceSpy.updateDatasetPublicity.mockReturnValue(offline()); + const c = setupComponent({ type: "dataset" }); + + c.publishDataset(); + + expect(datasetServiceSpy.updateDatasetPublicity).toHaveBeenCalledWith(1); + expect(notificationSpy.error).not.toHaveBeenCalled(); + expect(c.isPublic).toBe(false); + }); + + it("unpublishing a dataset: no notification, and it stays public", () => { + datasetServiceSpy.getDataset.mockReturnValue(of({ dataset: { isPublic: true } })); + datasetServiceSpy.updateDatasetPublicity.mockReturnValue(offline()); + const c = setupComponent({ type: "dataset" }); + + c.unpublishDataset(); + + expect(datasetServiceSpy.updateDatasetPublicity).toHaveBeenCalledWith(1); + expect(notificationSpy.error).not.toHaveBeenCalled(); + expect(c.isPublic).toBe(true); + }); + }); + + describe("confirmation for kinds that carry no publicity", () => { + it("unpublishing a workflow outside the workspace leaves the canvas state alone", () => { + // setWorkflowIsPublished only exists to keep an open editor in step; there is no editor here. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + const c = setupComponent({ type: "workflow", id: 8, inWorkspace: false }); + + c.verifyUnpublish(); + getFooterButton(capturedModalConfigs[0], "Unpublish").onClick(); + + expect(workflowPersistSpy.updateWorkflowIsPublished).toHaveBeenCalledWith(8, false); + expect(workflowActionSpy.setWorkflowIsPublished).not.toHaveBeenCalled(); + }); + + it("confirming Publish on a kind with no publish endpoint does nothing", () => { + const c = setupComponent({ type: "computing-unit", id: 4 }); + + c.verifyPublish(); + getFooterButton(capturedModalConfigs[0], "Publish").onClick(); + + expect(workflowPersistSpy.updateWorkflowIsPublished).not.toHaveBeenCalled(); + expect(datasetServiceSpy.updateDatasetPublicity).not.toHaveBeenCalled(); + }); + + it("confirming Unpublish on a kind with no publish endpoint does nothing", () => { + const c = setupComponent({ type: "computing-unit", id: 4 }); + c.isPublic = true; + + c.verifyUnpublish(); + getFooterButton(capturedModalConfigs[0], "Unpublish").onClick(); + + expect(workflowPersistSpy.updateWorkflowIsPublished).not.toHaveBeenCalled(); + expect(datasetServiceSpy.updateDatasetPublicity).not.toHaveBeenCalled(); + }); + }); }); diff --git a/frontend/src/app/dashboard/component/user/user-model/user-model.component.spec.ts b/frontend/src/app/dashboard/component/user/user-model/user-model.component.spec.ts index 2b7b28b617..f41e1635f1 100644 --- a/frontend/src/app/dashboard/component/user/user-model/user-model.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-model/user-model.component.spec.ts @@ -25,6 +25,7 @@ import { NzModalService } from "ng-zorro-antd/modal"; import { en_US, NZ_I18N } from "ng-zorro-antd/i18n"; import { commonTestImports, commonTestProviders } from "../../../../common/testing/test-utils"; import { SearchResultsComponent } from "../search-results/search-results.component"; +import { CardItemComponent } from "../list-item/card-item/card-item.component"; import { NgModel } from "@angular/forms"; import { UserService } from "../../../../common/service/user/user.service"; import { StubUserService } from "../../../../common/service/user/stub-user.service"; @@ -245,7 +246,7 @@ describe("UserModelComponent", () => { // ─── owner names ────────────────────────────────────────────────────────── - it("labels each card with its owner's display name", async () => { + it("labels each card with its owner's display name and avatar", async () => { modelServiceMock.retrieveAccessibleModels.mockReturnValue(of([listedModel(1)])); searchServiceMock.getUserInfo.mockReturnValue(of({ 1: { userName: "ada", avatar: "a.png" } })); @@ -254,6 +255,22 @@ describe("UserModelComponent", () => { expect(searchServiceMock.getUserInfo).toHaveBeenCalledWith([1]); expect(entry.ownerName).toBe("ada"); + // DashboardEntry starts every entry with an empty avatar, so only a lookup that actually + // forwards the value can produce this — asserting "" alone would pass with the write removed. + expect(entry.ownerAvatar).toBe("a.png"); + }); + + it("leaves the avatar empty for an owner who has not set one", async () => { + // getUserInfo omits `avatar` for such a user; forwarding undefined would put the string + // "undefined" in the avatar slot. + modelServiceMock.retrieveAccessibleModels.mockReturnValue(of([listedModel(1)])); + searchServiceMock.getUserInfo.mockReturnValue(of({ 1: { userName: "ada" } })); + + await component.search(); + const [entry] = (await capturedLoadMoreFn!(0, 20)).entries; + + expect(entry.ownerName).toBe("ada"); + expect(entry.ownerAvatar).toBe(""); }); it("still lists the models when the owner lookup fails", async () => { @@ -434,4 +451,65 @@ describe("UserModelComponent rendering", () => { expect(results.isPrivateSearch).toBe(true); expect(results.currentUid).toBe(component.currentUid); }); + + /** A /model/list row, in the shape DashboardEntry accepts. */ + function modelEntry(mid = 3, name = "resnet"): DashboardEntry { + return new DashboardEntry({ + isOwner: true, + ownerEmail: "[email protected]", + accessPrivilege: "WRITE", + size: 0, + model: { + mid, + ownerUid: 1, + name, + description: "", + framework: "pytorch", + format: "torchscript", + creationTime: mid, + isPublic: false, + isDownloadable: false, + }, + } as any); + } + + it("renders one card per model through the card template, with its outputs kept apart", () => { + // The card template is handed to the results list rather than instantiated here, so nothing + // else in this suite proves the entry reaches the card or that `deleted` and `refresh` — two + // adjacent outputs, one of them destructive — are not crossed. + const deleteModel = vi.spyOn(component, "deleteModel").mockImplementation(() => {}); + const search = vi.spyOn(component, "search").mockResolvedValue(); + const entry = modelEntry(); + const other = modelEntry(4, "bert"); + // A distinct viewer id: the card navigates by it and gates liking on it, so a template that + // dropped the binding would still render and would fail silently at runtime. + component.currentUid = 42; + + component.searchResultsComponent.entries = [entry, other]; + fixture.detectChanges(); + + const cards = fixture.debugElement.queryAll(By.directive(CardItemComponent)); + expect(cards, "the card template did not render one card per model").toHaveLength(2); + const card = cards[0]; + expect(card.componentInstance.entry).toBe(entry); + expect(cards[1].componentInstance.entry).toBe(other); + expect(card.componentInstance.currentUid).toBe(42); + expect(card.componentInstance.isPrivateSearch).toBe(true); + expect(card.componentInstance.editable).toBe(true); + + card.triggerEventHandler("deleted", undefined); + expect(deleteModel).toHaveBeenCalledWith(entry); + expect(search).not.toHaveBeenCalled(); + + card.triggerEventHandler("refresh", undefined); + expect(search).toHaveBeenCalledWith(true); + }); + + it("refuses to search before the results list has been queried", () => { + // search() reaches straight through the getter; without the guard the failure would surface + // as "cannot read reset of undefined" from inside an async callback. + const bare = TestBed.createComponent(UserModelComponent).componentInstance; + + expect(() => bare.searchResultsComponent).toThrowError("Property cannot be accessed before it is initialized."); + }); }); diff --git a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow-list-item/user-workflow-list-item.component.spec.ts b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow-list-item/user-workflow-list-item.component.spec.ts index 03712fd088..ad3d296445 100644 --- a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow-list-item/user-workflow-list-item.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow-list-item/user-workflow-list-item.component.spec.ts @@ -30,6 +30,7 @@ import { } from "../../../../../common/service/workflow-persist/workflow-persist.service"; import { DownloadService } from "../../../../service/user/download/download.service"; import { WorkflowExecutionHistoryComponent } from "../ngbd-modal-workflow-executions/workflow-execution-history.component"; +import { ShareAccessComponent } from "../../share-access/share-access.component"; import { Workflow } from "../../../../../common/type/workflow"; import { of } from "rxjs"; import { NzListComponent } from "ng-zorro-antd/list"; @@ -256,6 +257,64 @@ describe("UserWorkflowListItemComponent", () => { expect(spy).not.toHaveBeenCalled(); }); }); + + it("opens the share modal for this row's workflow, carrying its write access and the owner list", async () => { + const entry = makeWorkflowEntry({ wid: 21, name: "wf" }); + entry.workflow.accessLevel = "WRITE"; + component.entry = entry; + const persist = TestBed.inject(WorkflowPersistService); + // The owner list is the autocomplete the dialog exists to offer; it is the one awaited + // value in the method, so nothing else proves it is resolved before the modal opens. + vi.spyOn(persist, "retrieveOwners").mockReturnValue(of(["[email protected]", "[email protected]"])); + const modal = TestBed.inject(NzModalService); + const spy = vi.spyOn(modal, "create").mockReturnValue({} as any); + + await component.onClickOpenShareAccess(); + + expect(spy).toHaveBeenCalledWith( + expect.objectContaining({ + nzContent: ShareAccessComponent, + nzData: expect.objectContaining({ + writeAccess: true, + type: "workflow", + id: 21, + allOwners: ["[email protected]", "[email protected]"], + }), + }) + ); + }); + + it("marks the share modal read-only for a row the viewer can only read", async () => { + const entry = makeWorkflowEntry({ wid: 21 }); + entry.workflow.accessLevel = "READ"; + component.entry = entry; + const modal = TestBed.inject(NzModalService); + const spy = vi.spyOn(modal, "create").mockReturnValue({} as any); + + await component.onClickOpenShareAccess(); + + expect(spy).toHaveBeenCalledWith( + expect.objectContaining({ nzData: expect.objectContaining({ writeAccess: false }) }) + ); + }); + + describe("mis-wired inputs", () => { + // Both accessors are read from the template on every change-detection pass, so a silent + // undefined would surface as an unrelated crash deep in ng-zorro instead of here. + it("refuses to read the entry before one has been provided", () => { + component.entry = undefined as any; + + expect(() => component.entry).toThrowError("entry property must be provided to UserWorkflowListItemComponent."); + }); + + it("refuses to read a workflow off an entry that carries no workflow payload", () => { + // The guard tests for the payload, not for entry.type: an entry of any kind that does + // carry a workflow passes it, so this must not claim to be a kind check. + component.entry = { name: "ds" } as unknown as DashboardEntry; + + expect(() => component.workflow).toThrowError(/Entry must be workflow/); + }); + }); }); function sendInput(editableDescriptionInput: HTMLInputElement, text: string) { @@ -327,6 +386,17 @@ describe("UserWorkflowListItemComponent rendering", () => { .map(d => d.nativeElement as HTMLElement); } + /** + * The one element whose tooltip title satisfies the predicate. Indexing byTooltip() directly + * reports a missing or duplicated action as a TypeError on `undefined.click()` — or, worse, + * silently clicks the first of several; asserting the match is unique names the real problem. + */ + function onlyByTooltip(pred: (title: string) => boolean): HTMLElement { + const matches = byTooltip(pred); + expect(matches).toHaveLength(1); + return matches[0]; + } + beforeEach(async () => { await setup(); }); @@ -434,6 +504,37 @@ describe("UserWorkflowListItemComponent rendering", () => { expect(deleted).toHaveBeenCalledTimes(1); expect(duplicated).toHaveBeenCalledTimes(1); }); + + it("opens the executions modal from the history action", async () => { + await setup({ executionsTracking: true }); + render(makeWorkflowEntry({ wid: 11, name: "wf" })); + const modal = TestBed.inject(NzModalService); + const create = vi.spyOn(modal, "create").mockReturnValue({} as any); + + onlyByTooltip(t => t.startsWith("Executions of the workflow")).click(); + + expect(create).toHaveBeenCalledWith( + expect.objectContaining({ nzContent: WorkflowExecutionHistoryComponent, nzData: { wid: 11 } }) + ); + }); + }); + + describe("inline name and description editors", () => { + it("puts name editing behind the pencil and description editing behind the plus", () => { + // Two adjacent icon buttons on the same toolbar; swapping them would open the wrong editor. + render(makeWorkflowEntry()); + expect(component.editingName).toBe(false); + expect(component.editingDescription).toBe(false); + + onlyByTooltip(t => t === "Customize Workflow Name").click(); + + expect(component.editingName).toBe(true); + expect(component.editingDescription).toBe(false); + + onlyByTooltip(t => t === "Add Description").click(); + + expect(component.editingDescription).toBe(true); + }); }); describe("selection checkbox", () => { diff --git a/frontend/src/app/workspace/component/workspace.component.spec.ts b/frontend/src/app/workspace/component/workspace.component.spec.ts index 9bfd46386e..839358b854 100644 --- a/frontend/src/app/workspace/component/workspace.component.spec.ts +++ b/frontend/src/app/workspace/component/workspace.component.spec.ts @@ -380,6 +380,48 @@ describe("WorkspaceComponent", () => { vi.useRealTimers(); } }); + + it("does not persist an edit made by a signed-out visitor", async () => { + // A guest can still edit the canvas; persisting on their behalf would write to whatever + // workflow id the URL happens to carry. + vi.useFakeTimers(); + try { + const workflowChanged$ = new Subject<void>(); + await createFixture(); + workflowActionService.workflowChanged.mockReturnValue(workflowChanged$.asObservable()); + userService.isLogin.mockReturnValue(false); + workflowPersistService.isWorkflowPersistEnabled.mockReturnValue(true); + + component.registerAutoPersistWorkflow(); + workflowChanged$.next(); + vi.advanceTimersByTime(5000); + + expect(workflowPersistService.persistWorkflow).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + + it("does not persist when workflow persistence is switched off", async () => { + // The other half of the same guard. A deployment can turn persistence off, and while it is + // off a signed-in user's edits must not be written back either. + vi.useFakeTimers(); + try { + const workflowChanged$ = new Subject<void>(); + await createFixture(); + workflowActionService.workflowChanged.mockReturnValue(workflowChanged$.asObservable()); + userService.isLogin.mockReturnValue(true); + workflowPersistService.isWorkflowPersistEnabled.mockReturnValue(false); + + component.registerAutoPersistWorkflow(); + workflowChanged$.next(); + vi.advanceTimersByTime(5000); + + expect(workflowPersistService.persistWorkflow).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); }); describe("updateViewCount", () => {
