From 16c9f4eacd44fe9a75ae6cb26f42ce0864ba8062 Mon Sep 17 00:00:00 2001
From: Tom Robinson <tlrobinson@gmail.com>
Date: Sat, 10 Dec 2016 12:05:12 -0800
Subject: [PATCH] Refactor AggregationWidget for named aggregations
---
.../src/metabase/lib/expressions/formatter.js | 2 +-
.../components/AggregationWidget.jsx | 78 ++++++++++---------
2 files changed, 41 insertions(+), 39 deletions(-)
diff --git a/frontend/src/metabase/lib/expressions/formatter.js b/frontend/src/metabase/lib/expressions/formatter.js
index 0214ce38bd0..2e3a2f670c1 100644
--- a/frontend/src/metabase/lib/expressions/formatter.js
+++ b/frontend/src/metabase/lib/expressions/formatter.js
@@ -15,7 +15,7 @@ export function format(expr, {
aggregations = VALID_AGGREGATIONS
}, parens = false) {
const info = { tableMetadata, customFields, operators, aggregations };
- if (expr == null) {
+ if (expr == null || _.isEqual(expr, [])) {
return "";
}
if (typeof expr === "number") {
diff --git a/frontend/src/metabase/query_builder/components/AggregationWidget.jsx b/frontend/src/metabase/query_builder/components/AggregationWidget.jsx
index d9c526f7320..e383f40c7cc 100644
--- a/frontend/src/metabase/query_builder/components/AggregationWidget.jsx
+++ b/frontend/src/metabase/query_builder/components/AggregationWidget.jsx
@@ -51,31 +51,26 @@ export default class AggregationWidget extends Component {
const fieldId = AggregationClause.getField(aggregation);
let selectedAggregation = getAggregator(AggregationClause.getOperator(aggregation));
- if (selectedAggregation && !_.findWhere(tableMetadata.aggregation_options, { short: selectedAggregation.short })) {
- // if this table doesn't support the selected aggregation, prompt the user to select a different one
- selectedAggregation = null;
+ // if this table doesn't support the selected aggregation, prompt the user to select a different one
+ if (selectedAggregation && _.findWhere(tableMetadata.aggregation_options, { short: selectedAggregation.short })) {
+ return (
+ <span className="flex align-center">
+ { selectedAggregation.name.replace(" of ...", "") }
+ { fieldId &&
+ <span style={{paddingRight: "4px", paddingLeft: "4px"}} className="text-bold">of</span>
+ }
+ { fieldId &&
+ <FieldName
+ className="View-section-aggregation-target SelectionModule py1"
+ tableMetadata={tableMetadata}
+ field={fieldId}
+ fieldOptions={Query.getFieldOptions(tableMetadata.fields, true)}
+ customFieldOptions={this.props.customFields}
+ />
+ }
+ </span>
+ );
}
- return (
- <span className="View-section-aggregation QueryOption py1 mx1 flex align-center">
- { selectedAggregation ?
- selectedAggregation.name.replace(" of ...", "")
- :
- "Choose an aggregation"
- }
- { selectedAggregation && fieldId &&
- <span style={{paddingRight: "4px", paddingLeft: "4px"}} className="text-bold">of</span>
- }
- { selectedAggregation && fieldId &&
- <FieldName
- className="View-section-aggregation-target SelectionModule py1"
- tableMetadata={tableMetadata}
- field={fieldId}
- fieldOptions={Query.getFieldOptions(tableMetadata.fields, true)}
- customFieldOptions={this.props.customFields}
- />
- }
- </span>
- );
}
renderMetricAggregation() {
@@ -83,16 +78,14 @@ export default class AggregationWidget extends Component {
const metricId = AggregationClause.getMetric(aggregation);
let selectedMetric = _.findWhere(tableMetadata.metrics, { id: metricId });
- return (
- <span className="View-section-aggregation QueryOption p1">{selectedMetric ? selectedMetric.name.replace(" of ...", "") : "Choose an aggregation"}</span>
- );
+ if (selectedMetric) {
+ return selectedMetric.name.replace(" of ...", "")
+ }
}
renderCustomAggregation() {
const { aggregation, tableMetadata, customFields } = this.props;
- return (
- <span className="View-section-aggregation QueryOption p1">{aggregation ? format(aggregation, { tableMetadata, customFields }) : "Choose an aggregation"}</span>
- );
+ return format(aggregation, { tableMetadata, customFields });
}
renderPopover() {
@@ -121,20 +114,29 @@ export default class AggregationWidget extends Component {
}
render() {
- const { aggregation, addButton } = this.props;
+ const { aggregation, addButton, name } = this.props;
if (aggregation && aggregation.length > 0) {
+ let aggregationName = AggregationClause.isCustom(aggregation) ?
+ this.renderCustomAggregation()
+ : AggregationClause.isMetric(aggregation) ?
+ this.renderMetricAggregation()
+ :
+ this.renderStandardAggregation()
+
return (
<div className={cx("Query-section Query-section-aggregation", { "selected": this.state.isOpen })}>
<div>
<Clearable onClear={this.props.removeAggregation}>
<div id="Query-section-aggregation" onClick={this.open} className="Query-section Query-section-aggregation cursor-pointer">
- { AggregationClause.isCustom(aggregation) ?
- this.renderCustomAggregation()
- : AggregationClause.isMetric(aggregation) ?
- this.renderMetricAggregation()
- :
- this.renderStandardAggregation()
- }
+ <span className="View-section-aggregation QueryOption py1 mx1">
+ { aggregationName == null ?
+ "Choose an aggregation"
+ : name ?
+ name
+ :
+ aggregationName
+ }
+ </span>
</div>
</Clearable>
{this.renderPopover()}
--
GitLab