Skip to content

api created for employees,authentication and attendance.#10

Merged
harshGupta090722 merged 1 commit into
mainfrom
Development
Apr 30, 2026
Merged

api created for employees,authentication and attendance.#10
harshGupta090722 merged 1 commit into
mainfrom
Development

Conversation

@harshGupta090722
Copy link
Copy Markdown
Owner

@harshGupta090722 harshGupta090722 commented Apr 30, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Added user authentication and login system with session management
    • Implemented password change functionality for users
    • Added employee management system with full CRUD operations
    • Introduced attendance tracking with clock in/out capabilities
    • Added user profile management and editing features
    • Implemented department categorization for employees
  • Documentation

    • Updated learning documentation with configuration guidance

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

📝 Walkthrough

Walkthrough

This pull request introduces a complete backend infrastructure for an employee management system, adding Mongoose models for users and employees, authentication controllers with JWT support, attendance tracking, employee CRUD operations, middleware for route protection, and integration of all routes into the Express server.

Changes

Cohort / File(s) Summary
Documentation
Learnings.md
Adds notes about required libraries and configuration setup without a trailing newline.
Database Configuration
server/config/db.js
Removes unused connect import from Mongoose and updates database connection to use process.env.MONGODB_URI instead of a local constant reference.
Constants
server/constants/departments.js
Introduces a new DEPARTMENTS constant array containing department name strings for employee classification.
Data Models
server/models/User.js, server/models/Employee.js, server/models/Attendance.js
Creates three Mongoose schemas: User (email, password, role), Employee (identity, contact, compensation, employment status, bio, department), and Attendance (per-employee daily tracking with check-in/out times, status, working hours, day type).
Authentication Controller
server/controllers/authController.js
Implements login (credential validation, JWT issuance with 7-day expiry), session (returns current session or 401), and changePassword (validates and hashes new password) handlers.
Employee Controller
server/controllers/employeeController.js
Implements getEmployee (list with optional department filter), createEmployee (validates fields, hashes password, creates User and Employee records), updateEmployee (updates employee/user data and password if provided), and deleteEmployee (soft-delete via isDeleted flag) handlers.
Attendance Controller
server/controllers/attendanceController.js
Implements clockInOut (creates/updates daily attendance record, calculates status as LATE/PRESENT and day type based on hours worked) and getAttendance (retrieves attendance history with configurable limit, default 30) handlers.
Profile Controller
server/controllers/profileController.js
Implements getProfile (fetches employee profile or returns Admin fallback) and updateProfile (updates bio with soft-delete protection) handlers.
Authentication Middleware
server/middleware/auth.js
Introduces protect (validates Bearer JWT tokens) and adminProtect (restricts access to ADMIN role) middleware for route protection.
Route Modules
server/routes/authRoutes.js, server/routes/employeeRoutes.js, server/routes/attendanceRoutes.js, server/routes/profileRoutes.js
Defines four Express routers: auth routes (login, session, change-password), employee routes (CRUD with admin protection), attendance routes (clock-in/out, history), and profile routes (get/update with auth protection).
Server Integration
server/server.js
Imports and mounts all route modules under /api/auth, /api/employees, /api/profile, /api/attendance paths; loads environment variables and relocates database connection after route registration.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server
    participant JWT
    participant Database as Database

    Client->>Server: POST /api/auth/login<br/>(email, password, role)
    Server->>Database: Find User by email
    Database-->>Server: User record
    Server->>Server: Verify password with bcrypt
    alt Password valid
        Server->>JWT: Sign JWT with userId, role
        JWT-->>Server: JWT token (7-day expiry)
        Server-->>Client: { token, user }
    else Password invalid
        Server-->>Client: 401 Unauthorized
    end

    Client->>Server: GET /api/auth/session<br/>(Authorization: Bearer token)
    Server->>JWT: Verify JWT with JWT_SECRET
    JWT-->>Server: Decoded payload (userId, role)
    Server-->>Client: { userId, role, ... }
Loading
sequenceDiagram
    participant Client
    participant Server
    participant Auth Middleware
    participant Database

    Client->>Server: POST /api/attendance/clock-in<br/>(with session)
    Server->>Auth Middleware: Validate Bearer token
    Auth Middleware->>Server: req.session set
    Server->>Database: Find Employee by userId
    alt Employee not found
        Server-->>Client: 404 Not Found
    else Employee deactivated
        Server-->>Client: 403 Forbidden
    else Employee active
        Database-->>Server: Employee record
        Server->>Database: Find Attendance for today
        alt No attendance record
            Server->>Server: Set status: LATE if after 9:00,<br/>else PRESENT
            Server->>Database: Create Attendance with checkIn
            Database-->>Server: New record
            Server-->>Client: { checkIn, status }
        else Record exists, no checkOut
            Server->>Server: Calculate workingHours
            Server->>Server: Determine dayType
            Server->>Database: Update with checkOut, workingHours, dayType
            Database-->>Server: Updated record
            Server-->>Client: { checkIn, checkOut, workingHours, dayType }
        else Record has checkOut
            Server-->>Client: { (existing record) }
        end
    end
Loading
sequenceDiagram
    participant Client
    participant Server
    participant Protect Middleware
    participant AdminProtect Middleware
    participant Database

    Client->>Server: POST /api/employees<br/>(employee data with token)
    Server->>Protect Middleware: Verify JWT
    Protect Middleware-->>Server: req.session set
    Server->>AdminProtect Middleware: Check role == ADMIN
    alt Not ADMIN
        AdminProtect Middleware-->>Client: 401 Admin access required
    else Is ADMIN
        AdminProtect Middleware-->>Server: proceed
        Server->>Server: Hash password with bcrypt
        Server->>Database: Create User record
        Database-->>Server: User created
        Server->>Database: Create Employee record<br/>linked to userId
        Database-->>Server: Employee created
        Server-->>Client: { employee, userId, email }
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Hop, hop! The burrow grows,
With models, routes, and auth that flows,
JWTs hopping, employees tracked,
Attendance clocked, no detail lacked—
A system built with logic sound,
Where every check-in's safely found! 🥕✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: new API endpoints for employees, authentication, and attendance management across multiple controllers, routes, and models.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch Development

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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 18

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
server/config/db.js (1)

6-10: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not swallow the database connection failure.

connectDb() currently resolves after logging the error, so server/server.js still reaches app.listen(...) and boots an API without a working database.

Suggested fix
 export const connectDb = async () => {
     try {
         await mongoose.connect(process.env.MONGODB_URI);
         console.log("connect to database")
     } catch (error) {
-        console.log(error);
+        console.error("Failed to connect to database", error);
+        throw error;
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/config/db.js` around lines 6 - 10, connectDb currently catches
mongoose.connect errors and only logs them, allowing the server to continue
startup; modify the connectDb function to propagate failure (either rethrow the
caught error or call process.exit(1)) so server/server.js does not call
app.listen when the DB connection fails. Locate the connectDb function in
server/config/db.js and change the catch block to throw the error (or return a
rejected promise) instead of just console.log, ensuring upstream startup code
can handle the failure and stop the process.
🤖 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/attendanceController.js`:
- Around line 27-30: The code is calling the handler function getAttendance
instead of the Mongoose model; replace the erroneous call so the Attendance
model is queried (use Attendance.findOne) when looking up an existing record for
employee._id and today (where existing is assigned). Update the line that uses
getAttendance.findOne(...) to call Attendance.findOne(...) so the correct model
is queried at runtime.
- Around line 101-102: The limit parsing in attendanceController.js (variable
limit used in Attendance.find(...).limit(limit)) is unsafe;
parseInt(req.query.limit || 30) can produce NaN, negative, or huge values. Fix
by parsing with a radix (parseInt(req.query.limit, 10)), defaulting to 30 when
invalid (NaN or <=0), then clamp to a safeMax (e.g., 100) using Math.min to
ensure limit is an integer within range before passing it to
Attendance.find(...).limit(limit).
- Line 35: The isLate calculation currently uses "now.getHours() >= 9 &&
now.getMinutes() > 0" which incorrectly treats 9:00 as not late and 10:00 as
late only when minutes > 0; update the condition in the attendanceController.js
where isLate is computed to use "hour > 9 || (hour === 9 && minutes > 0)" logic
by reading now.getHours() and now.getMinutes() into variables (or inline) and
setting isLate accordingly so 9:01+ is late but 9:00 is not.
- Around line 56-69: The computed values workingHours and dayType are never
assigned to the attendance record before saving, so set them on the existing
object (e.g., existing.workingHours = workingHours and existing.dayType =
dayType) immediately before calling existing.save() in the checkout flow to
persist the metadata (refer to variables workingHours, dayType and method
existing.save).
- Line 2: The file imports the model with a named import { Employee } but the
model is exported as default, causing Employee to be undefined and calls like
Employee.findOne(...) to crash; change the import to use the default export
(i.e., import Employee from "../models/Employee.js") or update the model to
export a named Employee to match the existing import so that the symbol Employee
is defined for use in functions that call Employee.findOne, Employee.create,
etc.

In `@server/controllers/authController.js`:
- Line 1: The ES module import at the top uses a relative path without an
extension ("import User from \"../models/User\""); update this import to include
the .js extension so Node can resolve it (change the import statement
referencing User to use "../models/User.js").

In `@server/controllers/employeeController.js`:
- Around line 94-99: The updateEmployee handler currently only checks for
existence using Employee.findById(id); change the guard to also reject
soft-deleted records by checking employee.isDeleted (or equivalent flag) and
returning a 404 or 410 response; specifically, in updateEmployee after
retrieving the record with Employee.findById, if (!employee ||
employee.isDeleted === true) return res.status(404).json({ error: "Employee not
found" }) (or 410 if you prefer) so updates are blocked for soft-deleted
employees.
- Around line 50-69: Wrap the User.create and Employee.create calls in a MongoDB
transaction using mongoose.startSession() and session.withTransaction so both
inserts succeed or both roll back; start a session, call
session.withTransaction(async () => { await User.create([...], { session });
await Employee.create([...], { session }); }), ensure you pass the session
option into User.create and Employee.create, catch errors to abort/throw and
finally end the session; adjust the code around User.create and Employee.create
in employeeController.js to use this transactional pattern.
- Line 42: The request handlers are destructuring from req.bod (typo) which
breaks the create/update flows; update those destructuring sites to use req.body
instead (fix the line "const { firstName, lastName, email, phone, position,
department, basicSalary, allowances, deductions, joinDate, password, role, bio }
= req.body;" and the equivalent occurrence later in the file used by the update
handler). Ensure both the create and update handler functions that reference
req.bod are updated to req.body so the handlers receive the request payload
correctly.
- Around line 13-20: The list API builds the query object "where" without
filtering out soft-deleted records, so update the query construction in the
getEmployee handler to always include an isDeleted filter (e.g., set
where.isDeleted = false or equivalent) before calling Employee.find(where).
Ensure this is applied regardless of whether department is present so
Employee.find(where).sort(...) only returns non-deleted employees.
- Around line 91-113: The update payload references joinDate but it isn't
destructured (and req.bod appears mistyped); fix by adding joinDate to the
destructuring from req.body (e.g., const { ..., employmentStatus, joinDate } =
req.body) and ensure Employee.findByIdAndUpdate uses a safe conversion like
joinDate ? new Date(joinDate) : existing value (or omit if undefined) so
joinDate is defined and validated before calling Employee.findByIdAndUpdate.

In `@server/controllers/profileController.js`:
- Around line 11-21: When resolving the profile in getProfile, restrict the
admin fallback to only when session.role === "ADMIN" and avoid returning
soft-deleted Employee documents by adding an exclusion to the query; update the
Employee.findOne call to include a condition excluding deleted records (e.g., {
userId: session.userId, deleted: { $ne: true } } or your project’s isDeleted
flag) and change the branch that currently returns the Admin profile to instead
return the admin-style response only if session.role === "ADMIN" otherwise
return a 404/forbidden (e.g., res.status(404).json(...)); adjust any
tests/consumers accordingly.

In `@server/middleware/auth.js`:
- Around line 29-31: The adminProtect middleware currently uses the incorrect
condition `!req?.session?.role !== "ADMIN"`, which incorrectly denies admins;
update the check in the adminProtect function to directly verify the session
role (e.g., if (req?.session?.role !== "ADMIN") return 401) or equivalently
guard with if (!(req?.session?.role === "ADMIN")) so that only non-admins are
rejected; reference the adminProtect function and the req.session.role property
when making the change.
- Around line 1-15: The import for jsonwebtoken is incorrect—replace the named
import so jwt.verify isn't undefined: update the top-level import used by the
protect middleware (function protect) to import the module correctly (e.g., use
a default or namespace import for the jsonwebtoken module) so that
jwt.verify(token, process.env.JWT_SECRET) works; make sure the identifier used
in the import matches the one used in protect (jwt) and remove the existing
named import syntax.

In `@server/models/Attendance.js`:
- Around line 5-10: The employeeId field in the Attendance schema currently has
unique: true which prevents more than one attendance record per employee; remove
the unique: true option from the employeeId definition in
server/models/Attendance.js so individual employees can have multiple records
and rely on the compound index (e.g., the { employeeId, date } index) to enforce
uniqueness per-day; update the employeeId field definition (the employeeId
property in the Attendance schema) to omit unique and keep type, ref, and
required as-is.

In `@server/models/Employee.js`:
- Line 7: In server/models/Employee.js the schema uses the wrong Mongoose
ObjectId API (mongoose.Schema.types.ObjectId); update the schema field to use
mongoose.Schema.Types.ObjectId (uppercase "T") so the Employee schema loads
correctly—search for the occurrence of mongoose.Schema.types.ObjectId in the
file and replace it with mongoose.Schema.Types.ObjectId.

In `@server/models/User.js`:
- Around line 14-18: The User model's role field has a misspelled option key
"dafault" so Mongoose ignores the intended fallback; update the role field in
server/models/User.js by replacing the incorrect "dafault" property with the
correct "default" property and set it to "EMPLOYEE" (i.e., ensure the role
schema definition for role includes default: "EMPLOYEE" alongside type and
enum).

In `@server/routes/AttendanceRoutes.js`:
- Line 3: The import path for the auth middleware is incorrect: update the
import of the protect middleware in AttendanceRoutes.js (the current import
statement that references "../middlewares/auth.js") to point to the actual
module location "../middleware/auth.js" so the protect middleware used in the
route registrations is resolved correctly.

---

Outside diff comments:
In `@server/config/db.js`:
- Around line 6-10: connectDb currently catches mongoose.connect errors and only
logs them, allowing the server to continue startup; modify the connectDb
function to propagate failure (either rethrow the caught error or call
process.exit(1)) so server/server.js does not call app.listen when the DB
connection fails. Locate the connectDb function in server/config/db.js and
change the catch block to throw the error (or return a rejected promise) instead
of just console.log, ensuring upstream startup code can handle the failure and
stop the process.
🪄 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: 0c1c5b74-a51a-4ae2-99e4-b9b7243e4f12

📥 Commits

Reviewing files that changed from the base of the PR and between 21b95cd and e4f3de8.

📒 Files selected for processing (16)
  • Learnings.md
  • server/config/db.js
  • server/constants/departments.js
  • server/controllers/attendanceController.js
  • server/controllers/authController.js
  • server/controllers/employeeController.js
  • server/controllers/profileController.js
  • server/middleware/auth.js
  • server/models/Attendance.js
  • server/models/Employee.js
  • server/models/User.js
  • server/routes/AttendanceRoutes.js
  • server/routes/authRoutes.js
  • server/routes/employeeRoutes.js
  • server/routes/profileRoutes.js
  • server/server.js

@@ -0,0 +1,113 @@
import Attendance from "../models/Attendance.js";
import { Employee } from "../models/Employee.js"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify Employee model export/import forms
rg -n --type=js 'export default Employee|export const Employee|import\s+\{\s*Employee\s*\}\s+from|import\s+Employee\s+from'

Repository: harshGupta090722/EMS

Length of output: 670


Fix Employee import to match model export type.

Line 2 uses named import { Employee }, but the model is exported as default. Employee will be undefined and Employee.findOne(...) will crash.

Suggested fix
-import { Employee } from "../models/Employee.js"
+import Employee from "../models/Employee.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 { Employee } from "../models/Employee.js"
import Employee from "../models/Employee.js"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/controllers/attendanceController.js` at line 2, The file imports the
model with a named import { Employee } but the model is exported as default,
causing Employee to be undefined and calls like Employee.findOne(...) to crash;
change the import to use the default export (i.e., import Employee from
"../models/Employee.js") or update the model to export a named Employee to match
the existing import so that the symbol Employee is defined for use in functions
that call Employee.findOne, Employee.create, etc.

Comment on lines +27 to +30
const existing = await getAttendance.findOne({
employeeId: employee._id,
date: today
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Query the Attendance model, not the controller function.

Line 27 calls getAttendance.findOne(...); getAttendance is a handler function, so this throws at runtime. Use Attendance.findOne(...).

Suggested fix
-const existing = await getAttendance.findOne({
+const existing = await Attendance.findOne({
📝 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 existing = await getAttendance.findOne({
employeeId: employee._id,
date: today
})
const existing = await Attendance.findOne({
employeeId: employee._id,
date: today
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/controllers/attendanceController.js` around lines 27 - 30, The code is
calling the handler function getAttendance instead of the Mongoose model;
replace the erroneous call so the Attendance model is queried (use
Attendance.findOne) when looking up an existing record for employee._id and
today (where existing is assigned). Update the line that uses
getAttendance.findOne(...) to call Attendance.findOne(...) so the correct model
is queried at runtime.

const now = new Date();

if (!existing) {
const isLate = now.getHours() >= 9 && now.getMinutes() > 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Correct late-status time condition.

Current check marks 10:00 as not late (minutes === 0). Use hour > 9 || (hour === 9 && minutes > 0).

Suggested fix
-const isLate = now.getHours() >= 9 && now.getMinutes() > 0;
+const isLate = now.getHours() > 9 || (now.getHours() === 9 && now.getMinutes() > 0);
📝 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 isLate = now.getHours() >= 9 && now.getMinutes() > 0;
const isLate = now.getHours() > 9 || (now.getHours() === 9 && now.getMinutes() > 0);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/controllers/attendanceController.js` at line 35, The isLate
calculation currently uses "now.getHours() >= 9 && now.getMinutes() > 0" which
incorrectly treats 9:00 as not late and 10:00 as late only when minutes > 0;
update the condition in the attendanceController.js where isLate is computed to
use "hour > 9 || (hour === 9 && minutes > 0)" logic by reading now.getHours()
and now.getMinutes() into variables (or inline) and setting isLate accordingly
so 9:01+ is late but 9:00 is not.

Comment on lines +56 to +69
const workingHours = parseFloat(diffHours.toFixed(2));

let dayType = "Half Day";
if (workingHours >= 8)
dayType = "Full Day";
else if (workingHours >= 6)
dayType = "Three Quarter Day";
else if (workingHours >= 4)
dayType = "Half Day";
else
dayType = "Short Day";


await existing.save();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Persist computed workingHours and dayType on checkout.

You compute both values but never assign them to existing before save(), so checkout metadata is silently dropped.

Suggested fix
 const workingHours = parseFloat(diffHours.toFixed(2));
@@
 else
     dayType = "Short Day";

+existing.workingHours = workingHours;
+existing.dayType = dayType;

 await existing.save();
📝 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 workingHours = parseFloat(diffHours.toFixed(2));
let dayType = "Half Day";
if (workingHours >= 8)
dayType = "Full Day";
else if (workingHours >= 6)
dayType = "Three Quarter Day";
else if (workingHours >= 4)
dayType = "Half Day";
else
dayType = "Short Day";
await existing.save();
const workingHours = parseFloat(diffHours.toFixed(2));
let dayType = "Half Day";
if (workingHours >= 8)
dayType = "Full Day";
else if (workingHours >= 6)
dayType = "Three Quarter Day";
else if (workingHours >= 4)
dayType = "Half Day";
else
dayType = "Short Day";
existing.workingHours = workingHours;
existing.dayType = dayType;
await existing.save();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/controllers/attendanceController.js` around lines 56 - 69, The
computed values workingHours and dayType are never assigned to the attendance
record before saving, so set them on the existing object (e.g.,
existing.workingHours = workingHours and existing.dayType = dayType) immediately
before calling existing.save() in the checkout flow to persist the metadata
(refer to variables workingHours, dayType and method existing.save).

Comment on lines +101 to +102
const limit = parseInt(req.query.limit || 30);
const history = await Attendance.find({ employeeId: employee._id }).sort({ date: -1 }).limit(limit)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Bound and sanitize attendance history limit.

parseInt(req.query.limit || 30) allows huge values and can degrade response/database performance. Clamp to a safe max (e.g., 100) and guard invalid numbers.

Suggested fix
-const limit = parseInt(req.query.limit || 30);
+const parsed = Number.parseInt(req.query.limit ?? "30", 10);
+const limit = Number.isNaN(parsed) ? 30 : Math.min(Math.max(parsed, 1), 100);
📝 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 limit = parseInt(req.query.limit || 30);
const history = await Attendance.find({ employeeId: employee._id }).sort({ date: -1 }).limit(limit)
const parsed = Number.parseInt(req.query.limit ?? "30", 10);
const limit = Number.isNaN(parsed) ? 30 : Math.min(Math.max(parsed, 1), 100);
const history = await Attendance.find({ employeeId: employee._id }).sort({ date: -1 }).limit(limit)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/controllers/attendanceController.js` around lines 101 - 102, The limit
parsing in attendanceController.js (variable limit used in
Attendance.find(...).limit(limit)) is unsafe; parseInt(req.query.limit || 30)
can produce NaN, negative, or huge values. Fix by parsing with a radix
(parseInt(req.query.limit, 10)), defaulting to 30 when invalid (NaN or <=0),
then clamp to a safeMax (e.g., 100) using Math.min to ensure limit is an integer
within range before passing it to Attendance.find(...).limit(limit).

Comment thread server/middleware/auth.js
Comment on lines +29 to +31
export const adminProtect = (req, res, next) => {
if (!req?.session?.role !== "ADMIN") {
return res.status(401).json({ error: "Admin access required" })
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix the admin role check.

!req?.session?.role !== "ADMIN" is effectively always truthy, so adminProtect denies admins too. Since the employee CRUD routes use this middleware, those endpoints become unreachable.

Suggested fix
 export const adminProtect = (req, res, next) => {
-    if (!req?.session?.role !== "ADMIN") {
+    if (req?.session?.role !== "ADMIN") {
         return res.status(401).json({ error: "Admin access required" })
     }
📝 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
export const adminProtect = (req, res, next) => {
if (!req?.session?.role !== "ADMIN") {
return res.status(401).json({ error: "Admin access required" })
export const adminProtect = (req, res, next) => {
if (req?.session?.role !== "ADMIN") {
return res.status(401).json({ error: "Admin access required" })
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/middleware/auth.js` around lines 29 - 31, The adminProtect middleware
currently uses the incorrect condition `!req?.session?.role !== "ADMIN"`, which
incorrectly denies admins; update the check in the adminProtect function to
directly verify the session role (e.g., if (req?.session?.role !== "ADMIN")
return 401) or equivalently guard with if (!(req?.session?.role === "ADMIN")) so
that only non-admins are rejected; reference the adminProtect function and the
req.session.role property when making the change.

Comment on lines +5 to +10
employeeId: {
type: mongoose.Schema.Types.ObjectId,
ref: "Employee",
required: true,
unique: true
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Remove the per-employee unique constraint.

unique: true on employeeId allows only one attendance record per employee across all dates, so the second day’s record will fail before the { employeeId, date } index can help.

Suggested fix
     employeeId: {
         type: mongoose.Schema.Types.ObjectId,
         ref: "Employee",
-        required: true,
-        unique: true
+        required: true
     },
📝 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
employeeId: {
type: mongoose.Schema.Types.ObjectId,
ref: "Employee",
required: true,
unique: true
},
employeeId: {
type: mongoose.Schema.Types.ObjectId,
ref: "Employee",
required: true
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/models/Attendance.js` around lines 5 - 10, The employeeId field in the
Attendance schema currently has unique: true which prevents more than one
attendance record per employee; remove the unique: true option from the
employeeId definition in server/models/Attendance.js so individual employees can
have multiple records and rely on the compound index (e.g., the { employeeId,
date } index) to enforce uniqueness per-day; update the employeeId field
definition (the employeeId property in the Attendance schema) to omit unique and
keep type, ref, and required as-is.

Comment thread server/models/Employee.js

const EmployeeSchema = new mongoose.Schema({
userId: {
type: mongoose.Schema.types.ObjectId,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify ObjectId API usage patterns in models
rg -n --type=js 'Schema\.types\.ObjectId|Schema\.Types\.ObjectId'

Repository: harshGupta090722/EMS

Length of output: 214


Use the correct Mongoose ObjectId type API.

Line 7 uses mongoose.Schema.types.ObjectId (lowercase t); this should be mongoose.Schema.Types.ObjectId (uppercase T). The incorrect casing will cause the schema to fail at runtime.

Suggested fix
-        type: mongoose.Schema.types.ObjectId,
+        type: mongoose.Schema.Types.ObjectId,
📝 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
type: mongoose.Schema.types.ObjectId,
type: mongoose.Schema.Types.ObjectId,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/models/Employee.js` at line 7, In server/models/Employee.js the schema
uses the wrong Mongoose ObjectId API (mongoose.Schema.types.ObjectId); update
the schema field to use mongoose.Schema.Types.ObjectId (uppercase "T") so the
Employee schema loads correctly—search for the occurrence of
mongoose.Schema.types.ObjectId in the file and replace it with
mongoose.Schema.Types.ObjectId.

Comment thread server/models/User.js
Comment on lines +14 to +18
role: {
type: String,
enum: ["ADMIN", "EMPLOYEE"],
dafault: "EMPLOYEE"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix the misspelled default option on role.

dafault is ignored by Mongoose, so users created without an explicit role will not get the intended "EMPLOYEE" fallback. That can leave downstream auth checks operating on undefined.

Suggested fix
     role: {
         type: String,
         enum: ["ADMIN", "EMPLOYEE"],
-        dafault: "EMPLOYEE"
+        default: "EMPLOYEE"
     }
📝 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
role: {
type: String,
enum: ["ADMIN", "EMPLOYEE"],
dafault: "EMPLOYEE"
}
role: {
type: String,
enum: ["ADMIN", "EMPLOYEE"],
default: "EMPLOYEE"
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/models/User.js` around lines 14 - 18, The User model's role field has
a misspelled option key "dafault" so Mongoose ignores the intended fallback;
update the role field in server/models/User.js by replacing the incorrect
"dafault" property with the correct "default" property and set it to "EMPLOYEE"
(i.e., ensure the role schema definition for role includes default: "EMPLOYEE"
alongside type and enum).

@@ -0,0 +1,10 @@
import { Router } from "express";
import { clockInOut, getAttendance } from "../controllers/attendanceController.js";
import { protect } from "../middlewares/auth.js";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix middleware import path before route registration.

Line 3 imports from ../middlewares/auth.js, but the auth middleware in this PR context is under ../middleware/auth.js. This will fail module loading and break /api/attendance routes.

Suggested fix
-import { protect } from "../middlewares/auth.js";
+import { protect } from "../middleware/auth.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 { protect } from "../middlewares/auth.js";
import { protect } from "../middleware/auth.js";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/routes/AttendanceRoutes.js` at line 3, The import path for the auth
middleware is incorrect: update the import of the protect middleware in
AttendanceRoutes.js (the current import statement that references
"../middlewares/auth.js") to point to the actual module location
"../middleware/auth.js" so the protect middleware used in the route
registrations is resolved correctly.

@harshGupta090722 harshGupta090722 merged commit a4541cb into main Apr 30, 2026
1 check passed
This was referenced Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant