Improve useTableRequest for better reactivity (#8206)

* use data from store directly * load active users only when no filter is set * return invited user email in users.invite response * shares
outline · Jan 8, 2025 · cf3e29b · cf3e29b
1 parent 92a5954
commit cf3e29b
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 31 deletions.
diff --git a/app/hooks/useTableRequest.ts b/app/hooks/useTableRequest.ts
@@ -1,4 +1,5 @@
-import sortBy from "lodash/sortBy";
+import { ColumnSort } from "@tanstack/react-table";
+import orderBy from "lodash/orderBy";
 import React from "react";
 import {
   FetchPageParams,
@@ -12,6 +13,7 @@ const PAGE_SIZE = 25;
 
 type Props<T> = {
   data: T[];
+  sort: ColumnSort;
   reqFn: (params: FetchPageParams) => Promise<PaginatedResponse<T>>;
   reqParams: Omit<FetchPageParams, "offset" | "limit">;
 };
@@ -25,13 +27,14 @@ type Response<T> = {
 
 export function useTableRequest<T extends { id: string }>({
   data,
+  sort,
   reqFn,
   reqParams,
 }: Props<T>): Response<T> {
-  const [dataIds, setDataIds] = React.useState<string[]>();
   const [total, setTotal] = React.useState<number>();
   const [offset, setOffset] = React.useState({ value: INITIAL_OFFSET });
   const prevParamsRef = React.useRef(reqParams);
+  const sortRef = React.useRef<ColumnSort>(sort);
 
   const fetchPage = React.useCallback(
     () => reqFn({ ...reqParams, offset: offset.value, limit: PAGE_SIZE }),
@@ -48,6 +51,15 @@ export function useTableRequest<T extends { id: string }>({
     []
   );
 
+  const sortedData = data
+    ? orderBy(data, sortRef.current.id, sortRef.current.desc ? "desc" : "asc")
+    : undefined;
+
+  const next =
+    !loading && total && sortedData && sortedData.length < total
+      ? nextPage
+      : undefined;
+
   React.useEffect(() => {
     if (prevParamsRef.current !== reqParams) {
       prevParamsRef.current = reqParams;
@@ -63,14 +75,7 @@ export function useTableRequest<T extends { id: string }>({
         return;
       }
 
-      const ids = response.map((item) => item.id);
-
-      if (offset.value === INITIAL_OFFSET) {
-        setDataIds(response.map((item) => item.id));
-      } else {
-        setDataIds((prev) => (prev ?? []).concat(ids));
-      }
-
+      sortRef.current = sort; // Change sort once we receive a response from server - avoids flicker with stale data.
       setTotal(response[PAGINATION_SYMBOL]?.total);
     };
 
@@ -79,22 +84,10 @@ export function useTableRequest<T extends { id: string }>({
     return () => {
       ignore = true;
     };
-  }, [reqParams, offset, request]);
-
-  const filteredData = dataIds
-    ? sortBy(
-        data.filter((item) => dataIds.includes(item.id)),
-        (item) => dataIds.indexOf(item.id)
-      )
-    : undefined;
-
-  const next =
-    !loading && dataIds && total && dataIds.length < total
-      ? nextPage
-      : undefined;
+  }, [sort, reqParams, offset, request]);
 
   return {
-    data: filteredData,
+    data: sortedData,
     error,
     loading,
     next,

diff --git a/app/scenes/Settings/Members.tsx b/app/scenes/Settings/Members.tsx
@@ -7,7 +7,7 @@ import { useHistory, useLocation } from "react-router-dom";
 import { toast } from "sonner";
 import styled from "styled-components";
 import { depths, s } from "@shared/styles";
-import UsersStore from "~/stores/UsersStore";
+import UsersStore, { queriedUsers } from "~/stores/UsersStore";
 import { Action } from "~/components/Actions";
 import Button from "~/components/Button";
 import Fade from "~/components/Fade";
@@ -44,7 +44,7 @@ function Members() {
   const reqParams = React.useMemo(
     () => ({
       query: params.get("query") || undefined,
-      filter: params.get("filter") || undefined,
+      filter: params.get("filter") || "active",
       role: params.get("role") || undefined,
       sort: params.get("sort") || "name",
       direction: (params.get("direction") || "asc").toUpperCase() as
@@ -65,9 +65,11 @@ function Members() {
   const { data, error, loading, next } = useTableRequest({
     data: getFilteredUsers({
       users,
+      query: reqParams.query,
       filter: reqParams.filter,
       role: reqParams.role,
     }),
+    sort,
     reqFn: users.fetchPage,
     reqParams,
   });
@@ -181,10 +183,12 @@ function Members() {
 
 function getFilteredUsers({
   users,
+  query,
   filter,
   role,
 }: {
   users: UsersStore;
+  query?: string;
   filter?: string;
   role?: string;
 }) {
@@ -204,9 +208,15 @@ function getFilteredUsers({
       filteredUsers = users.active;
   }
 
-  return role
-    ? filteredUsers.filter((user) => user.role === role)
-    : filteredUsers;
+  if (role) {
+    filteredUsers = filteredUsers.filter((user) => user.role === role);
+  }
+
+  if (query) {
+    filteredUsers = queriedUsers(filteredUsers, query);
+  }
+
+  return filteredUsers;
 }
 
 const StickyFilters = styled(Flex)`

diff --git a/app/scenes/Settings/Shares.tsx b/app/scenes/Settings/Shares.tsx
@@ -45,6 +45,7 @@ function Shares() {
 
   const { data, error, loading, next } = useTableRequest({
     data: shares.orderedData,
+    sort,
     reqFn: shares.fetchPage,
     reqParams,
   });

diff --git a/app/stores/UsersStore.ts b/app/stores/UsersStore.ts
@@ -208,7 +208,7 @@ export default class UsersStore extends Store<User> {
   };
 }
 
-function queriedUsers(users: User[], query?: string) {
+export function queriedUsers(users: User[], query?: string) {
   const normalizedQuery = deburr((query || "").toLocaleLowerCase());
 
   return normalizedQuery

diff --git a/server/routes/api/users/users.ts b/server/routes/api/users/users.ts
@@ -545,6 +545,7 @@ router.post(
   validate(T.UsersInviteSchema),
   async (ctx: APIContext<T.UsersInviteReq>) => {
     const { invites } = ctx.input.body;
+    const actor = ctx.state.auth.user;
 
     if (invites.length > UserValidation.maxInvitesPerRequest) {
       throw ValidationError(
@@ -565,7 +566,9 @@ router.post(
     ctx.body = {
       data: {
         sent: response.sent,
-        users: response.users.map((user) => presentUser(user)),
+        users: response.users.map((user) =>
+          presentUser(user, { includeEmail: !!can(actor, "readEmail", user) })
+        ),
       },
     };
   }