Merge pull request #5547 from metabase/fix-5326

Only show supported aggregation drill-throughs for a field

frontend/src/metabase/lib/schema_metadata.js

@@ -480,6 +480,9 @@ export function getAggregator(short) {
     return _.findWhere(Aggregators, { short: short });
 }
 
+export const isCompatibleAggregatorForField = (aggregator, field) =>
+    aggregator.validFieldsFilters.every(filter => filter([field]).length === 1)
+
 export function getBreakouts(fields) {
     var result = populateFields(BreakoutAggregator, fields);
     result.fields = result.fields[0];

frontend/src/metabase/qb/components/drill/SummarizeColumnByTimeDrill.js

@@ -4,7 +4,11 @@ import React from "react";
 
 import StructuredQuery from "metabase-lib/lib/queries/StructuredQuery";
 import { getFieldRefFromColumn } from "metabase/qb/lib/actions";
-import { isNumeric, isDate } from "metabase/lib/schema_metadata";
+import {
+    isDate,
+    getAggregator,
+    isCompatibleAggregatorForField
+} from "metabase/lib/schema_metadata";
 import { capitalize } from "metabase/lib/formatting";
 
 import type {
@@ -20,28 +24,32 @@ export default ({ question, clicked }: ClickActionProps): ClickAction[] => {
 
     const dateField = query.table().fields.filter(isDate)[0];
     if (
-        !dateField ||
-        !clicked ||
-        !clicked.column ||
-        clicked.value !== undefined ||
-        !isNumeric(clicked.column)
+        !dateField || !clicked || !clicked.column || clicked.value !== undefined
     ) {
         return [];
     }
     const { column } = clicked;
 
-    return ["sum", "count"].map(aggregation => ({
-        name: "summarize-by-time",
-        section: "sum",
-        title: <span>{capitalize(aggregation)} by time</span>,
-        question: () =>
-            question
-                .summarize([aggregation, getFieldRefFromColumn(column)])
-                .pivot([
-                    "datetime-field",
-                    getFieldRefFromColumn(dateField),
-                    "as",
-                    "day"
-                ])
-    }));
+    return ["sum", "count"]
+        .map(getAggregator)
+        .filter(aggregator =>
+            isCompatibleAggregatorForField(aggregator, column))
+        .map(aggregator => ({
+            name: "summarize-by-time",
+            section: "sum",
+            title: <span>{capitalize(aggregator.short)} by time</span>,
+            question: () =>
+                question
+                    .summarize(
+                        aggregator.requiresField
+                            ? [aggregator.short, getFieldRefFromColumn(column)]
+                            : [aggregator.short]
+                    )
+                    .pivot([
+                        "datetime-field",
+                        getFieldRefFromColumn(dateField),
+                        "as",
+                        "day"
+                    ])
+        }));
 };

frontend/src/metabase/qb/components/drill/SummarizeColumnDrill.js

 /* @flow */
 
 import { getFieldRefFromColumn } from "metabase/qb/lib/actions";
-import { isNumeric } from "metabase/lib/schema_metadata";
+import {
+    getAggregator,
+    isCompatibleAggregatorForField
+} from "metabase/lib/schema_metadata";
 
 import type {
     ClickAction,
@@ -36,21 +39,34 @@ export default ({ question, clicked }: ClickActionProps): ClickAction[] => {
         !clicked ||
         !clicked.column ||
         clicked.value !== undefined ||
-        clicked.column.source !== "fields" ||
-        !isNumeric(clicked.column)
+        clicked.column.source !== "fields"
     ) {
+        // TODO Atte Keinänen 7/21/17: Does it slow down the drill-through option calculations remarkably
+        // that I removed the `isSummable` condition from here and use `isCompatibleAggregator` method below instead?
         return [];
     }
     const { column } = clicked;
 
-    // $FlowFixMe
-    return Object.entries(AGGREGATIONS).map(([aggregation, action]: [string, {
-        section: string,
-        title: string
-    }]) => ({
-        name: action.title.toLowerCase(),
-        ...action,
-        question: () =>
-            question.summarize([aggregation, getFieldRefFromColumn(column)])
-    }));
+    return (
+        Object.entries(AGGREGATIONS)
+            .map(([aggregationShort, action]) => [
+                getAggregator(aggregationShort),
+                action
+            ])
+            .filter(([aggregator]) =>
+                isCompatibleAggregatorForField(aggregator, column))
+            // $FlowFixMe
+            .map(([aggregator, action]: [any, {
+                section: string,
+                title: string
+            }]) => ({
+                name: action.title.toLowerCase(),
+                ...action,
+                question: () =>
+                    question.summarize([
+                        aggregator.short,
+                        getFieldRefFromColumn(column)
+                    ])
+            }))
+    );
 };