From e6cbbfc2083df64052431742b63cf8f3f1b98e9f Mon Sep 17 00:00:00 2001 From: sharikak54 Date: Tue, 8 Jul 2025 13:03:17 -0700 Subject: [PATCH 1/8] Check for existing duplicate results in the round --- .../ResultAttemptsForm/ResultAttemptsForm.jsx | 1 + client/src/lib/attempt-result.js | 46 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/client/src/components/admin/ResultAttemptsForm/ResultAttemptsForm.jsx b/client/src/components/admin/ResultAttemptsForm/ResultAttemptsForm.jsx index 029a3452..70dbfeec 100644 --- a/client/src/components/admin/ResultAttemptsForm/ResultAttemptsForm.jsx +++ b/client/src/components/admin/ResultAttemptsForm/ResultAttemptsForm.jsx @@ -100,6 +100,7 @@ function ResultAttemptsForm({ attemptResults, eventId, officialWorldRecords, + results ); if (submissionWarning) { diff --git a/client/src/lib/attempt-result.js b/client/src/lib/attempt-result.js index c3f085bb..435c2d11 100644 --- a/client/src/lib/attempt-result.js +++ b/client/src/lib/attempt-result.js @@ -399,6 +399,7 @@ export function attemptResultsWarning( attemptResults, eventId, officialWorldRecords = [], + results = [], ) { const skippedGapIndex = trimTrailingSkipped(attemptResults).indexOf(SKIPPED_VALUE); @@ -482,6 +483,21 @@ export function attemptResultsWarning( }; } } + + // Check whether this result is a duplicate of existing results in the same round. + // Excludes FMC and all-DNF results since ties are common. + // TODO: Does NOT check the in-progress batch, if any. + if (["333fm"].indexOf(eventId) === -1) { + const matches = findAllMatchingResults(attemptResults, results); + if (matches.length > 0) { + const matchesString = matches.map((match) => `${match.person.name} (${match.person.id})`).join(", "); + return { + description: `The result you're trying to submit matches all results for + the following competitor${matches.length > 1 ? "s" : ""}: ${matchesString}. + Please check that the results are accurate.`, + }; + } + } } return null; } @@ -545,3 +561,33 @@ function checkForDnsFollowedByValidResult(attemptResults) { index > dnsIndex && attempt !== SKIPPED_VALUE && attempt !== DNS_VALUE, ); } + +/** + * Check whether an attempt matches an existing attempt exactly. + */ +function findAllMatchingResults(attemptResults, results) { + const matches = results.filter( + (result) => { + if (result.attempts.length !== attemptResults.length) { + return false; + } + + let numDnfResults = 0; + for (let i = 0; i < result.attempts.length; i++) { + if (result.attempts[i].result !== attemptResults[i]) { + return false; + } + if (result.attempts[i].result === DNF_VALUE) { + numDnfResults++; + } + } + // Exclude all-DNF results since ties are common + if (numDnfResults === result.attempts.length) { + return false; + } + + return true; + } + ) + return matches; +} From ba1f5f3d09a43842af6a88e5fcce8349235a5949 Mon Sep 17 00:00:00 2001 From: sharikak54 Date: Tue, 8 Jul 2025 13:20:02 -0700 Subject: [PATCH 2/8] add tests for duplicate results check --- client/src/lib/tests/attempt-result.test.js | 34 +++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/client/src/lib/tests/attempt-result.test.js b/client/src/lib/tests/attempt-result.test.js index 0d8297db..c8f5b9ac 100644 --- a/client/src/lib/tests/attempt-result.test.js +++ b/client/src/lib/tests/attempt-result.test.js @@ -536,6 +536,40 @@ describe("attemptResultsWarning", () => { attemptResultsWarning(attemptResults, "333fm", worldRecords), ).toEqual(null); }); + + it("complains about exact duplicate results", () => { + const attemptResults = [500, 600, 700, 800, 900]; + const existingResults = [ + {attempts: [{result: 500}, {result: 600}, {result: 700}, {result: 800}, {result: 900}], person: {id: 2, name: "Person 2"}} + ]; + expect( + attemptResultsWarning(attemptResults, "333", [], existingResults) + ).toMatchObject({ + description: `The result you're trying to submit matches all results for + the following competitor: Person 2 (2). + Please check that the results are accurate.`, + }); + }); + + it("does not check for duplicates in FMC", () => { + const attemptResults = [25, 26]; + const existingResults = [ + {attempts: [{result: 25}, {result: 26}], person: {id: 2, name: "Person 2"}} + ]; + expect( + attemptResultsWarning(attemptResults, "333fm", [], existingResults) + ).toEqual(null); + }); + + it("does not warn about all-DNF duplicates", () => { + const attemptResults = [-1, -1, -1, -1, -1]; + const existingResults = [ + {attempts: [{result: -1}, {result: -1}, {result: -1}, {result: -1}, {result: -1}], person: {id: 2, name: "Person 2"}} + ]; + expect( + attemptResultsWarning(attemptResults, "333fm", [], existingResults) + ).toEqual(null); + }); }); describe("applyTimeLimit", () => { From 04d3320ba4f6126698f331fe93725cf4e8fa16af Mon Sep 17 00:00:00 2001 From: sharikak54 Date: Tue, 8 Jul 2025 13:57:10 -0700 Subject: [PATCH 3/8] Pass batch results to filter as well --- .../components/admin/AdminRound/AdminRoundContent.jsx | 1 + .../admin/ResultAttemptsForm/ResultAttemptsForm.jsx | 3 ++- client/src/lib/attempt-result.js | 11 ++++++++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/client/src/components/admin/AdminRound/AdminRoundContent.jsx b/client/src/components/admin/AdminRound/AdminRoundContent.jsx index 84db5a82..03277ab4 100644 --- a/client/src/components/admin/AdminRound/AdminRoundContent.jsx +++ b/client/src/components/admin/AdminRound/AdminRoundContent.jsx @@ -174,6 +174,7 @@ function AdminRoundContent({ round, competitionId, officialWorldRecords }) { 0) { - const matchesString = matches.map((match) => `${match.person.name} (${match.person.id})`).join(", "); + // Batch entries don't have `person` attributes, just `id` + const matchesString = matches.map((match) => + `${match.person ? match.person.name : "[batched competitor]"} (${match.id})` + ).join(", "); return { description: `The result you're trying to submit matches all results for the following competitor${matches.length > 1 ? "s" : ""}: ${matchesString}. @@ -566,15 +569,17 @@ function checkForDnsFollowedByValidResult(attemptResults) { * Check whether an attempt matches an existing attempt exactly. */ function findAllMatchingResults(attemptResults, results) { + const filteredAttemptResults = trimTrailingSkipped(attemptResults); + const matches = results.filter( (result) => { - if (result.attempts.length !== attemptResults.length) { + if (result.attempts.length !== filteredAttemptResults.length) { return false; } let numDnfResults = 0; for (let i = 0; i < result.attempts.length; i++) { - if (result.attempts[i].result !== attemptResults[i]) { + if (result.attempts[i].result !== filteredAttemptResults[i]) { return false; } if (result.attempts[i].result === DNF_VALUE) { From 330a2f57d4716c9815a5b5a62967bcf8239f2442 Mon Sep 17 00:00:00 2001 From: sharikak54 Date: Tue, 8 Jul 2025 14:03:38 -0700 Subject: [PATCH 4/8] add `person` to stored batch results; npm run format --- .../admin/AdminRound/AdminRoundContent.jsx | 4 +- .../ResultAttemptsForm/ResultAttemptsForm.jsx | 4 +- client/src/lib/attempt-result.js | 43 +++++++++---------- client/src/lib/tests/attempt-result.test.js | 33 +++++++++++--- 4 files changed, 51 insertions(+), 33 deletions(-) diff --git a/client/src/components/admin/AdminRound/AdminRoundContent.jsx b/client/src/components/admin/AdminRound/AdminRoundContent.jsx index 03277ab4..774afd7e 100644 --- a/client/src/components/admin/AdminRound/AdminRoundContent.jsx +++ b/client/src/components/admin/AdminRound/AdminRoundContent.jsx @@ -86,11 +86,11 @@ function AdminRoundContent({ round, competitionId, officialWorldRecords }) { onError: apolloErrorHandler, }); - function handleResultAttemptsSubmit(attempts) { + function handleResultAttemptsSubmit(attempts, person) { if (isBatchMode) { setBatchResults([ ...batchResults.filter((result) => result.id !== editedResult.id), - { id: editedResult.id, attempts, enteredAt: nowISOString() }, + { id: editedResult.id, attempts, person, enteredAt: nowISOString() }, ]); setEditedResult(null); } else { diff --git a/client/src/components/admin/ResultAttemptsForm/ResultAttemptsForm.jsx b/client/src/components/admin/ResultAttemptsForm/ResultAttemptsForm.jsx index 9c44dedc..6b741d15 100644 --- a/client/src/components/admin/ResultAttemptsForm/ResultAttemptsForm.jsx +++ b/client/src/components/admin/ResultAttemptsForm/ResultAttemptsForm.jsx @@ -92,7 +92,7 @@ function ResultAttemptsForm({ const attempts = trimTrailingSkipped(attemptResults).map((result) => ({ result, })); - onSubmit(attempts); + onSubmit(attempts, result.person); }); } @@ -101,7 +101,7 @@ function ResultAttemptsForm({ attemptResults, eventId, officialWorldRecords, - [...results, ...batchResults] + [...results, ...batchResults], ); if (submissionWarning) { diff --git a/client/src/lib/attempt-result.js b/client/src/lib/attempt-result.js index 0beef5ce..b0bf7693 100644 --- a/client/src/lib/attempt-result.js +++ b/client/src/lib/attempt-result.js @@ -490,10 +490,9 @@ export function attemptResultsWarning( if (["333fm"].indexOf(eventId) === -1) { const matches = findAllMatchingResults(attemptResults, results); if (matches.length > 0) { - // Batch entries don't have `person` attributes, just `id` - const matchesString = matches.map((match) => - `${match.person ? match.person.name : "[batched competitor]"} (${match.id})` - ).join(", "); + const matchesString = matches + .map((match) => `${match.person.name} (${match.person.id})`) + .join(", "); return { description: `The result you're trying to submit matches all results for the following competitor${matches.length > 1 ? "s" : ""}: ${matchesString}. @@ -571,28 +570,26 @@ function checkForDnsFollowedByValidResult(attemptResults) { function findAllMatchingResults(attemptResults, results) { const filteredAttemptResults = trimTrailingSkipped(attemptResults); - const matches = results.filter( - (result) => { - if (result.attempts.length !== filteredAttemptResults.length) { - return false; - } + const matches = results.filter((result) => { + if (result.attempts.length !== filteredAttemptResults.length) { + return false; + } - let numDnfResults = 0; - for (let i = 0; i < result.attempts.length; i++) { - if (result.attempts[i].result !== filteredAttemptResults[i]) { - return false; - } - if (result.attempts[i].result === DNF_VALUE) { - numDnfResults++; - } - } - // Exclude all-DNF results since ties are common - if (numDnfResults === result.attempts.length) { + let numDnfResults = 0; + for (let i = 0; i < result.attempts.length; i++) { + if (result.attempts[i].result !== filteredAttemptResults[i]) { return false; } - - return true; + if (result.attempts[i].result === DNF_VALUE) { + numDnfResults++; + } } - ) + // Exclude all-DNF results since ties are common + if (numDnfResults === result.attempts.length) { + return false; + } + + return true; + }); return matches; } diff --git a/client/src/lib/tests/attempt-result.test.js b/client/src/lib/tests/attempt-result.test.js index c8f5b9ac..6ff5053f 100644 --- a/client/src/lib/tests/attempt-result.test.js +++ b/client/src/lib/tests/attempt-result.test.js @@ -540,10 +540,19 @@ describe("attemptResultsWarning", () => { it("complains about exact duplicate results", () => { const attemptResults = [500, 600, 700, 800, 900]; const existingResults = [ - {attempts: [{result: 500}, {result: 600}, {result: 700}, {result: 800}, {result: 900}], person: {id: 2, name: "Person 2"}} + { + attempts: [ + { result: 500 }, + { result: 600 }, + { result: 700 }, + { result: 800 }, + { result: 900 }, + ], + person: { id: 2, name: "Person 2" }, + }, ]; expect( - attemptResultsWarning(attemptResults, "333", [], existingResults) + attemptResultsWarning(attemptResults, "333", [], existingResults), ).toMatchObject({ description: `The result you're trying to submit matches all results for the following competitor: Person 2 (2). @@ -554,20 +563,32 @@ describe("attemptResultsWarning", () => { it("does not check for duplicates in FMC", () => { const attemptResults = [25, 26]; const existingResults = [ - {attempts: [{result: 25}, {result: 26}], person: {id: 2, name: "Person 2"}} + { + attempts: [{ result: 25 }, { result: 26 }], + person: { id: 2, name: "Person 2" }, + }, ]; expect( - attemptResultsWarning(attemptResults, "333fm", [], existingResults) + attemptResultsWarning(attemptResults, "333fm", [], existingResults), ).toEqual(null); }); it("does not warn about all-DNF duplicates", () => { const attemptResults = [-1, -1, -1, -1, -1]; const existingResults = [ - {attempts: [{result: -1}, {result: -1}, {result: -1}, {result: -1}, {result: -1}], person: {id: 2, name: "Person 2"}} + { + attempts: [ + { result: -1 }, + { result: -1 }, + { result: -1 }, + { result: -1 }, + { result: -1 }, + ], + person: { id: 2, name: "Person 2" }, + }, ]; expect( - attemptResultsWarning(attemptResults, "333fm", [], existingResults) + attemptResultsWarning(attemptResults, "333fm", [], existingResults), ).toEqual(null); }); }); From f93a82c7bf17948c27411257bd42b0e0bc475896 Mon Sep 17 00:00:00 2001 From: sharikak54 Date: Sat, 12 Jul 2025 21:36:36 -0700 Subject: [PATCH 5/8] small review comments --- client/src/lib/attempt-result.js | 14 ++------------ client/src/lib/tests/attempt-result.test.js | 2 +- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/client/src/lib/attempt-result.js b/client/src/lib/attempt-result.js index b0bf7693..d9ecd91d 100644 --- a/client/src/lib/attempt-result.js +++ b/client/src/lib/attempt-result.js @@ -486,8 +486,7 @@ export function attemptResultsWarning( // Check whether this result is a duplicate of existing results in the same round. // Excludes FMC and all-DNF results since ties are common. - // TODO: Does NOT check the in-progress batch, if any. - if (["333fm"].indexOf(eventId) === -1) { + if (eventId !== "333fm" && completeAttempts.length > 0) { const matches = findAllMatchingResults(attemptResults, results); if (matches.length > 0) { const matchesString = matches @@ -574,22 +573,13 @@ function findAllMatchingResults(attemptResults, results) { if (result.attempts.length !== filteredAttemptResults.length) { return false; } - - let numDnfResults = 0; for (let i = 0; i < result.attempts.length; i++) { if (result.attempts[i].result !== filteredAttemptResults[i]) { return false; } - if (result.attempts[i].result === DNF_VALUE) { - numDnfResults++; - } - } - // Exclude all-DNF results since ties are common - if (numDnfResults === result.attempts.length) { - return false; } - return true; }); + return matches; } diff --git a/client/src/lib/tests/attempt-result.test.js b/client/src/lib/tests/attempt-result.test.js index 6ff5053f..b9b20a89 100644 --- a/client/src/lib/tests/attempt-result.test.js +++ b/client/src/lib/tests/attempt-result.test.js @@ -588,7 +588,7 @@ describe("attemptResultsWarning", () => { }, ]; expect( - attemptResultsWarning(attemptResults, "333fm", [], existingResults), + attemptResultsWarning(attemptResults, "333", [], existingResults), ).toEqual(null); }); }); From f97de5b3cef63e8b2c3b5775e250dce823b98926 Mon Sep 17 00:00:00 2001 From: sharikak54 Date: Sat, 12 Jul 2025 22:24:02 -0700 Subject: [PATCH 6/8] store the combined results and batchResults, and update it whenever the batch or round changes --- .../admin/AdminRound/AdminRoundContent.jsx | 29 +++++++++++++++++-- .../ResultAttemptsForm/ResultAttemptsForm.jsx | 4 +-- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/client/src/components/admin/AdminRound/AdminRoundContent.jsx b/client/src/components/admin/AdminRound/AdminRoundContent.jsx index 774afd7e..f2388b2a 100644 --- a/client/src/components/admin/AdminRound/AdminRoundContent.jsx +++ b/client/src/components/admin/AdminRound/AdminRoundContent.jsx @@ -59,6 +59,23 @@ function setStoredBatchResults(roundId, results) { } } +/** + * Combine results and batchResults to produce a temporary results-like array + * that has all batchResults changes applied. This allows us to display a + * warning if a result is being entered which matches a batched result exactly. + */ +function combineResultsAndBatchResults(results, batchResults) { + const combinedResults = [...results]; + batchResults.forEach((batchedResult) => { + const i = combinedResults.findIndex((res) => res.id === batchedResult.id); + combinedResults[i] = { + ...combinedResults[i], + attempts: batchedResult.attempts, + }; + }); + return combinedResults; +} + function AdminRoundContent({ round, competitionId, officialWorldRecords }) { const confirm = useConfirm(); const { enqueueSnackbar, closeSnackbar } = useSnackbar(); @@ -72,6 +89,9 @@ function AdminRoundContent({ round, competitionId, officialWorldRecords }) { getStoreBatchResults(round.id), ); const [isBatchMode, setIsBatchMode] = useState(batchResults.length > 0); + const [combinedResults, setCombinedResults] = useState(() => + combineResultsAndBatchResults(round.results, batchResults), + ); const formContainerRef = useRef(null); const [enterResults, { loading }] = useMutation(ENTER_RESULTS, { @@ -86,11 +106,11 @@ function AdminRoundContent({ round, competitionId, officialWorldRecords }) { onError: apolloErrorHandler, }); - function handleResultAttemptsSubmit(attempts, person) { + function handleResultAttemptsSubmit(attempts) { if (isBatchMode) { setBatchResults([ ...batchResults.filter((result) => result.id !== editedResult.id), - { id: editedResult.id, attempts, person, enteredAt: nowISOString() }, + { id: editedResult.id, attempts, enteredAt: nowISOString() }, ]); setEditedResult(null); } else { @@ -165,6 +185,9 @@ function AdminRoundContent({ round, competitionId, officialWorldRecords }) { useEffect(() => { setStoredBatchResults(round.id, batchResults); + setCombinedResults( + combineResultsAndBatchResults(round.results, batchResults), + ); }, [round.id, batchResults]); return ( @@ -174,7 +197,7 @@ function AdminRoundContent({ round, competitionId, officialWorldRecords }) { Date: Sat, 12 Jul 2025 22:29:53 -0700 Subject: [PATCH 7/8] remove warning for unchanged entries --- .../admin/ResultAttemptsForm/ResultAttemptsForm.jsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/src/components/admin/ResultAttemptsForm/ResultAttemptsForm.jsx b/client/src/components/admin/ResultAttemptsForm/ResultAttemptsForm.jsx index 2b11df36..9ba8283b 100644 --- a/client/src/components/admin/ResultAttemptsForm/ResultAttemptsForm.jsx +++ b/client/src/components/admin/ResultAttemptsForm/ResultAttemptsForm.jsx @@ -97,11 +97,16 @@ function ResultAttemptsForm({ } function confirmSubmission() { + // We don't want to show a duplicate warning if the user is submitting + // already-entered results for the correct competitor. This is often used + // as a quick way to refresh data without refreshing the whole page. + const resultsToCheck = combinedResults.filter((res) => res.id !== result.id); + const submissionWarning = attemptResultsWarning( attemptResults, eventId, officialWorldRecords, - combinedResults, + resultsToCheck, ); if (submissionWarning) { From 858b46b3caff8002104da3ca1294271e05702334 Mon Sep 17 00:00:00 2001 From: sharikak54 Date: Sat, 12 Jul 2025 22:45:37 -0700 Subject: [PATCH 8/8] AdminRound gets a new gql result (ie a new js object) whenever the round changes, so this useEffect can just use `round` as a dep --- client/src/components/admin/AdminRound/AdminRoundContent.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/components/admin/AdminRound/AdminRoundContent.jsx b/client/src/components/admin/AdminRound/AdminRoundContent.jsx index 7fd3d8d0..72b33531 100644 --- a/client/src/components/admin/AdminRound/AdminRoundContent.jsx +++ b/client/src/components/admin/AdminRound/AdminRoundContent.jsx @@ -207,7 +207,7 @@ function AdminRoundContent({ round, competitionId, officialWorldRecords }) { setCombinedResults( combineResultsAndBatchResults(round.results, batchResults), ); - }, [round.id, batchResults]); + }, [round, batchResults]); return ( <>