fix(ui): address Greptile review — role filter, remount, orphaned test

- Role filter: replace exact "user" match with isAdmin check so "member",
  "proxy_member", and any other non-admin role string are correctly captured
  by the new "Non-admin" option
- Keystroke remount: lift page state into TeamMemberTab, reset via useEffect
  on filter/search change; MemberTable accepts controlled currentPage /
  onPageChange props — table DOM no longer destroyed on every keystroke
- Test suite: restore missing it("should hide action buttons when
  canEditTeam is false") wrapper that left orphaned describe-scope code
- Tests: add "Non-admin" filter regression test alongside existing Admin test

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bytechoreographer 2026-04-28 10:52:45 +08:00
parent c201ae3ba9
commit 4b3a170d38
3 changed files with 47 additions and 6 deletions

View file

@ -22,6 +22,9 @@ export interface MemberTableProps {
/** When true, renders top-right pagination controls instead of the default antd bottom pagination. */
withPagination?: boolean;
defaultPageSize?: number;
/** Controlled current page (optional). When provided, pair with onPageChange. */
currentPage?: number;
onPageChange?: (page: number) => void;
}
export default function MemberTable({
@ -38,10 +41,15 @@ export default function MemberTable({
loading,
withPagination = false,
defaultPageSize = 50,
currentPage: controlledPage,
onPageChange,
}: MemberTableProps) {
const [page, setPage] = useState(1);
const [internalPage, setInternalPage] = useState(1);
const [pageSize, setPageSize] = useState(defaultPageSize);
const page = controlledPage ?? internalPage;
const setPage = onPageChange ?? setInternalPage;
const total = members.length;
const totalPages = Math.max(1, Math.ceil(total / pageSize));
const safePage = Math.min(page, totalPages);

View file

@ -403,12 +403,35 @@ describe("TeamMembersComponent", () => {
const roleSelect = screen.getByRole("combobox");
await user.click(roleSelect);
// "Admin" filter: only user2 (role: "admin") should remain
await user.click(screen.getByText("Admin"));
expect(screen.getAllByText("user2@test.com").length).toBeGreaterThanOrEqual(1);
expect(screen.queryByText("user1@test.com")).not.toBeInTheDocument();
});
it("should filter to non-admin members when Non-admin is selected", async () => {
const user = userEvent.setup();
renderWithProviders(
<TeamMembersComponent
teamData={createMockTeamData()}
canEditTeam={false}
handleMemberDelete={mockHandleMemberDelete}
setSelectedEditMember={mockSetSelectedEditMember}
setIsEditMemberModalVisible={mockSetIsEditMemberModalVisible}
setIsAddMemberModalVisible={mockSetIsAddMemberModalVisible}
/>,
);
const roleSelect = screen.getByRole("combobox");
await user.click(roleSelect);
// "Non-admin" filter: only user1 (role: "member") should remain
await user.click(screen.getByText("Non-admin"));
expect(screen.getAllByText("user1@test.com").length).toBeGreaterThanOrEqual(1);
expect(screen.queryByText("user2@test.com")).not.toBeInTheDocument();
});
it("should show all members when search is cleared", async () => {
const user = userEvent.setup();
renderWithProviders(
@ -430,7 +453,7 @@ describe("TeamMembersComponent", () => {
expect(screen.getAllByText("user2@test.com").length).toBeGreaterThanOrEqual(1);
});
it("should hide action buttons when canEditTeam is false", () => {
renderWithProviders(
<TeamMembersComponent
teamData={createMockTeamData()}

View file

@ -9,7 +9,7 @@ import { Input, Select, Space, Tooltip, Typography } from "antd";
import type { ColumnsType } from "antd/es/table";
import MemberTable from "@/components/common_components/MemberTable";
import { TeamData, TeamMembership } from "./TeamInfo";
import { useMemo, useState } from "react";
import { useEffect, useMemo, useState } from "react";
interface TeamMemberTabProps {
teamData: TeamData;
@ -31,6 +31,10 @@ export default function TeamMemberTab({
}: TeamMemberTabProps) {
const [searchText, setSearchText] = useState("");
const [roleFilter, setRoleFilter] = useState<string | null>(null);
const [memberTablePage, setMemberTablePage] = useState(1);
// Reset to page 1 when filter/search changes — without remounting MemberTable
useEffect(() => { setMemberTablePage(1); }, [searchText, roleFilter]);
// O(1) lookup instead of O(n) find() per member per column
const membershipsMap = useMemo(
@ -46,7 +50,12 @@ export default function TeamMemberTab({
const filteredMembers = useMemo(() => {
const q = searchText.trim().toLowerCase();
return teamData.team_info.members_with_roles.filter((m) => {
if (roleFilter && m.role?.toLowerCase() !== roleFilter) return false;
if (roleFilter) {
const role = m.role?.toLowerCase() ?? "";
const isAdmin = role === "admin" || role === "org_admin";
if (roleFilter === "admin" && !isAdmin) return false;
if (roleFilter === "non-admin" && isAdmin) return false;
}
if (!q) return true;
return (
m.user_email?.toLowerCase().includes(q) ||
@ -226,16 +235,17 @@ export default function TeamMemberTab({
style={{ width: 160 }}
options={[
{ value: "admin", label: "Admin" },
{ value: "user", label: "User" },
{ value: "non-admin", label: "Non-admin" },
]}
onChange={(v) => setRoleFilter(v ?? null)}
/>
</Space>
<MemberTable
key={`${searchText}::${roleFilter}`}
members={filteredMembers}
canEdit={canEditTeam}
withPagination
currentPage={memberTablePage}
onPageChange={setMemberTablePage}
onEdit={(record) => {
const membership = membershipsMap.get(record.user_id ?? "");
const enhancedMember = {