Copilot commented on code in PR #7655:
URL: https://github.com/apache/texera/pull/7655#discussion_r3780807115
##########
frontend/src/app/dashboard/component/admin/user/admin-user.component.spec.ts:
##########
@@ -611,4 +612,155 @@ describe("AdminUserComponent", () => {
expect(component.filterByRole([], mk({ role: Role.ADMIN }))).toBe(false);
});
});
+
+ /**
+ * The tests above call the component's methods; these drive the table
itself — the
+ * per-column search dropdowns, the click-to-edit cells and the row actions
— so a control
+ * that loses its handler fails here.
+ */
+ describe("rendered table", () => {
+ /** Seeds both lists, as loading the users does, and renders the rows. */
+ function renderUsers(users: User[]): void {
+ component.userList = [...users];
+ component.listOfDisplayUser = [...users];
+ fixture.detectChanges();
+ }
+
+ const rowNames = (): string[] =>
+ Array.from(fixture.nativeElement.querySelectorAll("tbody tr")).map(row =>
+ ((row as HTMLElement).querySelectorAll("td")[2]?.textContent ??
"").trim()
+ );
Review Comment:
`rowNames` is declared but never used, which adds dead code and may fail
builds when `noUnusedLocals` is enabled. Remove it (or use it in an assertion)
to keep the spec minimal.
##########
frontend/src/app/dashboard/component/admin/user/admin-user.component.spec.ts:
##########
@@ -611,4 +612,155 @@ describe("AdminUserComponent", () => {
expect(component.filterByRole([], mk({ role: Role.ADMIN }))).toBe(false);
});
});
+
+ /**
+ * The tests above call the component's methods; these drive the table
itself — the
+ * per-column search dropdowns, the click-to-edit cells and the row actions
— so a control
+ * that loses its handler fails here.
+ */
+ describe("rendered table", () => {
+ /** Seeds both lists, as loading the users does, and renders the rows. */
+ function renderUsers(users: User[]): void {
+ component.userList = [...users];
+ component.listOfDisplayUser = [...users];
+ fixture.detectChanges();
+ }
+
+ const rowNames = (): string[] =>
+ Array.from(fixture.nativeElement.querySelectorAll("tbody tr")).map(row =>
+ ((row as HTMLElement).querySelectorAll("td")[2]?.textContent ??
"").trim()
+ );
+
+ it("swaps a cell for an input when it is clicked, and saves on enter", ()
=> {
+ const saveSpy = vi.spyOn(component, "saveEdit").mockImplementation(() =>
{});
+ renderUsers([userA]);
+
+ const nameCell = fixture.nativeElement.querySelectorAll("tbody tr
td")[2].querySelector(".container");
+ nameCell.click();
+ fixture.detectChanges();
+
+ expect(component.editUid).toBe(userA.uid);
+ expect(component.editAttribute).toBe("name");
+ const input = fixture.nativeElement.querySelectorAll("tbody tr
td")[2].querySelector("input");
+ expect(input).not.toBeNull();
+
+ input.value = "Alicia";
+ input.dispatchEvent(new Event("input"));
+ fixture.detectChanges();
+ expect(component.editName).toBe("Alicia");
+
+ // Only Enter commits; other keys leave the edit open.
+ input.dispatchEvent(new KeyboardEvent("keydown", { key: "a", bubbles:
true }));
+ expect(saveSpy).not.toHaveBeenCalled();
+
+ input.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter",
bubbles: true }));
+ expect(saveSpy).toHaveBeenCalled();
+
+ // Clicking away saves too, so a half-typed edit is not silently dropped.
+ saveSpy.mockClear();
+ fixture.nativeElement
+ .querySelectorAll("tbody tr td")[2]
+ .querySelector("div")
+ .dispatchEvent(new Event("focusout", { bubbles: true }));
+
+ expect(saveSpy).toHaveBeenCalled();
+ });
+
+ it("does the same for the email and comment cells, and saves when the cell
loses focus", () => {
+ const saveSpy = vi.spyOn(component, "saveEdit").mockImplementation(() =>
{});
+ renderUsers([userA]);
+
+ const cells = () => fixture.nativeElement.querySelectorAll("tbody tr
td");
+ cells()[3].querySelector(".container").click();
+ fixture.detectChanges();
+ expect(component.editAttribute).toBe("email");
+ const emailInput = cells()[3].querySelector("input[type=email]");
+ expect(emailInput).not.toBeNull();
+ emailInput.value = "[email protected]";
+ emailInput.dispatchEvent(new Event("input"));
+ fixture.detectChanges();
+ expect(component.editEmail).toBe("[email protected]");
+ emailInput.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter",
bubbles: true }));
+ cells()[3]
+ .querySelector("div")
+ .dispatchEvent(new Event("focusout", { bubbles: true }));
+ expect(saveSpy).toHaveBeenCalled();
Review Comment:
This assertion can pass even if the email input's `(keydown.enter)` binding
is removed, because the subsequent `focusout` handler also calls `saveEdit()`.
Add an assertion immediately after the Enter key event (and clear the spy) so
the test verifies both bindings independently.
This issue also appears on line 700 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]
