Backend update, registration events#241
Conversation
|
@vani-tcvostro is attempting to deploy a commit to the openlake's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdded a new endpoint to retrieve a user's registered events with authentication checks. Implemented controller logic in two separate files (dashboardController and eventControllers), integrated the endpoint via a new authenticated route in the events router, and refactored error handling in the event registration endpoint. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (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)
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: 3
🧹 Nitpick comments (1)
backend/routes/events.js (1)
1-1: Consider importing from eventControllers instead.Given that
getRegisteredEventshandles event-related data, it would be more cohesive to keep it ineventControllers.jsrather thandashboardController.js. The dashboard controller handles role-based stats, while this is a straightforward event query.If you move the function to
eventControllers.js(where a duplicate already exists), update this import accordingly and remove the duplicate fromdashboardController.js.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/routes/events.js` at line 1, The route currently imports getRegisteredEvents from dashboardController; move the getRegisteredEvents function implementation to eventControllers.js (remove the duplicate in dashboardController.js), then update the import in backend/routes/events.js to require("../controllers/eventControllers") and reference the exported getRegisteredEvents there; ensure the exported function name and any dependencies (e.g., models or helper functions) are updated/required in eventControllers.js and tests or other callers still reference getRegisteredEvents.
🤖 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/eventControllers.js`:
- Around line 24-43: Remove the dead export exports.getRegisteredEvents from
backend/controllers/eventControllers.js since the route /registered-events is
wired to dashboardController.getRegisteredEvents; locate the function named
getRegisteredEvents (exports.getRegisteredEvents) in that file and delete the
entire async function block and its export to avoid duplication and confusion,
and run tests/lint to ensure no remaining references to
exports.getRegisteredEvents exist.
In `@backend/routes/events.js`:
- Around line 498-503: The "/registered-events" route registration using
router.get("/registered-events", isAuthenticated,
dashboardController.getRegisteredEvents) is declared after parameterized routes
(e.g., '/:id' and '/:eventId') so it will never be reached; relocate that
router.get(...) declaration to a position before any parameterized routes (for
example before the '/:eventId/is-contact' and '/:id' handlers) and remove the
duplicate declaration at the end of the file so the explicit
"/registered-events" path is matched first.
- Around line 259-264: The catch block in the events route has inconsistent
indentation for the lines handling CastError, console.error, and the
res.status(500) return; fix it by re-indenting the entire catch body so its
statements (the if (error?.name === "CastError") check, console.error("Event
registration error:", error), and return res.status(500).json(...)) align with
the surrounding function block style used in this file (ensure the same number
of spaces/tabs as other blocks in this handler to maintain consistent
formatting).
---
Nitpick comments:
In `@backend/routes/events.js`:
- Line 1: The route currently imports getRegisteredEvents from
dashboardController; move the getRegisteredEvents function implementation to
eventControllers.js (remove the duplicate in dashboardController.js), then
update the import in backend/routes/events.js to
require("../controllers/eventControllers") and reference the exported
getRegisteredEvents there; ensure the exported function name and any
dependencies (e.g., models or helper functions) are updated/required in
eventControllers.js and tests or other callers still reference
getRegisteredEvents.
🪄 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: d7d3c240-500e-4f7e-949e-b88e5a137ee4
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
backend/controllers/dashboardController.jsbackend/controllers/eventControllers.jsbackend/routes/events.js
| exports.getRegisteredEvents = async (req, res) => { | ||
| try { | ||
|
|
||
| if (!req.user) { | ||
| return res.status(401).json({ message: "Unauthorized: Please login first" }); | ||
| } | ||
|
|
||
| const userId = req.user._id; | ||
|
|
||
| const events = await Event.find({ | ||
| participants: userId | ||
| }); | ||
|
|
||
| res.status(200).json(events); | ||
|
|
||
| } catch (error) { | ||
| console.error("Error fetching registered events:", error); | ||
| res.status(500).json({ message: "Server error" }); | ||
| } | ||
| }; No newline at end of file |
There was a problem hiding this comment.
Dead code: this function is never used.
According to the route definition in backend/routes/events.js (lines 499-503), the /registered-events endpoint is wired to dashboardController.getRegisteredEvents, not this export. This implementation is identical to the one in dashboardController.js and is never called.
Remove this dead code to avoid confusion and maintenance burden.
🗑️ Proposed fix: remove unused function
-exports.getRegisteredEvents = async (req, res) => {
- try {
-
- if (!req.user) {
- return res.status(401).json({ message: "Unauthorized: Please login first" });
- }
-
- const userId = req.user._id;
-
- const events = await Event.find({
- participants: userId
- });
-
- res.status(200).json(events);
-
- } catch (error) {
- console.error("Error fetching registered events:", error);
- res.status(500).json({ message: "Server error" });
- }
-};📝 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.
| exports.getRegisteredEvents = async (req, res) => { | |
| try { | |
| if (!req.user) { | |
| return res.status(401).json({ message: "Unauthorized: Please login first" }); | |
| } | |
| const userId = req.user._id; | |
| const events = await Event.find({ | |
| participants: userId | |
| }); | |
| res.status(200).json(events); | |
| } catch (error) { | |
| console.error("Error fetching registered events:", error); | |
| res.status(500).json({ message: "Server error" }); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/controllers/eventControllers.js` around lines 24 - 43, Remove the
dead export exports.getRegisteredEvents from
backend/controllers/eventControllers.js since the route /registered-events is
wired to dashboardController.getRegisteredEvents; locate the function named
getRegisteredEvents (exports.getRegisteredEvents) in that file and delete the
entire async function block and its export to avoid duplication and confusion,
and run tests/lint to ensure no remaining references to
exports.getRegisteredEvents exist.
| if (error?.name === "CastError") { | ||
| return res.status(400).json({ message: "Invalid event ID format." }); | ||
| } | ||
| console.error("Event registration error:", error); | ||
| return res.status(500).json({ message: "Internal server error." }); | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation in catch block.
The catch block content is not properly indented relative to the function body. This appears to be a formatting error.
🧹 Proposed fix: correct indentation
} catch (error) {
- if (error?.name === "CastError") {
- return res.status(400).json({ message: "Invalid event ID format." });
- }
- console.error("Event registration error:", error);
- return res.status(500).json({ message: "Internal server error." });
-}
+ if (error?.name === "CastError") {
+ return res.status(400).json({ message: "Invalid event ID format." });
+ }
+ console.error("Event registration error:", error);
+ return res.status(500).json({ message: "Internal server error." });
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/routes/events.js` around lines 259 - 264, The catch block in the
events route has inconsistent indentation for the lines handling CastError,
console.error, and the res.status(500) return; fix it by re-indenting the entire
catch body so its statements (the if (error?.name === "CastError") check,
console.error("Event registration error:", error), and return
res.status(500).json(...)) align with the surrounding function block style used
in this file (ensure the same number of spaces/tabs as other blocks in this
handler to maintain consistent formatting).
| // Get registered events | ||
| router.get( | ||
| "/registered-events", | ||
| isAuthenticated, | ||
| dashboardController.getRegisteredEvents | ||
| ); |
There was a problem hiding this comment.
Critical: Route will never be reached due to ordering.
The /registered-events route is defined after the /:id route (line 269). Express matches routes in declaration order, so a request to /registered-events will be captured by /:id with id = "registered-events", resulting in a MongoDB CastError or unexpected behavior.
Move this route before all parameterized routes (/:id, /:eventId).
🐛 Proposed fix: move route before parameterized routes
Move the route definition to appear before line 151 (before /:eventId/is-contact), for example after line 12:
router.get("/latest", eventsController.getLatestEvents);
+// Get registered events for authenticated user
+router.get(
+ "/registered-events",
+ isAuthenticated,
+ dashboardController.getRegisteredEvents
+);
+
// Create a new event (new events can be created by admins only)
router.post(
"/create",And remove lines 498-503 from the end of the file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/routes/events.js` around lines 498 - 503, The "/registered-events"
route registration using router.get("/registered-events", isAuthenticated,
dashboardController.getRegisteredEvents) is declared after parameterized routes
(e.g., '/:id' and '/:eventId') so it will never be reached; relocate that
router.get(...) declaration to a position before any parameterized routes (for
example before the '/:eventId/is-contact' and '/:id' handlers) and remove the
duplicate declaration at the end of the file so the explicit
"/registered-events" path is matched first.
1 participant
name: "Pull Request"
about: Propose and submit changes to the project for review
title: "PR: Add endpoint for student registered events"
labels: ""
assignees: harshitap1305, sakshi1755
Changes Introduced
GET /api/students/:id/events).Why This Change?
Checklist
git checkout -b feature/student-events-endpoint).Deployment Notes
💬 Additional Notes
Summary by CodeRabbit