You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@texera.apache.org by gi...@apache.org on 2026/08/31 19:32:40 UTC
(texera) 01/01: test(frontend): cover the share-access, card-item and list-item components (#8133)
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 <xi...@uci.edu>
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 = "me@example.com" } = 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("me@example.com"));
+ }
+
+ /**
+ * 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 = ["first@example.com", "second@example.com"];
+ 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([
+ "first@example.com",
+ "second@example.com",
+ ]);
+
+ // 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(["first@example.com"]);
+ });
+
+ it("applies a level change and a revoke to the row they were issued from", () => {
+ asOwner();
+ accessServiceSpy.getAccessList.mockReturnValue(
+ of([{ email: "other@example.com", 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, "other@example.com", "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 other@example.com'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("owner@example.com"));
+ accessServiceSpy.getAccessList.mockReturnValue(
+ of([
+ { email: "me@example.com", name: "Me", privilege: Privilege.READ },
+ { email: "other@example.com", name: "Other", privilege: Privilege.READ },
+ ])
+ );
+ const c = setupComponent({ type: "workflow", currentEmail: "me@example.com" });
+ 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 = ["a@example.com"];
+
+ 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: "me@example.com" });
+
+ c.verifyRevokeAccess("other@example.com");
+ 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: "me@example.com", type: "workflow", id: 3 });
+ accessServiceSpy.getAccessList.mockClear();
+
+ c.changeAccessLevel("other@example.com", "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: "owner@example.com",
+ 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(["a@example.com", "b@example.com"]));
+ 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: ["a@example.com", "b@example.com"],
+ }),
+ })
+ );
+ });
+
+ 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", () => {