diff --git a/jsx/package-lock.json b/jsx/package-lock.json index e4d75755..ff70759a 100644 --- a/jsx/package-lock.json +++ b/jsx/package-lock.json @@ -14,7 +14,7 @@ "lodash": "^4.17.21", "prop-types": "^15.8.1", "react": "^17.0.2", - "react-bootstrap": "^2.7.4", + "react-bootstrap": "^2.10.1", "react-dom": "^17.0.2", "react-icons": "^4.8.0", "react-multi-select-component": "^4.3.4", @@ -8209,13 +8209,14 @@ } }, "node_modules/react-bootstrap": { - "version": "2.7.4", - "license": "MIT", + "version": "2.10.1", + "resolved": "https://registry.npmjs.org/react-bootstrap/-/react-bootstrap-2.10.1.tgz", + "integrity": "sha512-J3OpRZIvCTQK+Tg/jOkRUvpYLHMdGeU9KqFUBQrV0d/Qr/3nsINpiOJyZMWnM5SJ3ctZdhPA6eCIKpEJR3Ellg==", "dependencies": { - "@babel/runtime": "^7.21.0", + "@babel/runtime": "^7.22.5", "@restart/hooks": "^0.4.9", - "@restart/ui": "^1.6.3", - "@types/react-transition-group": "^4.4.5", + "@restart/ui": "^1.6.6", + "@types/react-transition-group": "^4.4.6", "classnames": "^2.3.2", "dom-helpers": "^5.2.1", "invariant": "^2.2.4", diff --git a/jsx/package.json b/jsx/package.json index 62c954c2..14a93582 100644 --- a/jsx/package.json +++ b/jsx/package.json @@ -34,7 +34,7 @@ "lodash": "^4.17.21", "prop-types": "^15.8.1", "react": "^17.0.2", - "react-bootstrap": "^2.7.4", + "react-bootstrap": "^2.10.1", "react-dom": "^17.0.2", "react-icons": "^4.8.0", "react-multi-select-component": "^4.3.4", diff --git a/jsx/src/components/PaginationFooter/PaginationFooter.jsx b/jsx/src/components/PaginationFooter/PaginationFooter.jsx index d0a8e8f8..803c85c8 100644 --- a/jsx/src/components/PaginationFooter/PaginationFooter.jsx +++ b/jsx/src/components/PaginationFooter/PaginationFooter.jsx @@ -5,11 +5,12 @@ import { FormControl } from "react-bootstrap"; import "./pagination-footer.css"; const PaginationFooter = (props) => { - let { offset, limit, visible, total, next, prev, handleLimit } = props; + const { offset, limit, visible, total, next, prev, handleLimit } = props; return (

- Displaying {offset + 1}-{offset + visible} {total ? `of ${total}` : ""} + Displaying {visible ? offset + 1 : offset}-{offset + visible}{" "} + {total ? `of ${total}` : ""}
{offset >= 1 ? ( - + ); }; @@ -337,8 +364,8 @@ const ServerDashboard = (props) => { + - , { onChange={handleSearch} /> + + {/* div.checkbox required for BS3 CSS */} +

+ +
+ {"> Manage Groups"} @@ -417,43 +462,23 @@ const ServerDashboard = (props) => { User{" "} setSortMethod(() => method)} + currentSort={sort} + setSort={setSort} + sortKey="name" testid="user-sort" /> - - Admin{" "} - setSortMethod(() => method)} - testid="admin-sort" - /> - - - Server{" "} - setSortMethod(() => method)} - testid="server-sort" - /> - + Admin + Server Last Activity{" "} setSortMethod(() => method)} + currentSort={sort} + setSort={setSort} + sortKey="last_activity" testid="last-activity-sort" /> - - Running{" "} - setSortMethod(() => method)} - testid="running-status-sort" - /> - Actions @@ -466,14 +491,13 @@ const ServerDashboard = (props) => { - - - + {/* Start all servers */} - - + {/* spacing between start/stop and Shutdown */} + {/* Shutdown Jupyterhub */} - {servers.flatMap(([user, server]) => serverRow(user, server))} @@ -590,30 +614,27 @@ ServerDashboard.propTypes = { }; const SortHandler = (props) => { - var { sorts, callback, testid } = props; - - var [direction, setDirection] = useState(undefined); + const { currentSort, setSort, sortKey, testid } = props; + const currentlySorted = currentSort && currentSort.endsWith(sortKey); + const descending = currentSort && currentSort.startsWith("-"); return (
{ - if (!direction) { - callback(sorts.desc); - setDirection("desc"); - } else if (direction == "asc") { - callback(sorts.desc); - setDirection("desc"); + if (!currentlySorted) { + setSort(sortKey); + } else if (descending) { + setSort(sortKey); } else { - callback(sorts.asc); - setDirection("asc"); + setSort("-" + sortKey); } }} > - {!direction ? ( + {!currentlySorted ? ( - ) : direction == "asc" ? ( + ) : descending ? ( ) : ( @@ -623,8 +644,9 @@ const SortHandler = (props) => { }; SortHandler.propTypes = { - sorts: PropTypes.object, - callback: PropTypes.func, + currentSort: PropTypes.string, + setSort: PropTypes.func, + sortKey: PropTypes.string, testid: PropTypes.string, }; diff --git a/jsx/src/components/ServerDashboard/ServerDashboard.test.js b/jsx/src/components/ServerDashboard/ServerDashboard.test.js index 101842f9..e3b660d1 100644 --- a/jsx/src/components/ServerDashboard/ServerDashboard.test.js +++ b/jsx/src/components/ServerDashboard/ServerDashboard.test.js @@ -34,9 +34,8 @@ const serverDashboardJsx = (props) => { // spies is a dict of properties to mock in // any API calls that will fire during the test should be mocked props = props || {}; - const defaultSpy = mockAsync(); if (!props.updateUsers) { - props.updateUsers = defaultSpy; + props.updateUsers = mockUpdateUsers; } return ( @@ -55,6 +54,14 @@ var mockAsync = (data) => var mockAsyncRejection = () => jest.fn().mockImplementation(() => Promise.reject()); +const defaultUpdateUsersParams = { + offset: 0, + limit: 2, + name_filter: "", + sort: "id", + state: "", +}; + var bar_servers = { "": { name: "", @@ -80,44 +87,64 @@ var bar_servers = { }, }; +/* create new user models */ +const newUser = (name) => { + return { + kind: "user", + name: name, + admin: false, + groups: [], + server: `/user/${name}`, + created: "2020-12-07T18:46:27.112695Z", + last_activity: "2020-12-07T21:00:33.336354Z", + servers: {}, + }; +}; + +const allUsers = [ + { + kind: "user", + name: "foo", + admin: true, + groups: [], + server: "/user/foo/", + pending: null, + created: "2020-12-07T18:46:27.112695Z", + last_activity: "2020-12-07T21:00:33.336354Z", + servers: { + "": { + name: "", + last_activity: "2020-12-07T20:58:02.437408Z", + started: "2020-12-07T20:58:01.508266Z", + pending: null, + ready: true, + state: { pid: 28085 }, + url: "/user/foo/", + user_options: {}, + progress_url: "/hub/api/users/foo/server/progress", + }, + }, + }, + { + kind: "user", + name: "bar", + admin: false, + groups: [], + server: null, + pending: null, + created: "2020-12-07T18:46:27.115528Z", + last_activity: "2020-12-07T20:43:51.013613Z", + servers: bar_servers, + }, +]; + +for (var i = 2; i < 10; i++) { + allUsers.push(newUser(`test-${i}`)); +} + var mockAppState = () => Object.assign({}, initialState, { - user_data: [ - { - kind: "user", - name: "foo", - admin: true, - groups: [], - server: "/user/foo/", - pending: null, - created: "2020-12-07T18:46:27.112695Z", - last_activity: "2020-12-07T21:00:33.336354Z", - servers: { - "": { - name: "", - last_activity: "2020-12-07T20:58:02.437408Z", - started: "2020-12-07T20:58:01.508266Z", - pending: null, - ready: true, - state: { pid: 28085 }, - url: "/user/foo/", - user_options: {}, - progress_url: "/hub/api/users/foo/server/progress", - }, - }, - }, - { - kind: "user", - name: "bar", - admin: false, - groups: [], - server: null, - pending: null, - created: "2020-12-07T18:46:27.115528Z", - last_activity: "2020-12-07T20:43:51.013613Z", - servers: bar_servers, - }, - ], + user_data: allUsers.slice(0, 2), user_page: { offset: 0, limit: 2, @@ -125,7 +152,7 @@ var mockAppState = () => next: { offset: 2, limit: 2, - url: "http://localhost:8000/hub/api/groups?offset=2&limit=2", + url: "http://localhost:8000/hub/api/users?offset=2&limit=2", }, }, }); @@ -143,6 +170,40 @@ var mockReducers = jest.fn((state, action) => { return state; }); +let mockUpdateUsers = jest.fn(({ offset, limit, sort, name_filter, state }) => { + /* mock updating users + + this has tom implement the server-side filtering, sorting, etc. + (at least whatever we want to test of it) + */ + let matchingUsers = allUsers; + if (state === "active") { + // only first user is active + matchingUsers = allUsers.slice(0, 1); + } + if (name_filter) { + matchingUsers = matchingUsers.filter((user) => + user.name.startsWith(name_filter), + ); + } + + const total = matchingUsers.length; + const items = matchingUsers.slice(offset, offset + limit); + + return Promise.resolve({ + items: items, + _pagination: { + offset: offset, + limit: limit, + total: total, + next: { + offset: offset + limit, + limit: limit, + }, + }, + }); +}); + let searchParams = new URLSearchParams(); beforeEach(() => { @@ -151,6 +212,7 @@ beforeEach(() => { return callback(mockAppState()); }); searchParams = new URLSearchParams(); + searchParams.set("limit", "2"); useSearchParams.mockImplementation(() => [ searchParams, @@ -164,6 +226,7 @@ afterEach(() => { useSearchParams.mockClear(); useSelector.mockClear(); mockReducers.mockClear(); + mockUpdateUsers.mockClear(); jest.runAllTimers(); }); @@ -267,71 +330,93 @@ test("Invokes the shutdownHub event on button click", async () => { }); test("Sorts according to username", async () => { + let rerender; + const testId = "user-sort"; await act(async () => { - render(serverDashboardJsx()); + rerender = render(serverDashboardJsx()).rerender; }); - let handler = screen.getByTestId("user-sort"); + expect(searchParams.get("sort")).toEqual(null); + let handler = screen.getByTestId(testId); fireEvent.click(handler); + expect(searchParams.get("sort")).toEqual("name"); - let first = screen.getAllByTestId("user-row-name")[0]; - expect(first.textContent).toContain("bar"); - - fireEvent.click(handler); - - first = screen.getAllByTestId("user-row-name")[0]; - expect(first.textContent).toContain("foo"); -}); - -test("Sorts according to admin", async () => { await act(async () => { - render(serverDashboardJsx()); + rerender(serverDashboardJsx()); + handler = screen.getByTestId(testId); }); - let handler = screen.getByTestId("admin-sort"); fireEvent.click(handler); + expect(searchParams.get("sort")).toEqual("-name"); - let first = screen.getAllByTestId("user-row-admin")[0]; - expect(first.textContent).toBe("admin"); + await act(async () => { + rerender(serverDashboardJsx()); + handler = screen.getByTestId(testId); + }); fireEvent.click(handler); - - first = screen.getAllByTestId("user-row-admin")[0]; - expect(first.textContent).toBe(""); + expect(searchParams.get("sort")).toEqual("name"); }); test("Sorts according to last activity", async () => { + let rerender; + const testId = "last-activity-sort"; await act(async () => { - render(serverDashboardJsx()); + rerender = render(serverDashboardJsx()).rerender; }); - let handler = screen.getByTestId("last-activity-sort"); + expect(searchParams.get("sort")).toEqual(null); + let handler = screen.getByTestId(testId); fireEvent.click(handler); + expect(searchParams.get("sort")).toEqual("last_activity"); - let first = screen.getAllByTestId("user-row-name")[0]; - expect(first.textContent).toContain("foo"); + await act(async () => { + rerender(serverDashboardJsx()); + handler = screen.getByTestId(testId); + }); fireEvent.click(handler); + expect(searchParams.get("sort")).toEqual("-last_activity"); - first = screen.getAllByTestId("user-row-name")[0]; - expect(first.textContent).toContain("bar"); + await act(async () => { + rerender(serverDashboardJsx()); + handler = screen.getByTestId(testId); + }); + + fireEvent.click(handler); + expect(searchParams.get("sort")).toEqual("last_activity"); }); -test("Sorts according to server status (running/not running)", async () => { +test("Filter according to server status (running/not running)", async () => { + let rerender; await act(async () => { - render(serverDashboardJsx()); + rerender = render(serverDashboardJsx()).rerender; }); - - let handler = screen.getByTestId("running-status-sort"); + console.log(rerender); + console.log("begin test"); + const label = "only active servers"; + let handler = screen.getByLabelText(label); + expect(handler.checked).toEqual(false); fireEvent.click(handler); - let first = screen.getAllByTestId("user-row-name")[0]; - expect(first.textContent).toContain("foo"); + // FIXME: need to force a rerender to get updated checkbox + // I don't think this should be required + await act(async () => { + rerender(serverDashboardJsx()); + handler = screen.getByLabelText(label); + }); + expect(searchParams.get("state")).toEqual("active"); + expect(handler.checked).toEqual(true); fireEvent.click(handler); - first = screen.getAllByTestId("user-row-name")[0]; - expect(first.textContent).toContain("bar"); + await act(async () => { + rerender(serverDashboardJsx()); + handler = screen.getByLabelText(label); + }); + handler = screen.getByLabelText(label); + expect(handler.checked).toEqual(false); + expect(searchParams.get("state")).toEqual(null); }); test("Shows server details with button click", async () => { @@ -494,23 +579,9 @@ test("Shows a UI error dialogue when stop user server returns an improper status test("Search for user calls updateUsers with name filter", async () => { let spy = mockAsync(); - let mockUpdateUsers = jest.fn((offset, limit, name_filter) => { - return Promise.resolve({ - items: [], - _pagination: { - offset: offset, - limit: limit, - total: offset + limit * 2, - next: { - offset: offset + limit, - limit: limit, - }, - }, - }); - }); await act(async () => { searchParams.set("offset", "2"); - render(serverDashboardJsx({ updateUsers: mockUpdateUsers })); + render(serverDashboardJsx()); }); let search = screen.getByLabelText("user-search"); @@ -538,17 +609,15 @@ test("Search for user calls updateUsers with name filter", async () => { }); test("Interacting with PaginationFooter causes state update and refresh via useEffect call", async () => { - let updateUsers = mockAsync(); - await act(async () => { - render(serverDashboardJsx({ updateUsers: updateUsers })); + render(serverDashboardJsx()); }); - expect(updateUsers).toBeCalledWith(0, 100, ""); + expect(mockUpdateUsers).toBeCalledWith(defaultUpdateUsersParams); var n = 3; expect(searchParams.get("offset")).toEqual(null); - expect(searchParams.get("limit")).toEqual(null); + expect(searchParams.get("limit")).toEqual("2"); let next = screen.getByTestId("paginate-next"); await act(async () => { @@ -556,8 +625,8 @@ test("Interacting with PaginationFooter causes state update and refresh via useE jest.runAllTimers(); }); - expect(searchParams.get("offset")).toEqual("100"); - expect(searchParams.get("limit")).toEqual(null); + expect(searchParams.get("offset")).toEqual("2"); + expect(searchParams.get("limit")).toEqual("2"); // FIXME: should call updateUsers, does in reality. // tests don't reflect reality due to mocked state/useSelector @@ -590,12 +659,9 @@ test("Start server and confirm pending state", async () => { ); }); - let mockUpdateUsers = jest.fn(() => Promise.resolve(mockAppState())); - await act(async () => { render( serverDashboardJsx({ - updateUsers: mockUpdateUsers, startServer: mockStartServer, }), ); @@ -604,16 +670,17 @@ test("Start server and confirm pending state", async () => { let actions = screen.getAllByTestId("user-row-server-activity")[1]; let buttons = getAllByRole(actions, "button"); - expect(buttons.length).toBe(2); + expect(buttons.length).toBe(3); expect(buttons[0].textContent).toBe("Start Server"); expect(buttons[1].textContent).toBe("Spawn Page"); + expect(buttons[2].textContent).toBe("Edit User"); await act(async () => { fireEvent.click(buttons[0]); }); expect(mockUpdateUsers.mock.calls).toHaveLength(1); - expect(buttons.length).toBe(2); + expect(buttons.length).toBe(3); expect(buttons[0].textContent).toBe("Start Server"); expect(buttons[0]).toBeDisabled(); expect(buttons[1].textContent).toBe("Spawn Page"); diff --git a/jsx/src/components/ServerDashboard/server-dashboard.css b/jsx/src/components/ServerDashboard/server-dashboard.css index 11ba4f92..2e4beab2 100644 --- a/jsx/src/components/ServerDashboard/server-dashboard.css +++ b/jsx/src/components/ServerDashboard/server-dashboard.css @@ -7,6 +7,13 @@ margin-left: auto; } +.btn-light { + /* backport bs5 btn-light colors */ + background-color: #f9fafb; + border-color: #f9fafb; + color: #000; +} + .server-dashboard-container .btn-light { border: 1px solid #ddd; } @@ -43,6 +50,47 @@ tr.noborder > td { vertical-align: inherit; } -.user-row .actions > * { - margin-right: 5px; +.user-row .actions button { + margin: 4px; +} + +.admin-header-buttons { + /* float header action buttons to the right */ + text-align: right; +} + +/* column widths for dashboard + +goals: + +- want stable width for running-status + so clicking the running filter doesn't cause a jump +- shrink fixed-content columns (action, admin) +- allow variable content columns (username, server name) + to claim remaining space + +*/ + +.admin-table-head #user-header { +} +.admin-table-head #admin-header { + width: 64px; +} +.admin-table-head #last-activity-header { + min-width: 180px; +} +.admin-table-head #actions-header { + width: 350px; +} + +/* vertical stack server buttons on small windows */ +@media (max-width: 991px) { + .admin-table-head #actions-header { + width: 140px; + } + .user-row .actions button { + /* full-width buttons when they get collapsed into a single column */ + margin: 4px 0px 4px 0px; + width: 100%; + } } diff --git a/jsx/src/util/jhapiUtil.js b/jsx/src/util/jhapiUtil.js index 99861057..5616b036 100644 --- a/jsx/src/util/jhapiUtil.js +++ b/jsx/src/util/jhapiUtil.js @@ -3,14 +3,11 @@ const base_url = jhdata.base_url || "/"; const xsrfToken = jhdata.xsrf_token; export const jhapiRequest = (endpoint, method, data) => { - let api_url = `${base_url}hub/api`; - let suffix = ""; + let api_url = new URL(`${base_url}hub/api` + endpoint, location.origin); if (xsrfToken) { - // add xsrf token to url parameter - var sep = endpoint.indexOf("?") === -1 ? "?" : "&"; - suffix = sep + "_xsrf=" + xsrfToken; + api_url.searchParams.set("_xsrf", xsrfToken); } - return fetch(api_url + endpoint + suffix, { + return fetch(api_url, { method: method, json: true, headers: { diff --git a/jsx/src/util/paginationParams.js b/jsx/src/util/paginationParams.js index 69b32d85..d1da95dd 100644 --- a/jsx/src/util/paginationParams.js +++ b/jsx/src/util/paginationParams.js @@ -17,7 +17,7 @@ export const usePaginationParams = () => { } }; const _setLimit = (params, limit) => { - if (limit < 10) limit = 10; + if (limit < 1) limit = 1; if (limit === window.api_page_limit) { params.delete("limit"); } else { diff --git a/jsx/src/util/withAPI.js b/jsx/src/util/withAPI.js index 1b49f46a..db9bf762 100644 --- a/jsx/src/util/withAPI.js +++ b/jsx/src/util/withAPI.js @@ -2,13 +2,16 @@ import { withProps } from "recompose"; import { jhapiRequest } from "./jhapiUtil"; const withAPI = withProps(() => ({ - updateUsers: (offset, limit, name_filter) => - jhapiRequest( - `/users?include_stopped_servers&offset=${offset}&limit=${limit}&name_filter=${ - name_filter || "" - }`, - "GET", - ).then((data) => data.json()), + updateUsers: (options) => { + let params = new URLSearchParams(); + params.set("include_stopped_servers", "1"); + for (let key in options) { + params.set(key, options[key]); + } + return jhapiRequest(`/users?${params.toString()}`, "GET").then((data) => + data.json(), + ); + }, updateGroups: (offset, limit) => jhapiRequest(`/groups?offset=${offset}&limit=${limit}`, "GET").then( (data) => data.json(), diff --git a/jupyterhub/apihandlers/base.py b/jupyterhub/apihandlers/base.py index 5ee37450..94e7b0ef 100644 --- a/jupyterhub/apihandlers/base.py +++ b/jupyterhub/apihandlers/base.py @@ -352,6 +352,10 @@ class APIHandler(BaseHandler): if include_stopped_servers: # add any stopped servers in the db seen = set(servers.keys()) + if isinstance(user, orm.User): + # need high-level User wrapper for spawner model + # FIXME: this shouldn't be needed! + user = self.users[user] for name, orm_spawner in user.orm_spawners.items(): if name not in seen and scope_filter(orm_spawner, kind='server'): servers[name] = self.server_model(orm_spawner, user=user) diff --git a/jupyterhub/tests/browser/test_browser.py b/jupyterhub/tests/browser/test_browser.py index 932b9c4c..9054138a 100644 --- a/jupyterhub/tests/browser/test_browser.py +++ b/jupyterhub/tests/browser/test_browser.py @@ -1116,8 +1116,9 @@ async def test_search_on_admin_page( displaying = browser.get_by_text("Displaying") if users_count_db_filtered <= 50: await expect(filtered_list_on_page).to_have_count(users_count_db_filtered) + start = 1 if users_count_db_filtered else 0 await expect(displaying).to_contain_text( - re.compile(f"1-{users_count_db_filtered}") + re.compile(f"{start}-{users_count_db_filtered}") ) # check that users names contain the search value in the filtered list for element in await filtered_list_on_page.get_by_test_id(