Merge pull request #2610 from scott-ray-wilson/group-tables-fixes/improvements

Fix: Group Tables/Pagination Fixes and Improvements

backend/src/ee/routes/v1/group-router.ts

@@ -165,7 +165,8 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => {
       querystring: z.object({
         offset: z.coerce.number().min(0).max(100).default(0).describe(GROUPS.LIST_USERS.offset),
         limit: z.coerce.number().min(1).max(100).default(10).describe(GROUPS.LIST_USERS.limit),
-        username: z.string().optional().describe(GROUPS.LIST_USERS.username)
+        username: z.string().trim().optional().describe(GROUPS.LIST_USERS.username),
+        search: z.string().trim().optional().describe(GROUPS.LIST_USERS.search)
       }),
       response: {
         200: z.object({

backend/src/ee/services/access-approval-policy/access-approval-policy-service.ts

@@ -124,7 +124,9 @@ export const accessApprovalPolicyServiceFactory = ({
     const verifyAllApprovers = [...approverUserIds];
 
     for (const groupId of groupApprovers) {
-      usersPromises.push(groupDAL.findAllGroupPossibleMembers({ orgId: actorOrgId, groupId, offset: 0 }));
+      usersPromises.push(
+        groupDAL.findAllGroupPossibleMembers({ orgId: actorOrgId, groupId, offset: 0 }).then((group) => group.members)
+      );
     }
     const verifyGroupApprovers = (await Promise.all(usersPromises))
       .flat()
@@ -327,7 +329,11 @@ export const accessApprovalPolicyServiceFactory = ({
         >[] = [];
 
         for (const groupId of groupApprovers) {
-          usersPromises.push(groupDAL.findAllGroupPossibleMembers({ orgId: actorOrgId, groupId, offset: 0 }));
+          usersPromises.push(
+            groupDAL
+              .findAllGroupPossibleMembers({ orgId: actorOrgId, groupId, offset: 0 })
+              .then((group) => group.members)
+          );
         }
         const verifyGroupApprovers = (await Promise.all(usersPromises))
           .flat()

backend/src/ee/services/access-approval-request/access-approval-request-service.ts

@@ -147,10 +147,12 @@ export const accessApprovalRequestServiceFactory = ({
     const groupUsers = (
       await Promise.all(
         approverGroupIds.map((groupApproverId) =>
-          groupDAL.findAllGroupPossibleMembers({
+          groupDAL
-            orgId: actorOrgId,
+            .findAllGroupPossibleMembers({
-            groupId: groupApproverId
+              orgId: actorOrgId,
-          })
+              groupId: groupApproverId
+            })
+            .then((group) => group.members)
         )
       )
     ).flat();

backend/src/ee/services/group/group-dal.ts

@@ -65,16 +65,18 @@ export const groupDALFactory = (db: TDbClient) => {
     groupId,
     offset = 0,
     limit,
-    username
+    username, // depreciated in favor of search
+    search
   }: {
     orgId: string;
     groupId: string;
     offset?: number;
     limit?: number;
     username?: string;
+    search?: string;
   }) => {
     try {
-      let query = db
+      const query = db
         .replicaNode()(TableName.OrgMembership)
         .where(`${TableName.OrgMembership}.orgId`, orgId)
         .join(TableName.Users, `${TableName.OrgMembership}.userId`, `${TableName.Users}.id`)
@@ -92,31 +94,39 @@ export const groupDALFactory = (db: TDbClient) => {
           db.ref("username").withSchema(TableName.Users),
           db.ref("firstName").withSchema(TableName.Users),
           db.ref("lastName").withSchema(TableName.Users),
-          db.ref("id").withSchema(TableName.Users).as("userId")
+          db.ref("id").withSchema(TableName.Users).as("userId"),
+          db.raw(`count(*) OVER() as total_count`)
         )
         .where({ isGhost: false })
-        .offset(offset);
+        .offset(offset)
+        .orderBy("firstName", "asc");
 
       if (limit) {
-        query = query.limit(limit);
+        void query.limit(limit);
       }
 
-      if (username) {
+      if (search) {
-        query = query.andWhere(`${TableName.Users}.username`, "ilike", `%${username}%`);
+        void query.andWhereRaw(`CONCAT_WS(' ', "firstName", "lastName", "username") ilike '%${search}%'`);
+      } else if (username) {
+        void query.andWhere(`${TableName.Users}.username`, "ilike", `%${username}%`);
       }
 
       const members = await query;
 
-      return members.map(
+      return {
-        ({ email, username: memberUsername, firstName, lastName, userId, groupId: memberGroupId }) => ({
+        members: members.map(
-          id: userId,
+          ({ email, username: memberUsername, firstName, lastName, userId, groupId: memberGroupId }) => ({
-          email,
+            id: userId,
-          username: memberUsername,
+            email,
-          firstName,
+            username: memberUsername,
-          lastName,
+            firstName,
-          isPartOfGroup: !!memberGroupId
+            lastName,
-        })
+            isPartOfGroup: !!memberGroupId
-      );
+          })
+        ),
+        // @ts-expect-error col select is raw and not strongly typed
+        totalCount: Number(members?.[0]?.total_count ?? 0)
+      };
     } catch (error) {
       throw new DatabaseError({ error, name: "Find all org members" });
     }

backend/src/ee/services/group/group-service.ts

@@ -221,7 +221,8 @@ export const groupServiceFactory = ({
     actor,
     actorId,
     actorAuthMethod,
-    actorOrgId
+    actorOrgId,
+    search
   }: TListGroupUsersDTO) => {
     if (!actorOrgId) throw new UnauthorizedError({ message: "No organization ID provided in request" });
 
@@ -244,17 +245,16 @@ export const groupServiceFactory = ({
         message: `Failed to find group with ID ${id}`
       });
 
-    const users = await groupDAL.findAllGroupPossibleMembers({
+    const { members, totalCount } = await groupDAL.findAllGroupPossibleMembers({
       orgId: group.orgId,
       groupId: group.id,
       offset,
       limit,
-      username
+      username,
+      search
     });
 
-    const count = await orgDAL.countAllOrgMembers(group.orgId);
+    return { users: members, totalCount };
-
-    return { users, totalCount: count };
   };
 
   const addUserToGroup = async ({ id, username, actor, actorId, actorAuthMethod, actorOrgId }: TAddUserToGroupDTO) => {

backend/src/ee/services/group/group-types.ts

@@ -38,6 +38,7 @@ export type TListGroupUsersDTO = {
   offset: number;
   limit: number;
   username?: string;
+  search?: string;
 } & TGenericPermission;
 
 export type TAddUserToGroupDTO = {

backend/src/ee/services/scim/scim-service.ts

@@ -834,10 +834,12 @@ export const scimServiceFactory = ({
       });
     }
 
-    const users = await groupDAL.findAllGroupPossibleMembers({
+    const users = await groupDAL
-      orgId: group.orgId,
+      .findAllGroupPossibleMembers({
-      groupId: group.id
+        orgId: group.orgId,
-    });
+        groupId: group.id
+      })
+      .then((g) => g.members);
 
     const orgMemberships = await orgDAL.findMembership({
       [`${TableName.OrgMembership}.orgId` as "orgId"]: orgId,

backend/src/lib/api-docs/constants.ts

@@ -18,7 +18,8 @@ export const GROUPS = {
     id: "The id of the group to list users for",
     offset: "The offset to start from. If you enter 10, it will start from the 10th user.",
     limit: "The number of users to return.",
-    username: "The username to search for."
+    username: "The username to search for.",
+    search: "The text string that user email or name will be filtered by."
   },
   ADD_USER: {
     id: "The id of the group to add the user to.",

frontend/src/hooks/api/groups/queries.tsx

@@ -10,13 +10,13 @@ export const groupKeys = {
     slug,
     offset,
     limit,
-    username
+    search
   }: {
     slug: string;
     offset: number;
     limit: number;
-    username: string;
+    search: string;
-  }) => [...groupKeys.forGroupUserMemberships(slug), { offset, limit, username }] as const
+  }) => [...groupKeys.forGroupUserMemberships(slug), { offset, limit, search }] as const
 };
 
 type TUser = {
@@ -33,27 +33,28 @@ export const useListGroupUsers = ({
   groupSlug,
   offset = 0,
   limit = 10,
-  username
+  search
 }: {
   id: string;
   groupSlug: string;
   offset: number;
   limit: number;
-  username: string;
+  search: string;
 }) => {
   return useQuery({
     queryKey: groupKeys.specificGroupUserMemberships({
       slug: groupSlug,
       offset,
       limit,
-      username
+      search
     }),
     enabled: Boolean(groupSlug),
+    keepPreviousData: true,
     queryFn: async () => {
       const params = new URLSearchParams({
         offset: String(offset),
         limit: String(limit),
-        username
+        search
       });
 
       const { data } = await apiRequest.get<{ users: TUser[]; totalCount: number }>(

frontend/src/hooks/index.ts

@@ -3,6 +3,7 @@ export { useLeaveConfirm } from "./useLeaveConfirm";
 export { usePagination } from "./usePagination";
 export { usePersistentState } from "./usePersistentState";
 export { usePopUp } from "./usePopUp";
+export { useResetPageHelper } from "./useResetPageHelper";
 export { useSyntaxHighlight } from "./useSyntaxHighlight";
 export { useTimedReset } from "./useTimedReset";
 export { useToggle } from "./useToggle";

frontend/src/hooks/useResetPageHelper.ts

@@ -0,0 +1,16 @@
+import { Dispatch, SetStateAction, useEffect } from "react";
+
+export const useResetPageHelper = ({
+  totalCount,
+  offset,
+  setPage
+}: {
+  totalCount: number;
+  offset: number;
+  setPage: Dispatch<SetStateAction<number>>;
+}) => {
+  useEffect(() => {
+    // reset page if no longer valid
+    if (totalCount <= offset) setPage(1);
+  }, [totalCount]);
+};

frontend/src/views/Org/MembersPage/components/OrgGroupsTab/components/OrgGroupsSection/OrgGroupMembersModal.tsx

@@ -21,6 +21,7 @@ import {
   Tr
 } from "@app/components/v2";
 import { OrgPermissionActions, OrgPermissionSubjects } from "@app/context";
+import { useDebounce, useResetPageHelper } from "@app/hooks";
 import { useAddUserToGroup, useListGroupUsers, useRemoveUserFromGroup } from "@app/hooks/api";
 import { UsePopUpState } from "@app/hooks/usePopUp";
 
@@ -33,18 +34,28 @@ export const OrgGroupMembersModal = ({ popUp, handlePopUpToggle }: Props) => {
   const [page, setPage] = useState(1);
   const [perPage, setPerPage] = useState(10);
   const [searchMemberFilter, setSearchMemberFilter] = useState("");
+  const [debouncedSearch] = useDebounce(searchMemberFilter);
 
   const popUpData = popUp?.groupMembers?.data as {
     groupId: string;
     slug: string;
   };
 
+  const offset = (page - 1) * perPage;
   const { data, isLoading } = useListGroupUsers({
     id: popUpData?.groupId,
     groupSlug: popUpData?.slug,
-    offset: (page - 1) * perPage,
+    offset,
     limit: perPage,
-    username: searchMemberFilter
+    search: debouncedSearch
+  });
+
+  const { totalCount = 0 } = data ?? {};
+
+  useResetPageHelper({
+    totalCount,
+    offset,
+    setPage
   });
 
   const { mutateAsync: assignMutateAsync } = useAddUserToGroup();
@@ -140,9 +151,9 @@ export const OrgGroupMembersModal = ({ popUp, handlePopUpToggle }: Props) => {
                 })}
             </TBody>
           </Table>
-          {!isLoading && data?.totalCount !== undefined && (
+          {!isLoading && totalCount > 0 && (
             <Pagination
-              count={data.totalCount}
+              count={totalCount}
               page={page}
               perPage={perPage}
               onChangePage={(newPage) => setPage(newPage)}
@@ -150,7 +161,10 @@ export const OrgGroupMembersModal = ({ popUp, handlePopUpToggle }: Props) => {
             />
           )}
           {!isLoading && !data?.users?.length && (
-            <EmptyState title="No users found" icon={faUsers} />
+            <EmptyState
+              title={debouncedSearch ? "No users match search" : "No users found"}
+              icon={faUsers}
+            />
           )}
         </TableContainer>
       </ModalContent>

frontend/src/views/Org/MembersPage/components/OrgGroupsTab/components/OrgGroupsSection/OrgGroupsTable.tsx

@@ -1,5 +1,11 @@
-import { useState } from "react";
+import { useMemo, useState } from "react";
-import { faEllipsis, faMagnifyingGlass, faUsers } from "@fortawesome/free-solid-svg-icons";
+import {
+  faArrowDown,
+  faArrowUp,
+  faEllipsis,
+  faMagnifyingGlass,
+  faUsers
+} from "@fortawesome/free-solid-svg-icons";
 import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
 import { twMerge } from "tailwind-merge";
 
@@ -11,6 +17,7 @@ import {
   DropdownMenuItem,
   DropdownMenuTrigger,
   EmptyState,
+  IconButton,
   Input,
   Select,
   SelectItem,
@@ -24,7 +31,9 @@ import {
   Tr
 } from "@app/components/v2";
 import { OrgPermissionActions, OrgPermissionSubjects, useOrganization } from "@app/context";
+import { useDebounce } from "@app/hooks";
 import { useGetOrganizationGroups, useGetOrgRoles, useUpdateGroup } from "@app/hooks/api";
+import { OrderByDirection } from "@app/hooks/api/generic/types";
 import { UsePopUpState } from "@app/hooks/usePopUp";
 
 type Props = {
@@ -43,12 +52,21 @@ type Props = {
   ) => void;
 };
 
+enum GroupsOrderBy {
+  Name = "name",
+  Slug = "slug",
+  Role = "role"
+}
+
 export const OrgGroupsTable = ({ handlePopUpOpen }: Props) => {
   const [searchGroupsFilter, setSearchGroupsFilter] = useState("");
+  const [debouncedSearch] = useDebounce(searchGroupsFilter.trim());
   const { currentOrg } = useOrganization();
   const orgId = currentOrg?.id || "";
   const { isLoading, data: groups } = useGetOrganizationGroups(orgId);
   const { mutateAsync: updateMutateAsync } = useUpdateGroup();
+  const [orderBy, setOrderBy] = useState(GroupsOrderBy.Name);
+  const [orderDirection, setOrderDirection] = useState(OrderByDirection.ASC);
 
   const { data: roles } = useGetOrgRoles(orgId);
 
@@ -72,6 +90,43 @@ export const OrgGroupsTable = ({ handlePopUpOpen }: Props) => {
     }
   };
 
+  const filteredGroups = useMemo(() => {
+    const filtered = debouncedSearch
+      ? groups?.filter(
+          ({ name, slug }) =>
+            name.toLowerCase().includes(debouncedSearch.toLowerCase()) ||
+            slug.toLowerCase().includes(debouncedSearch.toLowerCase())
+        )
+      : groups;
+
+    const ordered = filtered?.sort((a, b) => {
+      switch (orderBy) {
+        case GroupsOrderBy.Role: {
+          const aValue = a.role === "custom" ? (a.customRole?.name as string) : a.role;
+          const bValue = b.role === "custom" ? (b.customRole?.name as string) : b.role;
+
+          return aValue.toLowerCase().localeCompare(bValue.toLowerCase());
+        }
+        default:
+          return a[orderBy].toLowerCase().localeCompare(b[orderBy].toLowerCase());
+      }
+    });
+
+    return orderDirection === OrderByDirection.ASC ? ordered : ordered?.reverse();
+  }, [debouncedSearch, groups, orderBy, orderDirection]);
+
+  const handleSort = (column: GroupsOrderBy) => {
+    if (column === orderBy) {
+      setOrderDirection((prev) =>
+        prev === OrderByDirection.ASC ? OrderByDirection.DESC : OrderByDirection.ASC
+      );
+      return;
+    }
+
+    setOrderBy(column);
+    setOrderDirection(OrderByDirection.ASC);
+  };
+
   return (
     <div>
       <Input
@@ -84,16 +139,70 @@ export const OrgGroupsTable = ({ handlePopUpOpen }: Props) => {
         <Table>
           <THead>
             <Tr>
-              <Th>Name</Th>
+              <Th>
-              <Th>Slug</Th>
+                <div className="flex items-center">
-              <Th>Role</Th>
+                  Name
+                  <IconButton
+                    variant="plain"
+                    className={`ml-2 ${orderBy === GroupsOrderBy.Name ? "" : "opacity-30"}`}
+                    ariaLabel="sort"
+                    onClick={() => handleSort(GroupsOrderBy.Name)}
+                  >
+                    <FontAwesomeIcon
+                      icon={
+                        orderDirection === OrderByDirection.DESC && orderBy === GroupsOrderBy.Name
+                          ? faArrowUp
+                          : faArrowDown
+                      }
+                    />
+                  </IconButton>
+                </div>
+              </Th>
+              <Th>
+                <div className="flex items-center">
+                  Slug
+                  <IconButton
+                    variant="plain"
+                    className={`ml-2 ${orderBy === GroupsOrderBy.Slug ? "" : "opacity-30"}`}
+                    ariaLabel="sort"
+                    onClick={() => handleSort(GroupsOrderBy.Slug)}
+                  >
+                    <FontAwesomeIcon
+                      icon={
+                        orderDirection === OrderByDirection.DESC && orderBy === GroupsOrderBy.Slug
+                          ? faArrowUp
+                          : faArrowDown
+                      }
+                    />
+                  </IconButton>
+                </div>
+              </Th>
+              <Th>
+                <div className="flex items-center">
+                  Role
+                  <IconButton
+                    variant="plain"
+                    className={`ml-2 ${orderBy === GroupsOrderBy.Role ? "" : "opacity-30"}`}
+                    ariaLabel="sort"
+                    onClick={() => handleSort(GroupsOrderBy.Role)}
+                  >
+                    <FontAwesomeIcon
+                      icon={
+                        orderDirection === OrderByDirection.DESC && orderBy === GroupsOrderBy.Role
+                          ? faArrowUp
+                          : faArrowDown
+                      }
+                    />
+                  </IconButton>
+                </div>
+              </Th>
               <Th className="w-5" />
             </Tr>
           </THead>
           <TBody>
             {isLoading && <TableSkeleton columns={4} innerKey="org-groups" />}
             {!isLoading &&
-              groups?.map(({ id, name, slug, role, customRole }) => {
+              filteredGroups?.map(({ id, name, slug, role, customRole }) => {
                 return (
                   <Tr className="h-10" key={`org-group-${id}`}>
                     <Td>{name}</Td>

frontend/src/views/Org/MembersPage/components/OrgIdentityTab/components/IdentitySection/IdentityTable.tsx

@@ -1,4 +1,3 @@
-import { useEffect } from "react";
 import { useRouter } from "next/router";
 import {
   faArrowDown,
@@ -34,7 +33,7 @@ import {
   Tr
 } from "@app/components/v2";
 import { OrgPermissionActions, OrgPermissionSubjects, useOrganization } from "@app/context";
-import { usePagination } from "@app/hooks";
+import { usePagination, useResetPageHelper } from "@app/hooks";
 import { useGetIdentityMembershipOrgs, useGetOrgRoles, useUpdateIdentity } from "@app/hooks/api";
 import { OrderByDirection } from "@app/hooks/api/generic/types";
 import { OrgIdentityOrderBy } from "@app/hooks/api/organization/types";
@@ -87,10 +86,11 @@ export const IdentityTable = ({ handlePopUpOpen }: Props) => {
   );
 
   const { totalCount = 0 } = data ?? {};
-  useEffect(() => {
+  useResetPageHelper({
-    // reset page if no longer valid
+    totalCount,
-    if (totalCount <= offset) setPage(1);
+    offset,
-  }, [totalCount]);
+    setPage
+  });
 
   const { data: roles } = useGetOrgRoles(organizationId);
 

frontend/src/views/Project/KmsPage/components/CmekTable.tsx

@@ -1,4 +1,3 @@
-import { useEffect } from "react";
 import Link from "next/link";
 import {
   faArrowDown,
@@ -51,7 +50,7 @@ import {
   useProjectPermission,
   useWorkspace
 } from "@app/context";
-import { usePagination, usePopUp } from "@app/hooks";
+import { usePagination, usePopUp, useResetPageHelper } from "@app/hooks";
 import { useGetCmeksByProjectId, useUpdateCmek } from "@app/hooks/api/cmeks";
 import { CmekOrderBy, TCmek } from "@app/hooks/api/cmeks/types";
 import { OrderByDirection } from "@app/hooks/api/generic/types";
@@ -108,10 +107,11 @@ export const CmekTable = () => {
   });
 
   const { keys = [], totalCount = 0 } = data ?? {};
-  useEffect(() => {
+  useResetPageHelper({
-    // reset page if no longer valid
+    totalCount,
-    if (totalCount <= offset) setPage(1);
+    offset,
-  }, [totalCount]);
+    setPage
+  });
 
   const { popUp, handlePopUpOpen, handlePopUpToggle } = usePopUp([
     "upsertKey",

frontend/src/views/Project/MembersPage/components/IdentityTab/IdentityTab.tsx

@@ -1,4 +1,3 @@
-import { useEffect } from "react";
 import Link from "next/link";
 import {
   faArrowDown,
@@ -44,7 +43,7 @@ import {
 } from "@app/components/v2";
 import { ProjectPermissionActions, ProjectPermissionSub, useWorkspace } from "@app/context";
 import { withProjectPermission } from "@app/hoc";
-import { usePagination } from "@app/hooks";
+import { usePagination, useResetPageHelper } from "@app/hooks";
 import { useDeleteIdentityFromWorkspace, useGetWorkspaceIdentityMemberships } from "@app/hooks/api";
 import { OrderByDirection } from "@app/hooks/api/generic/types";
 import { IdentityMembership } from "@app/hooks/api/identities/types";
@@ -99,10 +98,11 @@ export const IdentityTab = withProjectPermission(
 
     const { totalCount = 0 } = data ?? {};
 
-    useEffect(() => {
+    useResetPageHelper({
-      // reset page if no longer valid
+      totalCount,
-      if (totalCount <= offset) setPage(1);
+      offset,
-    }, [totalCount]);
+      setPage
+    });
 
     const { mutateAsync: deleteMutateAsync } = useDeleteIdentityFromWorkspace();
 

frontend/src/views/SecretMainPage/SecretMainPage.tsx

@@ -16,7 +16,7 @@ import {
   useProjectPermission,
   useWorkspace
 } from "@app/context";
-import { useDebounce, usePagination, usePopUp } from "@app/hooks";
+import { useDebounce, usePagination, usePopUp, useResetPageHelper } from "@app/hooks";
 import {
   useGetImportedSecretsSingleEnv,
   useGetSecretApprovalPolicyOfABoard,
@@ -164,10 +164,11 @@ const SecretMainPageContent = () => {
     totalCount = 0
   } = data ?? {};
 
-  useEffect(() => {
+  useResetPageHelper({
-    // reset page if no longer valid
+    totalCount,
-    if (totalCount <= offset) setPage(1);
+    offset,
-  }, [totalCount]);
+    setPage
+  });
 
   // fetch imported secrets to show user the overriden ones
   const { data: importedSecrets } = useGetImportedSecretsSingleEnv({

frontend/src/views/SecretOverviewPage/SecretOverviewPage.tsx

@@ -55,7 +55,7 @@ import {
   useProjectPermission,
   useWorkspace
 } from "@app/context";
-import { useDebounce, usePagination, usePopUp } from "@app/hooks";
+import { useDebounce, usePagination, usePopUp, useResetPageHelper } from "@app/hooks";
 import {
   useCreateFolder,
   useCreateSecretV3,
@@ -213,10 +213,11 @@ export const SecretOverviewPage = () => {
     totalCount = 0
   } = overview ?? {};
 
-  useEffect(() => {
+  useResetPageHelper({
-    // reset page if no longer valid
+    totalCount,
-    if (totalCount <= offset) setPage(1);
+    offset,
-  }, [totalCount]);
+    setPage
+  });
 
   const { folderNames, getFolderByNameAndEnv, isFolderPresentInEnv } = useFolderOverview(folders);