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
WalkthroughNew Smart Room Booking feature: backend adds Room and RoomBooking models, REST controllers and routes with clash detection and role-based endpoints; frontend adds a RoomBooking component, page, navigation, and route protections for booking creation, review, and cancellation. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend
participant API as Backend API
participant DB as Database
User->>Frontend: View availability / select room, date, times
Frontend->>API: GET /api/rooms/availability?roomId=&date=
API->>DB: Query bookings for room & date (Pending/Approved)
DB-->>API: Return bookings
API-->>Frontend: Availability data
alt Client detects clash
Frontend->>User: Display clash warning
else No client-side clash
User->>Frontend: Submit booking request
Frontend->>API: POST /api/rooms/book {roomId,date,startTime,endTime,...}
API->>DB: Check for overlapping bookings (Pending/Approved)
alt Server-side clash found
API-->>Frontend: 409 Conflict + conflicting booking
else No clash
API->>DB: Create RoomBooking (status: Pending)
DB-->>API: Booking created
API-->>Frontend: 201 Created + booking
Frontend->>API: GET /api/rooms/bookings (refresh)
API->>DB: Fetch bookings (populated)
DB-->>API: Return bookings
API-->>Frontend: Updated booking list
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
![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.
Member
|
@gmarav05 From a UI/UX perspective, the interface currently needs some changes and improvements
|
Contributor
Author
|
@harshitap1305 yeah sure I will fix them. Sorry for the delay i had college exams. I will update this soon in my new commit. |
…sts, and past-time validation
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/Components/RoomBooking.jsx`:
- Around line 161-176: The refreshData function currently only calls fetchRooms
and fetchBookings so the event list stays stale; update refreshData to also call
the event-fetching logic (e.g., invoke fetchEvents alongside
fetchRooms/fetchBookings via Promise.all), then call setEvents with the returned
events and, if needed, update defaults for event selection by setting
filters.eventId and bookingForm.eventId when no value exists (similar to how
roomId is handled) so the “Linked Event” dropdown is refreshed after
creating/renaming events.
- Around line 330-339: The toggleAmenity logic and selection checks use exact
string equality so differently-cased amenity labels (e.g., "Projector" vs
"projector") produce duplicates; update toggleAmenity, addAmenityChip, and any
uses of selected to compare and dedupe amenities using a normalized form (e.g.,
toLowerCase()) and ensure new amenities are added in the same normalized form or
that the stored list maintains a canonical casing while comparisons use the
normalized key; specifically, in the toggleAmenity function, check existence
with prev.amenities.some(a => normalize(a) === normalize(amenity)) and filter
similarly, and update addAmenityChip/selected to use the same normalize(...)
helper so all checks and updates are consistent.
- Around line 524-555: The create-room inputs (controlled by roomForm and
setRoomForm) rely only on placeholders and lack accessible names; update each
text/number input to include an associated <label> linked via id/htmlFor or add
a clear aria-label/aria-labelledby so screen readers can identify them (e.g.,
the name, capacity, location inputs tied to roomForm.name/capacity/location).
Also ensure any sibling labels in other form blocks are connected using matching
id/htmlFor, and give the remove-amenity icon button an accessible name
(aria-label or visually hidden label) so it’s announced to assistive tech; apply
the same fixes to the other form controls mentioned.
- Around line 1008-1054: The current rendering filters inline and then checks
myRequests.length for the empty state, which leaves the table blank when there
are requests but none match filters.status; change this to compute a
filteredRequests array (e.g., const filteredRequests = myRequests.filter(booking
=> !filters.status || booking.status === filters.status)) and use
filteredRequests both for the .map that renders rows (still using booking._id,
booking.room?.name, formatDateTime, statusStyleMap and handleCancelBooking) and
for the empty-state check (filteredRequests.length === 0) so the "No requests
found." row appears when no bookings match the filter.
- Around line 847-867: Add a clickable handler for each time row so clicking a
slot copies room/date/time into the existing bookingForm state: implement a
small handler (e.g., onTimeSlotClick(slot)) that calls
setBookingForm({...bookingForm, room: currentRoom, date: selectedDate,
startTime: slot.start, endTime: slot.end, purpose: slot.booking?.purpose || ""})
and attach it to the row/rendered cell inside the timeSlots.map (reference the
timeSlots.map loop and slot object). Make the row accessible (role="button",
tabindex=0, onKeyPress to trigger on Enter) and add a visual cue (cursor:
pointer) so vacant or booked slots can be filled into the bookingForm; for
booked slots you can still populate form from slot.booking fields
(title/purpose/status) when present.
- Around line 64-69: The formatDate helper uses new Date(value) which treats
"YYYY-MM-DD" as UTC midnight and causes timezone shifts; change formatDate to
parse the input string into local date components (e.g., split the "YYYY-MM-DD"
string and construct new Date(year, month-1, day)) before calling
toLocaleDateString so the displayed day is correct for local time; update the
formatDate function to handle Date objects and strings robustly (detect string
pattern, parse into local Date) and keep using toLocaleDateString for
formatting.
🪄 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: 888e90df-8173-47cb-bba5-91d4927e66f9
📒 Files selected for processing (1)
frontend/src/Components/RoomBooking.jsx
Comment on lines
+64
to
+69
| const formatDate = (value) => | ||
| new Date(value).toLocaleDateString("en-IN", { | ||
| day: "2-digit", | ||
| month: "short", | ||
| year: "numeric", | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
for tz in UTC America/New_York Asia/Kolkata; do
TZ="$tz" node - <<'NODE'
const value = "2026-04-03";
const parsed = new Date(value);
console.log({
tz: process.env.TZ,
year: parsed.getFullYear(),
month: parsed.getMonth() + 1,
day: parsed.getDate(),
});
NODE
doneRepository: OpenLake/Student_Database_COSA
Length of output: 1796
🏁 Script executed:
#!/bin/bash
node - <<'NODE'
const value = "2026-04-03";
const parsed = new Date(value);
// Simulate timezone behavior
const tzTests = [
{ tz: "UTC", offset: 0 },
{ tz: "America/New_York", offset: -4 }, // EDT, April is EDT
{ tz: "Asia/Kolkata", offset: 5.5 }
];
console.log("Input: " + value);
console.log("Parsed as UTC: " + parsed.toISOString());
console.log("\nLocalDate method (getDate() returns):");
console.log(" year: " + parsed.getFullYear());
console.log(" month: " + (parsed.getMonth() + 1));
console.log(" day: " + parsed.getDate() + " ← This is UTC day, not local");
console.log("\nDemo: Manual timezone offset calculation:");
console.log(" UTC time: 2026-04-03 00:00:00");
console.log(" In America/New_York (UTC-4): 2026-04-02 20:00:00 ← Previous day!");
console.log(" In Asia/Kolkata (UTC+5:30): 2026-04-03 05:30:00 ← Same day");
NODERepository: OpenLake/Student_Database_COSA
Length of output: 1881
Fix timezone-dependent date shift in formatDate helper.
This helper parses plain date strings ("2026-04-03") using new Date(), which interprets them as UTC midnight. For users in negative UTC offsets (e.g., America/New_York), the resulting .toLocaleDateString() call converts back to the previous calendar day—causing timeline headers to display April 2 instead of April 3. Parse the string components into a local date instead, or use a library like date-fns with explicit timezone handling.
🤖 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 64 - 69, The formatDate
helper uses new Date(value) which treats "YYYY-MM-DD" as UTC midnight and causes
timezone shifts; change formatDate to parse the input string into local date
components (e.g., split the "YYYY-MM-DD" string and construct new Date(year,
month-1, day)) before calling toLocaleDateString so the displayed day is correct
for local time; update the formatDate function to handle Date objects and
strings robustly (detect string pattern, parse into local Date) and keep using
toLocaleDateString for formatting.
Comment on lines
+161
to
+176
| const refreshData = async () => { | ||
| try { | ||
| const [roomsData, bookingsData] = await Promise.all([ | ||
| fetchRooms(), | ||
| fetchBookings(), | ||
| ]); | ||
|
|
||
| setRooms(roomsData); | ||
| setBookings(bookingsData); | ||
|
|
||
| if (!filters.roomId && roomsData.length > 0) { | ||
| setFilters((prev) => ({ ...prev, roomId: roomsData[0]._id })); | ||
| } | ||
| if (!bookingForm.roomId && roomsData.length > 0) { | ||
| setBookingForm((prev) => ({ ...prev, roomId: roomsData[0]._id })); | ||
| } |
Contributor
There was a problem hiding this comment.
Refresh leaves the event dropdown stale.
refreshData only reloads rooms and bookings. If an event is created or renamed after mount, the “Linked Event” options stay outdated until a full page reload. Reuse the event-fetch logic here too.
🤖 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 161 - 176, The
refreshData function currently only calls fetchRooms and fetchBookings so the
event list stays stale; update refreshData to also call the event-fetching logic
(e.g., invoke fetchEvents alongside fetchRooms/fetchBookings via Promise.all),
then call setEvents with the returned events and, if needed, update defaults for
event selection by setting filters.eventId and bookingForm.eventId when no value
exists (similar to how roomId is handled) so the “Linked Event” dropdown is
refreshed after creating/renaming events.
Comment on lines
+330
to
+339
| const toggleAmenity = (amenity) => { | ||
| setRoomForm((prev) => { | ||
| const exists = prev.amenities.includes(amenity); | ||
| return { | ||
| ...prev, | ||
| amenities: exists | ||
| ? prev.amenities.filter((item) => item !== amenity) | ||
| : [...prev.amenities, amenity], | ||
| }; | ||
| }); |
Contributor
There was a problem hiding this comment.
Amenity matching becomes inconsistent once casing differs.
A custom projector is deduped in addAmenityChip, but clicking the preset Projector still adds a second chip because toggleAmenity and selected use exact string equality. Normalize both checks to the same case.
💡 Suggested fix
const toggleAmenity = (amenity) => {
setRoomForm((prev) => {
- const exists = prev.amenities.includes(amenity);
+ const exists = prev.amenities.some(
+ (item) => item.toLowerCase() === amenity.toLowerCase(),
+ );
return {
...prev,
amenities: exists
- ? prev.amenities.filter((item) => item !== amenity)
+ ? prev.amenities.filter(
+ (item) => item.toLowerCase() !== amenity.toLowerCase(),
+ )
: [...prev.amenities, amenity],
};
});
};-const selected = roomForm.amenities.includes(amenity);
+const selected = roomForm.amenities.some(
+ (item) => item.toLowerCase() === amenity.toLowerCase(),
+);Also applies to: 564-574
🤖 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 330 - 339, The
toggleAmenity logic and selection checks use exact string equality so
differently-cased amenity labels (e.g., "Projector" vs "projector") produce
duplicates; update toggleAmenity, addAmenityChip, and any uses of selected to
compare and dedupe amenities using a normalized form (e.g., toLowerCase()) and
ensure new amenities are added in the same normalized form or that the stored
list maintains a canonical casing while comparisons use the normalized key;
specifically, in the toggleAmenity function, check existence with
prev.amenities.some(a => normalize(a) === normalize(amenity)) and filter
similarly, and update addAmenityChip/selected to use the same normalize(...)
helper so all checks and updates are consistent.
Comment on lines
+524
to
+555
| <div className="grid grid-cols-1 md:grid-cols-3 gap-3"> | ||
| <input | ||
| type="text" | ||
| placeholder="Room name (e.g., LH-101)" | ||
| value={roomForm.name} | ||
| onChange={(event) => | ||
| setRoomForm((prev) => ({ ...prev, name: event.target.value })) | ||
| } | ||
| className="rounded-lg border border-slate-300 px-3 py-2 text-sm" | ||
| required | ||
| /> | ||
| <input | ||
| type="number" | ||
| min="1" | ||
| placeholder="Capacity" | ||
| value={roomForm.capacity} | ||
| onChange={(event) => | ||
| setRoomForm((prev) => ({ ...prev, capacity: event.target.value })) | ||
| } | ||
| className="rounded-lg border border-slate-300 px-3 py-2 text-sm" | ||
| required | ||
| /> | ||
| <input | ||
| type="text" | ||
| placeholder="Location" | ||
| value={roomForm.location} | ||
| onChange={(event) => | ||
| setRoomForm((prev) => ({ ...prev, location: event.target.value })) | ||
| } | ||
| className="rounded-lg border border-slate-300 px-3 py-2 text-sm" | ||
| required | ||
| /> |
Contributor
There was a problem hiding this comment.
Several controls still lack accessible names.
The create-room inputs rely on placeholders, the other forms render <label> siblings without htmlFor/id links, and the remove-amenity icon button has no accessible name. Screen-reader users will miss the clear-label accessibility target here.
Also applies to: 583-594, 612-614, 673-768, 808-833, 981-993
🤖 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 524 - 555, The
create-room inputs (controlled by roomForm and setRoomForm) rely only on
placeholders and lack accessible names; update each text/number input to include
an associated <label> linked via id/htmlFor or add a clear
aria-label/aria-labelledby so screen readers can identify them (e.g., the name,
capacity, location inputs tied to roomForm.name/capacity/location). Also ensure
any sibling labels in other form blocks are connected using matching id/htmlFor,
and give the remove-amenity icon button an accessible name (aria-label or
visually hidden label) so it’s announced to assistive tech; apply the same fixes
to the other form controls mentioned.
Comment on lines
+847
to
+867
| {timeSlots.map((slot) => ( | ||
| <tr key={slot.label} className="border-t border-slate-100"> | ||
| <td className="py-2 px-3 whitespace-nowrap">{slot.label}</td> | ||
| <td className="py-2 px-3"> | ||
| <span | ||
| className={`text-xs px-2 py-1 rounded-full font-medium ${ | ||
| slot.booking | ||
| ? "bg-rose-100 text-rose-700" | ||
| : "bg-emerald-100 text-emerald-700" | ||
| }`} | ||
| > | ||
| {slot.booking ? "Booked" : "Vacant"} | ||
| </span> | ||
| </td> | ||
| <td className="py-2 px-3 text-slate-700"> | ||
| {slot.booking | ||
| ? `${slot.booking.event?.title || slot.booking.purpose || "General booking"} (${slot.booking.status})` | ||
| : "-"} | ||
| </td> | ||
| </tr> | ||
| ))} |
Contributor
There was a problem hiding this comment.
Availability rows are still read-only.
The timetable visualizes vacancy, but there is no click target that copies room/date/time into bookingForm. Users still have to re-enter the same slot manually, so the click-to-fill UX this PR was aiming for is still missing.
🤖 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 847 - 867, Add a
clickable handler for each time row so clicking a slot copies room/date/time
into the existing bookingForm state: implement a small handler (e.g.,
onTimeSlotClick(slot)) that calls setBookingForm({...bookingForm, room:
currentRoom, date: selectedDate, startTime: slot.start, endTime: slot.end,
purpose: slot.booking?.purpose || ""}) and attach it to the row/rendered cell
inside the timeSlots.map (reference the timeSlots.map loop and slot object).
Make the row accessible (role="button", tabindex=0, onKeyPress to trigger on
Enter) and add a visual cue (cursor: pointer) so vacant or booked slots can be
filled into the bookingForm; for booked slots you can still populate form from
slot.booking fields (title/purpose/status) when present.
Comment on lines
+1008
to
+1054
| {myRequests | ||
| .filter((booking) => { | ||
| if (!filters.status) return true; | ||
| return booking.status === filters.status; | ||
| }) | ||
| .map((booking) => { | ||
| const canCancel = !["Cancelled", "Rejected"].includes(booking.status); | ||
| return ( | ||
| <tr key={booking._id} className="border-b border-slate-100 align-top"> | ||
| <td className="py-3 pr-3 text-slate-900 font-medium">{booking.room?.name || "-"}</td> | ||
| <td className="py-3 pr-3 text-slate-700"> | ||
| {booking.event?.title || booking.purpose || "General booking"} | ||
| </td> | ||
| <td className="py-3 pr-3 text-slate-600 whitespace-nowrap"> | ||
| {formatDateTime(booking.startTime)} | ||
| <br /> | ||
| {formatDateTime(booking.endTime)} | ||
| </td> | ||
| <td className="py-3 pr-3"> | ||
| <span | ||
| className={`text-xs px-2 py-1 rounded-full font-medium ${statusStyleMap[booking.status] || "bg-slate-100 text-slate-700"}`} | ||
| > | ||
| {booking.status} | ||
| </span> | ||
| </td> | ||
| <td className="py-3 pr-3"> | ||
| {canCancel ? ( | ||
| <button | ||
| onClick={() => handleCancelBooking(booking._id)} | ||
| className="inline-flex items-center gap-1 rounded-md border border-slate-300 px-2.5 py-1.5 text-xs font-medium text-slate-700 hover:bg-slate-100" | ||
| > | ||
| Cancel | ||
| </button> | ||
| ) : ( | ||
| <span className="text-xs text-slate-500">No action</span> | ||
| )} | ||
| </td> | ||
| </tr> | ||
| ); | ||
| })} | ||
| {myRequests.length === 0 && ( | ||
| <tr> | ||
| <td colSpan="5" className="py-8 text-center text-slate-500"> | ||
| No requests found. | ||
| </td> | ||
| </tr> | ||
| )} |
Contributor
There was a problem hiding this comment.
The status filter can render a blank table with no message.
The fallback only checks myRequests.length. If the user has requests but none match the selected status, the <tbody> renders nothing at all. Base both the rows and the empty state on the filtered array.
🤖 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 1008 - 1054, The
current rendering filters inline and then checks myRequests.length for the empty
state, which leaves the table blank when there are requests but none match
filters.status; change this to compute a filteredRequests array (e.g., const
filteredRequests = myRequests.filter(booking => !filters.status ||
booking.status === filters.status)) and use filteredRequests both for the .map
that renders rows (still using booking._id, booking.room?.name, formatDateTime,
statusStyleMap and handleCancelBooking) and for the empty-state check
(filteredRequests.length === 0) so the "No requests found." row appears when no
bookings match the filter.
Contributor
Author
|
Hello @harshitap1305 Can you please review my PR. Please let me know if you have any feedback or suggestions. |
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
2 participants
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
UI / Navigation