[Adit0717]

Description

Related PRS (if any):

This backend PR is related to the #5596 frontend PR.
…

Main changes explained:

1. Added deleteWeeklyGrading function in weeklyGradingController.js - validates team, date, and reviewer query params, finds the matching document in weeklyGradings using a day-range date filter (same pattern as getWeeklyGrading),
   and deletes it via findOneAndDelete. Returns 404 if no matching record is found.
2. Registered DELETE /weekly-grading route in weeklyGradingRouter.js
   …

How to test:

1. check into current branch

2. do npm install and run the app

3. Navigate to /pr-grading-dashboard and note an existing reviewer name (e.g. "test_reviewer_name"), or add a new reviewer from the same screen and use that name. The current week's Monday date is 2026-09-28.

4. In Postman, send the DELETE to remove that reviewer:
   DELETE http://localhost:4500/api/weekly-grading?team=TeamA&date=2026-09-28&reviewer=test_reviewer_name
   → Expected: { "message": "Reviewer "test_reviewer_name" removed successfully" }

Note: If testing against a past week's record, replace the date with that week's Monday (e.g. 2026-09-22 for last week). The date must match exactly how it was stored - always a Monday - otherwise the record will not be found and a 404 is returned.

5. Send the GET to confirm the reviewer is gone:
   GET http://localhost:4500/api/weekly-grading?team=TeamA&date=2026-09-28
   → Expected: empty array [] (or remaining reviewers if others exist)

6. Send the DELETE again for the same reviewer:
   DELETE http://localhost:4500/api/weekly-grading?team=TeamA&date=2026-09-28&reviewer=test_reviewer_name
   → Expected: 404 { "error": "Grading record not found" }

7. Refresh /pr-grading-dashboard - the removed reviewer should not reappear.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[manavkheni1]

Tested all four scenarios from the test plan directly via curl (created test data via the /weekly-grading/save endpoint since the PR's example date had no existing records in the dev DB). Confirmed: GET shows the record before deletion, DELETE returns the expected success message and removes the record, a follow-up GET returns an empty array confirming removal, and repeating the DELETE correctly returns a 404 with "Grading record not found". The endpoint behaves exactly as described.

[vidiyala99]

Tested 2a78837 by calling the controller against an in-memory MongoDB: saved two PRs for reviewer "Alice" (team TeamA, 2026-09-28) through saveWeeklyGrading, then called deleteWeeklyGrading with a Volunteer requestor:

before delete: prs 2, versionHistory 1
delete as Volunteer -> 200 {"message":"Reviewer \"Alice\" removed successfully"}
docs left 0

The happy path works as described (and matches the earlier tester's results), but:

1. Anyone who is logged in can delete any reviewer's record. There's no permission check on the route or in the controller, so a Volunteer removed Alice's whole record. findOneAndDelete also drops the versionHistory that saveWeeklyGrading keeps, so the grades can't be recovered. Please gate this with hasPermission(req.body.requestor, '<a grading permission>') (a 403 otherwise). Since save has no check either, it may be worth adding the same one there.

2. Coordinate with #2286, which renames teamCode to teamName on this model and controller. This query filters on teamCode, so whichever merges second needs updating, or the delete will always return 404.

3. Minor:

   • No tests were added for the new handler; the four cases from the test plan (success, 404, missing params, invalid date) would make a good weeklyGradingController.test.js (there are no tests for this controller on this branch yet).
   • The router change adds an extra blank line, which Prettier will flag.

Requesting changes for item 1.

[AaditTrivedi]

Reviewed by Aadit Trivedi.

Reviewed the new deleteWeeklyGrading handler and route (DELETE /weekly-grading). The earlier review points are still unresolved, and there's one more:

1. No permission check: any authenticated user, including a Volunteer, can delete a reviewer's weekly grading record. Please check a PR Dashboard edit permission with hasPermission before deleting.
2. The delete is permanent (findOneAndDelete), while the task asked for a soft delete. Marking the record as removed (for example isDeleted: true with who removed it and when) would allow recovery.
3. The day window uses setHours(0, 0, 0, 0) in the server's time zone; on a UTC server that doesn't match a Pacific day, so a record near midnight Pacific may not be found. Using the company time zone (moment.tz(date, 'America/Los_Angeles').startOf('day')) would fix that.
4. It filters on teamCode, which BE #2286 renames to teamName, so it will break if #2286 merges first.

Input validation (required params, string types, invalid dates) and the 404 for a missing record are good.

Required action
Please add a permission check, switch to a soft delete, and use the company time zone for the day range. Happy to re-review once updated.

[rahulrubugunday]

Tested locally on Aditya_fix_bug_remove-reviewer-weekly-grading at 2a78837 as DevAdmin, with frontend on #5596.

• DELETE /weekly-grading with team, date and reviewer returns 200 and the reviewer no longer shows for that week.

Needs changes:

1. The delete is permanent. After removing a test reviewer, the record was gone from GET /weekly-grading?team=TeamA entirely, including its version history. The task asked for a soft delete, and the frontend modal tells the user their history stays visible.

Requesting changes.

[Adit0717]

1. Permission check has been added to the deleteWeeklyGrading endpoint - a requestor is now required in the request body, and we verify accessPRTeamDashboard before allowing the delete. A 403 is returned for unauthorized users. So a volunteer cannot access /pr-grading-dashboard and make changes to it.

2. Soft delete - Keeping hard delete intentionally. The product owner clarified this feature is specifically for removing accidentally added reviewers, so there's no meaningful history to preserve. findOneAndDelete is appropriate here.
   Refer to this document. - Page 77 Title - PR Review Team Analytics Dashboard - Weekly PR Grading page Remove/Delete Reviewer. Checkout the comments provided.

3. Timezone - Keeping UTC-based day range. Dates are always stored from frontend-generated ISO date strings (toISOString().split('T')[0]), which parse as UTC midnight. The UTC range matches those records.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
93.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[AaditTrivedi]

Re-reviewed by Aadit Trivedi.

Ran weeklyGradingController.test.js and weeklyGradingRouter.test.js: [17/17] passing.

Fixed since my review

• The delete now requires hasPermission(requestor, 'accessPRTeamDashboard'), matching the permission the frontend in #5596 checks, so a Volunteer without it can't remove a reviewer. The tests cover both allowed and denied requests.

Pointers, not blocking

1. The delete is still permanent (findOneAndDelete). The status report asked whether a permanent delete is acceptable or a soft delete is required; if a soft delete is wanted, it should change before merge.
2. The day window still uses setHours in the server's time zone, so on a UTC server a record dated near midnight Pacific may not be found. Using moment.tz(date, 'America/Los_Angeles').startOf('day') would align it with the rest of the app.
3. The query still filters on teamCode, which BE #2286 renames to teamName; whichever merges second will need updating.

Approving, together with frontend #5596.

[vidiyala99]

Re-reviewed at 4562026 (my earlier changes-requested review was on 2a78837).

Tested the controller against a throwaway local MongoDB (127.0.0.1, not the shared database) with real Role and UserProfile documents, so hasPermission ran for real. Data: Alice in the week of 2026-10-19, Alice in the week of 2026-10-12, Bob in the week of 2026-10-19. The jest suites for prAnalytics pass (2 suites, 17 tests).

Results for deleteWeeklyGrading:

• No requestor: 400 "requestor is required".
• Volunteer: 403 "You are not authorized to remove a reviewer." and nothing is deleted (3 documents before and after). This was the blocker in my first review and it is fixed.
• Administrator (has accessPRTeamDashboard), Alice for the week of 2026-10-19: 200, 2 documents left. Alice's record for the week of 2026-10-12 and Bob's record are still there.
• The same delete again: 404 "Grading record not found".
• Reviewer sent as { $ne: 'x' }: 400 "Invalid query parameters", nothing deleted. A bad date gives 400.
• GET for 2026-10-19 returns only Bob and GET for 2026-10-12 returns only Alice, so the week filter and the de-duplication behave as the frontend PR expects. The auth middleware fills req.body.requestor from the JWT, so the check cannot be skipped by a client.

Remaining, none blocking:

1. POST /weekly-grading/save still has no permission check. As a Volunteer I could create a record for a new reviewer (200 and the document was written). That was already true before this PR, so it does not hold this one up, but it is worth a follow-up.
2. The delete window is date through date + 6 days, so it only finds the record when the caller sends the Monday that was used to save it. The frontend does that today. A caller sending a mid-week date would get a 404.
3. This and #2286 both change the weekly-grading model and controller (teamCode and teamName), so whichever lands second needs to be reconciled.

Approving. The frontend pair is HighestGoodNetworkApp#5596.

[one-community]

Thank you all, merging!

Fixed

[RohSrik]

I tested this PR locally along with its related frontend PR #5596. I added a test reviewer through the Weekly PR Grading Dashboard and confirmed that the record was saved and remained visible after refreshing the page. Using Postman, I verified that the GET endpoint returned the reviewer, the DELETE endpoint removed the reviewer with a 200 response, and a subsequent GET returned an empty array. I also confirmed that deleting the same reviewer again returned 404 with the expected error message. After refreshing the dashboard, the deleted reviewer did not reappear. I checked the previous week's records and confirmed that the existing reviewer and graded PRs were still visible. The functionality I tested worked as expected.