diff --git a/frontend/src/metabase-lib/lib/metadata/Table.ts b/frontend/src/metabase-lib/lib/metadata/Table.ts index 65fd8547a89590af36756c1532ff360b8eeea041..d47904d0c83cc2767d69d22e0959a07bd8cbd19c 100644 --- a/frontend/src/metabase-lib/lib/metadata/Table.ts +++ b/frontend/src/metabase-lib/lib/metadata/Table.ts @@ -6,12 +6,14 @@ import Question from "../Question"; // eslint-disable-line import/order import { singularize } from "metabase/lib/formatting"; import { getAggregationOperators } from "metabase/lib/schema_metadata"; import type { TableId } from "metabase-types/types/Table"; +import { isVirtualCardId } from "metabase/lib/saved-questions/saved-questions"; import { createLookupByProperty, memoizeClass } from "metabase-lib/lib/utils"; import Base from "./Base"; import type Metadata from "./Metadata"; import type Schema from "./Schema"; import type Field from "./Field"; import type Database from "./Database"; + /** * @typedef { import("./metadata").SchemaName } SchemaName * @typedef { import("./metadata").EntityType } EntityType @@ -33,6 +35,10 @@ class TableInner extends Base { metadata?: Metadata; db?: Database | undefined | null; + isVirtualCard() { + return isVirtualCardId(this.id); + } + hasSchema() { return (this.schema_name && this.db && this.db.schemas.length > 1) || false; } 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 deaa3f030d3eee708419dfa130fca67eb6a5e905..47eb07f20016d0532682a13932cdad624119cd78 100644 --- a/frontend/src/metabase-lib/lib/metadata/Table.unit.spec.ts +++ b/frontend/src/metabase-lib/lib/metadata/Table.unit.spec.ts @@ -1,6 +1,5 @@ import { PRODUCTS } from "__support__/sample_database_fixture"; import Table from "./Table"; -import type Field from "./Field"; describe("Table", () => { const productsTable = new Table(PRODUCTS); @@ -29,4 +28,17 @@ describe("Table", () => { expect(table.connectedTables()).toEqual([productsTable]); }); }); + + describe("isVirtualCard", () => { + it("should return false when the Table is not a virtual card table", () => { + expect(productsTable.isVirtualCard()).toBe(false); + }); + + it("should return true when the Table is a virtual card table", () => { + const virtualCardTable = new Table({ + id: "card__123", + }); + expect(virtualCardTable.isVirtualCard()).toBe(true); + }); + }); }); diff --git a/frontend/src/metabase-lib/lib/queries/StructuredQuery.ts b/frontend/src/metabase-lib/lib/queries/StructuredQuery.ts index ea84ff805cf84a8c57df90b7df6db4a399c3243b..57f198f3c15c72cf54beda88717dd8184026d6f2 100644 --- a/frontend/src/metabase-lib/lib/queries/StructuredQuery.ts +++ b/frontend/src/metabase-lib/lib/queries/StructuredQuery.ts @@ -1270,7 +1270,16 @@ class StructuredQueryInner extends AtomicQuery { for (const dimension of fkDimensions) { const field = dimension.field(); - if (field && explicitJoins.has(this._keyForFK(field, field.target))) { + const queryHasExplicitJoin = + field && explicitJoins.has(this._keyForFK(field, field.target)); + const isNestedCardTable = table?.isVirtualCard(); + const tableHasExplicitJoin = + isNestedCardTable && + table.fields.find( + tableField => tableField.id === field.fk_target_field_id, + ); + + if (queryHasExplicitJoin || tableHasExplicitJoin) { continue; } diff --git a/frontend/src/metabase/parameters/utils/mapping-options.js b/frontend/src/metabase/parameters/utils/mapping-options.js index 1050a200d8d0171876c39e80d2e4aa796fc8c468..e6d3061166784207625b5098b3bbefd984f6a0cf 100644 --- a/frontend/src/metabase/parameters/utils/mapping-options.js +++ b/frontend/src/metabase/parameters/utils/mapping-options.js @@ -72,8 +72,19 @@ export function getParameterMappingOptions( const question = new Question(card, metadata); const query = question.query(); const options = []; - - if (question.isStructured()) { + if (question.isDataset()) { + // treat the dataset/model question like it is already composed so that we can apply + // dataset/model-specific metadata to the underlying dimension options + const composedDatasetQuery = question.composeDataset().query(); + options.push( + ...composedDatasetQuery + .dimensionOptions( + parameter ? dimensionFilterForParameter(parameter) : undefined, + ) + .sections() + .flatMap(section => buildStructuredQuerySectionOptions(section)), + ); + } else if (question.isStructured()) { options.push( ...query .dimensionOptions( 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 e9300852aa31609e9d99867dafa1b5bb9900204c..639ef15204318eeb37011eb94ffd085bc2363b60 100644 --- a/frontend/src/metabase/parameters/utils/mapping-options.unit.spec.js +++ b/frontend/src/metabase/parameters/utils/mapping-options.unit.spec.js @@ -18,6 +18,54 @@ function native(native) { describe("parameters/utils/mapping-options", () => { describe("getParameterMappingOptions", () => { + describe("Model question", () => { + let dataset; + let virtualCardTable; + beforeEach(() => { + const question = ORDERS.question(); + dataset = question + .setCard({ ...question.card(), id: 123 }) + .setDataset(true); + + // create a virtual table for the card + // that contains fields with custom, model-specific metadata + virtualCardTable = ORDERS.clone(); + virtualCardTable.id = `card__123`; + virtualCardTable.fields = [ + ORDERS.CREATED_AT.clone({ + table_id: `card__123`, + uniqueId: `card__123:${ORDERS.CREATED_AT.id}`, + display_name: "~*~Created At~*~", + }), + ]; + + // add instances to the `metadata` instance + metadata.questions[dataset.id()] = dataset; + metadata.tables[virtualCardTable.id] = virtualCardTable; + virtualCardTable.fields.forEach(f => { + metadata.fields[f.uniqueId] = f; + }); + }); + + it("should return fields from the model question's virtual card table, as though it is already nested", () => { + const options = getParameterMappingOptions( + metadata, + { type: "date/single" }, + dataset.card(), + ); + + expect(options).toEqual([ + { + icon: "calendar", + isForeign: false, + name: "~*~Created At~*~", + sectionName: "Order", + target: ["dimension", ["field", 1, null]], + }, + ]); + }); + }); + describe("Structured Query", () => { it("should return field-id and fk-> dimensions", () => { const options = getParameterMappingOptions( diff --git a/frontend/test/metabase-lib/lib/queries/StructuredQuery-nesting.unit.spec.js b/frontend/test/metabase-lib/lib/queries/StructuredQuery-nesting.unit.spec.js index a87f5c911684c2dcb9a6c13840f13a873124fac3..ba838d929421f0e7998067e6516ff0988f5c4df8 100644 --- a/frontend/test/metabase-lib/lib/queries/StructuredQuery-nesting.unit.spec.js +++ b/frontend/test/metabase-lib/lib/queries/StructuredQuery-nesting.unit.spec.js @@ -1,4 +1,9 @@ -import { ORDERS } from "__support__/sample_database_fixture"; +import { + ORDERS, + PRODUCTS, + PEOPLE, + metadata, +} from "__support__/sample_database_fixture"; describe("StructuredQuery nesting", () => { describe("nest", () => { @@ -130,4 +135,77 @@ describe("StructuredQuery nesting", () => { expect(d.mbql()).toEqual(["field", ORDERS.TOTAL.id, null]); }); }); + + describe("model question", () => { + let dataset; + let virtualCardTable; + beforeEach(() => { + const question = ORDERS.question(); + dataset = question + .setCard({ ...question.card(), id: 123 }) + .setDataset(true); + + // create a virtual table for the card + // that contains fields from both Orders and Products tables + // to imitate an explicit join of Products to Orders + virtualCardTable = ORDERS.clone(); + virtualCardTable.id = `card__123`; + virtualCardTable.fields = virtualCardTable.fields + .map(f => + f.clone({ + table_id: `card__123`, + uniqueId: `card__123:${f.id}`, + }), + ) + .concat( + PRODUCTS.fields.map(f => { + const field = f.clone({ + table_id: `card__123`, + uniqueId: `card__123:${f.id}`, + }); + + return field; + }), + ); + + // add instances to the `metadata` instance + metadata.questions[dataset.id()] = dataset; + metadata.tables[virtualCardTable.id] = virtualCardTable; + virtualCardTable.fields.forEach(f => { + metadata.fields[f.uniqueId] = f; + }); + }); + + it("should not include implicit join dimensions when the underyling question has an explicit join", () => { + const nestedDatasetQuery = dataset.composeDataset().query(); + expect( + // get a list of all dimension options for the nested query + nestedDatasetQuery + .dimensionOptions() + .all() + .map(d => d.field().getPlainObject()), + ).toEqual([ + // Order fields + ...ORDERS.fields.map(f => + f + .clone({ + table_id: `card__123`, + uniqueId: `card__123:${f.id}`, + }) + .getPlainObject(), + ), + // Product fields from the explicit join + ...PRODUCTS.fields.map(f => + f + .clone({ + table_id: `card__123`, + uniqueId: `card__123:${f.id}`, + }) + .getPlainObject(), + ), + // People fields from the implicit join + ...PEOPLE.fields.map(f => f.getPlainObject()), + ]); + }); + }); });