From 564f76584bf1b3ec853576879e550d148440de97 Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Fri, 3 Jul 2020 13:02:35 -0300
Subject: [PATCH] Tolerate missing or falsey substitutions (#8907)

Previously the `getMessage` function would throw if a substitution was
falsey. Now it will accept any substitution, including `undefined`.

A substitution of `null` or `undefined` will still be reported to
Sentry and printed to the console as an error, but it will not
interrupt execution. Any `null` or `undefined` substitutions will be
rendered as empty strings.

Ideally we'd never pass in `null` or `undefined` as a substitution, but
in practice this sometimes just occurs breifly between renders, which
isn't a severe enough problem to justify crashing the UI.

The detection of React component substitutions has been updated as
well, to ensure that `null` values aren't counted as React
substitutions.
---
 ui/app/helpers/utils/i18n-helper.js      | 10 ++++++----
 ui/app/helpers/utils/i18n-helper.test.js | 16 ++++++++--------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/ui/app/helpers/utils/i18n-helper.js b/ui/app/helpers/utils/i18n-helper.js
index 01a369229..a32120b4f 100644
--- a/ui/app/helpers/utils/i18n-helper.js
+++ b/ui/app/helpers/utils/i18n-helper.js
@@ -43,7 +43,7 @@ export const getMessage = (localeCode, localeMessages, key, substitutions) => {
 
   const hasSubstitutions = Boolean(substitutions && substitutions.length)
   const hasReactSubstitutions = hasSubstitutions &&
-    substitutions.some((element) => typeof element === 'function' || typeof element === 'object')
+    substitutions.some((element) => element !== null && (typeof element === 'function' || typeof element === 'object'))
 
   // perform substitutions
   if (hasSubstitutions) {
@@ -55,10 +55,12 @@ export const getMessage = (localeCode, localeMessages, key, substitutions) => {
         return part
       }
       const substituteIndex = Number(subMatch[1]) - 1
-      if (substitutions[substituteIndex]) {
-        return substitutions[substituteIndex]
+      if (substitutions[substituteIndex] == null) {
+        const error = new Error(`Insufficient number of substitutions for message: '${phrase}'`)
+        log.error(error)
+        Sentry.captureException(error)
       }
-      throw new Error(`Insufficient number of substitutions for message: '${phrase}'`)
+      return substitutions[substituteIndex]
     })
 
     phrase = hasReactSubstitutions
diff --git a/ui/app/helpers/utils/i18n-helper.test.js b/ui/app/helpers/utils/i18n-helper.test.js
index 2832022e6..4531865ab 100644
--- a/ui/app/helpers/utils/i18n-helper.test.js
+++ b/ui/app/helpers/utils/i18n-helper.test.js
@@ -132,14 +132,14 @@ describe('i18n helper', function () {
       assert.equal(result, `${TEST_SUBSTITUTION_1} - ${TEST_SUBSTITUTION_2} - ${TEST_SUBSTITUTION_3} - ${TEST_SUBSTITUTION_4} - ${TEST_SUBSTITUTION_5}`)
     })
 
-    it('should throw an error when not passed as many substitutions as a message requires', function () {
-      assert.throws(
-        () => {
-          t(TEST_KEY_5, [ TEST_SUBSTITUTION_1, TEST_SUBSTITUTION_2 ])
-        },
-        Error,
-        `Insufficient number of substitutions for message: '$1 - $2 - $3'`
-      )
+    it('should correctly render falsey substitutions', function () {
+      const result = t(TEST_KEY_4, [ 0, -0, '', false, NaN ])
+      assert.equal(result, '0 - 0 -  - false - NaN')
+    })
+
+    it('should render nothing for "null" and "undefined" substitutions', function () {
+      const result = t(TEST_KEY_5, [ null, TEST_SUBSTITUTION_2 ])
+      assert.equal(result, ` - ${TEST_SUBSTITUTION_2} - `)
     })
 
     it('should return the correct message when a single react substitution is made', function () {