#143 Added remove BR feature

[PrinceK-Git]

No description provided.

[RsbhThakur]

Nice Work!
While testing in local, I noticed that while adding a BR causes the frontend spinner to freeze for about 30-40 seconds. This happens because await fetchCoursesForBr(user.rollNumber) at line 29 and 68 in server/modules/br/br.controller.js scrapes the academic portal synchronously and blocks the HTTP response from returning.
Could you make this non-blocking by removing the await and adding a catch block? It will make the UI feel instant again while the courses fetch quietly in the background.
Suggested change in both createBR and updateBRs:

fetchCoursesForBr(user.rollNumber).catch((err) => logger.error(err));

Tested in local by deleting the br - the br access was removed from the client, everything looks good to me, the pr is clean and self contained and working. just fix the two improvements as mentioned in this review.

[RsbhThakur]

@PrinceK-Git Rebase your PR on current dev, and implement the changes in the new students tab. also remove the old BR tab.

[PrinceK-Git]

If student is a br and someone removes the student using trash icon , the student gets deleted from user database but it's still sitting as a br in "BR" database , so to solve this problem i fixed handleRowDelete . Now if someone removes a student using trash icon , the student is removed from both the databases "user" and "BR" .

[RsbhThakur]

Detailed Findings & Code Review

1. UI/UX Hang on Delete (Students.jsx)

• Vulnerability/Bug: In admin/src/pages/Students.jsx, the handleRowDelete function updates rowLoadingId on click, but does not use a finally block to clear the loading state:

  const handleRowDelete = async (id) => {
      setRowLoadingId(id);
      try {
          const student = students.find(stud => stud._id===id);
          await deleteStudent(id);
          if(student && student.isBR) {
              await deleteBR(student.email);
          }
          loadStudents();
      } catch (err) {
          setError(err.message || "Failed to delete student.");
      }
  };

• Risk:
  • Failed Delete: If a student deletion fails for any reason, the spinner will spin indefinitely and the confirmation modal will remain open on screen, giving no exit path for the administrator.
  • Successful Delete: Even if it succeeds, the component state variables (rowConfirm and rowLoadingId) are not reset, leaving unnecessary reference values in memory.
• Fix: Wrap the reset logic in a finally block, matching the implementation of handleRowRefresh and handleRemoveBR:

  } finally {
      setRowLoadingId(null);
      setRowConfirm(null);
  }

---

2. Cascading Delete Client Chaining (Architectural Anti-Pattern)

• Design Issue: When deleting a student who is a BR, the client performs two chained HTTP calls:

  await deleteStudent(id);
  if(student && student.isBR) {
      await deleteBR(student.email);
  }

• Risk:
  • Inconsistent Database State: If deleteStudent succeeds but deleteBR fails (e.g., network timeout, browser crash, client termination), the student is successfully removed from the User collection, but their email is permanently orphaned in the BR database model.
• Fix: Move the cascading delete to the backend. The backend deleteStudent controller in server/modules/student/student.controller.js should check if the student is a BR and clean up the BR model atomically. The frontend should only need to fire a single deleteStudent request.

---

3. Unprotected BR API Endpoints (Security Recommendation)

• Security Vulnerability: In server/modules/br/br.routes.js, all endpoints (such as /updateList, /create, /delete) are entirely public and lack any authorization or authentication middleware:

  router.post("/updateList", updateBRs);
  router.post("/create", createBR);
  router.delete("/delete", deleteBR);

• Risk: Anyone can send raw API calls to these routes and arbitrarily grant or remove BR privileges for any student without needing admin credentials.
• Fix: Protect these routes using the backend isAdmin middleware.

[RsbhThakur]

Remove these dead code comments