fallback properly with pluralized strings (#7495)

Signed-off-by: Kerry Archibald <[email protected]>
matrix-org · Jan 10, 2022 · 5cfb046 · 5cfb046
1 parent 3680859
commit 5cfb046
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 27 deletions.
diff --git a/src/languageHandler.tsx b/src/languageHandler.tsx
@@ -41,6 +41,7 @@ counterpart.setSeparator('|');
 
 // see `translateWithFallback` for an explanation of fallback handling
 const FALLBACK_LOCALE = 'en';
+counterpart.setFallbackLocale(FALLBACK_LOCALE);
 
 interface ITranslatableError extends Error {
     translatedMessage: string;
@@ -79,12 +80,12 @@ export function _td(s: string): string { // eslint-disable-line @typescript-esli
  * should be wrapped with an appropriate `lang='en'` attribute
  * counterpart's `translate` doesn't expose a way to determine if the resulting translation
  * is in the target locale or a fallback locale
- * for this reason, we do not set a fallback via `counterpart.setFallbackLocale`
+ * for this reason, force fallbackLocale === locale in the first call to translate
  * and fallback 'manually' so we can mark fallback strings appropriately
  * */
 const translateWithFallback = (text: string, options?: object): { translated?: string, isFallback?: boolean } => {
-    const translated = counterpart.translate(text, options);
-    if (/^missing translation:/.test(translated)) {
+    const translated = counterpart.translate(text, { ...options, fallbackLocale: counterpart.getLocale() });
+    if (!translated || /^missing translation:/.test(translated)) {
         const fallbackTranslated = counterpart.translate(text, { ...options, locale: FALLBACK_LOCALE });
         return { translated: fallbackTranslated, isFallback: true };
     }

diff --git a/test/i18n-test/languageHandler-test.tsx b/test/i18n-test/languageHandler-test.tsx
@@ -118,7 +118,7 @@ describe('languageHandler', function() {
         });
     });
 
-    describe('when a translation string does not exist in active language', () => {
+    describe('for a non-en language', () => {
         beforeEach(async () => {
             stubClient();
             await setLanguage('lv');
@@ -130,33 +130,75 @@ describe('languageHandler', function() {
             setMissingEntryGenerator(counterpartDefaultMissingEntryGen);
         });
 
+        // mocked lv has only `"Uploading %(filename)s and %(count)s others|one"`
         const lvExistingPlural = 'Uploading %(filename)s and %(count)s others';
+        const lvNonExistingPlural = '%(spaceName)s and %(count)s others';
 
-        // lv does not have a pluralizer function
-        const noPluralizerCase = [
-            'handles plural strings when no pluralizer exists for language',
-            lvExistingPlural,
-            { count: 1, filename: 'test.txt' },
-            undefined,
-            'Uploading test.txt and 1 other',
-        ] as TestCase;
-
-        describe('_t', () => {
-            it.each([...testCasesEn, noPluralizerCase])(
-                "%s and translates with fallback locale",
-                async (_d, translationString, variables, tags, result) => {
-                    expect(_t(translationString, variables, tags)).toEqual(result);
-                },
-            );
+        describe('pluralization', () => {
+            const pluralCases = [
+                [
+                    'falls back when plural string exists but not for for count',
+                    lvExistingPlural,
+                    { count: 2, filename: 'test.txt' },
+                    undefined,
+                    'Uploading test.txt and 2 others',
+                ],
+                [
+                    'falls back when plural string does not exists at all',
+                    lvNonExistingPlural,
+                    { count: 2, spaceName: 'test' },
+                    undefined,
+                    'test and 2 others',
+                ],
+            ] as TestCase[];
+
+            describe('_t', () => {
+                it('translated correctly when plural string exists for count', () => {
+                    expect(_t(
+                        lvExistingPlural,
+                        { count: 1, filename: 'test.txt' }, undefined)).toEqual('Качване на test.txt и 1 друг');
+                });
+                it.each(pluralCases)(
+                    "%s",
+                    async (_d, translationString, variables, tags, result) => {
+                        expect(_t(translationString, variables, tags)).toEqual(result);
+                    },
+                );
+            });
+
+            describe('_tDom()', () => {
+                it('translated correctly when plural string exists for count', () => {
+                    expect(_tDom(
+                        lvExistingPlural,
+                        { count: 1, filename: 'test.txt' }, undefined)).toEqual('Качване на test.txt и 1 друг');
+                });
+                it.each(pluralCases)(
+                    "%s and translates with fallback locale, attributes fallback locale",
+                    async (_d, translationString, variables, tags, result) => {
+                        expect(_tDom(translationString, variables, tags)).toEqual(<span lang="en">{ result }</span>);
+                    },
+                );
+            });
         });
 
-        describe('_tDom()', () => {
-            it.each([...testCasesEn, noPluralizerCase])(
-                "%s and translates with fallback locale, attributes fallback locale",
-                async (_d, translationString, variables, tags, result) => {
-                    expect(_tDom(translationString, variables, tags)).toEqual(<span lang="en">{ result }</span>);
-                },
-            );
+        describe('when a translation string does not exist in active language', () => {
+            describe('_t', () => {
+                it.each(testCasesEn)(
+                    "%s and translates with fallback locale",
+                    async (_d, translationString, variables, tags, result) => {
+                        expect(_t(translationString, variables, tags)).toEqual(result);
+                    },
+                );
+            });
+
+            describe('_tDom()', () => {
+                it.each(testCasesEn)(
+                    "%s and translates with fallback locale, attributes fallback locale",
+                    async (_d, translationString, variables, tags, result) => {
+                        expect(_tDom(translationString, variables, tags)).toEqual(<span lang="en">{ result }</span>);
+                    },
+                );
+            });
         });
     });
 });