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
+ {loadError} +
+ {gradeError} +