diff --git a/frontend/src/metabase/lib/expressions/suggest.js b/frontend/src/metabase/lib/expressions/suggest.js
index e859f552b689f1cf43a4bf872a73b8c2c5cf6bd0..e4df647a6a8738ce348a1c003ab67cbf345d67cb 100644
--- a/frontend/src/metabase/lib/expressions/suggest.js
+++ b/frontend/src/metabase/lib/expressions/suggest.js
@@ -1,79 +1,37 @@
import _ from "underscore";
import {
- getExpressionName,
- // dimensions:
getDimensionName,
formatDimensionName,
- // metrics
formatMetricName,
- // segments
formatSegmentName,
} from "../expressions";
-import {
- parserWithRecovery,
- ExpressionCstVisitor,
- ExpressionParser,
-} from "./parser";
-
-import {
- AggregationFunctionName,
- CLAUSE_TOKENS,
- Case,
- FunctionName,
- Identifier,
- IdentifierString,
- LParen,
- Minus,
- NumberLiteral,
- StringLiteral,
- isTokenType,
- lexerWithRecovery,
-} from "./lexer";
-
import { partialMatch, enclosingFunction } from "./completer";
import getHelpText from "./helper_text_strings";
-import { ExpressionDimension } from "metabase-lib/lib/Dimension";
import {
- FUNCTIONS,
- OPERATORS,
+ EXPRESSION_FUNCTIONS,
+ AGGREGATION_FUNCTIONS,
MBQL_CLAUSES,
- isExpressionType,
- getFunctionArgType,
getMBQLName,
- EXPRESSION_TYPES,
EDITOR_FK_SYMBOLS,
} from "./config";
-const FUNCTIONS_BY_TYPE = {};
-const OPERATORS_BY_TYPE = {};
-for (const type of EXPRESSION_TYPES) {
- FUNCTIONS_BY_TYPE[type] = Array.from(FUNCTIONS)
- .filter(name => isExpressionType(MBQL_CLAUSES[name].type, type))
- .map(name => MBQL_CLAUSES[name]);
- OPERATORS_BY_TYPE[type] = Array.from(OPERATORS)
- .filter(name => isExpressionType(MBQL_CLAUSES[name].type, type))
- .map(name => MBQL_CLAUSES[name]);
-}
-
export function suggest({
source,
- cst,
query,
startRule,
targetOffset = source.length,
- expressionName,
} = {}) {
- const partialSource = source.slice(0, targetOffset);
+ let suggestions = [];
+ const partialSource = source.slice(0, targetOffset);
const matchPrefix = partialMatch(partialSource);
- const partialSuggestionMode =
- matchPrefix && matchPrefix.length > 0 && _.last(matchPrefix) !== "]";
- if (!partialSuggestionMode) {
+ if (!matchPrefix || _.last(matchPrefix) === "]") {
+ // no keystroke to match? show help text for the enclosing function
const functionDisplayName = enclosingFunction(partialSource);
if (functionDisplayName) {
const helpText = getHelpText(getMBQLName(functionDisplayName));
@@ -81,409 +39,106 @@ export function suggest({
return { helpText };
}
}
+ return { suggestions };
}
- const lexResult = lexerWithRecovery.tokenize(partialSource);
- if (lexResult.errors.length > 0) {
- throw lexResult.errors;
- }
- let tokenVector = lexResult.tokens;
- if (partialSuggestionMode) {
- tokenVector = tokenVector.slice(0, -1);
+ if (_.first(matchPrefix) !== "[") {
+ suggestions.push({
+ type: "functions",
+ name: "case",
+ text: "case(",
+ index: targetOffset,
+ });
+ suggestions.push(
+ ...Array.from(EXPRESSION_FUNCTIONS)
+ .map(name => MBQL_CLAUSES[name])
+ .map(func => ({
+ type: "functions",
+ name: func.displayName,
+ text: func.displayName + "(",
+ index: targetOffset,
+ })),
+ );
+ if (startRule === "aggregation") {
+ suggestions.push(
+ ...Array.from(AGGREGATION_FUNCTIONS)
+ .map(name => MBQL_CLAUSES[name])
+ .map(func => ({
+ type: "aggregations",
+ name: func.displayName,
+ text: func.displayName + "(",
+ index: targetOffset,
+ })),
+ );
+ }
}
- const context = getContext({
- cst,
- tokenVector,
- targetOffset,
- startRule,
- }) || { expectedType: startRule };
-
- const { expectedType } = context;
-
- let finalSuggestions = [];
-
- const syntacticSuggestions = parserWithRecovery.computeContentAssist(
- startRule,
- tokenVector,
- );
- for (const suggestion of syntacticSuggestions) {
- const { nextTokenType, ruleStack } = suggestion;
-
- // first to avoid skipping if lastInputTokenIsUnclosedIdentifierString
- if (nextTokenType === Identifier || nextTokenType === IdentifierString) {
- // fields, metrics, segments
- const parentRule = ruleStack.slice(-2, -1)[0];
- const isDimension =
- parentRule === "identifierExpression" &&
- (isExpressionType(expectedType, "expression") ||
- isExpressionType(expectedType, "boolean"));
- const isSegment =
- parentRule === "identifierExpression" &&
- isExpressionType(expectedType, "boolean");
- const isMetric =
- parentRule === "identifierExpression" &&
- isExpressionType(expectedType, "aggregation");
-
- if (isDimension) {
- let dimensions = [];
- if (
- context.token &&
- context.clause &&
- isTokenType(context.token.tokenType, AggregationFunctionName)
- ) {
- dimensions = query.aggregationFieldOptions(context.clause.name).all();
- } else {
- const dimensionFilter = dimension => {
- // not itself
- if (
- dimension instanceof ExpressionDimension &&
- dimension.name() === expressionName
- ) {
- return false;
- }
- if (expectedType === "expression" || expectedType === "boolean") {
- return true;
- }
- const field = dimension.field();
- return (
- (isExpressionType("number", expectedType) && field.isNumeric()) ||
- (isExpressionType("string", expectedType) && field.isString())
- );
- };
- dimensions = query.dimensionOptions(dimensionFilter).all();
- }
- finalSuggestions.push(
- ...dimensions.map(dimension => ({
- type: "fields",
- name: getDimensionName(dimension),
- text: formatDimensionName(dimension) + " ",
- alternates: EDITOR_FK_SYMBOLS.symbols.map(symbol =>
- getDimensionName(dimension, symbol),
- ),
- })),
- );
- }
- if (isSegment) {
- finalSuggestions.push(
- ...query.table().segments.map(segment => ({
- type: "segments",
- name: segment.name,
- text: formatSegmentName(segment),
- })),
- );
- }
- if (isMetric) {
- finalSuggestions.push(
- ...query.table().metrics.map(metric => ({
- type: "metrics",
- name: metric.name,
- text: formatMetricName(metric),
- })),
- );
- }
- } else if (
- isTokenType(nextTokenType, FunctionName) ||
- nextTokenType === Case
- ) {
- const database = query.database();
- let functions = [];
- if (isExpressionType(expectedType, "aggregation")) {
- // special case for aggregation
- finalSuggestions.push(
- // ...query
- // .aggregationOperatorsWithoutRows()
- // .filter(a => getExpressionName(a.short))
- // .map(aggregationOperator =>
- // functionSuggestion(
- // "aggregations",
- // aggregationOperator.short,
- // aggregationOperator.fields.length > 0,
- // ),
- // ),
- ...FUNCTIONS_BY_TYPE["aggregation"]
- .filter(clause => database.hasFeature(clause.requiresFeature))
- .map(clause =>
- functionSuggestion(
- "aggregations",
- clause.name,
- clause.args.length > 0,
- ),
- ),
- );
- finalSuggestions.push(
- ...["sum-where", "count-where", "share"].map(short =>
- functionSuggestion("aggregations", short, true),
+ if (_.last(matchPrefix) !== "]") {
+ suggestions.push(
+ ...query
+ .dimensionOptions(() => true)
+ .all()
+ .map(dimension => ({
+ type: "fields",
+ name: getDimensionName(dimension),
+ text: formatDimensionName(dimension) + " ",
+ alternates: EDITOR_FK_SYMBOLS.symbols.map(symbol =>
+ getDimensionName(dimension, symbol),
),
- );
- functions = FUNCTIONS_BY_TYPE["number"];
- } else {
- functions = FUNCTIONS_BY_TYPE[expectedType];
- }
- finalSuggestions.push(
- ...functions
- .filter(clause => database.hasFeature(clause.requiresFeature))
- .map(clause => functionSuggestion("functions", clause.name)),
+ index: targetOffset,
+ })),
+ );
+ suggestions.push(
+ ...query.table().segments.map(segment => ({
+ type: "segments",
+ name: segment.name,
+ text: formatSegmentName(segment),
+ index: targetOffset,
+ })),
+ );
+ if (startRule === "aggregation") {
+ suggestions.push(
+ ...query.table().metrics.map(metric => ({
+ type: "metrics",
+ name: metric.name,
+ text: formatMetricName(metric),
+ index: targetOffset,
+ })),
);
- if (nextTokenType === Case) {
- const caseSuggestion = {
- type: "functions",
- name: "case",
- text: "case(",
- };
- finalSuggestions.push(caseSuggestion);
- }
- } else if (
- nextTokenType === StringLiteral ||
- nextTokenType === NumberLiteral ||
- nextTokenType === Minus
- ) {
- // skip number/string literal
- } else {
- console.warn("non exhaustive match", nextTokenType.name);
}
}
// throw away any suggestion that is not a suffix of the last partialToken.
- if (partialSuggestionMode) {
- const partial = matchPrefix.toLowerCase();
- for (const suggestion of finalSuggestions) {
- suggestion: for (const text of [
- suggestion.name,
- suggestion.text,
- ...(suggestion.alternates || []),
- ]) {
- const lower = (text || "").toLowerCase();
- if (lower.startsWith(partial)) {
- suggestion.range = [0, partial.length];
+ const partial = matchPrefix.toLowerCase();
+ for (const suggestion of suggestions) {
+ suggestion: for (const text of [
+ suggestion.name,
+ suggestion.text,
+ ...(suggestion.alternates || []),
+ ]) {
+ const lower = (text || "").toLowerCase();
+ if (lower.startsWith(partial)) {
+ suggestion.range = [0, partial.length];
+ break suggestion;
+ }
+ let index = 0;
+ for (const part of lower.split(/\b/g)) {
+ if (part.startsWith(partial)) {
+ suggestion.range = [index, index + partial.length];
break suggestion;
}
- let index = 0;
- for (const part of lower.split(/\b/g)) {
- if (part.startsWith(partial)) {
- suggestion.range = [index, index + partial.length];
- break suggestion;
- }
- index += part.length;
- }
+ index += part.length;
}
}
- finalSuggestions = finalSuggestions.filter(suggestion => suggestion.range);
- }
- for (const suggestion of finalSuggestions) {
- suggestion.index = targetOffset;
- if (!suggestion.name) {
- suggestion.name = suggestion.text;
- }
}
+ suggestions = suggestions.filter(suggestion => suggestion.range);
// deduplicate suggestions and sort by type then name
return {
- suggestions: _.chain(finalSuggestions)
+ suggestions: _.chain(suggestions)
.uniq(suggestion => suggestion.text)
- .sortBy("name")
+ .sortBy("text")
.sortBy("type")
.value(),
};
}
-
-function functionSuggestion(type, clause, parens = true) {
- const name = getExpressionName(clause);
- return {
- type: type,
- name: name,
- text: name + (parens ? "(" : " "),
- };
-}
-
-const contextParser = new ExpressionParser({
- recoveryEnabled: true,
- tokenRecoveryEnabled: false,
-});
-
-export function getContext({
- source,
- cst,
- tokenVector = lexerWithRecovery.tokenize(source).tokens,
- targetOffset = source.length,
- startRule,
- ...options
-}) {
- if (!cst) {
- contextParser.input = tokenVector;
- cst = contextParser[startRule]();
- }
- const visitor = new ExpressionContextVisitor({
- targetOffset: targetOffset,
- tokenVector: tokenVector,
- ...options,
- });
- return visitor.visit(cst);
-}
-
-function findNextTextualToken(tokenVector, prevTokenEndOffset) {
- // The TokenVector is sorted, so we could use a BinarySearch to optimize performance
- const prevTokenIdx = tokenVector.findIndex(
- tok => tok.endOffset === prevTokenEndOffset,
- );
- for (let i = prevTokenIdx + 1; i >= 0 && i < tokenVector.length; i++) {
- if (!/^\s+$/.test(tokenVector[i].image)) {
- return tokenVector[i];
- }
- }
- return null;
-}
-
-export class ExpressionContextVisitor extends ExpressionCstVisitor {
- constructor({ targetOffset, tokenVector }) {
- super();
- this.targetOffset = targetOffset;
- this.tokenVector = tokenVector;
- this.stack = [];
- this.validateVisitor();
- }
-
- _context(clauseToken, index, currentContext) {
- const clause = CLAUSE_TOKENS.get(clauseToken.tokenType);
- let expectedType = getFunctionArgType(clause, index);
-
- if (
- (expectedType === "expression" || expectedType === "number") &&
- currentContext &&
- currentContext.expectedType === "aggregation" &&
- clause.type !== "aggregation"
- ) {
- expectedType = "aggregation";
- }
-
- return { clause, index, expectedType, clauseToken };
- }
-
- _isTarget(token) {
- const { targetOffset, tokenVector } = this;
- const next = findNextTextualToken(tokenVector, token.endOffset);
- if (
- targetOffset > token.endOffset &&
- (next === null || targetOffset <= next.startOffset)
- ) {
- return true;
- }
- return false;
- }
-
- _function(ctx, currentContext) {
- const { tokenVector } = this;
- const clauseToken = ctx.functionName[0];
- // special case: function clauses without parens sometimes causes the paren to be missing
- const parenToken = ctx.LParen
- ? ctx.LParen[0]
- : findNextTextualToken(tokenVector, clauseToken.endOffset);
- if (!parenToken || parenToken.tokenType !== LParen) {
- return;
- }
- const tokens = [parenToken, ...(ctx.Comma || [])];
- for (let index = 0; index < tokens.length; index++) {
- const token = tokens[index];
- if (this._isTarget(token)) {
- return this._context(clauseToken, index, currentContext);
- } else if (ctx.arguments && index < ctx.arguments.length) {
- const match = this.visit(
- ctx.arguments[index],
- this._context(clauseToken, index, currentContext),
- );
- if (match) {
- return match;
- }
- }
- }
- }
-
- _operator(ctx, currentContext) {
- // note: this should probably account for operator precedence / associativity but for now all of our operator clauses contain operators of the same precedence/associativity
- for (let index = 0; index < ctx.operators.length; index++) {
- const clauseToken = ctx.operators[index];
- if (this._isTarget(clauseToken)) {
- // NOTE: operators always (?) have the same type for every operand
- return this._context(clauseToken, 0, currentContext);
- } else {
- const match = this.visit(
- ctx.operands[index],
- this._context(clauseToken, 0, currentContext),
- );
- if (match) {
- return match;
- }
- }
- }
- }
-}
-
-const ALL_RULES = [
- "any",
- "expression",
- "aggregation",
- "boolean",
- "string",
- "number",
- "additionExpression",
- "multiplicationExpression",
- "functionExpression",
- "caseExpression",
- "identifierExpression",
- "identifier",
- "identifierString",
- "stringLiteral",
- "numberLiteral",
- "atomicExpression",
- "parenthesisExpression",
- "booleanExpression",
- "booleanUnaryExpression",
- "relationalExpression",
- "logicalAndExpression",
- "logicalOrExpression",
- "logicalNotExpression",
-];
-
-const TYPE_RULES = new Set([
- "expression",
- "aggregation",
- "boolean",
- "string",
- "number",
-]);
-
-for (const rule of ALL_RULES) {
- ExpressionContextVisitor.prototype[rule] = function(ctx, currentContext) {
- if (!currentContext && TYPE_RULES.has(rule)) {
- currentContext = { expectedType: rule };
- }
-
- // if we have operators or a functionName then handle that specially
- if (ctx.operators) {
- const match = this._operator(ctx, currentContext);
- if (match) {
- return match;
- }
- }
- if (ctx.functionName) {
- const match = this._function(ctx, currentContext);
- if (match) {
- return match;
- }
- }
-
- // this just visits every child
- for (const type in ctx) {
- for (const child of ctx[type]) {
- if (!child.tokenType && child.name) {
- const match = this.visit(child, currentContext);
- if (match) {
- return match;
- }
- } else if (this._isTarget(child)) {
- return currentContext;
- }
- }
- }
- };
-}
diff --git a/frontend/test/metabase/lib/expressions/suggest.unit.spec.js b/frontend/test/metabase/lib/expressions/suggest.unit.spec.js
index 7467e5fd11c2df2abd445258764d3c5f9edb89df..dd2ec0097d0191833524712f811181b3ab36c6df 100644
--- a/frontend/test/metabase/lib/expressions/suggest.unit.spec.js
+++ b/frontend/test/metabase/lib/expressions/suggest.unit.spec.js
@@ -1,95 +1,11 @@
-import {
- suggest as suggest_,
- getContext as getContext_,
-} from "metabase/lib/expressions/suggest";
+import { suggest as suggest_ } from "metabase/lib/expressions/suggest";
import _ from "underscore";
-import {
- aggregationOpts,
- expressionOpts,
- filterOpts,
-} from "./__support__/expressions";
+import { aggregationOpts, expressionOpts } from "./__support__/expressions";
import { ORDERS, REVIEWS } from "__support__/sample_dataset_fixture";
-const AGGREGATION_FUNCTIONS = [
- { type: "aggregations", text: "Average(" },
- { type: "aggregations", text: "Count " },
- { type: "aggregations", text: "CountIf(" },
- { type: "aggregations", text: "CumulativeCount " },
- { type: "aggregations", text: "CumulativeSum(" },
- { type: "aggregations", text: "Distinct(" },
- { type: "aggregations", text: "Max(" },
- { type: "aggregations", text: "Median(" },
- { type: "aggregations", text: "Min(" },
- { type: "aggregations", text: "Percentile(" },
- { type: "aggregations", text: "Share(" },
- { type: "aggregations", text: "StandardDeviation(" },
- { type: "aggregations", text: "Sum(" },
- { type: "aggregations", text: "SumIf(" },
- { type: "aggregations", text: "Variance(" },
-];
-const STRING_FUNCTIONS = [
- { text: "concat(", type: "functions" },
- { text: "lower(", type: "functions" },
- { text: "ltrim(", type: "functions" },
- { text: "regexextract(", type: "functions" },
- { text: "rtrim(", type: "functions" },
- { text: "replace(", type: "functions" },
- { text: "substring(", type: "functions" },
- { text: "trim(", type: "functions" },
- { text: "upper(", type: "functions" },
-];
-const NUMERIC_FUNCTIONS = [
- { text: "abs(", type: "functions" },
- { text: "ceil(", type: "functions" },
- { text: "exp(", type: "functions" },
- { text: "floor(", type: "functions" },
- { text: "length(", type: "functions" },
- { text: "log(", type: "functions" },
- { text: "power(", type: "functions" },
- { text: "round(", type: "functions" },
- { text: "sqrt(", type: "functions" },
-];
-
-const STRING_FUNCTIONS_EXCLUDING_REGEX = STRING_FUNCTIONS.filter(
- ({ text }) => text !== "regexextract(",
-);
-// const EXPRESSION_FUNCTIONS = [
-// { text: "case(", type: "functions" },
-// { text: "coalesce(", type: "functions" },
-// ];
-const FILTER_FUNCTIONS = [
- { text: "between(", type: "functions" },
- { text: "contains(", type: "functions" },
- { text: "endsWith(", type: "functions" },
- { text: "interval(", type: "functions" },
- { text: "isempty(", type: "functions" },
- { text: "isnull(", type: "functions" },
- { text: "startsWith(", type: "functions" },
-];
-
-// custom metadata defined in __support__/expressions
-const METRICS_CUSTOM = [{ type: "metrics", text: "[metric]" }];
-const FIELDS_CUSTOM = [
- { type: "fields", text: "[A] " },
- { type: "fields", text: "[B] " },
- { type: "fields", text: "[C] " },
- // quoted because conflicts with aggregation
- { type: "fields", text: "[Sum] " },
- // quoted because has a space
- { type: "fields", text: "[Toucan Sam] " },
- // quoted because conflicts with aggregation
- { type: "fields", text: "[count] " },
-];
-
-const FIELDS_CUSTOM_NON_NUMERIC = [
- { type: "fields", text: "[date] " },
- { type: "fields", text: "[text] " },
-];
-
// custom metadata defined in __support__/sample_dataset_fixture
-const METRICS_ORDERS = [{ type: "metrics", text: "[Total Order Value]" }];
const SEGMENTS_ORDERS = [{ text: "[Expensive Things]", type: "segments" }];
const FIELDS_ORDERS = [
{ text: "[Created At] ", type: "fields" },
@@ -122,6 +38,25 @@ const FIELDS_ORDERS = [
{ text: "[User → Zip] ", type: "fields" },
];
+// custom metadata defined in __support__/expressions
+const METRICS_CUSTOM = [{ type: "metrics", text: "[metric]" }];
+const FIELDS_CUSTOM = [
+ { type: "fields", text: "[A] " },
+ { type: "fields", text: "[B] " },
+ { type: "fields", text: "[C] " },
+ // quoted because conflicts with aggregation
+ { type: "fields", text: "[Sum] " },
+ // quoted because has a space
+ { type: "fields", text: "[Toucan Sam] " },
+ // quoted because conflicts with aggregation
+ { type: "fields", text: "[count] " },
+];
+
+const FIELDS_CUSTOM_NON_NUMERIC = [
+ { type: "fields", text: "[date] " },
+ { type: "fields", text: "[text] " },
+];
+
describe("metabase/lib/expression/suggest", () => {
describe("suggest()", () => {
function suggest(...args) {
@@ -133,41 +68,26 @@ describe("metabase/lib/expression/suggest", () => {
}
describe("expression", () => {
- it("should suggest expression functions, and fields in an expression", () => {
- expect(suggest({ source: "", ...expressionOpts })).toEqual([
- ...FIELDS_CUSTOM,
- ...FIELDS_CUSTOM_NON_NUMERIC,
- ...[
- { type: "functions", text: "case(" },
- { type: "functions", text: "coalesce(" },
- ...NUMERIC_FUNCTIONS,
- ...STRING_FUNCTIONS_EXCLUDING_REGEX,
- ].sort(suggestionSort),
- ]);
- });
-
- it("should suggest numeric fields after an aritmetic", () => {
- expect(suggest({ source: "1 + ", ...expressionOpts })).toEqual([
- ...FIELDS_CUSTOM,
- ...[{ type: "functions", text: "case(" }, ...NUMERIC_FUNCTIONS].sort(
- suggestionSort,
- ),
- ]);
- });
it("should suggest partial matches in expression", () => {
expect(suggest({ source: "1 + C", ...expressionOpts })).toEqual([
{ type: "fields", text: "[C] " },
{ type: "fields", text: "[count] " },
{ type: "functions", text: "case(" },
{ type: "functions", text: "ceil(" },
+ // FIXME: the last three should not appear
+ { type: "functions", text: "coalesce(" },
+ { type: "functions", text: "concat(" },
+ { type: "functions", text: "contains(" },
]);
});
+
it("should suggest partial matches in unterminated quoted string", () => {
expect(suggest({ source: "1 + [C", ...expressionOpts })).toEqual([
{ type: "fields", text: "[C] " },
{ type: "fields", text: "[count] " },
]);
});
+
it("should suggest foreign fields", () => {
expect(
suggest({
@@ -192,6 +112,7 @@ describe("metabase/lib/expression/suggest", () => {
{ text: "[User → Zip] ", type: "fields" },
]);
});
+
it("should suggest joined fields", () => {
expect(
suggest({
@@ -211,10 +132,11 @@ describe("metabase/lib/expression/suggest", () => {
{ text: "[Foo → Reviewer] ", type: "fields" },
]);
});
+
it("should suggest nested query fields", () => {
expect(
suggest({
- source: "",
+ source: "T",
query: ORDERS.query()
.aggregate(["count"])
.breakout(ORDERS.TOTAL)
@@ -223,12 +145,8 @@ describe("metabase/lib/expression/suggest", () => {
}),
).toEqual(
[
- { text: "[Count] ", type: "fields" },
{ text: "[Total] ", type: "fields" },
- { type: "functions", text: "case(" },
- { text: "coalesce(", type: "functions" },
- ...STRING_FUNCTIONS,
- ...NUMERIC_FUNCTIONS,
+ { text: "trim(", type: "functions" },
].sort(suggestionSort),
);
});
@@ -262,67 +180,80 @@ describe("metabase/lib/expression/suggest", () => {
}).name,
).toEqual("coalesce");
});
-
- xit("should suggest boolean options after case(", () => {
- expect(
- suggest({
- source: "case(",
- query: ORDERS.query(),
- startRule: "expression",
- }),
- ).toEqual([...SEGMENTS_ORDERS]);
- });
});
describe("aggregation", () => {
+ it("should suggest aggregations and metrics", () => {
+ expect(suggest({ source: "case([", ...aggregationOpts })).toEqual(
+ [
+ ...FIELDS_CUSTOM,
+ ...METRICS_CUSTOM,
+ ...FIELDS_CUSTOM_NON_NUMERIC,
+ { type: "segments", text: "[segment]" },
+ ].sort(suggestionSort),
+ );
+ });
+
it("should suggest partial matches after an aggregation", () => {
expect(suggest({ source: "average(c", ...aggregationOpts })).toEqual([
+ // FIXME: the next four should not appear
+ { type: "aggregations", text: "Count(" },
+ { type: "aggregations", text: "CountIf(" },
+ { type: "aggregations", text: "CumulativeCount(" },
+ { type: "aggregations", text: "CumulativeSum(" },
{ type: "fields", text: "[C] " },
{ type: "fields", text: "[count] " },
- { text: "case(", type: "functions" },
- // { text: "coalesce(", type: "functions" },
- { text: "ceil(", type: "functions" },
- ]);
- });
- it("should suggest aggregations and metrics after an operator", () => {
- expect(suggest({ source: "1 + ", ...aggregationOpts })).toEqual([
- ...[
- { type: "functions", text: "case(" },
- ...AGGREGATION_FUNCTIONS,
- ...NUMERIC_FUNCTIONS,
- ].sort(suggestionSort),
- ...METRICS_CUSTOM,
+ { type: "functions", text: "case(" },
+ { type: "functions", text: "ceil(" },
+ { type: "functions", text: "coalesce(" },
+ { type: "functions", text: "concat(" },
+ { type: "functions", text: "contains(" },
]);
});
+
it("should suggest partial matches in aggregation", () => {
expect(suggest({ source: "1 + C", ...aggregationOpts })).toEqual([
- { type: "aggregations", text: "Count " },
+ { type: "aggregations", text: "Count(" },
{ type: "aggregations", text: "CountIf(" },
- { type: "aggregations", text: "CumulativeCount " },
+ { type: "aggregations", text: "CumulativeCount(" },
{ type: "aggregations", text: "CumulativeSum(" },
+ { type: "fields", text: "[C] " },
+ { type: "fields", text: "[count] " },
{ type: "functions", text: "case(" },
{ type: "functions", text: "ceil(" },
+ { type: "functions", text: "coalesce(" },
+ { type: "functions", text: "concat(" },
+ { type: "functions", text: "contains(" },
]);
});
- it("should suggest aggregations and metrics in an aggregation", () => {
+ it("should show suggestions with matched 2-char prefix", () => {
expect(
suggest({
- source: "",
+ source: "to",
query: ORDERS.query(),
startRule: "aggregation",
}),
).toEqual([
- ...[
- { type: "functions", text: "case(" },
- ...AGGREGATION_FUNCTIONS,
- ...NUMERIC_FUNCTIONS,
- ].sort(suggestionSort),
- ...METRICS_ORDERS,
+ { type: "metrics", text: "[Total Order Value]" },
+ { type: "fields", text: "[Total] " },
+ ]);
+ });
+
+ it("should show suggestions with matched 3-char prefix", () => {
+ expect(
+ suggest({
+ source: "cou",
+ query: ORDERS.query(),
+ startRule: "aggregation",
+ }),
+ ).toEqual([
+ { type: "aggregations", text: "Count(" },
+ { type: "aggregations", text: "CountIf(" },
]);
});
- it("should show help text in an aggregation functiom", () => {
+ it("should show help text in an aggregation function", () => {
const { name, example } = helpText({
source: "Sum(",
query: ORDERS.query(),
@@ -334,18 +265,42 @@ describe("metabase/lib/expression/suggest", () => {
});
describe("filter", () => {
- it("should suggest filter functions, fields, and segments in a filter", () => {
+ it("should show suggestions with matched 1-char prefix", () => {
+ expect(
+ suggest({ source: "c", query: ORDERS.query(), startRule: "boolean" }),
+ ).toEqual([
+ { type: "fields", text: "[Created At] " },
+ { type: "fields", text: "[Product → Category] " },
+ { type: "fields", text: "[Product → Created At] " },
+ { type: "fields", text: "[User → City] " },
+ { type: "fields", text: "[User → Created At] " },
+ { type: "functions", text: "case(" },
+ { type: "functions", text: "ceil(" },
+ { type: "functions", text: "coalesce(" },
+ { type: "functions", text: "concat(" },
+ { type: "functions", text: "contains(" },
+ ]);
+ });
+
+ it("should show suggestions with matched 2-char prefix", () => {
expect(
- suggest({ source: "", query: ORDERS.query(), startRule: "boolean" }),
+ suggest({
+ source: "ca",
+ query: ORDERS.query(),
+ startRule: "boolean",
+ }),
).toEqual([
- ...FIELDS_ORDERS,
- ...[{ type: "functions", text: "case(" }, ...FILTER_FUNCTIONS].sort(
- suggestionSort,
- ),
- ...SEGMENTS_ORDERS,
+ { type: "fields", text: "[Product → Category] " },
+ { type: "functions", text: "case(" },
]);
});
+ it("should show all fields when '[' appears", () => {
+ expect(
+ suggest({ source: "[", query: ORDERS.query(), startRule: "boolean" }),
+ ).toEqual([...FIELDS_ORDERS, ...SEGMENTS_ORDERS].sort(suggestionSort));
+ });
+
it("should show help text in a filter function", () => {
const { name, example } = helpText({
source: "Contains(Total ",
@@ -357,134 +312,14 @@ describe("metabase/lib/expression/suggest", () => {
});
});
});
-
- describe("getContext", () => {
- function getContext(...args) {
- return cleanContext(getContext_(...args));
- }
-
- describe("aggregation", () => {
- it("should get operator context", () => {
- expect(getContext({ source: "1 +", ...aggregationOpts })).toEqual({
- clause: "+",
- expectedType: "aggregation",
- index: 0,
- });
- });
- it("should get operator context with trailing whitespace", () => {
- expect(getContext({ source: "1 + ", ...aggregationOpts })).toEqual({
- clause: "+",
- expectedType: "aggregation",
- index: 0,
- });
- });
- it("should get aggregation context", () => {
- expect(getContext({ source: "Average(", ...aggregationOpts })).toEqual({
- clause: "avg",
- expectedType: "number",
- index: 0,
- });
- });
- it("should get aggregation context with closing paren", () => {
- expect(
- getContext({
- source: "Average()",
- ...aggregationOpts,
- targetOffset: 8,
- }),
- ).toEqual({
- clause: "avg",
- expectedType: "number",
- index: 0,
- });
- });
- it("should get sum-where first argument", () => {
- expect(
- getContext({ source: "1 + SumIf(", ...aggregationOpts }),
- ).toEqual({
- clause: "sum-where",
- expectedType: "number",
- index: 0,
- });
- });
- it("should get sum-where second argument", () => {
- expect(
- getContext({ source: "1 + SumIf(Total = 10,", ...aggregationOpts }),
- ).toEqual({
- clause: "sum-where",
- expectedType: "boolean",
- index: 1,
- });
- });
- it("should get operator context inside aggregation", () => {
- expect(
- getContext({ source: "1 + Sum(2 /", ...aggregationOpts }),
- ).toEqual({
- clause: "/",
- expectedType: "number",
- index: 0,
- });
- });
- });
- describe("expression", () => {
- it("should get operator context", () => {
- expect(getContext({ source: "1 +", ...expressionOpts })).toEqual({
- clause: "+",
- expectedType: "number",
- index: 0,
- });
- });
- it("should get function context", () => {
- expect(getContext({ source: "trim(", ...expressionOpts })).toEqual({
- clause: "trim",
- expectedType: "string",
- index: 0,
- });
- });
- xit("should get boolean for first argument of case", () => {
- // it's difficult to type "case" correctly using the current system because the form is:
- // case([PREDICATE, EXPRESSION]+ [, ELSE-EXPRESSION]?)
- expect(getContext({ source: "case(", ...expressionOpts })).toEqual({
- clause: "case",
- expectedType: "boolean",
- index: 0,
- });
- });
- it("should get expression for second argument of case", () => {
- expect(getContext({ source: "case(Foo,", ...expressionOpts })).toEqual({
- clause: "case",
- expectedType: "expression",
- index: 1,
- });
- });
- });
- describe("filter", () => {
- it("should get function context", () => {
- expect(getContext({ source: "between(", ...filterOpts })).toEqual({
- clause: "between",
- expectedType: "expression",
- index: 0,
- });
- });
- });
- });
});
-function cleanContext(context) {
- delete context.clauseToken;
- if (context.clause) {
- context.clause = context.clause.name;
- }
- return context;
-}
-
function cleanSuggestions(suggestions) {
return _.chain(suggestions)
.map(s => _.pick(s, "type", "text"))
.sortBy("text")
- .sortBy("type")
.value();
}
const suggestionSort = (a, b) =>
- a.type.localeCompare(b.type) || a.text.localeCompare(b.text);
+ a.text < b.text ? -1 : a.text > b.text ? 1 : 0;
diff --git a/frontend/test/metabase/scenarios/question/filter.cy.spec.js b/frontend/test/metabase/scenarios/question/filter.cy.spec.js
index b6061919bb995abd59f08b91471f4af245c78a27..958486c4d385e84156fde2659c95a18f286ed7a9 100644
--- a/frontend/test/metabase/scenarios/question/filter.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/filter.cy.spec.js
@@ -369,6 +369,7 @@ describe("scenarios > question > filter", () => {
openProductsTable();
cy.findByText("Filter").click();
cy.findByText("Custom Expression").click();
+ typeInExpressionEditor("c");
// This issue has two problematic parts. We're testing for both:
cy.log("Popover should display all custom expression options");
@@ -380,7 +381,7 @@ describe("scenarios > question > filter", () => {
cy.log("Should not display error prematurely");
cy.get("[contenteditable='true']")
.click()
- .type("contains(");
+ .type("ontains(");
cy.findByText(/Checks to see if string1 contains string2 within it./i);
cy.button("Done").should("not.be.disabled");
cy.get(".text-error").should("not.exist");
@@ -473,9 +474,10 @@ describe("scenarios > question > filter", () => {
it("should offer case expression in the auto-complete suggestions", () => {
openExpressionEditorFromFreshlyLoadedPage();
+ typeInExpressionEditor("c");
popover().contains(/case/i);
- typeInExpressionEditor("c");
+ typeInExpressionEditor("a");
// "case" is still there after typing a bit
popover().contains(/case/i);
diff --git a/frontend/test/metabase/scenarios/question/new.cy.spec.js b/frontend/test/metabase/scenarios/question/new.cy.spec.js
index 40622ea1a966fa2ebcb47970e5dd1f0f116926f9..c594c00093ae29a30e07be47f290c50bd608ccf0 100644
--- a/frontend/test/metabase/scenarios/question/new.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/new.cy.spec.js
@@ -462,7 +462,7 @@ describe("scenarios > question > new", () => {
});
});
- it.skip("distinct inside custom expression should suggest non-numeric types (metabase#13469)", () => {
+ it("distinct inside custom expression should suggest non-numeric types (metabase#13469)", () => {
openReviewsTable({ mode: "notebook" });
cy.findByText("Summarize").click();
popover()
diff --git a/frontend/test/metabase/scenarios/question/notebook.cy.spec.js b/frontend/test/metabase/scenarios/question/notebook.cy.spec.js
index 0bcb90cfadba30a573799f7f197ef7317b3d654d..7dc0240d23c3f54b9c30efd63699f44b7d88e6dd 100644
--- a/frontend/test/metabase/scenarios/question/notebook.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/notebook.cy.spec.js
@@ -894,8 +894,11 @@ function joinTwoSavedQuestions() {
}
function addSimpleCustomColumn(name) {
+ cy.get("[contenteditable='true']")
+ .click()
+ .type("C");
popover()
- .findByText("Category")
+ .findByText("ategory")
.click();
cy.findByPlaceholderText("Something nice and descriptive")
.click()