Fall back to 'other' if a plural form is missing

app/views/examples/translated/index.njk

@@ -1,3 +1,5 @@
+{% set htmlLang = 'cy-GB' %}
+
 {% from "accordion/macro.njk" import govukAccordion %}
 {% from "back-link/macro.njk" import govukBackLink %}
 {% from "breadcrumbs/macro.njk" import govukBreadcrumbs %}

src/govuk/components/character-count/character-count.test.js

@@ -586,4 +586,41 @@ describe('Character count', () => {
       })
     })
   })
+
+  describe('in mismatched locale', () => {
+    it('does not error', async () => {
+      // Create a listener for the page error event that we can assert on later
+      const pageErrorListener = jest.fn()
+      page.on('pageerror', pageErrorListener)
+
+      await renderAndInitialise(page, 'character-count', {
+        nunjucksParams: examples.default,
+        config: {
+          // Override maxlength to 10
+          maxlength: 10
+        },
+        initialiser: function ({ config }) {
+          const $component = document.querySelector('[data-module]')
+
+          // Set locale to Welsh, which expects translations for 'one', 'two',
+          // 'few' 'many' and 'other' forms – with the default English strings
+          // provided we only have translations for 'one' and 'other'.
+          //
+          // We want to make sure we handle this gracefully in case users have
+          // an existing character count inside an incorrect locale.
+          $component.setAttribute('lang', 'cy')
+          new window.GOVUKFrontend.CharacterCount($component, config).init()
+        }
+      })
+
+      // Type 10 characters so we go 'through' all the different forms as we
+      // approach 0 characters remaining.
+      await page.type('.govuk-js-character-count', 'A'.repeat(10), {
+        delay: keyupWaitTime
+      })
+
+      // Expect the page error event not to have been fired
+      expect(pageErrorListener).not.toHaveBeenCalled()
+    })
+  })
 })

src/govuk/i18n.mjs

@@ -35,15 +35,7 @@ I18n.prototype.t = function (lookupKey, options) {
   // falsy, as this could legitimately be 0.
   if (options && typeof options.count !== 'undefined') {
     // Get the plural suffix
-    var pluralSuffix = this.getPluralSuffix(options.count)
-
-    // Overwrite our existing lookupKey
-    lookupKey = lookupKey + '.' + pluralSuffix
-
-    // Throw an error if this new key doesn't exist
-    if (!(lookupKey in this.translations)) {
-      throw new Error('i18n: Plural form ".' + pluralSuffix + '" is required for "' + this.locale + '" locale')
-    }
+    lookupKey = lookupKey + '.' + this.getPluralSuffix(lookupKey, options.count)
   }
 
   if (lookupKey in this.translations) {
@@ -138,37 +130,66 @@ I18n.prototype.hasIntlNumberFormatSupport = function () {
 /**
  * Get the appropriate suffix for the plural form.
  *
+ * Uses Intl.PluralRules (or our own fallback implementation) to get the
+ * 'preferred' form to use for the given count.
+ *
+ * Checks that a translation has been provided for that plural form – if it
+ * hasn't, it'll fall back to the 'other' plural form (unless that doesn't exist
+ * either, in which case an error will be thrown)
+ *
+ * @param {string} lookupKey - The lookup key of the string to use.
  * @param {number} count - Number used to determine which pluralisation to use.
  * @returns {PluralRule} The suffix associated with the correct pluralisation for this locale.
  */
-I18n.prototype.getPluralSuffix = function (count) {
+I18n.prototype.getPluralSuffix = function (lookupKey, count) {
   // Validate that the number is actually a number.
   //
   // Number(count) will turn anything that can't be converted to a Number type
   // into 'NaN'. isFinite filters out NaN, as it isn't a finite number.
   count = Number(count)
   if (!isFinite(count)) { return 'other' }
 
+  var preferredForm
+
   // Check to verify that all the requirements for Intl.PluralRules are met.
   // If so, we can use that instead of our custom implementation. Otherwise,
   // use the hardcoded fallback.
   if (this.hasIntlPluralRulesSupport()) {
-    return new Intl.PluralRules(this.locale).select(count)
+    preferredForm = new Intl.PluralRules(this.locale).select(count)
   } else {
-    return this.selectPluralRuleFromFallback(count)
+    preferredForm = this.selectPluralFormUsingFallbackRules(count)
+  }
+
+  // Use the correct plural form if provided
+  if (lookupKey + '.' + preferredForm in this.translations) {
+    return preferredForm
+  // Fall back to `other` if the plural form is missing, but log a warning
+  // to the console
+  } else if (lookupKey + '.other' in this.translations) {
+    if (console && 'warn' in console) {
+      console.warn('i18n: Missing plural form ".' + preferredForm + '" for "' +
+        this.locale + '" locale. Falling back to ".other".')
+    }
+
+    return 'other'
+  // If the required `other` plural form is missing, all we can do is error
+  } else {
+    throw new Error(
+      'i18n: Plural form ".other" is required for "' + this.locale + '" locale'
+    )
   }
 }
 
 /**
- * Get the plural rule using our fallback implementation
+ * Get the plural form using our fallback implementation
  *
  * This is split out into a separate function to make it easier to test the
  * fallback behaviour in an environment where Intl.PluralRules exists.
  *
  * @param {number} count - Number used to determine which pluralisation to use.
- * @returns {PluralRule} The suffix associated with the correct pluralisation for this locale.
+ * @returns {PluralRule} The pluralisation form for count in this locale.
  */
-I18n.prototype.selectPluralRuleFromFallback = function (count) {
+I18n.prototype.selectPluralFormUsingFallbackRules = function (count) {
   // Currently our custom code can only handle positive integers, so let's
   // make sure our number is one of those.
   count = Math.abs(Math.floor(count))

src/govuk/i18n.unit.test.mjs

@@ -150,15 +150,6 @@ describe('I18n', () => {
     })
 
     describe('pluralisation', () => {
-      it('throws an error if a required plural form is not provided ', () => {
-        const i18n = new I18n({
-          'test.other': 'testing testing'
-        }, {
-          locale: 'en'
-        })
-        expect(() => { i18n.t('test', { count: 1 }) }).toThrowError('i18n: Plural form ".one" is required for "en" locale')
-      })
-
       it('interpolates the count variable into the correct plural form', () => {
         const i18n = new I18n({
           'test.one': '%{count} test',
@@ -173,6 +164,121 @@ describe('I18n', () => {
     })
   })
 
+  describe('.getPluralSuffix', () => {
+    let consoleWarn
+
+    beforeEach(() => {
+      // Silence warnings in test output, and allow us to 'expect' them
+      consoleWarn = jest.spyOn(global.console, 'warn')
+        .mockImplementation(() => { /* noop */ })
+    })
+
+    afterEach(() => {
+      jest.restoreAllMocks()
+    })
+
+    it('uses `Intl.PluralRules` when available', () => {
+      const IntlPluralRulesSelect = jest.spyOn(global.Intl.PluralRules.prototype, 'select')
+        .mockImplementation(() => 'one')
+
+      const i18n = new I18n({
+        'test.one': 'test',
+        'test.other': 'test'
+      }, {
+        locale: 'en'
+      })
+
+      expect(i18n.getPluralSuffix('test', 1)).toBe('one')
+      expect(IntlPluralRulesSelect).toBeCalledWith(1)
+    })
+
+    it('falls back to internal fallback rules', () => {
+      const i18n = new I18n({
+        'test.one': 'test',
+        'test.other': 'test'
+      }, {
+        locale: 'en'
+      })
+
+      jest.spyOn(i18n, 'hasIntlPluralRulesSupport')
+        .mockImplementation(() => false)
+
+      const selectPluralFormUsingFallbackRules = jest.spyOn(i18n, 'selectPluralFormUsingFallbackRules')
+
+      i18n.getPluralSuffix('test', 1)
+      expect(selectPluralFormUsingFallbackRules).toBeCalledWith(1)
+    })
+
+    it('returns the preferred plural form for the locale if a translation exists', () => {
+      const i18n = new I18n({
+        'test.one': 'test',
+        'test.other': 'test'
+      }, {
+        locale: 'en'
+      })
+      expect(i18n.getPluralSuffix('test', 1)).toBe('one')
+    })
+
+    it.each([
+      { form: 'one', count: 1 },
+      { form: 'two', count: 2 },
+      { form: 'few', count: 3 },
+      { form: 'many', count: 6 }
+    ])('`$form` falls back to `other` if preferred form `$form` is missing', ({ count }) => {
+      const i18n = new I18n({
+        'test.other': 'test'
+      }, {
+        locale: 'cy'
+      })
+
+      expect(i18n.getPluralSuffix('test', count)).toBe('other')
+    })
+
+    it('logs a console warning when falling back to `other`', () => {
+      const i18n = new I18n({
+        'test.other': 'test'
+      }, {
+        locale: 'en'
+      })
+
+      i18n.getPluralSuffix('test', 1)
+
+      expect(consoleWarn).toHaveBeenCalledWith(
+        'i18n: Missing plural form ".one" for "en" locale. Falling back to ".other".'
+      )
+    })
+
+    it('throws an error if trying to use `other` but `other` is not provided', () => {
+      const i18n = new I18n({}, {
+        locale: 'en'
+      })
+
+      expect(() => { i18n.getPluralSuffix('test', 2) })
+        .toThrowError('i18n: Plural form ".other" is required for "en" locale')
+    })
+
+    it('throws an error if a plural form is not provided and neither is `other`', () => {
+      const i18n = new I18n({
+        'test.one': 'test'
+      }, {
+        locale: 'en'
+      })
+
+      expect(() => { i18n.getPluralSuffix('test', 2) })
+        .toThrowError('i18n: Plural form ".other" is required for "en" locale')
+    })
+
+    it('returns `other` for non-numbers', () => {
+      const i18n = new I18n({
+        'test.other': 'test'
+      }, {
+        locale: 'en'
+      })
+
+      expect(i18n.getPluralSuffix('test', 'nonsense')).toBe('other')
+    })
+  })
+
   describe('.getPluralRulesForLocale', () => {
     it('returns the correct rules for a locale in the map', () => {
       const locale = 'ar'
@@ -193,7 +299,7 @@ describe('I18n', () => {
     })
   })
 
-  describe('.selectPluralRuleFromFallback', () => {
+  describe('.selectPluralFormUsingFallbackRules', () => {
     // The locales we want to test, with numbers for any 'special cases' in
     // those locales we want to ensure are handled correctly
     const locales = [
@@ -215,7 +321,7 @@ describe('I18n', () => {
       const numbersToTest = [0, 1, 2, 5, 25, 100, ...localeNumbers]
 
       numbersToTest.forEach(num => {
-        expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num))
+        expect(i18n.selectPluralFormUsingFallbackRules(num)).toBe(intl.select(num))
       })
     })
 
@@ -226,7 +332,7 @@ describe('I18n', () => {
       const numbersToTest = [0, 1, 2, 5, 25, 100]
 
       numbersToTest.forEach(num => {
-        expect(i18n.selectPluralRuleFromFallback(num)).toBe('other')
+        expect(i18n.selectPluralFormUsingFallbackRules(num)).toBe('other')
       })
     })
   })