diff --git a/src/components/CommunityPortal/Activities/__tests__/ActivityList.test.jsx b/src/components/CommunityPortal/Activities/__tests__/ActivityList.test.jsx index d5751c59432..f0b1639a039 100644 --- a/src/components/CommunityPortal/Activities/__tests__/ActivityList.test.jsx +++ b/src/components/CommunityPortal/Activities/__tests__/ActivityList.test.jsx @@ -57,6 +57,7 @@ describe('ActivityList', () => { test('sorts activities by date (latest to earliest)', () => { renderActivityList(); + fireEvent.click(screen.getByLabelText(/Show Past Events:/i)); fireEvent.change(screen.getByLabelText(/Sort By:/i), { target: { value: 'latest' }, diff --git a/src/components/PRGradingScreen/PRGradingScreen.duplicates.test.jsx b/src/components/PRGradingScreen/PRGradingScreen.duplicates.test.jsx new file mode 100644 index 00000000000..c67eabfdf09 --- /dev/null +++ b/src/components/PRGradingScreen/PRGradingScreen.duplicates.test.jsx @@ -0,0 +1,196 @@ +import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'; +import axios from 'axios'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import PRGradingScreen from './PRGradingScreen'; + +vi.mock('axios'); +vi.mock('react-redux', () => ({ + useSelector: selector => selector({ theme: { darkMode: false } }), +})); + +const teamData = { + teamName: 'Test Team', + dateRange: { start: '5/18/2025', end: '5/25/2025' }, +}; +const reviewers = [ + { + id: '1', + reviewer: 'Alice', + prsNeeded: 3, + gradedPrs: [{ id: 'pr1', prNumbers: '1234 + 5678', grade: 'Okay' }], + }, + { id: '2', reviewer: 'Bob', prsNeeded: 3, gradedPrs: [] }, +]; + +const addPR = async (row, numbers) => { + const addNewButton = within(row).getByRole('button', { name: '+ Add new' }); + await waitFor(() => expect(addNewButton).toBeEnabled()); + fireEvent.click(addNewButton); + fireEvent.change(within(row).getByPlaceholderText('1070 or 1070 + 1256'), { + target: { value: numbers }, + }); + fireEvent.click(within(row).getByRole('button', { name: 'Add' })); + await waitFor(() => + expect(within(row).queryByRole('button', { name: 'Adding...' })).not.toBeInTheDocument(), + ); +}; + +describe('Weekly PR grading duplicate entries', () => { + beforeEach(() => { + vi.clearAllMocks(); + axios.get.mockResolvedValue({ data: [] }); + axios.post.mockResolvedValue({ data: { status: 'ok' } }); + }); + + it('allows a pair containing two unused PR numbers and counts it as one review', async () => { + render(); + const aliceRow = screen.getByRole('row', { name: /Alice/ }); + + await addPR(aliceRow, '9981 + 9982'); + + expect(within(aliceRow).getByText('9981 + 9982')).toBeInTheDocument(); + expect(within(aliceRow).getByRole('spinbutton')).toHaveValue(2); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }); + + it('rejects a PR already present inside a pair for the same reviewer', async () => { + render(); + const aliceRow = screen.getByRole('row', { name: /Alice/ }); + + await addPR(aliceRow, '5678 + 9999'); + + expect(screen.getByRole('alert')).toHaveTextContent('already been added'); + expect(within(aliceRow).getByRole('spinbutton')).toHaveValue(1); + expect(within(aliceRow).queryByText('5678 + 9999')).not.toBeInTheDocument(); + }); + + it('rejects a repeated number within a new pair', async () => { + render(); + const aliceRow = screen.getByRole('row', { name: /Alice/ }); + + await addPR(aliceRow, '4321 + 4321'); + + expect(screen.getByRole('alert')).toHaveTextContent('already been added'); + expect(within(aliceRow).getByRole('spinbutton')).toHaveValue(1); + }); + + it('rejects a repeated addition but allows another reviewer to use that PR number', async () => { + render(); + const aliceRow = screen.getByRole('row', { name: /Alice/ }); + const bobRow = screen.getByRole('row', { name: /Bob/ }); + + await addPR(aliceRow, '4321'); + await addPR(aliceRow, '4321'); + + expect(screen.getByRole('alert')).toHaveTextContent('already been added'); + expect(within(aliceRow).getByRole('spinbutton')).toHaveValue(2); + + await addPR(bobRow, '4321'); + expect(within(bobRow).getByRole('spinbutton')).toHaveValue(1); + }); + + it('loads a saved PR again after the screen is remounted', async () => { + axios.get + .mockResolvedValueOnce({ data: [] }) + .mockResolvedValueOnce({ data: [] }) + .mockResolvedValueOnce({ + data: [ + { + reviewer: 'Alice', + gradedPrs: [{ _id: 'saved1', prNumbers: '9981', grade: 'Okay' }], + }, + ], + }); + const { unmount } = render(); + await addPR(screen.getByRole('row', { name: /Alice/ }), '9981'); + unmount(); + + render(); + const aliceRow = screen.getByRole('row', { name: /Alice/ }); + await waitFor(() => expect(within(aliceRow).getByText('9981')).toBeInTheDocument()); + expect(within(aliceRow).getByRole('spinbutton')).toHaveValue(2); + }); + + it('keeps the input open when saving fails instead of showing an unsaved PR', async () => { + axios.post.mockRejectedValueOnce(new Error('Network error')); + render(); + const aliceRow = screen.getByRole('row', { name: /Alice/ }); + + await addPR(aliceRow, '9981'); + + expect(within(aliceRow).getByRole('alert')).toHaveTextContent('Could not save'); + expect(within(aliceRow).getByRole('spinbutton')).toHaveValue(1); + expect(within(aliceRow).queryByText('9981')).not.toBeInTheDocument(); + }); + + it('saves a changed grade and restores it after remounting', async () => { + axios.get.mockResolvedValueOnce({ data: [] }).mockResolvedValueOnce({ + data: [ + { + reviewer: 'Alice', + gradedPrs: [{ prNumbers: '1234 + 5678', grade: 'Exceptional' }], + }, + ], + }); + const { unmount } = render(); + await waitFor(() => + expect( + within(screen.getByRole('row', { name: /Alice/ })).getByText('+ Add new'), + ).toBeEnabled(), + ); + fireEvent.click(within(screen.getByRole('row', { name: /Alice/ })).getByText('1234 + 5678')); + fireEvent.click(screen.getAllByRole('checkbox')[0]); + + await waitFor(() => expect(axios.post).toHaveBeenCalledTimes(1)); + expect(axios.post.mock.calls[0][1].gradings[0].gradedPrs).toEqual([ + { prNumbers: '1234 + 5678', grade: 'Exceptional' }, + ]); + await waitFor(() => expect(screen.getAllByRole('checkbox')[0]).toBeChecked()); + unmount(); + + render(); + await waitFor(() => expect(axios.get).toHaveBeenCalledTimes(2)); + fireEvent.click(within(screen.getByRole('row', { name: /Alice/ })).getByText('1234 + 5678')); + await waitFor(() => expect(screen.getAllByRole('checkbox')[0]).toBeChecked()); + expect(within(screen.getByRole('row', { name: /Alice/ })).getByRole('spinbutton')).toHaveValue( + 1, + ); + }); + + it('keeps the previous grade and shows an error if saving fails', async () => { + axios.post.mockRejectedValueOnce(new Error('Network error')); + render(); + await waitFor(() => + expect( + within(screen.getByRole('row', { name: /Alice/ })).getByText('+ Add new'), + ).toBeEnabled(), + ); + fireEvent.click(within(screen.getByRole('row', { name: /Alice/ })).getByText('1234 + 5678')); + fireEvent.click(screen.getAllByRole('checkbox')[0]); + + expect(await screen.findByRole('alert')).toHaveTextContent('Could not save this grade'); + expect(axios.post.mock.calls[0][1].gradings[0].gradedPrs).toEqual([ + { prNumbers: '1234 + 5678', grade: 'Exceptional' }, + ]); + expect(screen.getAllByRole('checkbox')[0]).not.toBeChecked(); + expect(screen.getAllByRole('checkbox')[1]).toBeChecked(); + }); + + it('sends the backend-supported value for Cannot find image', async () => { + render(); + await waitFor(() => + expect( + within(screen.getByRole('row', { name: /Alice/ })).getByText('+ Add new'), + ).toBeEnabled(), + ); + fireEvent.click(within(screen.getByRole('row', { name: /Alice/ })).getByText('1234 + 5678')); + fireEvent.click(screen.getAllByRole('checkbox')[3]); + + await waitFor(() => + expect(axios.post.mock.calls[0][1].gradings[0].gradedPrs).toEqual([ + { prNumbers: '1234 + 5678', grade: 'No Correct Image' }, + ]), + ); + await waitFor(() => expect(screen.getAllByRole('checkbox')[3]).toBeChecked()); + }); +}); diff --git a/src/components/PRGradingScreen/PRGradingScreen.jsx b/src/components/PRGradingScreen/PRGradingScreen.jsx index 25a118011c8..489a6adf9b3 100644 --- a/src/components/PRGradingScreen/PRGradingScreen.jsx +++ b/src/components/PRGradingScreen/PRGradingScreen.jsx @@ -1,21 +1,43 @@ -import React, { useMemo, useState } from 'react'; import PropTypes from 'prop-types'; +import { useEffect, useMemo, useState } from 'react'; import { useSelector } from 'react-redux'; import { v4 as uuidv4 } from 'uuid'; import PRGradingView from './PRGradingView'; +import { + loadSavedGradings, + mergeSavedGradings, + saveAddedPR, + saveGradeChange, +} from './prGradingPersistence'; import { usePRPromotion } from './usePRPromotion'; +const hasDuplicatePR = (gradedPrs, value) => { + const enteredNumbers = value.split('+').map(number => number.trim()); + const existingNumbers = new Set( + gradedPrs.flatMap(pr => pr.prNumbers.split('+').map(number => number.trim())), + ); + return ( + new Set(enteredNumbers).size !== enteredNumbers.length || + enteredNumbers.some(number => existingNumbers.has(number)) + ); +}; + const PRGradingScreen = ({ teamData, reviewers }) => { const darkMode = useSelector(state => state.theme.darkMode); - - // Reviewer & grading state const [reviewerData, setReviewerData] = useState(reviewers || []); const [activeInput, setActiveInput] = useState(null); const [inputValue, setInputValue] = useState(''); + const [inputError, setInputError] = useState(''); const [showGradingModal, setShowGradingModal] = useState(null); const [isFinalized, setIsFinalized] = useState(false); + const [isLoadingSaved, setIsLoadingSaved] = useState(true); + const [isSavingPR, setIsSavingPR] = useState(false); + const [isSavingGrade, setIsSavingGrade] = useState(false); + const [gradeError, setGradeError] = useState(''); + const [loadError, setLoadError] = useState(''); + const [searchTerm, setSearchTerm] = useState(''); + const [roleFilter, setRoleFilter] = useState(''); - // Promotion business logic & state from hook const { promotingReviewer, promotedReviewerIds, @@ -24,102 +46,170 @@ const PRGradingScreen = ({ teamData, reviewers }) => { handleCancelPromotion, } = usePRPromotion(teamData?.teamName); - // Search filter state - const [searchTerm, setSearchTerm] = useState(''); - const [roleFilter, setRoleFilter] = useState(''); - - /* ---------------- SEARCH FILTER ---------------- */ + useEffect(() => { + if (!teamData || !reviewers) return undefined; + let active = true; + setIsLoadingSaved(true); + loadSavedGradings(teamData) + .then(saved => { + if (active) { + setReviewerData(mergeSavedGradings(reviewers, saved)); + setLoadError(''); + } + }) + .catch(() => { + if (active) setLoadError('Could not load saved PRs. Try again before adding a PR.'); + }) + .finally(() => { + if (active) setIsLoadingSaved(false); + }); + return () => { + active = false; + }; + }, [teamData, reviewers]); const availableRoles = useMemo(() => { - const roles = reviewerData.map(r => r.role).filter(Boolean); + const roles = reviewerData.map(reviewer => reviewer.role).filter(Boolean); return [...new Set(roles)]; }, [reviewerData]); - const filteredReviewers = useMemo(() => { - return reviewerData.filter(r => { - const nameMatch = r.reviewer.toLowerCase().includes(searchTerm.toLowerCase()); - const roleMatch = roleFilter ? r.role === roleFilter : true; - return nameMatch && roleMatch; - }); - }, [reviewerData, searchTerm, roleFilter]); + const filteredReviewers = useMemo( + () => + reviewerData.filter(reviewer => { + const nameMatch = reviewer.reviewer.toLowerCase().includes(searchTerm.toLowerCase()); + const roleMatch = roleFilter ? reviewer.role === roleFilter : true; + return nameMatch && roleMatch; + }), + [reviewerData, searchTerm, roleFilter], + ); const handleClearSearch = () => { setSearchTerm(''); setRoleFilter(''); }; - /* ---------------- ADD PR HANDLERS ---------------- */ - const validatePRNumber = value => { const trimmed = value.trim(); const pattern = /^\d+(\s*\+\s*\d+)?$/; - if (!trimmed) return false; - return pattern.test(trimmed); + if (!trimmed) return { isValid: false, error: 'PR number cannot be empty' }; + if (!pattern.test(trimmed)) return { isValid: false, error: 'Format: 1070 or 1070 + 1256' }; + return { isValid: true, error: '' }; }; const handleAddNewClick = reviewerId => { - if (isFinalized) return; + if (isFinalized || isLoadingSaved || loadError) return; setActiveInput(reviewerId); setInputValue(''); + setInputError(''); }; - const handleInputSubmit = reviewerId => { - if (isFinalized) return; - if (!validatePRNumber(inputValue)) { + const handleInputSubmit = async reviewerId => { + if (isFinalized || isLoadingSaved || isSavingPR || loadError) return; + const validation = validatePRNumber(inputValue); + if (!validation.isValid) { + setInputError(validation.error); return; } - const newPREntry = { id: uuidv4(), prNumbers: inputValue.trim(), grade: 'Okay' }; - setReviewerData(prev => - prev.map(r => - r.id === reviewerId - ? { ...r, gradedPrs: [...r.gradedPrs, newPREntry], prsReviewed: r.gradedPrs.length + 1 } - : r, - ), - ); - setActiveInput(null); - setInputValue(''); + const reviewer = reviewerData.find(item => item.id === reviewerId); + if (!reviewer) return; + if (hasDuplicatePR(reviewer.gradedPrs, inputValue)) { + setInputError('This PR number has already been added for this reviewer.'); + return; + } + + setIsSavingPR(true); + try { + const saved = await loadSavedGradings(teamData); + const currentReviewers = mergeSavedGradings(reviewerData, saved); + const currentReviewer = currentReviewers.find(item => item.id === reviewerId); + if (!currentReviewer) return; + if (hasDuplicatePR(currentReviewer.gradedPrs, inputValue)) { + setReviewerData(currentReviewers); + setInputError('This PR number has already been added for this reviewer.'); + return; + } + + const prNumbers = inputValue.trim(); + await saveAddedPR(teamData, currentReviewer, prNumbers); + const newPREntry = { id: uuidv4(), prNumbers, grade: 'Okay' }; + setReviewerData( + currentReviewers.map(item => + item.id === reviewerId + ? { + ...item, + gradedPrs: [...item.gradedPrs, newPREntry], + prsReviewed: item.gradedPrs.length + 1, + } + : item, + ), + ); + setActiveInput(null); + setInputValue(''); + setInputError(''); + } catch { + setInputError('Could not save this PR. Please try again.'); + } finally { + setIsSavingPR(false); + } }; const handleCancelInput = () => { - if (isFinalized) return; + if (isFinalized || isSavingPR) return; setActiveInput(null); setInputValue(''); + setInputError(''); }; - /* ---------------- MODAL HANDLERS ---------------- */ - const handlePRNumberClick = reviewerId => { - if (isFinalized) return; + if (isFinalized || isLoadingSaved || loadError) return; + setGradeError(''); setShowGradingModal(reviewerId); }; - const handleGradeChange = (reviewerId, prId, newGrade) => { - if (isFinalized) return; - setReviewerData(prev => - prev.map(r => - r.id === reviewerId - ? { - ...r, - gradedPrs: r.gradedPrs.map(pr => (pr.id === prId ? { ...pr, grade: newGrade } : pr)), - } - : r, - ), - ); + const handleGradeChange = async (reviewerId, prId, newGrade) => { + if (isFinalized || isLoadingSaved || isSavingGrade || loadError) return; + const reviewer = reviewerData.find(item => item.id === reviewerId); + const pr = reviewer?.gradedPrs.find(item => item.id === prId); + if (!pr || pr.grade === newGrade) return; + + setIsSavingGrade(true); + setGradeError(''); + try { + const savedGrade = newGrade === 'Cannot find image' ? 'No Correct Image' : newGrade; + await saveGradeChange(teamData, reviewer, pr.prNumbers, savedGrade); + setReviewerData(prev => + prev.map(item => + item.id === reviewerId + ? { + ...item, + gradedPrs: item.gradedPrs.map(gradedPR => + gradedPR.id === prId ? { ...gradedPR, grade: newGrade } : gradedPR, + ), + } + : item, + ), + ); + } catch { + setGradeError('Could not save this grade. Please try again.'); + } finally { + setIsSavingGrade(false); + } }; - const handleCloseGradingModal = () => setShowGradingModal(null); - const handleFinalize = () => setIsFinalized(true); + const handleCloseGradingModal = () => { + if (isSavingGrade) return; + setShowGradingModal(null); + setGradeError(''); + }; - if (!teamData || !reviewers) { - return
Error: Missing required props
; - } + if (!teamData || !reviewers) return
Error: Missing required props
; return ( setIsFinalized(true)} searchTerm={searchTerm} onSearchTermChange={setSearchTerm} roleFilter={roleFilter} @@ -131,7 +221,16 @@ const PRGradingScreen = ({ teamData, reviewers }) => { onPromoteClick={handlePromoteClick} activeInput={activeInput} inputValue={inputValue} - onInputValueChange={setInputValue} + inputError={inputError} + loadError={loadError} + gradeError={gradeError} + isLoadingSaved={isLoadingSaved} + isSavingPR={isSavingPR} + isSavingGrade={isSavingGrade} + onInputValueChange={value => { + setInputValue(value); + setInputError(''); + }} onAddNewClick={handleAddNewClick} onInputSubmit={handleInputSubmit} onCancelInput={handleCancelInput} @@ -141,7 +240,7 @@ const PRGradingScreen = ({ teamData, reviewers }) => { onCancelPromotion={handleCancelPromotion} showGradingModal={showGradingModal} onCloseGradingModal={handleCloseGradingModal} - gradingReviewer={reviewerData.find(r => r.id === showGradingModal)} + gradingReviewer={reviewerData.find(reviewer => reviewer.id === showGradingModal)} onGradeChange={handleGradeChange} /> ); diff --git a/src/components/PRGradingScreen/PRGradingScreen.module.css b/src/components/PRGradingScreen/PRGradingScreen.module.css index adfff57b389..38e40fb9641 100644 --- a/src/components/PRGradingScreen/PRGradingScreen.module.css +++ b/src/components/PRGradingScreen/PRGradingScreen.module.css @@ -381,7 +381,7 @@ } .pr-grading-screen-error-message { - color: #f8d7da; + color: #b02a37; font-size: 0.75rem; margin-top: 4px; text-align: center; @@ -989,6 +989,10 @@ color: #9ab0cc; } +.pr-grading-screen-error-message.dark-mode { + color: #f8d7da; +} + /* Promotion Confirmation Modal Actions (From Your Branch) */ .pr-promotion-no-history { margin-bottom: 16px; @@ -1247,4 +1251,4 @@ padding: 2px 6px; font-size: 12px; } -} \ No newline at end of file +} diff --git a/src/components/PRGradingScreen/PRGradingScreenContainer.test.jsx b/src/components/PRGradingScreen/PRGradingScreenContainer.test.jsx new file mode 100644 index 00000000000..448cef10efd --- /dev/null +++ b/src/components/PRGradingScreen/PRGradingScreenContainer.test.jsx @@ -0,0 +1,49 @@ +import { render, screen } from '@testing-library/react'; +import axios from 'axios'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import PRGradingScreenContainer from './index'; + +const route = vi.hoisted(() => ({ search: '', state: null })); + +vi.mock('axios'); +vi.mock('react-router-dom', () => ({ useLocation: () => route })); +vi.mock('./PRGradingScreen', () => ({ + default: ({ teamData, reviewers }) => ( +
+ {teamData.teamName} - {teamData.dateRange.start} - {reviewers.length} reviewers +
+ ), +})); + +describe('PR grading screen team selection', () => { + beforeEach(() => { + vi.clearAllMocks(); + route.search = ''; + route.state = null; + }); + + it('loads the built-in team when opened directly', () => { + render(); + + expect(screen.getByText(/91NePRT - 5\/18\/2025/)).toBeInTheDocument(); + expect(axios.get).not.toHaveBeenCalled(); + }); + + it('reloads a custom team from its URL after a refresh', async () => { + route.search = '?teamId=custom1&weekStart=2026-10-04'; + axios.get.mockResolvedValue({ + data: [ + { + _id: 'custom1', + teamName: 'Custom Team', + reviewerCount: 2, + reviewerNames: ['Alice', 'Bob'], + }, + ], + }); + + render(); + + expect(await screen.findByText(/Custom Team - 10\/4\/2026 - 2 reviewers/)).toBeInTheDocument(); + }); +}); diff --git a/src/components/PRGradingScreen/PRGradingTest.jsx b/src/components/PRGradingScreen/PRGradingTest.jsx index fd33f7d78e7..44bc714296c 100644 --- a/src/components/PRGradingScreen/PRGradingTest.jsx +++ b/src/components/PRGradingScreen/PRGradingTest.jsx @@ -50,7 +50,18 @@ const PRGradingTest = () => { const handleTeamSelect = teamId => { const team = allTeams.find(t => t.id === teamId); - history.push('/pr-grading-screen', { teamId, config: team }); + const params = new URLSearchParams({ teamId }); + if (team?.fromDB) { + const start = new Date(); + start.setDate(start.getDate() - start.getDay()); + const date = [ + start.getFullYear(), + String(start.getMonth() + 1).padStart(2, '0'), + String(start.getDate()).padStart(2, '0'), + ].join('-'); + params.set('weekStart', date); + } + history.push(`/pr-grading-screen?${params.toString()}`, { teamId, config: team }); }; const handleDeleteConfig = async (e, teamId) => { diff --git a/src/components/PRGradingScreen/PRGradingView.jsx b/src/components/PRGradingScreen/PRGradingView.jsx index 22a30e86789..a5ad8671bb2 100644 --- a/src/components/PRGradingScreen/PRGradingView.jsx +++ b/src/components/PRGradingScreen/PRGradingView.jsx @@ -21,6 +21,12 @@ const PRGradingView = ({ onPromoteClick, activeInput, inputValue, + inputError, + loadError, + gradeError, + isLoadingSaved, + isSavingPR, + isSavingGrade, onInputValueChange, onAddNewClick, onInputSubmit, @@ -55,16 +61,26 @@ const PRGradingView = ({ + {loadError && ( +

+ {loadError} +

+ )} + {gradeError && ( +

+ {gradeError} +

+ )} {/* Search and Filter Bar */}
onAddNewClick(reviewer.id)} > @@ -211,20 +228,35 @@ const PRGradingView = ({ onInputValueChange(e.target.value)} className={styles['pr-grading-screen-pr-number-input']} placeholder="1070 or 1070 + 1256" /> + {inputError && ( + + {inputError} + + )}
-
@@ -236,7 +268,7 @@ const PRGradingView = ({ @@ -259,6 +291,7 @@ const PRGradingView = ({

Grade PR for {gradingReviewer.reviewer}