From 8c95fdaa98fda493717b6329ad001124478e08e1 Mon Sep 17 00:00:00 2001
From: Ariya Hidayat <ariya@metabase.com>
Date: Tue, 1 Jun 2021 00:06:33 -0700
Subject: [PATCH] Aggregation query should always specify name (#16259)

---
 frontend/src/metabase/lib/query/aggregation.js     |  2 +-
 .../metabase/lib/query/aggregation.unit.spec.js    | 14 +++++++++++++-
 .../scenarios/question/custom_column.cy.spec.js    |  8 ++++++--
 .../metabase/scenarios/question/filter.cy.spec.js  |  8 ++++++--
 .../scenarios/question/notebook.cy.spec.js         |  4 ++--
 5 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/frontend/src/metabase/lib/query/aggregation.js b/frontend/src/metabase/lib/query/aggregation.js
index 8edb052ed55..db200c72a17 100644
--- a/frontend/src/metabase/lib/query/aggregation.js
+++ b/frontend/src/metabase/lib/query/aggregation.js
@@ -136,7 +136,7 @@ export function setName(
   return [
     "aggregation-options",
     getContent(aggregation),
-    { "display-name": name, ...getOptions(aggregation) },
+    { name, "display-name": name, ...getOptions(aggregation) },
   ];
 }
 export function setContent(
diff --git a/frontend/test/metabase/lib/query/aggregation.unit.spec.js b/frontend/test/metabase/lib/query/aggregation.unit.spec.js
index d7181213503..0e723b8f050 100644
--- a/frontend/test/metabase/lib/query/aggregation.unit.spec.js
+++ b/frontend/test/metabase/lib/query/aggregation.unit.spec.js
@@ -1,4 +1,4 @@
-import { getName } from "metabase/lib/query/aggregation";
+import { getName, setName } from "metabase/lib/query/aggregation";
 
 describe("getName", () => {
   it("should work with blank display name", () => {
@@ -8,3 +8,15 @@ describe("getName", () => {
     );
   });
 });
+
+describe("setName", () => {
+  it("should set the name and display-name", () => {
+    const expr = ["*", ["count"], 2];
+    const aggregation = ["aggregation-options", ["*", ["count"], 2], null];
+    expect(setName(aggregation, "DoubleCount")).toEqual([
+      "aggregation-options",
+      expr,
+      { "display-name": "DoubleCount", name: "DoubleCount" },
+    ]);
+  });
+});
diff --git a/frontend/test/metabase/scenarios/question/custom_column.cy.spec.js b/frontend/test/metabase/scenarios/question/custom_column.cy.spec.js
index 240cd7ffc0b..3838990f5a3 100644
--- a/frontend/test/metabase/scenarios/question/custom_column.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/custom_column.cy.spec.js
@@ -207,7 +207,7 @@ describe("scenarios > question > custom columns", () => {
       });
   });
 
-  it.skip("should be able to use custom expression after aggregation (metabase#13857)", () => {
+  it("should be able to use custom expression after aggregation (metabase#13857)", () => {
     const CE_NAME = "13857_CE";
     const CC_NAME = "13857_CC";
 
@@ -221,7 +221,11 @@ describe("scenarios > question > custom columns", () => {
         },
         "source-query": {
           aggregation: [
-            ["aggregation-options", ["*", 1, 1], { "display-name": CE_NAME }],
+            [
+              "aggregation-options",
+              ["*", 1, 1],
+              { name: CE_NAME, "display-name": CE_NAME },
+            ],
           ],
           breakout: [
             ["datetime-field", ["field-id", ORDERS.CREATED_AT], "month"],
diff --git a/frontend/test/metabase/scenarios/question/filter.cy.spec.js b/frontend/test/metabase/scenarios/question/filter.cy.spec.js
index 197379504ed..4d4b21a1538 100644
--- a/frontend/test/metabase/scenarios/question/filter.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/filter.cy.spec.js
@@ -301,7 +301,7 @@ describe("scenarios > question > filter", () => {
     cy.findAllByText("Fantastic Wool Shirt").should("not.exist");
   });
 
-  it.skip("should filter using Custom Expression from aggregated results (metabase#12839)", () => {
+  it("should filter using Custom Expression from aggregated results (metabase#12839)", () => {
     const CE_NAME = "Simple Math";
 
     cy.createQuestion({
@@ -310,7 +310,11 @@ describe("scenarios > question > filter", () => {
         filter: [">", ["field", CE_NAME, { "base-type": "type/Float" }], 0],
         "source-query": {
           aggregation: [
-            ["aggregation-options", ["+", 1, 1], { "display-name": CE_NAME }],
+            [
+              "aggregation-options",
+              ["+", 1, 1],
+              { name: CE_NAME, "display-name": CE_NAME },
+            ],
           ],
           breakout: [["field", PRODUCTS.CATEGORY, null]],
           "source-table": PRODUCTS_ID,
diff --git a/frontend/test/metabase/scenarios/question/notebook.cy.spec.js b/frontend/test/metabase/scenarios/question/notebook.cy.spec.js
index 1d58a606895..2e2ebb72883 100644
--- a/frontend/test/metabase/scenarios/question/notebook.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/notebook.cy.spec.js
@@ -456,7 +456,7 @@ describe("scenarios > question > notebook", () => {
       });
     });
 
-    it.skip("should be able to do subsequent aggregation on a custom expression (metabase#14649)", () => {
+    it("should be able to do subsequent aggregation on a custom expression (metabase#14649)", () => {
       cy.createQuestion({
         name: "14649_min",
         query: {
@@ -466,7 +466,7 @@ describe("scenarios > question > notebook", () => {
               [
                 "aggregation-options",
                 ["sum", ["field", ORDERS.SUBTOTAL, null]],
-                { "display-name": "Revenue" },
+                { name: "Revenue", "display-name": "Revenue" },
               ],
             ],
             breakout: [
-- 
GitLab