API created for Dashboard Data#13
Merged
Merged
Conversation
📝 WalkthroughWalkthroughA dashboard endpoint is implemented with role-based logic: admin users receive aggregated employee, attendance, and leave counts; non-admin users get personalized attendance and payslip data. Supporting infrastructure includes new route handler, API route registration, and documentation example. A build-level Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server as /api/dashboard<br/>(protect middleware)
participant SessionData as Session Data
participant AdminPath as Admin Path<br/>(queries: Employee,<br/>Attendance, Leave,<br/>Departments)
participant UserPath as Non-Admin Path<br/>(queries: Employee,<br/>Attendance, Leave,<br/>Payslip)
Client->>Server: GET /api/dashboard
Server->>SessionData: Check session.role
alt Admin Role
SessionData-->>Server: role = ADMIN
Server->>AdminPath: Count employees,<br/>attendance (today),<br/>pending leaves,<br/>departments
AdminPath-->>Server: {employeeCount,<br/>attendanceCount,<br/>leaveCount,<br/>departmentCount}
Server-->>Client: 200 {role, stats}
else Non-Admin Role
SessionData-->>Server: role = USER
Server->>UserPath: Look up employee<br/>by session.userId
alt Employee Found
UserPath-->>Server: Fetch attendance (month),<br/>pending leaves,<br/>latest payslip<br/>(concurrent queries)
UserPath-->>Server: {attendance,<br/>pendingLeaves,<br/>payslip}
Server-->>Client: 200 {role, employee,<br/>attendance, leaves,<br/>payslip}
else Employee Not Found
Server-->>Client: 404 {message}
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
server/package.json (1)
15-16: Removebcryptfrompackage.json— it is unused.The codebase imports and uses only
bcryptjs(found inauthController.jsandemployeeController.js). Thebcryptpackage on line 15 is not referenced anywhere and can be removed to eliminate the unnecessary native dependency and reduce the dependency footprint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/package.json` around lines 15 - 16, Remove the unused "bcrypt" dependency from package.json (the line with "bcrypt": "^6.0.0") since the codebase uses only bcryptjs; after removing that entry, run your package manager (npm install or yarn install) to update package-lock.json / yarn.lock and verify there are no imports of "bcrypt" (search for "require('bcrypt')" or "import bcrypt") in files like authController.js and employeeController.js to ensure nothing breaks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/controllers/dashboardController.js`:
- Around line 35-38: The Employee lookup in dashboardController.js uses
Employee.findOne({ userId: session.userId }) and therefore returns soft-deleted
records; update the query to also filter out deleted employees (e.g., include
isDeleted: false) or explicitly check employee.isDeleted after retrieval and
return 404 if true so soft-deleted users cannot fetch dashboard data; ensure you
still call .lean() and return the same 404 response when the filtered query
yields no result.
- Around line 1-4: Add an import for the Payslip model at the top of
dashboardController.js so the non-admin branch that calls Payslip.findOne(...)
(the Payslip symbol used around the non-admin dashboard logic) doesn't throw a
ReferenceError; mirror the existing model imports (e.g., Attendance, Employee,
LeaveApplication) and import Payslip before it's referenced in the file.
- Around line 40-47: currentMonthAttendance query uses an undefined today and
incorrectly sets the lower bound to the current day at 01:00; declare const
today = new Date() before the Attendance.countDocuments call and change the date
range to the start of the month and start of next month (e.g., new
Date(today.getFullYear(), today.getMonth(), 1, 0, 0, 0) as the $gte and new
Date(today.getFullYear(), today.getMonth() + 1, 1, 0, 0, 0) as the $lt) when
building the Attendance.countDocuments query that references employee._id.
- Around line 18-23: The todayAttendance query uses Attendance.countDocuments
with a date range where both bounds are the same (startOfDay), causing zero
results; update the upper bound to the start of the next day (e.g., compute
startOfDay with new Date().setHours(0,0,0,0) and set the $lt bound to new
Date(startOfDay).setDate(startOfDay.getDate() + 1) or use setHours(24,0,0,0)) so
the filter becomes date >= startOfDay && date < startOfNextDay; adjust the code
around Attendance.countDocuments and the todayAttendance variable accordingly.
In `@server/routes/dashboardRoutes.js`:
- Around line 2-3: The import for the protect middleware is pointing to the
wrong module; update the import statement that brings in protect (the one paired
with getDashboard) so it imports protect from the auth module in the middleware
directory (i.e., import protect from the auth.js middleware export) to match
other routes and fix module resolution errors.
---
Nitpick comments:
In `@server/package.json`:
- Around line 15-16: Remove the unused "bcrypt" dependency from package.json
(the line with "bcrypt": "^6.0.0") since the codebase uses only bcryptjs; after
removing that entry, run your package manager (npm install or yarn install) to
update package-lock.json / yarn.lock and verify there are no imports of "bcrypt"
(search for "require('bcrypt')" or "import bcrypt") in files like
authController.js and employeeController.js to ensure nothing breaks.
🪄 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: a4666a24-5f85-40ad-a549-451a34ed9459
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonserver/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
Learnings.mdpackage.jsonserver/controllers/dashboardController.jsserver/package.jsonserver/routes/dashboardRoutes.jsserver/server.js
💤 Files with no reviewable changes (1)
- package.json
Comment on lines
+1
to
+4
| import { DEPARTMENTS } from "../constants/departments.js"; | ||
| import Attendance from "../models/Attendance.js" | ||
| import Employee from "../models/Employee.js" | ||
| import LeaveApplication from "../models/LeaveApplication.js"; |
There was a problem hiding this comment.
Import Payslip before using it on Line 53.
The non-admin branch calls Payslip.findOne(...), but this model is never imported in this file. That path will crash with a ReferenceError.
Suggested fix
import { DEPARTMENTS } from "../constants/departments.js";
import Attendance from "../models/Attendance.js"
import Employee from "../models/Employee.js"
import LeaveApplication from "../models/LeaveApplication.js";
+import Payslip from "../models/Payslip.js";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { DEPARTMENTS } from "../constants/departments.js"; | |
| import Attendance from "../models/Attendance.js" | |
| import Employee from "../models/Employee.js" | |
| import LeaveApplication from "../models/LeaveApplication.js"; | |
| import { DEPARTMENTS } from "../constants/departments.js"; | |
| import Attendance from "../models/Attendance.js" | |
| import Employee from "../models/Employee.js" | |
| import LeaveApplication from "../models/LeaveApplication.js"; | |
| import Payslip from "../models/Payslip.js"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/controllers/dashboardController.js` around lines 1 - 4, Add an import
for the Payslip model at the top of dashboardController.js so the non-admin
branch that calls Payslip.findOne(...) (the Payslip symbol used around the
non-admin dashboard logic) doesn't throw a ReferenceError; mirror the existing
model imports (e.g., Attendance, Employee, LeaveApplication) and import Payslip
before it's referenced in the file.
Comment on lines
+18
to
+23
| Attendance.countDocuments({ | ||
| date: { | ||
| $gte: new Date(new Date().setHours(0, 0, 0, 0)), | ||
| $lt: new Date(new Date().setHours(0, 0, 0, 0)), | ||
| } | ||
| }), |
There was a problem hiding this comment.
todayAttendance will always be 0 with this range.
Line 20 and Line 21 both resolve to the same timestamp, so the query becomes date >= startOfDay && date < startOfDay, which no document can satisfy.
Suggested fix
+ const startOfToday = new Date();
+ startOfToday.setHours(0, 0, 0, 0);
+ const startOfTomorrow = new Date(startOfToday);
+ startOfTomorrow.setDate(startOfTomorrow.getDate() + 1);
+
const [totalEmployees, todayAttendance, pendingLeaves] = await
Promise.all([
Employee.countDocuments({ isDeleted: { $ne: true } }),
Attendance.countDocuments({
date: {
- $gte: new Date(new Date().setHours(0, 0, 0, 0)),
- $lt: new Date(new Date().setHours(0, 0, 0, 0)),
+ $gte: startOfToday,
+ $lt: startOfTomorrow,
}
}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/controllers/dashboardController.js` around lines 18 - 23, The
todayAttendance query uses Attendance.countDocuments with a date range where
both bounds are the same (startOfDay), causing zero results; update the upper
bound to the start of the next day (e.g., compute startOfDay with new
Date().setHours(0,0,0,0) and set the $lt bound to new
Date(startOfDay).setDate(startOfDay.getDate() + 1) or use setHours(24,0,0,0)) so
the filter becomes date >= startOfDay && date < startOfNextDay; adjust the code
around Attendance.countDocuments and the todayAttendance variable accordingly.
Comment on lines
+35
to
+38
| const employee = await Employee.findOne({ userId: session.userId }).lean(); | ||
|
|
||
| if (!employee) | ||
| return res.status(404).json({ error: "Employee not found" }); |
There was a problem hiding this comment.
Soft-deleted employees can still fetch dashboard data.
This lookup only checks userId. Because the JWT stays valid after login, a soft-deleted employee can still resolve here until the token expires unless isDeleted is filtered too.
Suggested fix
- const employee = await Employee.findOne({ userId: session.userId }).lean();
+ const employee = await Employee.findOne({
+ userId: session.userId,
+ isDeleted: { $ne: true },
+ }).lean();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const employee = await Employee.findOne({ userId: session.userId }).lean(); | |
| if (!employee) | |
| return res.status(404).json({ error: "Employee not found" }); | |
| const employee = await Employee.findOne({ | |
| userId: session.userId, | |
| isDeleted: { $ne: true }, | |
| }).lean(); | |
| if (!employee) | |
| return res.status(404).json({ error: "Employee not found" }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/controllers/dashboardController.js` around lines 35 - 38, The Employee
lookup in dashboardController.js uses Employee.findOne({ userId: session.userId
}) and therefore returns soft-deleted records; update the query to also filter
out deleted employees (e.g., include isDeleted: false) or explicitly check
employee.isDeleted after retrieval and return 404 if true so soft-deleted users
cannot fetch dashboard data; ensure you still call .lean() and return the same
404 response when the filtered query yields no result.
Comment on lines
+40
to
+47
| const [currentMonthAttendance, pendingLeaves, latestPayslip] = await Promise.all([ | ||
| Attendance.countDocuments({ | ||
| employeeId: employee._id, | ||
| date: { | ||
| $gte: new Date(today.getFullYear(), today.getMonth(), today.getDate(), 1), | ||
| $lt: new Date(today.getFullYear(), today.getMonth() + 1, 1), | ||
| } | ||
| }), |
There was a problem hiding this comment.
currentMonthAttendance can never work as written.
Line 44 and Line 45 reference today, which is never defined, so the employee dashboard will throw before this query runs. Even after defining it, the lower bound starts on the current day at 01:00, not at the first day of the month.
Suggested fix
+ const today = new Date();
+ const startOfMonth = new Date(today.getFullYear(), today.getMonth(), 1);
+ const startOfNextMonth = new Date(today.getFullYear(), today.getMonth() + 1, 1);
+
const [currentMonthAttendance, pendingLeaves, latestPayslip] = await Promise.all([
Attendance.countDocuments({
employeeId: employee._id,
date: {
- $gte: new Date(today.getFullYear(), today.getMonth(), today.getDate(), 1),
- $lt: new Date(today.getFullYear(), today.getMonth() + 1, 1),
+ $gte: startOfMonth,
+ $lt: startOfNextMonth,
}
}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/controllers/dashboardController.js` around lines 40 - 47,
currentMonthAttendance query uses an undefined today and incorrectly sets the
lower bound to the current day at 01:00; declare const today = new Date() before
the Attendance.countDocuments call and change the date range to the start of the
month and start of next month (e.g., new Date(today.getFullYear(),
today.getMonth(), 1, 0, 0, 0) as the $gte and new Date(today.getFullYear(),
today.getMonth() + 1, 1, 0, 0, 0) as the $lt) when building the
Attendance.countDocuments query that references employee._id.
Comment on lines
+2
to
+3
| import { protect } from "../middlewares/protect.js"; | ||
| import { getDashboard } from "../controllers/dashboardController.js"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
echo "Candidate middleware files:"
fd -t f 'protect\.js|auth\.js' server
echo
echo "Where protect is exported:"
rg -n -C2 --glob 'server/**/*.js' 'export\s+(const|function)\s+protect|export\s*\{\s*protect\s*\}' server
echo
echo "Where the dashboard route imports protect from:"
rg -n -C2 --glob 'server/routes/**/*.js' 'import\s*\{\s*protect\s*\}\s*from' server/routesRepository: harshGupta090722/EMS
Length of output: 1771
Fix incorrect import path for the protect middleware.
The protect middleware is exported from server/middleware/auth.js, but this file imports from a non-existent path. Update line 2:
import { protect } from "../middleware/auth.js";
All other route files use this correct path. The current import will fail during module resolution.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/dashboardRoutes.js` around lines 2 - 3, The import for the
protect middleware is pointing to the wrong module; update the import statement
that brings in protect (the one paired with getDashboard) so it imports protect
from the auth module in the middleware directory (i.e., import protect from the
auth.js middleware export) to match other routes and fix module resolution
errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores