From ba06573e17013dcb3fe47f4e9aae8f9d45920284 Mon Sep 17 00:00:00 2001 From: Vasilios Nicholas Date: Mon, 17 Aug 2026 14:07:32 -0400 Subject: [PATCH 1/5] Add praise for middlware factory function for authorization --- middleware/auth.js | 1 + 1 file changed, 1 insertion(+) diff --git a/middleware/auth.js b/middleware/auth.js index 478022a..8d9a509 100644 --- a/middleware/auth.js +++ b/middleware/auth.js @@ -16,6 +16,7 @@ const ROLE_RANK = { member: 1, treasurer: 2, admin: 3 }; // minRole via closure export const requireRole = (minRole) => { + //Nice middleware factory pattern. I did basically the same thing apart from returning the Authorization middleware as the second element in an array where the first element is the Authentication middleware so I could rest the array reprsenting a middleware stack onto routes for our routers. return (req, res, next) => { if (!req.isAuthenticated()) { return res.status(401).json({ message: "Not authenticated " }); From 818f6cdf79f5e59f52e82882bd15794a6eaa736d Mon Sep 17 00:00:00 2001 From: Vasilios Nicholas Date: Mon, 17 Aug 2026 14:26:16 -0400 Subject: [PATCH 2/5] Add question about error handling --- db/users-db.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/db/users-db.js b/db/users-db.js index fef4339..89a768e 100644 --- a/db/users-db.js +++ b/db/users-db.js @@ -53,6 +53,7 @@ function UsersCollection({ collectionName = "users" } = {}) { console.log("Registered new User in MongoDB ๐Ÿ“"); return { id: result.insertedId, email, firstName, lastName }; } catch (error) { + //If all you are going to do is print the error to the console than throw it again, why not just do this upstream (block where you are invoking this method)? console.error("Error registering new User", error); throw error; } @@ -197,7 +198,10 @@ function UsersCollection({ collectionName = "users" } = {}) { // amount (even $0), everyone else pays their tier price. amount: { $sum: { - $ifNull: ["$discount.amount", { $ifNull: ["$duesAmount", 0] }], + $ifNull: [ + "$discount.amount", + { $ifNull: ["$duesAmount", 0] }, + ], }, }, }, From bc869c6c64eac444dd44efdd5141d35d5f4cf5cc Mon Sep 17 00:00:00 2001 From: Vasilios Nicholas Date: Mon, 17 Aug 2026 14:40:20 -0400 Subject: [PATCH 3/5] Added comment about moving authorization logic to backend --- frontend/src/pages/auth/ProtectedRoute.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/pages/auth/ProtectedRoute.jsx b/frontend/src/pages/auth/ProtectedRoute.jsx index 853a98b..6c65a52 100644 --- a/frontend/src/pages/auth/ProtectedRoute.jsx +++ b/frontend/src/pages/auth/ProtectedRoute.jsx @@ -1,7 +1,7 @@ import { Navigate, Outlet } from "react-router"; import { useUser } from "../../context/UserContext.jsx"; import PropTypes from "prop-types"; - +//this should be handled on the backend via middleware export default function ProtectedRoute({ allow }) { const { user, loading } = useUser(); From 987526af6eabf0742266a14e0628402a61b736c1 Mon Sep 17 00:00:00 2001 From: Vasilios Nicholas Date: Mon, 17 Aug 2026 14:43:05 -0400 Subject: [PATCH 4/5] Add praise for use of useContext hook for authentication --- frontend/src/context/UserContext.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/context/UserContext.jsx b/frontend/src/context/UserContext.jsx index 2c760d3..330a515 100644 --- a/frontend/src/context/UserContext.jsx +++ b/frontend/src/context/UserContext.jsx @@ -2,7 +2,7 @@ import { useContext, createContext } from "react"; // The context now carries both the user and a setter so components (login, // logout) can update the logged-in user without a full page reload. -export const UserContext = createContext({ user: null, setUser: () => {} }); +export const UserContext = createContext({ user: null, setUser: () => {} }); //nice use of createContext & useContext! export const useUser = () => { const context = useContext(UserContext); if (context === undefined) { From 2716d7610e66c832b538019bf894baf85eec137d Mon Sep 17 00:00:00 2001 From: Vasilios Nicholas Date: Mon, 17 Aug 2026 15:21:48 -0400 Subject: [PATCH 5/5] semantic html - title and meta elements are not permitted outside of head element + instructions to fix --- .../admin/admin-event-management/AdminEventManagement.jsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frontend/src/pages/admin/admin-event-management/AdminEventManagement.jsx b/frontend/src/pages/admin/admin-event-management/AdminEventManagement.jsx index 39f3939..4f0709c 100644 --- a/frontend/src/pages/admin/admin-event-management/AdminEventManagement.jsx +++ b/frontend/src/pages/admin/admin-event-management/AdminEventManagement.jsx @@ -2,11 +2,12 @@ import { useState } from "react"; import Container from "react-bootstrap/Container"; import EventForm from "../../events/event-form/EventForm.jsx"; import EventList from "../../events/event-list/EventList.jsx"; - +//unused component? export default function AdminEventManagement() { const [refreshKey, setRefreshKey] = useState(0); return ( + {/*title and meta elements should be enclosed within the head element. You could use a useEffect hook to manipulate the DOM to change the content of the head element.*/} Event Management ยท ClubSync Create events and manage RSVPs for the active semester.

- {/* creating an event bumps refreshKey, which remounts the list below */} setRefreshKey((k) => k + 1)} /> -