Merge pull request #1921 from metabase/fix-valid-groupings

hide the "grouped by" useful questions from the data reference if the field is not a valid dimension.

frontend/src/query_builder/DataReferenceField.jsx

@@ -3,6 +3,7 @@ import React, { Component, PropTypes } from "react";
 import DataReferenceQueryButton from './DataReferenceQueryButton.jsx';
 import Icon from "metabase/components/Icon.jsx";
 import Query from "metabase/lib/query";
+import { isDimension } from "metabase/lib/schema_metadata";
 import inflection from 'inflection';
 
 import cx from "classnames";
@@ -112,19 +113,18 @@ export default class DataReferenceField extends Component {
         let fieldName = field.display_name;
         let tableName = table ? table.display_name : "";
 
-        // TODO: allow for filters/grouping via foreign keys
-        let validForCurrentQuestion = !query.query || query.query.source_table == undefined || query.query.source_table === field.table_id;
-
-        let useForCurrentQuestion = [];
-        if (validForCurrentQuestion) {
-            let validBreakout = false;
-            if (this.state.table) {
-                let usedFields = {};
-                query.query.breakout && query.query.breakout.forEach(f => usedFields[f] = true);
-                let breakoutOptions = Query.getFieldOptions(table.fields, true, table.breakout_options.validFieldsFilter, usedFields);
-                validBreakout = _.some(breakoutOptions.fields, f => f.id === field.id);
-            }
+        let useForCurrentQuestion = [],
+            usefulQuestions = [];
+
+        // determine if the selected field is a valid dimension on this table
+        let validBreakout = false;
+        if (this.state.table) {
+            const validDimensions = _.filter(table.fields, isDimension);
+            validBreakout = _.some(validDimensions, f => f.id === field.id);
+        }
 
+        // TODO: allow for filters/grouping via foreign keys
+        if (!query.query || query.query.source_table == undefined || query.query.source_table === field.table_id) {
             useForCurrentQuestion.push(
                 <li key="filter-by" className="mt1">
                     <a className="Button Button--white text-default text-brand-hover border-brand-hover no-decoration" onClick={this.filterBy}>
@@ -132,7 +132,9 @@ export default class DataReferenceField extends Component {
                         </a>
                 </li>
             );
-            if (validBreakout) {
+
+            // current field must be a valid breakout option for this table AND cannot already be in the breakout clause of our query
+            if (validBreakout && (query.query.breakout && !_.contains(query.query.breakout, field.id))) {
                 useForCurrentQuestion.push(
                     <li key="group-by" className="mt1">
                         <a className="Button Button--white text-default text-brand-hover border-brand-hover no-decoration" onClick={this.groupBy}>
@@ -143,14 +145,15 @@ export default class DataReferenceField extends Component {
             }
         }
 
-        let usefulQuestions = [];
         if (this.props.field.special_type === "number") {
             usefulQuestions.push(<li className="border-row-divider" key="sum"><DataReferenceQueryButton icon="illustration-icon-scalar" text={"Sum of all values of " + fieldName} onClick={this.setQuerySum} /></li>);
         }
         usefulQuestions.push(<li className="border-row-divider" key="distinct-values"><DataReferenceQueryButton icon="illustration-icon-table" text={"All distinct values of " + fieldName} onClick={this.setQueryDistinct} /></li>);
         let queryCountGroupedByText = "Number of " + inflection.pluralize(tableName) + " grouped by " + fieldName;
-        usefulQuestions.push(<li className="border-row-divider" key="count-bar"><DataReferenceQueryButton icon="illustration-icon-bars" text={queryCountGroupedByText} onClick={this.setQueryCountGroupedBy.bind(null, "bar")} /></li>);
-        usefulQuestions.push(<li className="border-row-divider" key="count-pie"><DataReferenceQueryButton icon="illustration-icon-pie" text={queryCountGroupedByText} onClick={this.setQueryCountGroupedBy.bind(null, "pie")} /></li>);
+        if (validBreakout) {
+            usefulQuestions.push(<li className="border-row-divider" key="count-bar"><DataReferenceQueryButton icon="illustration-icon-bars" text={queryCountGroupedByText} onClick={this.setQueryCountGroupedBy.bind(null, "bar")} /></li>);
+            usefulQuestions.push(<li className="border-row-divider" key="count-pie"><DataReferenceQueryButton icon="illustration-icon-pie" text={queryCountGroupedByText} onClick={this.setQueryCountGroupedBy.bind(null, "pie")} /></li>);
+        }
 
         let descriptionClasses = cx({ "text-grey-3": !this.props.field.description });
         let description = (<p className={descriptionClasses}>{this.props.field.description || "No description set."}</p>);