Skip to content

Isha taking over for Sohail - PR-Grading-Screen Frontend - #5414

Open
bfire9989 wants to merge 10 commits into
developmentfrom
pr-grading-screen-frontend
Open

bfire9989 wants to merge 10 commits into
developmentfrom
pr-grading-screen-frontend

Conversation

@bfire9989

Copy link
Copy Markdown
Contributor

Description

Implements the /pr-grading-screen route as a fully functional admin-only weekly PR grading interface. The route previously did not exist — navigating to it returned a 404. This PR registers the route, wires it to live backend data, and implements the full grading workflow including team switching, week navigation, saving, reviewer management, and automatic reviewer syncing from live user data.

Implements: PR Grading Screen feature
Related backend PR: #2286 (required — this frontend PR depends on the backend changes in #2286 to function)


Related PRs

This frontend PR is related to backend PR #2286.
To test this frontend PR you must first check out backend PR #2286.


Main Changes Explained

  • Create src/components/PRGradingScreen/index.jsx — full container rewrite: role gate (Owner/Administrator only), sync → config → grading load sequence, week selector (PST timezone), team dropdown, save/delete handlers, bootstrap logic for fresh weeks

  • Update src/components/PRGradingScreen/PRGradingScreen.jsx — replaced non-functional Done button with Save button wired to the API, added week and team dropdowns, add/remove reviewer UI, remove PR button, editable prsNeeded field, confirmation dialogs for save and delete, save status banner with dark mode support, dark mode fix for PR number input

  • Update src/components/PRGradingScreen/PRGradingScreen.module.css — added team selector, week selector, and save banner styles with full dark mode variants

  • Update src/actions/prGradingActions.js — added fetchWeeklyGrading (with weekStart param), saveWeeklyGrading, fetchPRGradingConfig, deleteWeeklyGradingReviewer, syncPRGradingReviewers

  • Update src/utils/URL.js — added PR_GRADING_SYNC_REVIEWERS and WEEKLY_GRADING_DELETE_REVIEWER endpoint constants

  • Update src/routes.jsx — registered /pr-grading-screen as a ProtectedRoute pointing to PRGradingScreenContainer

  • Update src/components/PRGradingDashboard/PRGradingDashboard.jsx — removed hardcoded TEAM_CODE = 'TeamA', wired to config API for dynamic team selection, save payload now uses teamName instead of teamCode

  • Update src/components/PRGradingScreen/mockData.js — added comment marking file as dev/testing only, not for production use

  • Create src/actions/__tests__/prGradingActions.test.js — 25 tests covering all five actions

  • Create src/components/PRGradingScreen/__tests__/PRGradingScreenContainer.test.jsx — container tests covering rendering paths, role gate, team switching, empty and error states

  • Create docs/pr-grading-screen.md — developer handoff documentation covering all changes, API reference, and outstanding items


How to Test

  1. Check out this branch and check out backend PR #2286 in the backend repo

  2. Install dependencies and start both servers:

    npm install
    npm run start:local
    
  3. Clear site data and cache in your browser (DevTools → Application → Clear Site Data)

  4. Log in as a user with Owner or Administrator role

  5. Navigate to /pr-grading-screen

  6. Verify the page loads without errors and shows:

    • A team dropdown populated with teams from GET /api/pr-grading-config
    • A week dropdown showing the current week and 3 prior weeks with date ranges (e.g. "This week (Jun 21 – Jun 27)")
    • Reviewer rows either from GET /api/weekly-grading (if data exists for this week) or bootstrapped from reviewerNames in the config (fresh week)
  7. Verify team switching: select a different team from the dropdown and confirm the reviewer rows update

  8. Verify week switching: select a prior week and confirm the data changes (or shows "No grading data for this team yet." if no records exist for that week)

  9. Verify Save:

    • Edit a prsNeeded value, add a PR number to a reviewer, change a grade in the modal
    • Click Save — confirm the confirmation dialog appears asking "Are you sure you want to save?"
    • Click Save in the dialog — confirm a green success banner appears and the data is persisted (refresh the page and verify the changes are still there)
  10. Verify Remove reviewer:

    • Click Remove on a reviewer row
    • Confirm the dialog appears: "Are you sure you want to delete reviewer X?"
    • Click Delete — confirm the reviewer is removed from the table and does not reappear on refresh
  11. Verify Add reviewer:

    • Click "+ Add Reviewer"
    • Enter a name and click Add
    • Confirm the reviewer appears in the table
    • Click Save to persist
  12. Verify role gate: log in as a non-admin user (Volunteer or Manager) and navigate to /pr-grading-screen — confirm you see "You do not have permission to access this page." and the table is not shown

  13. Verify dark mode: toggle dark mode and check:

    • PR number input field is dark (was previously white/invisible in dark mode)
    • Save success/error banner uses muted dark-mode-appropriate colors, not harsh green/red
    • All dropdowns, inputs, and modals are readable
  14. Verify error state: disconnect your backend and navigate to the page — confirm the error message "Failed to load grading data. Please try again." is shown, not a blank screen or mock data


Screenshots or Videos of Changes


Note

  • Full documentation of all changes, DB migration commands, and outstanding items is in a separate doc that can be obtained on request from the Owner.

@netlify

netlify Bot commented Jul 24, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for highestgoodnetwork-dev ready!

Name Link
🔨 Latest commit b415557
🔍 Latest deploy log https://app.netlify.com/projects/highestgoodnetwork-dev/deploys/6acd641c9c0e98d284d56df1
😎 Deploy Preview https://deploy-preview-5414--highestgoodnetwork-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@sonarqubecloud

Copy link
Copy Markdown

@one-community one-community changed the title PR-Grading-Screen Frontend Sohail - PR-Grading-Screen Frontend Jul 30, 2026
@one-community one-community added the High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible label Aug 1, 2026
@one-community one-community changed the title Sohail - PR-Grading-Screen Frontend Rithika taking over for Sohail - PR-Grading-Screen Frontend Aug 1, 2026
@iAbhi001
iAbhi001 self-requested a review August 2, 2026 02:10

@iAbhi001 iAbhi001 left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review & Local Testing Update

I pulled down frontend branch pr-grading-screen-frontend and backend branch pr-grading-screen-backend to test the changes locally.

When running the frontend test suite (npm test), I encountered 17 failing tests across 3 test files:

Test Files  3 failed | 314 passed | 1 skipped (318)
Tests       17 failed | 2123 passed | 14 skipped (2154)
Image Image

@sawantgayatri19 sawantgayatri19 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes to Acknowledge:

Implementation of the /pr-grading-screen route as an admin-only interface.

Integration with live backend data, including team switching, week selection (PST timezone), and saving functionality.

Addition of new unit test files (prGradingActions.test.js, PRGradingScreenContainer.test.jsx) and documentation (pr-grading-screen.md).

Action Items / Feedback to Include:

Note the current test suite failures (17 tests failing across 3 test files during local execution).

Remind the author that backend PR #2286 must be checked out and functioning alongside this frontend branch for proper testing.

@sawantgayatri19 sawantgayatri19 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed this PR along with the full history of feedback and current test results. Summarizing where things stand so we can get this closed out:

Blocking issues (need to be resolved before this can merge):

Unit and Integration Test Failures: Running the test suite currently results in 17 failing tests across 3 test files during local execution. Additionally, Netlify deploy previews and PR unit/React app test checks are failing. These must be investigated and resolved.

Backend Dependency Alignment: As noted in the PR description, this frontend branch requires backend PR #2286 (pr-grading-screen-backend) to be properly checked out and functioning for all API interactions. Ensure local environments are fully synced between both repositories.

What looks solid:

The implementation of the /pr-grading-screen route, role-based access gating (Owner/Administrator only), week/team dropdown logic, and the UI/dark mode styling updates all look clean and well-structured.

The added unit test files (prGradingActions.test.js, PRGradingScreenContainer.test.jsx) and developer handoff documentation (pr-grading-screen.md) are thorough.

Once the test suite passes cleanly and all deployment/integration checks succeed, this should be ready for a final round.

@sitaram1921 sitaram1921 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Rithika,

I reviewed this against development and exercised the endpoints locally. The core diagnosis in your description is right, the teamCode / teamName mismatch really was why the screen fell back to mock data, and the write-up is clear. A few things need sorting before this can go in.

Note:
I could not follow your test steps end to end: the paired frontend #5414 does not compile. routes.jsx ends up importing PRGradingDashboard twice (the branch adds it at line 162 while development already has it at line 200), which is a Babel parse error in the app router, so no page loads. That is a #5414 issue and I will raise it there, but it does mean the grading screen itself is untested.

Image

@RuthwikHM RuthwikHM left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was unable to get the branch working at all since I faced this issue.

Image I got rid of the redundant import but the page itself did not load after that too. Image Image

@one-community one-community changed the title Rithika taking over for Sohail - PR-Grading-Screen Frontend Isha taking over for Sohail - PR-Grading-Screen Frontend Oct 7, 2026

@vidiyala99 vidiyala99 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested fa2c5ee with backend OneCommunityGlobal/HGNRest#2286 (0f1c128) as Administrator on /pr-grading-screen, in dark mode. All writes were blocked in the browser, so nothing was saved to the shared dev database. The PR's suites pass (PRGradingScreen + prGradingActions, 2 files, 37/37).

  1. The week is wrong for anyone outside Pacific time. I loaded the page with the browser's time zone set three ways (same day, Oct 9):
Browser time zone Week value sent Label Header
America/Los_Angeles 2026-10-04 This week (Oct 4 – Oct 10) Oct 4, 2026 to Oct 10, 2026
America/New_York 2026-10-04 This week (Oct 3 – Oct 9) Oct 3 to Oct 9
Asia/Kolkata 2026-10-03 (a Saturday) This week (Oct 2 – Oct 7) Oct 2 to Oct 8

(screenshot 1 is India, screenshot 2 is Pacific). In India the week start itself is wrong, and the code passes it to fetchWeeklyGrading(name, weekStart) and the save, so a grader there would read and write grades under 2026-10-03, a week the Pacific admins never see. In New York only the labels are off, but they still name the wrong days.
Cause: getPSTDate() builds new Date(pst + 'T00:00:00'), which is midnight in the browser's zone, and getSunday/getSaturday then call toISOString(), which is UTC, so any zone east of UTC rolls back a day. formatShort and buildTeamData parse the same local midnight and format it in Pacific time, which shifts the labels for every zone east of Pacific. Doing the date math on the YYYY-MM-DD string in UTC (Date.UTC(y, m - 1, d), getUTCDay(), and formatting with timeZone: 'UTC') would make it independent of where the admin is. Many of our graders are outside Pacific time, so this matters.

  1. Opening the page writes. Every load sends POST /api/pr-grading-config/sync-reviewers before anything is read, so just viewing the screen changes the config. If the sync fails, the page shows "No grading data for this team yet." instead of an error, which hides the problem. Running the sync from an explicit action (or only when the reviewer list is out of date), and showing an error state when it fails, would be safer.

  2. Coordination: #5493 and #5630 also change PRGradingScreen.jsx, so whichever merges last needs a careful merge.

Requesting changes for item 1.

Screenshot 1: browser in Asia/Kolkata, week shows Oct 2 to Oct 7/8

Image

Screenshot 2: browser in America/Los_Angeles, Oct 4 to Oct 10

Image

@sonarqubecloud

Copy link
Copy Markdown

@Isha2303165

Copy link
Copy Markdown
Contributor

Hi @vidiyala99 . I have made changes according to your comment. Could you please review it once again?

@vidiyala99 vidiyala99 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-tested b415557 with backend OneCommunityGlobal/HGNRest#2286 (15b5d40) as Administrator, in dark and light mode. Writes were blocked in the browser, so nothing was saved to the shared dev database. PRGradingScreen and action suites pass (31 files, 265 tests), ESLint and stylelint clean.

Fixed: the week is now the same everywhere. With the browser set to America/Los_Angeles, America/New_York and Asia/Kolkata (where it is already Tuesday), all three send weekStart=2026-10-11 and show "This week (Oct 11 – Oct 17)". The new unit test also fails against the old code under UTC, so CI will catch a regression. Thank you for this.

New blocker: the Grade PR modal shows two sets of checkboxes. Each row now has 8 checkboxes under a 5-column header (screenshot 2). The 4 hardcoded cells at PRGradingScreen.jsx lines ~662-695 (Exceptional, Okay, Unsatisfactory, 'No Correct Image') were kept alongside the GRADE_OPTIONS.map cells that came in from development. On development the modal only has the GRADE_OPTIONS cells. They also disagree: ticking "Cannot find image" in the inline summary checks only the extra column. Removing the 4 hardcoded cells should fix it. This looks like a leftover from the merge in 1ec3a32.

Non-blocking notes:

  • "Cannot find image" saves as 'Cannot find image' (from gradeOptions.js, already on development via #5493), but the backend only accepts 'No Correct Image', so saving that grade fails. Not caused by this PR, but this screen is where it breaks; worth aligning with the backend.
  • The page runs the reviewer sync and grading requests before the role check, so a Volunteer opening the URL still sends them before seeing "You do not have permission". Skipping the load when not authorized would avoid that.
  • At 390px the table headers break mid-word and the search box runs into "+ Add Reviewer".

Requesting changes for the modal.

Screenshot 1: Asia/Kolkata, week Oct 11 to Oct 17 (dark)

Image

Screenshot 2: Grade PR modal, 8 checkboxes per row under 5 headers

Image

Screenshot 3: light mode, same week

Image

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants