Merge pull request #2050 from openzim/bug-2046

Always use fallback language strings, in case specified language is missing any keys

src/util/misc.ts

@@ -145,11 +145,17 @@ export function migrateChildren(from: any, to: any, beforeNode: any) {
 
 export function getStringsForLang(language: string, fallbackLanguage = 'en') {
   let strings: { [id: string]: string } = {}
-  try {
-    strings = JSON.parse(fs.readFileSync(path.join(__dirname, `../../translation/${language}.json`)).toString())
-  } catch (err) {
-    logger.warn(`Couldn't find strings file for [${language}], falling back to [${fallbackLanguage}]`)
-    strings = JSON.parse(fs.readFileSync(path.join(__dirname, `../../translation/${fallbackLanguage}.json`)).toString())
+  // Read fallbackLanguage first, so it initially populates the strings. Then, read the primary language file,
+  // overridding default strings with the values from the primary language.
+  for (const lang of [fallbackLanguage, language]) {
+    try {
+      const fileContents = fs.readFileSync(path.join(__dirname, `../../translation/${lang}.json`)).toString()
+      const langStrings = JSON.parse(fileContents)
+      delete langStrings['@metadata']
+      strings = { ...strings, ...langStrings }
+    } catch (err) {
+      logger.warn(`Couldn't find strings file for [${lang}]`)
+    }
   }
   return strings
 }

test/unit/misc.test.ts

@@ -1,4 +1,4 @@
-import { contains, getStrippedTitleFromHtml } from '../../src/util/misc.js'
+import { contains, getStrippedTitleFromHtml, getStringsForLang } from '../../src/util/misc.js'
 import domino from 'domino'
 import { jest } from '@jest/globals'
 
@@ -68,4 +68,42 @@ describe('Misc utility', () => {
       expect(createDocumentSpy).toBeCalledTimes(1) // only last one can't be parsed with regex
     })
   })
+
+  describe('getStringsForLang', () => {
+    test('skips missing files without error', () => {
+      const strings = getStringsForLang('XX', 'XX')
+      expect(strings).toEqual({})
+    })
+
+    test('returns en strings if lang file is missing completely', () => {
+      const strings = getStringsForLang('XX')
+      expect(strings).toEqual({
+        __direction: 'ltr',
+        DISCLAIMER: 'This article is issued from ${creator}. The text is licensed under ${license}. Additional terms may apply for the media files.',
+        LAST_EDITED_ON: 'Last edited on ${date}',
+        LICENSE_NAME: 'Creative Commons - Attribution - Sharealike',
+      })
+    })
+
+    test('falls back to en strings if lang file is missing certain fields', () => {
+      const strings = getStringsForLang('fi')
+      expect(strings).toEqual({
+        __direction: 'ltr',
+        DISCLAIMER: 'This article is issued from ${creator}. The text is licensed under ${license}. Additional terms may apply for the media files.',
+        LAST_EDITED_ON: 'Viimeksi muokattu ${date}',
+        LICENSE_NAME: 'Creative Commons - Nimeä - JaaSamoin',
+      })
+    })
+
+    test('falls back to specified fallback language', () => {
+      const strings = getStringsForLang('XX', 'de')
+      console.log(JSON.stringify(strings))
+      expect(strings).toEqual({
+        DISCLAIMER:
+          'Dieser Artikel wurde von ${creator} herausgegeben. Der Text ist als ${license} lizenziert. Möglicherweise können weitere Bestimmungen für Mediendateien gelten.',
+        LAST_EDITED_ON: 'Zuletzt bearbeitet am ${date}',
+        LICENSE_NAME: 'Creative Commons - Attribution - Sharealike',
+      })
+    })
+  })
 })