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-8352-4ff0b8b4ac415c4d60bfd73f37da5e3039ade2ec in repository https://gitbox.apache.org/repos/asf/texera.git
commit 5931d86a4dac8b60239d9b0a87a1bacb797d9af2 Author: Tanishq Gandhi <[email protected]> AuthorDate: Thu Sep 3 01:49:21 2026 +0000 feat(frontend): gate the file tree's delete and cover controls on write access (#8352) ### What changes were proposed in this PR? A collaborator with READ access saw write controls in the file tree. Nothing insecure happened — the backend rejects the calls — but the buttons should not be offered: - On a dataset shared READ-only, every file row showed a delete (trash) icon: the page bound `[isTreeNodeDeletable]="true"` unconditionally. - On **both** the dataset and the model page, an image row showed "Set as cover", because the shared file tree gated that button on `isImageFile(...)` and nothing else. Clicking it returned a red "User has no access to this dataset" toast. Changes: - `user-dataset-version-filetree.component.ts/.html` — a new `isCoverSettable` input, defaulting to `false` exactly like `isTreeNodeDeletable`, added to the "Set as cover" `*ngIf`. - `dataset-detail.component.html` — bind both inputs to `userHasWriteAccess()`, the helper that already gates the Settings tab, instead of the hardcoded `true`. - `model-detail.component.html` — bind the new input to its own `userHasWriteAccess()`. The model page already gated delete this way. The third consumer of the tree, `dataset-selection-modal`, binds neither input and so now renders no "Set as cover" button — it never wired the output up, so that button did nothing at all before. Verified against a local stack with a dataset and a model shared READ-only with a second user: | | delete icons | "Set as cover" | | --- | --- | --- | | dataset, READ user, before | 4 | 1 | | dataset, READ user, after | 0 | 0 | | model, READ user, before | 0 | 1 | | model, READ user, after | 0 | 0 | | dataset, owner, after | 4 | 1 | **Before** — READ-only collaborator sees a trash icon on every file, and "Set as cover" gives a 403: <img width="1440" height="900" alt="image" src="https://github.com/user-attachments/assets/5ca2c137-2457-4a8b-8178-2a0347fab0d7"; /> <img width="1440" height="900" alt="image" src="https://github.com/user-attachments/assets/227c2547-a332-42ea-97a6-9560bc2d7435"; /> **After** — the same user, same dataset, no write controls (the owner's view is unchanged): <img width="1440" height="900" alt="image" src="https://github.com/user-attachments/assets/87d76d20-0e18-4017-a9d4-045b1615c4b0"; /> ### Any related issues, documentation, discussions? Closes #8349. ### How was this PR tested? Specs added or updated: - `user-dataset-version-filetree.component.spec.ts` — new `withholds Set-as-cover unless the host allows it`; the existing `offers Set-as-cover on image files only` now opts in through the new input. 18 passed. - `dataset-detail.component.spec.ts` — new `offers the tree's write controls only to a writer`, asserting both inputs follow WRITE/READ. 143 passed. - `model-detail.component.spec.ts` — the same case for the model page. 81 passed. - `dataset-selection-modal.component.spec.ts` — unchanged, 16 passed. ``` cd frontend npx ng test --include src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.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 ``` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --- .../user-dataset-explorer/dataset-detail.component.html | 3 ++- .../user-dataset-explorer/dataset-detail.component.spec.ts | 10 ++++++++++ .../user-dataset-version-filetree.component.html | 2 +- .../user-dataset-version-filetree.component.spec.ts | 7 +++++++ .../user-dataset-version-filetree.component.ts | 3 +++ .../user-model-explorer/model-detail.component.html | 1 + .../user-model-explorer/model-detail.component.spec.ts | 14 ++++++++++++++ 7 files changed, 38 insertions(+), 2 deletions(-) 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 1053ea7c27..9aa0a60c09 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 @@ -439,7 +439,8 @@ </div> <texera-user-dataset-version-filetree [fileTreeNodes]="fileTreeNodeList" - [isTreeNodeDeletable]="true" + [isTreeNodeDeletable]="userHasWriteAccess()" + [isCoverSettable]="userHasWriteAccess()" (selectedTreeNode)="onVersionFileTreeNodeSelected($event)" (deletedTreeNode)="onPreviouslyUploadedFileDeleted($event)" (setCoverImage)="onSetCoverImage($event)"> 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 11be93b137..76505f0029 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 @@ -2527,6 +2527,16 @@ describe("DatasetDetailComponent rendered template", () => { expect(datasetService.deleteDatasetFile).toHaveBeenCalledWith(5, "nested/b.csv"); }); + it("offers the tree's write controls only to a writer", () => { + render({ userDatasetAccessLevel: "WRITE" }); + expect(tree().componentInstance.isTreeNodeDeletable).toBe(true); + expect(tree().componentInstance.isCoverSettable).toBe(true); + + render({ userDatasetAccessLevel: "READ" }); + expect(tree().componentInstance.isTreeNodeDeletable).toBe(false); + expect(tree().componentInstance.isCoverSettable).toBe(false); + }); + it("adopts the cover image the tree offered, qualified by the selected version", () => { tree().triggerEventHandler("setCoverImage", "nested/b.png"); diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.html b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.html index a120082e40..1b64346316 100644 --- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.html +++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.html @@ -55,7 +55,7 @@ <button nz-button nzType="link" - *ngIf="!node.data.children && isImageFile(node.data.name)" + *ngIf="isCoverSettable && !node.data.children && isImageFile(node.data.name)" class="icon-button" nz-tooltip="Set as cover" (click)="onSetCover(node.data)"> diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.spec.ts b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.spec.ts index ff9e82609a..38c2397353 100644 --- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.spec.ts @@ -241,8 +241,15 @@ describe("UserDatasetVersionFiletreeComponent", () => { expect(fixture.nativeElement.querySelector("i[nztype='delete']")).toBeNull(); }); + it("withholds Set-as-cover unless the host allows it", () => { + renderRows([file("photo.png")]); + + expect(fixture.nativeElement.querySelector("i[nztype='picture']")).toBeNull(); + }); + it("offers Set-as-cover on image files only", () => { const covers: string[] = []; + component.isCoverSettable = true; component.setCoverImage.subscribe((path: string) => covers.push(path)); renderRows([file("photo.png"), file("data.csv")]); diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.ts b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.ts index 885feceb88..a365864333 100644 --- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.ts +++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.ts @@ -69,6 +69,9 @@ export class UserDatasetVersionFiletreeComponent implements AfterViewInit { @Input() public isTreeNodeDeletable: boolean = false; + @Input() + public isCoverSettable: boolean = false; + @Input() public set fileTreeNodes(nodes: DatasetFileNode[]) { this._fileTreeNodes = nodes ?? []; 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 97515e372d..928ca5f5ba 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 @@ -349,6 +349,7 @@ <texera-user-dataset-version-filetree [fileTreeNodes]="fileTreeNodeList" [isTreeNodeDeletable]="userHasWriteAccess()" + [isCoverSettable]="userHasWriteAccess()" (selectedTreeNode)="onVersionFileTreeNodeSelected($event)" (deletedTreeNode)="onPreviouslyUploadedFileDeleted($event)" (setCoverImage)="onSetCoverImage($event)"> 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 3e5ab819f7..759d40e434 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 @@ -559,6 +559,20 @@ describe("ModelDetailComponent", () => { // The panel itself is covered by version-uploader.component.spec.ts; what matters here is that // the page hands it the model's own addressing, and what the page still owns around it. + it("offers the tree's write controls only to a writer", () => { + create(); + openTab("Versions & Files"); + const tree = () => fixture.debugElement.query(By.css("texera-user-dataset-version-filetree")).componentInstance; + + render({ userModelAccessLevel: "WRITE" }); + expect(tree().isTreeNodeDeletable).toBe(true); + expect(tree().isCoverSettable).toBe(true); + + render({ userModelAccessLevel: "READ" }); + expect(tree().isTreeNodeDeletable).toBe(false); + expect(tree().isCoverSettable).toBe(false); + }); + it("hands the version uploader the model endpoint and the model's identity", () => { create(); const root = openTab("Versions & Files");
