From d3a515858ab5a5a42d81fc2584b77c07354dc8e4 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Tue, 6 Aug 2024 13:29:34 -0400
Subject: [PATCH] Support both name & field ref-based column keys in viz
settings on read and upgrade on write (#46383)
---
.../temporal-unit-parameters.cy.spec.js | 6 +-
.../v1/queries/utils/column-key.ts | 81 +++++++
.../v1/queries/utils/column-key.unit.spec.ts | 208 ++++++++++++++++++
.../metabase-lib/v1/queries/utils/dataset.ts | 12 +-
.../v1/queries/utils/get-column-key.ts | 44 ----
.../queries/utils/get-column-key.unit.spec.js | 90 --------
frontend/src/metabase-types/api/card.ts | 12 +
.../pickers/DefaultPicker/DefaultPicker.tsx | 10 +-
.../ClickBehaviorSidebar.tsx | 2 +-
.../components/ClickBehaviorSidebar/utils.ts | 17 +-
.../utils/viz-settings/sync-viz-settings.ts | 39 +++-
.../sync-viz-settings.unit.spec.tsx | 29 +++
.../src/metabase/static-viz/lib/format.ts | 14 +-
.../src/metabase/static-viz/lib/settings.ts | 14 +-
.../ColumnFormattingAction.tsx | 2 +-
.../components/ChartSettings.jsx | 2 +-
.../settings/ChartSettingFieldPicker.jsx | 2 +-
.../settings/ChartSettingFieldsPartition.jsx | 2 +-
.../settings/ChartSettingNestedSettings.jsx | 31 ++-
.../TableColumnPanel/utils.ts | 2 +-
.../ChartSettingsListColumns.tsx | 2 +-
.../echarts/cartesian/model/util.ts | 5 +-
.../visualizations/lib/settings/column.js | 6 +-
.../visualizations/lib/settings/nested.js | 7 +-
.../lib/settings/nested.unit.spec.js | 1 +
.../visualizations/lib/settings/series.js | 3 +-
.../src/metabase/visualizations/lib/utils.js | 2 +-
.../visualizations/shared/settings/column.ts | 6 +-
.../visualizations/Table.unit.spec.js | 56 ++++-
src/metabase/formatter/datetime.clj | 3 +-
.../query_processor/streaming/common.clj | 3 +-
.../query_processor/streaming/xlsx.clj | 6 +-
32 files changed, 499 insertions(+), 220 deletions(-)
create mode 100644 frontend/src/metabase-lib/v1/queries/utils/column-key.ts
create mode 100644 frontend/src/metabase-lib/v1/queries/utils/column-key.unit.spec.ts
delete mode 100644 frontend/src/metabase-lib/v1/queries/utils/get-column-key.ts
delete mode 100644 frontend/src/metabase-lib/v1/queries/utils/get-column-key.unit.spec.js
diff --git a/e2e/test/scenarios/dashboard-filters/temporal-unit-parameters.cy.spec.js b/e2e/test/scenarios/dashboard-filters/temporal-unit-parameters.cy.spec.js
index ec8585e09b3..17faabcf419 100644
--- a/e2e/test/scenarios/dashboard-filters/temporal-unit-parameters.cy.spec.js
+++ b/e2e/test/scenarios/dashboard-filters/temporal-unit-parameters.cy.spec.js
@@ -387,9 +387,9 @@ describe("scenarios > dashboard > temporal unit parameters", () => {
.should("contain.text", "Started from")
.should("contain.text", multiBreakoutQuestionDetails.name);
queryBuilderMain().within(() => {
- cy.findByText("Product → Created At: Week").should("be.visible");
- cy.findByText("2022").should("be.visible");
- cy.findByText("2023").should("be.visible");
+ cy.findByText("Created At: Year").should("be.visible");
+ cy.findByText("April 24, 2022").should("be.visible");
+ cy.findByText("May 1, 2022").should("be.visible");
});
});
diff --git a/frontend/src/metabase-lib/v1/queries/utils/column-key.ts b/frontend/src/metabase-lib/v1/queries/utils/column-key.ts
new file mode 100644
index 00000000000..6fbe550f364
--- /dev/null
+++ b/frontend/src/metabase-lib/v1/queries/utils/column-key.ts
@@ -0,0 +1,81 @@
+import {
+ createFieldReference,
+ getBaseDimensionReference,
+ getNormalizedDimensionReference,
+ hasStringFieldName,
+ isAggregationReference,
+ isExpressionReference,
+ isFieldReference,
+ isValidDimensionReference,
+} from "metabase-lib/v1/references";
+import type {
+ ColumnSettings,
+ DatasetColumn,
+ VisualizationSettings,
+} from "metabase-types/api";
+
+export type DatasetColumnReference = Pick<DatasetColumn, "name" | "field_ref">;
+
+export const getColumnKey = (column: DatasetColumnReference) => {
+ return JSON.stringify(["name", column.name]);
+};
+
+export const getLegacyColumnKey = (column: DatasetColumnReference) => {
+ let fieldRef = column.field_ref;
+
+ if (!fieldRef) {
+ fieldRef = createFieldReference(column.name);
+ }
+
+ if (!isValidDimensionReference(fieldRef)) {
+ console.warn("Unknown field_ref", fieldRef);
+ return JSON.stringify(fieldRef);
+ }
+
+ fieldRef = getNormalizedDimensionReference(fieldRef);
+
+ if (
+ isFieldReference(fieldRef) ||
+ isExpressionReference(fieldRef) ||
+ isAggregationReference(fieldRef)
+ ) {
+ fieldRef = getBaseDimensionReference(fieldRef);
+ }
+
+ const isLegacyRef =
+ (isFieldReference(fieldRef) && hasStringFieldName(fieldRef)) ||
+ isAggregationReference(fieldRef);
+
+ return JSON.stringify(
+ isLegacyRef ? ["name", column.name] : ["ref", fieldRef],
+ );
+};
+
+export const getColumnNameFromKey = (key: string) => {
+ try {
+ const [tag, name] = JSON.parse(key);
+ return tag === "name" ? name : undefined;
+ } catch {
+ return undefined;
+ }
+};
+
+export const getColumnSettings = (
+ settings: VisualizationSettings | null | undefined,
+ column: DatasetColumnReference,
+) => {
+ return getObjectColumnSettings(settings?.column_settings, column);
+};
+
+// Gets the corresponding viz settings for the column. We check for both
+// legacy and modern keys because not all viz settings have been migrated.
+// To be extra safe and maintain backward compatibility, we check for the
+// legacy key first.
+export const getObjectColumnSettings = (
+ settings: Record<string, ColumnSettings> | null | undefined,
+ column: DatasetColumnReference,
+) => {
+ return (
+ settings?.[getLegacyColumnKey(column)] ?? settings?.[getColumnKey(column)]
+ );
+};
diff --git a/frontend/src/metabase-lib/v1/queries/utils/column-key.unit.spec.ts b/frontend/src/metabase-lib/v1/queries/utils/column-key.unit.spec.ts
new file mode 100644
index 00000000000..ff4594ef782
--- /dev/null
+++ b/frontend/src/metabase-lib/v1/queries/utils/column-key.unit.spec.ts
@@ -0,0 +1,208 @@
+import {
+ getColumnKey,
+ getColumnNameFromKey,
+ getColumnSettings,
+ getLegacyColumnKey,
+ getObjectColumnSettings,
+} from "metabase-lib/v1/queries/utils/column-key";
+import type { DatasetColumn } from "metabase-types/api";
+import {
+ createMockColumn,
+ createMockVisualizationSettings,
+} from "metabase-types/api/mocks";
+
+type TestCase = {
+ title: string;
+ column: DatasetColumn;
+ expectedKey: string;
+};
+
+describe("getColumnKey", () => {
+ it.each<TestCase>([
+ {
+ title: "id-based field ref",
+ column: createMockColumn({ name: "foo", field_ref: ["field", 1, null] }),
+ expectedKey: JSON.stringify(["name", "foo"]),
+ },
+ {
+ title: "name-based field ref",
+ column: createMockColumn({
+ name: "foo",
+ field_ref: ["field", "foo", { "base-type": "type/Text" }],
+ }),
+ expectedKey: JSON.stringify(["name", "foo"]),
+ },
+ ])("should create a name-based key: $title", ({ column, expectedKey }) => {
+ expect(getColumnKey(column)).toEqual(expectedKey);
+ });
+});
+
+describe("getLegacyColumnKey", () => {
+ it.each<TestCase>([
+ {
+ title: "id-based field ref",
+ column: createMockColumn({ name: "foo", field_ref: ["field", 1, null] }),
+ expectedKey: JSON.stringify(["ref", ["field", 1, null]]),
+ },
+ {
+ title: "explicitly joined column",
+ column: createMockColumn({
+ name: "foo",
+ field_ref: ["field", 1, { "join-alias": "x" }],
+ }),
+ expectedKey: JSON.stringify(["ref", ["field", 1, { "join-alias": "x" }]]),
+ },
+ {
+ title: "implicitly joined column",
+ column: createMockColumn({
+ name: "foo",
+ field_ref: ["field", 1, { "source-field": 2 }],
+ }),
+ expectedKey: JSON.stringify(["ref", ["field", 1, { "source-field": 2 }]]),
+ },
+ {
+ title: "temporal-unit is removed from the key",
+ column: createMockColumn({
+ name: "foo",
+ field_ref: ["field", 1, { "temporal-unit": "minute" }],
+ }),
+ expectedKey: JSON.stringify(["ref", ["field", 1, null]]),
+ },
+ {
+ title: "binning is removed from key",
+ column: createMockColumn({
+ name: "foo",
+ field_ref: [
+ "field",
+ 1,
+ {
+ "base-type": "type/Integer",
+ binning: { strategy: "num-bins", "num-bins": 10 },
+ },
+ ],
+ }),
+ expectedKey: JSON.stringify([
+ "ref",
+ ["field", 1, { "base-type": "type/Integer" }],
+ ]),
+ },
+ {
+ title: "expression",
+ column: createMockColumn({
+ name: "foo",
+ field_ref: ["expression", "foo"],
+ }),
+ expectedKey: JSON.stringify(["ref", ["expression", "foo"]]),
+ },
+ ])("should create a ref-based key: $title", ({ column, expectedKey }) => {
+ expect(getLegacyColumnKey(column)).toEqual(expectedKey);
+ });
+
+ it.each<TestCase>([
+ {
+ title: "name-based field ref",
+ column: createMockColumn({
+ name: "foo",
+ field_ref: ["field", "foo", { "base-type": "type/Text" }],
+ }),
+ expectedKey: JSON.stringify(["name", "foo"]),
+ },
+ {
+ title: "aggregation",
+ column: createMockColumn({
+ name: "count",
+ field_ref: ["aggregation", 0],
+ }),
+ expectedKey: JSON.stringify(["name", "count"]),
+ },
+ ])("should create a name-based key: $title", ({ column, expectedKey }) => {
+ expect(getLegacyColumnKey(column)).toEqual(expectedKey);
+ });
+});
+
+describe("getColumnNameFromKey", () => {
+ it("should return the name from a name-based key", () => {
+ expect(getColumnNameFromKey(JSON.stringify(["name", "foo"]))).toBe("foo");
+ });
+
+ it("should ignore a field ref-based key", () => {
+ expect(
+ getColumnNameFromKey(JSON.stringify(["ref", ["field", 1, null]])),
+ ).toBeUndefined();
+ });
+
+ it("should ignore invalid json", () => {
+ expect(getColumnNameFromKey("not a json string")).toBeUndefined();
+ });
+});
+
+describe("getColumnSettings", () => {
+ const column = createMockColumn({
+ name: "foo",
+ field_ref: ["field", 1, { "temporal-unit": "minute" }],
+ });
+
+ it("should ignore missing settings", () => {
+ expect(getColumnSettings(null, column)).toBeUndefined();
+ });
+
+ it("should ignore missing column_settings", () => {
+ const settings = createMockVisualizationSettings({
+ column_settings: undefined,
+ });
+ expect(getColumnSettings(settings, column)).toBeUndefined();
+ });
+
+ it("should prefer legacy keys over new keys", () => {
+ const settings = createMockVisualizationSettings({
+ column_settings: {
+ [getLegacyColumnKey(column)]: { column_title: "A" },
+ [getColumnKey(column)]: { column_title: "B" },
+ },
+ });
+ expect(getColumnSettings(settings, column)).toEqual({
+ column_title: "A",
+ });
+ });
+
+ it("should use new keys if old keys are not available", () => {
+ const settings = createMockVisualizationSettings({
+ column_settings: {
+ [getColumnKey(column)]: { column_title: "B" },
+ },
+ });
+ expect(getColumnSettings(settings, column)).toEqual({
+ column_title: "B",
+ });
+ });
+});
+
+describe("getObjectColumnSettings", () => {
+ const column = createMockColumn({
+ name: "foo",
+ field_ref: ["field", 1, { "temporal-unit": "minute" }],
+ });
+
+ it("should ignore missing settings", () => {
+ expect(getObjectColumnSettings(null, column)).toBeUndefined();
+ });
+
+ it("should prefer legacy keys over new keys", () => {
+ const settings = {
+ [getLegacyColumnKey(column)]: { column_title: "A" },
+ [getColumnKey(column)]: { column_title: "B" },
+ };
+ expect(getObjectColumnSettings(settings, column)).toEqual({
+ column_title: "A",
+ });
+ });
+
+ it("should use new keys if old keys are not available", () => {
+ const settings = {
+ [getColumnKey(column)]: { column_title: "B" },
+ };
+ expect(getObjectColumnSettings(settings, column)).toEqual({
+ column_title: "B",
+ });
+ });
+});
diff --git a/frontend/src/metabase-lib/v1/queries/utils/dataset.ts b/frontend/src/metabase-lib/v1/queries/utils/dataset.ts
index 3608f0a2baa..5a06108edbf 100644
--- a/frontend/src/metabase-lib/v1/queries/utils/dataset.ts
+++ b/frontend/src/metabase-lib/v1/queries/utils/dataset.ts
@@ -1,14 +1,12 @@
-import type {
- DatasetColumn,
- DatasetData,
- TableColumnOrderSetting,
-} from "metabase-types/api";
+import type { DatasetData, TableColumnOrderSetting } from "metabase-types/api";
+
+import type { DatasetColumnReference } from "./column-key";
export const datasetContainsNoResults = (data: DatasetData) =>
data.rows == null || data.rows.length === 0;
export function findColumnIndexesForColumnSettings(
- columns: Pick<DatasetColumn, "name" | "field_ref">[],
+ columns: DatasetColumnReference[],
columnSettings: TableColumnOrderSetting[],
) {
const columnIndexByKey = new Map(
@@ -20,7 +18,7 @@ export function findColumnIndexesForColumnSettings(
}
export function findColumnSettingIndexesForColumns(
- columns: Pick<DatasetColumn, "name" | "field_ref">[],
+ columns: DatasetColumnReference[],
columnSettings: TableColumnOrderSetting[],
) {
const columnSettingIndexByKey = new Map(
diff --git a/frontend/src/metabase-lib/v1/queries/utils/get-column-key.ts b/frontend/src/metabase-lib/v1/queries/utils/get-column-key.ts
deleted file mode 100644
index 34c34e53d75..00000000000
--- a/frontend/src/metabase-lib/v1/queries/utils/get-column-key.ts
+++ /dev/null
@@ -1,44 +0,0 @@
-import {
- createFieldReference,
- getBaseDimensionReference,
- getNormalizedDimensionReference,
- hasStringFieldName,
- isAggregationReference,
- isExpressionReference,
- isFieldReference,
- isValidDimensionReference,
-} from "metabase-lib/v1/references";
-import type { DatasetColumn } from "metabase-types/api";
-
-export const getColumnKey = (
- column: Pick<DatasetColumn, "name" | "field_ref">,
-) => {
- let fieldRef = column.field_ref;
-
- if (!fieldRef) {
- fieldRef = createFieldReference(column.name);
- }
-
- if (!isValidDimensionReference(fieldRef)) {
- console.warn("Unknown field_ref", fieldRef);
- return JSON.stringify(fieldRef);
- }
-
- fieldRef = getNormalizedDimensionReference(fieldRef);
-
- if (
- isFieldReference(fieldRef) ||
- isExpressionReference(fieldRef) ||
- isAggregationReference(fieldRef)
- ) {
- fieldRef = getBaseDimensionReference(fieldRef);
- }
-
- const isLegacyRef =
- (isFieldReference(fieldRef) && hasStringFieldName(fieldRef)) ||
- isAggregationReference(fieldRef);
-
- return JSON.stringify(
- isLegacyRef ? ["name", column.name] : ["ref", fieldRef],
- );
-};
diff --git a/frontend/src/metabase-lib/v1/queries/utils/get-column-key.unit.spec.js b/frontend/src/metabase-lib/v1/queries/utils/get-column-key.unit.spec.js
deleted file mode 100644
index 59495d6ba11..00000000000
--- a/frontend/src/metabase-lib/v1/queries/utils/get-column-key.unit.spec.js
+++ /dev/null
@@ -1,90 +0,0 @@
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
-
-describe("getColumnKey", () => {
- // NOTE: run legacy tests with and without a field_ref. without is disabled in latest since it now always uses
- // field_ref, leaving test code in place to compare against older versions
- for (const fieldRefEnabled of [/*false,*/ true]) {
- describe(fieldRefEnabled ? "with field_ref" : "without field_ref", () => {
- it("should return [ref [field ...]] for field", () => {
- expect(
- getColumnKey({
- name: "foo",
- id: 1,
- field_ref: fieldRefEnabled ? ["field", 1, null] : undefined,
- }),
- ).toEqual(JSON.stringify(["ref", ["field", 1, null]]));
- });
- it("should return [ref [field ...]] for foreign field", () => {
- expect(
- getColumnKey({
- name: "foo",
- id: 1,
- fk_field_id: 2,
- field_ref: fieldRefEnabled
- ? ["field", 1, { "source-field": 2 }]
- : undefined,
- }),
- ).toEqual(JSON.stringify(["ref", ["field", 1, { "source-field": 2 }]]));
- });
- it("should return [ref [expression ...]] for expression", () => {
- expect(
- getColumnKey({
- name: "foo",
- expression_name: "foo",
- field_ref: fieldRefEnabled ? ["expression", "foo"] : undefined,
- }),
- ).toEqual(JSON.stringify(["ref", ["expression", "foo"]]));
- });
- it("should return [name ...] for aggregation", () => {
- const col = {
- name: "foo",
- source: "aggregation",
- field_ref: fieldRefEnabled ? ["aggregation", 0] : undefined,
- };
- expect(getColumnKey(col, [col])).toEqual(
- // NOTE: not ideal, matches existing behavior, but should be ["aggregation", 0]
- JSON.stringify(["name", "foo"]),
- );
- });
- it("should return [name ...] for aggregation on field literal", () => {
- const col = {
- name: "foo",
- id: ["field", "foo", { "base-type": "type/Integer" }],
- field_ref: fieldRefEnabled
- ? ["field", "foo", { "base-type": "type/Integer" }]
- : undefined,
- };
- expect(getColumnKey(col, [col])).toEqual(
- // NOTE: not ideal, matches existing behavior, but should be ["field", "foo", {"base-type": "type/Integer"}]
- JSON.stringify(["name", "foo"]),
- );
- });
- it("should return [field ...] for native query column", () => {
- expect(
- getColumnKey({
- name: "foo",
- field_ref: fieldRefEnabled
- ? ["field", "foo", { "base-type": "type/Integer" }]
- : undefined,
- }),
- ).toEqual(
- // NOTE: not ideal, matches existing behavior, but should be ["field", "foo", {"base-type": "type/Integer"}]
- JSON.stringify(["name", "foo"]),
- );
- });
- });
- }
-
- describe("with field_ref", () => {
- it("should return [ref [field ...]] for joined field", () => {
- const col = {
- name: "foo",
- id: 1,
- field_ref: ["field", 1, { "join-alias": "x" }],
- };
- expect(getColumnKey(col)).toEqual(
- JSON.stringify(["ref", ["field", 1, { "join-alias": "x" }]]),
- );
- });
- });
-});
diff --git a/frontend/src/metabase-types/api/card.ts b/frontend/src/metabase-types/api/card.ts
index e954a59e778..3d8c4a6aedf 100644
--- a/frontend/src/metabase-types/api/card.ts
+++ b/frontend/src/metabase-types/api/card.ts
@@ -126,6 +126,15 @@ export type XAxisScale = "ordinal" | "histogram" | "timeseries" | NumericScale;
export type YAxisScale = NumericScale;
+export interface ColumnSettings {
+ column_title?: string;
+ number_separators?: string;
+ currency?: string;
+
+ // some options are untyped
+ [key: string]: any;
+}
+
export type VisualizationSettings = {
"graph.show_values"?: boolean;
"stackable.stack_type"?: StackType;
@@ -133,6 +142,9 @@ export type VisualizationSettings = {
// Table
"table.columns"?: TableColumnOrderSetting[];
+ // Keys here can be modern (returned by `getColumnKey`) or legacy (`getLegacyColumnKey`).
+ // Use `getColumnSettings` which checks for both keys.
+ column_settings?: Record<string, ColumnSettings>;
// X-axis
"graph.x_axis.title_text"?: string;
diff --git a/frontend/src/metabase/admin/datamodel/components/filters/pickers/DefaultPicker/DefaultPicker.tsx b/frontend/src/metabase/admin/datamodel/components/filters/pickers/DefaultPicker/DefaultPicker.tsx
index d2b0d6f41fa..1d4fe8ebda6 100644
--- a/frontend/src/metabase/admin/datamodel/components/filters/pickers/DefaultPicker/DefaultPicker.tsx
+++ b/frontend/src/metabase/admin/datamodel/components/filters/pickers/DefaultPicker/DefaultPicker.tsx
@@ -11,7 +11,7 @@ import {
isFuzzyOperator,
} from "metabase-lib/v1/operators/utils";
import type Filter from "metabase-lib/v1/queries/structured/Filter";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
+import { getColumnSettings } from "metabase-lib/v1/queries/utils/column-key";
import { isCurrency } from "metabase-lib/v1/types/utils/isa";
import type { DatasetColumn, FieldId, RowValue } from "metabase-types/api";
@@ -64,11 +64,9 @@ export function DefaultPicker({
?.question()
?.settings();
- const key = dimension?.column?.()
- ? getColumnKey(dimension.column() as DatasetColumn)
- : "";
-
- const columnSettings = visualizationSettings?.column_settings?.[key];
+ const column = dimension?.column?.();
+ const columnSettings =
+ column && getColumnSettings(visualizationSettings, column as DatasetColumn);
const fieldMetadata = field?.metadata?.fields[field?.id as FieldId];
const fieldSettings = {
diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebar.tsx b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebar.tsx
index f567b938ad5..301e2ba6b0b 100644
--- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebar.tsx
+++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebar.tsx
@@ -10,7 +10,7 @@ import {
canSaveClickBehavior,
clickBehaviorIsValid,
} from "metabase-lib/v1/parameters/utils/click-behavior";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
+import { getColumnKey } from "metabase-lib/v1/queries/utils/column-key";
import type {
Dashboard,
DashCardId,
diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/utils.ts b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/utils.ts
index a8c06f652d5..57744c0a60a 100644
--- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/utils.ts
+++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/utils.ts
@@ -1,7 +1,5 @@
-import { getIn } from "icepick";
-
import type { IconName } from "metabase/ui";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
+import { getColumnSettings } from "metabase-lib/v1/queries/utils/column-key";
import type {
ClickBehaviorType,
DatasetColumn,
@@ -23,10 +21,11 @@ export function getClickBehaviorForColumn(
dashcard: DashboardCard,
column: DatasetColumn,
) {
- return getIn(dashcard, [
- "visualization_settings",
- "column_settings",
- getColumnKey(column),
- "click_behavior",
- ]);
+ if (dashcard.visualization_settings) {
+ const columnSettings = getColumnSettings(
+ dashcard.visualization_settings,
+ column,
+ );
+ return columnSettings?.click_behavior;
+ }
}
diff --git a/frontend/src/metabase/querying/utils/viz-settings/sync-viz-settings.ts b/frontend/src/metabase/querying/utils/viz-settings/sync-viz-settings.ts
index 808e7332fc1..311ffc34838 100644
--- a/frontend/src/metabase/querying/utils/viz-settings/sync-viz-settings.ts
+++ b/frontend/src/metabase/querying/utils/viz-settings/sync-viz-settings.ts
@@ -1,5 +1,10 @@
import * as Lib from "metabase-lib";
+import {
+ getColumnKey,
+ getColumnNameFromKey,
+} from "metabase-lib/v1/queries/utils/column-key";
import type {
+ ColumnSettings,
Series,
SingleSeries,
VisualizationSettings,
@@ -53,6 +58,7 @@ export function syncVizSettings(
): VisualizationSettings {
let nextSettings = settings;
nextSettings = syncTableColumns(nextSettings, newColumns, oldColumns);
+ nextSettings = syncColumnSettings(nextSettings, newColumns, oldColumns);
nextSettings = syncGraphMetrics(nextSettings, newColumns, oldColumns);
return nextSettings;
}
@@ -90,7 +96,7 @@ type SyncColumnNamesOpts<T> = {
settings: T[];
newColumns: ColumnInfo[];
oldColumns: ColumnInfo[];
- getColumnName: (setting: T) => string;
+ getColumnName: (setting: T) => string | undefined;
setColumnName: (setting: T, newName: string) => T;
createSetting: (column: ColumnInfo) => T;
shouldCreateSetting: (column: ColumnInfo) => boolean | undefined;
@@ -115,8 +121,9 @@ function syncColumns<T>({
oldColumns.map(column => [column.key, column.name]),
);
const remappedSettings = settings.reduce((settings: T[], setting) => {
- const oldKey = oldKeyByName[getColumnName(setting)];
- const newName = newNameByKey[oldKey];
+ const oldName = getColumnName(setting);
+ const oldKey = oldName && oldKeyByName[oldName];
+ const newName = oldKey && newNameByKey[oldKey];
if (!oldKey) {
settings.push(setting);
} else if (newName) {
@@ -156,6 +163,32 @@ function syncTableColumns(
};
}
+function syncColumnSettings(
+ settings: VisualizationSettings,
+ newColumns: ColumnInfo[],
+ oldColumns: ColumnInfo[],
+): VisualizationSettings {
+ const columnSettings = settings["column_settings"];
+ if (!columnSettings) {
+ return settings;
+ }
+
+ const columnEntries = syncColumns<[string, ColumnSettings]>({
+ settings: Object.entries(columnSettings),
+ newColumns,
+ oldColumns,
+ getColumnName: ([key]) => getColumnNameFromKey(key),
+ setColumnName: ([_, setting], name) => [getColumnKey({ name }), setting],
+ createSetting: column => [getColumnKey(column), {}],
+ shouldCreateSetting: () => false,
+ });
+
+ return {
+ ...settings,
+ column_settings: Object.fromEntries(columnEntries),
+ };
+}
+
function syncGraphMetrics(
settings: VisualizationSettings,
newColumns: ColumnInfo[],
diff --git a/frontend/src/metabase/querying/utils/viz-settings/sync-viz-settings.unit.spec.tsx b/frontend/src/metabase/querying/utils/viz-settings/sync-viz-settings.unit.spec.tsx
index b342a8e9ca8..408a7325112 100644
--- a/frontend/src/metabase/querying/utils/viz-settings/sync-viz-settings.unit.spec.tsx
+++ b/frontend/src/metabase/querying/utils/viz-settings/sync-viz-settings.unit.spec.tsx
@@ -157,6 +157,35 @@ describe("syncVizSettings", () => {
});
});
+ describe("column_settings", () => {
+ it("should handle adding new columns with column.name changes", () => {
+ const oldColumns: ColumnInfo[] = [
+ { name: "ID", key: "ID" },
+ { name: "ID_2", key: "PEOPLE__ID" },
+ ];
+ const newColumns: ColumnInfo[] = [
+ { name: "ID", key: "ID" },
+ { name: "ID_2", key: "PRODUCTS__ID" },
+ { name: "ID_3", key: "PEOPLE__ID" },
+ ];
+
+ const oldSettings = createMockVisualizationSettings({
+ column_settings: {
+ '["name","ID"]': { column_title: "@ID" },
+ '["name","ID_2"]': { column_title: "ID@" },
+ },
+ });
+
+ const newSettings = syncVizSettings(oldSettings, newColumns, oldColumns);
+ expect(newSettings).toEqual({
+ column_settings: {
+ '["name","ID"]': { column_title: "@ID" },
+ '["name","ID_3"]': { column_title: "ID@" },
+ },
+ });
+ });
+ });
+
describe("graph.metrics", () => {
it("should not update the setting if the order of columns has changed", () => {
const oldColumns: ColumnInfo[] = [
diff --git a/frontend/src/metabase/static-viz/lib/format.ts b/frontend/src/metabase/static-viz/lib/format.ts
index 22d610b2f31..01d7e7d5341 100644
--- a/frontend/src/metabase/static-viz/lib/format.ts
+++ b/frontend/src/metabase/static-viz/lib/format.ts
@@ -19,7 +19,7 @@ import type {
} from "metabase/visualizations/shared/types/format";
import { getLabelsMetricColumn } from "metabase/visualizations/shared/utils/series";
import type { RemappingHydratedDatasetColumn } from "metabase/visualizations/types";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
+import { getColumnSettings } from "metabase-lib/v1/queries/utils/column-key";
import { rangeForValue } from "metabase-lib/v1/queries/utils/range-for-value";
import {
isCoordinate,
@@ -109,7 +109,7 @@ export const getStaticFormatters = (
): ChartTicksFormatters => {
const yTickFormatter = (value: StringLike) => {
const column = chartColumns.dimension.column;
- const columnSettings = settings.column_settings?.[getColumnKey(column)];
+ const columnSettings = getColumnSettings(settings, column);
const valueToFormat = getRemappedValue(value, column);
const options = getFormattingOptionsWithoutScaling({
@@ -125,8 +125,10 @@ export const getStaticFormatters = (
const percentXTicksFormatter = (percent: NumberLike) => {
const column = metricColumn.column;
- const number_separators =
- settings.column_settings?.[getColumnKey(column)]?.number_separators;
+ const number_separators = getColumnSettings(
+ settings,
+ column,
+ )?.number_separators;
const options = getFormattingOptionsWithoutScaling({
column,
@@ -141,7 +143,7 @@ export const getStaticFormatters = (
const xTickFormatter = (value: NumberLike) => {
const column = metricColumn.column;
- const columnSettings = settings.column_settings?.[getColumnKey(column)];
+ const columnSettings = getColumnSettings(settings, column);
const valueToFormat = getRemappedValue(value, column);
const options = getFormattingOptionsWithoutScaling({
@@ -168,7 +170,7 @@ export const getLabelsStaticFormatter = (
settings: VisualizationSettings,
): ValueFormatter => {
const column = getLabelsMetricColumn(chartColumns).column;
- const columnSettings = settings.column_settings?.[getColumnKey(column)];
+ const columnSettings = getColumnSettings(settings, column);
const options = getFormattingOptionsWithoutScaling({
column,
...columnSettings,
diff --git a/frontend/src/metabase/static-viz/lib/settings.ts b/frontend/src/metabase/static-viz/lib/settings.ts
index 2641b98f007..b5e0fd0c188 100644
--- a/frontend/src/metabase/static-viz/lib/settings.ts
+++ b/frontend/src/metabase/static-viz/lib/settings.ts
@@ -9,8 +9,7 @@ import type {
ComputedVisualizationSettings,
RemappingHydratedDatasetColumn,
} from "metabase/visualizations/types";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
-import { normalize } from "metabase-lib/v1/queries/utils/normalize";
+import { getObjectColumnSettings } from "metabase-lib/v1/queries/utils/column-key";
import { isCoordinate, isNumber } from "metabase-lib/v1/types/utils/isa";
import type {
DatasetColumn,
@@ -33,14 +32,11 @@ const getColumnSettings = (
column: DatasetColumn,
settings: VisualizationSettings,
): Record<string, unknown> => {
- const columnKey = Object.keys(settings.column_settings ?? {}).find(
- possiblyDenormalizedFieldRef =>
- normalize(possiblyDenormalizedFieldRef) === getColumnKey(column),
+ const storedSettings = getObjectColumnSettings(
+ settings.column_settings,
+ column,
);
-
- const columnSettings = columnKey
- ? { column, ...column.settings, ...settings.column_settings?.[columnKey] }
- : { column, ...column.settings };
+ const columnSettings = { column, ...column.settings, ...storedSettings };
if (isNumber(column) && !isCoordinate(column)) {
fillWithDefaultValue(columnSettings, "currency", getDefaultCurrency());
diff --git a/frontend/src/metabase/visualizations/click-actions/actions/ColumnFormattingAction/ColumnFormattingAction.tsx b/frontend/src/metabase/visualizations/click-actions/actions/ColumnFormattingAction/ColumnFormattingAction.tsx
index 7dd0fa04b3b..2c68422d352 100644
--- a/frontend/src/metabase/visualizations/click-actions/actions/ColumnFormattingAction/ColumnFormattingAction.tsx
+++ b/frontend/src/metabase/visualizations/click-actions/actions/ColumnFormattingAction/ColumnFormattingAction.tsx
@@ -8,7 +8,7 @@ import type {
LegacyDrill,
} from "metabase/visualizations/types";
import * as Lib from "metabase-lib";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
+import { getColumnKey } from "metabase-lib/v1/queries/utils/column-key";
import type { VisualizationSettings } from "metabase-types/api";
import { PopoverRoot } from "./ColumnFormattingAction.styled";
diff --git a/frontend/src/metabase/visualizations/components/ChartSettings.jsx b/frontend/src/metabase/visualizations/components/ChartSettings.jsx
index c3585ffacc8..ce175caed7c 100644
--- a/frontend/src/metabase/visualizations/components/ChartSettings.jsx
+++ b/frontend/src/metabase/visualizations/components/ChartSettings.jsx
@@ -24,7 +24,7 @@ import {
import { getSettingDefinitionsForColumn } from "metabase/visualizations/lib/settings/column";
import { keyForSingleSeries } from "metabase/visualizations/lib/settings/series";
import { getSettingsWidgetsForSeries } from "metabase/visualizations/lib/settings/visualization";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
+import { getColumnKey } from "metabase-lib/v1/queries/utils/column-key";
import {
SectionContainer,
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx
index 750fae32efc..4639ed19e55 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx
@@ -3,7 +3,7 @@ import { t } from "ttag";
import _ from "underscore";
import { keyForSingleSeries } from "metabase/visualizations/lib/settings/series";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
+import { getColumnKey } from "metabase-lib/v1/queries/utils/column-key";
import {
SettingsIcon,
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldsPartition.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldsPartition.jsx
index b5fb765347b..d8f13c66ad9 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldsPartition.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldsPartition.jsx
@@ -9,7 +9,7 @@ import _ from "underscore";
import Label from "metabase/components/type/Label";
import { DragDropContext } from "metabase/core/components/DragDropContext";
import CS from "metabase/css/core/index.css";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
+import { getColumnKey } from "metabase-lib/v1/queries/utils/column-key";
import {
DroppableContainer,
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingNestedSettings.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingNestedSettings.jsx
index 9153279194e..00e9af0a775 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingNestedSettings.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingNestedSettings.jsx
@@ -11,7 +11,7 @@ import ChartSettingsWidget from "../ChartSettingsWidget";
* @deprecated HOCs are deprecated
*/
const chartSettingNestedSettings =
- ({ getObjectKey, getSettingsWidgetsForObject }) =>
+ ({ getObjectKey, getObjectSettings, getSettingsWidgetsForObject }) =>
ComposedComponent =>
class extends Component {
constructor(props) {
@@ -55,15 +55,23 @@ const chartSettingNestedSettings =
}
};
- handleChangeSettingsForObjectKey = (objectKey, changedSettings) => {
- const { onChange } = this.props;
- const objectsSettings = this.props.value || {};
- const objectSettings = objectsSettings[objectKey] || {};
- const newSettings = updateSettings(objectSettings, changedSettings);
- onChange({
- ...objectsSettings,
- [objectKey]: newSettings,
- });
+ handleChangeSettingsForObjectKey = (changedKey, changedSettings) => {
+ const { objects, onChange } = this.props;
+ const oldSettings = this.props.value || {};
+ const newSettings = objects.reduce((newSettings, object) => {
+ const currentKey = getObjectKey(object);
+ const objectSettings = getObjectSettings(oldSettings, object);
+ if (currentKey === changedKey) {
+ newSettings[currentKey] = updateSettings(
+ objectSettings,
+ changedSettings,
+ );
+ } else {
+ newSettings[currentKey] = objectSettings;
+ }
+ return newSettings;
+ }, {});
+ onChange(newSettings);
};
render() {
@@ -76,7 +84,8 @@ const chartSettingNestedSettings =
);
if (editingObject) {
const objectsSettings = this.props.value || {};
- const objectSettings = objectsSettings[editingObjectKey] || {};
+ const objectSettings =
+ getObjectSettings(objectsSettings, editingObject) ?? {};
const objectSettingsWidgets = getSettingsWidgetsForObject(
series,
editingObject,
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingTableColumns/TableColumnPanel/utils.ts b/frontend/src/metabase/visualizations/components/settings/ChartSettingTableColumns/TableColumnPanel/utils.ts
index 1b49edb770b..0cb8d64620a 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingTableColumns/TableColumnPanel/utils.ts
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingTableColumns/TableColumnPanel/utils.ts
@@ -1,7 +1,7 @@
import type { IconName } from "metabase/ui";
import { getIconForField } from "metabase-lib/v1/metadata/utils/fields";
+import { getColumnKey } from "metabase-lib/v1/queries/utils/column-key";
import { findColumnIndexesForColumnSettings } from "metabase-lib/v1/queries/utils/dataset";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
import type {
DatasetColumn,
TableColumnOrderSetting,
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingsListColumns/ChartSettingsListColumns.tsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingsListColumns/ChartSettingsListColumns.tsx
index 15fc8bba604..ab564b66b97 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingsListColumns/ChartSettingsListColumns.tsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingsListColumns/ChartSettingsListColumns.tsx
@@ -5,7 +5,7 @@ import _ from "underscore";
import Button from "metabase/core/components/Button";
import type Question from "metabase-lib/v1/Question";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
+import { getColumnKey } from "metabase-lib/v1/queries/utils/column-key";
import type {
ConcreteFieldReference,
DatasetColumn,
diff --git a/frontend/src/metabase/visualizations/echarts/cartesian/model/util.ts b/frontend/src/metabase/visualizations/echarts/cartesian/model/util.ts
index 203451274e2..217d9a2f23c 100644
--- a/frontend/src/metabase/visualizations/echarts/cartesian/model/util.ts
+++ b/frontend/src/metabase/visualizations/echarts/cartesian/model/util.ts
@@ -3,7 +3,7 @@ import type {
ComputedVisualizationSettings,
RemappingHydratedDatasetColumn,
} from "metabase/visualizations/types";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
+import { getColumnSettings } from "metabase-lib/v1/queries/utils/column-key";
import {
NEGATIVE_BAR_DATA_LABEL_KEY_SUFFIX,
@@ -28,8 +28,7 @@ export function getColumnScaling(
settings: ComputedVisualizationSettings,
) {
const columnSettings =
- settings.column?.(column) ??
- settings.column_settings?.[getColumnKey(column)];
+ settings.column?.(column) ?? getColumnSettings(settings, column);
const scale = columnSettings?.scale;
return Number.isFinite(scale) ? (scale as number) : 1;
}
diff --git a/frontend/src/metabase/visualizations/lib/settings/column.js b/frontend/src/metabase/visualizations/lib/settings/column.js
index 77319f7859f..1e8b79d7335 100644
--- a/frontend/src/metabase/visualizations/lib/settings/column.js
+++ b/frontend/src/metabase/visualizations/lib/settings/column.js
@@ -23,7 +23,10 @@ import {
isDateWithoutTime,
isNumber,
} from "metabase-lib/v1/types/utils/isa";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
+import {
+ getColumnKey,
+ getObjectColumnSettings,
+} from "metabase-lib/v1/queries/utils/column-key";
import {
findColumnIndexesForColumnSettings,
findColumnSettingIndexesForColumns,
@@ -67,6 +70,7 @@ export function columnSettings({
objectName: "column",
getObjects: getColumns,
getObjectKey: getColumnKey,
+ getObjectSettings: getObjectColumnSettings,
getSettingDefinitionsForObject: getSettingDefinitionsForColumn,
component: ChartNestedSettingColumns,
getInheritedSettingsForObject: getInhertiedSettingsForColumn,
diff --git a/frontend/src/metabase/visualizations/lib/settings/nested.js b/frontend/src/metabase/visualizations/lib/settings/nested.js
index c49187f3b12..3bb60312e2f 100644
--- a/frontend/src/metabase/visualizations/lib/settings/nested.js
+++ b/frontend/src/metabase/visualizations/lib/settings/nested.js
@@ -11,6 +11,7 @@ export function nestedSettings(
objectName = "object",
getObjects,
getObjectKey,
+ getObjectSettings,
getSettingDefinitionsForObject,
getInheritedSettingsForObject = () => ({}),
component,
@@ -42,7 +43,7 @@ export function nestedSettings(
allComputedSettings[key] = getComputedSettingsForObject(
series,
object,
- allStoredSettings[key] || {},
+ getObjectSettings(allStoredSettings, object) ?? {},
extra,
);
}
@@ -77,6 +78,7 @@ export function nestedSettings(
// decorate with nested settings HOC
const widget = chartSettingNestedSettings({
getObjectKey,
+ getObjectSettings,
getSettingsWidgetsForObject,
})(component);
@@ -112,7 +114,8 @@ export function nestedSettings(
const key = getObjectKey(object);
if (!cache.has(key)) {
const inheritedSettings = getInheritedSettingsForObject(object);
- const storedSettings = settings[id][key] || {};
+ const storedSettings =
+ getObjectSettings(settings[id], object) ?? {};
cache.set(key, {
...getComputedSettingsForObject(
series,
diff --git a/frontend/src/metabase/visualizations/lib/settings/nested.unit.spec.js b/frontend/src/metabase/visualizations/lib/settings/nested.unit.spec.js
index 00e54303a45..3b036b6a267 100644
--- a/frontend/src/metabase/visualizations/lib/settings/nested.unit.spec.js
+++ b/frontend/src/metabase/visualizations/lib/settings/nested.unit.spec.js
@@ -8,6 +8,7 @@ describe("nestedSettings", () => {
objectName: "nested",
getObjects: () => [1, 2, 3],
getObjectKey: object => String(object),
+ getObjectSettings: (objects, key) => objects[key],
getSettingDefinitionsForObject: () => ({
foo: { getDefault: object => `foo${object}` },
}),
diff --git a/frontend/src/metabase/visualizations/lib/settings/series.js b/frontend/src/metabase/visualizations/lib/settings/series.js
index c28f0da8c48..c65a68de7d0 100644
--- a/frontend/src/metabase/visualizations/lib/settings/series.js
+++ b/frontend/src/metabase/visualizations/lib/settings/series.js
@@ -1,6 +1,5 @@
import { getIn } from "icepick";
import { t } from "ttag";
-import _ from "underscore";
import ChartNestedSettingSeries from "metabase/visualizations/components/settings/ChartNestedSettingSeries";
import {
@@ -205,6 +204,8 @@ export function seriesSetting({
objectName: "series",
getObjects: (series, settings) => series,
getObjectKey: keyForSingleSeries,
+ getObjectSettings: (settings, object) =>
+ settings[keyForSingleSeries(object)],
getSettingDefinitionsForObject: getSettingDefinitionsForSingleSeries,
component: ChartNestedSettingSeries,
readDependencies: [SERIES_COLORS_SETTING_KEY, ...readDependencies],
diff --git a/frontend/src/metabase/visualizations/lib/utils.js b/frontend/src/metabase/visualizations/lib/utils.js
index 353c3725394..ecb56d212ee 100644
--- a/frontend/src/metabase/visualizations/lib/utils.js
+++ b/frontend/src/metabase/visualizations/lib/utils.js
@@ -4,7 +4,7 @@ import { t } from "ttag";
import _ from "underscore";
import { isNotNull } from "metabase/lib/types";
-import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
+import { getColumnKey } from "metabase-lib/v1/queries/utils/column-key";
import { isDimension, isMetric, isDate } from "metabase-lib/v1/types/utils/isa";
export const MAX_SERIES = 100;
diff --git a/frontend/src/metabase/visualizations/shared/settings/column.ts b/frontend/src/metabase/visualizations/shared/settings/column.ts
index 69430bff255..a9a72be0c8e 100644
--- a/frontend/src/metabase/visualizations/shared/settings/column.ts
+++ b/frontend/src/metabase/visualizations/shared/settings/column.ts
@@ -1,9 +1,9 @@
import { isCurrency, isPercentage } from "metabase-lib/v1/types/utils/isa";
-import type { DatasetColumn } from "metabase-types/api";
+import type { ColumnSettings, DatasetColumn } from "metabase-types/api";
export function getDefaultNumberStyle(
column: DatasetColumn,
- columnSettings: Record<string, string>,
+ columnSettings: ColumnSettings,
) {
if (isCurrency(column) && columnSettings["currency"]) {
return "currency";
@@ -46,7 +46,7 @@ const CURRENCIES_WITH_SYMBOLS = new Set([
export function getDefaultCurrencyStyle(
_column: any,
- columnSettings: Record<string, string>,
+ columnSettings: ColumnSettings,
) {
const c = columnSettings["currency"] || "USD";
return CURRENCIES_WITH_SYMBOLS.has(c) ? "symbol" : "code";
diff --git a/frontend/src/metabase/visualizations/visualizations/Table.unit.spec.js b/frontend/src/metabase/visualizations/visualizations/Table.unit.spec.js
index 99259d248d5..72d9da2f296 100644
--- a/frontend/src/metabase/visualizations/visualizations/Table.unit.spec.js
+++ b/frontend/src/metabase/visualizations/visualizations/Table.unit.spec.js
@@ -7,8 +7,10 @@ import { renderWithProviders, screen, within } from "__support__/ui";
import ChartSettings from "metabase/visualizations/components/ChartSettings";
import registerVisualizations from "metabase/visualizations/register";
import Question from "metabase-lib/v1/Question";
+import { createMockVisualizationSettings } from "metabase-types/api/mocks";
import {
createSampleDatabase,
+ ORDERS,
ORDERS_ID,
SAMPLE_DB_ID,
} from "metabase-types/api/mocks/presets";
@@ -20,7 +22,9 @@ const metadata = createMockMetadata({
});
const ordersTable = metadata.table(ORDERS_ID);
-const setup = ({ vizType }) => {
+const setup = ({ display, visualization_settings = {} }) => {
+ const onChange = jest.fn();
+
const Container = () => {
const [question, setQuestion] = useState(
new Question(
@@ -32,14 +36,15 @@ const setup = ({ vizType }) => {
},
database: SAMPLE_DB_ID,
},
- display: vizType,
- visualization_settings: {},
+ display,
+ visualization_settings,
},
metadata,
),
);
- const onChange = update => {
+ const handleChange = update => {
+ onChange(update);
setQuestion(q => {
const newQuestion = q.updateSettings(update);
return new Question(thaw(newQuestion.card()), metadata);
@@ -48,7 +53,7 @@ const setup = ({ vizType }) => {
return (
<ChartSettings
- onChange={onChange}
+ onChange={handleChange}
series={[
{
card: question.card(),
@@ -66,13 +71,15 @@ const setup = ({ vizType }) => {
};
renderWithProviders(<Container />);
+
+ return { onChange };
};
// these visualizations share column settings, so all the tests should work for both
-["table", "object"].forEach(vizType => {
- describe(`${vizType} column settings`, () => {
+["table", "object"].forEach(display => {
+ describe(`${display} column settings`, () => {
it("should show you related columns in structured queries", async () => {
- setup({ vizType });
+ setup({ display });
await userEvent.click(screen.getByText("Add or remove columns"));
expect(screen.getByText("User")).toBeInTheDocument();
@@ -85,7 +92,7 @@ const setup = ({ vizType }) => {
});
it("should allow you to show and hide columns", async () => {
- setup({ vizType });
+ setup({ display });
await userEvent.click(await screen.findByTestId("Tax-hide-button"));
expect(
@@ -98,7 +105,7 @@ const setup = ({ vizType }) => {
});
it("should allow you to update a column name", async () => {
- setup({ vizType });
+ setup({ display });
await userEvent.click(
await screen.findByTestId("Subtotal-settings-button"),
);
@@ -109,5 +116,34 @@ const setup = ({ vizType }) => {
await userEvent.click(await screen.findByText("Tax"));
expect(await screen.findByText("Subtotal Updated")).toBeInTheDocument();
});
+
+ it("should rewrite field ref-based column_settings keys to name-based keys on update", async () => {
+ const { onChange } = setup({
+ display,
+ visualization_settings: createMockVisualizationSettings({
+ column_settings: {
+ [JSON.stringify(["ref", ["field", ORDERS.TOTAL, null]])]: {
+ column_title: "Total1",
+ },
+ [JSON.stringify(["ref", ["field", ORDERS.SUBTOTAL, null]])]: {
+ column_title: "Subtotal1",
+ },
+ },
+ }),
+ });
+ await userEvent.click(
+ await screen.findByTestId("Subtotal1-settings-button"),
+ );
+ const input = await screen.findByDisplayValue("Subtotal1");
+ await userEvent.clear(input);
+ await userEvent.type(input, "Subtotal2");
+ await userEvent.click(await screen.findByText("Total1"));
+ expect(onChange).toHaveBeenCalledWith({
+ column_settings: {
+ [JSON.stringify(["name", "TOTAL"])]: { column_title: "Total1" },
+ [JSON.stringify(["name", "SUBTOTAL"])]: { column_title: "Subtotal2" },
+ },
+ });
+ });
});
});
diff --git a/src/metabase/formatter/datetime.clj b/src/metabase/formatter/datetime.clj
index 63b8d1411c1..3b5128a9e5b 100644
--- a/src/metabase/formatter/datetime.clj
+++ b/src/metabase/formatter/datetime.clj
@@ -62,7 +62,8 @@
;; match a key with metadata, even if we do have the correct name or id
(update-keys #(select-keys % [::mb.viz/field-id ::mb.viz/column-name])))]
(or (all-cols-settings {::mb.viz/field-id field-id-or-name})
- (all-cols-settings {::mb.viz/column-name (or field-id-or-name column-name)}))))
+ (all-cols-settings {::mb.viz/column-name field-id-or-name})
+ (all-cols-settings {::mb.viz/column-name column-name}))))
(defn- determine-time-format
"Given viz-settings with a time-style and possible time-enabled (precision) entry, create the format string.
diff --git a/src/metabase/query_processor/streaming/common.clj b/src/metabase/query_processor/streaming/common.clj
index 1056c4110b7..f3fa7d8c3b1 100644
--- a/src/metabase/query_processor/streaming/common.clj
+++ b/src/metabase/query_processor/streaming/common.clj
@@ -101,7 +101,8 @@
(:name col))
col-settings' (update-keys col-settings #(select-keys % [::mb.viz/field-id ::mb.viz/column-name]))
format-settings (or (get col-settings' {::mb.viz/field-id id-or-name})
- (get col-settings' {::mb.viz/column-name id-or-name}))
+ (get col-settings' {::mb.viz/column-name id-or-name})
+ (get col-settings' {::mb.viz/column-name (:name col)}))
is-currency? (or (isa? (:semantic_type col) :type/Currency)
(= (::mb.viz/number-style format-settings) "currency"))
merged-settings (if is-currency?
diff --git a/src/metabase/query_processor/streaming/xlsx.clj b/src/metabase/query_processor/streaming/xlsx.clj
index 3aecd54fc82..60d4595b12a 100644
--- a/src/metabase/query_processor/streaming/xlsx.clj
+++ b/src/metabase/query_processor/streaming/xlsx.clj
@@ -449,7 +449,8 @@
styles (.next sty-it)
id-or-name (or (:id col) (:name col))
settings (or (get col-settings {::mb.viz/field-id id-or-name})
- (get col-settings {::mb.viz/column-name id-or-name}))
+ (get col-settings {::mb.viz/column-name id-or-name})
+ (get col-settings {::mb.viz/column-name (:name col)}))
scaled-val (if (and value (::mb.viz/scale settings))
(* value (::mb.viz/scale settings))
value)
@@ -480,7 +481,8 @@
styles (.next sty-it)
id-or-name (or (:id col) (:name col))
settings (or (get col-settings {::mb.viz/field-id id-or-name})
- (get col-settings {::mb.viz/column-name id-or-name}))
+ (get col-settings {::mb.viz/column-name id-or-name})
+ (get col-settings {::mb.viz/column-name (:name col)}))
scaled-val (if (and value (::mb.viz/scale settings))
(* value (::mb.viz/scale settings))
value)
--
GitLab