[🔥AUDIT🔥] Fix up plural form fallback when no translation is present. (#2374)

🖍 _This is an audit!_ 🖍

## Summary:
I may have introduced this in my last fix, actually - but this now helps us ensure that we don't accidentally use the other language pluralization rules when we are falling back to English strings.

Issue: FEI-6007

## Test plan:
The tests pass!

Author: jeresig

Auditors: #wonder-blocks

Required Reviewers:

Approved By:

Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ⏭️  dependabot

Pull Request URL: #2374

Author: jeresig

.changeset/dry-kangaroos-boil.md

@@ -0,0 +1,5 @@
+---
+"@khanacademy/wonder-blocks-i18n": patch
+---
+
+Fix a bug with pluralization for fallback untranslated languages.

packages/wonder-blocks-i18n/src/functions/__tests__/i18n-store.test.ts

@@ -84,10 +84,10 @@ describe("getPluralTranslation", () => {
         // Act
         const result = getPluralTranslation(
             {
-                lang: TEST_LOCALE,
+                lang: "en",
                 messages: ["test singular", "test plural"],
             },
-            0,
+            1,
         );
 
         // Assert
@@ -104,10 +104,10 @@ describe("getPluralTranslation", () => {
         // Act
         const result = getPluralTranslation(
             {
-                lang: TEST_LOCALE,
+                lang: "en",
                 messages: ["test singular", "test plural"],
             },
-            1,
+            2,
         );
 
         // Assert
@@ -135,10 +135,10 @@ describe("getPluralTranslation", () => {
         // Act
         const result = getPluralTranslation(
             {
-                lang: TEST_LOCALE,
+                lang: "en",
                 messages: ["test singular", "test plural"],
             },
-            0,
+            1,
         );
 
         // Assert

packages/wonder-blocks-i18n/src/functions/__tests__/i18n.test.ts

@@ -142,6 +142,17 @@ describe("i18n", () => {
             expect(result).toMatchInlineSnapshot(`"arrrr mateys"`);
         });
 
+        it("ngettext should handle missing translations", () => {
+            // Arrange
+            jest.spyOn(Locale, "getLocale").mockImplementation(() => "ru");
+
+            // Act
+            const result = ngettext("Singular", "Plural", 0);
+
+            // Assert
+            expect(result).toMatchInlineSnapshot(`"Plural"`);
+        });
+
         it("doNotTranslate should not translate", () => {
             // Arrange
             loadTranslations(TEST_LOCALE, {

packages/wonder-blocks-i18n/src/functions/i18n-store.ts

@@ -5,8 +5,11 @@
  */
 import FakeTranslate from "./i18n-faketranslate";
 import {getLocale} from "./locale";
+import {allPluralForms} from "./plural-forms";
 import {PluralConfigurationObject} from "./types";
 
+type Language = keyof typeof allPluralForms;
+
 // The cache of strings that have been translated, by locale.
 const localeMessageStore = new Map<
     string,
@@ -18,6 +21,20 @@ const localeMessageStore = new Map<
 // Create a fake translate object to use if we can't find a translation.
 const {translate: fakeTranslate} = new FakeTranslate();
 
+/*
+ * Return the ngettext position that matches the given number and lang.
+ *
+ * Arguments:
+ *  - num: The number upon which to toggle the plural forms.
+ *  - lang: The language to use as the basis for the pluralization.
+ */
+export const ngetpos = function (num: number, lang?: Language): number {
+    const pluralForm = (lang && allPluralForms[lang]) || allPluralForms["en"];
+    const pos = pluralForm(num);
+    // Map true to 1 and false to 0, keep any numeric return value the same.
+    return pos === true ? 1 : pos ? pos : 0;
+};
+
 /**
  * Get the translation for a given id and locale.
  *
@@ -89,12 +106,15 @@ export const getSingularTranslation = (
  */
 export const getPluralTranslation = (
     pluralConfig: PluralConfigurationObject,
-    idx: number,
+    num: number,
 ) => {
     const {lang, messages} = pluralConfig;
 
     // We try to find the translation in the cache.
-    const translatedMessages = getTranslationFromStore(messages[0], lang);
+    const translatedMessages = getTranslationFromStore(
+        messages[0],
+        getLocale(),
+    );
 
     // We found the translation so we can return the right plural form.
     if (translatedMessages) {
@@ -103,10 +123,13 @@ export const getPluralTranslation = (
             // just in case.
             return translatedMessages;
         }
+        // Get the translated string
+        const idx = ngetpos(num, getLocale());
         return translatedMessages[idx];
     }
 
     // Otherwise, there's no translation, so we try to do fake translation.
+    const idx = ngetpos(num, lang);
     return fakeTranslate(messages[idx]);
 };
 

packages/wonder-blocks-i18n/src/functions/i18n.ts

@@ -3,7 +3,6 @@
 /* To fix, remove an entry above, run ka-lint, and fix errors. */
 import * as React from "react";
 
-import {allPluralForms} from "./plural-forms";
 import {getLocale} from "./locale";
 import {PluralConfigurationObject} from "./types";
 import {getPluralTranslation, getSingularTranslation} from "./i18n-store";
@@ -47,8 +46,6 @@ interface _Overloads {
     ): string;
 }
 
-type Language = keyof typeof allPluralForms;
-
 const interpolationMarker = /%\(([\w_]+)\)s/g;
 
 /**
@@ -210,7 +207,7 @@ export const ngettext: ngettextOverloads = (
         typeof singular === "object"
             ? singular
             : {
-                  lang: getLocale(),
+                  lang: "en",
                   // We know plural is a string if singular is not a config object
                   messages: [singular, plural as any],
               };
@@ -220,11 +217,7 @@ export const ngettext: ngettextOverloads = (
     const actualOptions: NGetOptions =
         (typeof singular === "object" ? num : (options as any)) || {};
 
-    // Get the translated string
-    const idx = ngetpos(actualNum, pluralConfObj.lang);
-
-    // The common (non-error) case is messages[idx].
-    const translation = getPluralTranslation(pluralConfObj, idx);
+    const translation = getPluralTranslation(pluralConfObj, actualNum);
 
     // Get the options to substitute into the string.
     // We automatically add in the 'magic' option-variable 'num'.
@@ -245,20 +238,6 @@ const formatNumber = (num: number): string => {
     return Intl.NumberFormat(getLocale()).format(num);
 };
 
-/*
- * Return the ngettext position that matches the given number and lang.
- *
- * Arguments:
- *  - num: The number upon which to toggle the plural forms.
- *  - lang: The language to use as the basis for the pluralization.
- */
-export const ngetpos = function (num: number, lang?: Language): number {
-    const pluralForm = (lang && allPluralForms[lang]) || allPluralForms["en"];
-    const pos = pluralForm(num);
-    // Map true to 1 and false to 0, keep any numeric return value the same.
-    return pos === true ? 1 : pos ? pos : 0;
-};
-
 /*
  * A dummy identity function.  It's used as a signal to automatic
  * translation-identification tools that they shouldn't mark this

packages/wonder-blocks-i18n/src/index.ts

@@ -4,9 +4,12 @@ export {
     ngettext,
     doNotTranslate,
     doNotTranslateYet, // used by handlebars translation functions in webapp
-    ngetpos,
 } from "./functions/i18n";
-export {loadTranslations, clearTranslations} from "./functions/i18n-store";
+export {
+    loadTranslations,
+    clearTranslations,
+    ngetpos,
+} from "./functions/i18n-store";
 
 export {localeToFixed, getDecimalSeparator} from "./functions/l10n";
 export {getLocale, setLocale} from "./functions/locale";