admin: don't use state change to update offset

set offset -> request page -> response sets offset is a recipe for races

instead, send request with new offset and only update offset state

made easier by consolidating page update requests into single loadPageData

jsx/src/components/ServerDashboard/ServerDashboard.jsx

@@ -41,7 +41,7 @@ const ServerDashboard = (props) => {
   let user_data = useSelector((state) => state.user_data);
   const user_page = useSelector((state) => state.user_page);
 
-  const { setOffset, offset, setLimit, handleLimit, limit, setPagination } =
+  const { offset, setLimit, handleLimit, limit, setPagination } =
     usePaginationParams();
 
   const name_filter = searchParams.get("name_filter") || "";
@@ -123,25 +123,38 @@ const ServerDashboard = (props) => {
       } else {
         params.set("state", new_state_filter);
       }
-      console.log("setting search params", params.toString());
       return params;
     });
   };
 
   // the callback to update the displayed user list
-  const updateUsersWithParams = () =>
+  const updateUsersWithParams = (params) => {
-    updateUsers({
+    if (params) {
-      offset,
+      if (params.offset !== undefined && params.offset < 0) {
+        params.offset = 0;
+      }
+    }
+    return updateUsers({
+      offset: offset,
       limit,
       name_filter,
       sort,
       state: state_filter,
+      ...params,
     });
+  };
 
-  useEffect(() => {
+  // single callback to reload the page
-    updateUsersWithParams()
+  // uses current state, or params can be specified if state
+  // should be updated _after_ load, e.g. offset
+  const loadPageData = (params) => {
+    return updateUsersWithParams(params)
       .then((data) => dispatchPageUpdate(data.items, data._pagination))
       .catch((err) => setErrorAlert("Failed to update user list."));
+  };
+
+  useEffect(() => {
+    loadPageData();
   }, [offset, limit, name_filter, sort, state_filter]);
 
   if (!user_data || !user_page) {
@@ -172,14 +185,7 @@ const ServerDashboard = (props) => {
           action(user.name, server.name)
             .then((res) => {
               if (res.status < 300) {
-                updateUsersWithParams()
+                loadPageData();
-                  .then((data) => {
-                    dispatchPageUpdate(data.items, data._pagination);
-                  })
-                  .catch(() => {
-                    setIsDisabled(false);
-                    setErrorAlert(`Failed to update users list.`);
-                  });
               } else {
                 setErrorAlert(`Failed to ${name.toLowerCase()}.`);
                 setIsDisabled(false);
@@ -519,13 +525,7 @@ const ServerDashboard = (props) => {
                         return res;
                       })
                       .then((res) => {
-                        updateUsersWithParams()
+                        loadPageData();
-                          .then((data) => {
-                            dispatchPageUpdate(data.items, data._pagination);
-                          })
-                          .catch(() =>
-                            setErrorAlert(`Failed to update users list.`),
-                          );
                         return res;
                       })
                       .catch(() => setErrorAlert(`Failed to start servers.`));
@@ -556,13 +556,7 @@ const ServerDashboard = (props) => {
                         return res;
                       })
                       .then((res) => {
-                        updateUsersWithParams()
+                        loadPageData();
-                          .then((data) => {
-                            dispatchPageUpdate(data.items, data._pagination);
-                          })
-                          .catch(() =>
-                            setErrorAlert(`Failed to update users list.`),
-                          );
                         return res;
                       })
                       .catch(() => setErrorAlert(`Failed to stop servers.`));
@@ -590,8 +584,13 @@ const ServerDashboard = (props) => {
           limit={limit}
           visible={user_data.length}
           total={total}
-          next={() => setOffset(offset + limit)}
+          // don't trigger via setOffset state change,
-          prev={() => setOffset(offset - limit)}
+          // which can cause infinite cycles.
+          // offset state will be set upon reply via setPagination
+          next={() => loadPageData({ offset: offset + limit })}
+          prev={() =>
+            loadPageData({ offset: limit > offset ? 0 : offset - limit })
+          }
           handleLimit={handleLimit}
         />
         <br></br>

jsx/src/components/ServerDashboard/ServerDashboard.test.js

@@ -608,7 +608,7 @@ test("Search for user calls updateUsers with name filter", async () => {
   // expect(mockUpdateUsers).toBeCalledWith(0, 100, "ab");
 });
 
-test("Interacting with PaginationFooter causes state update and refresh via useEffect call", async () => {
+test("Interacting with PaginationFooter requests page update", async () => {
   await act(async () => {
     render(serverDashboardJsx());
   });
@@ -625,14 +625,10 @@ test("Interacting with PaginationFooter causes state update and refresh via useE
     jest.runAllTimers();
   });
 
-  expect(searchParams.get("offset")).toEqual("2");
+  expect(mockUpdateUsers).toBeCalledWith({
-  expect(searchParams.get("limit")).toEqual("2");
+    ...defaultUpdateUsersParams,
-
+    offset: 2,
-  // FIXME: should call updateUsers, does in reality.
+  });
-  // tests don't reflect reality due to mocked state/useSelector
-  // unclear how to fix this.
-  // expect(callbackSpy.mock.calls).toHaveLength(2);
-  // expect(callbackSpy).toHaveBeenCalledWith(2, 2, "");
 });
 
 test("Server delete button exists for named servers", async () => {