From f2e1642813677ae3033ea3d002f62ca1b0144a85 Mon Sep 17 00:00:00 2001
From: Paul Rosenzweig <paulrosenzweig@users.noreply.github.com>
Date: Thu, 12 Dec 2019 15:22:59 -0500
Subject: [PATCH] include fieldRef in table.columns default (#11508)

---
 .../visualizations/visualizations/Table.jsx   |  1 +
 .../scenarios/query_builder.cy.spec.js        | 57 +++++++++++++++++++
 .../lib/settings/visualization.unit.spec.js   | 13 +++++
 3 files changed, 71 insertions(+)

diff --git a/frontend/src/metabase/visualizations/visualizations/Table.jsx b/frontend/src/metabase/visualizations/visualizations/Table.jsx
index e5e2fd175f1..3fea847336f 100644
--- a/frontend/src/metabase/visualizations/visualizations/Table.jsx
+++ b/frontend/src/metabase/visualizations/visualizations/Table.jsx
@@ -176,6 +176,7 @@ export default class Table extends Component {
       ]) =>
         cols.map(col => ({
           name: col.name,
+          fieldRef: col.field_ref,
           enabled: col.visibility_type !== "details-only",
         })),
       getProps: ([
diff --git a/frontend/test/metabase/scenarios/query_builder.cy.spec.js b/frontend/test/metabase/scenarios/query_builder.cy.spec.js
index 62e0489a239..d7620e18031 100644
--- a/frontend/test/metabase/scenarios/query_builder.cy.spec.js
+++ b/frontend/test/metabase/scenarios/query_builder.cy.spec.js
@@ -76,6 +76,63 @@ describe("query builder", () => {
       });
     });
   });
+
+  describe("column settings", () => {
+    it("should allow you to remove a column and add two foreign columns", () => {
+      // oddly specific test inspired by https://github.com/metabase/metabase/issues/11499
+
+      // get a really wide window, so we don't need to mess with scrolling the table horizontally
+      cy.viewport(1600, 800);
+
+      loadOrdersTable();
+      cy.contains("Settings").click();
+
+      // wait for settings sidebar to open
+      cy.get(".border-right.overflow-x-hidden")
+        .invoke("width")
+        .should("be.gt", 350);
+
+      cy.contains("Table options")
+        .parents(".scroll-y")
+        .first()
+        .as("tableOptions");
+
+      // remove Total column
+      cy.get("@tableOptions")
+        .contains("Total")
+        .scrollIntoView()
+        .nextAll(".Icon-close")
+        .click();
+
+      // Add people.category
+      cy.get("@tableOptions")
+        .contains("Category")
+        .scrollIntoView()
+        .nextAll(".Icon-add")
+        .click();
+
+      // wait a Category value to appear in the table, so we know the query completed
+      cy.contains("Widget");
+
+      // Add people.ean
+      cy.get("@tableOptions")
+        .contains("Ean")
+        .scrollIntoView()
+        .nextAll(".Icon-add")
+        .click();
+
+      // wait a Ean value to appear in the table, so we know the query completed
+      cy.contains("8833419218504");
+
+      // confirm that the table contains the right columns
+      cy.get(".Visualization .TableInteractive").as("table");
+      cy.get("@table").contains("Product → Category");
+      cy.get("@table").contains("Product → Ean");
+      cy.get("@table")
+        .contains("Total")
+        .should("not.exist");
+    });
+  });
 });
 
 function loadOrdersTable() {
diff --git a/frontend/test/metabase/visualizations/lib/settings/visualization.unit.spec.js b/frontend/test/metabase/visualizations/lib/settings/visualization.unit.spec.js
index eaed22fd98b..8ca3a2c6bf7 100644
--- a/frontend/test/metabase/visualizations/lib/settings/visualization.unit.spec.js
+++ b/frontend/test/metabase/visualizations/lib/settings/visualization.unit.spec.js
@@ -153,6 +153,19 @@ describe("visualization_settings", () => {
         expect(settings["graph.show_values"]).toBe(false);
       });
     });
+    describe("table.columns", () => {
+      it("should include fieldRef in default table.columns", () => {
+        const card = { visualization_settings: {} };
+        const cols = [
+          NumberColumn({ name: "some number", field_ref: ["field-id", 123] }),
+        ];
+        const {
+          "table.columns": [setting],
+        } = getComputedSettingsForSeries([{ card, data: { cols } }]);
+
+        expect(setting.fieldRef).toEqual(["field-id", 123]);
+      });
+    });
   });
 
   describe("getStoredSettingsForSeries", () => {
-- 
GitLab