Copilot commented on code in PR #7443:
URL: https://github.com/apache/texera/pull/7443#discussion_r3743430702
##########
frontend/src/app/dashboard/component/user/user-dataset/user-dataset-list-item/user-dataset-list-item.component.spec.ts:
##########
@@ -51,6 +51,9 @@ class TestHostComponent {
@ViewChild(UserDatasetListItemComponent, { static: true }) inner!:
UserDatasetListItemComponent;
}
+import { NzTooltipDirective } from "ng-zorro-antd/tooltip";
+import { By } from "@angular/platform-browser";
+
Review Comment:
Imports are split into two blocks, with new import statements appearing
after the TestHostComponent declaration. This is inconsistent with the
import-at-top layout used across the frontend specs and makes the file harder
to scan/maintain. Please move these imports into the main import section at the
top of the file.
##########
frontend/src/app/dashboard/component/user/user-dataset/user-dataset-list-item/user-dataset-list-item.component.spec.ts:
##########
@@ -308,4 +311,101 @@ describe("UserDatasetListItemComponent", () => {
expect(component.refresh).toBeInstanceOf(EventEmitter);
});
});
+ /**
+ * Whether this row offers any editing is decided in the template, and by
TWO conditions rather
+ * than one: the list must be editable AND the viewer must hold WRITE on the
dataset. The suite
+ * above exercises the component's methods and never renders, so neither
condition was pinned.
+ */
+ describe("rendered row", () => {
+ /** Re-renders the host with the given entry and list-level editability. */
+ function render(over: Partial<DashboardDataset> = {}, editable = true):
HTMLElement {
+ fixture.componentInstance.entry = makeEntry(over);
+ fixture.componentInstance.editable = editable;
+ fixture.detectChanges();
+ component = fixture.componentInstance.inner;
+ return fixture.nativeElement as HTMLElement;
+ }
+
+ /** Titles of every tooltip on the row; interpolated ones never reach the
DOM as attributes. */
+ function tooltipTitles(): unknown[] {
+ return fixture.debugElement
+ .queryAll(By.directive(NzTooltipDirective))
+ .map(d => (d.injector.get(NzTooltipDirective) as
NzTooltipDirective).directiveTitle);
+ }
+
+ function hasTooltip(pred: (t: string) => boolean): boolean {
+ return tooltipTitles().some(t => typeof t === "string" && pred(t));
+ }
+
+ it("offers the editing controls to a writer on an editable list", () => {
+ render({ accessPrivilege: "WRITE" }, true);
+
+ expect(hasTooltip(t => t === "Customize Dataset Name")).toBe(true);
+ expect(hasTooltip(t => t === "Add Description")).toBe(true);
+ });
+
+ it("withholds them from a reader, even on an editable list", () => {
+ // READ access must not be offered a rename it cannot persist; the list
being editable is not
+ // on its own permission to change someone else's dataset.
+ render({ accessPrivilege: "READ" }, true);
+
+ expect(hasTooltip(t => t === "Customize Dataset Name")).toBe(false);
+ expect(hasTooltip(t => t === "Add Description")).toBe(false);
+ });
+
+ it("withholds them on a non-editable list, even from a writer", () => {
+ render({ accessPrivilege: "WRITE" }, false);
+
+ expect(hasTooltip(t => t === "Customize Dataset Name")).toBe(false);
+ });
Review Comment:
This test is meant to cover the "editable" half of the permission gate for
*both* editing controls. It currently only asserts the rename tooltip is
absent; adding an assertion for "Add Description" would ensure dropping
`editable` from that button’s `*ngIf` is also caught.
This issue also appears on line 362 of the same file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
