From d327c451eadbc873b70ca42a007afd0e5b78747c Mon Sep 17 00:00:00 2001
From: Aleksandr Lesnenko <alxnddr@users.noreply.github.com>
Date: Mon, 17 Oct 2022 16:03:16 +0400
Subject: [PATCH] untie `getColumnKey` function from code not usable in static
viz rendering environment (#25925)
* untie getColumnKey function from unimportable in static viz environment code
* review
* spec
---
frontend/src/metabase-lib/lib/Dimension.ts | 77 +++-------
frontend/src/metabase-lib/lib/Question.ts | 5 +-
.../metabase-lib/lib/parameters/utils/mbql.js | 9 +-
.../metabase-lib/lib/queries/NativeQuery.ts | 5 +-
.../metabase-lib/lib/queries/utils/dataset.js | 35 +----
.../lib/queries/utils/get-column-key.ts | 50 +++++++
.../queries/utils/get-column-key.unit.spec.js | 91 ++++++++++++
frontend/src/metabase-lib/lib/references.ts | 124 +++++++++++++++++
.../metabase-lib/lib/references.unit.spec.ts | 131 ++++++++++++++++++
frontend/src/metabase-types/api/dataset.ts | 8 +-
frontend/src/metabase-types/api/query.ts | 67 +++++++++
.../ClickBehaviorSidebar.tsx | 10 +-
.../ClickBehaviorSidebarContent.tsx | 6 +-
.../ClickBehaviorSidebarHeader.tsx | 5 +-
.../TableClickBehaviorView.tsx | 10 +-
.../components/ClickBehaviorSidebar/utils.ts | 8 +-
.../modes/components/drill/FormatAction.jsx | 4 +-
.../filters/pickers/DefaultPicker.jsx | 4 +-
.../components/RowChart/utils/format.ts | 5 +-
.../settings/ChartSettingFieldPicker.jsx | 4 +-
.../settings/ChartSettingFieldsPartition.jsx | 4 +-
.../settings/ChartSettingOrderedColumns.jsx | 8 +-
.../ChartSettingsListColumns.tsx | 8 +-
.../visualizations/lib/settings/column.js | 4 +-
.../metabase-lib/lib/Dimension.unit.spec.js | 61 --------
.../test/metabase/lib/dataset.unit.spec.js | 93 -------------
26 files changed, 540 insertions(+), 296 deletions(-)
create mode 100644 frontend/src/metabase-lib/lib/queries/utils/get-column-key.ts
create mode 100644 frontend/src/metabase-lib/lib/queries/utils/get-column-key.unit.spec.js
create mode 100644 frontend/src/metabase-lib/lib/references.ts
create mode 100644 frontend/src/metabase-lib/lib/references.unit.spec.ts
diff --git a/frontend/src/metabase-lib/lib/Dimension.ts b/frontend/src/metabase-lib/lib/Dimension.ts
index 8e981b97351..acc9b23089d 100644
--- a/frontend/src/metabase-lib/lib/Dimension.ts
+++ b/frontend/src/metabase-lib/lib/Dimension.ts
@@ -34,6 +34,15 @@ import Aggregation from "metabase-lib/lib/queries/structured/Aggregation";
import Filter from "metabase-lib/lib/queries/structured/Filter";
import StructuredQuery from "metabase-lib/lib/queries/StructuredQuery";
import NativeQuery from "metabase-lib/lib/queries/NativeQuery";
+import {
+ isFieldReference,
+ isExpressionReference,
+ isAggregationReference,
+ isTemplateTagReference,
+ normalizeReferenceOptions,
+ getBaseDimensionReference,
+ BASE_DIMENSION_REFERENCE_OMIT_OPTIONS,
+} from "metabase-lib/lib/references";
/**
* A dimension option returned by the query_metadata API
@@ -87,23 +96,6 @@ export default class Dimension {
this._options = options;
}
- /**
- * Canonically the field clause should use `null` instead of empty options. Keys with null values should get removed.
- */
- static normalizeOptions(options: any): any {
- if (!options) {
- return null;
- }
-
- // recursively normalize maps inside options.
- options = _.mapObject(options, val =>
- typeof val === "object" ? this.normalizeOptions(val) : val,
- );
- // remove null/undefined options from map.
- options = _.omit(options, value => value == null);
- return _.isEmpty(options) ? null : options;
- }
-
/**
* Parses an MBQL expression into an appropriate Dimension subclass, if possible.
* Metadata should be provided if you intend to use the display name or render methods.
@@ -219,7 +211,7 @@ export default class Dimension {
*/
_dimensionForOption(option: DimensionOption) {
// fill in the parent field ref
- const fieldRef = this.baseDimension().mbql();
+ const fieldRef = getBaseDimensionReference(this.mbql());
let mbql = option.mbql;
if (mbql) {
@@ -565,13 +557,6 @@ export default class Dimension {
return this.withoutOptions("temporal-unit");
}
- /**
- * Return a copy of this Dimension with any binning options removed.
- */
- withoutBinning(): Dimension {
- return this.withoutOptions("binning");
- }
-
withoutJoinAlias(): Dimension {
return this.withoutOptions("join-alias");
}
@@ -580,7 +565,7 @@ export default class Dimension {
* Return a copy of this Dimension with any temporal bucketing or binning options removed.
*/
baseDimension(): Dimension {
- return this.withoutTemporalBucketing().withoutBinning();
+ return this.withoutOptions(...BASE_DIMENSION_REFERENCE_OMIT_OPTIONS);
}
isValidFKRemappingTarget() {
@@ -675,21 +660,12 @@ export default class Dimension {
*/
export class FieldDimension extends Dimension {
- /**
- * Whether `clause` is an array, and a valid `:field` clause
- */
- static isFieldClause(clause): boolean {
- return (
- Array.isArray(clause) && clause.length === 3 && clause[0] === "field"
- );
- }
-
static parseMBQL(
mbql,
metadata = null,
query = null,
): FieldDimension | null | undefined {
- if (FieldDimension.isFieldClause(mbql)) {
+ if (isFieldReference(mbql)) {
return Object.freeze(
new FieldDimension(mbql[1], mbql[2], metadata, query),
);
@@ -742,7 +718,7 @@ export class FieldDimension extends Dimension {
[fieldIdOrName, options],
metadata,
query,
- Object.freeze(Dimension.normalizeOptions(options)),
+ Object.freeze(normalizeReferenceOptions(options)),
);
this._fieldIdOrName = fieldIdOrName;
@@ -764,7 +740,7 @@ export class FieldDimension extends Dimension {
}
// this should be considered equivalent to an equivalent MBQL clause
- if (FieldDimension.isFieldClause(somethingElse)) {
+ if (isFieldReference(somethingElse)) {
const dimension = FieldDimension.parseMBQL(
somethingElse,
this._metadata,
@@ -1142,21 +1118,12 @@ const isFieldDimension = dimension => dimension instanceof FieldDimension;
export class ExpressionDimension extends Dimension {
_expressionName: ExpressionName;
- /**
- * Whether `clause` is an array, and a valid `:expression` clause
- */
- static isExpressionClause(clause): boolean {
- return (
- Array.isArray(clause) && clause.length >= 2 && clause[0] === "expression"
- );
- }
-
static parseMBQL(
mbql: any,
metadata?: Metadata | null | undefined,
query?: StructuredQuery | null | undefined,
): Dimension | null | undefined {
- if (ExpressionDimension.isExpressionClause(mbql)) {
+ if (isExpressionReference(mbql)) {
const [expressionName, options] = mbql.slice(1);
return new ExpressionDimension(expressionName, options, metadata, query);
}
@@ -1174,7 +1141,7 @@ export class ExpressionDimension extends Dimension {
[expressionName, options],
metadata,
query,
- Object.freeze(Dimension.normalizeOptions(options)),
+ Object.freeze(normalizeReferenceOptions(options)),
);
this._expressionName = expressionName;
@@ -1195,7 +1162,7 @@ export class ExpressionDimension extends Dimension {
);
}
- if (ExpressionDimension.isExpressionClause(somethingElse)) {
+ if (isExpressionReference(somethingElse)) {
const dimension = ExpressionDimension.parseMBQL(
somethingElse,
this._metadata,
@@ -1411,7 +1378,7 @@ export class AggregationDimension extends Dimension {
metadata?: Metadata | null | undefined,
query?: StructuredQuery | null | undefined,
): Dimension | null | undefined {
- if (Array.isArray(mbql) && mbql[0] === "aggregation") {
+ if (isAggregationReference(mbql)) {
const [aggregationIndex, options] = mbql.slice(1);
return new AggregationDimension(
aggregationIndex,
@@ -1434,7 +1401,7 @@ export class AggregationDimension extends Dimension {
[aggregationIndex, options],
metadata,
query,
- Object.freeze(Dimension.normalizeOptions(options)),
+ Object.freeze(normalizeReferenceOptions(options)),
);
this._aggregationIndex = aggregationIndex;
@@ -1542,15 +1509,11 @@ export class TemplateTagDimension extends FieldDimension {
metadata: Metadata = null,
query: NativeQuery = null,
): FieldDimension | null | undefined {
- return TemplateTagDimension.isTemplateTagClause(mbql)
+ return isTemplateTagReference(mbql)
? Object.freeze(new TemplateTagDimension(mbql[1], metadata, query))
: null;
}
- static isTemplateTagClause(clause) {
- return Array.isArray(clause) && clause[0] === "template-tag";
- }
-
validateTemplateTag(): ValidationError | null {
const tag = this.tag();
if (!tag) {
diff --git a/frontend/src/metabase-lib/lib/Question.ts b/frontend/src/metabase-lib/lib/Question.ts
index b705be49f10..3ccfd722f20 100644
--- a/frontend/src/metabase-lib/lib/Question.ts
+++ b/frontend/src/metabase-lib/lib/Question.ts
@@ -22,8 +22,6 @@ import {
AggregationDimension,
FieldDimension,
} from "metabase-lib/lib/Dimension";
-import Mode from "metabase-lib/lib/Mode";
-import { isStandard } from "metabase-lib/lib/queries/utils/filter";
import { isFK } from "metabase-lib/lib/types/utils/isa";
import { memoizeClass, sortObject } from "metabase-lib/lib/utils";
@@ -81,6 +79,7 @@ import {
ALERT_TYPE_ROWS,
ALERT_TYPE_TIMESERIES_GOAL,
} from "metabase-lib/lib/Alert";
+import { getBaseDimensionReference } from "metabase-lib/lib/references";
export type QuestionCreatorOpts = {
databaseId?: DatabaseId;
@@ -713,7 +712,7 @@ class QuestionInner {
const dimension = query.columnDimensionWithName(name);
return {
name: name,
- field_ref: dimension.baseDimension().mbql(),
+ field_ref: getBaseDimensionReference(dimension.mbql()),
enabled: true,
};
}),
diff --git a/frontend/src/metabase-lib/lib/parameters/utils/mbql.js b/frontend/src/metabase-lib/lib/parameters/utils/mbql.js
index 63d8c7fc6ab..f8cd68de093 100644
--- a/frontend/src/metabase-lib/lib/parameters/utils/mbql.js
+++ b/frontend/src/metabase-lib/lib/parameters/utils/mbql.js
@@ -6,15 +6,13 @@ import {
EXCLUDE_OPTIONS,
EXCLUDE_UNITS,
} from "metabase-lib/lib/queries/utils/query-time";
-import Dimension, {
- FieldDimension,
- TemplateTagDimension,
-} from "metabase-lib/lib/Dimension";
+import Dimension, { FieldDimension } from "metabase-lib/lib/Dimension";
import { isDimensionTarget } from "metabase-lib/lib/parameters/utils/targets";
import {
getParameterSubType,
isDateParameter,
} from "metabase-lib/lib/parameters/utils/parameter-type";
+import { isTemplateTagReference } from "metabase-lib/lib/references";
import { getParameterOperatorName } from "metabase-lib/lib/parameters/utils/operators";
import { hasParameterValue } from "metabase-lib/lib/parameters/utils/parameter-values";
@@ -168,8 +166,7 @@ export function isFieldFilterParameterConveratableToMBQL(parameter) {
const hasValue = hasParameterValue(value);
const hasWellFormedTarget = Array.isArray(target?.[1]);
const hasFieldDimensionTarget =
- isDimensionTarget(target) &&
- !TemplateTagDimension.isTemplateTagClause(target[1]);
+ isDimensionTarget(target) && !isTemplateTagReference(target[1]);
return hasValue && hasWellFormedTarget && hasFieldDimensionTarget;
}
diff --git a/frontend/src/metabase-lib/lib/queries/NativeQuery.ts b/frontend/src/metabase-lib/lib/queries/NativeQuery.ts
index bcc24093b60..ac382b2898a 100644
--- a/frontend/src/metabase-lib/lib/queries/NativeQuery.ts
+++ b/frontend/src/metabase-lib/lib/queries/NativeQuery.ts
@@ -25,6 +25,7 @@ import Variable from "metabase-lib/lib/variables/Variable";
import TemplateTagVariable from "metabase-lib/lib/variables/TemplateTagVariable";
import { createTemplateTag } from "metabase-lib/lib/queries/TemplateTag";
import ValidationError from "metabase-lib/lib/ValidationError";
+import { isFieldReference } from "metabase-lib/lib/references";
import Dimension, { FieldDimension, TemplateTagDimension } from "../Dimension";
import DimensionOptions from "../DimensionOptions";
@@ -561,9 +562,7 @@ export default class NativeQuery extends AtomicQuery {
const templateTags = this.templateTags();
return templateTags
.filter(
- tag =>
- tag.type === "dimension" &&
- FieldDimension.isFieldClause(tag.dimension),
+ tag => tag.type === "dimension" && isFieldReference(tag.dimension),
)
.map(tag => {
const dimension = FieldDimension.parseMBQL(
diff --git a/frontend/src/metabase-lib/lib/queries/utils/dataset.js b/frontend/src/metabase-lib/lib/queries/utils/dataset.js
index 3cddda337b3..263663beabb 100644
--- a/frontend/src/metabase-lib/lib/queries/utils/dataset.js
+++ b/frontend/src/metabase-lib/lib/queries/utils/dataset.js
@@ -1,10 +1,7 @@
import _ from "underscore";
import StructuredQuery from "metabase-lib/lib/queries/StructuredQuery";
-import Dimension, {
- AggregationDimension,
- FieldDimension,
-} from "metabase-lib/lib/Dimension";
+import Dimension, { FieldDimension } from "metabase-lib/lib/Dimension";
export const datasetContainsNoResults = data =>
data.rows == null || data.rows.length === 0;
@@ -37,36 +34,6 @@ export function fieldRefWithOption(fieldRef, key, value) {
return dimension && dimension.withOption(key, value).mbql();
}
-export const keyForColumn = column => {
- let fieldRef = column.field_ref;
- if (!fieldRef) {
- console.error("column is missing field_ref", column);
- fieldRef = new FieldDimension(column.name).mbql();
- }
-
- let dimension = Dimension.parseMBQL(fieldRef);
- if (!dimension) {
- console.warn("Unknown field_ref", fieldRef);
- return JSON.stringify(fieldRef);
- }
-
- dimension = dimension.baseDimension();
-
- // match bug where field w/ join alias returned field w/o join alias instead
- if (dimension instanceof FieldDimension) {
- dimension = dimension.withoutOptions("join-alias");
- }
-
- // match legacy behavior which didn't have "field-literal" or "aggregation" field refs
- const isLegacyRef =
- (dimension instanceof FieldDimension && dimension.isStringFieldName()) ||
- dimension instanceof AggregationDimension;
-
- return JSON.stringify(
- isLegacyRef ? ["name", column.name] : ["ref", dimension.mbql()],
- );
-};
-
/**
* finds the column object from the dataset results for the given `table.columns` column setting
* @param {Column[]} columns Dataset results columns
diff --git a/frontend/src/metabase-lib/lib/queries/utils/get-column-key.ts b/frontend/src/metabase-lib/lib/queries/utils/get-column-key.ts
new file mode 100644
index 00000000000..23b0d25370d
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/queries/utils/get-column-key.ts
@@ -0,0 +1,50 @@
+import { DatasetColumn } from "metabase-types/api";
+import {
+ createFieldReference,
+ getBaseDimensionReference,
+ getDimensionReferenceWithoutOptions,
+ getNormalizedDimensionReference,
+ hasStringFieldName,
+ isAggregationReference,
+ isExpressionReference,
+ isFieldReference,
+ isValidDimensionReference,
+} from "metabase-lib/lib/references";
+
+export const getColumnKey = (
+ column: Pick<DatasetColumn, "name" | "field_ref">,
+) => {
+ let fieldRef = column.field_ref;
+
+ if (!fieldRef) {
+ console.error("column is missing field_ref", column);
+ 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);
+ }
+
+ if (isFieldReference(fieldRef)) {
+ fieldRef = getDimensionReferenceWithoutOptions(fieldRef, ["join-alias"]);
+ }
+
+ const isLegacyRef =
+ (isFieldReference(fieldRef) && hasStringFieldName(fieldRef)) ||
+ isAggregationReference(fieldRef);
+
+ return JSON.stringify(
+ isLegacyRef ? ["name", column.name] : ["ref", fieldRef],
+ );
+};
diff --git a/frontend/src/metabase-lib/lib/queries/utils/get-column-key.unit.spec.js b/frontend/src/metabase-lib/lib/queries/utils/get-column-key.unit.spec.js
new file mode 100644
index 00000000000..94a60f8e5f3
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/queries/utils/get-column-key.unit.spec.js
@@ -0,0 +1,91 @@
+import { getColumnKey } from "metabase-lib/lib/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", null]]));
+ });
+ 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", () => {
+ 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(
+ // NOTE: not ideal, matches existing behavior, but should be ["field", 1, {"join-alias": "x"}]
+ JSON.stringify(["ref", ["field", 1, null]]),
+ );
+ });
+ });
+});
diff --git a/frontend/src/metabase-lib/lib/references.ts b/frontend/src/metabase-lib/lib/references.ts
new file mode 100644
index 00000000000..bdd7a4cc122
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/references.ts
@@ -0,0 +1,124 @@
+import _ from "underscore";
+import {
+ AggregationReference,
+ ColumnName,
+ DimensionReference,
+ DimensionReferenceWithOptions,
+ ExpressionReference,
+ FieldId,
+ FieldReference,
+ ReferenceOptions,
+ TemplateTagReference,
+} from "metabase-types/api/query";
+
+export const isFieldReference = (mbql: any): mbql is FieldReference => {
+ return Array.isArray(mbql) && mbql.length === 3 && mbql[0] === "field";
+};
+
+export const isExpressionReference = (
+ mbql: any,
+): mbql is ExpressionReference => {
+ return Array.isArray(mbql) && mbql.length >= 2 && mbql[0] === "expression";
+};
+
+export const isAggregationReference = (
+ mbql: any,
+): mbql is AggregationReference => {
+ return Array.isArray(mbql) && mbql[0] === "aggregation";
+};
+
+export const isTemplateTagReference = (
+ mbql: any,
+): mbql is TemplateTagReference => {
+ return Array.isArray(mbql) && mbql[0] === "template-tag";
+};
+
+export const createFieldReference = (
+ columnNameOrFieldId: ColumnName | FieldId,
+): FieldReference => ["field", columnNameOrFieldId, null];
+
+export const isValidDimensionReference = (
+ mbql: any,
+): mbql is DimensionReference => {
+ return [
+ isFieldReference,
+ isExpressionReference,
+ isAggregationReference,
+ isTemplateTagReference,
+ ].some(predicate => predicate(mbql));
+};
+
+export const normalizeReferenceOptions = (
+ options?: ReferenceOptions | null,
+): ReferenceOptions | null => {
+ if (!options) {
+ return null;
+ }
+
+ // recursively normalize maps inside options.
+ options = _.mapObject(options, val =>
+ typeof val === "object" ? normalizeReferenceOptions(val) : val,
+ );
+ // remove null/undefined options from map.
+ options = _.omit(options, value => value == null);
+ return _.isEmpty(options) ? null : options;
+};
+
+export const getNormalizedDimensionReference = (
+ mbql: DimensionReference,
+): DimensionReference => {
+ if (
+ isFieldReference(mbql) ||
+ isExpressionReference(mbql) ||
+ isAggregationReference(mbql)
+ ) {
+ const normalizedReference = [...mbql] as DimensionReference;
+ const normalizedOptions = normalizeReferenceOptions(mbql[2]);
+ normalizedReference[2] = normalizedOptions;
+
+ return normalizedReference;
+ }
+
+ return mbql;
+};
+
+export const getDimensionReferenceWithoutOptions = (
+ mbql: DimensionReferenceWithOptions,
+ optionsKeysToOmit: string[],
+): DimensionReferenceWithOptions => {
+ const newReference = mbql.slice() as DimensionReferenceWithOptions;
+ const options = newReference[2];
+
+ if (!options) {
+ return newReference;
+ }
+
+ newReference[2] =
+ options == null ? null : _.omit(options, ...optionsKeysToOmit);
+
+ if (_.isEmpty(newReference[2])) {
+ newReference[2] = null;
+ }
+
+ return newReference;
+};
+
+export const BASE_DIMENSION_REFERENCE_OMIT_OPTIONS = [
+ "temporal-unit",
+ "binning",
+];
+
+export const getBaseDimensionReference = (
+ mbql: DimensionReferenceWithOptions,
+) =>
+ getDimensionReferenceWithoutOptions(
+ mbql,
+ BASE_DIMENSION_REFERENCE_OMIT_OPTIONS,
+ );
+
+/**
+ * Whether this Field clause has a string Field name (as opposed to an integer Field ID). This generally means the
+ * Field comes from a native query.
+ */
+export const hasStringFieldName = (mbql: FieldReference) =>
+ typeof mbql[1] === "string";
diff --git a/frontend/src/metabase-lib/lib/references.unit.spec.ts b/frontend/src/metabase-lib/lib/references.unit.spec.ts
new file mode 100644
index 00000000000..e70e5773be0
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/references.unit.spec.ts
@@ -0,0 +1,131 @@
+import {
+ isFieldReference,
+ isExpressionReference,
+ isAggregationReference,
+ isTemplateTagReference,
+ normalizeReferenceOptions,
+} from "metabase-lib/lib/references";
+
+describe("reference predicates", () => {
+ describe("isFieldReference", () => {
+ it("returns true for valid field references", () => {
+ const validReferences = [
+ ["field", 123, null],
+ ["field", "column_name", null],
+ ["field", "column_name", {}],
+ ];
+
+ validReferences.forEach(ref => expect(isFieldReference(ref)).toBe(true));
+ });
+
+ it("returns false for invalid field references", () => {
+ const invalidReferences = [
+ null,
+ "field",
+ ["field", 123],
+ ["aggregation", 123, null],
+ ["field", "column_name", null, { "source-field": true }],
+ ];
+
+ invalidReferences.forEach(ref =>
+ expect(isFieldReference(ref)).toBe(false),
+ );
+ });
+ });
+
+ describe("isAggregationReference", () => {
+ it("returns true for valid aggregation references", () => {
+ const validReferences = [
+ ["aggregation", 123, null],
+ ["aggregation", "string", null],
+ ["aggregation", "column_name", { "base-type": "number" }],
+ ];
+
+ validReferences.forEach(ref =>
+ expect(isAggregationReference(ref)).toBe(true),
+ );
+ });
+
+ it("returns false for invalid aggregation references", () => {
+ const invalidReferences = [null, 123, "aggregation", ["field", 123]];
+
+ invalidReferences.forEach(ref =>
+ expect(isAggregationReference(ref)).toBe(false),
+ );
+ });
+ });
+
+ describe("isExpressionReference", () => {
+ it("returns true for valid expression references", () => {
+ const validReferences = [
+ ["expression", "string", null],
+ ["expression", "column_name", { "base-type": "number" }],
+ ];
+
+ validReferences.forEach(ref =>
+ expect(isExpressionReference(ref)).toBe(true),
+ );
+ });
+
+ it("returns false for invalid expression references", () => {
+ const invalidReferences = [
+ null,
+ 123,
+ "expression",
+ ["expression"],
+ ["field", 123],
+ ];
+
+ invalidReferences.forEach(ref =>
+ expect(isExpressionReference(ref)).toBe(false),
+ );
+ });
+ });
+
+ describe("isTemplateTagReference", () => {
+ it("returns true for valid template tag references", () => {
+ const validReferences = [["template-tag", "tag name"]];
+
+ validReferences.forEach(ref =>
+ expect(isTemplateTagReference(ref)).toBe(true),
+ );
+ });
+
+ it("returns false for invalid template tag references", () => {
+ const invalidReferences = [null, "tag", ["templateTag", 123]];
+
+ invalidReferences.forEach(ref =>
+ expect(isTemplateTagReference(ref)).toBe(false),
+ );
+ });
+ });
+});
+
+describe("normalizeReferenceOptions", () => {
+ it("should remove empty options map", () => {
+ expect(normalizeReferenceOptions(null)).toEqual(null);
+ expect(normalizeReferenceOptions({})).toEqual(null);
+ });
+ it("should remove null/undefined keys", () => {
+ expect(
+ normalizeReferenceOptions({
+ "temporal-unit": undefined,
+ binning: { strategy: "default" },
+ "base-type": undefined,
+ }),
+ ).toEqual({
+ binning: { strategy: "default" },
+ });
+ });
+ it("should recursively normalize maps options", () => {
+ expect(
+ normalizeReferenceOptions({
+ binning: { strategy: "default", other: null } as any,
+ }),
+ ).toStrictEqual({
+ binning: { strategy: "default" },
+ });
+ });
+ // TODO -- it should also remove empty arrays, but we currently don't have any situations where there might be
+ // one.
+});
diff --git a/frontend/src/metabase-types/api/dataset.ts b/frontend/src/metabase-types/api/dataset.ts
index 7db0792e60b..3f2ab32a6d4 100644
--- a/frontend/src/metabase-types/api/dataset.ts
+++ b/frontend/src/metabase-types/api/dataset.ts
@@ -1,4 +1,7 @@
-import type { DatetimeUnit } from "metabase-types/api/query";
+import type {
+ DatetimeUnit,
+ DimensionReference,
+} from "metabase-types/api/query";
import { DatabaseId } from "./database";
import { DownloadPermission } from "./permissions";
@@ -6,11 +9,14 @@ export type RowValue = string | number | null | boolean;
export type RowValues = RowValue[];
export interface DatasetColumn {
+ id?: number;
display_name: string;
source: string;
name: string;
remapped_to_column?: DatasetColumn;
unit?: DatetimeUnit;
+ field_ref?: DimensionReference;
+ expression_name?: any;
}
export interface DatasetData {
diff --git a/frontend/src/metabase-types/api/query.ts b/frontend/src/metabase-types/api/query.ts
index 45ec40e6ac0..0b3007fb365 100644
--- a/frontend/src/metabase-types/api/query.ts
+++ b/frontend/src/metabase-types/api/query.ts
@@ -40,3 +40,70 @@ export type DatetimeUnit =
| "quarter"
| "quarter-of-year"
| "year";
+
+export interface ReferenceOptions {
+ binning?: BinningOptions;
+ "temporal-unit"?: DatetimeUnit;
+ "join-alias"?: string;
+ "base-type"?: string;
+}
+
+type BinningOptions =
+ | DefaultBinningOptions
+ | NumBinsBinningOptions
+ | BinWidthBinningOptions;
+
+interface DefaultBinningOptions {
+ strategy: "default";
+}
+
+interface NumBinsBinningOptions {
+ strategy: "num-bins";
+ "num-bins": number;
+}
+
+interface BinWidthBinningOptions {
+ strategy: "bin-width";
+ "bin-width": number;
+}
+
+export type ReferenceOptionsKeys =
+ | "source-field"
+ | "base-type"
+ | "join-alias"
+ | "temporal-unit"
+ | "binning";
+
+export type FieldId = number;
+export type ColumnName = string;
+export type FieldReference = [
+ "field",
+ FieldId | ColumnName,
+ ReferenceOptions | null,
+];
+
+export type ExpressionName = string;
+export type ExpressionReference = [
+ "expression",
+ ExpressionName,
+ ReferenceOptions | null,
+];
+
+export type AggregationIndex = number;
+export type AggregationReference = [
+ "aggregation",
+ AggregationIndex,
+ ReferenceOptions | null,
+];
+
+export type TagName = string;
+export type TemplateTagReference = ["template-tag", TagName];
+
+export type DimensionReferenceWithOptions =
+ | FieldReference
+ | ExpressionReference
+ | AggregationReference;
+
+export type DimensionReference =
+ | DimensionReferenceWithOptions
+ | TemplateTagReference;
diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebar.tsx b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebar.tsx
index a81834c7f36..1b091299b20 100644
--- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebar.tsx
+++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebar.tsx
@@ -16,12 +16,12 @@ import type {
CardId,
ClickBehavior,
DatasetData,
+ DatasetColumn,
} from "metabase-types/api";
-import type { Column } from "metabase-types/types/Dataset";
import { isTableDisplay } from "metabase/lib/click-behavior";
import { clickBehaviorIsValid } from "metabase-lib/lib/parameters/utils/click-behavior";
-import { keyForColumn } from "metabase-lib/lib/queries/utils/dataset";
+import { getColumnKey } from "metabase-lib/lib/queries/utils/get-column-key";
import { getClickBehaviorForColumn } from "./utils";
import ClickBehaviorSidebarContent from "./ClickBehaviorSidebarContent";
import ClickBehaviorSidebarHeader from "./ClickBehaviorSidebarHeader";
@@ -67,7 +67,9 @@ function ClickBehaviorSidebar({
boolean | null
>(null);
- const [selectedColumn, setSelectedColumn] = useState<Column | null>(null);
+ const [selectedColumn, setSelectedColumn] = useState<DatasetColumn | null>(
+ null,
+ );
const [originalVizSettings, setOriginalVizSettings] = useState<
VizSettings | undefined | null
@@ -105,7 +107,7 @@ function ClickBehaviorSidebar({
click_behavior: nextClickBehavior,
});
} else {
- onUpdateDashCardColumnSettings(id, keyForColumn(selectedColumn), {
+ onUpdateDashCardColumnSettings(id, getColumnKey(selectedColumn), {
click_behavior: nextClickBehavior,
});
}
diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarContent.tsx b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarContent.tsx
index b1c835b72a0..1b195761137 100644
--- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarContent.tsx
+++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarContent.tsx
@@ -11,8 +11,8 @@ import type {
CardId,
ClickBehavior,
DatasetData,
+ DatasetColumn,
} from "metabase-types/api";
-import type { Column } from "metabase-types/types/Dataset";
import { isTableDisplay } from "metabase/lib/click-behavior";
import { getClickBehaviorForColumn } from "./utils";
@@ -29,7 +29,7 @@ interface Props {
clickBehavior?: ClickBehavior;
isTypeSelectorVisible: boolean | null;
hasSelectedColumn: boolean;
- onColumnSelected: (column: Column) => void;
+ onColumnSelected: (column: DatasetColumn) => void;
onSettingsChange: (clickBehavior?: Partial<ClickBehavior>) => void;
onTypeSelectorVisibilityChange: (isVisible: boolean) => void;
}
@@ -62,7 +62,7 @@ function ClickBehaviorSidebar({
<TableClickBehaviorView
columns={columns}
dashcard={dashcard}
- getClickBehaviorForColumn={(column: Column) =>
+ getClickBehaviorForColumn={(column: DatasetColumn) =>
getClickBehaviorForColumn(dashcard, column)
}
onColumnClick={onColumnSelected}
diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarHeader/ClickBehaviorSidebarHeader.tsx b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarHeader/ClickBehaviorSidebarHeader.tsx
index 7947f01d54c..30681a9150a 100644
--- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarHeader/ClickBehaviorSidebarHeader.tsx
+++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarHeader/ClickBehaviorSidebarHeader.tsx
@@ -8,8 +8,7 @@ import {
getActionButtonLabel,
} from "metabase/writeback/utils";
-import type { DashboardOrderedCard } from "metabase-types/api";
-import type { Column } from "metabase-types/types/Dataset";
+import type { DashboardOrderedCard, DatasetColumn } from "metabase-types/api";
import { isTableDisplay } from "metabase/lib/click-behavior";
import { Heading, SidebarHeader } from "../ClickBehaviorSidebar.styled";
@@ -29,7 +28,7 @@ function DefaultHeader({ children }: { children: React.ReactNode }) {
interface Props {
dashcard: DashboardOrderedCard;
- selectedColumn?: Column | null;
+ selectedColumn?: DatasetColumn | null;
onUnsetColumn: () => void;
}
diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/TableClickBehaviorView/TableClickBehaviorView.tsx b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/TableClickBehaviorView/TableClickBehaviorView.tsx
index 1a09ce97cd9..bdac0d2cd6a 100644
--- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/TableClickBehaviorView/TableClickBehaviorView.tsx
+++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/TableClickBehaviorView/TableClickBehaviorView.tsx
@@ -6,8 +6,8 @@ import type {
DashboardOrderedCard,
ClickBehavior,
ClickBehaviorType,
+ DatasetColumn,
} from "metabase-types/api";
-import type { Column as IColumn } from "metabase-types/types/Dataset";
import { hasActionsMenu } from "metabase/lib/click-behavior";
import Column from "./Column";
@@ -33,10 +33,12 @@ function explainClickBehaviorType(
}
interface Props {
- columns: IColumn[];
+ columns: DatasetColumn[];
dashcard: DashboardOrderedCard;
- getClickBehaviorForColumn: (column: IColumn) => ClickBehavior | undefined;
- onColumnClick: (column: IColumn) => void;
+ getClickBehaviorForColumn: (
+ column: DatasetColumn,
+ ) => ClickBehavior | undefined;
+ onColumnClick: (column: DatasetColumn) => void;
}
function TableClickBehaviorView({
diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/utils.ts b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/utils.ts
index 9a35adba1bf..ce67d613588 100644
--- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/utils.ts
+++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/utils.ts
@@ -4,10 +4,10 @@ import { getIn } from "icepick";
import type {
ClickBehaviorType,
DashboardOrderedCard,
+ DatasetColumn,
} from "metabase-types/api";
-import type { Column } from "metabase-types/types/Dataset";
import { hasActionsMenu } from "metabase/lib/click-behavior";
-import { keyForColumn } from "metabase-lib/lib/queries/utils/dataset";
+import { getColumnKey } from "metabase-lib/lib/queries/utils/get-column-key";
type ClickBehaviorOption = {
value: ClickBehaviorType | "menu";
@@ -42,12 +42,12 @@ export function getClickBehaviorOptionName(
}
export function getClickBehaviorForColumn(
dashcard: DashboardOrderedCard,
- column: Column,
+ column: DatasetColumn,
) {
return getIn(dashcard, [
"visualization_settings",
"column_settings",
- keyForColumn(column),
+ getColumnKey(column),
"click_behavior",
]);
}
diff --git a/frontend/src/metabase/modes/components/drill/FormatAction.jsx b/frontend/src/metabase/modes/components/drill/FormatAction.jsx
index d59912ff870..34ae41233db 100644
--- a/frontend/src/metabase/modes/components/drill/FormatAction.jsx
+++ b/frontend/src/metabase/modes/components/drill/FormatAction.jsx
@@ -7,7 +7,7 @@ import { t } from "ttag";
import { getSettingsWidgetsForSeries } from "metabase/visualizations/lib/settings/visualization";
import ChartSettingsWidget from "metabase/visualizations/components/ChartSettingsWidget";
import { updateSettings } from "metabase/visualizations/lib/settings";
-import { keyForColumn } from "metabase-lib/lib/queries/utils/dataset";
+import { getColumnKey } from "metabase-lib/lib/queries/utils/get-column-key";
export default ({ question, clicked }) => {
if (
@@ -56,7 +56,7 @@ export default ({ question, clicked }) => {
...columnSettingsWidget,
props: {
...columnSettingsWidget.props,
- initialKey: keyForColumn(column),
+ initialKey: getColumnKey(column),
},
}}
hidden={false}
diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx
index c599259e549..5ab0da8a310 100644
--- a/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx
+++ b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx
@@ -13,7 +13,7 @@ import {
isFuzzyOperator,
} from "metabase-lib/lib/operators/utils";
import { isCurrency } from "metabase-lib/lib/types/utils/isa";
-import { keyForColumn } from "metabase-lib/lib/queries/utils/dataset";
+import { getColumnKey } from "metabase-lib/lib/queries/utils/get-column-key";
import TextPicker from "./TextPicker";
import SelectPicker from "./SelectPicker";
import NumberPicker from "./NumberPicker";
@@ -66,7 +66,7 @@ export default function DefaultPicker({
const visualizationSettings = filter?.query()?.question()?.settings();
- const key = keyForColumn(dimension.column());
+ const key = getColumnKey(dimension.column());
const columnSettings = visualizationSettings?.column_settings?.[key];
const fieldMetadata = field?.metadata?.fields[field?.id];
diff --git a/frontend/src/metabase/static-viz/components/RowChart/utils/format.ts b/frontend/src/metabase/static-viz/components/RowChart/utils/format.ts
index 0c97a879f35..b7a5ec72f8b 100644
--- a/frontend/src/metabase/static-viz/components/RowChart/utils/format.ts
+++ b/frontend/src/metabase/static-viz/components/RowChart/utils/format.ts
@@ -3,6 +3,7 @@ import { ChartColumns } from "metabase/visualizations/lib/graph/columns";
import { getStackOffset } from "metabase/visualizations/lib/settings/stacking";
import { formatNumber, formatPercent } from "metabase/static-viz/lib/numbers";
import { ChartTicksFormatters } from "metabase/visualizations/shared/types/format";
+import { getColumnKey } from "metabase-lib/lib/queries/utils/get-column-key";
export const getXValueMetricColumn = (chartColumns: ChartColumns) => {
// For multi-metrics charts we use the first metic column settings for formatting
@@ -21,7 +22,9 @@ export const getStaticFormatters = (
};
const metricColumnSettings =
- settings.column_settings?.[getXValueMetricColumn(chartColumns).column.name];
+ settings.column_settings?.[
+ getColumnKey(getXValueMetricColumn(chartColumns).column)
+ ];
const xTickFormatter = (value: any) =>
formatNumber(value, metricColumnSettings);
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx
index 2729e06ad6a..c6f2f984e79 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx
@@ -2,7 +2,7 @@
import React from "react";
import { t } from "ttag";
import _ from "underscore";
-import { keyForColumn } from "metabase-lib/lib/queries/utils/dataset";
+import { getColumnKey } from "metabase-lib/lib/queries/utils/get-column-key";
import ChartSettingSelect from "./ChartSettingSelect";
import {
SettingsIcon,
@@ -25,7 +25,7 @@ const ChartSettingFieldPicker = ({
if (value && showColumnSetting && columns) {
const column = _.findWhere(columns, { name: value });
if (column && columnHasSettings(column)) {
- columnKey = keyForColumn(column);
+ columnKey = getColumnKey(column);
}
}
return (
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldsPartition.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldsPartition.jsx
index 8cd5ee55aa1..b33fa9d2b85 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldsPartition.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldsPartition.jsx
@@ -8,7 +8,7 @@ import { splice } from "icepick";
import Label from "metabase/components/type/Label";
-import { keyForColumn } from "metabase-lib/lib/queries/utils/dataset";
+import { getColumnKey } from "metabase-lib/lib/queries/utils/get-column-key";
import {
DroppableContainer,
FieldPartitionColumn,
@@ -40,7 +40,7 @@ class ChartSettingFieldsPartition extends React.Component {
{
id: "column_settings",
props: {
- initialKey: keyForColumn(column),
+ initialKey: getColumnKey(column),
},
},
targetElement,
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingOrderedColumns.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingOrderedColumns.jsx
index 6526cde281f..f3750bfe9b8 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingOrderedColumns.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingOrderedColumns.jsx
@@ -4,10 +4,8 @@ import { t } from "ttag";
import _ from "underscore";
import { getFriendlyName } from "metabase/visualizations/lib/utils";
-import {
- keyForColumn,
- findColumnForColumnSetting,
-} from "metabase-lib/lib/queries/utils/dataset";
+import { getColumnKey } from "metabase-lib/lib/queries/utils/get-column-key";
+import { findColumnForColumnSetting } from "metabase-lib/lib/queries/utils/dataset";
import StructuredQuery from "metabase-lib/lib/queries/StructuredQuery";
import ColumnItem from "./ColumnItem";
@@ -45,7 +43,7 @@ export default class ChartSettingOrderedColumns extends Component {
{
id: "column_settings",
props: {
- initialKey: keyForColumn(column),
+ initialKey: getColumnKey(column),
},
},
targetElement,
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingsListColumns/ChartSettingsListColumns.tsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingsListColumns/ChartSettingsListColumns.tsx
index 18a64ff2a2b..3c807d92c8d 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingsListColumns/ChartSettingsListColumns.tsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingsListColumns/ChartSettingsListColumns.tsx
@@ -4,10 +4,10 @@ import _ from "underscore";
import Button from "metabase/core/components/Button";
-import { Column } from "metabase-types/types/Dataset";
import { FieldId } from "metabase-types/types/Field";
import { ConcreteField } from "metabase-types/types/Query";
-import { keyForColumn } from "metabase-lib/lib/queries/utils/dataset";
+import { DatasetColumn } from "metabase-types/api";
+import { getColumnKey } from "metabase-lib/lib/queries/utils/get-column-key";
import Question from "metabase-lib/lib/Question";
import {
@@ -25,7 +25,7 @@ type SettingValue = {
interface Props {
value: SettingValue;
- columns: Column[];
+ columns: DatasetColumn[];
question: Question;
onChange: (value: SettingValue) => void;
onShowWidget: (config: unknown, targetElement: HTMLElement | null) => void;
@@ -78,7 +78,7 @@ function ChartSettingsListColumns({
{
id: "column_settings",
props: {
- initialKey: keyForColumn(column),
+ initialKey: getColumnKey(column),
},
},
targetElement,
diff --git a/frontend/src/metabase/visualizations/lib/settings/column.js b/frontend/src/metabase/visualizations/lib/settings/column.js
index d863644152c..52af23bb00e 100644
--- a/frontend/src/metabase/visualizations/lib/settings/column.js
+++ b/frontend/src/metabase/visualizations/lib/settings/column.js
@@ -33,7 +33,7 @@ export function columnSettings({
section: t`Formatting`,
objectName: "column",
getObjects: getColumns,
- getObjectKey: keyForColumn,
+ getObjectKey: getColumnKey,
getSettingDefintionsForObject: getSettingDefintionsForColumn,
component: ChartNestedSettingColumns,
getInheritedSettingsForObject: getInhertiedSettingsForColumn,
@@ -50,7 +50,7 @@ import {
isCurrency,
isDateWithoutTime,
} from "metabase-lib/lib/types/utils/isa";
-import { keyForColumn } from "metabase-lib/lib/queries/utils/dataset";
+import { getColumnKey } from "metabase-lib/lib/queries/utils/get-column-key";
import { nestedSettings } from "./nested";
export function getGlobalSettingsForColumn(column) {
diff --git a/frontend/test/metabase-lib/lib/Dimension.unit.spec.js b/frontend/test/metabase-lib/lib/Dimension.unit.spec.js
index 88272fd527a..ad61e28f810 100644
--- a/frontend/test/metabase-lib/lib/Dimension.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/Dimension.unit.spec.js
@@ -213,31 +213,6 @@ describe("Dimension", () => {
metadata,
);
- describe("STATIC METHODS", () => {
- describe("normalizeOptions()", () => {
- it("should remove empty options map", () => {
- expect(FieldDimension.normalizeOptions(null)).toEqual(null);
- expect(FieldDimension.normalizeOptions({})).toEqual(null);
- });
- it("should remove null/undefined keys", () => {
- expect(
- FieldDimension.normalizeOptions({
- x: false,
- y: null,
- z: undefined,
- }),
- ).toEqual({ x: false });
- });
- it("should recursively normalize maps options", () => {
- expect(
- FieldDimension.normalizeOptions({ binning: { x: null } }),
- ).toBe(null);
- });
- // TODO -- it should also remove empty arrays, but we currently don't have any situations where there might be
- // one.
- });
- });
-
describe("INSTANCE METHODS", () => {
describe("mbql()", () => {
it(
@@ -940,24 +915,6 @@ describe("Dimension", () => {
).toBeNull();
});
});
-
- describe("isTemplateTagClause", () => {
- it("returns false for a field clause", () => {
- expect(
- TemplateTagDimension.isTemplateTagClause(["field", 123, null]),
- ).toBe(false);
- });
-
- it("returns false for a non-array clause", () => {
- expect(TemplateTagDimension.isTemplateTagClause("foo")).toBe(false);
- });
-
- it("returns true for a template tag clause", () => {
- expect(
- TemplateTagDimension.isTemplateTagClause(templateTagClause),
- ).toBe(true);
- });
- });
});
describe("INSTANCE METHODS", () => {
@@ -1095,24 +1052,6 @@ describe("Dimension", () => {
).toBeNull();
});
});
-
- describe("isTemplateTagClause", () => {
- it("returns false for a field clause", () => {
- expect(
- TemplateTagDimension.isTemplateTagClause(["field", 123, null]),
- ).toBe(false);
- });
-
- it("returns false for a non-array clause", () => {
- expect(TemplateTagDimension.isTemplateTagClause("foo")).toBe(false);
- });
-
- it("returns true for a template tag clause", () => {
- expect(
- TemplateTagDimension.isTemplateTagClause(templateTagClause),
- ).toBe(true);
- });
- });
});
describe("INSTANCE METHODS", () => {
diff --git a/frontend/test/metabase/lib/dataset.unit.spec.js b/frontend/test/metabase/lib/dataset.unit.spec.js
index 5573c0646e4..06235baf8e3 100644
--- a/frontend/test/metabase/lib/dataset.unit.spec.js
+++ b/frontend/test/metabase/lib/dataset.unit.spec.js
@@ -3,7 +3,6 @@ import {
fieldRefForColumn,
syncTableColumnsToQuery,
findColumnForColumnSetting,
- keyForColumn,
} from "metabase-lib/lib/queries/utils/dataset";
describe("metabase/util/dataset", () => {
@@ -177,96 +176,4 @@ describe("metabase/util/dataset", () => {
expect(column).toBe(columns[1]);
});
});
-
- describe("keyForColumn", () => {
- // 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(
- keyForColumn({
- 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(
- keyForColumn({
- 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(
- keyForColumn({
- name: "foo",
- expression_name: "foo",
- field_ref: fieldRefEnabled ? ["expression", "foo"] : undefined,
- }),
- ).toEqual(JSON.stringify(["ref", ["expression", "foo", null]]));
- });
- it("should return [name ...] for aggregation", () => {
- const col = {
- name: "foo",
- source: "aggregation",
- field_ref: fieldRefEnabled ? ["aggregation", 0] : undefined,
- };
- expect(keyForColumn(col, [col])).toEqual(
- // NOTE: not ideal, matches existing behavior, but should be ["aggregation", 0]
- JSON.stringify(["name", "foo"]),
- );
- });
- it("should return [name ...] for aggregation", () => {
- const col = {
- name: "foo",
- id: ["field", "foo", { "base-type": "type/Integer" }],
- field_ref: fieldRefEnabled
- ? ["field", "foo", { "base-type": "type/Integer" }]
- : undefined,
- };
- expect(keyForColumn(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(
- keyForColumn({
- 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(keyForColumn(col)).toEqual(
- // NOTE: not ideal, matches existing behavior, but should be ["field", 1, {"join-alias": "x"}]
- JSON.stringify(["ref", ["field", 1, null]]),
- );
- });
- });
- });
});
--
GitLab