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-8381-5de83017defe685cc29703eb37628383fbbf3bfb in repository https://gitbox.apache.org/repos/asf/texera.git
commit 1cbe857007a6526c6087f187667ef3b3c9ade546 Author: Tanishq Gandhi <[email protected]> AuthorDate: Thu Sep 3 22:36:44 2026 +0000 feat(frontend): render an absent fact as an em dash and a size as 0 B everywhere (#8381) ### What changes were proposed in this PR? A resource's size, and the facts it has none of, rendered four different ways depending on where you looked. This settles on one convention: **a fact with nothing to show renders "—", and a size renders `0 B`**, because zero bytes is a meaningful answer. **Card view hid the size row; list view showed `0 B`.** A dataset created but never uploaded to showed no size line at all in card view (`*ngIf="size"` hides a zero) and `Size: 0 B` in list view. Card view now always renders the row, so both views agree. **A copied workflow claimed `Size: 0 B`.** Only search asks the backend for workflow sizes (`SearchService` → `GET /workflow/size`), so a `DashboardEntry` built straight from a duplicate response carried no size and rendered as an empty workflow beside correctly-sized siblings. Both duplicate paths now fetch the sizes before the rows go on screen — the list items read the size once on binding, so filling it in afterwards would not show. **The two detail cards disagreed with each other.** The dataset Data Card left "Last updated" and "Latest version file" blank; the model Model Card em-dashed them *and* its size. Now both pages em-dash the version facts and both render `0 B` for the size. `formatSize` is deliberately unchanged. This is a `feat` rather than a `fix`: it is a deliberate, user-facing change to how these fields render, and it touches the model detail page, which does not exist on the release branches — so it should not be backported. **Before** — no size line in card view, `0 B` in list view, a fresh copy claiming `0 B`, and the two detail cards disagreeing: <img width="1440" height="900" alt="issue4-1-datasets-card-view-before" src="https://github.com/user-attachments/assets/5afd125b-f1e3-49b6-b782-6c0657773e9e" /> <img width="1440" height="900" alt="issue4-2-datasets-list-view-before" src="https://github.com/user-attachments/assets/edcca435-c936-4049-8a0b-65d43dfabf71" /> <img width="1440" height="900" alt="issue4-7-duplicated-workflow-before" src="https://github.com/user-attachments/assets/04e02e80-d232-4dec-99bc-35126c4db916" /> <img width="1440" height="900" alt="issue4-3-empty-dataset-stats-before" src="https://github.com/user-attachments/assets/90a00ca4-11a7-428c-88b4-7024899090eb" /> **After** — card view reads `0 B` like list view, the copy reports its real size, and both detail cards read the same: <img width="1440" height="900" alt="issue4-1-datasets-card-view-after" src="https://github.com/user-attachments/assets/2d7f4121-b52e-4269-b773-5ba6012e1961" /> <img width="1440" height="900" alt="issue4-7-duplicated-workflow-after" src="https://github.com/user-attachments/assets/2520ff1a-fbb8-4211-b616-32b7086a3946" /> <img width="1440" height="900" alt="issue4-3-empty-dataset-stats-after" src="https://github.com/user-attachments/assets/ace12205-b959-4658-b568-d9be8a89fcef" /> <img width="1440" height="900" alt="issue4-4-empty-model-stats-after" src="https://github.com/user-attachments/assets/496ecdf4-d6d1-459f-9922-c70c6da9b38a" /> ### Any related issues, documentation, discussions? Closes #8380. ### How was this PR tested? Specs added: - `card-item.component.spec.ts` — `still reports an empty resource's size, so card and list view agree`, asserting the row renders and reads `0 B` at size 0. 80 passed. - `user-workflow.component.spec.ts` — `asks for the copy's size, which the duplicate response does not carry`, asserting `getSizes` is called with the new wid and the entry takes that size. 74 passed. - `dataset-detail.component.spec.ts` — `em-dashes the facts a dataset with no versions has none of` and `shows the real facts once a version exists`, covering both legs of each stat. 144 passed. - `model-detail.component.spec.ts` — the same case for the model card, pinning "—" for the version facts and `0 B` for the size. 80 passed. - `StubWorkflowPersistService` gained a `getSizes` so the duplicate specs exercise the new call. One existing model spec, `dashes out the latest-version facts for a model with no versions`, asserted the card contained no `0 B` — it pinned the old model-only convention and is superseded by the new per-field case, so it was removed rather than edited. ``` cd frontend npx ng test --include src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts npx ng test --include src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts npx ng test --include src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.spec.ts npx ng test --include src/app/dashboard/component/user/user-model/user-model-explorer/model-detail.component.spec.ts ``` Checked by hand against a local stack: an empty dataset reads `0 B` in both views, the dataset and model cards both show `—` for the version facts and `0 B` for the size, and a freshly copied workflow reports 109.00 B like its siblings instead of 0 B. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --- .../stub-workflow-persist.service.ts | 9 ++++- .../list-item/card-item/card-item.component.html | 4 +-- .../card-item/card-item.component.spec.ts | 10 ++++++ .../dataset-detail.component.html | 4 +-- .../dataset-detail.component.spec.ts | 40 +++++++++++++++++++++ .../model-detail.component.html | 4 +-- .../model-detail.component.spec.ts | 36 +++++++++++++------ .../user-workflow/user-workflow.component.spec.ts | 13 +++++++ .../user/user-workflow/user-workflow.component.ts | 42 ++++++++++++++++++---- 9 files changed, 136 insertions(+), 26 deletions(-) diff --git a/frontend/src/app/common/service/workflow-persist/stub-workflow-persist.service.ts b/frontend/src/app/common/service/workflow-persist/stub-workflow-persist.service.ts index 8c8a2982e8..4c873fa2ba 100644 --- a/frontend/src/app/common/service/workflow-persist/stub-workflow-persist.service.ts +++ b/frontend/src/app/common/service/workflow-persist/stub-workflow-persist.service.ts @@ -18,7 +18,7 @@ */ import { Injectable } from "@angular/core"; -import { Observable } from "rxjs"; +import { Observable, of } from "rxjs"; import { DashboardWorkflow } from "../../../dashboard/type/dashboard-workflow.interface"; import { Workflow } from "../../type/workflow"; import { SearchFilterParameters, searchTestEntries } from "../../../dashboard/type/search-filter-parameters"; @@ -60,4 +60,11 @@ export class StubWorkflowPersistService { observer.next(this.testWorkflows.map(i => i.workflow.workflow.wid as number).filter(i => i)); }); } + + /** + * reports the stored size of each requested workflow + */ + public getSizes(wids: number[]): Observable<Record<number, number>> { + return of(Object.fromEntries(wids.map(wid => [wid, 0]))); + } } diff --git a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.html b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.html index 383cc2ef95..4cd50dc8de 100644 --- a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.html +++ b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.html @@ -144,9 +144,7 @@ nzTheme="outline"></i> {{ formatTime(entry.creationTime) }}</span > - <span - *ngIf="size" - title="Size" + <span title="Size" ><i nz-icon nzType="file" 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 255e40b0d2..1b22b370a0 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 @@ -1034,6 +1034,16 @@ describe("CardItemComponent", () => { expect(errorSpy).toHaveBeenCalled(); }); + it("still reports an empty resource's size, so card and list view agree", () => { + component.entry = makeWorkflowEntry(); + component.size = 0; + fixture.detectChanges(); + + const row = fixture.debugElement.query(By.css('span[title="Size"]')); + expect(row).toBeTruthy(); + expect((row.nativeElement as HTMLElement).textContent).toContain("0 B"); + }); + 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. diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.html b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.html index 9aa0a60c09..acaf1ec5e8 100644 --- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.html +++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.html @@ -111,7 +111,7 @@ </div> <div class="stat-row"> <span class="stat-label">Last updated</span> - <span class="stat-value">{{ latestVersionCreationTime }}</span> + <span class="stat-value">{{ latestVersionCreationTime || "—" }}</span> </div> <div class="stat-row"> <span class="stat-label">Versions</span> @@ -119,7 +119,7 @@ </div> <div class="stat-row"> <span class="stat-label">Latest version file</span> - <span class="stat-value">{{ latestVersionFileName }}</span> + <span class="stat-value">{{ latestVersionFileName || "—" }}</span> </div> <div class="stat-row"> <span class="stat-label">Latest version size</span> diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.spec.ts b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.spec.ts index 0c8d4a6de5..023ed327d4 100644 --- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.spec.ts @@ -2233,6 +2233,46 @@ describe("DatasetDetailComponent rendered template", () => { }); }); + describe("data card stats", () => { + /** The stat value rendered beside a label. */ + const stat = (el: HTMLElement, label: string): string => { + const row = Array.from(el.querySelectorAll<HTMLElement>(".stat-row")).find( + r => text(q<HTMLElement>(r, ".stat-label")) === label + ); + expect(row, `expected a stat row labelled "${label}"`).toBeDefined(); + return text(q<HTMLElement>(row!, ".stat-value")); + }; + + it("em-dashes the facts a dataset with no versions has none of", () => { + const el = render({ + did: 5, + versions: [], + latestVersionCreationTime: "", + latestVersionFileName: "", + latestVersionSize: undefined, + }); + + expect(stat(el, "Last updated")).toBe("—"); + expect(stat(el, "Latest version file")).toBe("—"); + // A size has a meaningful zero, so it keeps reading 0 B rather than an em dash. + expect(stat(el, "Latest version size")).toBe("0 B"); + }); + + it("shows the real facts once a version exists", () => { + const el = render({ + did: 5, + versions: [aVersion({ name: "v1" })], + latestVersionCreationTime: "09/02/2026 11:10:11", + latestVersionFileName: "/dataset/o/ds/v1/a.csv", + latestVersionSize: 2048, + }); + + expect(stat(el, "Last updated")).toBe("09/02/2026 11:10:11"); + expect(stat(el, "Latest version file")).toBe("/dataset/o/ds/v1/a.csv"); + expect(stat(el, "Latest version size")).toBe("2.00 KB"); + }); + }); + describe("settings hints", () => { // Visibility and Downloadable are near-identical rows, so a hint or a switch // is only meaningful next to the label it belongs to: reading them as one diff --git a/frontend/src/app/dashboard/component/user/user-model/user-model-explorer/model-detail.component.html b/frontend/src/app/dashboard/component/user/user-model/user-model-explorer/model-detail.component.html index 928ca5f5ba..ca772155c4 100644 --- a/frontend/src/app/dashboard/component/user/user-model/user-model-explorer/model-detail.component.html +++ b/frontend/src/app/dashboard/component/user/user-model/user-model-explorer/model-detail.component.html @@ -151,9 +151,7 @@ </div> <div class="stat-row"> <span class="stat-label">Latest version size</span> - <span class="stat-value" - >{{ latestVersionSize === undefined ? "—" : formatSize(latestVersionSize) }}</span - > + <span class="stat-value">{{ formatSize(latestVersionSize) }}</span> </div> </div> </nz-card> diff --git a/frontend/src/app/dashboard/component/user/user-model/user-model-explorer/model-detail.component.spec.ts b/frontend/src/app/dashboard/component/user/user-model/user-model-explorer/model-detail.component.spec.ts index 617b2534f6..bb9d0e2900 100644 --- a/frontend/src/app/dashboard/component/user/user-model/user-model-explorer/model-detail.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-model/user-model-explorer/model-detail.component.spec.ts @@ -512,16 +512,6 @@ describe("ModelDetailComponent", () => { expect(hubService["toggleLike"]).toHaveBeenCalled(); }); - it("dashes out the latest-version facts for a model with no versions", () => { - create(); - const stats = q(render(), ".data-card-stats").textContent ?? ""; - - // "0 B" would assert a zero-byte version that does not exist; the card already - // uses an em dash for an absent framework or format. - expect(stats).not.toContain("0 B"); - expect(stats.match(/—/g)?.length).toBe(3); - }); - it("shows the empty-version notice until a version exists", () => { create(); const root = openTab("Versions & Files"); @@ -578,6 +568,32 @@ describe("ModelDetailComponent", () => { expect(treeFor("READ").isCoverSettable).toBe(false); }); + it("em-dashes the facts a model with no versions has none of, but keeps 0 B for its size", () => { + create(); + const root = render({ + versions: [], + latestVersionCreationTime: "", + latestVersionFileName: "", + latestVersionSize: undefined, + modelFormat: "", + }); + + /** The stat value rendered beside a label. */ + const stat = (label: string): string => { + const row = Array.from(root.querySelectorAll<HTMLElement>(".stat-row")).find( + r => (q<HTMLElement>(r, ".stat-label").textContent ?? "").trim() === label + ); + expect(row, `expected a stat row labelled "${label}"`).toBeDefined(); + return (q<HTMLElement>(row!, ".stat-value").textContent ?? "").trim(); + }; + + expect(stat("Last updated")).toBe("—"); + expect(stat("Latest version file")).toBe("—"); + expect(stat("Format")).toBe("—"); + // A size has a meaningful zero, so it reads 0 B here and on the dataset page. + expect(stat("Latest version size")).toBe("0 B"); + }); + it("hands the version uploader the model endpoint and the model's identity", () => { create(); const root = openTab("Versions & Files"); diff --git a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts index ecc9c1c68c..6d59455394 100644 --- a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts @@ -987,6 +987,19 @@ describe("SavedWorkflowSectionComponent", () => { expect(entries[0].accessibleUserIds).toEqual([1]); }); + it("asks for the copy's size, which the duplicate response does not carry", async () => { + const persist = TestBed.inject(WorkflowPersistService) as any; + persist.duplicateWorkflow = vi.fn().mockReturnValue(of([makeDashboardWorkflow(201, "dup")])); + persist.getSizes = vi.fn().mockReturnValue(of({ 201: 4096 })); + setEntries([]); + + await component.onClickDuplicateWorkflow(makeEntry(5, "orig")); + + expect(persist.getSizes).toHaveBeenCalledWith([201]); + // Without this the row would claim 0 B next to correctly-sized siblings. + expect(component.searchResultsComponent.entries[0].size).toBe(4096); + }); + it("skips the user-info lookup and access grant when there is no owner or current user", async () => { const persist = TestBed.inject(WorkflowPersistService) as any; persist.duplicateWorkflow = vi diff --git a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts index 41437c533f..29ebb13afa 100644 --- a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts +++ b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts @@ -417,7 +417,10 @@ export class UserWorkflowComponent implements AfterViewInit, OnDestroy { return entry; }); - this.searchResultsComponent.entries = [...newEntries, ...this.searchResultsComponent.entries]; + this.searchResultsComponent.entries = [ + ...(await firstValueFrom(this.withWorkflowSizes(newEntries))), + ...this.searchResultsComponent.entries, + ]; } catch (err: unknown) { console.log("Error duplicating workflow:", err); // @ts-ignore // TODO: fix this with notification component @@ -426,6 +429,29 @@ export class UserWorkflowComponent implements AfterViewInit, OnDestroy { } } + /** + * A DashboardEntry built from a duplicate response carries no size — only search asks the + * backend for one — so it would render as an empty workflow. Fill the sizes in before the + * rows go on screen, since the list items read the size once on binding. + */ + private withWorkflowSizes(entries: DashboardEntry[]): Observable<DashboardEntry[]> { + const wids = entries.map(e => e.workflow.workflow.wid).filter((wid): wid is number => wid != null); + if (wids.length === 0) { + return of(entries); + } + return this.workflowPersistService.getSizes(wids).pipe( + map(sizes => { + entries.forEach(entry => { + const wid = entry.workflow.workflow.wid; + if (wid != null && sizes[wid] != null) { + entry.setSize(sizes[wid]); + } + }); + return entries; + }) + ); + } + /** * deleteWorkflow trigger the delete workflow * component. If user confirms the deletion, the method sends @@ -582,13 +608,15 @@ export class UserWorkflowComponent implements AfterViewInit, OnDestroy { if (targetWids.length > 0) { this.workflowPersistService .duplicateWorkflow(targetWids) - .pipe(untilDestroyed(this)) + .pipe( + switchMap(duplicatedWorkflowsInfo => + this.withWorkflowSizes(duplicatedWorkflowsInfo.map(info => new DashboardEntry(info))) + ), + untilDestroyed(this) + ) .subscribe({ - next: duplicatedWorkflowsInfo => { - this.searchResultsComponent.entries = [ - ...duplicatedWorkflowsInfo.map(duplicatedWorkflowInfo => new DashboardEntry(duplicatedWorkflowInfo)), - ...this.searchResultsComponent.entries, - ]; + next: sizedEntries => { + this.searchResultsComponent.entries = [...sizedEntries, ...this.searchResultsComponent.entries]; // this.searchResultsComponent.clearAllSelections(); }, // TODO: fix this with notification component
