Revert "show columns added after the dashcard has been created in the viz...

Revert "show columns added after the dashcard has been created in the viz settings (#33886)" (#34116)

frontend/src/metabase/visualizations/lib/settings/column.js

@@ -502,94 +502,52 @@ export const getTitleForColumn = (column, series, settings) => {
 
 export const buildTableColumnSettings = ({
   getIsColumnVisible = col => col.visibility_type !== "details-only",
-} = {}) => {
-  const isValid = ([{ card, data }], settings) => {
-    const columns = settings["table.columns"] || [];
-    const enabledColumns = columns.filter(column => column.enabled);
-    // If "table.columns" happened to be an empty array,
-    // it will be treated as "all columns are hidden",
-    // This check ensures it's not empty,
-    // otherwise it will be overwritten by `getDefault` below
-    return (
-      columns.length !== 0 &&
-      _.all(
-        enabledColumns,
-        columnSetting =>
-          findColumnIndexForColumnSetting(data.cols, columnSetting) >= 0,
-      )
-    );
-  };
-
-  const getValue = (series, settings) => {
-    const [
+} = {}) => ({
+  // NOTE: table column settings may be identified by fieldRef (possible not normalized) or column name:
+  //   { name: "COLUMN_NAME", enabled: true }
+  //   { fieldRef: ["field", 2, {"source-field": 1}], enabled: true }
+  "table.columns": {
+    section: t`Columns`,
+    title: t`Columns`,
+    widget: ChartSettingTableColumns,
+    getHidden: (series, vizSettings) => vizSettings["table.pivot"],
+    isValid: ([{ card, data }]) => {
+      const columns = card.visualization_settings["table.columns"];
+      const enabledColumns = columns.filter(column => column.enabled);
+      // If "table.columns" happened to be an empty array,
+      // it will be treated as "all columns are hidden",
+      // This check ensures it's not empty,
+      // otherwise it will be overwritten by `getDefault` below
+      return (
+        card.visualization_settings["table.columns"].length !== 0 &&
+        _.all(
+          enabledColumns,
+          columnSetting =>
+            findColumnIndexForColumnSetting(data.cols, columnSetting) >= 0,
+        )
+      );
+    },
+    getDefault: ([
       {
-        data: { cols = [] },
+        data: { cols },
       },
-    ] = series;
-
-    const columnSettings = settings["table.columns"];
-
-    if (!columnSettings) {
-      // default settings
-      return getDefault(series, settings);
-    }
-
-    if (!isValid(series, settings)) {
-      return getDefault(series, settings);
-    }
-
-    const datasetColumnsNotInSettings = cols.filter(
-      datasetColumn =>
-        findColumnIndexForColumnSetting(columnSettings, datasetColumn) < 0,
-    );
-
-    const extraColumnSettings = datasetColumnsNotInSettings.map(
-      datasetColumn => ({
-        name: datasetColumn.name,
-        enabled: false,
-        fieldRef: datasetColumn.field_ref,
-      }),
-    );
-
-    return columnSettings.concat(extraColumnSettings);
-  };
-
-  const getDefault = ([
-    {
-      data: { cols },
-    },
-  ]) => {
-    return cols.map(col => ({
-      name: col.name,
-      fieldRef: col.field_ref,
-      enabled: getIsColumnVisible(col),
-    }));
-  };
-
-  return {
-    // NOTE: table column settings may be identified by fieldRef (possible not normalized) or column name:
-    //   { name: "COLUMN_NAME", enabled: true }
-    //   { fieldRef: ["field", 2, {"source-field": 1}], enabled: true }
-    "table.columns": {
-      section: t`Columns`,
-      title: t`Columns`,
-      widget: ChartSettingTableColumns,
-      getHidden: (series, vizSettings) => vizSettings["table.pivot"],
-      isValid,
-      getDefault,
-      getValue,
-      getProps: (series, settings) => {
-        const [
-          {
-            data: { cols },
-          },
-        ] = series;
+    ]) =>
+      cols.map(col => ({
+        name: col.name,
+        fieldRef: col.field_ref,
+        enabled: getIsColumnVisible(col),
+      })),
+    getProps: (series, settings) => {
+      const [
+        {
+          data: { cols },
+        },
+      ] = series;
 
-        return {
-          columns: cols,
-          getColumnName: column => getTitleForColumn(column, series, settings),
-        };
-      },
+      return {
+        columns: cols,
+        getColumnName: column => getTitleForColumn(column, series, settings),
+      };
     },
-  };
-};
+  },
+});

frontend/src/metabase/visualizations/lib/settings/column.unit.spec.js

-import {
-  columnSettings,
-  buildTableColumnSettings,
-} from "metabase/visualizations/lib/settings/column";
+import { columnSettings } from "metabase/visualizations/lib/settings/column";
 
 import { getComputedSettings } from "metabase/visualizations/lib/settings";
-import {
-  createMockColumn,
-  createMockSingleSeries,
-  createMockTableColumnOrderSetting,
-} from "metabase-types/api/mocks";
-import { ORDERS } from "metabase-types/api/mocks/presets";
 
 function seriesWithColumn(col) {
   return [
@@ -104,98 +95,4 @@ describe("column settings", () => {
     expect(time_style).toEqual("h:mm A");
     expect(date_style).toEqual("");
   });
-
-  it("should show new columns and show them as disabled (metabase#25592)", () => {
-    const storedSettings = {
-      "table.columns": [
-        createMockTableColumnOrderSetting({
-          name: "ID",
-          fieldRef: ["field", ORDERS.ID, null],
-          enabled: true,
-        }),
-      ],
-    };
-
-    const series = [
-      createMockSingleSeries(
-        {},
-        {
-          data: {
-            cols: [
-              createMockColumn({
-                id: ORDERS.ID,
-                name: "ID",
-                display_name: "Id",
-                field_ref: ["field", ORDERS.ID, null],
-              }),
-              createMockColumn({
-                id: ORDERS.SUBTOTAL,
-                name: "SUBTOTAL",
-                display_name: "Total",
-                field_ref: ["field", ORDERS.SUBTOTAL, null],
-              }),
-            ],
-          },
-        },
-      ),
-    ];
-
-    const computedValue = buildTableColumnSettings()["table.columns"].getValue(
-      series,
-      storedSettings,
-    );
-
-    const computedSubtotal = computedValue.find(
-      ({ name }) => name === "SUBTOTAL",
-    );
-
-    expect(computedSubtotal).not.toBeUndefined();
-    expect(computedSubtotal.enabled).toBe(false);
-  });
-
-  it("table should should generate default columns when table.columns entries do not match data.cols (metabase#28304)", () => {
-    const storedSettings = {
-      "table.columns": [
-        createMockTableColumnOrderSetting({
-          name: "TAX",
-          fieldRef: ["field", ORDERS.TAX, null],
-          enabled: true,
-        }),
-        createMockTableColumnOrderSetting({
-          name: "DISCOUNT",
-          fieldRef: ["field", ORDERS.DISCOUNT, null],
-          enabled: false,
-        }),
-      ],
-    };
-
-    const series = [
-      createMockSingleSeries(
-        {},
-        {
-          data: {
-            cols: [
-              createMockColumn({
-                id: ORDERS.ID,
-                name: "ID",
-                display_name: "Id",
-                field_ref: ["field", ORDERS.ID, null],
-              }),
-            ],
-          },
-        },
-      ),
-    ];
-
-    const computedValue = buildTableColumnSettings()["table.columns"].getValue(
-      series,
-      storedSettings,
-    );
-
-    expect(computedValue.length).toBe(1);
-    expect(computedValue[0]).toMatchObject({
-      name: "ID",
-      enabled: true,
-    });
-  });
 });