diff --git a/frontend/src/metabase-lib/lib/Dimension.ts b/frontend/src/metabase-lib/lib/Dimension.ts
index 06948bdb717d9749095da9ac9d92aec339fbf06a..c8165cc62f465486f41ef50b91183b5a9df40d1c 100644
--- a/frontend/src/metabase-lib/lib/Dimension.ts
+++ b/frontend/src/metabase-lib/lib/Dimension.ts
@@ -2,7 +2,6 @@
// @ts-nocheck
import { t, ngettext, msgid } from "ttag";
import _ from "underscore";
-import { merge } from "icepick";
import { stripId, FK_SYMBOL } from "metabase/lib/formatting";
import { TYPE } from "metabase/lib/types";
import { TemplateTagVariable } from "metabase-lib/lib/Variable";
@@ -27,7 +26,6 @@ import ValidationError, {
import { IconName } from "metabase-types/types";
import { getFieldValues, getRemappings } from "metabase/lib/query/field";
import { DATETIME_UNITS, formatBucketing } from "metabase/lib/query_time";
-import { getQuestionIdFromVirtualTableId } from "metabase/lib/saved-questions";
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";
@@ -796,36 +794,13 @@ export class FieldDimension extends Dimension {
return typeof this._fieldIdOrName === "string";
}
- _createField(fieldInfo, { hydrate = false } = {}): Field {
+ _createField(fieldInfo): Field {
const field = new Field({
...fieldInfo,
metadata: this._metadata,
query: this._query,
});
- // This is normally done when calculating metadata,
- // but since we're merging plain objects without these fields,
- // we need to repeat the hydration again.
- // We should definitely move it out of there
- if (hydrate) {
- field.table =
- this.query()?.table() ?? this._metadata.table(field.table_id);
- field.table_id = field.table?.id;
-
- if (field.isFK()) {
- field.target = this._metadata.field(field.fk_target_field_id);
- }
-
- if (field.name_field != null) {
- field.field_name = this._metadata.field(field.name_field);
- } else if (field.table && field.isPK()) {
- field.field_name = _.find(field.table.fields, f => f.isEntityName());
- }
-
- field.values = getFieldValues(field);
- field.remappings = new Map(getRemappings(field));
- }
-
return field;
}
@@ -842,120 +817,55 @@ export class FieldDimension extends Dimension {
}
}
- _getFieldMetadataFromSavedDataset() {
- const identifierProp = this._getIdentifierProp();
- const questionAssociatedWithDimension = this.query()?.question();
- if (
- questionAssociatedWithDimension?.isSaved() &&
- questionAssociatedWithDimension.isDataset()
- ) {
- const fieldMetadata = _.findWhere(
- questionAssociatedWithDimension.getResultMetadata(),
- {
- [identifierProp]: this.fieldIdOrName(),
- },
- );
-
- return fieldMetadata;
- }
- }
-
- _getFieldMetadataFromNestedDataset() {
+ _findMatchingQueryField() {
const identifierProp = this._getIdentifierProp();
- const virtualTableCardId = getQuestionIdFromVirtualTableId(
- this.query()?.sourceTableId?.(),
- );
- if (virtualTableCardId != null) {
- const question = this._metadata?.question(virtualTableCardId);
- const fieldMetadata = question?.isDataset()
- ? _.findWhere(question.getResultMetadata(), {
- [identifierProp]: this.fieldIdOrName(),
- })
- : undefined;
-
- return fieldMetadata;
+ const fieldIdentifier = this.fieldIdOrName();
+ if (this._query) {
+ const queryTableFields = this._query.table()?.fields;
+ return _.findWhere(queryTableFields, {
+ [identifierProp]: fieldIdentifier,
+ });
}
}
- _getFieldMetadataFromQueryTable() {
- const identifierProp = this._getIdentifierProp();
- const table = this.query()?.table();
- return _.findWhere(table?.fields, {
- [identifierProp]: this.fieldIdOrName(),
+ _createFallbackField(): Field {
+ return this._createField({
+ id: this.isIntegerFieldId() ? this.fieldIdOrName() : this.mbql(),
+ field_ref: this.mbql(),
+ name: this.isStringFieldName() && this.fieldIdOrName(),
+ display_name: this.fieldIdOrName(),
+ base_type: this.getOption("base-type"),
});
}
- _combineFieldWithExtraMetadata(field: Field | undefined, fieldMetadata) {
- if (field) {
- if (!fieldMetadata || field === fieldMetadata) {
- return field;
- }
-
- const fieldObject = merge(
- field instanceof Field ? field.getPlainObject() : field,
- fieldMetadata instanceof Field
- ? fieldMetadata.getPlainObject()
- : fieldMetadata,
- );
- return this._createField(fieldObject, { hydrate: true });
- }
-
- if (fieldMetadata) {
- if (fieldMetadata instanceof Field) {
- return fieldMetadata;
- }
-
- return this._createField(fieldMetadata);
- }
- }
-
field(): Field {
// If a Field is cached on the FieldDimension instance, we can shortwire this method and
// return the cached Field.
- const cachedField = this._getTrustedFieldCachedOnInstance();
- if (cachedField) {
- return cachedField;
- }
-
- let fieldMetadata;
-
- // Models contain custom metadata for fields, but when these fields have integer ids,
- // they are clobbered by the metadata from the real table that the card is based on,
- // so we need to check the result_metadata of the nested card and merge it with the
- // Field that exists in the Metadata object.
- if (this.isIntegerFieldId()) {
- fieldMetadata =
- this._getFieldMetadataFromSavedDataset() ||
- this._getFieldMetadataFromNestedDataset();
- }
-
- // In scenarios where the Field id is not an integer, we need to grab the Field from the
- // query's table, because the Field won't exist in the Metadata object (and string Field ids
- // are not sufficiently unique to be stored properly in the Metadata object).
- fieldMetadata = fieldMetadata || this._getFieldMetadataFromQueryTable();
-
- // The `fieldMetadata` object may have metadata that overrides the regular field object
- // (e.g. a custom field display name or description on a model)
- const field = this._metadata?.field(this.fieldIdOrName());
- const combinedField = this._combineFieldWithExtraMetadata(
- field,
- fieldMetadata,
- );
+ const locallyCachedField = this._getTrustedFieldCachedOnInstance();
+ if (locallyCachedField) {
+ return locallyCachedField;
+ }
+
+ // Prioritize pulling a `field` from the Dimenion's associated query (if one exists)
+ // because it might have locally overriding metadata on it.
+ const fieldFromQuery = this._findMatchingQueryField();
+ if (fieldFromQuery) {
+ return fieldFromQuery;
+ }
- if (combinedField) {
- return combinedField;
+ const maybeTableId = this._query?.table()?.id;
+ const fieldFromGlobalState =
+ this._metadata?.field(this.fieldIdOrName(), maybeTableId) ||
+ this._metadata?.field(this.fieldIdOrName());
+ if (fieldFromGlobalState) {
+ return fieldFromGlobalState;
}
+ // Hitting this return statement means that there is a bug.
+ // This primarily serves as a way to guarantee that this function returns a Field to avoid errors in dependent code.
// Despite being unable to find a field, we _might_ still have enough data to know a few things about it.
// For example, if we have an mbql field reference, it might contain a `base-type`
- return this._createField({
- id: this.isIntegerFieldId() ? this.fieldIdOrName() : this.mbql(),
- name: this.isStringFieldName() && this.fieldIdOrName(),
- // NOTE: this display_name will likely be incorrect
- // if a `FieldDimension` isn't associated with a query then we don't know which table it belongs to
- display_name: this._fieldIdOrName,
- base_type: this.getOption("base-type"),
- });
+ return this._createFallbackField();
}
tableId() {
diff --git a/frontend/src/metabase-lib/lib/Question.ts b/frontend/src/metabase-lib/lib/Question.ts
index 7d534e5240878472881d4d848c43d9bd50f2bee5..d05e0cdacb871fe4091dfe69c1b0f31471c2a509 100644
--- a/frontend/src/metabase-lib/lib/Question.ts
+++ b/frontend/src/metabase-lib/lib/Question.ts
@@ -75,6 +75,7 @@ import {
} from "metabase-lib/lib/Alert";
import { utf8_to_b64url } from "metabase/lib/encoding";
import { CollectionId } from "metabase-types/api";
+import { getQuestionVirtualTableId } from "metabase/lib/saved-questions/saved-questions";
type QuestionUpdateFn = (q: Question) => Promise<void> | null | undefined;
@@ -583,7 +584,7 @@ class QuestionInner {
type: "query",
database: this.databaseId(),
query: {
- "source-table": "card__" + this.id(),
+ "source-table": getQuestionVirtualTableId(this.card()),
},
},
};
@@ -600,7 +601,7 @@ class QuestionInner {
type: "query",
database: this.databaseId(),
query: {
- "source-table": "card__" + this.id(),
+ "source-table": getQuestionVirtualTableId(this.card()),
},
});
}
@@ -1021,11 +1022,18 @@ class QuestionInner {
}
dependentMetadata(): DependentMetadataItem[] {
- if (!this.isDataset()) {
- return [];
- }
const dependencies = [];
+ // we frequently treat dataset/model questions like they are already nested
+ // so we need to fetch the virtual card table representation of the Question
+ // so that we can properly access the table's fields in various scenarios
+ if (this.isDataset()) {
+ dependencies.push({
+ type: "table",
+ id: getQuestionVirtualTableId(this.card()),
+ });
+ }
+
this.getResultMetadata().forEach(field => {
if (isFK(field) && field.fk_target_field_id) {
dependencies.push({
diff --git a/frontend/src/metabase-lib/lib/metadata/Base.ts b/frontend/src/metabase-lib/lib/metadata/Base.ts
index 52b03b42b22962bbabd91cbad59107f9656b143c..9285b69350ec2734e3acc60975dba6ce37ee3887 100644
--- a/frontend/src/metabase-lib/lib/metadata/Base.ts
+++ b/frontend/src/metabase-lib/lib/metadata/Base.ts
@@ -1,7 +1,7 @@
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-nocheck
export default class Base {
- _plainObject = null;
+ _plainObject: Record<string, unknown>;
constructor(object = {}) {
this._plainObject = object;
diff --git a/frontend/src/metabase-lib/lib/metadata/Field.ts b/frontend/src/metabase-lib/lib/metadata/Field.ts
index 74707a42e5fb5bb84c7ecfac259163451b03b913..c1a4bc07911822d8fd3f1adc67fa11329a373ce6 100644
--- a/frontend/src/metabase-lib/lib/metadata/Field.ts
+++ b/frontend/src/metabase-lib/lib/metadata/Field.ts
@@ -2,6 +2,7 @@
// @ts-nocheck
import _ from "underscore";
import moment from "moment-timezone";
+
import { createLookupByProperty, memoizeClass } from "metabase-lib/lib/utils";
import { formatField, stripId } from "metabase/lib/formatting";
import { getFieldValues } from "metabase/lib/query/field";
@@ -40,6 +41,7 @@ import type StructuredQuery from "metabase-lib/lib/queries/StructuredQuery";
import type NativeQuery from "metabase-lib/lib/queries/NativeQuery";
import type Table from "./Table";
import type Metadata from "./Metadata";
+import { getUniqueFieldId } from "./utils";
export const LONG_TEXT_MIN = 80;
@@ -65,6 +67,7 @@ class FieldInner extends Base {
has_field_values?: "list" | "search" | "none";
values: any[];
metadata?: Metadata;
+ source?: string;
// added when creating "virtual fields" that are associated with a given query
query?: StructuredQuery | NativeQuery;
@@ -77,6 +80,19 @@ class FieldInner extends Base {
return this.id;
}
+ // `uniqueId` is set by our normalizr schema so it is not always available,
+ // if the Field instance was instantiated outside of an entity
+ getUniqueId() {
+ if (this.uniqueId) {
+ return this.uniqueId;
+ }
+
+ const uniqueId = getUniqueFieldId(this.getId(), this.table_id);
+ this.uniqueId = uniqueId;
+
+ return uniqueId;
+ }
+
parent() {
return this.metadata ? this.metadata.field(this.parent_id) : null;
}
@@ -263,6 +279,10 @@ class FieldInner extends Base {
}
}
+ // 1. `_fieldInstance` is passed in so that we can shortwire any subsequent calls to `field()` form the dimension instance
+ // 2. The distinction between "fields" and "dimensions" is fairly fuzzy, and this method is "wrong" in the sense that
+ // The `ref` of this Field instance MIGHT be something like ["aggregation", "count"] which means that we should
+ // instantiate an AggregationDimension, not a FieldDimension, but there are bugs with that route, and this seems to work for now...
dimension() {
const ref = this.reference();
const fieldDimension = new FieldDimension(
@@ -420,6 +440,18 @@ class FieldInner extends Base {
});
}
+ clone(fieldMetadata) {
+ if (fieldMetadata instanceof Field) {
+ throw new Error("`fieldMetadata` arg must be a plain object");
+ }
+
+ const plainObject = this.getPlainObject();
+ const newField = new Field({ ...this, ...fieldMetadata });
+ newField._plainObject = { ...plainObject, ...fieldMetadata };
+
+ return newField;
+ }
+
/**
* Returns a FKDimension for this field and the provided field
* @param {Field} foreignField
diff --git a/frontend/src/metabase-lib/lib/metadata/Field.unit.spec.ts b/frontend/src/metabase-lib/lib/metadata/Field.unit.spec.ts
index da0ad1c9ea170848cc2c2e06df46cbf2bf3030ea..b3e766848d16359bc6f71670d4abcfad8d02c4c6 100644
--- a/frontend/src/metabase-lib/lib/metadata/Field.unit.spec.ts
+++ b/frontend/src/metabase-lib/lib/metadata/Field.unit.spec.ts
@@ -6,11 +6,14 @@ import Schema from "./Schema";
import Metadata from "./Metadata";
import Base from "./Base";
import Dimension from "../Dimension";
+import { createMockConcreteField } from "./mocks";
+
describe("Field", () => {
describe("instantiation", () => {
it("should create an instance of Schema", () => {
expect(new Field()).toBeInstanceOf(Field);
});
+
it("should add `object` props to the instance (because it extends Base)", () => {
expect(new Field()).toBeInstanceOf(Base);
expect(
@@ -20,10 +23,12 @@ describe("Field", () => {
).toHaveProperty("foo", "bar");
});
});
+
describe("parent", () => {
it("should return null when `metadata` does not exist on instance", () => {
expect(new Field().parent()).toBeNull();
});
+
it("should return the field that matches the instance's `parent_id` when `metadata` exists on the instance", () => {
const parentField = new Field({
id: 1,
@@ -41,6 +46,7 @@ describe("Field", () => {
expect(field.parent()).toBe(parentField);
});
});
+
describe("path", () => {
it("should return list of fields starting with instance, ending with root parent", () => {
const rootField = new Field({
@@ -66,6 +72,7 @@ describe("Field", () => {
expect(field.path()).toEqual([rootField, parentField, field]);
});
});
+
describe("displayName", () => {
it("should return a field's display name", () => {
expect(
@@ -74,6 +81,7 @@ describe("Field", () => {
}).displayName(),
).toBe("foo");
});
+
it("should prioritize the `display_name` field over `name`", () => {
expect(
new Field({
@@ -82,6 +90,7 @@ describe("Field", () => {
}).displayName(),
).toBe("bar");
});
+
it("should prioritize the name in the field's `dimensions` property if it has one", () => {
const field = new Field({
dimensions: {
@@ -91,6 +100,7 @@ describe("Field", () => {
});
expect(field.displayName()).toBe("dimensions");
});
+
describe("includePath flag", () => {
let field;
beforeEach(() => {
@@ -118,6 +128,7 @@ describe("Field", () => {
name: "field",
});
});
+
it("should add parent field display names to the field's display name when enabled", () => {
expect(
field.displayName({
@@ -125,6 +136,7 @@ describe("Field", () => {
}),
).toBe("rootField: parentField: field");
});
+
it("should be enabled by default", () => {
expect(
field.displayName({
@@ -132,6 +144,7 @@ describe("Field", () => {
}),
).toBe(field.displayName());
});
+
it("should exclude parent field display names when disabled", () => {
expect(
field.displayName({
@@ -140,6 +153,7 @@ describe("Field", () => {
).toBe("field");
});
});
+
describe("includeTable flag", () => {
let field;
beforeEach(() => {
@@ -148,6 +162,7 @@ describe("Field", () => {
name: "field",
});
});
+
it("should do nothing when there is no table on the field instance", () => {
expect(
field.displayName({
@@ -155,6 +170,7 @@ describe("Field", () => {
}),
).toBe("field");
});
+
it("should add the table name to the start of the field name", () => {
field.table = new Table({
display_name: "table",
@@ -166,6 +182,7 @@ describe("Field", () => {
).toBe("table → field");
});
});
+
describe("includeSchema flag", () => {
let field;
beforeEach(() => {
@@ -174,6 +191,7 @@ describe("Field", () => {
name: "field",
});
});
+
it("won't do anything if enabled and includeTable is not enabled", () => {
expect(
field.displayName({
@@ -181,6 +199,7 @@ describe("Field", () => {
}),
).toBe("field");
});
+
it("should add a combined schema + table display name to the start of the field name", () => {
field.table = new Table({
display_name: "table",
@@ -197,6 +216,7 @@ describe("Field", () => {
});
});
});
+
describe("targetObjectName", () => {
it("should return the display name of the field stripped of an appended id", () => {
const field = new Field({
@@ -205,6 +225,7 @@ describe("Field", () => {
expect(field.targetObjectName()).toBe("field");
});
});
+
describe("dimension", () => {
it("should return the field's dimension when the id is an mbql field", () => {
const field = new Field({
@@ -214,6 +235,7 @@ describe("Field", () => {
expect(dimension).toBeInstanceOf(Dimension);
expect(dimension.fieldIdOrName()).toBe(123);
});
+
it("should return the field's dimension when the id is not an mbql field", () => {
const field = new Field({
id: 123,
@@ -223,6 +245,7 @@ describe("Field", () => {
expect(dimension.fieldIdOrName()).toBe(123);
});
});
+
describe("getDefaultDateTimeUnit", () => {
describe("when the field is of type `type/DateTime`", () => {
it("should return 'day'", () => {
@@ -237,6 +260,7 @@ describe("Field", () => {
});
});
});
+
describe("when field is of type `type/DateTime`", () => {
it("should return a time unit depending on the number of days in the 'fingerprint'", () => {
const field = new Field({
@@ -252,6 +276,7 @@ describe("Field", () => {
expect(field.getDefaultDateTimeUnit()).toBe("month");
});
});
+
describe("remappedField", () => {
it("should return the 'human readable' field tied to the field's dimension", () => {
const field1 = new Field({
@@ -275,6 +300,7 @@ describe("Field", () => {
field.metadata = metadata;
expect(field.remappedField()).toBe(field1);
});
+
it("should return the field's name_field", () => {
const nameField = new Field();
const field = new Field({
@@ -283,10 +309,12 @@ describe("Field", () => {
});
expect(field.remappedField()).toBe(nameField);
});
+
it("should return null when the field has no name_field or no dimension with a 'human readable' field", () => {
expect(new Field().remappedField()).toBe(null);
});
});
+
describe("remappedValue", () => {
it("should call a given value using the instance's remapping property", () => {
const field = new Field({
@@ -296,6 +324,7 @@ describe("Field", () => {
});
expect(field.remappedValue(2)).toBe(1);
});
+
it("should convert a numeric field into a number if it is not a number", () => {
const field = new Field({
isNumeric: () => true,
@@ -306,6 +335,7 @@ describe("Field", () => {
expect(field.remappedValue("2.5rem")).toBe(2.5);
});
});
+
describe("hasRemappedValue", () => {
it("should call a given value using the instance's remapping property", () => {
const field = new Field({
@@ -315,6 +345,7 @@ describe("Field", () => {
});
expect(field.hasRemappedValue(2)).toBe(true);
});
+
it("should convert a numeric field into a number if it is not a number", () => {
const field = new Field({
isNumeric: () => true,
@@ -325,6 +356,7 @@ describe("Field", () => {
expect(field.hasRemappedValue("2.5rem")).toBe(true);
});
});
+
describe("isSearchable", () => {
it("should be true when the field is a string", () => {
const field = new Field({
@@ -339,6 +371,7 @@ describe("Field", () => {
expect(field.isSearchable()).toBe(false);
});
});
+
describe("fieldValues", () => {
it("should return the values on a field instance", () => {
const values = [[1], [2]];
@@ -347,6 +380,7 @@ describe("Field", () => {
});
expect(field.fieldValues()).toEqual(values);
});
+
it("should wrap raw values in arrays to match the format of remapped values", () => {
const values = [1, 2];
const field = new Field({
@@ -355,6 +389,7 @@ describe("Field", () => {
expect(field.fieldValues()).toEqual([[1], [2]]);
});
});
+
describe("hasFieldValues", () => {
it("should be true when a field has values", () => {
expect(
@@ -363,6 +398,7 @@ describe("Field", () => {
}).hasFieldValues(),
).toBe(true);
});
+
it("should be false when a field has no values", () => {
expect(
new Field({
@@ -376,4 +412,57 @@ describe("Field", () => {
).toBe(false);
});
});
+
+ describe("getUniqueId", () => {
+ describe("when the `uniqueId` field exists on the instance", () => {
+ it("should return the `uniqueId`", () => {
+ const field = new Field({
+ uniqueId: "foo",
+ });
+ expect(field.getUniqueId()).toBe("foo");
+ });
+ });
+
+ describe("when the `uniqueId` field does not exist on the instance of a concrete Field", () => {
+ let field;
+ beforeEach(() => {
+ field = createMockConcreteField({
+ apiOpts: {
+ id: 1,
+ table_id: 2,
+ },
+ });
+ });
+
+ it("should create a `uniqueId`", () => {
+ expect(field.getUniqueId()).toBe(1);
+ });
+
+ it("should set the `uniqueId` on the Field instance", () => {
+ field.getUniqueId();
+ expect(field.uniqueId).toBe(1);
+ });
+ });
+
+ describe("when the `uniqueId` field does not exist on the instance of a Field from a virtual card Table", () => {
+ let field;
+ beforeEach(() => {
+ field = createMockConcreteField({
+ apiOpts: {
+ id: 1,
+ table_id: "card__123",
+ },
+ });
+ });
+
+ it("should create a `uniqueId`", () => {
+ expect(field.getUniqueId()).toBe("card__123:1");
+ });
+
+ it("should set the `uniqueId` on the Field instance", () => {
+ field.getUniqueId();
+ expect(field.uniqueId).toBe("card__123:1");
+ });
+ });
+ });
});
diff --git a/frontend/src/metabase-lib/lib/metadata/Metadata.ts b/frontend/src/metabase-lib/lib/metadata/Metadata.ts
index ded4bbafb41047abd0f12aa7f5adee33b2e54f80..349ee94cec73527ebe63db1f9b2a8a0c40e8d4dd 100644
--- a/frontend/src/metabase-lib/lib/metadata/Metadata.ts
+++ b/frontend/src/metabase-lib/lib/metadata/Metadata.ts
@@ -6,6 +6,8 @@ import type Question from "../Question";
import type Database from "./Database";
import type Table from "./Table";
import type Schema from "./Schema";
+import type Field from "./Field";
+import { getUniqueFieldId } from "./utils";
/**
* @typedef { import("./metadata").DatabaseId } DatabaseId
@@ -102,12 +104,16 @@ export default class Metadata extends Base {
return (tableId != null && this.tables[tableId]) || null;
}
- /**
- * @param {FieldId} fieldId
- * @returns {?Field}
- */
- field(fieldId): Field | null {
- return (fieldId != null && this.fields[fieldId]) || null;
+ field(
+ fieldId: Field["id"] | undefined | null,
+ tableId?: Table["id"] | undefined | null,
+ ): Field | null {
+ if (fieldId == null) {
+ return null;
+ }
+
+ const uniqueId = getUniqueFieldId(fieldId, tableId);
+ return this.fields[uniqueId] || null;
}
question(cardId): Question | null {
diff --git a/frontend/src/metabase-lib/lib/metadata/Metadata.unit.spec.ts b/frontend/src/metabase-lib/lib/metadata/Metadata.unit.spec.ts
index 9149f4eeb3dd99759983e8094a7b4a936ce19cb2..c535c37566beb6f82f07b1b326ea444d543bb1bb 100644
--- a/frontend/src/metabase-lib/lib/metadata/Metadata.unit.spec.ts
+++ b/frontend/src/metabase-lib/lib/metadata/Metadata.unit.spec.ts
@@ -5,10 +5,10 @@ import Base from "./Base";
import Database from "./Database";
import Table from "./Table";
import Schema from "./Schema";
-import Field from "./Field";
import Segment from "./Segment";
import Metric from "./Metric";
-import Question from "../Question";
+import { createMockConcreteField } from "./mocks";
+
describe("Metadata", () => {
describe("instantiation", () => {
it("should create an instance of Metadata", () => {
@@ -146,7 +146,6 @@ describe("Metadata", () => {
["database", obj => new Database(obj)],
["schema", obj => new Schema(obj)],
["table", obj => new Table(obj)],
- ["field", obj => new Field(obj)],
].forEach(([fnName, instantiate]) => {
describe(fnName, () => {
let instanceA;
@@ -181,4 +180,73 @@ describe("Metadata", () => {
});
});
});
+
+ describe("`field`", () => {
+ it("should return null when given a nil fieldId arg", () => {
+ const metadata = new Metadata({
+ fields: {},
+ });
+ expect(metadata.field()).toBeNull();
+ expect(metadata.field(null)).toBeNull();
+ });
+
+ describe("when given a fieldId and no tableId", () => {
+ it("should return null when there is no matching field", () => {
+ const metadata = new Metadata({
+ fields: {},
+ });
+ expect(metadata.field(1)).toBeNull();
+ });
+
+ it("should return the matching Field instance", () => {
+ const field = createMockConcreteField({ apiOpts: { id: 1 } });
+ const uniqueId = field.getUniqueId();
+ const metadata = new Metadata({
+ fields: {
+ [uniqueId]: field,
+ },
+ });
+
+ expect(metadata.field(1)).toBe(field);
+ });
+ });
+
+ describe("when given a fieldId and a concrete tableId", () => {
+ it("should ignore the tableId arg because these fields are stored using the field's id", () => {
+ const field = createMockConcreteField({
+ apiOpts: { id: 1, table_id: 1 },
+ });
+ const uniqueId = field.getUniqueId();
+ const metadata = new Metadata({
+ fields: {
+ [uniqueId]: field,
+ },
+ });
+
+ expect(metadata.field(1, 1)).toBe(field);
+ // to prove the point that the `tableId` is ignore in this scenario
+ expect(metadata.field(1, 2)).toBe(field);
+ expect(metadata.field(1)).toBe(field);
+ });
+ });
+
+ describe("when given a fieldId and a virtual card tableId", () => {
+ it("should return the matching Field instance, stored using the field's `uniqueId`", () => {
+ const field = createMockConcreteField({
+ apiOpts: { id: 1, table_id: "card__123" },
+ });
+ const uniqueId = field.getUniqueId();
+ const metadata = new Metadata({
+ fields: {
+ [uniqueId]: field,
+ },
+ });
+
+ expect(metadata.field(1, "card__123")).toBe(field);
+ expect(metadata.field("card__123:1")).toBe(field);
+
+ expect(metadata.field(1)).not.toBe(field);
+ });
+ });
+ });
});
diff --git a/frontend/src/metabase-lib/lib/metadata/Table.ts b/frontend/src/metabase-lib/lib/metadata/Table.ts
index ee924b2cdb68dfd6acadacbce74f8b4816dd57fa..da8692aa5cb305ba13259136d715946631531294 100644
--- a/frontend/src/metabase-lib/lib/metadata/Table.ts
+++ b/frontend/src/metabase-lib/lib/metadata/Table.ts
@@ -142,6 +142,13 @@ class TableInner extends Base {
return pks;
}
+ clone() {
+ const plainObject = this.getPlainObject();
+ const newTable = new Table(this);
+ newTable._plainObject = plainObject;
+ return newTable;
+ }
+
/**
* @private
* @param {string} description
diff --git a/frontend/src/metabase-lib/lib/metadata/Table.unit.spec.ts b/frontend/src/metabase-lib/lib/metadata/Table.unit.spec.ts
index 5e5490daf026829efce22b7585d476f80b74a4a2..deaa3f030d3eee708419dfa130fca67eb6a5e905 100644
--- a/frontend/src/metabase-lib/lib/metadata/Table.unit.spec.ts
+++ b/frontend/src/metabase-lib/lib/metadata/Table.unit.spec.ts
@@ -1,5 +1,6 @@
import { PRODUCTS } from "__support__/sample_database_fixture";
import Table from "./Table";
+import type Field from "./Field";
describe("Table", () => {
const productsTable = new Table(PRODUCTS);
diff --git a/frontend/src/metabase-lib/lib/metadata/mocks.ts b/frontend/src/metabase-lib/lib/metadata/mocks.ts
new file mode 100644
index 0000000000000000000000000000000000000000..140d22d5c7654f8f9ba1ac333f6eafc42971b2d0
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/metadata/mocks.ts
@@ -0,0 +1,18 @@
+import Field from "./Field";
+import { createMockField } from "metabase-types/api/mocks/field";
+import { Field as ApiField } from "metabase-types/api";
+
+export function createMockConcreteField({
+ apiOpts,
+ instanceOpts,
+}: {
+ apiOpts?: Partial<ApiField>;
+ instanceOpts?: Partial<Field>;
+}) {
+ const mockField = createMockField(apiOpts);
+ const instance = new Field(mockField);
+
+ Object.assign(instance, instanceOpts);
+
+ return instance;
+}
diff --git a/frontend/src/metabase-lib/lib/metadata/utils.ts b/frontend/src/metabase-lib/lib/metadata/utils.ts
new file mode 100644
index 0000000000000000000000000000000000000000..b563e89fddd4a2b4155771983a7a5383904673a7
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/metadata/utils.ts
@@ -0,0 +1,11 @@
+import { isVirtualCardId } from "metabase/lib/saved-questions/saved-questions";
+import type Field from "./Field";
+import type Table from "./Table";
+
+export function getUniqueFieldId(id: Field["id"], tableId: Table["id"]) {
+ if (isVirtualCardId(tableId)) {
+ return `${tableId}:${id}`;
+ }
+
+ return id;
+}
diff --git a/frontend/src/metabase-lib/lib/metadata/utils.unit.spec.ts b/frontend/src/metabase-lib/lib/metadata/utils.unit.spec.ts
new file mode 100644
index 0000000000000000000000000000000000000000..79b666d4a6f4c2bb4887a688097080c9627c3c91
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/metadata/utils.unit.spec.ts
@@ -0,0 +1,19 @@
+import { getUniqueFieldId } from "./utils";
+
+describe("metabase-lib/metadata/utils", () => {
+ describe("getUniqueFieldId", () => {
+ describe("when the given tableId arg is NOT a virtual card table", () => {
+ it("should return the field's id", () => {
+ expect(getUniqueFieldId(1, 2)).toEqual(1);
+ // @ts-expect-error -- testing for when our types fail
+ expect(getUniqueFieldId(1, undefined)).toEqual(1);
+ });
+ });
+
+ describe("when the given tableId arg is a virtual card table", () => {
+ it("should return a string from the combined ids", () => {
+ expect(getUniqueFieldId(1, "card__123")).toEqual("card__123:1");
+ });
+ });
+ });
+});
diff --git a/frontend/src/metabase-lib/lib/queries/NativeQuery.ts b/frontend/src/metabase-lib/lib/queries/NativeQuery.ts
index 445624634c32147809880f10414fd8420ef1c1ed..70a0f1ffb3ad42d7d1211adf7fff4c369945e547 100644
--- a/frontend/src/metabase-lib/lib/queries/NativeQuery.ts
+++ b/frontend/src/metabase-lib/lib/queries/NativeQuery.ts
@@ -29,6 +29,8 @@ import { createTemplateTag } from "metabase-lib/lib/queries/TemplateTag";
import DimensionOptions from "../DimensionOptions";
import ValidationError from "metabase-lib/lib/ValidationError";
+import { getNativeQueryTable } from "./utils/native-query-table";
+
type DimensionFilter = (dimension: Dimension) => boolean;
type VariableFilter = (variable: Variable) => boolean;
export const NATIVE_QUERY_TEMPLATE: NativeDatasetQuery = {
@@ -189,19 +191,8 @@ export default class NativeQuery extends AtomicQuery {
);
}
- table(): Table | null | undefined {
- const database = this.database();
- const collection = this.collection();
-
- if (!database || !collection) {
- return null;
- }
-
- return (
- _.findWhere(database.tables, {
- name: collection,
- }) || null
- );
+ table(): Table | null {
+ return getNativeQueryTable(this);
}
queryText(): string {
diff --git a/frontend/src/metabase-lib/lib/queries/StructuredQuery.ts b/frontend/src/metabase-lib/lib/queries/StructuredQuery.ts
index 723e202cffe37889278efdfb187c00dee4893a9d..5000b911bd6580f098a295840cf67703473dad0a 100644
--- a/frontend/src/metabase-lib/lib/queries/StructuredQuery.ts
+++ b/frontend/src/metabase-lib/lib/queries/StructuredQuery.ts
@@ -51,6 +51,12 @@ import Table from "../metadata/Table";
import Field from "../metadata/Field";
import { TYPE } from "metabase/lib/types";
import { fieldRefForColumn } from "metabase/lib/dataset";
+import {
+ isVirtualCardId,
+ getQuestionIdFromVirtualTableId,
+} from "metabase/lib/saved-questions";
+
+import { getStructuredQueryTable } from "./utils/structured-query-table";
type DimensionFilter = (dimension: Dimension) => boolean;
type FieldFilter = (filter: Field) => boolean;
@@ -326,41 +332,8 @@ class StructuredQueryInner extends AtomicQuery {
/**
* @returns the table object, if a table is selected and loaded.
*/
- table(): Table {
- const sourceQuery = this.sourceQuery();
-
- if (sourceQuery) {
- const fields = sourceQuery.columns().map(column => {
- const id = [
- "field",
- column.name,
- {
- "base-type": column.base_type,
- },
- ];
-
- return new Field({
- ...column,
- // TODO FIXME -- Do NOT use field-literal unless you're referring to a native query
- id,
- source: "fields",
- // HACK: need to thread the query through to this fake Field
- query: this,
- });
- });
-
- return new Table({
- id: this.sourceTableId(),
- name: "",
- display_name: "",
- db: sourceQuery.database(),
- fields,
- segments: [],
- metrics: [],
- });
- }
-
- return this.metadata().table(this.sourceTableId());
+ table(): Table | null {
+ return getStructuredQueryTable(this);
}
/**
@@ -1402,7 +1375,7 @@ class StructuredQueryInner extends AtomicQuery {
f.parent_id == null
);
})
- .sortBy(d => d.displayName().toLowerCase())
+ .sortBy(d => d.displayName()?.toLowerCase())
.sortBy(d => {
const type = d.field().semantic_type;
return type === TYPE.PK ? 0 : type === TYPE.Name ? 1 : 2;
@@ -1604,6 +1577,12 @@ class StructuredQueryInner extends AtomicQuery {
* returns the original Table object at the beginning of the nested queries
*/
rootTable(): Table {
+ const question = this.question();
+ const questionTableId = question?.tableId();
+ if (questionTableId != null) {
+ return this.metadata().table(questionTableId);
+ }
+
return this.rootQuery().table();
}
@@ -1675,6 +1654,13 @@ class StructuredQueryInner extends AtomicQuery {
id: tableId,
foreignTables,
});
+
+ if (isVirtualCardId(tableId)) {
+ addDependency({
+ type: "question",
+ id: getQuestionIdFromVirtualTableId(tableId),
+ });
+ }
}
// any explicitly joined tables
diff --git a/frontend/src/metabase-lib/lib/queries/structured/Join.ts b/frontend/src/metabase-lib/lib/queries/structured/Join.ts
index 03b255000e398de6d4a41c969ea1a1fd504f7ace..f38418ba0ad6d95385f666a2db5a7d8155d309a3 100644
--- a/frontend/src/metabase-lib/lib/queries/structured/Join.ts
+++ b/frontend/src/metabase-lib/lib/queries/structured/Join.ts
@@ -559,14 +559,14 @@ export default class Join extends MBQLObjectClause {
const sourceTable = this.joinSourceTableId();
const sourceQuery = this.joinSourceQuery();
return sourceTable
- ? new StructuredQuery(this.query().question(), {
+ ? new StructuredQuery(this.query().question().setDataset(false), {
type: "query",
query: {
"source-table": sourceTable,
},
})
: sourceQuery
- ? new StructuredQuery(this.query().question(), {
+ ? new StructuredQuery(this.query().question().setDataset(false), {
type: "query",
query: sourceQuery,
})
diff --git a/frontend/src/metabase-lib/lib/queries/utils/native-query-table.ts b/frontend/src/metabase-lib/lib/queries/utils/native-query-table.ts
new file mode 100644
index 0000000000000000000000000000000000000000..effd43a60ecce2455d4c9c67d749768f040b4b21
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/queries/utils/native-query-table.ts
@@ -0,0 +1,28 @@
+import _ from "underscore";
+
+import type Table from "metabase-lib/lib/metadata/Table";
+
+import type NativeQuery from "../NativeQuery";
+import { getDatasetTable } from "./nested-card-query-table";
+
+export function getNativeQueryTable(nativeQuery: NativeQuery): Table | null {
+ const question = nativeQuery.question();
+ const isDataset = question?.isDataset();
+
+ if (isDataset) {
+ return getDatasetTable(nativeQuery);
+ }
+
+ const database = nativeQuery.database();
+ const collection = nativeQuery.collection();
+ if (database && collection) {
+ return (
+ _.findWhere(database.tables, {
+ name: collection,
+ }) || null
+ );
+ }
+
+ // Native queries aren't always associated with a specific table
+ return null;
+}
diff --git a/frontend/src/metabase-lib/lib/queries/utils/native-query-table.unit.spec.ts b/frontend/src/metabase-lib/lib/queries/utils/native-query-table.unit.spec.ts
new file mode 100644
index 0000000000000000000000000000000000000000..d1488eb52e5c908a2a7642e0b78475c16db1b32d
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/queries/utils/native-query-table.unit.spec.ts
@@ -0,0 +1,100 @@
+import { merge } from "icepick";
+
+import {
+ metadata,
+ PRODUCTS,
+ SAMPLE_DATABASE,
+} from "__support__/sample_database_fixture";
+import Question from "metabase-lib/lib/Question";
+import Table from "metabase-lib/lib/metadata/Table";
+import Field from "metabase-lib/lib/metadata/Field";
+import type NativeQuery from "metabase-lib/lib/queries/NativeQuery";
+
+import { getNativeQueryTable } from "./native-query-table";
+
+const NATIVE_QUERY_CARD = {
+ id: 1,
+ dataset_query: {
+ database: SAMPLE_DATABASE?.id,
+ type: "native",
+ native: {
+ query: "select * from ORDERS where CREATED_AT = {{created}}",
+ },
+ },
+};
+
+describe("metabase-lib/queries/utils/native-query-table", () => {
+ describe("native query associated with a dataset/model question", () => {
+ const PRODUCT_ID_WITH_OVERRIDING_METADATA = new Field({
+ ...PRODUCTS.ID.getPlainObject(),
+ display_name: "~*~Products.ID~*~",
+ });
+
+ const nativeDatasetQuestion = new Question(NATIVE_QUERY_CARD, metadata)
+ .setDataset(true)
+ .setDisplayName("Native Dataset Question");
+
+ const nestedNativeDatasetTable = new Table({
+ id: "card__1",
+ display_name: nativeDatasetQuestion.displayName(),
+ name: nativeDatasetQuestion.displayName(),
+ fields: [PRODUCT_ID_WITH_OVERRIDING_METADATA],
+ });
+
+ metadata.questions = {
+ [nativeDatasetQuestion.id()]: nativeDatasetQuestion,
+ };
+ metadata.tables[nestedNativeDatasetTable.id] = nestedNativeDatasetTable;
+
+ const table = getNativeQueryTable(
+ nativeDatasetQuestion.query() as NativeQuery,
+ );
+
+ it("should return a nested card table using the given query's question", () => {
+ expect(table?.getPlainObject()).toEqual(
+ expect.objectContaining({
+ display_name: "Native Dataset Question",
+ id: "card__1",
+ name: "Native Dataset Question",
+ }),
+ );
+
+ expect(table?.fields.map(field => field.getPlainObject())).toEqual([
+ {
+ ...PRODUCT_ID_WITH_OVERRIDING_METADATA.getPlainObject(),
+ display_name: "~*~Products.ID~*~",
+ },
+ ]);
+ });
+ });
+
+ describe("native query associated with botha `collection` and a `database`", () => {
+ const nativeQuestionWithCollection = new Question(
+ merge(NATIVE_QUERY_CARD, {
+ dataset_query: {
+ native: {
+ collection: PRODUCTS.name,
+ },
+ },
+ }),
+ metadata,
+ );
+
+ const table = getNativeQueryTable(
+ nativeQuestionWithCollection.query() as NativeQuery,
+ );
+
+ it("should return the concrete `table` associated with the given collection name", () => {
+ expect(table).toBe(PRODUCTS);
+ });
+ });
+
+ describe("basic native query question", () => {
+ const nativeQuestion = new Question(NATIVE_QUERY_CARD, metadata);
+ const table = getNativeQueryTable(nativeQuestion.query() as NativeQuery);
+
+ it("should not return a table", () => {
+ expect(table).toBeNull();
+ });
+ });
+});
diff --git a/frontend/src/metabase-lib/lib/queries/utils/nested-card-query-table.ts b/frontend/src/metabase-lib/lib/queries/utils/nested-card-query-table.ts
new file mode 100644
index 0000000000000000000000000000000000000000..ed99a354551743f4ec976c93fbf2fbc74915985f
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/queries/utils/nested-card-query-table.ts
@@ -0,0 +1,70 @@
+import {
+ getQuestionIdFromVirtualTableId,
+ getQuestionVirtualTableId,
+} from "metabase/lib/saved-questions";
+import type Table from "metabase-lib/lib/metadata/Table";
+import type Question from "metabase-lib/lib/Question";
+import type StructuredQuery from "../StructuredQuery";
+import type NativeQuery from "../NativeQuery";
+import { createVirtualField, createVirtualTable } from "./virtual-table";
+
+// This function expects a `sourceTableId` to exist in the `metadata.table` cache
+// It also expects the card associated with the `sourceTableId` to exist in the `metadata.question` cache
+export function getNestedCardTable(query: StructuredQuery): Table | null {
+ const sourceTableId = query.sourceTableId();
+ const metadata = query.metadata();
+ const nestedCardTable = metadata.table(sourceTableId);
+ if (nestedCardTable) {
+ return nestedCardTable;
+ }
+
+ const questionId = getQuestionIdFromVirtualTableId(sourceTableId);
+ const nestedQuestion = metadata.question(questionId);
+ // There are scenarios (and possible race conditions) in the application where
+ // the nested card table might not be available, but if we have access to a Question
+ // with result_metadata then we might as well use it to create virtual fields
+ if (nestedQuestion) {
+ return createVirtualTableUsingQuestionMetadata(nestedQuestion);
+ }
+
+ return null;
+}
+
+// Treat the Dataset/Model like a Question that uses itself as its source table
+// Expects the Question to have been fetched as a virtual table
+export function getDatasetTable(
+ query: StructuredQuery | NativeQuery,
+): Table | null {
+ const question = query.question();
+ const composedDatasetQuestion = question.composeDataset();
+ const composedQuestionQuery =
+ composedDatasetQuestion.query() as StructuredQuery;
+ return getNestedCardTable(composedQuestionQuery);
+}
+
+function createVirtualTableUsingQuestionMetadata(question: Question): Table {
+ const metadata = question.metadata();
+ const questionResultMetadata = question.getResultMetadata();
+ const questionDisplayName = question.displayName() as string;
+ const query = question.query() as StructuredQuery | NativeQuery;
+ const fields = questionResultMetadata.map((fieldMetadata: any) => {
+ const field = metadata.field(fieldMetadata.id);
+ const virtualField = field
+ ? field.clone(fieldMetadata)
+ : createVirtualField(fieldMetadata);
+
+ virtualField.query = query;
+ virtualField.metadata = metadata;
+
+ return virtualField;
+ });
+
+ return createVirtualTable({
+ id: getQuestionVirtualTableId(question.card()),
+ name: questionDisplayName,
+ display_name: questionDisplayName,
+ db: question?.database(),
+ fields,
+ metadata,
+ });
+}
diff --git a/frontend/src/metabase-lib/lib/queries/utils/structured-query-table.ts b/frontend/src/metabase-lib/lib/queries/utils/structured-query-table.ts
new file mode 100644
index 0000000000000000000000000000000000000000..54c074b34a0578f6fdf213ec3aa7e40e084c7e64
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/queries/utils/structured-query-table.ts
@@ -0,0 +1,76 @@
+import type Field from "metabase-lib/lib/metadata/Field";
+import type Table from "metabase-lib/lib/metadata/Table";
+import type { Field as FieldRef } from "metabase-types/types/Query";
+
+import { isVirtualCardId } from "metabase/lib/saved-questions";
+
+import type StructuredQuery from "../StructuredQuery";
+import { createVirtualTable, createVirtualField } from "./virtual-table";
+import { getDatasetTable, getNestedCardTable } from "./nested-card-query-table";
+
+export function getStructuredQueryTable(query: StructuredQuery): Table | null {
+ const sourceQuery = query.sourceQuery();
+ // 1. Query has a source query. Use the source query as a table.
+ if (sourceQuery) {
+ return getSourceQueryTable(query);
+ }
+
+ // 2. Query has a source table that is a nested card.
+ const sourceTableId = query.sourceTableId();
+ if (isVirtualCardId(sourceTableId)) {
+ return getNestedCardTable(query);
+ }
+
+ // 3. The query's question is a dataset.
+ const question = query.question();
+ const isDataset = question?.isDataset() ?? false;
+ if (isDataset) {
+ return getDatasetTable(query);
+ }
+
+ // 4. The query's table is a concrete table, assuming one exists in `metadata`.
+ // Faiure to find a table at this point indicates that there is a bug.
+ return query.metadata().table(sourceTableId);
+}
+
+function getFieldsForSourceQueryTable(
+ originalQuery: StructuredQuery,
+ sourceQuery: StructuredQuery,
+): Field[] {
+ const metadata = originalQuery.metadata();
+ return sourceQuery.columns().map(column => {
+ // Not sure why we build out `id` like this, but it's what the old code did
+ const id: FieldRef = [
+ "field",
+ column.name,
+ {
+ "base-type": column.base_type,
+ },
+ ];
+
+ const virtualField = createVirtualField({
+ ...column,
+ id,
+ query: originalQuery,
+ metadata,
+ });
+
+ return virtualField;
+ });
+}
+
+function getSourceQueryTable(query: StructuredQuery): Table {
+ const sourceQuery = query.sourceQuery() as StructuredQuery;
+ const fields = getFieldsForSourceQueryTable(query, sourceQuery);
+ const sourceTableId = sourceQuery.sourceTableId() as Table["id"];
+
+ return createVirtualTable({
+ id: sourceTableId,
+ db: sourceQuery.database(),
+ fields,
+ metadata: sourceQuery.metadata(),
+ // intentionally set these to "" so that we fallback to a title of "Previous results" in join steps
+ display_name: "",
+ name: "",
+ });
+}
diff --git a/frontend/src/metabase-lib/lib/queries/utils/structured-query-table.unit.spec.ts b/frontend/src/metabase-lib/lib/queries/utils/structured-query-table.unit.spec.ts
new file mode 100644
index 0000000000000000000000000000000000000000..f55909d5010616f02a9f9f40df45cf9019bb0fb8
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/queries/utils/structured-query-table.unit.spec.ts
@@ -0,0 +1,219 @@
+import _ from "underscore";
+
+import {
+ metadata,
+ PRODUCTS,
+ ORDERS,
+ SAMPLE_DATABASE,
+} from "__support__/sample_database_fixture";
+import Question from "metabase-lib/lib/Question";
+import Table from "metabase-lib/lib/metadata/Table";
+import Field from "metabase-lib/lib/metadata/Field";
+
+import type StructuredQuery from "../StructuredQuery";
+import { getStructuredQueryTable } from "./structured-query-table";
+
+describe("metabase-lib/queries/utils/structured-query-table", () => {
+ describe("Card that relies on another card as its source table", () => {
+ const NESTED_CARD_QUESTION = new Question(
+ {
+ dataset_query: {
+ database: SAMPLE_DATABASE?.id,
+ query: {
+ "source-table": "card__1",
+ },
+ type: "query",
+ },
+ id: 2,
+ display: "table",
+ database_id: SAMPLE_DATABASE?.id,
+ table_id: PRODUCTS.id,
+ name: "Question based on another question",
+ },
+ metadata,
+ );
+
+ const BASE_QUESTION = new Question(
+ {
+ ...PRODUCTS.newQuestion().card(),
+ id: 1,
+ display: "table",
+ database_id: SAMPLE_DATABASE?.id,
+ table_id: PRODUCTS.id,
+ name: "Base question",
+ },
+ metadata,
+ );
+
+ const NESTED_CARD_TABLE = new Table({
+ id: "card__1",
+ display_name: BASE_QUESTION.displayName(),
+ name: BASE_QUESTION.displayName(),
+ });
+ NESTED_CARD_TABLE.fields = [
+ new Field({
+ name: "boolean",
+ display_name: "boolean",
+ base_type: "type/Boolean",
+ effective_type: "type/Boolean",
+ semantic_type: null,
+ field_ref: [
+ "field",
+ "boolean",
+ {
+ "base-type": "type/Boolean",
+ },
+ ],
+ }),
+ new Field({
+ base_type: "type/Text",
+ display_name: "Foo",
+ effective_type: "type/Text",
+ field_ref: ["expression", "Foo"],
+ id: ["field", "Foo", { "base-type": "type/Text" }],
+ name: "Foo",
+ semantic_type: null,
+ table_id: "card__1",
+ }),
+ new Field({
+ id: PRODUCTS.CATEGORY.id,
+ display_name: "~*~ Category ~*~",
+ }),
+ ];
+
+ metadata.questions = {
+ [NESTED_CARD_QUESTION.id()]: NESTED_CARD_QUESTION,
+ [BASE_QUESTION.id()]: BASE_QUESTION,
+ };
+
+ metadata.tables[NESTED_CARD_TABLE.id] = NESTED_CARD_TABLE;
+
+ const table = getStructuredQueryTable(
+ NESTED_CARD_QUESTION.query() as StructuredQuery,
+ );
+
+ it("should return a table", () => {
+ expect(table).toBeInstanceOf(Table);
+ });
+
+ it("should return a virtual table based on the nested card", () => {
+ expect(table?.getPlainObject()).toEqual(
+ NESTED_CARD_TABLE.getPlainObject(),
+ );
+ expect(table?.fields).toEqual(NESTED_CARD_TABLE.fields);
+ });
+ });
+
+ describe("Dataset/model card", () => {
+ const ORDERS_USER_ID_FIELD = metadata
+ .field(ORDERS.USER_ID.id)
+ ?.getPlainObject();
+ const OVERWRITTEN_USER_ID_FIELD_METADATA = {
+ ...ORDERS_USER_ID_FIELD,
+ display_name: "Foo",
+ description: "Bar",
+ fk_target_field_id: 1,
+ semantic_type: "type/Price",
+ settings: {
+ show_mini_bar: true,
+ },
+ };
+
+ const ORDERS_DATASET = ORDERS.question()
+ .setCard({ ...ORDERS.question().card(), id: 3 })
+ .setDataset(true)
+ .setDisplayName("Dataset Question");
+
+ const ORDERS_DATASET_TABLE = new Table({
+ id: "card__3",
+ display_name: ORDERS_DATASET.displayName(),
+ name: ORDERS_DATASET.displayName(),
+ fields: [new Field(OVERWRITTEN_USER_ID_FIELD_METADATA)],
+ });
+
+ metadata.questions = {
+ [ORDERS_DATASET.id()]: ORDERS_DATASET,
+ };
+
+ metadata.tables[ORDERS_DATASET_TABLE.id] = ORDERS_DATASET_TABLE;
+
+ const table = getStructuredQueryTable(ORDERS_DATASET.query());
+ it("should return a nested card table using the given query's question", () => {
+ expect(table?.getPlainObject()).toEqual(
+ expect.objectContaining({
+ display_name: "Dataset Question",
+ id: "card__3",
+ name: "Dataset Question",
+ }),
+ );
+
+ expect(table?.fields.map(field => field.getPlainObject())).toEqual([
+ OVERWRITTEN_USER_ID_FIELD_METADATA,
+ ]);
+ });
+ });
+
+ describe("Card that relies on a source query", () => {
+ const SOURCE_QUERY_QUESTION = new Question(
+ {
+ dataset_query: {
+ database: SAMPLE_DATABASE?.id,
+ query: { "source-query": { "source-table": PRODUCTS.id } },
+ type: "query",
+ },
+ id: 2,
+ display: "table",
+ database_id: SAMPLE_DATABASE?.id,
+ table_id: PRODUCTS.id,
+ name: "Question using a nested query",
+ },
+ metadata,
+ );
+
+ metadata.questions = {
+ [SOURCE_QUERY_QUESTION.id()]: SOURCE_QUERY_QUESTION,
+ };
+
+ const table = getStructuredQueryTable(
+ SOURCE_QUERY_QUESTION.query() as StructuredQuery,
+ );
+
+ it("should return a virtual table based on the nested query", () => {
+ expect(table?.getPlainObject()).toEqual({
+ id: 3,
+ display_name: "",
+ name: "",
+ });
+ });
+
+ it("should contain fields", () => {
+ const fields = _.sortBy(
+ (table?.fields as Field[]).map(field => field.getPlainObject()),
+ "name",
+ );
+ const nestedQueryProductFields = _.sortBy(
+ PRODUCTS.fields.map((field: Field) => {
+ const column = field.dimension().column();
+ return {
+ ...column,
+ id: ["field", column.name, { "base-type": column.base_type }],
+ source: "fields",
+ };
+ }),
+ "name",
+ );
+
+ expect(fields).toEqual(nestedQueryProductFields);
+ });
+ });
+
+ describe("Card that has a concrete source table", () => {
+ const table = getStructuredQueryTable(
+ ORDERS.newQuestion().query() as StructuredQuery,
+ );
+
+ it("should return the concrete table stored on the Metadata object", () => {
+ expect(table).toBe(ORDERS);
+ });
+ });
+});
diff --git a/frontend/src/metabase-lib/lib/queries/utils/virtual-table.ts b/frontend/src/metabase-lib/lib/queries/utils/virtual-table.ts
new file mode 100644
index 0000000000000000000000000000000000000000..f7a022b3ca334b9dbf1ee086e7f7e8443def98a5
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/queries/utils/virtual-table.ts
@@ -0,0 +1,66 @@
+import Table from "metabase-lib/lib/metadata/Table";
+import Field from "metabase-lib/lib/metadata/Field";
+import type Database from "metabase-lib/lib/metadata/Database";
+import type Metadata from "metabase-lib/lib/metadata/Metadata";
+import type StructuredQuery from "../StructuredQuery";
+import type NativeQuery from "../NativeQuery";
+
+type VirtualTableProps = {
+ metadata: Metadata;
+ fields?: Field[];
+ db?: Database | null;
+} & Partial<Table>;
+
+type VirtualFieldProps = {
+ metadata: Metadata;
+ query: StructuredQuery | NativeQuery;
+} & Partial<Field>;
+
+// For when you have no Table
+export function createVirtualTable({
+ metadata,
+ fields,
+ db,
+ ...rest
+}: VirtualTableProps): Table {
+ const table = new Table({
+ name: "",
+ display_name: "",
+ ...rest,
+ });
+
+ Object.assign(table, {
+ fields: fields || [],
+ db,
+ metadata,
+ segments: [],
+ metrics: [],
+ });
+
+ table.metadata = metadata;
+ table.db = db;
+ table.fields = fields || [];
+
+ table.fields.forEach(field => {
+ field.table = table;
+ field.table_id = table.id;
+ });
+
+ return table;
+}
+
+export function createVirtualField({
+ metadata,
+ query,
+ ...rest
+}: VirtualFieldProps): Field {
+ const field = new Field({
+ source: "fields",
+ ...rest,
+ });
+
+ field.metadata = metadata;
+ field.query = query;
+
+ return field;
+}
diff --git a/frontend/src/metabase-lib/lib/queries/utils/virtual-table.unit.spec.ts b/frontend/src/metabase-lib/lib/queries/utils/virtual-table.unit.spec.ts
new file mode 100644
index 0000000000000000000000000000000000000000..83acce6d5cd1507629ec6444e12800e9a12ee700
--- /dev/null
+++ b/frontend/src/metabase-lib/lib/queries/utils/virtual-table.unit.spec.ts
@@ -0,0 +1,68 @@
+import { createVirtualField, createVirtualTable } from "./virtual-table";
+import { metadata, PRODUCTS } from "__support__/sample_database_fixture";
+import Field from "metabase-lib/lib/metadata/Field";
+import Table from "metabase-lib/lib/metadata/Table";
+
+describe("metabase-lib/queries/utils/virtual-table", () => {
+ const query = PRODUCTS.newQuestion().query();
+ const field = createVirtualField({
+ id: 123,
+ metadata,
+ query,
+ });
+
+ describe("createVirtualField", () => {
+ it("should return a new Field instance", () => {
+ expect(field.id).toBe(123);
+ expect(field).toBeInstanceOf(Field);
+ });
+
+ it("should set `metadata` and `query` on the field instance but not its underlying plain object", () => {
+ expect(field.metadata).toBe(metadata);
+ expect(field.query).toBe(query);
+
+ const plainObject = field.getPlainObject() as any;
+ expect(plainObject.metadata).toBeUndefined();
+ expect(plainObject.query).toBeUndefined();
+ });
+ });
+
+ describe("createVirtualTable", () => {
+ const query = PRODUCTS.newQuestion().query();
+ const field1 = createVirtualField({
+ id: 1,
+ metadata,
+ query,
+ });
+ const field2 = createVirtualField({
+ id: 2,
+ metadata,
+ query,
+ });
+
+ const table = createVirtualTable({
+ id: 456,
+ metadata,
+ fields: [field1, field2],
+ });
+
+ it("should return a new Table instance", () => {
+ expect(table.id).toBe(456);
+ expect(table).toBeInstanceOf(Table);
+ });
+
+ it("should set `metadata` on the table instance but not its underlying plain object", () => {
+ expect(table.metadata).toBe(metadata);
+
+ const plainObject = table.getPlainObject() as any;
+ expect(plainObject.metadata).toBeUndefined();
+ });
+
+ it("should add a table reference to its fields", () => {
+ expect(table.fields.every(field => field.table === table)).toBe(true);
+ expect(table.fields.every(field => field.table_id === table.id)).toBe(
+ true,
+ );
+ });
+ });
+});
diff --git a/frontend/src/metabase-types/api/field.ts b/frontend/src/metabase-types/api/field.ts
index 702f885a382cb0f7cee9f7962f7d276bcb9f2d3b..150904493b47ef51b4d806d156103d1e91c29eb9 100644
--- a/frontend/src/metabase-types/api/field.ts
+++ b/frontend/src/metabase-types/api/field.ts
@@ -42,7 +42,7 @@ export interface Field {
description: string | null;
nfc_path: string[] | null;
- fingerprint: FieldFingerprint;
+ fingerprint?: FieldFingerprint;
}
export type FieldDimension = {
diff --git a/frontend/src/metabase-types/api/mocks/field.ts b/frontend/src/metabase-types/api/mocks/field.ts
new file mode 100644
index 0000000000000000000000000000000000000000..de75b99377e515b4990a4edd7334d00bec87f421
--- /dev/null
+++ b/frontend/src/metabase-types/api/mocks/field.ts
@@ -0,0 +1,12 @@
+import { Field } from "metabase-types/api";
+
+export const createMockField = (opts?: Partial<Field>): Field => ({
+ id: 1,
+ display_name: "Mock Field",
+ table_id: 1,
+ name: "mock_field",
+ base_type: "type/Text",
+ description: null,
+ nfc_path: null,
+ ...opts,
+});
diff --git a/frontend/src/metabase/parameters/utils/mapping-options.unit.spec.js b/frontend/src/metabase/parameters/utils/mapping-options.unit.spec.js
index 1b840ebebff10a406af2113b93700df7e28071c8..e9300852aa31609e9d99867dafa1b5bb9900204c 100644
--- a/frontend/src/metabase/parameters/utils/mapping-options.unit.spec.js
+++ b/frontend/src/metabase/parameters/utils/mapping-options.unit.spec.js
@@ -98,13 +98,14 @@ describe("parameters/utils/mapping-options", () => {
{ type: "date/single" },
structured({
"source-query": {
- "source-table": ORDERS.id,
+ "source-table": PRODUCTS.id,
},
}),
);
expect(options).toEqual([
{
- sectionName: null,
+ // this is a source query, and tables for source queries do not have a display_name
+ sectionName: "",
name: "Created At",
icon: "calendar",
target: [
diff --git a/frontend/src/metabase/query_builder/actions/core/updateQuestion.ts b/frontend/src/metabase/query_builder/actions/core/updateQuestion.ts
index cc4c25ac1e7ae2b6b60797dddedd248258f093aa..81b54e16e79eda13ae76d550996bf537cfe9825d 100644
--- a/frontend/src/metabase/query_builder/actions/core/updateQuestion.ts
+++ b/frontend/src/metabase/query_builder/actions/core/updateQuestion.ts
@@ -227,8 +227,16 @@ export const updateQuestion = (
}
}
- const currentDependencies = currentQuestion?.query().dependentMetadata();
- const nextDependencies = newQuestion.query().dependentMetadata();
+ const currentDependencies = currentQuestion
+ ? [
+ ...currentQuestion.dependentMetadata(),
+ ...currentQuestion.query().dependentMetadata(),
+ ]
+ : [];
+ const nextDependencies = [
+ ...newQuestion.dependentMetadata(),
+ ...newQuestion.query().dependentMetadata(),
+ ];
try {
if (!_.isEqual(currentDependencies, nextDependencies)) {
await dispatch(loadMetadataForCard(newQuestion.card()));
diff --git a/frontend/src/metabase/query_builder/actions/core/updateQuestion.unit.spec.ts b/frontend/src/metabase/query_builder/actions/core/updateQuestion.unit.spec.ts
index 84b234d248db7fbc624af2b5617a487104ecf9dd..a5a5c47a94aa5ae55cb7bb5e452b35911bb4733f 100644
--- a/frontend/src/metabase/query_builder/actions/core/updateQuestion.unit.spec.ts
+++ b/frontend/src/metabase/query_builder/actions/core/updateQuestion.unit.spec.ts
@@ -31,6 +31,7 @@ import {
PEOPLE,
PRODUCTS,
state as entitiesState,
+ metadata,
} from "__support__/sample_database_fixture";
import * as navigation from "../navigation";
@@ -69,6 +70,12 @@ async function setup({
shouldUpdateUrl,
shouldStartAdHocQuestion,
}: SetupOpts) {
+ if (originalQuestion.id()) {
+ metadata.questions = {
+ [originalQuestion.id()]: originalQuestion,
+ };
+ }
+
const dispatch = jest.fn().mockReturnValue({ mock: "mock" });
const queryResult = createMockDataset({
@@ -289,7 +296,6 @@ describe("QB Actions > updateQuestion", () => {
describe(questionType, () => {
it("turns question into ad-hoc", async () => {
const { result } = await setup({ question });
-
expect(result.card.id).toBeUndefined();
expect(result.card.name).toBeUndefined();
expect(result.card.description).toBeUndefined();
@@ -440,18 +446,63 @@ describe("QB Actions > updateQuestion", () => {
});
describe("structured", () => {
- STRUCTURED_TEST_CASES.forEach(testCase => {
+ const modelTestCases = STRUCTURED_TEST_CASES.filter(testCase => {
+ return testCase.question.isDataset();
+ });
+ const structuredQuestionTestCases = STRUCTURED_TEST_CASES.filter(
+ testCase => {
+ return !testCase.question.isDataset();
+ },
+ );
+
+ modelTestCases.forEach(testCase => {
const { question, questionType } = testCase;
describe(questionType, () => {
- it("doesn't refresh question metadata if dependent metadata doesn't change", async () => {
+ it("loads metadata for the model", async () => {
const loadMetadataSpy = jest.spyOn(
metadataActions,
"loadMetadataForCard",
);
await setup({ question });
+ expect(loadMetadataSpy).toHaveBeenCalledTimes(1);
+ });
+ it("refreshes question metadata if there's difference in dependent metadata", async () => {
+ const loadMetadataSpy = jest.spyOn(
+ metadataActions,
+ "loadMetadataForCard",
+ );
+ const join = new Join(PRODUCTS_JOIN_CLAUSE);
+ const query = question.query() as StructuredQuery;
+ const questionWithJoin = query.join(join).question();
+
+ await setup({
+ question: questionWithJoin,
+ originalQuestion: question,
+ });
+
+ expect(loadMetadataSpy).toHaveBeenCalledWith(
+ expect.objectContaining({
+ dataset_query: questionWithJoin.datasetQuery(),
+ }),
+ );
+ });
+ });
+ });
+
+ structuredQuestionTestCases.forEach(testCase => {
+ const { question, questionType } = testCase;
+
+ describe(questionType, () => {
+ it("doesn't refresh question metadata if dependent metadata doesn't change", async () => {
+ const loadMetadataSpy = jest.spyOn(
+ metadataActions,
+ "loadMetadataForCard",
+ );
+
+ await setup({ question });
expect(loadMetadataSpy).not.toHaveBeenCalled();
});
diff --git a/frontend/src/metabase/query_builder/actions/models.js b/frontend/src/metabase/query_builder/actions/models.js
index c737d654f23acc920474961011e3d8e084972779..4ac8475c64125d12aff2f933e9bc9f7908716fe7 100644
--- a/frontend/src/metabase/query_builder/actions/models.js
+++ b/frontend/src/metabase/query_builder/actions/models.js
@@ -4,14 +4,16 @@ import { merge } from "icepick";
import { t } from "ttag";
import { isLocalField, isSameField } from "metabase/lib/query/field_ref";
-
import { addUndo } from "metabase/redux/undo";
+import { loadMetadataForQueries } from "metabase/redux/metadata";
+import Questions from "metabase/entities/questions";
import { getOriginalCard, getQuestion, getResultsMetadata } from "../selectors";
-import { apiUpdateQuestion, updateQuestion } from "./core";
+import { apiUpdateQuestion, updateQuestion, API_UPDATE_QUESTION } from "./core";
import { runQuestionQuery } from "./querying";
import { setQueryBuilderMode } from "./ui";
+import { getMetadata } from "metabase/selectors/metadata";
export const setDatasetEditorTab = datasetEditorTab => dispatch => {
dispatch(setQueryBuilderMode("dataset", { datasetEditorTab }));
@@ -28,8 +30,22 @@ export const onCancelDatasetChanges = () => (dispatch, getState) => {
export const turnQuestionIntoDataset = () => async (dispatch, getState) => {
const question = getQuestion(getState());
- const dataset = question.setDataset(true);
- await dispatch(apiUpdateQuestion(dataset, { rerunQuery: true }));
+
+ await dispatch(
+ Questions.actions.update(
+ {
+ id: question.id(),
+ },
+ question.setDataset(true).setDisplay("table").card(),
+ ),
+ );
+
+ const metadata = getMetadata(getState());
+ const dataset = metadata.question(question.id());
+
+ await dispatch(loadMetadataForQueries([], [dataset.dependentMetadata()]));
+
+ dispatch.action(API_UPDATE_QUESTION, dataset.card());
dispatch(
addUndo({
diff --git a/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx b/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx
index 32f26e92cd6dd0976e4c7283dcab844750d99aae..16a9fca60af896e15c46e4caf19eb99164366881 100644
--- a/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx
+++ b/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx
@@ -255,7 +255,7 @@ function DatasetEditor(props) {
const focusedField = useMemo(() => {
const field = fields[focusedFieldIndex];
if (field) {
- const fieldMetadata = metadata.field(field.id);
+ const fieldMetadata = metadata.field(field.id, field.table_id);
return {
...fieldMetadata,
...field,
diff --git a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.tsx b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.tsx
index 5f25bae6735e01a2d63dce2e7a0d460c43ed1537..2098b69f960e134f6000040b900346085f4fe12d 100644
--- a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.tsx
+++ b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.tsx
@@ -173,7 +173,7 @@ const SegmentListItem = ({
<InlineOperatorSelector
fieldName={t`Filter down to a segment`}
iconName="filter"
- tableName={isSearch ? query.table().displayName() : undefined}
+ tableName={isSearch ? query.table()?.displayName() : undefined}
/>
<SegmentFilterSelect
query={query}
diff --git a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/BulkFilterModal.tsx b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/BulkFilterModal.tsx
index 1fe8550363dea6613aedf55b0bd9d3461a3619a8..b904e34ad616a05a0a7439bab7bf9007fae45589 100644
--- a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/BulkFilterModal.tsx
+++ b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/BulkFilterModal.tsx
@@ -252,7 +252,7 @@ const getQuery = (question: Question) => {
const getTitle = (query: StructuredQuery, singleTable: boolean) => {
const table = query.table();
- if (singleTable) {
+ if (singleTable && table) {
return t`Filter ${pluralize(table.displayName())} by`;
} else {
return t`Filter by`;
diff --git a/frontend/src/metabase/schema.js b/frontend/src/metabase/schema.js
index 894a44964316734728141a0ddaabcc03752ff27e..c828736972446072049aa07a0e94d55ae3154432 100644
--- a/frontend/src/metabase/schema.js
+++ b/frontend/src/metabase/schema.js
@@ -3,6 +3,7 @@
import { schema } from "normalizr";
import { generateSchemaId, entityTypeForObject } from "metabase/lib/schema";
import { SAVED_QUESTIONS_VIRTUAL_DB_ID } from "metabase/lib/saved-questions";
+import { getUniqueFieldId } from "metabase-lib/lib/metadata/utils";
export const QuestionSchema = new schema.Entity("questions");
export const BookmarkSchema = new schema.Entity("bookmarks");
@@ -43,7 +44,20 @@ export const TableSchema = new schema.Entity(
},
},
);
-export const FieldSchema = new schema.Entity("fields");
+
+export const FieldSchema = new schema.Entity("fields", undefined, {
+ processStrategy(field) {
+ const uniqueId = getUniqueFieldId(field.id, field.table_id);
+ return {
+ ...field,
+ uniqueId,
+ };
+ },
+ idAttribute: field => {
+ return getUniqueFieldId(field.id, field.table_id);
+ },
+});
+
export const SegmentSchema = new schema.Entity("segments");
export const MetricSchema = new schema.Entity("metrics");
export const PersistedModelSchema = new schema.Entity("persistedModels");
diff --git a/frontend/src/metabase/selectors/metadata.js b/frontend/src/metabase/selectors/metadata.js
index a1bf385038c899779a778c05be03db537f22d346..6f792a721610dcb95031c5df88563c191968e45f 100644
--- a/frontend/src/metabase/selectors/metadata.js
+++ b/frontend/src/metabase/selectors/metadata.js
@@ -68,18 +68,41 @@ export const getShallowFields = getNormalizedFields;
export const getShallowMetrics = getNormalizedMetrics;
export const getShallowSegments = getNormalizedSegments;
-export const instantiateDatabase = obj => new Database(obj);
-export const instantiateSchema = obj => new Schema(obj);
-export const instantiateTable = obj => new Table(obj);
+export const instantiateDatabase = (obj, metadata) => {
+ const instance = new Database(obj);
+ instance.metadata = metadata;
+ return instance;
+};
+export const instantiateSchema = (obj, metadata) => {
+ const instance = new Schema(obj);
+ instance.metadata = metadata;
+ return instance;
+};
+export const instantiateTable = (obj, metadata) => {
+ const instance = new Table(obj);
+ instance.metadata = metadata;
+ return instance;
+};
// We need a way to distinguish field objects that come from the server
// vs. those that are created client-side to handle lossy transformations between
// Field instances and FieldDimension instances.
// There are scenarios where we are failing to convert FieldDimensions back into Fields,
// and as a safeguard we instantiate a new Field that is missing most of its properties.
-export const instantiateField = obj =>
- new Field({ ...obj, _comesFromEndpoint: true });
-export const instantiateSegment = obj => new Segment(obj);
-export const instantiateMetric = obj => new Metric(obj);
+export const instantiateField = (obj, metadata) => {
+ const instance = new Field({ ...obj, _comesFromEndpoint: true });
+ instance.metadata = metadata;
+ return instance;
+};
+export const instantiateSegment = (obj, metadata) => {
+ const instance = new Segment(obj);
+ instance.metadata = metadata;
+ return instance;
+};
+export const instantiateMetric = (obj, metadata) => {
+ const instance = new Metric(obj);
+ instance.metadata = metadata;
+ return instance;
+};
export const instantiateQuestion = (obj, metadata) =>
new Question(obj, metadata);
@@ -100,7 +123,7 @@ export const getMetadata = createSelector(
meta.databases = copyObjects(meta, databases, instantiateDatabase);
meta.schemas = copyObjects(meta, schemas, instantiateSchema);
meta.tables = copyObjects(meta, tables, instantiateTable);
- meta.fields = copyObjects(meta, fields, instantiateField);
+ meta.fields = copyObjects(meta, fields, instantiateField, "uniqueId");
meta.segments = copyObjects(meta, segments, instantiateSegment);
meta.metrics = copyObjects(meta, metrics, instantiateMetric);
meta.questions = copyObjects(meta, questions, instantiateQuestion);
@@ -197,14 +220,18 @@ export const getSegments = createSelector(
// UTILS:
// clone each object in the provided mapping of objects
-export function copyObjects(metadata, objects, instantiate) {
+export function copyObjects(
+ metadata,
+ objects,
+ instantiate,
+ identifierProp = "id",
+) {
const copies = {};
for (const object of Object.values(objects)) {
- if (object && object.id != null) {
- copies[object.id] = instantiate(object, metadata);
- copies[object.id].metadata = metadata;
+ if (object?.[identifierProp] != null) {
+ copies[object[identifierProp]] = instantiate(object, metadata);
} else {
- console.warn("Missing id:", object);
+ console.warn(`Missing ${identifierProp}:`, object);
}
}
return copies;
diff --git a/frontend/test/__support__/sample_database_fixture.js b/frontend/test/__support__/sample_database_fixture.js
index c0e1db05cf518e7a52cb835536bc5ac2e1ec689d..5e5c25105399e741e4cf699e1dbce16761488aba 100644
--- a/frontend/test/__support__/sample_database_fixture.js
+++ b/frontend/test/__support__/sample_database_fixture.js
@@ -2,9 +2,11 @@
import React from "react";
import { Provider } from "react-redux";
import { getStore } from "metabase/store";
+import { normalize } from "normalizr";
+import { chain } from "icepick";
import { getMetadata } from "metabase/selectors/metadata";
-import { chain } from "icepick";
+import { FieldSchema } from "metabase/schema";
import state from "./sample_database_fixture.json";
export { default as state } from "./sample_database_fixture.json";
@@ -34,8 +36,16 @@ function aliasTablesAndFields(metadata) {
}
}
+function normalizeFields(fields) {
+ return normalize(fields, [FieldSchema]).entities.fields || {};
+}
+
export function createMetadata(updateState = state => state) {
- const stateModified = updateState(chain(state)).value();
+ const stateModified = updateState(chain(state)).thaw().value();
+ stateModified.entities.fields = normalizeFields(
+ stateModified.entities.fields,
+ );
+
const metadata = getMetadata(stateModified);
aliasTablesAndFields(metadata);
return metadata;
@@ -106,6 +116,9 @@ export function makeMetadata(metadata) {
}
}
}
+
+ metadata.fields = normalizeFields(metadata.fields);
+
return getMetadata({ entities: metadata });
}
diff --git a/frontend/test/metabase-lib/lib/Dimension.unit.spec.js b/frontend/test/metabase-lib/lib/Dimension.unit.spec.js
index 1e23c0f625d6afb2d3af250511caa8f728c176b8..9e3f4f27c916e4082d41a5b01be3a5c6d026f147 100644
--- a/frontend/test/metabase-lib/lib/Dimension.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/Dimension.unit.spec.js
@@ -58,56 +58,8 @@ const nestedQuestionCard = {
display: "table",
};
-const cardWithResultMetadata = {
- id: 123,
- dataset: true,
- display: "table",
- visualization_settings: {},
- dataset_query: {
- type: "query",
- database: SAMPLE_DATABASE.id,
- query: {
- "source-table": ORDERS.id,
- },
- },
- result_metadata: [
- {
- id: ORDERS.ID.id,
- display_name: "Foo",
- },
- {
- name: ORDERS.TOTAL.name,
- display_name: "Bar",
- },
- ],
-};
-
const PRODUCT_CATEGORY_FIELD_ID = 21;
-const ORDERS_USER_ID_FIELD = metadata.field(ORDERS.USER_ID.id).getPlainObject();
-
-const OVERWRITTEN_USER_ID_FIELD_METADATA = {
- ...ORDERS_USER_ID_FIELD,
- display_name: "Foo",
- description: "Bar",
- fk_target_field_id: 1,
- semantic_type: "type/Price",
- settings: {
- show_mini_bar: true,
- },
-};
-
-const ORDERS_DATASET = ORDERS.question()
- .setDataset(true)
- .setResultsMetadata({
- columns: [OVERWRITTEN_USER_ID_FIELD_METADATA],
- });
-ORDERS_DATASET.card().id = 111;
-
-// It isn't actually possible to overwrite metadata for non-models,
-// it's just needed to test it's only possible for models
-const ORDERS_WITH_OVERWRITTEN_METADATA = ORDERS_DATASET.setDataset(false);
-
describe("Dimension", () => {
describe("STATIC METHODS", () => {
describe("parseMBQL(mbql metadata)", () => {
@@ -373,32 +325,6 @@ describe("Dimension", () => {
expect(field.id).toEqual(ORDERS.TOTAL.id);
expect(field.base_type).toEqual("type/Float");
});
-
- it("should merge model's field results metadata with field info", () => {
- const dimension = Dimension.parseMBQL(
- ["field", ORDERS.USER_ID.id, null],
- metadata,
- ORDERS_DATASET.query(),
- );
-
- const field = dimension.field();
- const fieldInfo = _.omit(field.getPlainObject(), "metadata", "query");
-
- expect(fieldInfo).toEqual(OVERWRITTEN_USER_ID_FIELD_METADATA);
- });
-
- it("should not merge regular question's field results metadata with field info", () => {
- const dimension = Dimension.parseMBQL(
- ["field", ORDERS.USER_ID.id, null],
- metadata,
- ORDERS_WITH_OVERWRITTEN_METADATA.query(),
- );
-
- const field = dimension.field();
- const fieldInfo = _.omit(field.getPlainObject(), "metadata", "query");
-
- expect(fieldInfo).toEqual(ORDERS_USER_ID_FIELD);
- });
});
});
});
@@ -981,57 +907,6 @@ describe("Dimension", () => {
});
});
- describe("Dimension connected to saved question with result_metadata", () => {
- describe("field", () => {
- it("should return a Field with properties from the field in the question's result_metadata", () => {
- const questionWithResultMetadata = new Question(
- cardWithResultMetadata,
- metadata,
- );
- const fieldDimensionUsingIdProp = Dimension.parseMBQL(
- ["field", ORDERS.ID.id, null],
- metadata,
- questionWithResultMetadata.query(),
- );
-
- const idField = fieldDimensionUsingIdProp.field();
- expect(idField.id).toBe(ORDERS.ID.id);
- expect(idField.display_name).toBe("Foo");
- expect(idField.description).toBe(ORDERS.ID.description);
- });
- });
- });
-
- describe("Dimension connected to query based on nested card with result_metadata", () => {
- describe("field", () => {
- it("should return a Field with properties from the field in the question's result_metadata", () => {
- metadata.questions[cardWithResultMetadata.id] = new Question(
- cardWithResultMetadata,
- metadata,
- );
-
- const questionWithResultMetadata = new Question(
- cardWithResultMetadata,
- metadata,
- );
- const unsavedQuestionBasedOnCard = questionWithResultMetadata
- .composeThisQuery()
- .setResultsMetadata([]);
-
- const fieldDimensionUsingIdProp = Dimension.parseMBQL(
- ["field", ORDERS.ID.id, null],
- metadata,
- unsavedQuestionBasedOnCard.query(),
- );
-
- const idField = fieldDimensionUsingIdProp.field();
- expect(idField.id).toBe(ORDERS.ID.id);
- expect(idField.display_name).toBe("Foo");
- expect(idField.description).toBe(ORDERS.ID.description);
- });
- });
- });
-
describe("TemplateTagDimension", () => {
describe("dimension tag (ie a field filter)", () => {
const templateTagClause = ["template-tag", "foo"];
diff --git a/frontend/test/metabase-lib/lib/Question.unit.spec.js b/frontend/test/metabase-lib/lib/Question.unit.spec.js
index c0b8350343c4e50c8d1513e3ae06e6073fefe873..1e364fd1c3ae77bb562f4019f9503b1fdfebf5b0 100644
--- a/frontend/test/metabase-lib/lib/Question.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/Question.unit.spec.js
@@ -939,7 +939,6 @@ describe("Question", () => {
const question = new Question(
{
...card,
- dataset: true,
result_metadata: [
{ semantic_type: SEMANTIC_TYPE.FK, fk_target_field_id: 5 },
],
@@ -954,7 +953,6 @@ describe("Question", () => {
const question = new Question(
{
...card,
- dataset: true,
result_metadata: [{ fk_target_field_id: 5 }],
},
metadata,
@@ -962,20 +960,6 @@ describe("Question", () => {
expect(question.dependentMetadata()).toEqual([]);
});
-
- it("should return nothing for regular questions", () => {
- const question = new Question(
- {
- ...card,
- result_metadata: [
- { semantic_type: SEMANTIC_TYPE.FK, fk_target_field_id: 5 },
- ],
- },
- metadata,
- );
-
- expect(question.dependentMetadata()).toEqual([]);
- });
});
describe("Question.prototype.setDashboardProps", () => {
diff --git a/frontend/test/metabase/lib/expressions/__support__/expressions.js b/frontend/test/metabase/lib/expressions/__support__/expressions.js
index 9ea1f436ddf3753b718363a67226be66247f953e..60b6b8c1f79f92260fdb5081152772174413cdf1 100644
--- a/frontend/test/metabase/lib/expressions/__support__/expressions.js
+++ b/frontend/test/metabase/lib/expressions/__support__/expressions.js
@@ -30,19 +30,66 @@ const metadata = makeMetadata({
},
},
fields: {
- 1: { table: 1, display_name: "A", base_type: TYPE.Float },
- 2: { table: 1, display_name: "B", base_type: TYPE.Float },
- 3: { table: 1, display_name: "C", base_type: TYPE.Float },
- 10: { table: 1, display_name: "Toucan Sam", base_type: TYPE.Float },
- 11: { table: 1, display_name: "Sum", base_type: TYPE.Float },
- 12: { table: 1, display_name: "count", base_type: TYPE.Float },
- 13: { table: 1, display_name: "text", base_type: TYPE.Text },
- 14: { table: 1, display_name: "date", base_type: TYPE.DateTime },
+ 1: {
+ id: 1,
+ table_id: 1,
+ table: 1,
+ display_name: "A",
+ base_type: TYPE.Float,
+ },
+ 2: {
+ id: 2,
+ table_id: 1,
+ table: 1,
+ display_name: "B",
+ base_type: TYPE.Float,
+ },
+ 3: {
+ id: 3,
+ table_id: 1,
+ table: 1,
+ display_name: "C",
+ base_type: TYPE.Float,
+ },
+ 10: {
+ id: 10,
+ table_id: 1,
+ table: 1,
+ display_name: "Toucan Sam",
+ base_type: TYPE.Float,
+ },
+ 11: {
+ id: 11,
+ table_id: 1,
+ table: 1,
+ display_name: "Sum",
+ base_type: TYPE.Float,
+ },
+ 12: {
+ id: 12,
+ table_id: 1,
+ table: 1,
+ display_name: "count",
+ base_type: TYPE.Float,
+ },
+ 13: {
+ id: 13,
+ table_id: 1,
+ table: 1,
+ display_name: "text",
+ base_type: TYPE.Text,
+ },
+ 14: {
+ id: 14,
+ table_id: 1,
+ table: 1,
+ display_name: "date",
+ base_type: TYPE.DateTime,
+ },
},
});
export const query = metadata.table(1).query();
-
export const expressionOpts = { query, startRule: "expression" };
export const aggregationOpts = { query, startRule: "aggregation" };
export const filterOpts = { query, startRule: "boolean" };
diff --git a/frontend/test/metabase/scenarios/joins/joins.cy.spec.js b/frontend/test/metabase/scenarios/joins/joins.cy.spec.js
index 0b83af2630bb7196357ac9698cae2b20427139ce..e1b4a5557b5746360eadd6c71676b9600743e8bf 100644
--- a/frontend/test/metabase/scenarios/joins/joins.cy.spec.js
+++ b/frontend/test/metabase/scenarios/joins/joins.cy.spec.js
@@ -188,7 +188,7 @@ describe("scenarios > question > joined questions", () => {
cy.icon("add_data").click();
enterCustomColumnDetails({
- formula: "[Question 5 → Sum of Rating] / [Sum of Rating]",
+ formula: "[Question 5 → Sum of Rating] / [Sum of Total]",
name: "Sum Divide",
});
diff --git a/frontend/test/metabase/scenarios/models/helpers/e2e-models-helpers.js b/frontend/test/metabase/scenarios/models/helpers/e2e-models-helpers.js
index 2a4d68155484b8ccfe4c654ec2ef01ec2dc7eab6..60191042e9dfe050d2611ff2b52343fa8756e2db 100644
--- a/frontend/test/metabase/scenarios/models/helpers/e2e-models-helpers.js
+++ b/frontend/test/metabase/scenarios/models/helpers/e2e-models-helpers.js
@@ -84,9 +84,9 @@ export function assertIsQuestion() {
export function turnIntoModel() {
interceptIfNotPreviouslyDefined({
- method: "POST",
- url: "/api/dataset",
- alias: "dataset",
+ method: "PUT",
+ url: "/api/card/*",
+ alias: "cardUpdate",
});
openQuestionActions();
@@ -96,7 +96,7 @@ export function turnIntoModel() {
modal().within(() => {
cy.button("Turn this into a model").click();
});
- cy.wait("@dataset");
+ cy.wait("@cardUpdate");
}
export function selectFromDropdown(option, clickOpts) {
diff --git a/frontend/test/metabase/scenarios/models/models-with-aggregation-and-breakout.cy.spec.js b/frontend/test/metabase/scenarios/models/models-with-aggregation-and-breakout.cy.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..daba298a815c80626266e66bc01e4720c6d54856
--- /dev/null
+++ b/frontend/test/metabase/scenarios/models/models-with-aggregation-and-breakout.cy.spec.js
@@ -0,0 +1,37 @@
+import { restore } from "__support__/e2e/helpers";
+
+import { turnIntoModel } from "./helpers/e2e-models-helpers";
+import { SAMPLE_DATABASE } from "__support__/e2e/cypress_sample_database";
+const { ORDERS, ORDERS_ID } = SAMPLE_DATABASE;
+
+describe("scenarios > models with aggregation and breakout", () => {
+ beforeEach(() => {
+ restore();
+ cy.signInAsAdmin();
+ cy.intercept("POST", "/api/dataset").as("dataset");
+ cy.intercept("PUT", "/api/card/*").as("updateCard");
+
+ cy.createQuestion(
+ {
+ name: "model with aggregation & breakout",
+ display: "line",
+ query: {
+ "source-table": ORDERS_ID,
+ aggregation: [["distinct", ["field", ORDERS.PRODUCT_ID, null]]],
+ breakout: [
+ ["field", ORDERS.CREATED_AT, { "temporal-unit": "month" }],
+ ],
+ },
+ },
+ { visitQuestion: true },
+ );
+ });
+
+ it("should be possible to convert a question with an aggregation and breakout into a model", () => {
+ turnIntoModel();
+ cy.wait("@updateCard");
+
+ cy.findByText("Created At: Month");
+ cy.findByText("Distinct values of Product ID");
+ });
+});
diff --git a/frontend/test/metabase/scenarios/models/reproductions/23449-remapped-custom-value.cy.spec.js b/frontend/test/metabase/scenarios/models/reproductions/23449-remapped-custom-value.cy.spec.js
index 794c6404bbdce3ff67f539d4ba106933a2a10f50..410ca85db15b4819254837b4f73a9e3c0dc5a624 100644
--- a/frontend/test/metabase/scenarios/models/reproductions/23449-remapped-custom-value.cy.spec.js
+++ b/frontend/test/metabase/scenarios/models/reproductions/23449-remapped-custom-value.cy.spec.js
@@ -5,7 +5,7 @@ const { REVIEWS, REVIEWS_ID } = SAMPLE_DATABASE;
const questionDetails = { query: { "source-table": REVIEWS_ID, limit: 2 } };
-describe.skip("issue 23449", () => {
+describe("issue 23449", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
@@ -36,13 +36,13 @@ describe.skip("issue 23449", () => {
});
function turnIntoModel() {
- cy.intercept("POST", "/api/dataset").as("dataset");
+ cy.intercept("PUT", "/api/card/*").as("cardUpdate");
openQuestionActions();
cy.findByText("Turn into a model").click();
cy.findByText("Turn this into a model").click();
- cy.wait("@dataset").then(({ response }) => {
+ cy.wait("@cardUpdate").then(({ response }) => {
expect(response.body.error).to.not.exist;
});
}