From 9b3e2e2c919722d011f43cefa98622d6f0fae5c1 Mon Sep 17 00:00:00 2001
From: Tom Robinson <tlrobinson@gmail.com>
Date: Mon, 26 Aug 2019 14:21:14 -0700
Subject: [PATCH] Fix existing column settings for native/nested queries
 (#10704)

Revert to previous behavior that uses `name` instead of `field_ref` for `field-literal` columns
---
 frontend/src/metabase/lib/dataset.js          | 10 +-
 .../visualizations/lib/settings/column.js     |  2 +-
 .../lib/settings/column.unit.spec.js          | 99 +++++++++++++++++++
 3 files changed, 109 insertions(+), 2 deletions(-)
 create mode 100644 frontend/test/metabase/visualizations/lib/settings/column.unit.spec.js

diff --git a/frontend/src/metabase/lib/dataset.js b/frontend/src/metabase/lib/dataset.js
index 534caf2797e..bda6ece808a 100644
--- a/frontend/src/metabase/lib/dataset.js
+++ b/frontend/src/metabase/lib/dataset.js
@@ -131,7 +131,15 @@ function fieldRefForColumn_LEGACY(
 
 export const keyForColumn = (column: Column): string => {
   const ref = fieldRefForColumn(column);
-  return JSON.stringify(ref ? ["ref", ref] : ["name", column.name]);
+  // match legacy behavior which didn't have "field-literal" or "aggregation" field refs
+  if (
+    Array.isArray(ref) &&
+    ref[0] !== "field-literal" &&
+    ref[0] !== "aggregation"
+  ) {
+    return JSON.stringify(["ref", ref]);
+  }
+  return JSON.stringify(["name", column.name]);
 };
 
 /**
diff --git a/frontend/src/metabase/visualizations/lib/settings/column.js b/frontend/src/metabase/visualizations/lib/settings/column.js
index 3019f4370b9..e72aa5bf4ac 100644
--- a/frontend/src/metabase/visualizations/lib/settings/column.js
+++ b/frontend/src/metabase/visualizations/lib/settings/column.js
@@ -58,7 +58,7 @@ type ColumnSettingDef = SettingDef & {
 export function columnSettings({
   getColumns = DEFAULT_GET_COLUMNS,
   ...def
-}: ColumnSettingDef) {
+}: ColumnSettingDef = {}) {
   return nestedSettings("column_settings", {
     section: t`Formatting`,
     objectName: "column",
diff --git a/frontend/test/metabase/visualizations/lib/settings/column.unit.spec.js b/frontend/test/metabase/visualizations/lib/settings/column.unit.spec.js
new file mode 100644
index 00000000000..b0258af57ca
--- /dev/null
+++ b/frontend/test/metabase/visualizations/lib/settings/column.unit.spec.js
@@ -0,0 +1,99 @@
+import { columnSettings } from "metabase/visualizations/lib/settings/column";
+
+import { getComputedSettings } from "metabase/visualizations/lib/settings";
+
+function seriesWithColumn(col) {
+  return [
+    {
+      card: {},
+      data: {
+        cols: [
+          {
+            name: "foo",
+            base_type: "type/Float",
+            special_type: "type/Currency",
+            ...col,
+          },
+        ],
+      },
+    },
+  ];
+}
+
+describe("column settings", () => {
+  it("should find by column name", () => {
+    const series = seriesWithColumn({});
+    const defs = { ...columnSettings() };
+    const stored = {
+      column_settings: {
+        '["name","foo"]': {
+          currency: "BTC",
+        },
+      },
+    };
+    const computed = getComputedSettings(defs, series, stored);
+    expect(computed.column(series[0].data.cols[0]).currency).toEqual("BTC");
+  });
+  it("should find by column 'field-id' ref", () => {
+    const series = seriesWithColumn({
+      id: 42,
+      field_ref: ["field-id", 42],
+    });
+    const defs = { ...columnSettings() };
+    const stored = {
+      column_settings: {
+        '["ref",["field-id",42]]': {
+          currency: "BTC",
+        },
+      },
+    };
+    const computed = getComputedSettings(defs, series, stored);
+    expect(computed.column(series[0].data.cols[0]).currency).toEqual("BTC");
+  });
+  // DISABLED to match legacy behavior until we determine the best way to reference columns
+  xit("should find by column 'field-literal' ref", () => {
+    const series = seriesWithColumn({
+      field_ref: ["field-literal", "foo", "type/Float"],
+    });
+    const defs = { ...columnSettings() };
+    const stored = {
+      column_settings: {
+        '["ref",["field-literal","foo","type/Float"]]': {
+          currency: "BTC",
+        },
+      },
+    };
+    const computed = getComputedSettings(defs, series, stored);
+    expect(computed.column(series[0].data.cols[0]).currency).toEqual("BTC");
+  });
+  it("should find by column name if it also has a 'field-literal' ref", () => {
+    const series = seriesWithColumn({
+      field_ref: ["field-literal", "foo", "type/Float"],
+    });
+    const defs = { ...columnSettings() };
+    const stored = {
+      column_settings: {
+        '["name","foo"]': {
+          currency: "BTC",
+        },
+      },
+    };
+    const computed = getComputedSettings(defs, series, stored);
+    expect(computed.column(series[0].data.cols[0]).currency).toEqual("BTC");
+  });
+  it("should find by column name if it also has a 'aggregation' ref", () => {
+    const series = seriesWithColumn({
+      field_ref: ["aggregation", 0],
+    });
+    const defs = { ...columnSettings() };
+    const stored = {
+      column_settings: {
+        '["name","foo"]': {
+          currency: "BTC",
+        },
+      },
+    };
+    const computed = getComputedSettings(defs, series, stored);
+    expect(computed.column(series[0].data.cols[0]).currency).toEqual("BTC");
+  });
+});
-- 
GitLab