Expressions in AggregationPicker without legacy query

frontend/src/metabase-lib/aggregation.ts

@@ -1,9 +1,9 @@
 import * as ML from "cljs/metabase.lib.js";
 import type {
+  Aggregatable,
   AggregationClause,
   AggregationOperator,
   ColumnMetadata,
-  MetricMetadata,
   Query,
 } from "./types";
 
@@ -30,7 +30,7 @@ export function selectedAggregationOperators(
 export function aggregate(
   query: Query,
   stageIndex: number,
-  clause: AggregationClause | MetricMetadata,
+  clause: Aggregatable,
 ): Query {
   return ML.aggregate(query, stageIndex, clause);
 }

frontend/src/metabase-lib/expressions/process.js

@@ -1,5 +1,7 @@
 import { t } from "ttag";
 
+import * as Lib from "metabase-lib";
+
 import { parse, adjustBooleans } from "./recursive-parser";
 import { resolve } from "./resolver";
 
@@ -31,13 +33,23 @@ export function processSource(options) {
     }
   };
 
-  const { source, startRule } = options;
+  const { source, query, stageIndex, startRule } = options;
 
-  let expression;
+  let expression = null;
+  let expressionClause = null;
   let compileError;
   try {
     const parsed = parse(source);
     expression = adjustBooleans(resolve(parsed, startRule, resolveMBQLField));
+
+    // query and stageIndex are not available outside of notebook editor (e.g. in Metrics or Segments).
+    if (typeof query !== "undefined" && typeof stageIndex !== "undefined") {
+      expressionClause = Lib.expressionClauseForLegacyExpression(
+        query,
+        stageIndex,
+        expression,
+      );
+    }
   } catch (e) {
     console.warn("compile error", e);
     compileError = e;
@@ -46,6 +58,7 @@ export function processSource(options) {
   return {
     source,
     expression,
+    expressionClause,
     compileError,
   };
 }

frontend/src/metabase-lib/types.ts

@@ -25,7 +25,10 @@ export type MetricMetadata = unknown & { _opaque: typeof MetricMetadata };
 declare const AggregationClause: unique symbol;
 export type AggregationClause = unknown & { _opaque: typeof AggregationClause };
 
-export type Aggregatable = AggregationClause | MetricMetadata;
+export type Aggregatable =
+  | AggregationClause
+  | MetricMetadata
+  | ExpressionClause;
 
 declare const AggregationOperator: unique symbol;
 export type AggregationOperator = unknown & {

frontend/src/metabase/common/components/AggregationPicker/AggregationPicker.tsx

@@ -9,10 +9,7 @@ import { useToggle } from "metabase/hooks/use-toggle";
 import { ExpressionWidget } from "metabase/query_builder/components/expressions/ExpressionWidget";
 import { ExpressionWidgetHeader } from "metabase/query_builder/components/expressions/ExpressionWidgetHeader";
 
-import type {
-  Aggregation as LegacyAggregationClause,
-  Expression as LegacyExpressionClause,
-} from "metabase-types/api";
+import type { Expression as LegacyExpressionClause } from "metabase-types/api";
 import * as Lib from "metabase-lib";
 import * as AGGREGATION from "metabase-lib/queries/utils/aggregation";
 import type LegacyAggregation from "metabase-lib/queries/structured/Aggregation";
@@ -40,7 +37,6 @@ interface AggregationPickerProps {
   legacyClause?: LegacyAggregation;
   maxHeight?: number;
   onSelect: (operator: Lib.Aggregatable) => void;
-  onSelectLegacy: (operator: LegacyAggregationClause) => void;
   onClose?: () => void;
 }
 
@@ -75,7 +71,6 @@ export function AggregationPicker({
   legacyClause,
   maxHeight = DEFAULT_MAX_HEIGHT,
   onSelect,
-  onSelectLegacy,
   onClose,
 }: AggregationPickerProps) {
   const [
@@ -202,18 +197,27 @@ export function AggregationPicker({
   );
 
   const handleExpressionChange = useCallback(
-    (name: string, expression: LegacyExpressionClause) => {
-      const aggregation = AGGREGATION.setName(expression, name);
-      onSelectLegacy(aggregation as LegacyAggregationClause);
+    (
+      name: string,
+      _expression: LegacyExpressionClause,
+      expressionClause: Lib.ExpressionClause,
+    ) => {
+      const updatedExpressionClause = Lib.withExpressionName(
+        expressionClause,
+        name,
+      );
+      onSelect(updatedExpressionClause);
       onClose?.();
     },
-    [onSelectLegacy, onClose],
+    [onSelect, onClose],
   );
 
   if (isEditingExpression) {
     return (
       <ExpressionWidget
         legacyQuery={legacyQuery}
+        query={query}
+        stageIndex={stageIndex}
         name={AGGREGATION.getName(legacyClause)}
         expression={AGGREGATION.getContent(legacyClause)}
         withName

frontend/src/metabase/common/components/AggregationPicker/AggregationPicker.unit.spec.tsx

@@ -141,7 +141,6 @@ function setup({
     : baseOperators;
 
   const onSelect = jest.fn();
-  const onSelectLegacy = jest.fn();
 
   function handleSelect(clause: Lib.Aggregatable) {
     const nextQuery = Lib.aggregate(query, 0, clause);
@@ -159,7 +158,6 @@ function setup({
       operators={operators}
       hasExpressionInput={hasExpressionInput}
       onSelect={handleSelect}
-      onSelectLegacy={onSelectLegacy}
     />,
   );
 
@@ -168,7 +166,7 @@ function setup({
     return lastCall?.[0];
   }
 
-  return { metadata, getRecentClause, onSelectLegacy };
+  return { metadata, getRecentClause, onSelect };
 }
 
 describe("AggregationPicker", () => {
@@ -392,15 +390,15 @@ describe("AggregationPicker", () => {
 
   describe("custom expressions", () => {
     it("should allow to enter a custom expression", async () => {
-      const { onSelectLegacy } = setup();
+      const { onSelect } = setup();
 
       userEvent.click(screen.getByText("Custom Expression"));
       userEvent.type(screen.getByLabelText("Expression"), "1 + 1");
       userEvent.type(screen.getByLabelText("Name"), "My expression");
       userEvent.click(screen.getByRole("button", { name: "Done" }));
 
       await waitFor(() =>
-        expect(onSelectLegacy).toHaveBeenCalledWith([
+        expect(onSelect).toHaveBeenCalledWith([
           "aggregation-options",
           ["+", 1, 1],
           { "display-name": "My expression", name: "My expression" },

frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/ExpressionEditorTextfield.tsx

@@ -8,6 +8,7 @@ import * as ace from "ace-builds/src-noconflict/ace";
 import type { Ace } from "ace-builds";
 import type { Expression } from "metabase-types/api";
 import ExplicitSize from "metabase/components/ExplicitSize";
+import type * as Lib from "metabase-lib";
 import { format } from "metabase-lib/expressions/format";
 import { processSource } from "metabase-lib/expressions/process";
 import { diagnose } from "metabase-lib/expressions/diagnostics";
@@ -47,15 +48,23 @@ interface ExpressionEditorTextfieldProps {
   expression: Expression | undefined;
   name: string;
   legacyQuery: StructuredQuery;
+  query?: Lib.Query;
+  stageIndex?: number;
   startRule?: string;
   width?: number;
   reportTimezone?: string;
   textAreaId?: string;
 
-  onChange: (expression: Expression | null) => void;
+  onChange: (
+    expression: Expression | null,
+    expressionClause: Lib.ExpressionClause | null,
+  ) => void;
   onError: (error: ErrorWithMessage | null) => void;
   onBlankChange: (isBlank: boolean) => void;
-  onCommit: (expression: Expression | null) => void;
+  onCommit: (
+    expression: Expression | null,
+    expressionClause: Lib.ExpressionClause | null,
+  ) => void;
   helpTextTarget: RefObject<HTMLElement>;
 }
 
@@ -279,23 +288,29 @@ class ExpressionEditorTextfield extends React.Component<
       return;
     }
 
+    const { onChange, onError } = this.props;
+
     this.clearSuggestions();
 
     const errorMessage = this.diagnoseExpression();
     this.setState({ errorMessage });
 
     // whenever our input blurs we push the updated expression to our parent if valid
     if (errorMessage) {
-      this.props.onError(errorMessage);
+      onError(errorMessage);
     } else {
-      const expression = this.compileExpression();
-      if (expression) {
+      const compiledExpression = this.compileExpression();
+
+      if (compiledExpression) {
+        const { expression, expressionClause } = compiledExpression;
+
         if (!isExpression(expression)) {
           console.warn("isExpression=false", expression);
         }
-        this.props.onChange(expression);
+
+        onChange(expression, expressionClause);
       } else {
-        this.props.onError({ message: t`Invalid expression` });
+        onError({ message: t`Invalid expression` });
       }
     }
   };
@@ -331,18 +346,20 @@ class ExpressionEditorTextfield extends React.Component<
 
   compileExpression() {
     const { source } = this.state;
-    const { legacyQuery, startRule, name } = this.props;
+    const { legacyQuery, query, stageIndex, startRule, name } = this.props;
     if (!source || source.length === 0) {
       return null;
     }
-    const { expression } = processSource({
+    const { expression, expressionClause } = processSource({
       name,
       source,
+      query,
       legacyQuery,
+      stageIndex,
       startRule,
     });
 
-    return expression;
+    return { expression, expressionClause };
   }
 
   diagnoseExpression(): ErrorWithMessage | null {
@@ -362,6 +379,8 @@ class ExpressionEditorTextfield extends React.Component<
     const {
       legacyQuery,
       startRule = ExpressionEditorTextfield.defaultProps.startRule,
+      onCommit,
+      onError,
     } = this.props;
     const { source } = this.state;
     const errorMessage = diagnose(
@@ -372,12 +391,16 @@ class ExpressionEditorTextfield extends React.Component<
     this.setState({ errorMessage });
 
     if (errorMessage) {
-      this.props.onError(errorMessage);
+      onError(errorMessage);
     } else {
-      const expression = this.compileExpression();
+      const compiledExpression = this.compileExpression();
+
+      if (compiledExpression) {
+        const { expression, expressionClause } = compiledExpression;
 
-      if (isExpression(expression)) {
-        this.props.onCommit(expression);
+        if (isExpression(expression)) {
+          onCommit(expression, expressionClause);
+        }
       }
     }
   }