Vasilios Nicholas Peer Review for Final Project

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] },
+                  ],
                 },
               },
             },

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) {

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 (
     <Container className="px-5">
+      {/*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.*/}
       <title>Event Management · ClubSync</title>
       <meta
         name="description"
@@ -17,10 +18,8 @@ export default function AdminEventManagement() {
       <p className="spacing-after-moto">
         Create events and manage RSVPs for the active semester.
       </p>
-
       {/* creating an event bumps refreshKey, which remounts the list below */}
       <EventForm onCreated={() => setRefreshKey((k) => k + 1)} />
-
       <div className="mt-4">
         <EventList key={refreshKey} basePath="/admin/event-management" />
       </div>

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();
 

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 " });