Merge pull request #3495 from metabase/metrics-segments-fixes

Metrics segments fixes

frontend/src/metabase/lib/schema_metadata.js

@@ -111,7 +111,7 @@ export const isMetric    = (col) => (col && col.source !== "breakout") && isSumm
 export const isNumericBaseType = (field) => isa(field && field.base_type, TYPE.Number);
 
 // ZipCode, ID, etc derive from Number but should not be formatted as numbers
-export const isNumber = (field) => (field.special_type == null || field.special_type === TYPE.Number);
+export const isNumber = (field) => field && isNumericBaseType(field) && (field.special_type == null || field.special_type === TYPE.Number);
 
 export const isCoordinate = (field) => isa(field && field.special_type, TYPE.Coordinate);
 export const isLatitude = (field) => isa(field && field.special_type, TYPE.Latitude);
@@ -472,7 +472,7 @@ export function addValidOperatorsToFields(table) {
     for (let field of table.fields) {
         field.valid_operators = getOperators(field, table);
     }
-    table.aggregation_options = getAggregators(table);
+    table.aggregation_options = getAggregatorsWithFields(table);
     table.breakout_options = getBreakouts(table.fields);
     return table;
 }

frontend/src/metabase/query_builder/AggregationWidget.jsx

@@ -7,7 +7,7 @@ import Popover from "metabase/components/Popover.jsx";
 
 import Query from "metabase/lib/query";
 import { AggregationClause } from "metabase/lib/query";
-import { getAggregator, getAggregatorsWithFields } from "metabase/lib/schema_metadata";
+import { getAggregator } from "metabase/lib/schema_metadata";
 
 import cx from "classnames";
 import _ from "underscore";
@@ -31,17 +31,6 @@ export default class AggregationWidget extends Component {
         updateAggregation: PropTypes.func.isRequired
     };
 
-
-    componentWillMount() {
-        this.componentWillReceiveProps(this.props);
-    }
-
-    componentWillReceiveProps(newProps) {
-        this.setState({
-            availableAggregations: getAggregatorsWithFields(newProps.tableMetadata)
-        });
-    }
-
     setAggregation(aggregation) {
         this.props.updateAggregation(aggregation);
     }
@@ -104,7 +93,7 @@ export default class AggregationWidget extends Component {
                 >
                     <AggregationPopover
                         aggregation={aggregation}
-                        availableAggregations={this.state.availableAggregations}
+                        availableAggregations={tableMetadata.aggregation_options}
                         tableMetadata={tableMetadata}
                         customFields={this.props.customFields}
                         onCommitAggregation={this.setAggregation}

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

@@ -24,15 +24,18 @@ describe('formatting', () => {
     });
 
     describe("formatValue", () => {
+        it("should format numbers with null column", () => {
+            expect(formatValue(12345)).toEqual("12345");
+        });
         it("should format numbers with commas", () => {
-            expect(formatValue(12345, { column: { special_type: TYPE.Number }})).toEqual("12,345");
+            expect(formatValue(12345, { column: { base_type: TYPE.Number, special_type: TYPE.Number }})).toEqual("12,345");
         });
         it("should format zip codes without commas", () => {
-            expect(formatValue(12345, { column: { special_type: TYPE.ZipCode }})).toEqual("12345");
+            expect(formatValue(12345, { column: { base_type: TYPE.Number, special_type: TYPE.ZipCode }})).toEqual("12345");
         });
         it("should format latitude and longitude columns correctly", () => {
-            expect(formatValue(37.7749, { column: { special_type: TYPE.Latitude }})).toEqual("37.77490000");
-            expect(formatValue(-122.4194, { column: { special_type: TYPE.Longitude }})).toEqual("-122.41940000");
+            expect(formatValue(37.7749, { column: { base_type: TYPE.Number, special_type: TYPE.Latitude }})).toEqual("37.77490000");
+            expect(formatValue(-122.4194, { column: { base_type: TYPE.Number, special_type: TYPE.Longitude }})).toEqual("-122.41940000");
         });
     });
 });