feat(auth): add pagination support to Auth user list#300
Conversation
udaycodespace
requested review from
Nitin-kumar-yadav1307 and
yash-pouranik
as code owners
|
Warning Review limit reached
More reviews will be available in 45 minutes and 53 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds backend admin endpoints for listing and deleting users, and implements paginated user browsing in the Auth page with guarded paginated fetches, create/delete refreshes, and a Pagination control under the user table. ChangesAdmin users (backend + frontend)
Sequence Diagram(s)sequenceDiagram
participant Client as AuthPage
participant Server as DashboardAPI
participant DB as MongoDB
Client->>Server: GET /:projectId/admin/users?page=&limit= (reqId)
Server->>DB: find(query, projection).sort(createdAt).skip((page-1)*limit).limit(limit)
Server->>DB: countDocuments(query)
DB-->>Server: items, total
Server-->>Client: { items, total, page, limit }
Client->>Client: if reqId matches → set users, set totalRecords
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
Quick update: PR is up, implementation is completed. I'll do final testing and upload screenshots/recordings tonight.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/web-dashboard/src/pages/Auth.jsx`:
- Around line 83-92: In the useEffect in Auth.jsx that fetches
`/api/projects/${projectId}/collections/users/data` wrap the post-await state
updates with a re-check for component liveness (or a requestId guard) to avoid
stale overwrites: after the `await api.get(...)` and before calling `setUsers`
and `setTotalRecords` verify `isMounted` (or compare the captured requestId
against the latest) and only proceed if still valid; apply this check around the
usage of `normalizeUsersResponse`, `setUsers`, and `setTotalRecords` so an
out-of-order response cannot overwrite newer pagination state.
- Around line 237-246: The delete path (handleDeleteUser) currently removes the
user from state with setUsers but doesn't update setTotalRecords, causing
pagination drift; after removing the user call setTotalRecords(prev =>
Math.max(prev - 1, 0)) to keep the total in sync and, if the current page
becomes empty (i.e., normalizeUsersResponse(users) length === 0 and page > 1),
decrement the page or trigger a refetch so the UI moves to the previous page;
update the logic in handleDeleteUser near the setUsers call and reference
setTotalRecords, setUsers, normalizeUsersResponse and page handling to implement
this fix.
🪄 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
Run ID: 2932cc8c-9005-4ecf-85a8-26d2a351bc59
📒 Files selected for processing (1)
apps/web-dashboard/src/pages/Auth.jsx
|
Some coderabbitai |
|
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 `@apps/web-dashboard/src/pages/Auth.jsx`:
- Around line 173-176: The delete-success callback is using stale closure values
for users and page; change the mutations to use functional updaters so they read
the latest state: call setUsers(prevUsers => { const next =
normalizeUsersResponse(prevUsers).filter(u => u._id !== userId); return next;
}), call setTotalRecords(prev => Math.max(prev - 1, 0)), and compute page
rollback from the latest users/page by using setPage(prevPage => { if
(next.length === 0 && prevPage > 1) return prevPage - 1; return prevPage; }) (or
use synced refs for users/page if you prefer), ensuring you reference
normalizeUsersResponse, setUsers, setTotalRecords, setPage and userId in the
updated logic.
🪄 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
Run ID: 2479fb51-b6d5-4299-8ba3-9914c7494abc
📒 Files selected for processing (1)
apps/web-dashboard/src/pages/Auth.jsx
|
Addressed all CodeRabbit feedback:
Final pagination validation and screenshots are in progress. |
|
I can see that only frontend is changed |
Yes, only frontend changes were made. Pagination is handled by the existing collection data endpoint ( |
|
Final validation completed.
Screenshots attached below. 
|
|
2 months back when we integrated auth as a service let me know ur views @coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Find Auth.jsx
fd -i "Auth.jsx" --exec cat {}
# Find userauth route/controller files
fd -i "userauth" -e js -e ts -e jsx -e tsx --exec echo "FILE: {}" \; --exec cat {} \; 2>/dev/null
# Also search by pattern in route registrations
rg -rn "userauth" --type js --type ts -C3 2>/dev/null | head -100
# Look for any pagination support in userauth routes
rg -rn "page\|limit\|pagination" --type js --type ts -C2 2>/dev/null | grep -i "userauth\|auth" | head -50Length of output: 211489
The ProblemThe PR fetches the user list using: GET /api/projects/${projectId}/collections/users/data?page=${page}&limit=${limit}This is a generic collection CRUD endpoint — it has no awareness that the
The Inconsistency in This PRLooking at
Why This HappenedThere's no paginated list endpoint under // In dashboard-api userAuth.controller.js
module.exports.listAdminUsers = async (req, res) => {
try {
const project = req.project;
const page = Math.max(1, parseInt(req.query.page) || 1);
const limit = Math.min(100, Math.max(1, parseInt(req.query.limit) || 50));
const skip = (page - 1) * limit;
const usersColConfig = project.collections.find(c => c.name === 'users');
if (!usersColConfig) return res.status(404).json({ error: "Auth collection not found" });
const connection = await getConnection(project._id);
const Model = getCompiledModel(connection, usersColConfig, project._id, project.resources.db.isExternal);
const [items, total] = await Promise.all([
Model.find({}, { password: 0 }).skip(skip).limit(limit).lean(),
Model.countDocuments()
]);
res.json({ items, total, page, limit });
} catch (err) {
res.status(500).json({ error: err.message });
}
};Then register it in the dashboard-api admin routes and update SummaryThe PR author's implementation was a pragmatic workaround for a missing endpoint — the pagination logic itself is sound, but it's wired to the wrong API. The real fix needed here is:
Would you like me to open a follow-up issue for adding these missing admin endpoints, |
|
yh all routes should go through - /admin |
Done. I moved the Auth user list/delete flow to dedicated Also added paginated admin user APIs on the backend with password exclusion, and updated the frontend to use those endpoints consistently. Tested locally by changing page sizes (10/20/50/100), verifying paginated refetches, and confirming list/delete requests hit |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web-dashboard/src/pages/Auth.jsx (2)
252-263:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse current pagination when refreshing after create.
Lines 252-255 reuse
pageandlimitfrom the submit-time closure. If pagination changes before the POST resolves, this follow-up GET fetches the old page, incrementslatestUsersRequestId, and can overwrite the newer page's data with stale results. Please drive this refresh through a sharedfetchUsershelper or read the latest pagination from synced refs before issuing the GET.🤖 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 `@apps/web-dashboard/src/pages/Auth.jsx` around lines 252 - 263, The GET after create can use stale closure variables (page, limit) and overwrite newer data; update the flow to drive the refresh via a shared fetchUsers helper (e.g., create/fix a fetchUsers function called by UI and by the POST follow-up) or read the latest pagination from synced refs before requesting (use pageRef.current and limitRef.current) so the GET uses current pagination; keep the optimistic request id logic (latestUsersRequestId.current) and call setUsers/ setTotalRecords with normalizeUsersResponse only after confirming requestId === latestUsersRequestId.current.
75-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlock stale row actions while a new page is loading.
Lines 75-103 refetch on every
page/limitchange, but the component never enters a users-loading state before those requests. After Line 339 changespage, the paginator can already show the new page whileUserTablestill renders the old rows, so edit/delete actions stay live against stale data. Add a dedicated loading state for paginated fetches and clear/disable the table until that request settles.Also applies to: 335-344
🤖 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 `@apps/web-dashboard/src/pages/Auth.jsx` around lines 75 - 103, Add a dedicated users-loading state (e.g., usersLoading / setUsersLoading) and use it around the paginated users fetch in the existing useEffect/fetchData flow: set usersLoading true (and optionally clear users via setUsers([])) immediately before issuing the users API call triggered by page/limit changes, and set usersLoading false in the finally block after the users request settles; keep the existing latestUsersRequestId.current check and only call setUsers/setTotalRecords when the request is current. Also update the table/paginator (or the row action handlers) to disable edit/delete when usersLoading is true so row actions are blocked until the paginated fetch completes.
🤖 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 `@apps/dashboard-api/src/controllers/userAuth.controller.js`:
- Around line 241-268: Update listAdminUsers to conform to the API contract and
sanitize errors: change the handler signature to (req, res, next), and on
success return res.json({ success: true, data: { items, total, page, limit },
message: "" }) instead of the current payload; in the catch block do not expose
err.message — call next(new AppError("Failed to list admin users", 500)) (or
similar sanitized message) so failures are routed through the AppError flow;
apply the same pattern (add next param, standardized success object, and
AppError-based error forwarding) to the other admin handler referenced around
lines 270-288.
- Around line 281-286: Validate the userId at the start of deleteAdminUser using
mongoose.Types.ObjectId.isValid(userId) before constructing a new ObjectId; if
invalid, call next(new AppError("Invalid user id", 400)). After validation,
perform Model.deleteOne(...) and if deletedCount === 0 call next(new
AppError("User not found", 404)). For success, return the unified response shape
res.json({ success: true, data: null, message: "User deleted successfully" }).
Replace direct res.status(...).json({ error: ... }) error returns with AppError
via next(...) so global error handler formats errors consistently.
In `@apps/dashboard-api/src/routes/projects.js`:
- Around line 51-52: The test mock for the userAuth controller is missing two
exported handlers causing route registration to receive undefined; update the
mocked module in the failing test to include listAdminUsers: jest.fn(() => ...)
and deleteAdminUser: jest.fn(() => ...) so the router.get (listAdminUsers) and
router.delete (deleteAdminUser) in projects.js receive functions during Jest
runs; locate the mock of ../controllers/userAuth.controller in the test file and
add those two jest.fn mocks to the exported mock object.
---
Outside diff comments:
In `@apps/web-dashboard/src/pages/Auth.jsx`:
- Around line 252-263: The GET after create can use stale closure variables
(page, limit) and overwrite newer data; update the flow to drive the refresh via
a shared fetchUsers helper (e.g., create/fix a fetchUsers function called by UI
and by the POST follow-up) or read the latest pagination from synced refs before
requesting (use pageRef.current and limitRef.current) so the GET uses current
pagination; keep the optimistic request id logic (latestUsersRequestId.current)
and call setUsers/ setTotalRecords with normalizeUsersResponse only after
confirming requestId === latestUsersRequestId.current.
- Around line 75-103: Add a dedicated users-loading state (e.g., usersLoading /
setUsersLoading) and use it around the paginated users fetch in the existing
useEffect/fetchData flow: set usersLoading true (and optionally clear users via
setUsers([])) immediately before issuing the users API call triggered by
page/limit changes, and set usersLoading false in the finally block after the
users request settles; keep the existing latestUsersRequestId.current check and
only call setUsers/setTotalRecords when the request is current. Also update the
table/paginator (or the row action handlers) to disable edit/delete when
usersLoading is true so row actions are blocked until the paginated fetch
completes.
🪄 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
Run ID: ff911ad5-fea6-4cb4-8616-103fd86bb0f0
📒 Files selected for processing (3)
apps/dashboard-api/src/controllers/userAuth.controller.jsapps/dashboard-api/src/routes/projects.jsapps/web-dashboard/src/pages/Auth.jsx
|
|
yh this looks more prod ready |
Sure, please review once. Happy to make any further changes if required. |
2 participants
🚀 Pull Request Description
Fixes #298
Implemented pagination support for the Auth page user list to make it consistent with the existing Database page pagination behavior.
Changes Made
page,limit,totalRecords) inAuth.jsxThis allows users to navigate through all available auth users instead of being restricted to the first set of records.
🛠️ Type of Change
🧪 Testing & Validation
Backend Verification:
npm testin thebackend/directory and all tests passed.Frontend Verification:
npm run lintin thefrontend/directory.📸 Screenshots / Recordings (Pending)
The current test project does not contain enough Auth users to demonstrate multi-page navigation visually.
Current verification completed:
pageandlimitparameters.Console verification output:
I am currently preparing additional test data to fully validate page navigation behavior and will update this PR with screenshots and/or recordings once testing is completed.
✅ Checklist
Built with ❤️ for urBackend.
Summary by CodeRabbit
New Features
Bug Fixes