fix(helpers): ent-4366 optional chaining for numbro (#832)

RedHatInsights · Dec 6, 2021 · 33daf62 · 33daf62
1 parent 9a51b9b
commit 33daf62
Show file tree

Hide file tree

Showing 13 changed files with 318 additions and 216 deletions.
diff --git a/src/common/__tests__/__snapshots__/helpers.test.js.snap b/src/common/__tests__/__snapshots__/helpers.test.js.snap
@@ -1,5 +1,11 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`Helpers should apply a number display function: number display function result 1`] = `
+t {
+  "_value": 11,
+}
+`;
+
 exports[`Helpers should expose a window object: limited window object 1`] = `
 Object {
   "DEV_MODE": false,
@@ -35,6 +41,7 @@ Object {
   "noop": [Function],
   "noopPromise": Promise {},
   "noopTranslate": [Function],
+  "numberDisplay": [Function],
 }
 `;
 
@@ -73,6 +80,7 @@ Object {
   "noop": [Function],
   "noopPromise": Promise {},
   "noopTranslate": [Function],
+  "numberDisplay": [Function],
 }
 `;
 
@@ -110,5 +118,6 @@ Object {
   "noop": [Function],
   "noopPromise": Promise {},
   "noopTranslate": [Function],
+  "numberDisplay": [Function],
 }
 `;
diff --git a/src/common/__tests__/helpers.test.js b/src/common/__tests__/helpers.test.js
@@ -23,6 +23,13 @@ describe('Helpers', () => {
     expect(helpers.isPromise(() => 'lorem')).toBe(false);
   });
 
+  it('should apply a number display function', () => {
+    expect(helpers.numberDisplay(null)).toBe(null);
+    expect(helpers.numberDisplay(undefined)).toBe(undefined);
+    expect(helpers.numberDisplay(NaN)).toBe(NaN);
+    expect(helpers.numberDisplay(11)).toMatchSnapshot('number display function result');
+  });
+
   it('should expose a window object', () => {
     helpers.browserExpose({ lorem: 'ipsum' });
     expect(window[helpers.UI_WINDOW_ID]).toMatchSnapshot('window object');

diff --git a/src/common/helpers.js b/src/common/helpers.js
@@ -1,3 +1,5 @@
+import numbro from 'numbro';
+
 /**
  * Generate a random'ish ID.
  *
@@ -60,6 +62,23 @@ const noopTranslate = (key, value, components) => {
   })`;
 };
 
+/**
+ * ToDo: review adding "locale" for numbro
+ */
+/**
+ * Convenience wrapper for numbro. Numbro assumes all values passed to it conform as "number".
+ * This allows us to optional chain the function results.
+ *
+ * @param {*} value
+ * @returns {numbro.Numbro|*}
+ */
+const numberDisplay = value => {
+  if (typeof value !== 'number' || Number.isNaN(value)) {
+    return value;
+  }
+  return numbro(value);
+};
+
 /**
  * Is dev mode active.
  * Associated with using the NPM script "start". See dotenv config files for activation.
@@ -277,6 +296,7 @@ const helpers = {
   noop,
   noopPromise,
   noopTranslate,
+  numberDisplay,
   DEV_MODE,
   PROD_MODE,
   REVIEW_MODE,

diff --git a/src/components/graphCard/__tests__/__snapshots__/graphCardMetricTotals.test.js.snap b/src/components/graphCard/__tests__/__snapshots__/graphCardMetricTotals.test.js.snap
@@ -146,7 +146,7 @@ exports[`GraphCardMetricTotals Component should handle multiple display states:
         >
           <CardBody>
             <div>
-              t(curiosity-graph.cardBodyMetric_total, {"context":"","total":"NAN"}, [object Object])
+              t(curiosity-graph.cardBodyMetric_total, {"context":""}, [object Object])
             </div>
           </CardBody>
         </MinHeight>
@@ -177,7 +177,7 @@ exports[`GraphCardMetricTotals Component should handle multiple display states:
         >
           <CardBody>
             <div>
-              t(curiosity-graph.cardBodyMetric_total, {"context":"","total":"NAN"}, [object Object])
+              t(curiosity-graph.cardBodyMetric_total, {"context":""}, [object Object])
             </div>
           </CardBody>
         </MinHeight>

diff --git a/src/components/graphCard/graphCardChartTooltip.js b/src/components/graphCard/graphCardChartTooltip.js
@@ -1,11 +1,11 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import numbro from 'numbro';
 import { useProduct, useProductGraphTallyQuery } from '../productView/productViewContext';
 import { getTooltipDate } from './graphCardHelpers';
 import { translate } from '../i18n/i18n';
 import { ChartIcon } from '../chart/chartIcon';
 import { RHSM_API_QUERY_SET_TYPES } from '../../services/rhsm/rhsmConstants';
+import { helpers } from '../../common';
 
 /**
  * A custom chart tooltip.
@@ -93,9 +93,10 @@ const GraphCardChartTooltip = ({
               const updatedDataFacetValue =
                 (typeof dataFacet.value === 'number' &&
                   !Number.isInteger(dataFacet.value) &&
-                  numbro(dataFacet.value)
-                    .format({ average: true, mantissa: 5, trimMantissa: true, lowPrecision: true })
-                    .toUpperCase()) ||
+                  helpers
+                    .numberDisplay(dataFacet.value)
+                    ?.format({ average: true, mantissa: 5, trimMantissa: true, lowPrecision: true })
+                    ?.toUpperCase()) ||
                 dataFacet.value;
 
               return (

diff --git a/src/components/graphCard/graphCardHelpers.js b/src/components/graphCard/graphCardHelpers.js
@@ -1,8 +1,7 @@
 import moment from 'moment';
 import { chart_color_green_300 as chartColorGreenDark } from '@patternfly/react-tokens';
-import numbro from 'numbro';
 import { RHSM_API_QUERY_GRANULARITY_TYPES as GRANULARITY_TYPES } from '../../types/rhsmApiTypes';
-import { dateHelpers } from '../../common';
+import { dateHelpers, helpers } from '../../common';
 
 /**
  * Update chart/graph filters with base settings with styling.
@@ -156,10 +155,6 @@ const xAxisTickFormat = ({ callback, date, granularity, tick, previousDate } = {
   return formattedDate;
 };
 
-/**
- * ToDo: review adding "locale" for numbro
- * Original settings, numbro(tick).format({ average: true, mantissa: 1, optionalMantissa: true });
- */
 /**
  * Format y axis ticks.
  *
@@ -173,14 +168,15 @@ const yAxisTickFormat = ({ callback, tick } = {}) => {
     return callback({ tick });
   }
 
-  return numbro(tick)
-    .format({
+  return helpers
+    .numberDisplay(tick)
+    ?.format({
       average: true,
       mantissa: 1,
       trimMantissa: true,
       lowPrecision: false
     })
-    .toUpperCase();
+    ?.toUpperCase();
 };
 
 /**

diff --git a/src/components/graphCard/graphCardMetricTotals.js b/src/components/graphCard/graphCardMetricTotals.js
@@ -2,12 +2,11 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { Card, CardBody, CardFooter, CardTitle, Flex, FlexItem } from '@patternfly/react-core';
 import moment from 'moment';
-import numbro from 'numbro';
 import { useProductGraphTallyQuery } from '../productView/productViewContext';
 import { useMetricsSelector } from './graphCardContext';
 import { MinHeight } from '../minHeight/minHeight';
 import { Loader, SkeletonSize } from '../loader/loader';
-import { dateHelpers } from '../../common';
+import { dateHelpers, helpers } from '../../common';
 import { toolbarFieldOptions } from '../toolbar/toolbarFieldRangedMonthly';
 import { RHSM_API_QUERY_SET_TYPES } from '../../services/rhsm/rhsmConstants';
 import { translate } from '../i18n/i18n';
@@ -66,9 +65,15 @@ const GraphCardMetricTotals = ({
                       'curiosity-graph.cardBodyMetric_total',
                       {
                         context: (dailyHasData && metricId) || '',
-                        total: numbro(dailyValue)
-                          .format({ average: true, mantissa: 5, trimMantissa: true, lowPrecision: false })
-                          .toUpperCase()
+                        total: helpers
+                          .numberDisplay(dailyValue)
+                          ?.format({
+                            average: true,
+                            mantissa: 5,
+                            trimMantissa: true,
+                            lowPrecision: false
+                          })
+                          ?.toUpperCase()
                       },
                       [<strong title={dailyValue} aria-label={dailyValue} />]
                     )}
@@ -103,9 +108,10 @@ const GraphCardMetricTotals = ({
                       'curiosity-graph.cardBodyMetric_total',
                       {
                         context: (monthlyHasData && metricId) || '',
-                        total: numbro(monthlyValue)
-                          .format({ average: true, mantissa: 5, trimMantissa: true, lowPrecision: false })
-                          .toUpperCase()
+                        total: helpers
+                          .numberDisplay(monthlyValue)
+                          ?.format({ average: true, mantissa: 5, trimMantissa: true, lowPrecision: false })
+                          ?.toUpperCase()
                       },
                       [<strong title={monthlyValue} aria-label={monthlyValue} />]
                     )}

diff --git a/src/components/i18n/__tests__/__snapshots__/i18n.test.js.snap b/src/components/i18n/__tests__/__snapshots__/i18n.test.js.snap
@@ -554,7 +554,7 @@ Array [
     "keys": Array [
       Object {
         "key": "curiosity-graph.cardActionTotal",
-        "match": "translate('curiosity-graph.cardActionTotal', { context: 'coreHours', total: numbro(totalCoreHours)",
+        "match": "translate('curiosity-graph.cardActionTotal', { context: 'coreHours', total: helpers .numberDisplay(totalCoreHours)",
       },
       Object {
         "key": "curiosity-inventory.label",
@@ -567,7 +567,7 @@ Array [
     "keys": Array [
       Object {
         "key": "curiosity-graph.cardActionTotal",
-        "match": "translate('curiosity-graph.cardActionTotal', { context: 'coreHours', total: numbro(totalCoreHours)",
+        "match": "translate('curiosity-graph.cardActionTotal', { context: 'coreHours', total: helpers .numberDisplay(totalCoreHours)",
       },
       Object {
         "key": "curiosity-inventory.label",