From 9ff09f00d143c86682454f2ab6a30b64b6c7ff09 Mon Sep 17 00:00:00 2001
From: Tom Robinson <tlrobinson@gmail.com>
Date: Sat, 10 Dec 2016 10:22:01 -0800
Subject: [PATCH] Support for metrics and custom fields in expressions
---
.../src/metabase/lib/expressions/formatter.js | 100 +++++++-------
.../src/metabase/lib/expressions/index.js | 22 +++-
.../src/metabase/lib/expressions/parser.js | 74 +++++++----
.../components/AggregationPopover.jsx | 1 +
.../components/AggregationWidget.jsx | 4 +-
.../query_builder/components/FieldName.jsx | 9 +-
.../expressions/ExpressionEditorTextfield.jsx | 20 ++-
.../components/expressions/Expressions.jsx | 5 +-
frontend/test/unit/lib/expressions.spec.js | 123 ------------------
.../unit/lib/expressions/formatter.spec.js | 54 ++++++++
.../test/unit/lib/expressions/parser.spec.js | 2 +-
11 files changed, 202 insertions(+), 212 deletions(-)
create mode 100644 frontend/test/unit/lib/expressions/formatter.spec.js
diff --git a/frontend/src/metabase/lib/expressions/formatter.js b/frontend/src/metabase/lib/expressions/formatter.js
index c1828428602..e896b0c82a9 100644
--- a/frontend/src/metabase/lib/expressions/formatter.js
+++ b/frontend/src/metabase/lib/expressions/formatter.js
@@ -1,61 +1,71 @@
import _ from "underscore";
-import { VALID_OPERATORS, VALID_AGGREGATIONS, isValidArg, isField, isExpression, normalizeName } from "../expressions";
-import { AggregationClause } from "../query";
+import {
+ VALID_OPERATORS, VALID_AGGREGATIONS,
+ isField, isMath, isMetric, isAggregation, isExpressionReference,
+ formatMetricName, formatFieldName, formatExpressionName
+} from "../expressions";
-function formatField(fieldRef, { fields }) {
- const fieldId = fieldRef[1];
- const field = _.findWhere(fields, {id: fieldId});
-
- if (!field) throw 'field with ID does not exist: ' + fieldId;
-
- let displayName = field.display_name;
- return displayName.indexOf(' ') === -1 ? displayName : ('"' + displayName + '"');
+// convert a MBQL expression back into an expression string
+export function format(expr, {
+ tableMetadata = {},
+ customFields = {},
+ operators = VALID_OPERATORS,
+ aggregations = VALID_AGGREGATIONS
+}, parens = false) {
+ const info = { tableMetadata, customFields, operators, aggregations };
+ if (typeof expr === "number") {
+ return formatLiteral(expr);
+ }
+ if (isField(expr)) {
+ return formatField(expr, info);
+ }
+ if (isMetric(expr)) {
+ return formatMetric(expr, info);
+ }
+ if (isMath(expr)) {
+ return formatMath(expr, info, parens);
+ }
+ if (isAggregation(expr)) {
+ return formatAggregation(expr, info);
+ }
+ if (isExpressionReference(expr)) {
+ return formatExpressionReference(expr, info);
+ }
+ throw new Error("Unknown expression " + JSON.stringify(expr));
}
-function formatMetric(metricRef, { metrics }) {
- const metricId = metricRef[1];
- const metric = _.findWhere(metrics, { id: metricId });
-
- if (!metric) throw 'metric with ID does not exist: ' + metricId;
-
- return normalizeName(metric.name) + "()";
+function formatLiteral(expr) {
+ return JSON.stringify(expr);
}
-function formatNestedExpression(expression, tableMetadata, parens = false) {
- let formattedExpression = format(expression, tableMetadata);
- if (VALID_OPERATORS.has(expression[0]) && parens) {
- return '(' + formattedExpression + ')';
- } else {
- return formattedExpression;
+function formatField([, fieldId], { tableMetadata: { fields } }) {
+ const field = _.findWhere(fields, { id: fieldId });
+ if (!field) {
+ throw 'field with ID does not exist: ' + fieldId;
}
+ return formatFieldName(field.display_name);
}
-function formatArg(arg, tableMetadata, parens = false) {
- if (!isValidArg(arg)) throw 'Invalid expression argument:' + arg;
-
- return isField(arg) ? formatField(arg, tableMetadata) :
- isExpression(arg) ? formatNestedExpression(arg, tableMetadata, parens) :
- typeof arg === 'number' ? arg :
- null;
+function formatMetric([, metricId], { tableMetadata: { metrics } }) {
+ const metric = _.findWhere(metrics, { id: metricId });
+ if (!metric) {
+ throw 'metric with ID does not exist: ' + metricId;
+ }
+ return formatMetricName(metric.name) + "()";
}
-/// convert a parsed expression back into an expression string
-export function format(expression, tableMetadata = {}) {
- const { operators = VALID_OPERATORS, aggregations = VALID_AGGREGATIONS } = tableMetadata;
+function formatExpressionReference([, expressionName]) {
+ return formatExpressionName(expressionName);
+}
- if (!expression) return null;
- if (!isExpression(expression)) throw 'Invalid expression: ' + expression;
+function formatMath([operator, ...args], info, parens) {
+ let formatted = args.map(arg => format(arg, info, true)).join(` ${operator} `)
+ return parens ? `(${formatted})` : formatted;
+}
- const [op, ...args] = expression;
- if (AggregationClause.isMetric(expression)) {
- return formatMetric(expression, tableMetadata)
- } else if (operators.has(op)) {
- return args.map(arg => formatArg(arg, tableMetadata, true)).join(` ${op} `)
- } else if (aggregations.has(op)) {
- return `${aggregations.get(op)}(${args.map(arg => formatArg(arg, tableMetadata, false)).join(", ")})`;
- } else {
- throw new Error("Unknown clause " + op);
- }
+function formatAggregation([aggregation, ...args], info) {
+ const { aggregations } = info;
+ return `${aggregations.get(aggregation)}(${args.map(arg => format(arg, info)).join(", ")})`;
}
diff --git a/frontend/src/metabase/lib/expressions/index.js b/frontend/src/metabase/lib/expressions/index.js
index 73c8cf3c8da..05b2e156484 100644
--- a/frontend/src/metabase/lib/expressions/index.js
+++ b/frontend/src/metabase/lib/expressions/index.js
@@ -21,15 +21,24 @@ export const VALID_AGGREGATIONS = new Map(Object.entries({
"max": "Max"
}));
+export function formatMetricName(metricName) {
+ return titleize(metricName).replace(/\W+/g, "")
+}
+
+export function formatFieldName(fieldName) {
+ return /^\w+$/.test(fieldName) ?
+ fieldName :
+ JSON.stringify(fieldName);
+}
-export function normalizeName(name) {
- return titleize(name).replace(/\W+/g, "")
+export function formatExpressionName(name) {
+ return formatFieldName(name);
}
// move to query lib
export function isExpression(expr) {
- return isMath(expr) || isAggregation(expr) || isMetric(expr);
+ return isMath(expr) || isAggregation(expr) || isField(expr) || isMetric(expr) || isExpressionReference(expr);
}
export function isField(expr) {
@@ -37,7 +46,8 @@ export function isField(expr) {
}
export function isMetric(expr) {
- return Array.isArray(expr) && expr.length === 2 && mbqlEq(expr[0], 'metric') && typeof expr[1] === 'number';
+ // case sensitive, unlike most mbql
+ return Array.isArray(expr) && expr.length === 2 && expr[0] === "METRIC" && typeof expr[1] === 'number';
}
export function isMath(expr) {
@@ -48,6 +58,10 @@ export function isAggregation(expr) {
return Array.isArray(expr) && VALID_AGGREGATIONS.has(expr[0]) && _.all(expr.slice(1), isValidArg);
}
+export function isExpressionReference(expr) {
+ return Array.isArray(expr) && expr.length === 2 && mbqlEq(expr[0], 'expression') && typeof expr[1] === 'string';
+}
+
export function isValidArg(arg) {
return isExpression(arg) || isField(arg) || typeof arg === 'number';
}
diff --git a/frontend/src/metabase/lib/expressions/parser.js b/frontend/src/metabase/lib/expressions/parser.js
index a04c92b39d4..399487668cb 100644
--- a/frontend/src/metabase/lib/expressions/parser.js
+++ b/frontend/src/metabase/lib/expressions/parser.js
@@ -1,7 +1,7 @@
const { Lexer, Parser, extendToken, getImage } = require("chevrotain");
const _ = require("underscore");
-import { VALID_AGGREGATIONS, normalizeName } from "../expressions";
+import { VALID_AGGREGATIONS, formatFieldName, formatMetricName, formatExpressionName } from "../expressions";
export const AGGREGATION_ARITY = new Map([
["Count", 0],
@@ -54,10 +54,10 @@ const ExpressionsLexer = new Lexer(allTokens);
class ExpressionsParser extends Parser {
- constructor(input, tableMetadata) {
+ constructor(input, options) {
super(input, allTokens, { recoveryEnabled: false });
- this._tableMetadata = tableMetadata;
+ this._options = options;
let $ = this;
@@ -131,7 +131,11 @@ class ExpressionsParser extends Parser {
let metricName = $.CONSUME(Identifier).image;
$.CONSUME(LParen);
$.CONSUME(RParen);
- return ["METRIC", this.getMetricIdForName(metricName)];
+ const metric = this.getMetricForName(metricName);
+ if (metric != null) {
+ return ["METRIC", metric.id];
+ }
+ throw new Error("Unknown metric \"" + metricName + "\"");
});
$.RULE("fieldExpression", () => {
@@ -139,7 +143,15 @@ class ExpressionsParser extends Parser {
{ALT: () => JSON.parse($.CONSUME(StringLiteral).image) },
{ALT: () => $.CONSUME(Identifier).image }
]);
- return ["field-id", this.getFieldIdForName(fieldName)];
+ const field = this.getFieldForName(fieldName);
+ if (field != null) {
+ return ["field-id", field.id];
+ }
+ const expression = this.getExpressionForName(fieldName);
+ if (expression != null) {
+ return ["expression", fieldName];
+ }
+ throw new Error("Unknown field \"" + fieldName + "\"");
});
$.RULE("atomicExpression", (outsideAggregation) => {
@@ -166,24 +178,19 @@ class ExpressionsParser extends Parser {
Parser.performSelfAnalysis(this);
}
- getFieldIdForName(fieldName) {
- const fields = this._tableMetadata && this._tableMetadata.fields || [];
- for (const field of fields) {
- if (field.display_name.toLowerCase() === fieldName.toLowerCase()) {
- return field.id;
- }
- }
- throw new Error("Unknown field \"" + fieldName + "\"");
+ getFieldForName(fieldName) {
+ const fields = this._options.tableMetadata && this._options.tableMetadata.fields;
+ return _.findWhere(fields, { display_name: fieldName });
}
- getMetricIdForName(metricName) {
- const metrics = this._tableMetadata && this._tableMetadata.metrics || [];
- for (const metric of metrics) {
- if (normalizeName(metric.name).toLowerCase() === metricName.toLowerCase()) {
- return metric.id;
- }
- }
- throw new Error("Unknown metric \"" + metricName + "\"");
+ getExpressionForName(expressionName) {
+ const customFields = this._options && this._options.customFields;
+ return customFields[expressionName];
+ }
+
+ getMetricForName(metricName) {
+ const metrics = this._options.tableMetadata && this._options.tableMetadata.metrics;
+ return _.find(metrics, (metric) => formatMetricName(metric.name) === metricName);
}
}
@@ -196,11 +203,12 @@ function getTokenSource(TokenClass) {
return TokenClass.PATTERN.source.replace(/^\\/, "");
}
-export function compile(source, tableMetadata, { startRule } = {}) {
+export function compile(source, options = {}) {
if (!source) {
return [];
}
- const parser = new ExpressionsParser(ExpressionsLexer.tokenize(source).tokens, tableMetadata);
+ const { startRule } = options;
+ const parser = new ExpressionsParser(ExpressionsLexer.tokenize(source).tokens, options);
const expression = parser[startRule]();
if (parser.errors.length > 0) {
throw parser.errors;
@@ -210,7 +218,12 @@ export function compile(source, tableMetadata, { startRule } = {}) {
// No need for more than one instance.
const parserInstance = new ExpressionsParser([])
-export function suggest(source, tableMetadata, { startRule, index = source.length } = {}) {
+export function suggest(source, {
+ tableMetadata,
+ customFields,
+ startRule,
+ index = source.length
+} = {}) {
const partialSource = source.slice(0, index);
const lexResult = ExpressionsLexer.tokenize(partialSource);
if (lexResult.errors.length > 0) {
@@ -270,9 +283,14 @@ export function suggest(source, tableMetadata, { startRule, index = source.lengt
finalSuggestions.push(...tableMetadata.fields.map(field => ({
type: "fields",
name: field.display_name,
- text: /^\w+$/.test(field.display_name) ?
- field.display_name + " " : // need a space to terminate identifier
- JSON.stringify(field.display_name),
+ text: formatFieldName(field.display_name) + " ",
+ prefixTrim: /\w+$/,
+ postfixTrim: /^\w+\s*/
+ })))
+ finalSuggestions.push(...Object.keys(customFields).map(expressionName => ({
+ type: "fields",
+ name: expressionName,
+ text: formatExpressionName(expressionName) + " ",
prefixTrim: /\w+$/,
postfixTrim: /^\w+\s*/
})))
@@ -295,7 +313,7 @@ export function suggest(source, tableMetadata, { startRule, index = source.lengt
finalSuggestions.push(...tableMetadata.metrics.map(metric => ({
type: "metrics",
name: metric.name,
- text: normalizeName(metric.name) + "() ",
+ text: formatMetricName(metric.name) + "() ",
prefixTrim: /\w+$/,
postfixTrim: /^\w+\(\)?/
})))
diff --git a/frontend/src/metabase/query_builder/components/AggregationPopover.jsx b/frontend/src/metabase/query_builder/components/AggregationPopover.jsx
index c9781c78a54..f49057a2d29 100644
--- a/frontend/src/metabase/query_builder/components/AggregationPopover.jsx
+++ b/frontend/src/metabase/query_builder/components/AggregationPopover.jsx
@@ -179,6 +179,7 @@ export default class AggregationPopover extends Component {
startRule="aggregation"
expression={aggregation}
tableMetadata={tableMetadata}
+ customFields={this.props.customFields}
onChange={(parsedExpression) => this.setState({aggregation: parsedExpression, error: null})}
onError={(errorMessage) => this.setState({error: errorMessage})}
/>
diff --git a/frontend/src/metabase/query_builder/components/AggregationWidget.jsx b/frontend/src/metabase/query_builder/components/AggregationWidget.jsx
index c7f2911c195..d9c526f7320 100644
--- a/frontend/src/metabase/query_builder/components/AggregationWidget.jsx
+++ b/frontend/src/metabase/query_builder/components/AggregationWidget.jsx
@@ -89,9 +89,9 @@ export default class AggregationWidget extends Component {
}
renderCustomAggregation() {
- const { aggregation, tableMetadata } = this.props;
+ const { aggregation, tableMetadata, customFields } = this.props;
return (
- <span className="View-section-aggregation QueryOption p1">{aggregation ? format(aggregation, tableMetadata) : "Choose an aggregation"}</span>
+ <span className="View-section-aggregation QueryOption p1">{aggregation ? format(aggregation, { tableMetadata, customFields }) : "Choose an aggregation"}</span>
);
}
diff --git a/frontend/src/metabase/query_builder/components/FieldName.jsx b/frontend/src/metabase/query_builder/components/FieldName.jsx
index c8189adc0d3..d21e0f44cf5 100644
--- a/frontend/src/metabase/query_builder/components/FieldName.jsx
+++ b/frontend/src/metabase/query_builder/components/FieldName.jsx
@@ -37,9 +37,12 @@ export default class FieldName extends Component {
parts.push(<span key={"fkName"+index}>{stripId(fkField.display_name)}</span>);
parts.push(<span key={"fkIcon"+index} className="px1"><Icon name="connections" size={10} /></span>);
}
- // target field itself
- // using i.getIn to avoid exceptions when field is undefined
- parts.push(<span key="field">{Query.getFieldPathName(fieldTarget.field.id, fieldTarget.table)}</span>);
+ if (fieldTarget.field.id != null) {
+ parts.push(<span key="field">{Query.getFieldPathName(fieldTarget.field.id, fieldTarget.table)}</span>);
+ } else {
+ // expressions, etc
+ parts.push(<span key="field">{fieldTarget.field.display_name}</span>);
+ }
// datetime-field unit
if (fieldTarget.unit != null) {
parts.push(<span key="unit">{": " + formatBucketing(fieldTarget.unit)}</span>);
diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx
index 64e02385ede..dc5e8585aae 100644
--- a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx
+++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx
@@ -32,6 +32,7 @@ export default class ExpressionEditorTextfield extends Component {
static propTypes = {
expression: PropTypes.array, // should be an array like [parsedExpressionObj, expressionString]
tableMetadata: PropTypes.object.isRequired,
+ customFields: PropTypes.object,
onChange: PropTypes.func.isRequired,
onError: PropTypes.func.isRequired,
startRule: PropTypes.string.isRequired
@@ -51,12 +52,17 @@ export default class ExpressionEditorTextfield extends Component {
// we only refresh our state if we had no previous state OR if our expression or table has changed
if (!this.state || this.props.expression != newProps.expression || this.props.tableMetadata != newProps.tableMetadata) {
let parsedExpression = newProps.expression;
- let expressionString = format(newProps.expression, this.props.tableMetadata);
+ let expressionString = format(newProps.expression, {
+ tableMetadata: newProps.tableMetadata,
+ customFields: newProps.customFields,
+ });
let expressionErrorMessage = null;
let suggestions = [];
try {
if (expressionString) {
- compile(expressionString, newProps.tableMetadata, {
+ compile(expressionString, {
+ tableMetadata: newProps.tableMetadata,
+ customFields: newProps.customFields,
startRule: newProps.startRule
});
}
@@ -179,7 +185,9 @@ export default class ExpressionEditorTextfield extends Component {
let parsedExpression;
try {
- parsedExpression = compile(expressionString, this.props.tableMetadata, {
+ parsedExpression = compile(expressionString, {
+ tableMetadata: this.props.tableMetadata,
+ customFields: this.props.customFields,
startRule: this.props.startRule
})
} catch (e) {
@@ -187,7 +195,9 @@ export default class ExpressionEditorTextfield extends Component {
console.error("expression error:", expressionErrorMessage);
}
try {
- suggestions = suggest(expressionString, this.props.tableMetadata, {
+ suggestions = suggest(expressionString, {
+ tableMetadata: this.props.tableMetadata,
+ customFields: this.props.customFields,
startRule: this.props.startRule,
index: inputElement.selectionStart
})
@@ -244,7 +254,7 @@ export default class ExpressionEditorTextfield extends Component {
</li>
,
<li style={{ paddingTop: "2px", paddingBottom: "2px" }}
- className={cx("cursor-pointer", {"text-bold text-brand": i === this.state.highlightedSuggestion})}
+ className={cx("cursor-pointer text-brand-hover", {"text-bold text-brand": i === this.state.highlightedSuggestion})}
onMouseDownCapture={(e) => this.onSuggestionMouseDown(e, i)}
>
{ suggestion.prefixLength ?
diff --git a/frontend/src/metabase/query_builder/components/expressions/Expressions.jsx b/frontend/src/metabase/query_builder/components/expressions/Expressions.jsx
index 05fb7b84ddb..f2a1360e225 100644
--- a/frontend/src/metabase/query_builder/components/expressions/Expressions.jsx
+++ b/frontend/src/metabase/query_builder/components/expressions/Expressions.jsx
@@ -32,7 +32,10 @@ export default class Expressions extends Component {
{ sortedNames && sortedNames.map(name =>
<div key={name} className="pb1 text-brand text-bold cursor-pointer flex flex-row align-center justify-between" onClick={() => onEditExpression(name)}>
<span>{name}</span>
- <Tooltip tooltip={format(expressions[name], this.props.tableMetadata)}>
+ <Tooltip tooltip={format(expressions[name], {
+ tableMetadata: this.props.tableMetadata,
+ customFields: expressions
+ })}>
<span className="QuestionTooltipTarget" />
</Tooltip>
</div>
diff --git a/frontend/test/unit/lib/expressions.spec.js b/frontend/test/unit/lib/expressions.spec.js
index 88cd5f87697..e69de29bb2d 100644
--- a/frontend/test/unit/lib/expressions.spec.js
+++ b/frontend/test/unit/lib/expressions.spec.js
@@ -1,123 +0,0 @@
-import _ from "underscore";
-import { formatExpression, parseExpressionString, tokenAtPosition, tokensToExpression } from "metabase/lib/expressions";
-
-const mockFields = [
- {id: 1, display_name: "A"},
- {id: 2, display_name: "B"},
- {id: 3, display_name: "C"},
- {id: 10, display_name: "Toucan Sam"}
-];
-
-const mathOperators = new Set(['+', '-', '*', '/']);
-
-const parsedMathOperators = {
- "+": { value: '+', start: 2, end: 3, parsedValue: '+' },
- "-": { value: '-', start: 2, end: 3, parsedValue: '-' },
- "*": { value: '*', start: 2, end: 3, parsedValue: '*' },
- "/": { value: '/', start: 2, end: 3, parsedValue: '/' }
-}
-
-function stripStartEnd(list) {
- return list.map(i => {
- delete i.start;
- delete i.end;
-
- if (_.isArray(i.value)) {
- i.value = stripStartEnd(i.value);
- }
-
- return i;
- });
-}
-
-
-describe("parseExpressionString", () => {
- it("should return empty array for null or empty string", () => {
- expect(parseExpressionString()).toEqual([]);
- expect(parseExpressionString(null)).toEqual([]);
- expect(parseExpressionString("")).toEqual([]);
- });
-
- // simplest possible expression
- it("can parse single operator math", () => {
- expect(stripStartEnd(parseExpressionString("A - B", mockFields, mathOperators)))
- .toEqual([
- { value: 'A', suggestions: [mockFields[0], mockFields[3]], parsedValue: [ 'field-id', 1 ] },
- { value: '-', parsedValue: '-' },
- { value: 'B', suggestions: [], parsedValue: [ 'field-id', 2 ] }
- ]);
- });
-
- it("can parse function with no arguments", () => {
- expect(stripStartEnd(parseExpressionString("count()", mockFields, mathOperators)))
- .toEqual([
- { value: 'count', parsedValue: [ 'count' ] }
- ]);
- });
-
- // quoted field name w/ a space in it
- it("can parse a field with quotes and spaces", () => {
- expect(stripStartEnd(parseExpressionString("\"Toucan Sam\" + B", mockFields, mathOperators)))
- .toEqual([
- { value: 'Toucan Sam', suggestions: [], parsedValue: [ 'field-id', 10 ] },
- { value: '+', parsedValue: '+' },
- { value: 'B', suggestions: [], parsedValue: [ 'field-id', 2 ] }
- ]);
- });
-
- // parentheses / nested parens
- it("can parse expressions with parentheses", () => {
- expect(stripStartEnd(parseExpressionString("\"Toucan Sam\" + (A * (B / C))", mockFields, mathOperators)))
- .toEqual([
- { value: 'Toucan Sam', suggestions: [], parsedValue: [ 'field-id', 10 ] },
- { value: '+', parsedValue: '+' },
- { value: [{ value: 'A', suggestions: [mockFields[0], mockFields[3]], parsedValue: [ 'field-id', 1 ] },
- { value: '*', parsedValue: '*' },
- { value: [{ value: 'B', suggestions: [], parsedValue: [ 'field-id', 2 ] },
- { value: '/', parsedValue: '/' },
- { value: 'C', suggestions: [mockFields[2], mockFields[3]], parsedValue: [ 'field-id', 3 ] }],
- isParent: true}],
- isParent: true }
- ]);
- });
-
- // fks
- // multiple tables with the same field name resolution
-});
-
-
-describe("formatExpression", () => {
- it("should return empty array for null or empty string", () => {
- expect(parseExpressionString()).toEqual([]);
- expect(parseExpressionString(null)).toEqual([]);
- expect(parseExpressionString("")).toEqual([]);
- });
-
- it("can format simple expressions", () => {
- expect(formatExpression(["+", ["field-id", 1], ["field-id", 2]], mockFields)).toEqual("A + B");
- });
-
- it("can format expressions with parentheses", () => {
- expect(formatExpression(["+", ["/", ["field-id", 1], ["field-id", 2]], ["field-id", 3]], mockFields)).toEqual("(A / B) + C");
- expect(formatExpression(["+", ["/", ["field-id", 1], ["*", ["field-id", 2], ["field-id", 2]]], ["field-id", 3]], mockFields)).toEqual("(A / (B * B)) + C");
- });
-
- it("quotes fields with spaces in them", () => {
- expect(formatExpression(["+", ["/", ["field-id", 1], ["field-id", 10]], ["field-id", 3]], mockFields)).toEqual("(A / \"Toucan Sam\") + C");
- });
-
- it("format aggregations", () => {
- expect(formatExpression(["count"], mockFields)).toEqual("count()");
- expect(formatExpression(["sum", ["field-id", 1]], mockFields)).toEqual("sum(A)");
- });
-
- it("nested aggregation", () => {
- expect(formatExpression(["+", 1, ["count"]], mockFields)).toEqual("1 + count()");
- expect(formatExpression(["/", ["sum", ["field-id", 1]], ["count"]], mockFields)).toEqual("sum(A) / count()");
- });
-
- it("aggregation with expressions", () => {
- // TODO: get rid of the extra parens
- expect(formatExpression(["sum", ["/", ["field-id", 1], ["field-id", 2]]], mockFields)).toEqual("sum((A / B))");
- });
-});
diff --git a/frontend/test/unit/lib/expressions/formatter.spec.js b/frontend/test/unit/lib/expressions/formatter.spec.js
new file mode 100644
index 00000000000..00a65395f86
--- /dev/null
+++ b/frontend/test/unit/lib/expressions/formatter.spec.js
@@ -0,0 +1,54 @@
+import { format } from "metabase/lib/expressions/formatter";
+
+const mockMetadata = {
+ tableMetadata: {
+ fields: [
+ {id: 1, display_name: "A"},
+ {id: 2, display_name: "B"},
+ {id: 3, display_name: "C"},
+ {id: 10, display_name: "Toucan Sam"}
+ ],
+ metrics: [
+ {id: 1, name: "foo bar"},
+ ]
+ }
+}
+
+fdescribe("lib/expressions/parser", () => {
+ describe("format", () => {
+ it("can format simple expressions", () => {
+ expect(format(["+", ["field-id", 1], ["field-id", 2]], mockMetadata)).toEqual("A + B");
+ });
+
+ it("can format expressions with parentheses", () => {
+ expect(format(["+", ["/", ["field-id", 1], ["field-id", 2]], ["field-id", 3]], mockMetadata)).toEqual("(A / B) + C");
+ expect(format(["+", ["/", ["field-id", 1], ["*", ["field-id", 2], ["field-id", 2]]], ["field-id", 3]], mockMetadata)).toEqual("(A / (B * B)) + C");
+ });
+
+ it("quotes fields with spaces in them", () => {
+ expect(format(["+", ["/", ["field-id", 1], ["field-id", 10]], ["field-id", 3]], mockMetadata)).toEqual("(A / \"Toucan Sam\") + C");
+ });
+
+ it("format aggregations", () => {
+ expect(format(["count"], mockMetadata)).toEqual("Count()");
+ expect(format(["sum", ["field-id", 1]], mockMetadata)).toEqual("Sum(A)");
+ });
+
+ it("nested aggregation", () => {
+ expect(format(["+", 1, ["count"]], mockMetadata)).toEqual("1 + Count()");
+ expect(format(["/", ["sum", ["field-id", 1]], ["count"]], mockMetadata)).toEqual("Sum(A) / Count()");
+ });
+
+ it("aggregation with expressions", () => {
+ expect(format(["sum", ["/", ["field-id", 1], ["field-id", 2]]], mockMetadata)).toEqual("Sum(A / B)");
+ });
+
+ it("expression with metric", () => {
+ expect(format(["+", 1, ["METRIC", 1]], mockMetadata)).toEqual("1 + FooBar()");
+ });
+
+ it("expression with custom field", () => {
+ expect(format(["+", 1, ["sum", ["expression", "foo bar"]]], mockMetadata)).toEqual("1 + Sum(\"foo bar\")");
+ });
+ });
+});
diff --git a/frontend/test/unit/lib/expressions/parser.spec.js b/frontend/test/unit/lib/expressions/parser.spec.js
index b9ec57bff5e..ca5ac4addc3 100644
--- a/frontend/test/unit/lib/expressions/parser.spec.js
+++ b/frontend/test/unit/lib/expressions/parser.spec.js
@@ -8,7 +8,7 @@ const mockFields = [
{id: 10, display_name: "Toucan Sam"}
];
-fdescribe("lib/expressions/parser", () => {
+describe("lib/expressions/parser", () => {
describe("compile()", () => {
it("should return empty array for null or empty string", () => {
expect(compile()).toEqual([]);
--
GitLab