Merge pull request #154 from sillsdev/BL-15916-retired-iso-code-duplicates

fix: don't create duplicate languages from retired ISO 639-3 codes (BL-15916)

Author: hatton

components/language-chooser/common/find-language/language-data/languageData.json

@@ -622,3 +622,65 @@ describe("canonical tag is first in alternativeTags", () => {
     expect(ojg?.alternativeTags[0].split("-")[0]).toBe("oj");
   });
 });
+
+// langtags.json sometimes keeps a code that ISO has retired in an entry's iso639_3 field even
+// though the entry's own tag already uses the surviving code, e.g. tag "enm-Latn-IE" with
+// iso639_3 "yol". Such an entry must fold into the surviving code rather than becoming a
+// duplicate language of its own. See the comment in scripts/langtagProcessing.ts and BL-15916.
+describe("entries whose langtags iso639_3 code has been retired by ISO", () => {
+  // [retired code, surviving code the tag uses, exonym of the surviving language]
+  const retiredToSurviving = [
+    ["yol", "enm", "Middle English (1100-1500)"], // Yola; the surviving code is a *different* language
+    ["kpp", "jkp", "Paku Karen"],
+    ["jeg", "oyb", "Oy"],
+    ["dek", "sqm", "Suma"],
+    ["tpw", "tpn", "Tupinambá"],
+  ];
+
+  it("should fold into the surviving language instead of becoming a duplicate, and stay findable by the retired code", () => {
+    for (const [retired, surviving, exonym] of retiredToSurviving) {
+      expect(
+        getLanguageBySubtag(retired),
+        `${retired} is retired and should not be a language entry of its own`
+      ).toBeUndefined();
+
+      const survivor = getLanguageBySubtag(surviving);
+      expect(survivor, `${surviving} should be a language entry`).toBeDefined();
+      expect(survivor?.exonym).toBe(exonym);
+      expect(
+        survivor?.alternativeTags,
+        `the retired tag ${retired} should have folded into ${surviving}`
+      ).toContain(retired);
+      // Note this is the raw search. For yol/enm in particular the surviving language is then
+      // filtered out by defaultSearchResultModifier (enm is on the historic-language exclusion
+      // list), so a user searching "yol" sees nothing - that is intended, see the next test.
+      expect(
+        searchForLanguage(retired).some((result) =>
+          codeMatches(result.iso639_3_code, surviving)
+        ),
+        `the raw search for retired code ${retired} should still reach ${surviving}`
+      ).toBe(true);
+    }
+  });
+
+  it("should not offer Yola's entry as a selectable Middle English card", () => {
+    // Regression guard for the user-visible symptom: langtags' "enm-Latn-IE" entry (iso639_3
+    // "yol") used to produce a card reading "Middle English (1100-1500) - A language of Ireland"
+    // carrying the subtag enm. That mislabelled Yola, would have written the tag enm onto a
+    // collection, and slipped past the deliberate enm exclusion in
+    // defaultExcludedHistoricLanguages.ts because the entry was keyed "yol".
+    const searchString = "Middle English";
+    const results = defaultSearchResultModifier(
+      searchForLanguage(searchString),
+      searchString
+    );
+    // The yol assertion is the one carrying the regression guard; enm is additionally covered by
+    // the historic-language exclusion, so assert both to pin the whole user-visible outcome.
+    for (const code of ["yol", "enm"]) {
+      expect(
+        results.some((result) => codeMatches(result.iso639_3_code, code)),
+        `${code} should not be offered as a language option`
+      ).toBe(false);
+    }
+  });
+});

components/language-chooser/common/find-language/languageSearch.spec.ts

@@ -15,6 +15,7 @@ import {
   stripMacrolanguageParenthetical,
   stripMacrolanguageParentheticalFromAll,
   iso639_1To639_3,
+  isCurrentIsoCode,
 } from "./langtagProcessingHelpers";
 
 import fs from "fs";
@@ -142,6 +143,36 @@ function parseLangtagsJson() {
       }
     }
 
+    // langtags.json sometimes keeps a code ISO has since retired in the iso639_3 field even
+    // though the entry's own tag already uses the surviving code, e.g. tag "enm-Latn-IE" with
+    // iso639_3 "yol", or tag "jkp-Zyyy-MM" with iso639_3 "kpp". Filing such an entry under the
+    // retired code creates a second, duplicate card for a language we already list under its
+    // current code. Worse, when the surviving code belongs to a *different* language the
+    // duplicate also inherits the wrong name and subtag: "yol" (Yola) was showing up as
+    // "Middle English (1100-1500) - A language of Ireland" with subtag "enm", which would write
+    // the tag enm onto a collection and slipped past the deliberate enm historic-language
+    // exclusion (see defaultExcludedHistoricLanguages.ts). So when the iso639_3 code is no
+    // longer in iso-639-3.tab but the tag's language subtag is, trust the tag. Entries whose
+    // retired code matches their own tag (e.g. "aoh"/Arma) are left alone - there is no
+    // surviving code to fold them into. See BL-15916.
+    //
+    // Caveat for whoever regenerates this data next: addOrCombineLangtagsEntry does not
+    // overwrite `exonym` on the combine path, so a folded pair keeps whichever entry langtags
+    // happened to list first. For all five current pairs both entries carry the same `name`, so
+    // it makes no difference. But if a future retired/surviving pair had *differing* names and
+    // langtags listed the retired one first, the merged card would inherit the retired
+    // language's name - the very symptom this block exists to prevent. Worth re-checking the
+    // folded entries' exonyms after a langtags update rather than assuming ordering holds.
+    const subtagIso639_3 = iso639_1To639_3[languageSubtag] || languageSubtag;
+    if (
+      augmentedEntry.iso639_3 &&
+      subtagIso639_3 !== augmentedEntry.iso639_3 &&
+      !isCurrentIsoCode(augmentedEntry.iso639_3) &&
+      isCurrentIsoCode(subtagIso639_3)
+    ) {
+      augmentedEntry.iso639_3 = subtagIso639_3;
+    }
+
     // If listed with a macrolanguage code, this is a "representative language", we need to identify it by its equivalent
     // individual language code. See macrolanguageNotes.md
     if (isMacrolanguage(entry.iso639_3) || isMacrolanguage(languageSubtag)) {

components/language-chooser/common/find-language/scripts/langtagProcessing.ts

@@ -246,6 +246,13 @@ export function isMacrolanguage(iso639_3: string) {
   return isoCodesDetails[iso639_3]?.isMacrolanguage || false;
 }
 
+// Is this code listed in the current iso-639-3.tab? Codes that ISO has retired are not,
+// even though langtags.json may still refer to them. (Accepts either an ISO 639-1 or an
+// ISO 639-3 code, since isoCodesDetails is keyed by both.)
+export function isCurrentIsoCode(code: string | undefined) {
+  return !!code && !!isoCodesDetails[code];
+}
+
 //  Internal helper used by langtagProcessing.ts for data cleaning. Assumes `langtag` is a canonical BCP-47 tag
 function defaultScriptForLanguage(
   languageTag: string,

components/language-chooser/common/find-language/scripts/langtagProcessingHelpers.ts

@@ -24,19 +24,28 @@ export function languageCardTestId(languageCode: string) {
 // time — letting lazyload mount the newly-visible cards — until the requested card exists, then
 // brings it into view. Use this instead of a bare scrollIntoViewIfNeeded when the target card may
 // be below the initially-rendered window (e.g. a fuzzy match that isn't near the top).
+// Results also arrive in batches, so reaching the bottom of the list does NOT mean the card is
+// absent — more results may still be on their way. Sweeping only once and giving up at the bottom
+// made this flaky under load, so keep sweeping from the top until the card mounts or we run out of
+// time, and let the caller's expect() report the failure if it never does.
+//
+// Consequence: this is for asserting a card IS there. Don't use it to assert a card is absent —
+// it deliberately burns the full timeout below before returning an empty locator.
+const SCROLL_FOR_CARD_TIMEOUT_MS = 15000;
 export async function scrollListToLanguageCard(page, isoCode: string) {
   const card = page.getByTestId(languageCardTestId(isoCode));
   const list = page.locator("#language-card-list");
-  for (let i = 0; i < 40; i++) {
-    if ((await card.count()) > 0) break;
-    const movedDown = await list.evaluate((el: HTMLElement) => {
+  const giveUpAt = Date.now() + SCROLL_FOR_CARD_TIMEOUT_MS;
+  while ((await card.count()) === 0 && Date.now() < giveUpAt) {
+    const reachedBottom = await list.evaluate((el: HTMLElement) => {
       const before = el.scrollTop;
       el.scrollBy(0, Math.max(1, el.clientHeight - 40));
-      return el.scrollTop > before;
+      if (el.scrollTop > before) return false;
+      el.scrollTop = 0; // at the bottom; sweep again in case more results have arrived since
+      return true;
     });
     // Let react-lazyload (and any still-streaming search results) render the newly-visible cards.
-    await page.waitForTimeout(150);
-    if (!movedDown) break; // reached the bottom of the list
+    await page.waitForTimeout(reachedBottom ? 300 : 150);
   }
   await card.scrollIntoViewIfNeeded();
   return card;