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/22 08:12:55 UTC
(texera) branch gh-readonly-queue/main/pr-7833-425f5a5ad94ed600f6aa0ccdce29f366b17b586d created (now 489b8324cc)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-7833-425f5a5ad94ed600f6aa0ccdce29f366b17b586d
in repository https://gitbox.apache.org/repos/asf/texera.git
at 489b8324cc test(frontend): close the branch arms left in the dataset explorer components (#7833)
This branch includes the following new commits:
new 489b8324cc test(frontend): close the branch arms left in the dataset explorer components (#7833)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(texera) 01/01: test(frontend): close the branch arms left in the dataset explorer components (#7833)
Posted by gi...@apache.org.
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-7833-425f5a5ad94ed600f6aa0ccdce29f366b17b586d
in repository https://gitbox.apache.org/repos/asf/texera.git
commit 489b8324cc4a3c3260d477fd6a1dda59dc944504
Author: Xinyuan Lin <xi...@uci.edu>
AuthorDate: Sat Aug 22 08:12:33 2026 +0000
test(frontend): close the branch arms left in the dataset explorer components (#7833)
### What changes were proposed in this PR?
Test-only. **No production file changes.**
Both templates here reached 100% in merged PRs #7681 and #7772, which
lifted these `.ts` files as a side effect — so what remained was branch
arms and uncalled handlers rather than whole untested regions. Measured
with istanbul JSON from `--coverage --coverage-reporters=json`, one spec
at a time, identical `--include` filter for the before and after runs:
**`dataset-detail.component.ts`**
| | before | after |
|---|---|---|
| statements | 425/430 (98.84%) | **428/430 (99.53%)** |
| branch arms | 177/203 (87.19%) | **193/203 (95.07%)** |
| functions | 115/120 (95.83%) | **120/120 (100%)** |
In the Codecov framing used in the issue: 5 missed lines + 24 partial →
**2 missed + 9 partial**. 15 of the 24 partial lines are now fully
covered. Closed 3 of 5 missed statement lines, **16 of 26 missed branch
arms**, and **5 of 5 missed functions**. Its template is re-confirmed at
455/455 statements and 46/46 branch arms, so nothing was added there.
**`user-dataset-version-creator.component.ts`**
| | before | after |
|---|---|---|
| statements | 58/63 (92.06%) | **62/63 (98.41%)** |
| branch arms | 22/24 (91.67%) | **24/24 (100%)** |
| functions | 14/16 (87.50%) | 15/16 (93.75%) |
Codecov framing: 5 missed + 2 partial → **1 missed + 0 partial**.
Tests **188 → 214**: dataset-detail 173 → 192, version-creator 15 → 18,
plus the repair round below.
### An honest correction to the second target's count
The assessment called it 5 lines + 2 arms. The real closable count is
**4 statements + 2 arms + 1 function**, because **one of its 5 missed
lines is dead code**: `get formControlNames(): string[]`. A repo-wide
grep returns exactly one hit — its own declaration — with no template
and no TypeScript reference. It is left uncovered deliberately rather
than cemented; deleting the getter belongs in a separate production PR.
### Verification
Review raised 14 findings across two lenses; the repair round took the
suite to 214 and every accepted finding was proved by running the
reviewer's proposed mutation first, then re-running after the fix to
confirm it dies on the named test.
Neither file is an instance of #7458 — no `TestBed.overrideComponent` in
either spec, which the non-zero starting percentages already implied.
One earlier misdiagnosis was deliberately **not** re-reported:
`dataset-detail.component.html:593`'s `[disabled]` binding does work.
`NgModel` routes it through `control.disable()`, deferred to a
microtask, so the DOM lags one turn behind `detectChanges()` — asserting
immediately reads a stale value and looks like an inert binding.
### Any related issues, documentation, discussions?
Closes #7832
### How was this PR tested?
```
npx ng test --watch=false --include="**/dataset-detail.component.spec.ts" --include="**/user-dataset-version-creator.component.spec.ts"
```
```
Test Files 2 passed (2)
Tests 214 passed (214)
```
`yarn format:ci` exits 0; no `junit.xml` left behind.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
---
.../dataset-detail.component.spec.ts | 364 ++++++++++++++++++++-
.../user-dataset-version-creator.component.spec.ts | 76 ++++-
2 files changed, 438 insertions(+), 2 deletions(-)
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 7bb6af6e0d..af5c6aed18 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
@@ -37,7 +37,7 @@ import { NotificationService } from "../../../../../common/service/notification/
import { DownloadService } from "../../../../service/user/download/download.service";
import { UserService } from "../../../../../common/service/user/user.service";
import { MOCK_USER, StubUserService } from "../../../../../common/service/user/stub-user.service";
-import { HubService } from "../../../../../hub/service/hub.service";
+import { ActionType, EntityType, HubService } from "../../../../../hub/service/hub.service";
import { AdminSettingsService } from "../../../../service/admin/settings/admin-settings.service";
import { FileUploadItem } from "../../../../type/dashboard-file.interface";
import { DatasetFileNode, getFullPathFromDatasetFileNode } from "../../../../../common/type/datasetVersionFileTree";
@@ -337,6 +337,79 @@ describe("DatasetDetailComponent upload queue", () => {
expect(finalize).toHaveBeenCalledWith("owner@texera.com", "test-dataset", "b.txt", true);
expect(onCanceled).toHaveBeenCalledTimes(1);
});
+
+ it("frees the slot of an upload aborted before its first part went out", () => {
+ // A task sits at "initializing" until the service reports its first progress.
+ // Cancelling in that window still has to hand the slot to whatever is queued
+ // behind it, or the queue stalls on an upload that never started.
+ dropFiles("a.txt", "b.txt", "c.txt", "d.txt");
+ expect(uploadedPaths).toEqual(["a.txt", "b.txt", "c.txt"]);
+ const initializing = component.uploadTasks.find(t => t.filePath === "a.txt")!;
+ expect(initializing.status).toBe("initializing");
+
+ component.onClickAbortUploadProgress(initializing as any);
+
+ expect(uploadedPaths).toContain("d.txt");
+ expect(component.activeCount).toBe(3);
+ });
+
+ it("does not free a second slot when a finished upload's row is dismissed", () => {
+ // The row's button becomes "Close" once the upload is done, and the slot was
+ // already released by the completion; releasing it a second time would let a
+ // fourth upload run past the concurrency cap.
+ dropFiles("a.txt", "b.txt", "c.txt", "d.txt");
+ finishUpload(0, "a.txt");
+ expect(uploadedPaths).toContain("d.txt");
+ const finished = component.uploadTasks.find(t => t.filePath === "a.txt")!;
+ expect(finished.status).toBe("finished");
+
+ component.onClickAbortUploadProgress(finished as any);
+
+ expect(component.activeCount).toBe(3);
+ const dismissed = component.uploadTasks.find(t => t.filePath === "a.txt")!;
+ expect(dismissed.status).toBe("aborted");
+
+ // The row lingers for five seconds after being dismissed, so the same X is
+ // still there to be clicked again — and that click must not release either.
+ component.onClickAbortUploadProgress(dismissed as any);
+
+ expect(component.activeCount).toBe(3);
+ });
+
+ it("does not free a second slot when a failed upload's row is dismissed", () => {
+ // The failure handler already released this upload's slot and let the queued
+ // fourth file start; dismissing the row it left behind must not release a
+ // second slot, or a fifth upload would run past the cap of three.
+ dropFiles("a.txt", "b.txt", "c.txt", "d.txt");
+ uploadSubjects[0].error(new HttpErrorResponse({ status: 500 }));
+ expect(uploadedPaths).toContain("d.txt");
+ const failed = component.uploadTasks.find(t => t.filePath === "a.txt")!;
+ expect(failed.status).toBe("failed");
+
+ component.onClickAbortUploadProgress(failed as any);
+
+ expect(component.activeCount).toBe(3);
+ expect(multipartUploadSpy).toHaveBeenCalledTimes(4);
+ });
+
+ it("cancelExistingUpload aborts an upload whose first part has not gone out", () => {
+ // Until the service reports a first chunk the task sits at "initializing".
+ // A re-drop in that window has to abort that attempt rather than fall
+ // through and race a second multipart upload against it for the same path,
+ // which is exactly the 409 the upload error handler warns about.
+ dropFiles("b.txt");
+ expect(component.uploadTasks.find(t => t.filePath === "b.txt")!.status).toBe("initializing");
+ const onCanceled = vi.fn();
+
+ component.cancelExistingUpload("b.txt", onCanceled);
+
+ expect(finalize).toHaveBeenCalledWith("owner@texera.com", "test-dataset", "b.txt", true);
+ expect(component.uploadTasks.find(t => t.filePath === "b.txt")!.status).toBe("aborted");
+ // The slot goes back to the queue instead of being held by an attempt that
+ // is no longer running.
+ expect(component.activeCount).toBe(0);
+ expect(onCanceled).toHaveBeenCalledTimes(1);
+ });
});
/**
@@ -631,6 +704,19 @@ describe("DatasetDetailComponent upload queue", () => {
expect(component.queuedFileNames).toEqual([]);
});
+ it("starts no upload at all when the route carried no dataset id", () => {
+ // Every multipart call is addressed to a dataset, so without one there is
+ // nowhere to upload into: the drop is refused outright rather than leaving
+ // rows on the panel for uploads that were never started.
+ component.did = undefined;
+
+ dropFiles("f1.txt", "f2.txt");
+
+ expect(multipartUploadSpy).not.toHaveBeenCalled();
+ expect(component.uploadTasks).toEqual([]);
+ expect(component.activeCount).toBe(0);
+ });
+
it("starts the next queued upload when an active upload finishes", () => {
dropFiles("f1.txt", "f2.txt", "f3.txt", "f4.txt", "f5.txt");
@@ -1009,6 +1095,69 @@ describe("DatasetDetailComponent behavior", () => {
expect(hubServiceStub.getCounts).not.toHaveBeenCalled();
expect(hubServiceStub.postView).not.toHaveBeenCalled();
});
+
+ it("reads a counts response with no like tally as no likes, and no liked record as not liked", () => {
+ // Both are legitimate wire shapes: `counts` is a partial map keyed by action
+ // type, and `isLiked` simply omits entities the user has no record against.
+ hubServiceStub.getCounts.mockReturnValue(of([{ counts: {} }]));
+ hubServiceStub.isLiked.mockReturnValue(of([]));
+
+ createComponent({ did: 5 });
+ // Seed both fields with values the response cannot produce, so falling back
+ // is distinguishable from leaving whatever happened to be there.
+ component.likeCount = 9;
+ component.isLiked = true;
+ login();
+ fixture.detectChanges();
+
+ const likeTag: HTMLElement = fixture.nativeElement.querySelector(".like-tag");
+ expect(likeTag).not.toBeNull();
+ expect((likeTag.textContent ?? "").trim()).toBe("0");
+ expect(likeTag.classList).not.toContain("liked");
+ // The tally shown is the dataset's own like count, not some other entity's
+ // or some other action's: nothing else in the suite pins these arguments.
+ expect(hubServiceStub.getCounts).toHaveBeenCalledWith([EntityType.Dataset], [5], [ActionType.Like]);
+ });
+
+ it("leaves the chunk size untouched when only that setting fails to load", () => {
+ // A distinct value per key, so a setting that lands in the wrong field is
+ // visible: 7 chunks and 2 files cannot stand in for one another.
+ adminSettingsServiceStub.getPublicSetting.mockImplementation((key: string) =>
+ key === "multipart_upload_chunk_size_mib"
+ ? throwError(() => new Error("boom"))
+ : of(key === "max_number_of_concurrent_uploading_file_chunks" ? "7" : "2")
+ );
+
+ createComponent({ did: 5 });
+ // A sentinel the class default cannot supply, so "the failed fetch wrote
+ // nothing" is distinguishable from "it wrote the default back".
+ component.chunkSizeMiB = 42;
+ login();
+ fixture.detectChanges();
+
+ expect(component.chunkSizeMiB).toBe(42);
+ expect(component.maxConcurrentChunks).toBe(7);
+ expect(component.maxConcurrentFiles).toBe(2);
+ });
+
+ it("leaves both concurrency limits untouched when their settings fail to load", () => {
+ adminSettingsServiceStub.getPublicSetting.mockImplementation((key: string) =>
+ key === "multipart_upload_chunk_size_mib" ? of("128") : throwError(() => new Error("boom"))
+ );
+
+ createComponent({ did: 5 });
+ // Sentinels again. A failed fetch that wrote anything here — a reset or a
+ // NaN — would stall the queue outright, since `activeUploads < NaN` is
+ // never true, and a plain default-valued assertion could not see it.
+ component.maxConcurrentChunks = 41;
+ component.maxConcurrentFiles = 40;
+ login();
+ fixture.detectChanges();
+
+ expect(component.chunkSizeMiB).toBe(128);
+ expect(component.maxConcurrentChunks).toBe(41);
+ expect(component.maxConcurrentFiles).toBe(40);
+ });
});
describe("retrieveDatasetInfo", () => {
@@ -1386,6 +1535,50 @@ describe("DatasetDetailComponent behavior", () => {
expect(component.datasetIsDownloadable).toBe(true);
expect(notificationServiceStub.error).toHaveBeenCalledWith("Failed to change the dataset download permission");
});
+
+ it("marks the dataset private and names that state, not the one it left", () => {
+ createComponent();
+ component.did = 5;
+ component.datasetName = "MyDS";
+ component.datasetIsPublic = true;
+
+ component.onPublicStatusChange(false);
+
+ expect(component.datasetIsPublic).toBe(false);
+ expect(notificationServiceStub.success).toHaveBeenCalledWith("Dataset MyDS is now private");
+ });
+
+ it("marks downloads allowed and names that state, not the one it left", () => {
+ createComponent();
+ component.did = 5;
+ component.datasetIsDownloadable = false;
+
+ component.onDownloadableStatusChange(true);
+
+ expect(component.datasetIsDownloadable).toBe(true);
+ expect(notificationServiceStub.success).toHaveBeenCalledWith("Dataset downloads are now allowed");
+ });
+
+ it("does not attempt a publicity change without a dataset id", () => {
+ createComponent();
+ component.did = undefined;
+
+ component.onPublicStatusChange(true);
+
+ expect(datasetServiceStub.updateDatasetPublicity).not.toHaveBeenCalled();
+ expect(component.datasetIsPublic).toBe(false);
+ });
+
+ it("does not attempt a download-permission change without a dataset id", () => {
+ createComponent();
+ component.did = undefined;
+ component.datasetIsDownloadable = true;
+
+ component.onDownloadableStatusChange(false);
+
+ expect(datasetServiceStub.updateDatasetDownloadable).not.toHaveBeenCalled();
+ expect(component.datasetIsDownloadable).toBe(true);
+ });
});
describe("onClickOpenVersionCreator", () => {
@@ -1568,6 +1761,77 @@ describe("DatasetDetailComponent behavior", () => {
expect(hubServiceStub.postLike).not.toHaveBeenCalled();
expect(hubServiceStub.postUnlike).not.toHaveBeenCalled();
});
+
+ it("leaves the dataset liked when the server refuses the unlike", () => {
+ hubServiceStub.postUnlike.mockReturnValue(of(false));
+ // A tally only a refresh could produce, so a refresh that must not happen shows.
+ hubServiceStub.getCounts.mockReturnValue(of([{ counts: { like: 99 } }]));
+ createComponent();
+ component.did = 5;
+ component.currentUid = MOCK_USER.uid;
+ component.isLiked = true;
+ component.likeCount = 5;
+
+ component.toggleLike();
+
+ // The unlike is addressed to this dataset, not to some other entity type
+ // that happens to share the id.
+ expect(hubServiceStub.postUnlike).toHaveBeenCalledWith(5, EntityType.Dataset);
+ // Showing the heart as unfilled after a refused unlike would misreport the
+ // stored state, and the next click would then try to like it again.
+ expect(component.isLiked).toBe(true);
+ expect(component.likeCount).toBe(5);
+ expect(hubServiceStub.getCounts).not.toHaveBeenCalled();
+ });
+
+ it("leaves the dataset unliked when the server refuses the like", () => {
+ hubServiceStub.postLike.mockReturnValue(of(false));
+ hubServiceStub.getCounts.mockReturnValue(of([{ counts: { like: 99 } }]));
+ createComponent();
+ component.did = 5;
+ component.currentUid = MOCK_USER.uid;
+ component.isLiked = false;
+ component.likeCount = 5;
+
+ component.toggleLike();
+
+ expect(hubServiceStub.postLike).toHaveBeenCalledWith(5, EntityType.Dataset);
+ expect(component.isLiked).toBe(false);
+ expect(component.likeCount).toBe(5);
+ expect(hubServiceStub.getCounts).not.toHaveBeenCalled();
+ });
+
+ it("reads a refreshed count with no like tally as no likes after unliking", () => {
+ hubServiceStub.postUnlike.mockReturnValue(of(true));
+ hubServiceStub.getCounts.mockReturnValue(of([{ counts: {} }]));
+ createComponent();
+ component.did = 5;
+ component.currentUid = MOCK_USER.uid;
+ component.isLiked = true;
+ component.likeCount = 5;
+
+ component.toggleLike();
+
+ // The refresh re-reads this dataset's like tally: a request for another
+ // entity, another action or another id would return a stranger's count.
+ expect(hubServiceStub.getCounts).toHaveBeenCalledWith([EntityType.Dataset], [5], [ActionType.Like]);
+ expect(component.likeCount).toBe(0);
+ });
+
+ it("reads a refreshed count with no like tally as no likes after liking", () => {
+ hubServiceStub.postLike.mockReturnValue(of(true));
+ hubServiceStub.getCounts.mockReturnValue(of([{ counts: {} }]));
+ createComponent();
+ component.did = 5;
+ component.currentUid = MOCK_USER.uid;
+ component.isLiked = false;
+ component.likeCount = 5;
+
+ component.toggleLike();
+
+ expect(hubServiceStub.getCounts).toHaveBeenCalledWith([EntityType.Dataset], [5], [ActionType.Like]);
+ expect(component.likeCount).toBe(0);
+ });
});
describe("cover image and description persistence", () => {
@@ -1598,6 +1862,52 @@ describe("DatasetDetailComponent behavior", () => {
expect(notificationServiceStub.error).toHaveBeenCalledWith("nope");
});
+ it("drops the previous cover url when the refreshed one cannot be fetched", () => {
+ datasetServiceStub.updateDatasetCoverImage.mockReturnValue(of({}));
+ datasetServiceStub.getDatasetCoverUrl.mockReturnValue(throwError(() => new Error("boom")));
+ createComponent();
+ component.did = 5;
+ component.selectedVersion = makeVersion({ name: "v1" });
+ component.coverImageUrl = "http://stale";
+
+ component.onSetCoverImage("img.png");
+
+ // The stale url still points at the cover that was just replaced, so keeping
+ // it would show the old image as though the change had not been made.
+ expect(component.coverImageUrl).toBeNull();
+ expect(notificationServiceStub.success).toHaveBeenCalledWith("Cover image updated.");
+ });
+
+ it("does not surface a non-HTTP failure's own message when setting the cover image", () => {
+ // A rejection from below the HTTP layer, shaped like a response but not one:
+ // only a real HttpErrorResponse carries a body the backend meant for a user,
+ // so this text must stay out of the toast. Reading `.error.message` off
+ // anything that has it would leak the transport detail instead.
+ datasetServiceStub.updateDatasetCoverImage.mockReturnValue(
+ throwError(() => ({ status: 0, error: { message: "connect ECONNREFUSED 127.0.0.1:8080" } }))
+ );
+ createComponent();
+ component.did = 5;
+ component.selectedVersion = makeVersion({ name: "v1" });
+
+ component.onSetCoverImage("img.png");
+
+ expect(notificationServiceStub.error).toHaveBeenCalledWith("Failed to set cover image");
+ });
+
+ it("falls back to a generic message when the error body carries none", () => {
+ datasetServiceStub.updateDatasetCoverImage.mockReturnValue(
+ throwError(() => new HttpErrorResponse({ error: {}, status: 500 }))
+ );
+ createComponent();
+ component.did = 5;
+ component.selectedVersion = makeVersion({ name: "v1" });
+
+ component.onSetCoverImage("img.png");
+
+ expect(notificationServiceStub.error).toHaveBeenCalledWith("Failed to set cover image");
+ });
+
it("does nothing when there is no selected version to attach the cover to", () => {
createComponent();
component.did = 5;
@@ -1719,6 +2029,58 @@ describe("DatasetDetailComponent behavior", () => {
});
});
+ describe("onPreviouslyUploadedFileDeleted", () => {
+ const node: DatasetFileNode = {
+ name: "a.txt",
+ type: "file",
+ parentDir: "/dataset/owner@texera.com/ds/v1/nested",
+ };
+
+ it("does not delete a file without a dataset id", () => {
+ createComponent();
+ component.did = undefined;
+
+ component.onPreviouslyUploadedFileDeleted(node);
+
+ expect(datasetServiceStub.deleteDatasetFile).not.toHaveBeenCalled();
+ });
+
+ it("toasts an error and stages nothing when the deletion fails", () => {
+ datasetServiceStub.deleteDatasetFile.mockReturnValue(throwError(() => new Error("boom")));
+ createComponent();
+ component.did = 5;
+ const emit = vi.fn();
+ component.userMakeChanges.subscribe(emit);
+
+ component.onPreviouslyUploadedFileDeleted(node);
+
+ expect(notificationServiceStub.error).toHaveBeenCalledWith("Failed to delete the file");
+ // A file the backend still holds is not a staged change, so counting it would
+ // offer a version to create out of a deletion that never happened.
+ expect(component.pendingChangesCount).toBe(0);
+ expect(component.userHasPendingChanges).toBe(false);
+ expect(emit).not.toHaveBeenCalled();
+ });
+
+ it("stages the deletion under the same path the next diff response confirms", () => {
+ createComponent();
+ component.did = 5;
+
+ component.onPreviouslyUploadedFileDeleted(node);
+
+ expect(datasetServiceStub.deleteDatasetFile).toHaveBeenCalledWith(5, "nested/a.txt");
+ expect(component.pendingChangesCount).toBe(1);
+
+ // The diff response reports staged paths relative to the version root, and
+ // only an exact match retires the locally staged entry. A key in any other
+ // form never reconciles, so the Finished header counts this one deletion
+ // twice until the next version is created.
+ component.onStagedObjectsUpdated([{ path: "nested/a.txt", pathType: "file", diffType: "removed", sizeBytes: 0 }]);
+
+ expect(component.pendingChangesCount).toBe(1);
+ });
+ });
+
describe("onSaveDatasetName", () => {
it("seeds editedDatasetName from the loaded dataset name", () => {
datasetServiceStub.getDataset.mockReturnValue(
diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-creator/user-dataset-version-creator.component.spec.ts b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-creator/user-dataset-version-creator.component.spec.ts
index 1c7ebdd007..a9d94d1486 100644
--- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-creator/user-dataset-version-creator.component.spec.ts
+++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-creator/user-dataset-version-creator.component.spec.ts
@@ -25,7 +25,7 @@ import { FormsModule, ReactiveFormsModule } from "@angular/forms";
import { FieldType, FieldTypeConfig, FormlyModule } from "@ngx-formly/core";
import { HttpClientTestingModule } from "@angular/common/http/testing";
import { NZ_MODAL_DATA, NzModalRef } from "ng-zorro-antd/modal";
-import { of, throwError } from "rxjs";
+import { of, Subject, throwError } from "rxjs";
import { UserDatasetVersionCreatorComponent } from "./user-dataset-version-creator.component";
import { DatasetService } from "../../../../../service/user/dataset/dataset.service";
import { NotificationService } from "../../../../../../common/service/notification/notification.service";
@@ -138,6 +138,78 @@ describe("UserDatasetVersionCreatorComponent", () => {
expect(modalClose).toHaveBeenCalledWith({ did: 7 });
});
+ it("onClickCreate reports both the typed and the sanitized name when the name had to be rewritten", async () => {
+ const fixture = await createFixture({ isCreatingVersion: false });
+ fixture.detectChanges();
+ const component = fixture.componentInstance;
+ // Padded on both sides, and the only junk at the end is the whitespace itself.
+ // That is what separates the four candidate trims: only stripping the leading
+ // side leaves a trailing run for the dash rule to collapse, so "my-dataset-"
+ // distinguishes it from "my-dataset" (trim), "-my-dataset" (trimEnd) and
+ // "-my-dataset-" (no trim at all).
+ component.form.get("name")?.setValue(" My Dataset ");
+ createDataset.mockReturnValue(of({ did: 7 }));
+
+ component.onClickCreate();
+
+ expect(createDataset.mock.calls[0][0]).toMatchObject({ name: "my-dataset-" });
+ // The writer typed one name and got another, so the toast has to name both —
+ // and in that order, or it reads as the rewrite having gone the other way.
+ expect(notifySuccess).toHaveBeenCalledWith(
+ "Dataset ' My Dataset ' was sanitized to 'my-dataset-' and created successfully."
+ );
+ // The spinner comes back down on the success path too, or the form stays
+ // locked behind a request that has already returned.
+ expect(component.isCreating).toBe(false);
+ });
+
+ it("onClickCreate reports a name that needed no rewriting just once", async () => {
+ const fixture = await createFixture({ isCreatingVersion: false });
+ fixture.detectChanges();
+ const component = fixture.componentInstance;
+ // Already lower-case, alphanumeric and hyphenated: sanitization is a no-op.
+ component.form.get("name")?.setValue("already-clean-1");
+ createDataset.mockReturnValue(of({ did: 7 }));
+
+ component.onClickCreate();
+
+ expect(component.isDatasetNameSanitized).toBe(false);
+ expect(notifySuccess).toHaveBeenCalledWith("Dataset 'already-clean-1' created successfully.");
+ });
+
+ it("onClickCreate notifies with the stored name and closes with null when dataset creation fails", async () => {
+ const fixture = await createFixture({ isCreatingVersion: false });
+ fixture.detectChanges();
+ const component = fixture.componentInstance;
+ component.form.get("name")?.setValue("My Dataset");
+ createDataset.mockReturnValue(throwError(() => ({ error: { message: "duplicate name" } })));
+
+ component.onClickCreate();
+
+ // The failure names the dataset as it would have been stored, and the backend's
+ // own reason, so the writer can tell a clash from a rejected name.
+ expect(notifyError).toHaveBeenCalledWith("Dataset my-dataset creation failed: duplicate name");
+ // The spinner has to come back down, or the form is left permanently locked.
+ expect(component.isCreating).toBe(false);
+ expect(modalClose).toHaveBeenCalledWith(null);
+ });
+
+ it("onClickCreate locks the form while the creation request is still outstanding", async () => {
+ const fixture = await createFixture({ isCreatingVersion: false });
+ fixture.detectChanges();
+ const component = fixture.componentInstance;
+ component.form.get("name")?.setValue("My Dataset");
+ // Never emits: the request is still in flight when we look at the flag.
+ createDataset.mockReturnValue(new Subject().asObservable());
+
+ component.onClickCreate();
+
+ // `isCreating` drives both the spinner and the disabled state of the create
+ // button, so without it being raised a second click fires a second create.
+ expect(component.isCreating).toBe(true);
+ expect(modalClose).not.toHaveBeenCalled();
+ });
+
it("onClickCreate creates a dataset version and closes the modal on success", async () => {
const fixture = await createFixture({ isCreatingVersion: true, did: 42 });
fixture.detectChanges();
@@ -150,6 +222,7 @@ describe("UserDatasetVersionCreatorComponent", () => {
expect(createDatasetVersion).toHaveBeenCalledWith(42, "v2 notes");
expect(notifySuccess).toHaveBeenCalledWith("Version Created");
expect(modalClose).toHaveBeenCalledWith({ dvid: 1 });
+ expect(component.isCreating).toBe(false);
});
it("onClickCreate notifies and closes with null when version creation fails", async () => {
@@ -163,6 +236,7 @@ describe("UserDatasetVersionCreatorComponent", () => {
expect(notifyError).toHaveBeenCalled();
expect(modalClose).toHaveBeenCalledWith(null);
+ expect(component.isCreating).toBe(false);
});
it("onClickCancel closes the modal with null", async () => {