Merge pull request #83 from coder13/agent/fix-competition-results

Merge competition result sources safely

Author: coder13

src/containers/CompetitionResults/CompetitionResults.test.tsx

@@ -43,8 +43,8 @@ jest.mock('react-i18next', () => ({
       if (key === 'competition.results.average') return 'Avg';
       if (key === 'competition.results.best') return 'Best';
       if (key === 'competition.results.attempts') return 'Attempts';
-      if (key === 'competition.results.liveResultsDelayNote') {
-        return 'Data is pulled from WCA Live and may be delayed.';
+      if (key === 'competition.results.resultsSourceNote') {
+        return 'Results are merged from WCIF and WCA Live when available. WCA Live data may be delayed.';
       }
       if (key === 'competition.results.allResults') return 'All results';
       if (key === 'competition.results.unknownCompetitor') {
@@ -273,7 +273,9 @@ describe('CompetitionResultsContainer', () => {
     expect(screen.queryByRole('link', { name: 'Round 2' })).not.toBeInTheDocument();
     expect(screen.queryByRole('table', { name: 'Results' })).not.toBeInTheDocument();
     expect(
-      screen.getByText('Data is pulled from WCA Live and may be delayed.'),
+      screen.getByText(
+        'Results are merged from WCIF and WCA Live when available. WCA Live data may be delayed.',
+      ),
     ).toBeInTheDocument();
   });
 
@@ -316,7 +318,9 @@ describe('CompetitionResultsContainer', () => {
     expect(screen.getAllByText('12.00')).toHaveLength(2);
     expect(screen.queryByLabelText('Attempts')).not.toBeInTheDocument();
     expect(
-      screen.getByText('Data is pulled from WCA Live and may be delayed.'),
+      screen.getByText(
+        'Results are merged from WCIF and WCA Live when available. WCA Live data may be delayed.',
+      ),
     ).toBeInTheDocument();
     expect(
       screen
@@ -511,7 +515,9 @@ describe('CompetitionResultsContainer', () => {
       within(screen.getByRole('row', { name: /2 Nick Silvestri/ })).getAllByText('15.00'),
     ).toHaveLength(2);
     expect(
-      screen.getByText('Data is pulled from WCA Live and may be delayed.'),
+      screen.getByText(
+        'Results are merged from WCIF and WCA Live when available. WCA Live data may be delayed.',
+      ),
     ).toBeInTheDocument();
   });
 

src/containers/CompetitionResults/CompetitionResults.tsx

@@ -274,7 +274,7 @@ export function CompetitionResultsContainer({
                     roundNumber: selectedRound.roundNumber,
                   })}
                 </h2>
-                <NoteBox text={t('competition.results.liveResultsDelayNote')} />
+                <NoteBox text={t('competition.results.resultsSourceNote')} />
                 <ResultsEventRoundSelector
                   selectedRoundId={selectedRound.round.id}
                   rounds={selectedEventRoundLinks}
@@ -313,7 +313,7 @@ export function CompetitionResultsContainer({
   return (
     <Container className="pt-4">
       <div className="flex flex-col p-2 space-y-4 type-body">
-        <NoteBox text={t('competition.results.liveResultsDelayNote')} />
+        <NoteBox text={t('competition.results.resultsSourceNote')} />
         <RoundActionPicker mode="results" events={pickerEvents} LinkComponent={LinkComponent} />
       </div>
     </Container>

src/containers/CompetitionResults/resultSources.ts

@@ -5,6 +5,16 @@ import { normalizeResultRecordTag } from './ResultRecordBadge';
 
 const roundTypeOrder = ['0', '1', 'c', '2', 'e', '3', 'b', 'd', 'f'];
 
+const normalizeName = (value: string) => value.trim().toLowerCase();
+
+export const findUniquePersonByName = (persons: Person[], name: string) => {
+  const matchingPersons = persons.filter(
+    (person) => normalizeName(person.name) === normalizeName(name),
+  );
+
+  return matchingPersons.length === 1 ? matchingPersons[0] : undefined;
+};
+
 const compareRoundTypeIds = (a: string, b: string) => {
   const aIndex = roundTypeOrder.indexOf(a);
   const bIndex = roundTypeOrder.indexOf(b);
@@ -24,12 +34,13 @@ const compareRoundTypeIds = (a: string, b: string) => {
   return a.localeCompare(b);
 };
 
-export const findPersonForApiResult = (persons: Person[], result: WcaCompetitionResult) =>
-  persons.find(
-    (person) =>
-      (result.wca_id && person.wcaId === result.wca_id) ||
-      person.name.toLocaleLowerCase() === result.name.toLocaleLowerCase(),
-  );
+export const findPersonForApiResult = (persons: Person[], result: WcaCompetitionResult) => {
+  if (result.wca_id) {
+    return persons.find((person) => person.wcaId === result.wca_id);
+  }
+
+  return findUniquePersonByName(persons, result.name);
+};
 
 export const getWcaApiRoundTypeMap = (results: WcaCompetitionResult[]) => {
   const roundTypeIdsByEventId = new Map<string, string[]>();

src/containers/CompetitionResults/resultsProvider.test.ts

@@ -1,6 +1,7 @@
 import { Competition, Person } from '@wca/helpers';
 import { WcaLiveCompetitorResult, WcaLiveRound } from '@/hooks/queries/useWcaLive';
 import { WcaCompetitionResult } from '@/lib/api';
+import { findPersonForApiResult } from './resultSources';
 import { getPersonalResultsFromSources, getRoundResultsFromSources } from './resultsProvider';
 
 jest.mock('@/lib/events', () => ({
@@ -186,6 +187,8 @@ const liveRoundResult = (
   person: {
     id: '2',
     registrantId: 2,
+    wcaUserId: 1002,
+    wcaId: '2010BOBS01',
     name: 'Bob Solver',
   },
   ...overrides,
@@ -222,6 +225,39 @@ const livePersonalResult = (
   ...overrides,
 });
 
+describe('findPersonForApiResult', () => {
+  const duplicateNamePerson: Person = {
+    ...persons[0],
+    registrantId: 4,
+    wcaUserId: 1004,
+    wcaId: '2010ALIC02',
+  };
+
+  it('matches an exact WCA ID before considering duplicate names', () => {
+    expect(
+      findPersonForApiResult(
+        [...persons, duplicateNamePerson],
+        apiResult({ name: 'Alice Solver', wca_id: '2010ALIC02' }),
+      ),
+    ).toBe(duplicateNamePerson);
+  });
+
+  it('does not fall back to a name when a WCA ID is present but unmatched', () => {
+    expect(
+      findPersonForApiResult(persons, apiResult({ name: 'Alice Solver', wca_id: '2099MISS01' })),
+    ).toBeUndefined();
+  });
+
+  it('does not use an ambiguous name when the API result has no WCA ID', () => {
+    expect(
+      findPersonForApiResult(
+        [...persons, duplicateNamePerson],
+        apiResult({ name: 'Alice Solver', wca_id: null }),
+      ),
+    ).toBeUndefined();
+  });
+});
+
 describe('resultsProvider', () => {
   describe('getRoundResultsFromSources', () => {
     it('returns no round results without WCIF or a selected round', () => {
@@ -301,6 +337,8 @@ describe('resultsProvider', () => {
             average: 2200,
             round_type_id: '1',
             attempts: [2100, 2200, 2300],
+            best_index: 2,
+            worst_index: 0,
             regional_single_record: 'NR',
             regional_average_record: 'WR',
           }),
@@ -326,6 +364,12 @@ describe('resultsProvider', () => {
         singleRecordTag: 'NR',
         averageRecordTag: 'WR',
       });
+      expect(results.find((result) => result.personId === 2)).not.toHaveProperty(
+        'bestAttemptIndex',
+      );
+      expect(results.find((result) => result.personId === 2)).not.toHaveProperty(
+        'worstAttemptIndex',
+      );
       expect(results.find((result) => result.personId === 1)).toMatchObject({
         ranking: 1,
         best: 1200,
@@ -341,6 +385,98 @@ describe('resultsProvider', () => {
       });
     });
 
+    it('matches live round results by WCA user id when registrant id is missing', () => {
+      const results = getRoundResultsFromSources({
+        wcif,
+        selectedRound: selectedRound(0),
+        wcaApiResults: [],
+        wcaLiveRound: liveRound([
+          liveRoundResult({
+            id: 'live-alice',
+            ranking: 4,
+            best: 900,
+            average: 1000,
+            attempts: [{ result: 900 }, { result: 1000 }, { result: 1100 }],
+            person: {
+              id: 'missing-registrant-id',
+              registrantId: null,
+              wcaUserId: 1001,
+              wcaId: '2010ALIC01',
+              name: 'Alice Published Name',
+            },
+          }),
+        ]),
+      });
+
+      expect(results).toHaveLength(3);
+      expect(results.filter((result) => result.personId === 1)).toHaveLength(1);
+      expect(results.find((result) => result.personId === 1)).toMatchObject({
+        personName: 'Alice Published Name',
+        ranking: 4,
+        best: 900,
+        average: 1000,
+        attempts: [{ result: 900 }, { result: 1000 }, { result: 1100 }],
+      });
+    });
+
+    it('leaves a live result unlinked when its name is ambiguous and identifiers are missing', () => {
+      const duplicateNamePerson: Person = {
+        ...persons[0],
+        registrantId: 4,
+        wcaUserId: 1004,
+        wcaId: '2010ALIC02',
+      };
+      const duplicateNameWcif = {
+        ...wcif,
+        persons: [...persons, duplicateNamePerson],
+      } as Competition;
+      const results = getRoundResultsFromSources({
+        wcif: duplicateNameWcif,
+        selectedRound: selectedRound(0),
+        wcaApiResults: [],
+        wcaLiveRound: liveRound([
+          liveRoundResult({
+            id: 'live-ambiguous-alice',
+            person: {
+              id: 'missing-identifiers',
+              registrantId: null,
+              name: 'Alice Solver',
+            },
+          }),
+        ]),
+      });
+
+      expect(results.find((result) => result.id === 'live-ambiguous-alice')).toMatchObject({
+        personId: null,
+        personName: 'Alice Solver',
+      });
+    });
+
+    it('does not override an unmatched live identity with a name match', () => {
+      const results = getRoundResultsFromSources({
+        wcif,
+        selectedRound: selectedRound(0),
+        wcaApiResults: [],
+        wcaLiveRound: liveRound([
+          liveRoundResult({
+            id: 'live-conflicting-alice',
+            person: {
+              id: 'conflicting-identifiers',
+              registrantId: null,
+              wcaUserId: 9999,
+              wcaId: '2099MISS01',
+              name: 'Alice Solver',
+            },
+          }),
+        ]),
+      });
+
+      expect(results.find((result) => result.id === 'live-conflicting-alice')).toMatchObject({
+        personId: null,
+        personName: 'Alice Solver',
+      });
+    });
+
     it('matches WCA API results by WCA ID before using the displayed name', () => {
       const results = getRoundResultsFromSources({
         wcif,
@@ -479,6 +615,23 @@ describe('resultsProvider', () => {
       ]);
     });
 
+    it('does not include a same-name API result with a conflicting WCA ID', () => {
+      const results = getPersonalResultsFromSources({
+        wcif,
+        person: persons[0],
+        wcaApiResults: [
+          apiResult({
+            id: 9201,
+            name: 'Alice Solver',
+            wca_id: '2010BOBS01',
+          }),
+        ],
+        wcaLiveResults: [],
+      });
+
+      expect(results[0].rounds.map((round) => round.roundId)).toEqual(['333-r1']);
+    });
+
     it('sorts live personal events and rounds before merging them', () => {
       const secondRound = livePersonalResult({
         id: 'live-personal-333-r2',