diff --git a/jsx/src/components/Groups/Groups.jsx b/jsx/src/components/Groups/Groups.jsx index 052769fa8..3771ea24e 100644 --- a/jsx/src/components/Groups/Groups.jsx +++ b/jsx/src/components/Groups/Groups.jsx @@ -14,8 +14,7 @@ const Groups = (props) => { const dispatch = useDispatch(); const navigate = useNavigate(); - const { setOffset, offset, handleLimit, limit, setPagination } = - usePaginationParams(); + const { offset, handleLimit, limit, setPagination } = usePaginationParams(); const total = groups_page ? groups_page.total : undefined; @@ -32,11 +31,22 @@ const Groups = (props) => { }); }; + // single callback to reload the page + // uses current state, or params can be specified if state + // should be updated _after_ load, e.g. offset + const loadPageData = (params) => { + params = params || {}; + return updateGroups( + params.offset === undefined ? offset : params.offset, + params.limit === undefined ? limit : params.limit, + ) + .then((data) => dispatchPageUpdate(data.items, data._pagination)) + .catch((err) => setErrorAlert("Failed to update group list.")); + }; + useEffect(() => { - updateGroups(offset, limit).then((data) => - dispatchPageUpdate(data.items, data._pagination), - ); - }, [offset, limit]); + loadPageData(); + }, [limit]); if (!groups_data || !groups_page) { return
; @@ -72,8 +82,10 @@ const Groups = (props) => { limit={limit} visible={groups_data.length} total={total} - next={() => setOffset(offset + limit)} - prev={() => setOffset(offset - limit)} + next={() => loadPageData({ offset: offset + limit })} + prev={() => + loadPageData({ offset: limit > offset ? 0 : offset - limit }) + } handleLimit={handleLimit} /> diff --git a/jsx/src/components/Groups/Groups.test.js b/jsx/src/components/Groups/Groups.test.js index 025a5c031..dd5ed172a 100644 --- a/jsx/src/components/Groups/Groups.test.js +++ b/jsx/src/components/Groups/Groups.test.js @@ -112,8 +112,8 @@ test("Renders nothing if required data is not available", async () => { expect(noShow).toBeVisible(); }); -test("Interacting with PaginationFooter causes state update and refresh via useEffect call", async () => { - let upgradeGroupsSpy = mockAsync(); +test("Interacting with PaginationFooter causes page refresh", async () => { + let updateGroupsSpy = mockAsync(); let setSearchParamsSpy = mockAsync(); let searchParams = new URLSearchParams({ limit: "2" }); useSearchParams.mockImplementation(() => [ @@ -125,11 +125,11 @@ test("Interacting with PaginationFooter causes state update and refresh via useE ]); let _, setSearchParams; await act(async () => { - render(groupsJsx(upgradeGroupsSpy)); + render(groupsJsx(updateGroupsSpy)); [_, setSearchParams] = useSearchParams(); }); - expect(upgradeGroupsSpy).toBeCalledWith(0, 2); + expect(updateGroupsSpy).toBeCalledWith(0, 2); var lastState = mockReducers.mock.results[mockReducers.mock.results.length - 1].value; @@ -140,9 +140,7 @@ test("Interacting with PaginationFooter causes state update and refresh via useE await act(async () => { fireEvent.click(next); }); - expect(setSearchParamsSpy).toBeCalledWith("limit=2&offset=2"); - - // FIXME: mocked useSelector, state seem to prevent updateGroups from being called - // making the test environment not representative - // expect(callbackSpy).toHaveBeenCalledWith(2, 2); + expect(updateGroupsSpy).toBeCalledWith(2, 2); + // mocked updateGroups means callback after load doesn't fire + // expect(setSearchParamsSpy).toBeCalledWith("limit=2&offset=2"); }); diff --git a/jsx/src/components/ServerDashboard/ServerDashboard.jsx b/jsx/src/components/ServerDashboard/ServerDashboard.jsx index 7d6f2d218..d1b0a3a0e 100644 --- a/jsx/src/components/ServerDashboard/ServerDashboard.jsx +++ b/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,26 +123,39 @@ 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 = () => - updateUsers({ - offset, + const updateUsersWithParams = (params) => { + if (params) { + if (params.offset !== undefined && params.offset < 0) { + params.offset = 0; + } + } + return updateUsers({ + offset: offset, limit, name_filter, sort, state: state_filter, + ...params, }); + }; - useEffect(() => { - updateUsersWithParams() + // single callback to reload the page + // 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.")); - }, [offset, limit, name_filter, sort, state_filter]); + }; + + useEffect(() => { + loadPageData(); + }, [limit, name_filter, sort, state_filter]); if (!user_data || !user_page) { return ; @@ -172,14 +185,7 @@ const ServerDashboard = (props) => { action(user.name, server.name) .then((res) => { if (res.status < 300) { - updateUsersWithParams() - .then((data) => { - dispatchPageUpdate(data.items, data._pagination); - }) - .catch(() => { - setIsDisabled(false); - setErrorAlert(`Failed to update users list.`); - }); + loadPageData(); } else { setErrorAlert(`Failed to ${name.toLowerCase()}.`); setIsDisabled(false); @@ -519,13 +525,7 @@ const ServerDashboard = (props) => { return res; }) .then((res) => { - updateUsersWithParams() - .then((data) => { - dispatchPageUpdate(data.items, data._pagination); - }) - .catch(() => - setErrorAlert(`Failed to update users list.`), - ); + loadPageData(); return res; }) .catch(() => setErrorAlert(`Failed to start servers.`)); @@ -556,13 +556,7 @@ const ServerDashboard = (props) => { return res; }) .then((res) => { - updateUsersWithParams() - .then((data) => { - dispatchPageUpdate(data.items, data._pagination); - }) - .catch(() => - setErrorAlert(`Failed to update users list.`), - ); + loadPageData(); 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)} - prev={() => setOffset(offset - limit)} + // don't trigger via setOffset state change, + // 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} />