Repository navigation
Add search/filtering and infinite scroll to the admin users page - #738
Conversation
The users list previously paginated with Previous/Next buttons and had no way to find an account. Search (q) and role filters now apply server-side in loadAdminUsersData (shared WHERE clause for the slice and its COUNT), surface in the admin_user_list MCP capability, and the sidebar list loads additional pages via an IntersectionObserver sentinel instead of paging. Shared where the reuse is real: replace-location.ts (URL-backed filter state, extracted from the secrets page), AccountManagementSearchField (used by secrets and users), and infinite-scroll.ts pairing with the previously unused infinite-list.ts. Mutation responses now include the updated target user so the client patches it into the scrolled window instead of resetting to page one. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThe PR adds server-side admin-user filters, infinite scrolling, URL-synchronized search and role filters, mutation payload synchronization, shared client navigation components, MCP filter inputs, and regression coverage. ChangesAdmin users filtering and synchronization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant AdminUsersPage
participant AdminUsersHandler
participant LoadAdminUsersData
participant D1
Browser->>AdminUsersPage: Enter q or role filter
AdminUsersPage->>Browser: Replace URL and dispatch navigate
AdminUsersPage->>AdminUsersHandler: Request filtered admin users
AdminUsersHandler->>LoadAdminUsersData: Load filtered page
LoadAdminUsersData->>D1: Count and select filtered users
D1-->>AdminUsersPage: Return users and total
AdminUsersPage->>AdminUsersPage: Append pages through infiniteScrollSentinel
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🔎 Preview deployed: https://kody-pr-738.kody-a99.workers.dev Worker: Mocks:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/worker/client/routes/admin-users.tsx (1)
558-563: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDebounce the server-side search input.
Because admin search is server-side,
onInput→replaceLocation→ re-render triggersloadAdminUserson every keystroke (currentHref !== lastLoadedHref).loadRequestIdkeeps results correct, but a fast typist fires a request per character. Unlike account-secrets (which filters client-side), this hits the API repeatedly. Consider debouncing the URL write / fetch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/client/routes/admin-users.tsx` around lines 558 - 563, Debounce the server-side search update in the input handler around replaceLocation and buildHrefWithUpdatedFilters, delaying URL writes and the resulting loadAdminUsers request until typing pauses. Preserve the latest search value and ensure pending updates are cancelled or superseded so only the final input triggers navigation.packages/worker/client/infinite-scroll.ts (1)
21-32: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueOptional: a rejecting
loadMoreis swallowed and stalls the sentinel.The async IIFE has no
catch, so if a futureloadMorerejects (the type allowsPromise<boolean>), it becomes an unhandled rejection and the sentinel never re-observes. Today’s only caller (loadMoreUsers) catches internally and returnsfalse, so this is latent — worth a defensivecatchif the helper is reused.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/client/infinite-scroll.ts` around lines 21 - 32, Update the async IIFE around loadMore in the infinite-scroll observer to catch rejected promises, ensuring the sentinel is re-observed and the observer does not remain stalled when loadMore fails. Preserve the existing aborted or false-result behavior and running cleanup in the finally block.packages/worker/src/mcp/capabilities/admin/admin-user-list.ts (1)
25-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the shared role enum for this input.
roleis already defined asroleNameSchema; using it here instead ofz.string()would reject invalid values up front instead of letting them fall through to the backend and return all users.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/capabilities/admin/admin-user-list.ts` around lines 25 - 32, Update the role field in the admin user-list input schema to reuse the existing roleNameSchema instead of z.string(), preserving its optional behavior and description. This ensures invalid role values are rejected before reaching the backend.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/client/routes/admin-users.tsx`:
- Around line 333-343: The applyMutationPayload function must refresh the
user-list snapshot after role mutations that can change membership in the active
role filter. Re-seed the list and total count from the mutation payload when
available, or otherwise update the total and remove rows that no longer match
the active filter, while preserving the existing updatedUser patch for
unaffected mutations.
---
Nitpick comments:
In `@packages/worker/client/infinite-scroll.ts`:
- Around line 21-32: Update the async IIFE around loadMore in the
infinite-scroll observer to catch rejected promises, ensuring the sentinel is
re-observed and the observer does not remain stalled when loadMore fails.
Preserve the existing aborted or false-result behavior and running cleanup in
the finally block.
In `@packages/worker/client/routes/admin-users.tsx`:
- Around line 558-563: Debounce the server-side search update in the input
handler around replaceLocation and buildHrefWithUpdatedFilters, delaying URL
writes and the resulting loadAdminUsers request until typing pauses. Preserve
the latest search value and ensure pending updates are cancelled or superseded
so only the final input triggers navigation.
In `@packages/worker/src/mcp/capabilities/admin/admin-user-list.ts`:
- Around line 25-32: Update the role field in the admin user-list input schema
to reuse the existing roleNameSchema instead of z.string(), preserving its
optional behavior and description. This ensures invalid role values are rejected
before reaching the backend.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 22fb1f6c-f659-494b-89a0-0bc334e4f742
📒 Files selected for processing (12)
e2e/admin-rbac.spec.tspackages/worker/client/infinite-list.node.test.tspackages/worker/client/infinite-scroll.tspackages/worker/client/replace-location.tspackages/worker/client/routes/account-management-components.tsxpackages/worker/client/routes/account-secrets.tsxpackages/worker/client/routes/admin-users.tsxpackages/worker/src/app/admin-users-data.tspackages/worker/src/app/handlers/admin-users.node.test.tspackages/worker/src/app/handlers/admin-users.tspackages/worker/src/app/loader-data.tspackages/worker/src/mcp/capabilities/admin/admin-user-list.ts
… page links Role mutations now drop the target from the list when it no longer matches the active role filter and take the refreshed filtered total from the server. Initial loads (SSR and client) strip the page param so a stale ?page=N link cannot anchor the window past rows infinite scroll can never reach. The admin_user_list role input reuses roleNameSchema to reject unknown roles up front. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/admin-rbac.spec.ts`:
- Around line 119-123: Strengthen the deep-link regression assertion in the test
around the `/admin/users?pageSize=1&page=99999` navigation so the “Showing” text
requires a positive visible-row count rather than accepting zero. Keep the
existing total-count validation and heading visibility checks unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 251fac4d-dcc8-4f59-ae59-43ec63380766
📒 Files selected for processing (4)
e2e/admin-rbac.spec.tspackages/worker/client/routes/admin-users.tsxpackages/worker/src/app/handlers/admin-users.tspackages/worker/src/mcp/capabilities/admin/admin-user-list.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/worker/src/mcp/capabilities/admin/admin-user-list.ts
- packages/worker/src/app/handlers/admin-users.ts
- packages/worker/client/routes/admin-users.tsx
… unknown roles Mutations now reset the infinite list before replacing the window so an in-flight load-more from before the mutation cannot merge stale rows back in, and only known role names count as an active filter (matching the server, which ignores unknown role values). Tighten the stale-page e2e assertion to require a non-empty list. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4d0d524. Configure here.
| }) | ||
| resetPlanDraft() | ||
| ensureSelection() | ||
| } |
There was a problem hiding this comment.
Stale page after filter mutation
Medium Severity
When a role mutation removes the updated account from the active role filter, applyMutationPayload shrinks the in-memory list and total but leaves loadedThroughPage unchanged. Later loadMoreUsers requests the next numbered page from the server, which can skip or mis-order accounts relative to the edited client window.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4d0d524. Configure here.
| pageSize = payload.pageSize | ||
| total = payload.total | ||
| resetPlanDraft() | ||
| applyMutationPayload(payload) |
There was a problem hiding this comment.
Selection reset after filter drop
Medium Severity
After a role change that drops the target from the active role filter, applyMutationPayload calls ensureSelection to pick a visible account, but submitRoleAction immediately sets selectedUserId back to the mutated user. getSelectedUser then returns null, so no list row is active and the detail pane shows the empty state despite accounts remaining.
Reviewed by Cursor Bugbot for commit 4d0d524. Configure here.
2 participants
Summary
q, matching username or email) and a role filter (role) to the admin users page, mirroring the secrets page's URL-backed filter UX. Filters apply in SQL with a WHERE clause shared by the page slice and itsCOUNT(*), so the reported total always matches the filtered set.createInfiniteList, with stale in-flight pages invalidated whenever filters change.replace-location.ts(extracted from the secrets page),AccountManagementSearchField(now used by secrets and admin users, ready for the other sidebar list pages), andinfinite-scroll.ts.updatedUser) so the client patches it into a deeply-scrolled list instead of resetting to page one.admin_user_listMCP capability gains matchingquery/roleinputs.Test plan
q/role/pagination composition, unknown-role tolerance,updatedUsermutation contract (admin-users.node.test.ts)createInfiniteList: append/dedupe, stale-page invalidation on reset, error handlingadmin-rbac.spec.ts): typing in search filters the list and syncs the URL, filtered API totals, no-matches empty state, sentinel auto-load with?pageSize=1npm run validategreen locallySystem recap — extends existing primitives (medium risk)
Mode: recap · Base:
main@d3034257· Head:4d0d5242Classification: extends — the admin users JSON API contract gains
q/rolequery params and anupdatedUserfield on mutation responses; theadmin_user_listMCP capability input schema gainsquery/role. No primitives added;primitives.yamlunchanged.Primitives touched
app-ui/admin/users(.json)filter params, infinite scroll, shared client utilitiesrbacadmin_user_listcapability acceptsquery/role; guards unchangedd1-app-dbusers/user_rolestables; no schema changeSystem map
Search and scroll flow from the admin users UI through the RBAC-guarded JSON API into filtered D1 queries.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Before / after
GET /admin/users.jsonpage,pageSizeq(username/email substring),role(known role name)POST /admin/users.jsonupdatedUser(refreshed target for in-place patching)admin_user_listcapabilitypage,pageSizequery,role(validated againstroleNameSchema)?page=N?pageso stale deep links seed from page 1Invariants
Admin routes still expose account metadata only (
adminUserListItemFieldNamesunchanged; e2e privacy assertions still pass); all queries remain guarded byread:user:any/update:user:any.Plan vs actual
q/rolefilters, infinite scroll viacreateInfiniteList, sharedreplace-location.ts/AccountManagementSearchField/infinite-scroll.ts,updatedUsermutation patching.?page=Nso stale deep links cannot orphan earlier rows.Note
Medium Risk
Extends the admin users JSON API and in-place role/plan mutation handling on RBAC-guarded routes; behavior is well tested but touches entitlement and role assignment flows.
Overview
Adds server-side search (
qon username/email) and a role filter to the admin users list, with URL-backed controls viareplaceLocationand a sharedAccountManagementSearchField(secrets page refactored to use the same helpers).Replaces Previous/Next pagination with infinite scroll:
createInfiniteList, an intersection-observer sentinel (infinite-scroll.ts), and initial loads that strip?pageso deep links always seed from page one. Filter changes reset the list and invalidate in-flightloadMorerequests.Role/plan POST responses now include
updatedUserso the client can patch a scrolled list in place; rows drop when a mutation removes the active role filter. D1 list queries share one filteredWHEREfor rows andCOUNT(*).The
admin_user_listMCP capability accepts matchingquery/roleinputs. E2E and handler/unit tests cover search, scroll, mutations, and privacy boundaries.Reviewed by Cursor Bugbot for commit 4d0d524. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit