improvement: org user groups tables search fixed and col sort added a…

…nd group add users pagination fixed and search improved to include first and last name
Infisical · Oct 18, 2024 · d677654 · d677654
1 parent b22a93a
commit d677654
Show file tree

Hide file tree

Showing 18 changed files with 247 additions and 81 deletions.
diff --git a/backend/src/ee/routes/v1/group-router.ts b/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({

diff --git a/backend/src/ee/services/access-approval-policy/access-approval-policy-service.ts b/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()

diff --git a/backend/src/ee/services/access-approval-request/access-approval-request-service.ts b/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({
-            orgId: actorOrgId,
-            groupId: groupApproverId
-          })
+          groupDAL
+            .findAllGroupPossibleMembers({
+              orgId: actorOrgId,
+              groupId: groupApproverId
+            })
+            .then((group) => group.members)
         )
       )
     ).flat();

diff --git a/backend/src/ee/services/group/group-dal.ts b/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) {
-        query = query.andWhere(`${TableName.Users}.username`, "ilike", `%${username}%`);
+      if (search) {
+        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(
-        ({ email, username: memberUsername, firstName, lastName, userId, groupId: memberGroupId }) => ({
-          id: userId,
-          email,
-          username: memberUsername,
-          firstName,
-          lastName,
-          isPartOfGroup: !!memberGroupId
-        })
-      );
+      return {
+        members: members.map(
+          ({ email, username: memberUsername, firstName, lastName, userId, groupId: memberGroupId }) => ({
+            id: userId,
+            email,
+            username: memberUsername,
+            firstName,
+            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" });
     }

diff --git a/backend/src/ee/services/group/group-service.ts b/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, totalCount: count };
+    return { users: members, totalCount };
   };
 
   const addUserToGroup = async ({ id, username, actor, actorId, actorAuthMethod, actorOrgId }: TAddUserToGroupDTO) => {

diff --git a/backend/src/ee/services/group/group-types.ts b/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 = {

diff --git a/backend/src/ee/services/scim/scim-service.ts b/backend/src/ee/services/scim/scim-service.ts
@@ -834,10 +834,12 @@ export const scimServiceFactory = ({
       });
     }
 
-    const users = await groupDAL.findAllGroupPossibleMembers({
-      orgId: group.orgId,
-      groupId: group.id
-    });
+    const users = await groupDAL
+      .findAllGroupPossibleMembers({
+        orgId: group.orgId,
+        groupId: group.id
+      })
+      .then((g) => g.members);
 
     const orgMemberships = await orgDAL.findMembership({
       [`${TableName.OrgMembership}.orgId` as "orgId"]: orgId,

diff --git a/backend/src/lib/api-docs/constants.ts b/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.",

diff --git a/frontend/src/hooks/api/groups/queries.tsx b/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;
-  }) => [...groupKeys.forGroupUserMemberships(slug), { offset, limit, username }] as const
+    search: string;
+  }) => [...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 }>(

diff --git a/frontend/src/hooks/index.ts b/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";
diff --git a/frontend/src/hooks/useResetPageHelper.ts b/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]);
+};
diff --git a/.../MembersPage/components/OrgGroupsTab/components/OrgGroupsSection/OrgGroupMembersModal.tsx b/.../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,17 +151,20 @@ 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)}
               onChangePerPage={(newPerPage) => setPerPage(newPerPage)}
             />
           )}
           {!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>