Implement Event Registration Functionality.#216
Conversation
|
@UtkarshUmap is attempting to deploy a commit to the openlake's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Rate limit exceeded@UtkarshUmap has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds student event registration: backend POST Changes
Sequence Diagram(s)sequenceDiagram
participant Student (UI)
participant Frontend App
participant Backend API
participant Database
participant Auth Service
Student ->> Frontend App: Click "Register" on EventTile
Frontend App ->> Frontend App: show confirmation modal
Student ->> Frontend App: Confirm registration
Frontend App ->> Backend API: POST /api/events/:eventId/register (auth token)
Backend API ->> Auth Service: validate token / get userId
Auth Service -->> Backend API: userId
Backend API ->> Database: find event by eventId
Database -->> Backend API: event document
Backend API ->> Database: update $addToSet participants (with registration window & capacity checks)
Database -->> Backend API: updated event / error
Backend API -->> Frontend App: 200 OK or error (404/400/409/500)
Frontend App ->> Frontend App: update local event.participants / button state
Frontend App -->> Student: show success or error feedback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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)
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}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/Components/Events/EventTile.jsx (1)
10-23: Guard registration state whencurrentUserIdis missing (avoid enabling Register for unauthenticated/unknown user).Right now
String(currentUserId)withundefined/nullwill makeisRegisteredfalse and enable the Register button, which then fails later at the API layer.Proposed patch
- const isRegistered = event.participants?.some( - (p) => String(p?._id || p) === String(currentUserId), - ); + const isRegistered = + !!currentUserId && + event.participants?.some((p) => String(p?._id || p) === String(currentUserId));frontend/src/Components/Events/EventList.jsx (1)
47-93: Fix optimistic update: prevent duplicate participants + avoid mixed participant shapes; add in-flight state.Current code can (1) append the same user multiple times (double-click Confirm) and (2) mutate
participantsfrom “populated objects” into a mixed array ({_id: ...}objects + raw ids), which can break other UI that assumes a consistent shape.Proposed patch
const EventList = () => { @@ const [selectedEvent, setSelectedEvent] = useState(null); const [showConfirm, setShowConfirm] = useState(false); + const [isRegistering, setIsRegistering] = useState(false); @@ const confirmRegistration = async () => { try { - const response = await api.post( - `/api/events/${selectedEvent._id}/register`, - ); + if (!selectedEvent?._id || !currentUserId || isRegistering) return; + setIsRegistering(true); + + await api.post(`/api/events/${selectedEvent._id}/register`); // update local state so UI updates immediately - updateEvent({ - ...selectedEvent, - participants: [...(selectedEvent.participants || []), currentUserId], - }); + updateEvent((() => { + const prev = selectedEvent.participants || []; + const already = prev.some((p) => String(p?._id || p) === String(currentUserId)); + if (already) return selectedEvent; + // keep shape compatible with "populated participants" usage + return { ...selectedEvent, participants: [...prev, { _id: currentUserId }] }; + })()); setShowConfirm(false); } catch (err) { const message = err.response?.data?.message || "Registration failed"; alert(message); + } finally { + setIsRegistering(false); } };
🤖 Fix all issues with AI agents
In @backend/routes/events.js:
- Around line 208-236: The current non-atomic check reads
event.participants.length then calls Event.findByIdAndUpdate which can
oversubscribe under concurrent requests; replace the two-step logic with a
single conditional update such as using Event.findOneAndUpdate (or a
transaction) that includes a predicate enforcing capacity — e.g., include
event.registration.max_participants in the query (using $expr and $lt with $size
or compare participants array length) and perform the $addToSet in the same
operation so the insert only succeeds if participants.length <
event.registration.max_participants; update the code paths that currently
reference event.registration.max_participants, event.participants, and
Event.findByIdAndUpdate accordingly and handle the case where findOneAndUpdate
returns null as the registration-full response.
🧹 Nitpick comments (3)
frontend/src/Components/Events/EventTile.jsx (1)
52-69: Make the Register button safer (type="button", optional callback, and disable whencurrentUserIdmissing).This prevents accidental form submission and avoids runtime errors if
onRegisterisn’t provided (or while auth state is still loading).Proposed patch
<button + type="button" onClick={(e) => { e.stopPropagation(); - onRegister(event); + onRegister?.(event); }} - disabled={isRegistered} + disabled={isRegistered || !currentUserId} className={`inline-flex items-center px-3 py-1 rounded-full text-xs font-medium transition ${ isRegistered ? "bg-green-100 text-green-800 cursor-default" : "bg-blue-100 text-blue-800 hover:bg-blue-200" }`} >frontend/src/Components/Events/EventList.jsx (2)
110-116: Prefer a stable key (event._id) over array index forEventTile.This avoids incorrect tile reuse when events are inserted/removed/re-sorted.
Proposed patch
- <EventTile - key={i} + <EventTile + key={event._id} index={i + 1} event={event} currentUserId={currentUserId} onRegister={handleRegister} />
136-161: Add minimal dialog semantics + disable actions while registering.This improves accessibility and prevents interactions during an in-flight request.
Proposed patch
{showConfirm && selectedEvent && ( - <div className="fixed inset-0 bg-black/40 flex items-center justify-center z-50"> - <div className="bg-white rounded-lg p-6 w-96"> + <div className="fixed inset-0 bg-black/40 flex items-center justify-center z-50"> + <div + className="bg-white rounded-lg p-6 w-96" + role="dialog" + aria-modal="true" + aria-label="Confirm registration" + > @@ <button className="px-4 py-2 bg-gray-100 rounded" onClick={() => setShowConfirm(false)} + disabled={isRegistering} > Cancel </button> <button className="px-4 py-2 bg-blue-600 text-white rounded" onClick={confirmRegistration} + disabled={isRegistering} > Confirm </button>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/routes/events.jsfrontend/src/Components/Events/EventList.jsxfrontend/src/Components/Events/EventTile.jsx
🧰 Additional context used
🧬 Code graph analysis (3)
backend/routes/events.js (2)
backend/models/schema.js (2)
User(634-634)Event(639-639)backend/utils/roles.js (1)
ROLES(1-9)
frontend/src/Components/Events/EventTile.jsx (3)
frontend/src/hooks/useSidebar.js (2)
useSidebar(39-47)useSidebar(39-47)frontend/src/utils/eventHelpers.js (2)
formatDate(1-5)formatDate(1-5)frontend/src/Components/Events/EventList.jsx (1)
currentUserId(47-47)
frontend/src/Components/Events/EventList.jsx (3)
frontend/src/utils/api.js (1)
api(2-5)frontend/src/hooks/useEvents.js (1)
updateEvent(53-59)frontend/src/Components/Events/EventTile.jsx (1)
EventTile(10-90)
Changes done as suggested by CodeRabbitAI .
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/src/Components/Events/EventTile.jsx:
- Around line 20-22: The error is caused by stray diff marker characters ('+')
at the start of the lines defining the isRegistered constant; remove those
leading '+' characters so the declaration reads as a normal JS/JSX statement.
Locate the isRegistered definition (uses currentUserId and
event.participants?.some((p) => String(p?._id || p) === String(currentUserId)))
and ensure there are no diff artifacts or extra characters before "const
isRegistered =" so the parser can compile correctly.
🧹 Nitpick comments (2)
frontend/src/Components/Events/EventTile.jsx (2)
14-15: Consider adding PropTypes validation for new props.The new props
currentUserIdandonRegisterare required for the registration flow but lack validation. Adding PropTypes (or TypeScript types) would help catch integration issues early.🔍 Proposed PropTypes validation
Add this import at the top:
import PropTypes from 'prop-types';Add this at the bottom of the file:
EventTile.propTypes = { event: PropTypes.object.isRequired, index: PropTypes.number.isRequired, compact: PropTypes.bool, currentUserId: PropTypes.string, onRegister: PropTypes.func, };
54-68: Consider adding a loading state during registration.The button implementation correctly prevents re-registration via
disabled={isRegistered}, but doesn't account for the in-progress state during the asynconRegistercall. Users could potentially trigger multiple registration attempts if they click rapidly before the state updates.💡 Optional enhancement with loading state
If
EventList.jsx(the parent component) exposes a loading state, you could enhance the button:disabled={isRegistered || isRegistering} className={`inline-flex items-center px-3 py-1 rounded-full text-xs font-medium transition ${ isRegistered ? "bg-green-100 text-green-800 cursor-default" : isRegistering ? "bg-gray-100 text-gray-600 cursor-wait" : "bg-blue-100 text-blue-800 hover:bg-blue-200" }`} > {isRegistered ? "Registered" : isRegistering ? "Registering..." : "Register"} </button>This would require passing an
isRegisteringprop from the parent component.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/Components/Events/EventTile.jsx
🧰 Additional context used
🪛 Biome (2.1.2)
frontend/src/Components/Events/EventTile.jsx
[error] 20-20: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
🔇 Additional comments (1)
frontend/src/Components/Events/EventTile.jsx (1)
20-22: Well-implemented registration status check.The
isRegisteredlogic correctly handles edge cases: missingcurrentUserId, undefined participants array, and participants stored as either populated objects (p._id) or plain ObjectId strings. The string conversion ensures reliable comparison across different data shapes.
Done Changes suggested by CodeRabbitAI.
Changes suggested by CodeRabbitAI .
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/routes/events.js (1)
243-245: Inconsistent style and indentation issue.
- Line 243 uses optional chaining (
error?.name), but the codebase elsewhere uses defensive checks for "lint/parse compatibility" (see comment at line 156). Line 370 also useserror.namewithout optional chaining.- Line 244 has incorrect indentation (7 spaces instead of 8).
✨ Suggested fix
- if (error?.name === "CastError") { - return res.status(400).json({ message: "Invalid event ID format." }); + if (error && error.name === "CastError") { + return res.status(400).json({ message: "Invalid event ID format." }); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/routes/events.js
🧰 Additional context used
🧬 Code graph analysis (1)
backend/routes/events.js (2)
backend/models/schema.js (2)
User(634-634)Event(639-639)backend/utils/roles.js (1)
ROLES(1-9)
🔇 Additional comments (3)
backend/routes/events.js (3)
136-136: LGTM!Minor spacing/formatting adjustment with no functional impact.
181-206: Good implementation of the protected registration endpoint.The middleware chain correctly enforces authentication and student role. The use of
.equals()for ObjectId comparison at line 202 is appropriate, and returning a 409 for duplicate registrations provides clear feedback to the frontend.
208-222: Registration window validation looks correct.The conditional checks for
registration.startandregistration.endare properly handled, only enforced whenregistration.requiredis true, aligning with the schema design.
2 participants
name: " Pull Request"
about: Create Register button for event on student dashboard.
title: "Implement Event Registration Functionality."
Related Issue
Changes Introduced
Why This Change?
Summary by CodeRabbit
New Features
Bug Fixes / Validation
✏️ Tip: You can customize this high-level summary in your review settings.