diff --git a/frontend/src/metabase-lib/metadata/Metric.ts b/frontend/src/metabase-lib/metadata/Metric.ts index 77f5bfabdd4b40a1f326088b2d92dd9d27a08a80..735ee9ed896cfe611f174b2d82f4b993914ab28b 100644 --- a/frontend/src/metabase-lib/metadata/Metric.ts +++ b/frontend/src/metabase-lib/metadata/Metric.ts @@ -48,7 +48,7 @@ class Metric { } /** Column name when this metric is used in a query */ - columnName() { + columnName(): string | null { const aggregation = this.aggregation(); if (aggregation) { diff --git a/frontend/src/metabase-lib/metadata/Table.ts b/frontend/src/metabase-lib/metadata/Table.ts index d81cf75ec92e8eff57ab436e55ac597e036b0bea..181c8b16247568044da1fe6de769633d39ad48b7 100644 --- a/frontend/src/metabase-lib/metadata/Table.ts +++ b/frontend/src/metabase-lib/metadata/Table.ts @@ -1,19 +1,11 @@ -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-nocheck - +import _ from "underscore"; // NOTE: this needs to be imported first due to some cyclical dependency nonsense import Question from "../Question"; // eslint-disable-line import/order import { singularize } from "metabase/lib/formatting"; -import type { - Table as ITable, - TableFieldOrder, - TableId, - TableVisibilityType, -} from "metabase-types/api"; +import type { NormalizedTable } from "metabase-types/api"; import { isVirtualCardId } from "metabase-lib/metadata/utils/saved-questions"; import { getAggregationOperators } from "metabase-lib/operators/utils"; -import { createLookupByProperty, memoizeClass } from "metabase-lib/utils"; -import Base from "./Base"; +import StructuredQuery from "metabase-lib/queries/StructuredQuery"; import type Metadata from "./Metadata"; import type Schema from "./Schema"; import type Field from "./Field"; @@ -21,41 +13,48 @@ import type Database from "./Database"; import type Metric from "./Metric"; import type Segment from "./Segment"; -/** - * @typedef { import("./Metadata").SchemaName } SchemaName - * @typedef { import("./Metadata").EntityType } EntityType - * @typedef { import("./Metadata").StructuredQuery } StructuredQuery - */ - -/** This is the primary way people interact with tables */ - -class TableInner extends Base { - id: TableId; - name: string; - description?: string; - fks?: any[]; +interface Table + extends Omit< + NormalizedTable, + "db" | "schema" | "fields" | "segments" | "metrics" + > { + db?: Database; schema?: Schema; - display_name: string; - schema_name: string; - db_id: number; - fields: Field[]; - field_order: TableFieldOrder; - metrics: Metric[]; - segments: Segment[]; + fields?: Field[]; + segments?: Segment[]; + metrics?: Metric[]; metadata?: Metadata; - db?: Database | undefined | null; - visibility_type: TableVisibilityType; +} + +class Table { + private readonly _plainObject: NormalizedTable; + + constructor(table: NormalizedTable) { + this._plainObject = table; + this.aggregationOperators = _.memoize(this.aggregationOperators); + this.aggregationOperatorsLookup = _.memoize( + this.aggregationOperatorsLookup, + ); + this.fieldsLookup = _.memoize(this.fieldsLookup); + Object.assign(this, table); + } - getPlainObject(): ITable { + getPlainObject(): NormalizedTable { return this._plainObject; } + getFields() { + return this.fields ?? []; + } + isVirtualCard() { return isVirtualCardId(this.id); } hasSchema() { - return (this.schema_name && this.db && this.db.schemas.length > 1) || false; + return ( + (this.schema_name && this.db && this.db.getSchemas().length > 1) || false + ); } // Could be replaced with hydrated database property in selectors/metadata.js (instead / in addition to `table.db`) @@ -84,20 +83,18 @@ class TableInner extends Base { return match ? parseInt(match[1]) : null; } - /** - * @returns {StructuredQuery} - */ query(query = {}) { - return this.question() - .query() - .updateQuery(q => ({ ...q, ...query })); + return (this.question().query() as StructuredQuery).updateQuery(q => ({ + ...q, + ...query, + })); } dimensions() { - return this.fields.map(field => field.dimension()); + return this.getFields().map(field => field.dimension()); } - displayName({ includeSchema } = {}) { + displayName({ includeSchema }: { includeSchema?: boolean } = {}) { return ( (includeSchema && this.schema ? this.schema.displayName() + "." : "") + this.display_name @@ -114,7 +111,7 @@ class TableInner extends Base { } dateFields() { - return this.fields.filter(field => field.isDate()); + return this.getFields().filter(field => field.isDate()); } // AGGREGATIONS @@ -123,16 +120,18 @@ class TableInner extends Base { } aggregationOperatorsLookup() { - return createLookupByProperty(this.aggregationOperators(), "short"); + return Object.fromEntries( + this.aggregationOperators().map(op => [op.short, op]), + ); } - aggregationOperator(short) { + aggregationOperator(short: string) { return this.aggregationOperatorsLookup()[short]; } // FIELDS fieldsLookup() { - return createLookupByProperty(this.fields, "id"); + return Object.fromEntries(this.getFields().map(field => [field.id, field])); } // @deprecated: use fieldsLookup @@ -146,22 +145,25 @@ class TableInner extends Base { connectedTables(): Table[] { const fks = this.fks || []; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore return fks.map(fk => new Table(fk.origin.table)); } foreignTables(): Table[] { - if (!Array.isArray(this.fields)) { + const fields = this.getFields(); + if (!fields) { return []; } - return this.fields + return fields .filter(field => field.isFK() && field.fk_target_field_id) - .map(field => this.metadata.field(field.fk_target_field_id)?.table) - .filter(Boolean); + .map(field => this.metadata?.field(field.fk_target_field_id)?.table) + .filter(Boolean) as Table[]; } primaryKeys(): { field: Field; index: number }[] { - const pks = []; - this.fields.forEach((field, index) => { + const pks: { field: Field; index: number }[] = []; + this.getFields().forEach((field, index) => { if (field.isPK()) { pks.push({ field, index }); } @@ -170,16 +172,11 @@ class TableInner extends Base { } clone() { - const plainObject = this.getPlainObject(); - const newTable = new Table(this); - newTable._plainObject = plainObject; - return newTable; + const table = new Table(this.getPlainObject()); + Object.assign(table, this); + return table; } } // eslint-disable-next-line import/no-default-export -- deprecated usage -export default class Table extends memoizeClass<TableInner>( - "aggregationOperators", - "aggregationOperatorsLookup", - "fieldsLookup", -)(TableInner) {} +export default Table; diff --git a/frontend/src/metabase-lib/metadata/Table.unit.spec.ts b/frontend/src/metabase-lib/metadata/Table.unit.spec.ts index 47eb07f20016d0532682a13932cdad624119cd78..42ff484e439fd0bc628e5e765ad2fea6fea1e360 100644 --- a/frontend/src/metabase-lib/metadata/Table.unit.spec.ts +++ b/frontend/src/metabase-lib/metadata/Table.unit.spec.ts @@ -1,3 +1,5 @@ +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-nocheck import { PRODUCTS } from "__support__/sample_database_fixture"; import Table from "./Table"; diff --git a/frontend/src/metabase-lib/queries/utils/native-query-table.unit.spec.ts b/frontend/src/metabase-lib/queries/utils/native-query-table.unit.spec.ts index 90b066fc06ec14bd745cdd19b78a27d8751ad0e7..0db673354e3e70872cf1423e767cdb3da99685bd 100644 --- a/frontend/src/metabase-lib/queries/utils/native-query-table.unit.spec.ts +++ b/frontend/src/metabase-lib/queries/utils/native-query-table.unit.spec.ts @@ -1,3 +1,5 @@ +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-nocheck import { merge } from "icepick"; import { diff --git a/frontend/src/metabase-lib/queries/utils/nested-card-query-table.ts b/frontend/src/metabase-lib/queries/utils/nested-card-query-table.ts index be42ed5f23326c92f1b1bae9076611cdde58445f..d0c85cd1629d190a46e58ec093818a91d10f4d66 100644 --- a/frontend/src/metabase-lib/queries/utils/nested-card-query-table.ts +++ b/frontend/src/metabase-lib/queries/utils/nested-card-query-table.ts @@ -63,7 +63,7 @@ function createVirtualTableUsingQuestionMetadata(question: Question): Table { id: getQuestionVirtualTableId(question.id()), name: questionDisplayName, display_name: questionDisplayName, - db: question?.database(), + db: question?.database() ?? undefined, fields, metadata, }); diff --git a/frontend/src/metabase-lib/queries/utils/structured-query-table.ts b/frontend/src/metabase-lib/queries/utils/structured-query-table.ts index 446238a922f412c8d5ac74b2001c529b05be4003..909eb12b1609a2cf8dbc19b86d5efc9f454fc8f3 100644 --- a/frontend/src/metabase-lib/queries/utils/structured-query-table.ts +++ b/frontend/src/metabase-lib/queries/utils/structured-query-table.ts @@ -66,7 +66,7 @@ function getSourceQueryTable(query: StructuredQuery): Table { return createVirtualTable({ id: sourceTableId, - db: sourceQuery.database(), + db: sourceQuery.database() ?? undefined, fields, metadata: sourceQuery.metadata(), // intentionally set these to "" so that we fallback to a title of "Previous results" in join steps diff --git a/frontend/src/metabase-lib/queries/utils/structured-query-table.unit.spec.ts b/frontend/src/metabase-lib/queries/utils/structured-query-table.unit.spec.ts index bf8613e276604569f15fbc40bdda36f920095c9c..c9612cfaed950badf35d89609170414e9a564256 100644 --- a/frontend/src/metabase-lib/queries/utils/structured-query-table.unit.spec.ts +++ b/frontend/src/metabase-lib/queries/utils/structured-query-table.unit.spec.ts @@ -1,3 +1,5 @@ +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-nocheck import _ from "underscore"; import { diff --git a/frontend/src/metabase-lib/queries/utils/virtual-table.ts b/frontend/src/metabase-lib/queries/utils/virtual-table.ts index e7413227d325325c18d49315c0549fa0b2e06758..2ea898b3809084ca99d2ae0fd0e64f825f5b6294 100644 --- a/frontend/src/metabase-lib/queries/utils/virtual-table.ts +++ b/frontend/src/metabase-lib/queries/utils/virtual-table.ts @@ -23,6 +23,8 @@ export function createVirtualTable({ db, ...rest }: VirtualTableProps): Table { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore const table = new Table({ name: "", display_name: "", diff --git a/frontend/src/metabase-lib/queries/utils/virtual-table.unit.spec.ts b/frontend/src/metabase-lib/queries/utils/virtual-table.unit.spec.ts index 43b88d04458dfd65e1c1cb607fefb45972d7134e..1645f128bacd78de40e258729b0629c444281a5a 100644 --- a/frontend/src/metabase-lib/queries/utils/virtual-table.unit.spec.ts +++ b/frontend/src/metabase-lib/queries/utils/virtual-table.unit.spec.ts @@ -62,8 +62,8 @@ describe("metabase-lib/queries/utils/virtual-table", () => { }); 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( + 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 9257907c83df8d1545b1dbde0894ee5171d9c206..a77176cec75609de11539229502c7475c0064477 100644 --- a/frontend/src/metabase-types/api/field.ts +++ b/frontend/src/metabase-types/api/field.ts @@ -1,5 +1,5 @@ import { RowValue } from "./dataset"; -import { TableId } from "./table"; +import { Table, TableId } from "./table"; export type FieldId = number; @@ -66,6 +66,7 @@ export type FieldDimensionOption = { export interface ConcreteField { id: FieldId; table_id: TableId; + table?: Table; name: string; display_name: string; diff --git a/frontend/src/metabase-types/api/foreign-key.ts b/frontend/src/metabase-types/api/foreign-key.ts deleted file mode 100644 index 1aef56eb5fc1f7bb2e255b50973ea9e47666f6cb..0000000000000000000000000000000000000000 --- a/frontend/src/metabase-types/api/foreign-key.ts +++ /dev/null @@ -1,14 +0,0 @@ -import type { ConcreteField } from "./field"; -import type { Table } from "./table"; - -type ForeignKeyField = ConcreteField & { - table: Table; -}; - -export interface ForeignKey { - destination: ForeignKeyField; - destination_id: number; - origin: ForeignKeyField; - origin_id: number; - relationship: string; // enum? -} diff --git a/frontend/src/metabase-types/api/index.ts b/frontend/src/metabase-types/api/index.ts index 5cba3cd9d21d23e1471e70d9641bd852b3133bc0..5289dcaef1257f0181298291fd16389e880a6666 100644 --- a/frontend/src/metabase-types/api/index.ts +++ b/frontend/src/metabase-types/api/index.ts @@ -10,7 +10,6 @@ export * from "./dashboard"; export * from "./database"; export * from "./dataset"; export * from "./field"; -export * from "./foreign-key"; export * from "./group"; export * from "./metabot"; export * from "./metric"; diff --git a/frontend/src/metabase-types/api/mocks/table.ts b/frontend/src/metabase-types/api/mocks/table.ts index 3911b7dd51145a776a600ee9d56b678db5bef5e4..c3cac0af3f4dd097ab91afa348568138df5ccafb 100644 --- a/frontend/src/metabase-types/api/mocks/table.ts +++ b/frontend/src/metabase-types/api/mocks/table.ts @@ -1,4 +1,5 @@ -import { Table, Schema } from "metabase-types/api"; +import { Table, Schema, ForeignKey } from "metabase-types/api"; +import { createMockField } from "metabase-types/api/mocks/field"; export const createMockTable = (opts?: Partial<Table>): Table => { return { @@ -21,3 +22,14 @@ export const createMockSchema = (opts?: Partial<Schema>): Schema => ({ name: "Schema 1", ...opts, }); + +export const createMockForeignKey = ( + opts?: Partial<ForeignKey>, +): ForeignKey => ({ + origin: createMockField({ id: 1 }), + origin_id: 1, + destination: createMockField({ id: 2 }), + destination_id: 2, + relationship: "", + ...opts, +}); diff --git a/frontend/src/metabase-types/api/table.ts b/frontend/src/metabase-types/api/table.ts index 96f49f52dd780552839d8e133d35c1b890da7809..190141f6454e08a5cb923c46732ed3e420568074 100644 --- a/frontend/src/metabase-types/api/table.ts +++ b/frontend/src/metabase-types/api/table.ts @@ -1,5 +1,4 @@ import type { Database, DatabaseId, InitialSyncStatus } from "./database"; -import type { ForeignKey } from "./foreign-key"; import type { Field, FieldDimensionOption } from "./field"; import type { Metric } from "./metric"; import type { Segment } from "./segment"; @@ -71,3 +70,11 @@ export interface TableListQuery { include_hidden?: boolean; include_editable_data_model?: boolean; } + +export interface ForeignKey { + origin: Field; + origin_id: number; + destination: Field; + destination_id: number; + relationship: string; // enum? +} diff --git a/frontend/src/metabase/components/MetadataInfo/TableInfo/ColumnCount.unit.spec.tsx b/frontend/src/metabase/components/MetadataInfo/TableInfo/ColumnCount.unit.spec.tsx index 246703d1edb6c1c6f09d11ecb97df4ae14ca8cce..6b8c0d70585a597238683a5ce971441688ebca01 100644 --- a/frontend/src/metabase/components/MetadataInfo/TableInfo/ColumnCount.unit.spec.tsx +++ b/frontend/src/metabase/components/MetadataInfo/TableInfo/ColumnCount.unit.spec.tsx @@ -1,32 +1,53 @@ import React from "react"; -import { render, screen } from "@testing-library/react"; +import { checkNotNull } from "metabase/core/utils/types"; +import { getMetadata } from "metabase/selectors/metadata"; +import { Table } from "metabase-types/api"; +import { createMockField, createMockTable } from "metabase-types/api/mocks"; +import { createMockState } from "metabase-types/store/mocks"; +import { createMockEntitiesState } from "__support__/store"; +import { renderWithProviders, screen } from "__support__/ui"; +import ColumnCount from "./ColumnCount"; -import Table from "metabase-lib/metadata/Table"; +interface SetupOpts { + table: Table; +} -import ColumnCount from "./ColumnCount"; +function setup({ table }: SetupOpts) { + const state = createMockState({ + entities: createMockEntitiesState({ + tables: [table], + }), + }); + const metadata = getMetadata(state); -function setup(table: Table) { - return render(<ColumnCount table={table} />); + renderWithProviders( + <ColumnCount table={checkNotNull(metadata.table(table.id))} />, + ); } describe("ColumnCount", () => { it("should show a non-plural label for a table with a single field", () => { - const table = new Table({ fields: [{}] }); - setup(table); + setup({ + table: createMockTable({ + fields: [createMockField()], + }), + }); expect(screen.getByText("1 column")).toBeInTheDocument(); }); it("should show a plural label for a table with multiple fields", () => { - const table = new Table({ fields: [{}, {}] }); - setup(table); + setup({ + table: createMockTable({ + fields: [createMockField(), createMockField()], + }), + }); expect(screen.getByText("2 columns")).toBeInTheDocument(); }); it("should handle a scenario where a table has no fields property", () => { - const table = new Table({ id: 123, display_name: "Foo" }); - setup(table); + setup({ table: createMockTable() }); expect(screen.getByText("0 columns")).toBeInTheDocument(); }); diff --git a/frontend/src/metabase/components/MetadataInfo/TableInfo/ConnectedTables.unit.spec.tsx b/frontend/src/metabase/components/MetadataInfo/TableInfo/ConnectedTables.unit.spec.tsx index f0bf6beee936ee8336cd1c6761b50e5e72685fbb..715c5a25573c647ce7ec134de1f7d619806db92a 100644 --- a/frontend/src/metabase/components/MetadataInfo/TableInfo/ConnectedTables.unit.spec.tsx +++ b/frontend/src/metabase/components/MetadataInfo/TableInfo/ConnectedTables.unit.spec.tsx @@ -1,46 +1,67 @@ import React from "react"; -import { render, screen } from "@testing-library/react"; +import { checkNotNull } from "metabase/core/utils/types"; +import { Table } from "metabase-types/api"; +import { + createMockField, + createMockForeignKey, + createMockTable, +} from "metabase-types/api/mocks"; +import { createMockState } from "metabase-types/store/mocks"; +import { createMockEntitiesState } from "__support__/store"; +import { getMetadata } from "metabase/selectors/metadata"; +import { renderWithProviders, screen } from "__support__/ui"; +import ConnectedTables from "./ConnectedTables"; -import Table from "metabase-lib/metadata/Table"; +const EMPTY_TABLE = createMockTable(); -import ConnectedTables from "./ConnectedTables"; +const TABLE_WITH_FKS = createMockTable({ + id: 1, + fks: [ + createMockForeignKey({ + origin: createMockField({ + table: createMockTable({ + id: 2, + display_name: "Foo", + }), + }), + }), + createMockForeignKey({ + origin: createMockField({ + table: createMockTable({ + id: 3, + display_name: "Bar", + }), + }), + }), + ], +}); + +interface SetupOpts { + table: Table; +} + +function setup({ table }: SetupOpts) { + const state = createMockState({ + entities: createMockEntitiesState({ + tables: [table], + }), + }); + const metadata = getMetadata(state); -function setup(table: Table) { - return render(<ConnectedTables table={table} />); + return renderWithProviders( + <ConnectedTables table={checkNotNull(metadata.table(table.id))} />, + ); } describe("ConnectedTables", () => { it("should show nothing when the table has no fks", () => { - const table = new Table(); - const { container } = setup(table); + const { container } = setup({ table: EMPTY_TABLE }); expect(container).toBeEmptyDOMElement(); }); it("should show a label for each connected table", () => { - const table = new Table({ - id: 1, - fks: [ - { - origin: { - table: { - id: 2, - display_name: "Foo", - }, - }, - }, - { - origin: { - table: { - id: 3, - display_name: "Bar", - }, - }, - }, - ], - }); - - setup(table); + setup({ table: TABLE_WITH_FKS }); expect(screen.getByText("Foo")).toBeInTheDocument(); expect(screen.getByText("Bar")).toBeInTheDocument(); diff --git a/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.unit.spec.tsx b/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.unit.spec.tsx index 43ca84013ede3f50882fcd4434b8a173aa5e9223..cbf96345d4c4a4cb49cd1cc9cfb336d6544ba213 100644 --- a/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.unit.spec.tsx +++ b/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.unit.spec.tsx @@ -1,98 +1,135 @@ import React from "react"; -import { render, screen } from "@testing-library/react"; +import { getMetadata } from "metabase/selectors/metadata"; +import { TableId, Table } from "metabase-types/api"; +import { + createMockField, + createMockForeignKey, + createMockTable, +} from "metabase-types/api/mocks"; +import { createMockState } from "metabase-types/store/mocks"; +import { createMockEntitiesState } from "__support__/store"; +import { renderWithProviders, screen } from "__support__/ui"; +import { TableInfo } from "./TableInfo"; -import { PRODUCTS } from "__support__/sample_database_fixture"; -import Table from "metabase-lib/metadata/Table"; +const TABLE_ID = 1; -import { TableInfo } from "./TableInfo"; +const TABLE_FK = createMockForeignKey({ + origin: createMockField({ + table: createMockTable({ + id: 111, + db_id: 222, + display_name: "Connected Table", + }), + }), +}); + +const TABLE_FIELD = createMockField({ + table_id: TABLE_ID, +}); + +const TABLE = createMockTable({ + id: TABLE_ID, + fks: [TABLE_FK], + fields: [TABLE_FIELD], + description: "Description", +}); + +const TABLE_WITH_FKS = createMockTable({ + id: TABLE_ID, + fks: [TABLE_FK], + fields: undefined, +}); -const productsTable = new Table({ - ...PRODUCTS, - fields: [1, 2, 3], - fks: [ - { - origin: { - table: { - id: 111, - db_id: 222, - display_name: "Connected Table", - }, - }, - }, - ], +const TABLE_WITH_FIELDS = createMockTable({ + id: TABLE_ID, + fks: undefined, + fields: [TABLE_FIELD], }); -const tableWithoutDescription = new Table({ - id: 123, - display_name: "Foo", - fields: [], + +const TABLE_WITHOUT_DESCRIPTION = createMockTable({ + id: TABLE_ID, + description: null, }); -const fetchForeignKeys = jest.fn(); -const fetchMetadata = jest.fn(); +interface SetupOpts { + id: TableId; + table?: Table; +} + +function setup({ id, table }: SetupOpts) { + const state = createMockState({ + entities: createMockEntitiesState({ + tables: table ? [table] : [], + }), + }); + const metadata = getMetadata(state); -function setup({ id, table }: { table: Table | undefined; id: Table["id"] }) { - fetchForeignKeys.mockReset(); - fetchMetadata.mockReset(); + const fetchMetadata = jest.fn(); + const fetchForeignKeys = jest.fn(); - return render( + renderWithProviders( <TableInfo tableId={id} - table={table} - fetchForeignKeys={fetchForeignKeys} + table={metadata.table(table?.id) ?? undefined} fetchMetadata={fetchMetadata} + fetchForeignKeys={fetchForeignKeys} />, + { storeInitialState: state }, ); + + return { fetchMetadata, fetchForeignKeys }; } describe("TableInfo", () => { it("should fetch table metadata if fields are missing", () => { - setup({ - id: PRODUCTS.id, - table: new Table({ ...PRODUCTS, fields: undefined, fks: [] }), + const { fetchMetadata, fetchForeignKeys } = setup({ + id: TABLE_ID, + table: TABLE_WITH_FKS, }); expect(fetchMetadata).toHaveBeenCalledWith({ - id: PRODUCTS.id, + id: TABLE_ID, }); expect(fetchForeignKeys).not.toHaveBeenCalled(); }); it("should fetch table metadata if the table is undefined", () => { - setup({ id: 123, table: undefined }); + const { fetchMetadata, fetchForeignKeys } = setup({ + id: TABLE_ID, + table: undefined, + }); expect(fetchMetadata).toHaveBeenCalledWith({ - id: 123, + id: TABLE_ID, }); expect(fetchForeignKeys).toHaveBeenCalledWith({ - id: 123, + id: TABLE_ID, }); }); it("should fetch fks if fks are undefined on table", () => { - setup({ - id: PRODUCTS.id, - table: new Table({ ...PRODUCTS, fields: [1, 2, 3], fks: undefined }), + const { fetchMetadata, fetchForeignKeys } = setup({ + id: TABLE_ID, + table: TABLE_WITH_FIELDS, }); expect(fetchMetadata).not.toHaveBeenCalled(); - expect(fetchForeignKeys).toHaveBeenCalledWith({ - id: PRODUCTS.id, - }); + expect(fetchForeignKeys).toHaveBeenCalledWith({ id: TABLE_ID }); }); it("should not send requests fetching table metadata when metadata is already present", () => { - setup({ - id: PRODUCTS.id, - table: productsTable, + const { fetchMetadata, fetchForeignKeys } = setup({ + id: TABLE_ID, + table: TABLE, }); - expect(fetchForeignKeys).not.toHaveBeenCalled(); expect(fetchMetadata).not.toHaveBeenCalled(); + expect(fetchForeignKeys).not.toHaveBeenCalled(); }); it("should display a placeholder if table has no description", async () => { setup({ - id: tableWithoutDescription.id, - table: tableWithoutDescription, + id: TABLE_ID, + table: TABLE_WITHOUT_DESCRIPTION, }); expect(await screen.findByText("No description")).toBeInTheDocument(); @@ -100,19 +137,17 @@ describe("TableInfo", () => { describe("after metadata has been fetched", () => { it("should display the given table's description", () => { - setup({ id: PRODUCTS.id, table: productsTable }); - expect( - screen.getByText(PRODUCTS.description as string), - ).toBeInTheDocument(); + setup({ id: TABLE_ID, table: TABLE }); + expect(screen.getByText(TABLE.description ?? "")).toBeInTheDocument(); }); it("should show a count of columns on the table", () => { - setup({ id: PRODUCTS.id, table: productsTable }); - expect(screen.getByText("3 columns")).toBeInTheDocument(); + setup({ id: TABLE_ID, table: TABLE }); + expect(screen.getByText("1 column")).toBeInTheDocument(); }); it("should list connected tables", () => { - setup({ id: PRODUCTS.id, table: productsTable }); + setup({ id: TABLE_ID, table: TABLE }); expect(screen.getByText("Connected Table")).toBeInTheDocument(); }); }); diff --git a/frontend/src/metabase/components/MetadataInfo/TableLabel/TableLabel.unit.spec.tsx b/frontend/src/metabase/components/MetadataInfo/TableLabel/TableLabel.unit.spec.tsx index 77a6a160755c20552b6be217e439c062acde3767..c2de1d26c257fea20a23002c66948363f60e63b8 100644 --- a/frontend/src/metabase/components/MetadataInfo/TableLabel/TableLabel.unit.spec.tsx +++ b/frontend/src/metabase/components/MetadataInfo/TableLabel/TableLabel.unit.spec.tsx @@ -1,12 +1,35 @@ import React from "react"; -import { render, screen } from "@testing-library/react"; - -import Table from "metabase-lib/metadata/Table"; +import { checkNotNull } from "metabase/core/utils/types"; +import { Table } from "metabase-types/api"; +import { createMockTable } from "metabase-types/api/mocks"; +import { createMockState } from "metabase-types/store/mocks"; +import { createMockEntitiesState } from "__support__/store"; +import { getMetadata } from "metabase/selectors/metadata"; +import { renderWithProviders, screen } from "__support__/ui"; import TableLabel from "./TableLabel"; +interface SetupOpts { + table: Table; +} + +const setup = ({ table }: SetupOpts) => { + const state = createMockState({ + entities: createMockEntitiesState({ + tables: [table], + }), + }); + + const metadata = getMetadata(state); + + renderWithProviders( + <TableLabel table={checkNotNull(metadata.table(table.id))} />, + { storeInitialState: state }, + ); +}; + describe("TableLabel", () => { it("should display the given table's display name", () => { - render(<TableLabel table={new Table({ id: 1, display_name: "Foo" })} />); + setup({ table: createMockTable({ id: 1, display_name: "Foo" }) }); expect(screen.getByText("Foo")).toBeInTheDocument(); }); }); 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 13cdd7aae7c3377d483e2a54adf0f9d855bc2e5c..36831b6e4487aea1091c99f19e052897917d32ac 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 @@ -79,7 +79,7 @@ async function setup({ const queryResult = createMockDataset({ data: { - cols: ORDERS.fields.map(field => field.column()), + cols: ORDERS.fields?.map(field => field.column()), }, }); diff --git a/frontend/src/metabase/query_builder/components/dataref/TablePane.tsx b/frontend/src/metabase/query_builder/components/dataref/TablePane.tsx index 113500e7e9f5a43ffc747fc082984bd404b7fdb3..482b66a790c9cc15dc97f470d7d4df20979d33b1 100644 --- a/frontend/src/metabase/query_builder/components/dataref/TablePane.tsx +++ b/frontend/src/metabase/query_builder/components/dataref/TablePane.tsx @@ -44,7 +44,7 @@ const TablePane = ({ table, onItemClick, onBack, onClose }: TablePaneProps) => ( )} </div> <div className="my2"> - {table.fields.length ? ( + {table.fields?.length ? ( <> <FieldList fields={table.fields} diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionWidget.unit.spec.tsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionWidget.unit.spec.tsx index 59b98dc0d2871f26c41ffd517262a13094cd97fc..2e5f0daa49be26b1ee3a9ccb4278dd21ede2f49b 100644 --- a/frontend/src/metabase/query_builder/components/expressions/ExpressionWidget.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionWidget.unit.spec.tsx @@ -1,5 +1,6 @@ import React from "react"; import userEvent from "@testing-library/user-event"; +import { checkNotNull } from "metabase/core/utils/types"; import { getIcon, render, screen } from "__support__/ui"; import { createMockEntitiesState } from "__support__/store"; import { getMetadata } from "metabase/selectors/metadata"; @@ -131,7 +132,7 @@ const createMockQueryForExpressions = () => { }); const metadata = getMetadata(state); - const query = metadata.table(ORDERS_ID)?.query(); + const query = checkNotNull(metadata.table(ORDERS_ID)).query(); return query; }; diff --git a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/utils.unit.spec.ts b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/utils.unit.spec.ts index afa87da87e5655856cf63e1d65a7357881555656..43716f639c3e7530d96c8134e2e3974ff4818034 100644 --- a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/utils.unit.spec.ts +++ b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/utils.unit.spec.ts @@ -37,7 +37,7 @@ describe("BulkFilterModal utils", () => { it("flags between filters with misordered arguments", () => { const filter = new Filter([ "between", - ["field", ORDERS.fields[1].id, null], + ["field", ORDERS.fields?.[1].id, null], 20, 10, ]); @@ -49,7 +49,7 @@ describe("BulkFilterModal utils", () => { it("hasBackwardsArguments identifies between filters with correctly ordered arguments", () => { const filter = new Filter([ "between", - ["field", ORDERS.fields[1].id, null], + ["field", ORDERS.fields?.[1].id, null], 20, 100, ]); @@ -62,7 +62,7 @@ describe("BulkFilterModal utils", () => { describe("swapFilterArguments", () => { it("swaps arguments in a between filter", () => { const filter = new Filter( - ["between", ["field", ORDERS.fields[1].id, null], 20, 10], + ["between", ["field", ORDERS.fields?.[1].id, null], 20, 10], null, query, ); @@ -77,7 +77,7 @@ describe("BulkFilterModal utils", () => { describe("handleEmptyBetween", () => { it("replaces a between filter with an empty second argument with a >= filter", () => { const filter = new Filter( - ["between", ["field", ORDERS.fields[1].id, null], 20, null], + ["between", ["field", ORDERS.fields?.[1].id, null], 20, null], null, query, ); @@ -90,7 +90,7 @@ describe("BulkFilterModal utils", () => { it("replaces a between filter with an empty first argument with a <= filter", () => { const filter = new Filter( - ["between", ["field", ORDERS.fields[1].id, null], undefined, 30], + ["between", ["field", ORDERS.fields?.[1].id, null], undefined, 30], null, query, ); @@ -105,7 +105,7 @@ describe("BulkFilterModal utils", () => { describe("fixBetweens", () => { it("handles empty between filters", () => { const filter = new Filter( - ["between", ["field", ORDERS.fields[1].id, null], 30, null], + ["between", ["field", ORDERS.fields?.[1].id, null], 30, null], null, query, ); @@ -118,7 +118,7 @@ describe("BulkFilterModal utils", () => { it("handles backwards between filters", () => { const filter = new Filter( - ["between", ["field", ORDERS.fields[1].id, null], 30, 20], + ["between", ["field", ORDERS.fields?.[1].id, null], 30, 20], null, query, ); @@ -131,7 +131,7 @@ describe("BulkFilterModal utils", () => { it("ignores valid between filters", () => { const filter = new Filter( - ["between", ["field", ORDERS.fields[1].id, null], 40, 50], + ["between", ["field", ORDERS.fields?.[1].id, null], 40, 50], null, query, ); @@ -144,7 +144,7 @@ describe("BulkFilterModal utils", () => { it("ignores non-between filters", () => { const filter = new Filter( - ["=", ["field", ORDERS.fields[1].id, null], 40, 50], + ["=", ["field", ORDERS.fields?.[1].id, null], 40, 50], null, query, ); @@ -177,9 +177,9 @@ describe("BulkFilterModal utils", () => { const testQuery = makeQuery({ filter: [ "and", - ["between", ["field", ORDERS.fields[1].id, null], 80, 50], - ["between", ["field", ORDERS.fields[1].id, null], 30, null], - ["=", ["field", ORDERS.fields[1].id, null], 8, 9, 7], + ["between", ["field", ORDERS.fields?.[1].id, null], 80, 50], + ["between", ["field", ORDERS.fields?.[1].id, null], 30, null], + ["=", ["field", ORDERS.fields?.[1].id, null], 8, 9, 7], ], }); diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/ExpressionStep.unit.spec.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/ExpressionStep.unit.spec.tsx index 487c0f0b573f4d89ccc860d69ac5d685cb0fde79..9cdecfa71dcdbba36eb4922f7eb29a9bbd1752c3 100644 --- a/frontend/src/metabase/query_builder/components/notebook/steps/ExpressionStep.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/notebook/steps/ExpressionStep.unit.spec.tsx @@ -1,5 +1,6 @@ import React from "react"; import userEvent from "@testing-library/user-event"; +import { checkNotNull } from "metabase/core/utils/types"; import { render, screen, within } from "__support__/ui"; import { createMockEntitiesState } from "__support__/store"; import { getMetadata } from "metabase/selectors/metadata"; @@ -76,7 +77,7 @@ const createMockQueryForExpressions = ( }); const metadata = getMetadata(state); - let query = metadata.table(ORDERS_ID)?.query(); + let query = checkNotNull(metadata.table(ORDERS_ID)).query(); if (expressions) { Object.entries(expressions).forEach(([name, expression]) => { diff --git a/frontend/src/metabase/selectors/metadata.ts b/frontend/src/metabase/selectors/metadata.ts index ea6350b80539994f36a4ad7c007e5efc4b1486e6..d32f54182f7d2daf88fddc58e436346776a04e6b 100644 --- a/frontend/src/metabase/selectors/metadata.ts +++ b/frontend/src/metabase/selectors/metadata.ts @@ -189,7 +189,7 @@ export const getMetadata: ( if (field.name_field != null) { return metadata.field(field.name_field); } else if (field.table && field.isPK()) { - return _.find(field.table.fields, f => f.isEntityName()); + return _.find(field.table.getFields(), f => f.isEntityName()); } }); diff --git a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectRelationships.tsx b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectRelationships.tsx index f628248942afae41009d7c897aa29c6d59c3c7c1..05ecd7632016e3b6b128b37f688469129068e58b 100644 --- a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectRelationships.tsx +++ b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectRelationships.tsx @@ -37,7 +37,9 @@ export function Relationships({ const fkCountsByTable = foreignKeyCountsByOriginTable(tableForeignKeys); const sortedForeignTables = tableForeignKeys.sort((a, b) => - a.origin.table.display_name.localeCompare(b.origin.table.display_name), + (a.origin.table?.display_name ?? "").localeCompare( + b.origin.table?.display_name ?? "", + ), ); return ( @@ -55,8 +57,16 @@ export function Relationships({ <Relationship key={`${fk.origin_id}-${fk.destination_id}`} fk={fk} - fkCountInfo={tableForeignKeyReferences?.[fk.origin.id]} - fkCount={fkCountsByTable?.[fk.origin.table.id] || 0} + fkCountInfo={ + fk.origin.id != null + ? tableForeignKeyReferences?.[fk.origin.id] + : null + } + fkCount={ + (fk.origin.table != null && + fkCountsByTable?.[fk.origin.table?.id]) || + 0 + } foreignKeyClicked={foreignKeyClicked} /> ))} @@ -81,7 +91,7 @@ function Relationship({ const fkCountValue = fkCountInfo?.value || 0; const isLoaded = fkCountInfo?.status === 1; const fkClickable = isLoaded && Boolean(fkCountInfo.value); - const originTableName = fk.origin.table.display_name; + const originTableName = fk.origin.table?.display_name ?? ""; const relationName = inflect(originTableName, fkCountValue); diff --git a/frontend/test/__support__/sample_database_fixture.ts b/frontend/test/__support__/sample_database_fixture.ts index 930e1a59e758aa088314a4ffc53e7b07e6ba397e..565b11234c6ba223cd0727ec08f8f0775698dd03 100644 --- a/frontend/test/__support__/sample_database_fixture.ts +++ b/frontend/test/__support__/sample_database_fixture.ts @@ -38,7 +38,7 @@ function aliasTablesAndFields(metadata: Metadata) { // @ts-ignore database[table.name] = table; } - for (const field of table.fields) { + for (const field of table.getFields()) { if (!(field.name in table)) { // @ts-ignore table[field.name] = field;