From a8dabd6ae3303de8d472323422b7194769b5929f Mon Sep 17 00:00:00 2001
From: Ariya Hidayat <ariya@metabase.com>
Date: Thu, 17 Feb 2022 10:51:42 -0800
Subject: [PATCH] Custom expression editor: do not always suggest an open
parenthesis (#20533)
For functions which do not accept any arguments, i.e. COUNT and
CUMULATIVECOUNT, do not suggest the completion with the "(" character.
---
frontend/src/metabase/lib/expressions/suggest.js | 10 ++++++++--
.../expressions/ExpressionEditorTextfield.jsx | 2 +-
.../test/metabase/lib/expressions/suggest.unit.spec.js | 10 +++++-----
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/frontend/src/metabase/lib/expressions/suggest.js b/frontend/src/metabase/lib/expressions/suggest.js
index 8c90b18b027..a61f1bac92e 100644
--- a/frontend/src/metabase/lib/expressions/suggest.js
+++ b/frontend/src/metabase/lib/expressions/suggest.js
@@ -19,6 +19,12 @@ import {
EDITOR_FK_SYMBOLS,
} from "./config";
+const suggestionText = func => {
+ const { displayName, args } = func;
+ const suffix = args.length > 0 ? "(" : " ";
+ return displayName + suffix;
+};
+
export function suggest({
source,
query,
@@ -59,7 +65,7 @@ export function suggest({
.map(func => ({
type: "functions",
name: func.displayName,
- text: func.displayName + "(",
+ text: suggestionText(func),
index: targetOffset,
icon: "function",
order: 1,
@@ -73,7 +79,7 @@ export function suggest({
.map(func => ({
type: "aggregations",
name: func.displayName,
- text: func.displayName + "(",
+ text: suggestionText(func),
index: targetOffset,
icon: "function",
order: 1,
diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx
index b05fbf3f5de..df8a2570355 100644
--- a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx
+++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx
@@ -138,7 +138,7 @@ export default class ExpressionEditorTextfield extends React.Component {
const extraTrim = openParen && alreadyOpenParen ? 1 : 0;
const replacement = suggested.slice(0, suggested.length - extraTrim);
- const updatedExpression = prefix + replacement.trim() + postfix;
+ const updatedExpression = prefix + replacement + postfix;
this.handleExpressionChange(updatedExpression);
const caretPos = updatedExpression.length - postfix.length;
diff --git a/frontend/test/metabase/lib/expressions/suggest.unit.spec.js b/frontend/test/metabase/lib/expressions/suggest.unit.spec.js
index e1e1e11ed0b..5489aefd1e6 100644
--- a/frontend/test/metabase/lib/expressions/suggest.unit.spec.js
+++ b/frontend/test/metabase/lib/expressions/suggest.unit.spec.js
@@ -207,9 +207,9 @@ describe("metabase/lib/expression/suggest", () => {
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: "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] " },
@@ -223,9 +223,9 @@ describe("metabase/lib/expression/suggest", () => {
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] " },
@@ -258,7 +258,7 @@ describe("metabase/lib/expression/suggest", () => {
startRule: "aggregation",
}),
).toEqual([
- { type: "aggregations", text: "Count(" },
+ { type: "aggregations", text: "Count " },
{ type: "aggregations", text: "CountIf(" },
]);
});
--
GitLab