Repository navigation
Priyanka - Add CSV export to weekly PR grading screen - #5613
muthapriyanka wants to merge 10 commits into
Conversation
✅ Deploy Preview for highestgoodnetwork-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
manavkheni1
left a comment
manavkheni1
left a comment
There was a problem hiding this comment.
Tested locally using the built-in test harness (/pr-grading-test, "91NePRT" configuration, 8 reviewers). Verified the exported CSV matches the on-screen table exactly — reviewer names, PR reviewed counts, PRs needed, PR number formatting (including "+" grouped entries), and blank fields for reviewers with no PRs reviewed. Filename correctly includes the date range. Confirmed search filter and role filter each correctly limit the export to only the visible/matching rows, and a newly added PR is included in the export even before clicking "Done." No issues found.
vidiyala99
left a comment
vidiyala99
left a comment
There was a problem hiding this comment.
Tested at a8af74c locally (frontend on this branch, backend on development) as Admin in light and dark mode. The PR's tests pass (csvExport.test.js + PRGradingScreen.export.test.jsx: 2 files, 4 tests).
Working:
- Export works and matches the table. On
/pr-grading-testwith the "ComplexTeam" configuration, Export to CSV downloadedweekly-pr-grading-6-15-2025-to-6-22-2025.csv:
"Reviewer Name","PR Reviewed Count","PRs Needed","PR Numbers","Notes"
"David Williams-Johnson","3","20","1000 + 2000; 3000; 4000 + 5000",""
"Emma","0","0","",""
"Frank","1","50","9000",""
"Grace","4","5","10000 + 11000; 12000; 13000 + 14000; 15000",""
Grouped PRs keep their "+", the reviewer with no PRs gets empty fields, and the filename uses the date range shown on screen. /pr-grading-screen exports its 91NePRT rows the same way.
2. CSV safety: values are quoted, quotes are doubled, cells starting with =, +, - or @ get a leading ', and the BOM keeps Excel on UTF-8.
3. Dark mode: the button is readable next to Done (screenshot 2).
Blocking:
- Merge conflicts with
development. GitHub shows the branch as not mergeable, and a local merge oforigin/developmentconflicts inPRGradingScreen.jsxandPRGradingScreen.module.css. #5491 (promote buttons under each reviewer) merged intodevelopmenttoday and rewrote both files. Please mergedevelopmentand check the Export button still sits correctly next to Done and 5491's promote buttons.
Question:
2. Which screen does task 34 mean? The button only appears where PRGradingScreen renders: /pr-grading-screen and /pr-grading-test, which both show hardcoded sample data (91NePRT, 5/18/2025), and the "Review for This Week" dialog that #5574 adds. The grading dashboard that shows the current review week (Oct 12 - Oct 18), /pr-grading-dashboard (PRGradingDashboard), has no Export button (screenshot 3). If coordinators are meant to export that weekly data, it may need to go there too. Worth confirming with Jae.
Minor:
URL.revokeObjectURL(url)runs right afterlink.click(). Chrome is fine with that, but some browsers cancel the download if the URL is revoked in the same tick; wrapping it insetTimeout(..., 0)is the usual safe pattern.
Requesting changes for the merge conflict.
1. Admin, light: /pr-grading-test (ComplexTeam) with Export to CSV next to Done
2. Admin, dark: same screen, button readable
3. Admin, light: /pr-grading-dashboard (current review week) has no Export button
muthapriyanka
added
the
High Priority - Please Review First
AaditTrivedi
left a comment
AaditTrivedi
left a comment
There was a problem hiding this comment.
Reviewed by Aadit Trivedi.
Checked out the PR (no conflicts with development) and ran csvExport.test.js and PRGradingScreen.export.test.jsx: [5/5] passing.
Verified
- Restores the Export to CSV button on the weekly PR grading screen, which an earlier merge (#5056) removed.
- Every value is quoted with embedded quotes doubled, so commas, quotes, or line breaks in names and notes can't break the CSV columns.
- Values pass through
protectSpreadsheetValue, which guards against spreadsheet formula injection (cells starting with characters like=or+). - It exports the reviewers currently shown (
filteredReviewers), joins multiple PR numbers with "; ", uses CRLF line endings for spreadsheet compatibility, and names the file by date range (weekly-pr-grading-<start>-to-<end>.csv).
Approving.
rahulrubugunday
left a comment
rahulrubugunday
left a comment
There was a problem hiding this comment.
Tested locally on Priyanka_weekly_pr_grading_csv at fb9507d as DevAdmin, with backend on development, in light and dark mode. No conflicts with development.
- Export to CSV shows up next to Done on the weekly PR grading screen (91NePRT test team).
- Clicking it downloads weekly-pr-grading-5-18-2025-to-5-25-2025.csv with one row per reviewer: name, PR reviewed count, PRs needed and PR numbers all match what's on screen.
Needs changes:
- The grades aren't in the export. Abi's PRs are graded Exceptional, Okay, Exceptional and Unsatisfactory on screen, but the CSV only has the PR numbers. The earlier export from #4934 had a Grades column, so it would be good to add that back.
- The grading screen doesn't work well in dark mode. The table headers (Reviewer Name, PR reviewed, PRs Needed, PR Numbers) and the grade table headers (PR Number, Exceptional, Okay, Unsatisfactory, Cannot find image) are light text on a white background, so they can't be read. The PR number chips next to each reviewer are also hard to read.
Requesting changes.
AaditTrivedi
left a comment
AaditTrivedi
left a comment
There was a problem hiding this comment.
Re-reviewed by Aadit Trivedi.
After the new commits "Defer CSV download URL cleanup" and "Include PR grades in CSV and fix grading dark mode": ran the CSV tests (6/6 passing). The new Grades column follows the same order as PR Numbers (joined with "; ") and goes through the same quoting and formula-injection protection as the other columns. Deferring URL.revokeObjectURL until after the click avoids browsers canceling the download. Everything from my earlier approval still holds.
Approving.
AlekyaJogini
left a comment
AlekyaJogini
left a comment
There was a problem hiding this comment.
Reviewed by Alekya.
How I tested
Checked out the PR branch (commit 7d5a960) on Node 20.19.4 with the backend on development. Ran npx vitest run csvExport PRGradingScreen.export: 2 test files, 6 tests passed. Logged in as my Admin test account and opened the Weekly PR grading screen (91NePRT, 5/18/2025 to 5/25/2025) in dark mode. I only exported CSVs and did not add PRs or click Done.
Verified working
-
The Grades column is now in the CSV . Abi's grades match the screen: 3100 + 1410 Exceptional, 2045 Okay, 1876 + 2234 Exceptional, 3301 Unsatisfactory. The other reviewers have grades too, and Cari (0 PRs) has empty fields. The file name uses the date range (weekly-pr-grading-5-18-2025-to-5-25-2025.csv).
-
Dark mode is fixed the table headers, the grade table headers and the PR number chips are readable.
- Search filter: searching "abi" and exporting gives a CSV with only Abi's row.
- With no matching reviewers ("No reviewers found") the export downloads a valid file with only the header row.
Issues found
None
Not tested
- Role filter
- Adding a PR and exporting before Done
iAbhi001
left a comment
iAbhi001
left a comment
There was a problem hiding this comment.
Tested locally on branch Priyanka_weekly_pr_grading_csv at commit 7d5a960 with backend on development as Admin across both light and dark mode, and confirmed all merge conflicts with development are resolved. All 6 tests across both test files pass cleanly. The Export to CSV button renders properly next to Done, the deferred object URL cleanup prevents download cancellation, and the exported file accurately reflects the on-screen date range and reviewer data. The newly restored Grades column correctly aligns with the PR entries, spreadsheet formula injection protections and value escaping are functioning as expected, search filtering limits the rows properly, and the earlier dark mode contrast issues on the headers and chips are fixed. Approving!
443413b
vidiyala99
left a comment
vidiyala99
left a comment
There was a problem hiding this comment.
Re-reviewed at 443413b (my earlier changes-requested review is on a8af74c).
Tested both screens in a browser in light and dark mode. Each component was rendered on its own with a mocked store, and the grading API was mocked, so nothing touched a real database. The vitest suites for PRGradingScreen/ and PRGradingDashboard/ pass (4 files, 19 tests).
Fixed since my last review:
- The merge conflict is gone. GitHub reports the PR as mergeable.
- The download URL is revoked in a
setTimeout. - Export to CSV was added to the Team Alpha dashboard, which answers my question about task 34. The tooltip says the Review Week selector does not filter the records yet, which matches what the screen does.
- The CSV now has a Grades column.
Downloaded files:
- Screen (
weekly-pr-grading-10-12-2026-to-10-18-2026.csv):"Abi","2","10","3100 + 1410; 2200","Okay; Exceptional","","Ben =cmd","1","10","4100","Unsatisfactory",""and"Cleo","0","5","","","". Searching for "Abi" exports only Abi's row. - Dashboard (
team-alpha-pr-grading-displayed-data.csv):"Alice","7","10","1070 + 1256; 2000","Exceptional; Okay",""and"Bob","2","8","3000","No Correct Image","". The count comes from the storedprsReviewed, as the code intends.
Blocking (dark mode):
- "PRs Needed" is unreadable in dark mode on the weekly screen. The numbers in that column (10 for Abi and Ben in screenshot 2) are
rgb(51, 51, 51)onrgb(27, 42, 65).dmis nowstyles['dark-mode'], so the dark rules now apply, but.pr-grading-screen-td-neededhas no dark-mode color. Please add one for the dark class (the other cells in that row already switch to light text). The Export to CSV and Done buttons are readable; the dashboard in dark mode (screenshot 4) is fine.
Heads up for merge order: this branch and #5630 both edit PRGradingScreen.jsx and PRGradingView.jsx, and a merge of the two conflicts in both files. Whichever lands second will need to resolve it.
Non-blocking: the dashboard CSV shows the stored grade name "No Correct Image" while the weekly screen labels it "Cannot find image". Fine for a data export, just worth knowing.
1. Weekly screen, light: Export to CSV next to Done
2. Weekly screen, dark: the "PRs Needed" numbers (10) are barely visible
3. Dashboard, light
4. Dashboard, dark
5. Dashboard, phone width
iAbhi001
left a comment
iAbhi001
left a comment
There was a problem hiding this comment.
Re-tested locally on latest commit in both light and dark modes across the weekly PR grading screen and the Team Alpha PR grading dashboard.
The latest fix for the PRs Needed column contrast in dark mode works as expected, and the numbers are now clearly legible against the dark background. The Export to CSV button on both the weekly grading screen and the Team Alpha dashboard downloads cleanly with matching visible rows, properly aligned Grades columns, escaped values, formula injection protections, and deferred URL cleanup. All unit tests across PRGradingScreen and PRGradingDashboard continue to pass without conflicts with development.
Everything looks solid and functions properly. Approving the PR.
AnshShah-18
left a comment
AnshShah-18
left a comment
There was a problem hiding this comment.
I've tested PR #5613 locally and everything I checked is working correctly. The Export to CSV button downloads the weekly PR grading data with the correct date-range filename, expected columns, reviewer information, PR numbers, and grades. I also verified that reviewer search and role filters are reflected correctly in the exported CSV, with only the visible filtered rows included. Everything I tested worked as expected.
Approved—nice work!
one-community
changed the title
2006d9e
|
Description
The Weekly PR grading screen did not have an Export to CSV option. Coordinators had to copy reviewer data manually to use it outside the application. This PR adds a client-side CSV download of the reviewers currently visible on the screen.
The export includes reviewer name, PR reviewed count, PRs needed, PR numbers, and notes when available. Its filename includes the date range displayed on the page. Search and role filters also apply to the exported rows.
Implements the Weekly PR grading screen Export to CSV task:
Related PRs (if any)
Main changes explained
csvExport.jsto build and download the CSV, escape CSV values, protect spreadsheet-formula-like values, and create a date-range filename.How to test
npm installif needed.npm run start:local./pr-grading-test, then select a original to open the Weekly PR grading screen.Video of changes
Export_CSV.mp4