feat(ui): add column sorting to backend-ready data-tables#6558
Conversation
Wire user-controllable sort into the Devices, Containers, API Keys, and Device Chooser tables, mirroring the existing admin/devices pattern. Each clickable header toggles asc/desc, resets the page to 1, and forwards sort_by/order_by to the SDK query. The backing endpoints already accept these params, so no backend change is needed. Column keys were aligned to the backend sort fields (hostname -> name, created -> created_at, expires -> expires_in) since DataTable passes the column key as the sort field. Fixes: shellhub-io/team#165
|
Claude finished @luannmoreira's task in 12m 56s —— View job Code Review CompleteReviewed 11 files across code quality, security, testing, TypeScript patterns, and architecture — no new issues found. The code looks good as-is. Verified:
If you push additional changes and want a new review, tag |
|
Solid change overall. I was just thinking about extracting that sorting logic into a shared hook, because most of it is repeated in the pages: the field and order This would reduce the code repetition and also make it easier to integrate into other tables in the future, if any other gets the sorting logic in the back-end. Do you think it's valid? |
@luannmoreira Please verify if we can implement it this way, so we avoid duplicating all that. |
The sort field/order state and the asc/desc toggle (defaulting to ascending on name fields) were duplicated across the devices, containers, API keys, device-chooser, and admin devices tables. Move the logic into a generic useTableSort hook so new sortable tables reuse it instead of copying the handler.
|
Good call @luizhf42 — addressed in 64c3e4e. Extracted the repeated state into a generic const { sortBy, orderBy, handleSort } = useTableSort<SortField>({
defaultField: "last_seen",
onSortChange: () => setPage(1),
});Migrated all 5 call sites — Devices, Containers, API Keys, Device Chooser, and the pre-existing admin Devices table (the pattern this PR mirrored), so there's no stray copy left. Behavior is unchanged; the existing sort tests (72 across the 4 suites) still pass. New sortable tables can now just reuse the hook. |
|
/review |
|
Claude finished @otavio's task in 23m 7s —— View job Code Review CompleteReviewed 13 files across code quality, security, testing, TypeScript patterns, and architecture — no new issues found. The code looks good as-is. Verified:
If you've addressed the feedback and want a new review, tag |
What
Adds user-controllable column sorting to four console data-tables whose API endpoints already accept
sort_by/order_by: Devices, Containers, API Keys, and the billing Device Chooser ("All Devices" tab). Frontend-only — no backend change.Why
These lists rendered a
DataTablewith no sorting, even though the component and the backing endpoints already supported it. Only the admin Devices list was wired up. This brings the user-facing tables in line.Fixes: shellhub-io/team#165
Changes
Each table mirrors the existing
admin/devicespattern — localsortBy/orderBystate, ahandleSorttoggle (same field flips asc⇄desc; switching field resets direction; every sort resets to page 1),sortField/sortOrder/onSortpassed toDataTable, and the data hook forwardingsort_by/order_byto the SDK query.pages/devices,hooks/useDevices): sortable Hostname (name), Last Seen (last_seen); defaultlast_seendesc.pages/containers,hooks/useContainers): sortable Hostname (name), Last Seen (last_seen); defaultlast_seendesc.pages/team/ApiKeysTab,hooks/useApiKeys): sortable Name (name), Created (created_at), Expires (expires_in); defaultcreated_atdesc. Field names match the backend sorter (api/routes/api-key.go,api/services/api-key.go).components/billing/DeviceChooserDialog, "All" tab): sortable Hostname, Last Seen.Column
keys were aligned to the backend sort fields (hostname→name,created→created_at,expires→expires_in) becauseDataTablepasses the column key as the sort field.Testing
TDD (red→green) per table. New/updated test files for all four assert that sortable headers render as controls and that clicking them drives the hook with the expected
sortBy/orderByand toggles direction.cd ui/apps/console && npm run test -- <the four table test files>→ 72 pass;tsc --noEmitandeslintclean.Non-sortable derived columns (tags, online indicator, role) are intentionally left non-sortable. Tables whose endpoints lack sort params (Sessions, Firewall Rules, Public Keys, several admin lists) are out of scope and tracked separately in shellhub-io/team#165.