Open
Conversation
|
@gmarav05 is attempting to deploy a commit to the openlake's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
WalkthroughA new Smart Room Booking system was implemented across backend and frontend. Backend introduces room and booking models, REST controllers for room management and booking workflows with clash detection, and authenticated routes with role-based access control. Frontend replaces a basic form with a comprehensive interface featuring room creation, booking management with clash warning, availability filtering, status updates, and cancellation with role-specific actions. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend
participant API as Backend API
participant DB as Database
User->>Frontend: Selects room, date, time slot
Frontend->>API: GET /api/rooms/availability
API->>DB: Query bookings by room & date
DB-->>API: Return pending/approved bookings
API-->>Frontend: Return availability data
alt Time slot clashes
Frontend->>User: Display "Clash Warning"
else No clash detected
User->>Frontend: Submit booking request
Frontend->>API: POST /api/rooms/book
API->>API: Validate time range clash
alt Clash found
API-->>Frontend: 409 Conflict
Frontend->>User: Show conflicting booking
else No clash
API->>DB: Create RoomBooking (Pending)
DB-->>API: Booking created
API-->>Frontend: 201 + booking details
Frontend->>API: GET /api/rooms/bookings
API->>DB: Fetch all bookings (populated)
DB-->>API: Return bookings with room/event/user
API-->>Frontend: Updated booking list
Frontend->>User: Refresh & display new booking
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The review spans heterogeneous changes across backend models, controllers, routes, and a comprehensive frontend rewrite. Key areas requiring careful attention: clash detection logic correctness, authorization middleware enforcement across role-based endpoints (president vs. general secretary distinction), request body validation in booking submission, role-aware UI state management in the component, and API integration patterns with the shared Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
frontend/src/Components/RoomBooking.jsx (2)
514-538: Consider adding explicit label-input associations for better accessibility.The labels are nested correctly, but adding explicit
idandhtmlForattributes would improve screen reader compatibility and is a best practice for form accessibility.♿ Example for one input
<div> - <label className="block text-xs font-medium text-slate-600 mb-1"> + <label htmlFor="room-select" className="block text-xs font-medium text-slate-600 mb-1"> Room </label> <select + id="room-select" value={bookingForm.roomId}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/Components/RoomBooking.jsx` around lines 514 - 538, Add explicit id/htmlFor attributes to associate the Room label with its select input in the RoomBooking component: give the select a stable id (e.g., "room-select") and set the enclosing label's htmlFor to that id so screen readers correctly link the label to bookingForm.roomId; update the JSX around bookingForm.roomId, setBookingForm, and the rooms mapping accordingly and ensure the id is unique within the form.
346-376: Consider adding loading states to prevent duplicate actions.The
handleReviewBookingandhandleCancelBookingfunctions don't disable their buttons during the API call, which could allow users to trigger duplicate requests by clicking rapidly.🔄 Example approach using a processing state
+ const [processingId, setProcessingId] = useState(null); const handleReviewBooking = async (bookingId, status) => { try { setError(""); setSuccessMessage(""); + setProcessingId(bookingId); await api.put(`/api/rooms/bookings/${bookingId}/status`, { status }); // ... } catch (err) { // ... + } finally { + setProcessingId(null); } };Then in the button:
<button onClick={() => handleReviewBooking(booking._id, "Approved")} + disabled={processingId === booking._id} // ... >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/Components/RoomBooking.jsx` around lines 346 - 376, Add a per-booking "processing" state and use it inside handleReviewBooking and handleCancelBooking to prevent duplicate clicks: introduce a state like processingId (with setter setProcessingId) and at the start of each handler setProcessingId(bookingId), then clear it in a finally block (setProcessingId(null)) so errors still reset the flag; update the buttons that call handleReviewBooking and handleCancelBooking to include disabled={processingId === booking._id} (or equivalent) so the UI disables the specific booking action while its API call is in-flight.backend/controllers/roomBookingController.js (2)
114-126: Same date matching consideration applies here.The date filter at line 119 has the same exact-match fragility mentioned in
getAvailability. Consider applying the same date range approach for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/controllers/roomBookingController.js` around lines 114 - 126, The getBookings handler currently sets query.date = new Date(date) which requires exact-match; change it to use a date range like in getAvailability: compute startOfDay and endOfDay for the passed date and set query.date = { $gte: startOfDay, $lt: endOfDay } so RoomBooking.find(query).populate('room event bookedBy') returns all bookings for that calendar day; update getBookings to parse the incoming date string, create start/end Date objects, and assign the range to query.date instead of a single Date object.
99-104: Use centralized role constants instead of hardcoding.The role list duplicates
ROLE_GROUPS.ADMINfrombackend/utils/roles.js. If roles change, this code would need separate updates.♻️ Proposed refactor to use centralized roles
+const { ROLE_GROUPS } = require('../utils/roles'); + const { Room, RoomBooking, Event, User } = require('../models/schema');if ( String(booking.bookedBy) !== String(req.user._id) && - !['PRESIDENT', 'GENSEC_SCITECH', 'GENSEC_ACADEMIC', 'GENSEC_CULTURAL', 'GENSEC_SPORTS', 'CLUB_COORDINATOR'].includes(req.user.role) + !ROLE_GROUPS.ADMIN.includes(req.user.role) ) { return res.status(403).json({ message: 'Forbidden' }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/controllers/roomBookingController.js` around lines 99 - 104, Replace the hardcoded admin-role array in the authorization check with the centralized ROLE_GROUPS.ADMIN constant from backend/utils/roles.js: import ROLE_GROUPS at the top of roomBookingController.js, then change the condition that checks req.user.role (the block referencing booking and req.user._id) to use ROLE_GROUPS.ADMIN (e.g., ROLE_GROUPS.ADMIN.includes(req.user.role)) instead of the literal array; keep the existing bookedBy vs req.user._id comparison logic intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/controllers/roomBookingController.js`:
- Around line 60-70: The getAvailability handler uses an exact date match (query
= { date: new Date(date) }) which fails when stored dates include time
components; update getAvailability to build a date range instead: parse the
incoming date into a start-of-day and end-of-day (e.g., start.setHours(0,0,0,0)
and end = new Date(start); end.setDate(end.getDate()+1)) and replace the
equality with a range query on RoomBooking.find (e.g., { date: { $gte: start,
$lt: end } }), keeping the existing optional roomId branch and populate('room
event bookedBy') behavior.
In `@backend/models/schema.js`:
- Around line 697-704: Add a schema-level validation to ensure endTime >
startTime on the booking schema: in backend/models/schema.js add a custom
validator (either on the endTime field or via a pre('validate') hook) that
compares this.endTime and this.startTime and rejects/surfaces a validation error
when endTime is <= startTime; reference the booking schema (e.g., BookingSchema
or the schema that defines startTime and endTime) so saves/creates fail early
and the bookRoom overlap logic no longer assumes invalid windows.
In `@backend/seed.js`:
- Around line 26-31: The seedRooms function is defined but never invoked, so
room records aren't created; update the seedDB function to call seedRooms
(preferably await seedRooms() inside the async seedDB) or invoke seedRooms()
where other seeding functions are run so the sampleRooms are inserted; ensure
the call uses await if seedDB is async to preserve ordering and error
propagation and reference the seedRooms symbol when adding the call.
---
Nitpick comments:
In `@backend/controllers/roomBookingController.js`:
- Around line 114-126: The getBookings handler currently sets query.date = new
Date(date) which requires exact-match; change it to use a date range like in
getAvailability: compute startOfDay and endOfDay for the passed date and set
query.date = { $gte: startOfDay, $lt: endOfDay } so
RoomBooking.find(query).populate('room event bookedBy') returns all bookings for
that calendar day; update getBookings to parse the incoming date string, create
start/end Date objects, and assign the range to query.date instead of a single
Date object.
- Around line 99-104: Replace the hardcoded admin-role array in the
authorization check with the centralized ROLE_GROUPS.ADMIN constant from
backend/utils/roles.js: import ROLE_GROUPS at the top of
roomBookingController.js, then change the condition that checks req.user.role
(the block referencing booking and req.user._id) to use ROLE_GROUPS.ADMIN (e.g.,
ROLE_GROUPS.ADMIN.includes(req.user.role)) instead of the literal array; keep
the existing bookedBy vs req.user._id comparison logic intact.
In `@frontend/src/Components/RoomBooking.jsx`:
- Around line 514-538: Add explicit id/htmlFor attributes to associate the Room
label with its select input in the RoomBooking component: give the select a
stable id (e.g., "room-select") and set the enclosing label's htmlFor to that id
so screen readers correctly link the label to bookingForm.roomId; update the JSX
around bookingForm.roomId, setBookingForm, and the rooms mapping accordingly and
ensure the id is unique within the form.
- Around line 346-376: Add a per-booking "processing" state and use it inside
handleReviewBooking and handleCancelBooking to prevent duplicate clicks:
introduce a state like processingId (with setter setProcessingId) and at the
start of each handler setProcessingId(bookingId), then clear it in a finally
block (setProcessingId(null)) so errors still reset the flag; update the buttons
that call handleReviewBooking and handleCancelBooking to include
disabled={processingId === booking._id} (or equivalent) so the UI disables the
specific booking action while its API call is in-flight.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5e8049d-f375-45e9-9157-e46ca64c690a
📒 Files selected for processing (10)
backend/controllers/roomBookingController.jsbackend/index.jsbackend/models/schema.jsbackend/routes/roomBooking.jsbackend/seed.jsfrontend/src/Components/RoomBooking.jsxfrontend/src/config/dashboardComponents.jsfrontend/src/config/navbarConfig.jsfrontend/src/pages/roomBookingPage.jsxfrontend/src/routes/AdminRoutes.js
Comment on lines
+60
to
+70
| exports.getAvailability = async (req, res) => { | ||
| try { | ||
| const { date, roomId } = req.query; | ||
| const query = { date: new Date(date) }; | ||
| if (roomId) query.room = roomId; | ||
| const bookings = await RoomBooking.find(query).populate('room event bookedBy'); | ||
| res.json(bookings); | ||
| } catch (err) { | ||
| res.status(500).json({ message: 'Error fetching availability' }); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Date matching may fail due to time component differences.
The exact date match { date: new Date(date) } is fragile. If the stored date has a different time component (e.g., due to timezone handling), the query won't match. Consider using a date range query.
🛡️ Proposed fix using date range
exports.getAvailability = async (req, res) => {
try {
const { date, roomId } = req.query;
- const query = { date: new Date(date) };
+ const queryDate = new Date(date);
+ const nextDate = new Date(queryDate);
+ nextDate.setDate(nextDate.getDate() + 1);
+ const query = { date: { $gte: queryDate, $lt: nextDate } };
if (roomId) query.room = roomId;
const bookings = await RoomBooking.find(query).populate('room event bookedBy');
res.json(bookings);📝 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
| exports.getAvailability = async (req, res) => { | |
| try { | |
| const { date, roomId } = req.query; | |
| const query = { date: new Date(date) }; | |
| if (roomId) query.room = roomId; | |
| const bookings = await RoomBooking.find(query).populate('room event bookedBy'); | |
| res.json(bookings); | |
| } catch (err) { | |
| res.status(500).json({ message: 'Error fetching availability' }); | |
| } | |
| }; | |
| exports.getAvailability = async (req, res) => { | |
| try { | |
| const { date, roomId } = req.query; | |
| const queryDate = new Date(date); | |
| const nextDate = new Date(queryDate); | |
| nextDate.setDate(nextDate.getDate() + 1); | |
| const query = { date: { $gte: queryDate, $lt: nextDate } }; | |
| if (roomId) query.room = roomId; | |
| const bookings = await RoomBooking.find(query).populate('room event bookedBy'); | |
| res.json(bookings); | |
| } catch (err) { | |
| res.status(500).json({ message: 'Error fetching availability' }); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/controllers/roomBookingController.js` around lines 60 - 70, The
getAvailability handler uses an exact date match (query = { date: new Date(date)
}) which fails when stored dates include time components; update getAvailability
to build a date range instead: parse the incoming date into a start-of-day and
end-of-day (e.g., start.setHours(0,0,0,0) and end = new Date(start);
end.setDate(end.getDate()+1)) and replace the equality with a range query on
RoomBooking.find (e.g., { date: { $gte: start, $lt: end } }), keeping the
existing optional roomId branch and populate('room event bookedBy') behavior.
Comment on lines
+697
to
+704
| startTime: { | ||
| type: Date, | ||
| required: true, | ||
| }, | ||
| endTime: { | ||
| type: Date, | ||
| required: true, | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether any temporal guard already exists in model/controller paths.
rg -nP -C3 'startTime|endTime|validate|bookRoom|getAvailability' backend/models/schema.js backend/controllers/roomBookingController.jsRepository: OpenLake/Student_Database_COSA
Length of output: 3022
Add schema validation: endTime must be after startTime.
Invalid booking windows (e.g., endTime ≤ startTime) can be persisted, breaking the clash detection logic in bookRoom. The overlap query assumes valid time windows and would silently fail to detect conflicts.
💡 Proposed fix
const roomBookingSchema = new mongoose.Schema({
@@
endTime: {
type: Date,
required: true,
},
@@
});
+roomBookingSchema.path("endTime").validate(function (value) {
+ return this.startTime instanceof Date && value instanceof Date && value > this.startTime;
+}, "endTime must be later than startTime");
+
roomBookingSchema.index({ room: 1, date: 1, startTime: 1, endTime: 1 });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/models/schema.js` around lines 697 - 704, Add a schema-level
validation to ensure endTime > startTime on the booking schema: in
backend/models/schema.js add a custom validator (either on the endTime field or
via a pre('validate') hook) that compares this.endTime and this.startTime and
rejects/surfaces a validation error when endTime is <= startTime; reference the
booking schema (e.g., BookingSchema or the schema that defines startTime and
endTime) so saves/creates fail early and the bookRoom overlap logic no longer
assumes invalid windows.
Comment on lines
+26
to
+31
| const seedRooms = async () => { | ||
| console.log("Seeding sample rooms..."); | ||
| await Room.deleteMany({}); | ||
| await Room.insertMany(sampleRooms); | ||
| console.log("Sample rooms seeded!"); | ||
| }; |
Contributor
There was a problem hiding this comment.
seedRooms is never executed, so room data is never seeded.
seedRooms is defined on Line 26 but not called in seedDB(); this leaves fresh environments without room records for booking workflows.
💡 Proposed fix
async function seedDB() {
try {
@@
await clearData();
await seedOrganizationalUnits();
+ await seedRooms();
await seedUsers();
await seedPositions();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/seed.js` around lines 26 - 31, The seedRooms function is defined but
never invoked, so room records aren't created; update the seedDB function to
call seedRooms (preferably await seedRooms() inside the async seedDB) or invoke
seedRooms() where other seeding functions are run so the sampleRooms are
inserted; ensure the call uses await if seedDB is async to preserve ordering and
error propagation and reference the seedRooms symbol when adding the call.
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.
name: "Pull Request"
about: Propose and submit changes to the project for review
title: "PR: Smart Room Booking Frontend Implementation"
labels: ""
assignees: harshitap1305, sakshi1755
Related Issue
Changes Introduced
Why This Change?
Screenshots / Video (if applicable)
Screen Recording 2026-03-19 at 6.35.12 PM.zip
Testing
npm testin the relevant directory).npm run build.Documentation Updates
README.mdwith new instructions.Checklist
git checkout -b feature/my-amazing-feature).Deployment Notes
💬 Additional Notes
feature/room-booking-frontendSummary by CodeRabbit
New Features
Navigation Updates