Merge formatScalar into formatValue. Move col param to options object

frontend/src/admin/datamodel/containers/MetricForm.jsx

@@ -7,7 +7,7 @@ import FieldSet from "../components/FieldSet.jsx";
 import PartialQueryBuilder from "../components/PartialQueryBuilder.jsx";
 import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper.jsx";
 
-import { formatScalar } from "metabase/lib/formatting";
+import { formatValue } from "metabase/lib/formatting";
 
 import { metricFormSelectors } from "../selectors";
 import { reduxForm } from "redux-form";
@@ -83,7 +83,7 @@ export default class MetricForm extends Component {
                                 aggregation_options: tableMetadata.aggregation_options.filter(a => a.short !== "rows"),
                                 metrics: null
                             }}
-                            previewSummary={previewSummary == null ? "" : "Result: " + formatScalar(previewSummary)}
+                            previewSummary={previewSummary == null ? "" : "Result: " + formatValue(previewSummary)}
                             updatePreviewSummary={this.updatePreviewSummary.bind(this)}
                             {...definition}
                         />

frontend/src/admin/datamodel/containers/SegmentForm.jsx

@@ -7,7 +7,7 @@ import FieldSet from "../components/FieldSet.jsx";
 import PartialQueryBuilder from "../components/PartialQueryBuilder.jsx";
 import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper.jsx";
 
-import { formatScalar } from "metabase/lib/formatting";
+import { formatValue } from "metabase/lib/formatting";
 
 import { segmentFormSelectors } from "../selectors";
 import { reduxForm } from "redux-form";
@@ -82,7 +82,7 @@ export default class SegmentForm extends Component {
                                     ...tableMetadata,
                                     segments: null
                                 }}
-                                previewSummary={previewSummary == null ? "" : formatScalar(previewSummary) + " rows"}
+                                previewSummary={previewSummary == null ? "" : formatValue(previewSummary) + " rows"}
                                 updatePreviewSummary={this.updatePreviewSummary.bind(this)}
                                 {...definition}
                             />

frontend/src/lib/formatting.js

@@ -28,16 +28,12 @@ export function formatNumber(number, options = {}) {
     }
 }
 
-export function formatScalar(scalar, options = {}) {
-    if (typeof scalar === "number") {
-        return formatNumber(scalar, { comma: true, ...options });
-    } else {
-        return String(scalar);
-    }
-}
-
 function formatMajorMinor(major, minor, options = {}) {
-    options = { jsx: false, majorWidth: 3, ...options };
+    options = {
+        jsx: false,
+        majorWidth: 3,
+        ...options
+    };
     if (options.jsx) {
         return (
             <span>
@@ -51,7 +47,7 @@ function formatMajorMinor(major, minor, options = {}) {
     }
 }
 
-export function formatTimeWithUnit(value, unit, options = {}) {
+function formatTimeWithUnit(value, unit, options = {}) {
     let m = moment.parseZone(value);
     if (options.utcOffset != null) {
         m.utcOffset(options.utcOffset);
@@ -85,10 +81,15 @@ export function formatTimeWithUnit(value, unit, options = {}) {
     return String(value);
 }
 
-export function formatValue(value, column, options = {}) {
-    options = { jsx: false, ...options };
+export function formatValue(value, options = {}) {
+    let column = options.column;
+    options = {
+        jsx: false,
+        comma: column && column.special_type === "number",
+        ...options
+    };
     if (value == undefined) {
-        return null
+        return null;
     } else if (column && column.unit != null) {
         return formatTimeWithUnit(value, column.unit, options);
     } else if (isDate(column) || moment.isDate(value) || moment.isMoment(value) || moment(value, ["YYYY-MM-DD'T'HH:mm:ss.SSSZ"], true).isValid()) {
@@ -99,9 +100,7 @@ export function formatValue(value, column, options = {}) {
         if (column && (column.special_type === "latitude" || column.special_type === "longitude")) {
             return DECIMAL_DEGREES_FORMATTER(value)
         } else {
-            // don't show comma unless it's a number special_type (and eventually currency, etc)
-            let comma = column && column.special_type === "number";
-            return formatNumber(value, { comma, ...options });
+            return formatNumber(value, options);
         }
     } else if (typeof value === "object") {
         // no extra whitespace for table cells

frontend/src/lib/schema_metadata.js

@@ -59,6 +59,10 @@ const TYPES = {
 };
 
 export function isFieldType(type, field) {
+    if (!field) {
+        return false;
+    }
+
     let def = TYPES[type];
     // check to see if it belongs to any of the field types:
     for (let prop of ["field", "base", "special"]) {

frontend/src/visualizations/PieChart.jsx

@@ -40,8 +40,8 @@ export default class PieChart extends Component {
         const { series, hovered, onHoverChange, className, gridSize } = this.props;
         const { data } = series[0];
 
-        const formatDimension = (dimension, jsx = true) => formatValue(dimension, data.cols[0], { jsx, majorWidth: 0 })
-        const formatMetric    =    (metric, jsx = true) => formatValue(metric, data.cols[1], { jsx, majorWidth: 0 })
+        const formatDimension = (dimension, jsx = true) => formatValue(dimension, { column: data.cols[0], jsx, majorWidth: 0 })
+        const formatMetric    =    (metric, jsx = true) => formatValue(metric, { column: data.cols[1], jsx, majorWidth: 0 })
         const formatPercent   =               (percent) => (100 * percent).toFixed(2) + "%"
 
         let total = data.rows.reduce((sum, row) => sum + row[1], 0);

frontend/src/visualizations/Scalar.jsx

@@ -5,10 +5,11 @@ import Ellipsified from "metabase/components/Ellipsified.jsx";
 import BarChart from "./BarChart.jsx";
 
 import Urls from "metabase/lib/urls";
-import { formatScalar } from "metabase/lib/formatting";
+import { formatValue } from "metabase/lib/formatting";
 import { isSameSeries } from "metabase/visualizations/lib/utils";
 
 import cx from "classnames";
+import i from "icepick";
 
 export default class Scalar extends Component {
     static displayName = "Number";
@@ -98,9 +99,10 @@ export default class Scalar extends Component {
 
         let isSmall = gridSize && gridSize.width < 4;
 
-        let scalarValue = (data && data.rows && data.rows[0] && data.rows[0][0] != null) ? data.rows[0][0] : "";
+        let scalarValue = i.getIn(data, ["rows", 0, 0]);
         let stringifiedScalar = String(scalarValue);
-        let formattedScalarValue = formatScalar(scalarValue, { compact: isSmall });
+        let formattedScalarValue = scalarValue == undefined ? "" :
+            formatValue(scalarValue, { column: i.getIn(data, ["cols", 0]), compact: isSmall });
 
         return (
             <div className={cx(className, styles.Scalar, styles[isSmall ? "small" : "large"])}>

frontend/src/visualizations/TableInteractive.jsx

@@ -175,7 +175,7 @@ export default class TableInteractive extends Component {
     }
 
     cellRenderer(cellData, cellDataKey, rowData, rowIndex, columnData, width) {
-        cellData = cellData != null ? formatValue(cellData, this.props.data.cols[cellDataKey], { jsx: true }) : null;
+        cellData = cellData != null ? formatValue(cellData, { column: this.props.data.cols[cellDataKey], jsx: true }) : null;
 
         var key = 'cl'+rowIndex+'_'+cellDataKey;
         if (this.props.cellIsClickableFn(rowIndex, cellDataKey)) {

frontend/src/visualizations/TableSimple.jsx

@@ -88,7 +88,7 @@ export default class TableSimple extends Component {
                                 <tr key={rowIndex} ref={rowIndex === 0 ? "firstRow" : null}>
                                     {row.map((cell, colIndex) =>
                                         <td key={colIndex} style={{ whiteSpace: "nowrap" }} className="px1 border-bottom">
-                                            { cell == null ? "-" : formatValue(cell, cols[colIndex], { jsx: true }) }
+                                            { cell == null ? "-" : formatValue(cell, { column: cols[colIndex], jsx: true }) }
                                         </td>
                                     )}
                                 </tr>

frontend/src/visualizations/components/ChartTooltip.jsx

@@ -43,7 +43,7 @@ export default class ChartTooltip extends Component {
                             [["key", 0], ["value", 1]].map(([propName, colIndex]) =>
                                 <tr key={propName} className="">
                                     <th className="text-light text-right">{getFriendlyName(s.data.cols[colIndex])}:</th>
-                                    <th className="pl1 text-bold text-left">{formatValue(hovered.data[propName], s.data.cols[colIndex], { jsx: true, majorWidth: 0 })}</th>
+                                    <th className="pl1 text-bold text-left">{formatValue(hovered.data[propName], { column: s.data.cols[colIndex], jsx: true, majorWidth: 0 })}</th>
                                 </tr>
                             )
                         }

frontend/src/visualizations/lib/CardRenderer.js

@@ -99,7 +99,7 @@ function applyChartTimeseriesXAxis(chart, settings, series, xValues) {
 
         if (dimensionColumn && dimensionColumn.unit) {
             // need to pass the utcOffset here since d3.time returns Dates not Moments and thus doesn't propagate the offset
-            chart.xAxis().tickFormat(d => formatValue(d, dimensionColumn, { utcOffset: utcOffset }));
+            chart.xAxis().tickFormat(d => formatValue(d, { column: dimensionColumn, utcOffset: utcOffset }));
         } else {
             chart.xAxis().tickFormat(d3.time.format.multi([
                 [".%L",    (d) => d.getMilliseconds()],
@@ -153,7 +153,7 @@ function applyChartOrdinalXAxis(chart, settings, series, xValues) {
             let visibleKeys = xValues.filter((v, i) => i % keyInterval === 0);
             chart.xAxis().tickValues(visibleKeys);
         }
-        chart.xAxis().tickFormat(d => formatValue(d, dimensionColumn));
+        chart.xAxis().tickFormat(d => formatValue(d, { column: dimensionColumn }));
     } else {
         chart.xAxis().ticks(0);
         chart.xAxis().tickFormat('');

frontend/test/unit/lib/formatting.spec.js

-import { formatNumber, formatScalar } from 'metabase/lib/formatting';
+import { formatNumber } from 'metabase/lib/formatting';
 
 describe('formatting', () => {
     describe('formatNumber', () => {
@@ -21,13 +21,4 @@ describe('formatting', () => {
             expect(formatNumber(0.0001/3)).toEqual("0.000033");
         });
     });
-
-    describe('formatScalar', () => {
-        it('should format numbers correctly', () => {
-            expect(formatScalar(0)).toEqual("0");
-        });
-        it('should format strings correctly', () => {
-            expect(formatScalar("asdf")).toEqual("asdf");
-        });
-    });
 });