From 78a796cea68f2b99a96b37948eac879681595552 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 6 Mar 2024 23:16:43 +0100 Subject: [PATCH 1/8] server-side sorting of admin page removes in-page sort, which removes sort by server name, sort by running Running column switches from sort to filter, matching the `?state` query parameter in the API needs some CSS on the column widths to avoid jumps when toggling active servers --- jsx/package-lock.json | 13 +- jsx/package.json | 2 +- .../ServerDashboard/ServerDashboard.jsx | 155 ++++++++++-------- .../ServerDashboard/server-dashboard.css | 48 ++++++ jsx/src/util/withAPI.js | 17 +- 5 files changed, 154 insertions(+), 81 deletions(-) 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/ServerDashboard/ServerDashboard.jsx b/jsx/src/components/ServerDashboard/ServerDashboard.jsx index 6ffe7619..19e7f0e9 100644 --- a/jsx/src/components/ServerDashboard/ServerDashboard.jsx +++ b/jsx/src/components/ServerDashboard/ServerDashboard.jsx @@ -7,6 +7,7 @@ import { Button, Col, Row, + Form, FormControl, Card, CardGroup, @@ -32,21 +33,6 @@ RowListItem.propTypes = { const ServerDashboard = (props) => { const base_url = window.base_url || "/"; const [searchParams, setSearchParams] = useSearchParams(); - // sort methods - const usernameDesc = (e) => e.sort((a, b) => (a.name > b.name ? 1 : -1)), - usernameAsc = (e) => e.sort((a, b) => (a.name < b.name ? 1 : -1)), - adminDesc = (e) => e.sort((a) => (a.admin ? -1 : 1)), - adminAsc = (e) => e.sort((a) => (a.admin ? 1 : -1)), - dateDesc = (e) => - e.sort((a, b) => - new Date(a.last_activity) - new Date(b.last_activity) > 0 ? -1 : 1, - ), - dateAsc = (e) => - e.sort((a, b) => - new Date(a.last_activity) - new Date(b.last_activity) > 0 ? 1 : -1, - ), - runningAsc = (e) => e.sort((a) => (a.server == null ? -1 : 1)), - runningDesc = (e) => e.sort((a) => (a.server == null ? 1 : -1)); const [errorAlert, setErrorAlert] = useState(null); const [sortMethod, setSortMethod] = useState(null); @@ -59,6 +45,8 @@ const ServerDashboard = (props) => { usePaginationParams(); const name_filter = searchParams.get("name_filter") || ""; + const sort = searchParams.get("sort") || "id"; + const state_filter = searchParams.get("state") || ""; const total = user_page ? user_page.total : undefined; @@ -76,7 +64,13 @@ const ServerDashboard = (props) => { } = props; const dispatchPageUpdate = (data, page) => { + // trigger page update in state + // in response to fetching updated user list + // data is list of user records + // page is _pagination part of response + // persist page info in url query setPagination(page); + // persist user data, triggers rerender dispatch({ type: "USER_PAGE", value: { @@ -87,6 +81,8 @@ const ServerDashboard = (props) => { }; const setNameFilter = (new_name_filter) => { + // persist ?name_filter parameter + // store in url param, clear when value is empty setSearchParams((params) => { // clear offset when name filter changes if (new_name_filter !== name_filter) { @@ -101,26 +97,56 @@ const ServerDashboard = (props) => { }); }; + const setSort = (sort) => { + // persist ?sort parameter + // store in url param, clear when value is default ('id') + setSearchParams((params) => { + if (sort === "id") { + params.delete("id"); + } else { + params.set("sort", sort); + } + return params; + }); + }; + + const setStateFilter = (state_filter) => { + // persist ?state filter + // store in url param, clear when value is default ('') + setSearchParams((params) => { + if (!state_filter) { + params.delete("state"); + } else { + params.set("state", state_filter); + } + return params; + }); + }; + + // the callback to update the displayed user list + const updateUsersWithParams = () => + updateUsers({ + offset, + limit, + name_filter, + sort, + state: state_filter, + }); + useEffect(() => { - updateUsers(offset, limit, name_filter) + updateUsersWithParams() .then((data) => dispatchPageUpdate(data.items, data._pagination)) .catch((err) => setErrorAlert("Failed to update user list.")); - }, [offset, limit, name_filter]); + }, [offset, limit, name_filter, sort, state_filter]); if (!user_data || !user_page) { return
; } - var slice = [offset, limit, name_filter]; - const handleSearch = debounce(async (event) => { setNameFilter(event.target.value); }, 300); - if (sortMethod != null) { - user_data = sortMethod(user_data); - } - const ServerButton = ({ server, user, action, name, extraClass }) => { var [isDisabled, setIsDisabled] = useState(false); return ( @@ -132,7 +158,7 @@ const ServerDashboard = (props) => { action(user.name, server.name) .then((res) => { if (res.status < 300) { - updateUsers(...slice) + updateUsersWithParams() .then((data) => { dispatchPageUpdate(data.items, data._pagination); }) @@ -417,42 +443,39 @@ 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 @@ -490,7 +513,7 @@ const ServerDashboard = (props) => { return res; }) .then((res) => { - updateUsers(...slice) + updateUsersWithParams() .then((data) => { dispatchPageUpdate(data.items, data._pagination); }) @@ -526,7 +549,7 @@ const ServerDashboard = (props) => { return res; }) .then((res) => { - updateUsers(...slice) + updateUsersWithParams() .then((data) => { dispatchPageUpdate(data.items, data._pagination); }) @@ -590,30 +613,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 +643,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/server-dashboard.css b/jsx/src/components/ServerDashboard/server-dashboard.css index 11ba4f92..6f55de11 100644 --- a/jsx/src/components/ServerDashboard/server-dashboard.css +++ b/jsx/src/components/ServerDashboard/server-dashboard.css @@ -46,3 +46,51 @@ tr.noborder > td { .user-row .actions > * { margin-right: 5px; } + +.form-check-inline { + /* inline form check doesn't get inline css + I _think_ this is because our bootstrap css is outdated + */ + display: inline-block; +} + +/* 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 label { + /* clear margin-bottom to keep label on baseline */ + margin-bottom: 0px; +} + +.admin-table-head #user-header { +} +.admin-table-head #admin-header { + width: 64px; +} +.admin-table-head #server-header { +} +.admin-table-head #last-activity-header { + width: 180px; +} +.admin-table-head #running-status-header { + width: 300px; +} +.admin-table-head #actions-header { + width: 80px; +} + +/* vertical stack server buttons on small windows */ +@media (max-width: 991px) { + .admin-table-head #running-status-header { + width: 140px; + } +} diff --git a/jsx/src/util/withAPI.js b/jsx/src/util/withAPI.js index 1b49f46a..4938a35d 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["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(), From 8cbe1eac2b64c762992c211b272e988adb136521 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 8 Mar 2024 09:05:55 +0100 Subject: [PATCH 2/8] use URL api to construct API url avoids imperfect logic detecting `?` --- jsx/src/util/jhapiUtil.js | 9 +++------ jsx/src/util/withAPI.js | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) 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/withAPI.js b/jsx/src/util/withAPI.js index 4938a35d..db9bf762 100644 --- a/jsx/src/util/withAPI.js +++ b/jsx/src/util/withAPI.js @@ -4,7 +4,7 @@ import { jhapiRequest } from "./jhapiUtil"; const withAPI = withProps(() => ({ updateUsers: (options) => { let params = new URLSearchParams(); - params["include_stopped_servers"] = "1"; + params.set("include_stopped_servers", "1"); for (let key in options) { params.set(key, options[key]); } From bc3bb47672dd5b47c072d147d07a69ee18a92ef3 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 8 Mar 2024 09:06:46 +0100 Subject: [PATCH 3/8] pagination: fix offset display when there are 0 results --- jsx/src/components/PaginationFooter/PaginationFooter.jsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 ? ( - + ); }; @@ -363,8 +364,8 @@ const ServerDashboard = (props) => { + - , { onChange={handleSearch} /> + + {/* div.checkbox required for BS3 CSS */} +

+ +
+ {"> Manage Groups"} @@ -460,23 +479,6 @@ const ServerDashboard = (props) => { testid="last-activity-sort" /> - - - Actions @@ -489,14 +491,13 @@ const ServerDashboard = (props) => { - - - + {/* Start all servers */} - - + {/* spacing between start/stop and Shutdown */} + {/* Shutdown Jupyterhub */} - {servers.flatMap(([user, server]) => serverRow(user, server))} diff --git a/jsx/src/components/ServerDashboard/server-dashboard.css b/jsx/src/components/ServerDashboard/server-dashboard.css index 6f55de11..f66f629c 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; } @@ -47,11 +54,9 @@ tr.noborder > td { margin-right: 5px; } -.form-check-inline { - /* inline form check doesn't get inline css - I _think_ this is because our bootstrap css is outdated - */ - display: inline-block; +.admin-header-buttons { + /* float header action buttons to the right */ + text-align: right; } /* column widths for dashboard @@ -66,31 +71,21 @@ goals: */ -.admin-table-head label { - /* clear margin-bottom to keep label on baseline */ - margin-bottom: 0px; -} - .admin-table-head #user-header { } .admin-table-head #admin-header { width: 64px; } -.admin-table-head #server-header { -} .admin-table-head #last-activity-header { - width: 180px; -} -.admin-table-head #running-status-header { - width: 300px; + min-width: 180px; } .admin-table-head #actions-header { - width: 80px; + width: 350px; } /* vertical stack server buttons on small windows */ @media (max-width: 991px) { - .admin-table-head #running-status-header { + .admin-table-head #actions-header { width: 140px; } } From d87e2dd3ae81d01d44acf0ef1b2fa8331ef379eb Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 8 Mar 2024 09:50:31 +0100 Subject: [PATCH 5/8] fix GET /users?include_stopped_servers when users aren't in UserDict server model needs high-level User object for `progress_url` (it probably shouldn't) --- jupyterhub/apihandlers/base.py | 4 ++++ 1 file changed, 4 insertions(+) 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) From f47d0a15246161f4e198595ea042511cb1b03e26 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 8 Mar 2024 10:08:11 +0100 Subject: [PATCH 6/8] space all the way around action buttons 4px margin matches 8px cell padding (margin is added on both sides) and center when buttons collapse to single column --- jsx/src/components/ServerDashboard/server-dashboard.css | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/jsx/src/components/ServerDashboard/server-dashboard.css b/jsx/src/components/ServerDashboard/server-dashboard.css index f66f629c..2e4beab2 100644 --- a/jsx/src/components/ServerDashboard/server-dashboard.css +++ b/jsx/src/components/ServerDashboard/server-dashboard.css @@ -50,8 +50,8 @@ tr.noborder > td { vertical-align: inherit; } -.user-row .actions > * { - margin-right: 5px; +.user-row .actions button { + margin: 4px; } .admin-header-buttons { @@ -88,4 +88,9 @@ goals: .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%; + } } From d0665a9f2162b0375ea40ad1d2228703da6449f0 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 8 Mar 2024 12:01:38 +0100 Subject: [PATCH 7/8] update tests for sort, state filter --- .../ServerDashboard/ServerDashboard.jsx | 6 +- .../ServerDashboard/ServerDashboard.test.js | 265 +++++++++++------- jsx/src/util/paginationParams.js | 2 +- 3 files changed, 170 insertions(+), 103 deletions(-) diff --git a/jsx/src/components/ServerDashboard/ServerDashboard.jsx b/jsx/src/components/ServerDashboard/ServerDashboard.jsx index 7ca387e3..c6fd4983 100644 --- a/jsx/src/components/ServerDashboard/ServerDashboard.jsx +++ b/jsx/src/components/ServerDashboard/ServerDashboard.jsx @@ -35,7 +35,6 @@ const ServerDashboard = (props) => { const [searchParams, setSearchParams] = useSearchParams(); const [errorAlert, setErrorAlert] = useState(null); - const [sortMethod, setSortMethod] = useState(null); const [collapseStates, setCollapseStates] = useState({}); let user_data = useSelector((state) => state.user_data); @@ -123,6 +122,7 @@ const ServerDashboard = (props) => { } else { params.set("state", new_state_filter); } + console.log("setting search params", params.toString()); return params; }); }; @@ -491,7 +491,7 @@ const ServerDashboard = (props) => { - + {/* Start all servers */} {/* spacing between start/stop and Shutdown */} - + {/* Shutdown Jupyterhub */}