aglinxinyuan commented on code in PR #7443:
URL: https://github.com/apache/texera/pull/7443#discussion_r3745702907
##########
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:
Good catch — moved both into the top import block.
##########
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:
You're right, and I confirmed it by mutation: dropping `editable` from the
add-description `*ngIf` left all 28 tests green. Added the second assertion,
and that same mutation is now red.
Also correct — `editingDescription = editable && entry.accessPrivilege ===
'WRITE'` reduced to the privilege check alone survived the whole suite. Added
the WRITE-on-a-non-editable-list case, which kills it.
--
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]
