From 6167daa2be2f018ca554577681c13bf8f5cd715f Mon Sep 17 00:00:00 2001 From: Dalton <daltojohnso@users.noreply.github.com> Date: Tue, 30 Aug 2022 16:23:34 -0400 Subject: [PATCH] Fix some typing issues related to string table ids (#25106) --- .../src/metabase-lib/lib/metadata/Field.ts | 14 ++++++++++--- .../src/metabase-lib/lib/metadata/Metadata.ts | 21 ++++++++++++------- .../src/metabase-lib/lib/metadata/Schema.ts | 4 ++-- .../src/metabase-lib/lib/metadata/Table.ts | 13 ++++++++---- frontend/src/metabase-types/api/table.ts | 4 +++- frontend/src/metabase-types/types/Table.ts | 4 +++- .../permissions/selectors/confirmations.ts | 12 +++++++---- .../selectors/data-permissions/breadcrumbs.ts | 10 +++++---- .../data-permissions/permission-editor.ts | 2 +- .../admin/permissions/utils/data-entity-id.ts | 11 +++++----- .../utils/graph/data-permissions.ts | 10 ++++----- .../utils/graph/permissions-diff.ts | 12 +++++++---- .../MetadataInfo/TableInfo/TableInfo.tsx | 6 +++--- .../TableInfo/TableInfo.unit.spec.tsx | 2 +- .../components/ObjectDetail/ObjectDetail.tsx | 7 ++++--- 15 files changed, 83 insertions(+), 49 deletions(-) diff --git a/frontend/src/metabase-lib/lib/metadata/Field.ts b/frontend/src/metabase-lib/lib/metadata/Field.ts index 3394eb1f694..74707a42e5f 100644 --- a/frontend/src/metabase-lib/lib/metadata/Field.ts +++ b/frontend/src/metabase-lib/lib/metadata/Field.ts @@ -32,11 +32,14 @@ import { getIconForField, getFilterOperators, } from "metabase/lib/schema_metadata"; -import { FieldFingerprint } from "metabase-types/api/field"; -import { Field as FieldRef } from "metabase-types/types/Query"; import { FieldDimension } from "../Dimension"; -import Table from "./Table"; import Base from "./Base"; +import type { FieldFingerprint } from "metabase-types/api/field"; +import type { Field as FieldRef } from "metabase-types/types/Query"; +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"; export const LONG_TEXT_MIN = 80; @@ -57,9 +60,14 @@ class FieldInner extends Base { fingerprint?: FieldFingerprint; base_type: string | null; table?: Table; + table_id?: Table["id"]; target?: Field; has_field_values?: "list" | "search" | "none"; values: any[]; + metadata?: Metadata; + + // added when creating "virtual fields" that are associated with a given query + query?: StructuredQuery | NativeQuery; getId() { if (Array.isArray(this.id)) { diff --git a/frontend/src/metabase-lib/lib/metadata/Metadata.ts b/frontend/src/metabase-lib/lib/metadata/Metadata.ts index 8d3d14e63d7..ded4bbafb41 100644 --- a/frontend/src/metabase-lib/lib/metadata/Metadata.ts +++ b/frontend/src/metabase-lib/lib/metadata/Metadata.ts @@ -2,8 +2,11 @@ // @ts-nocheck import _ from "underscore"; import Base from "./Base"; -import Question from "../Question"; -import Database from "./Database"; +import type Question from "../Question"; +import type Database from "./Database"; +import type Table from "./Table"; +import type Schema from "./Schema"; + /** * @typedef { import("./metadata").DatabaseId } DatabaseId * @typedef { import("./metadata").SchemaId } SchemaId @@ -18,7 +21,9 @@ import Database from "./Database"; */ export default class Metadata extends Base { - databases: Database[]; + databases: { [databaseId: string]: Database }; + questions: { [cardId: string]: Question }; + tables: { [tableId: string]: Table }; /** * @deprecated this won't be sorted or filtered in a meaningful way @@ -76,7 +81,7 @@ export default class Metadata extends Base { * @param {DatabaseId} databaseId * @returns {?Database} */ - database(databaseId) { + database(databaseId): Database | null { return (databaseId != null && this.databases[databaseId]) || null; } @@ -84,7 +89,7 @@ export default class Metadata extends Base { * @param {SchemaId} schemaId * @returns {Schema} */ - schema(schemaId) { + schema(schemaId): Schema | null { return (schemaId != null && this.schemas[schemaId]) || null; } @@ -93,7 +98,7 @@ export default class Metadata extends Base { * @param {TableId} tableId * @returns {?Table} */ - table(tableId) { + table(tableId): Table | null { return (tableId != null && this.tables[tableId]) || null; } @@ -101,11 +106,11 @@ export default class Metadata extends Base { * @param {FieldId} fieldId * @returns {?Field} */ - field(fieldId) { + field(fieldId): Field | null { return (fieldId != null && this.fields[fieldId]) || null; } - question(cardId) { + question(cardId): Question | null { return (cardId != null && this.questions[cardId]) || null; } diff --git a/frontend/src/metabase-lib/lib/metadata/Schema.ts b/frontend/src/metabase-lib/lib/metadata/Schema.ts index 4426e151adf..67d62c3901a 100644 --- a/frontend/src/metabase-lib/lib/metadata/Schema.ts +++ b/frontend/src/metabase-lib/lib/metadata/Schema.ts @@ -2,8 +2,8 @@ // @ts-nocheck import Base from "./Base"; import { titleize, humanize } from "metabase/lib/formatting"; -import Database from "./Database"; -import Table from "./Table"; +import type Database from "./Database"; +import type Table from "./Table"; /** * Wrapper class for a {@link Database} schema. Contains {@link Table}s. */ diff --git a/frontend/src/metabase-lib/lib/metadata/Table.ts b/frontend/src/metabase-lib/lib/metadata/Table.ts index c2b3ef5b2d5..ee924b2cdb6 100644 --- a/frontend/src/metabase-lib/lib/metadata/Table.ts +++ b/frontend/src/metabase-lib/lib/metadata/Table.ts @@ -2,13 +2,15 @@ // @ts-nocheck // NOTE: this needs to be imported first due to some cyclical dependency nonsense import Question from "../Question"; -import Schema from "./Schema"; import Base from "./Base"; import { singularize } from "metabase/lib/formatting"; import { getAggregationOperators } from "metabase/lib/schema_metadata"; import { createLookupByProperty, memoizeClass } from "metabase-lib/lib/utils"; -import Field from "./Field"; - +import type Metadata from "./Metadata"; +import type Schema from "./Schema"; +import type Field from "./Field"; +import type Database from "./Database"; +import type { TableId } from "metabase-types/types/Table"; /** * @typedef { import("./metadata").SchemaName } SchemaName * @typedef { import("./metadata").EntityType } EntityType @@ -18,7 +20,8 @@ import Field from "./Field"; /** This is the primary way people interact with tables */ class TableInner extends Base { - id: number; + id: TableId; + name: string; description?: string; fks?: any[]; schema?: Schema; @@ -26,6 +29,8 @@ class TableInner extends Base { schema_name: string; db_id: number; fields: Field[]; + metadata?: Metadata; + db?: Database | undefined | null; hasSchema() { return (this.schema_name && this.db && this.db.schemas.length > 1) || false; diff --git a/frontend/src/metabase-types/api/table.ts b/frontend/src/metabase-types/api/table.ts index 91e63785d68..790afe8bdb6 100644 --- a/frontend/src/metabase-types/api/table.ts +++ b/frontend/src/metabase-types/api/table.ts @@ -2,7 +2,9 @@ import { ForeignKey } from "./foreign-key"; import { Database } from "./database"; import { Field } from "./field"; -export type TableId = number | string; // can be string for virtual questions (e.g. "card__17") +export type ConcreteTableId = number; +export type VirtualTableId = string; // e.g. "card__17" where 17 is a card id +export type TableId = ConcreteTableId | VirtualTableId; export type VisibilityType = | null diff --git a/frontend/src/metabase-types/types/Table.ts b/frontend/src/metabase-types/types/Table.ts index 1819378bd55..59f0bd8395c 100644 --- a/frontend/src/metabase-types/types/Table.ts +++ b/frontend/src/metabase-types/types/Table.ts @@ -10,8 +10,10 @@ import { Segment } from "./Segment"; import { Metric } from "./Metric"; import { DatabaseId } from "./Database"; import { ForeignKey } from "../api/foreign-key"; +import { TableId as _TableId } from "metabase-types/api"; + +export type TableId = _TableId; -export type TableId = number; export type SchemaName = string; type TableVisibilityType = string; // FIXME diff --git a/frontend/src/metabase/admin/permissions/selectors/confirmations.ts b/frontend/src/metabase/admin/permissions/selectors/confirmations.ts index a1d5e2621c7..f55405ad8ff 100644 --- a/frontend/src/metabase/admin/permissions/selectors/confirmations.ts +++ b/frontend/src/metabase/admin/permissions/selectors/confirmations.ts @@ -6,9 +6,13 @@ import { getNativePermission, getSchemasPermission, } from "metabase/admin/permissions/utils/graph"; -import { Group, GroupsPermissions } from "metabase-types/api"; -import { EntityId } from "../types"; -import Database from "metabase-lib/lib/metadata/Database"; +import type { + Group, + GroupsPermissions, + ConcreteTableId, +} from "metabase-types/api"; +import type { EntityId } from "../types"; +import type Database from "metabase-lib/lib/metadata/Database"; export const getDefaultGroupHasHigherAccessText = (defaultGroup: Group) => t`The "${defaultGroup.name}" group has a higher level of access than this, which will override this setting. You should limit or revoke the "${defaultGroup.name}" group's access to this item.`; @@ -139,7 +143,7 @@ export function getRevokingAccessToAllTablesWarningModal( const allTableEntityIds = database.tables.map(table => ({ databaseId: table.db_id, schemaName: table.schema_name || "", - tableId: table.id, + tableId: table.id as ConcreteTableId, })); // Show the warning only if user tries to revoke access to the very last table of all schemas diff --git a/frontend/src/metabase/admin/permissions/selectors/data-permissions/breadcrumbs.ts b/frontend/src/metabase/admin/permissions/selectors/data-permissions/breadcrumbs.ts index 903713c778d..0f69c67425f 100644 --- a/frontend/src/metabase/admin/permissions/selectors/data-permissions/breadcrumbs.ts +++ b/frontend/src/metabase/admin/permissions/selectors/data-permissions/breadcrumbs.ts @@ -1,4 +1,6 @@ -import Metadata from "metabase-lib/lib/metadata/Metadata"; +import type Metadata from "metabase-lib/lib/metadata/Metadata"; +import type Schema from "metabase-lib/lib/metadata/Schema"; +import type Table from "metabase-lib/lib/metadata/Table"; import { Group } from "metabase-types/api"; import _ from "underscore"; @@ -46,7 +48,7 @@ export const getDatabasesEditorBreadcrumbs = ( return [groupItem, databaseItem]; } - const schema = database.schema(schemaName); + const schema = database.schema(schemaName) as Schema; const schemaItem = { id: schema.name, text: schema.name, @@ -79,7 +81,7 @@ export const getGroupsDataEditorBreadcrumbs = ( return [databaseItem]; } - const schema = database.schema(schemaName); + const schema = database.schema(schemaName) as Schema; const schemaItem = { id: schema.id, text: schema.name, @@ -92,7 +94,7 @@ export const getGroupsDataEditorBreadcrumbs = ( return [databaseItem, hasMultipleSchemas && schemaItem].filter(Boolean); } - const table = metadata.table(tableId); + const table = metadata.table(tableId) as Table; const tableItem = { id: table.id, diff --git a/frontend/src/metabase/admin/permissions/selectors/data-permissions/permission-editor.ts b/frontend/src/metabase/admin/permissions/selectors/data-permissions/permission-editor.ts index ddf3bc6dbc4..c57955aaee4 100644 --- a/frontend/src/metabase/admin/permissions/selectors/data-permissions/permission-editor.ts +++ b/frontend/src/metabase/admin/permissions/selectors/data-permissions/permission-editor.ts @@ -164,7 +164,7 @@ export const getDatabasesPermissionEditor = createSelector( if (database && (schemaName != null || hasSingleSchema)) { const schema: Schema = hasSingleSchema ? database.getSchemas()[0] - : database.schema(schemaName); + : (database.schema(schemaName) as Schema); entities = schema .getTables() diff --git a/frontend/src/metabase/admin/permissions/utils/data-entity-id.ts b/frontend/src/metabase/admin/permissions/utils/data-entity-id.ts index 423c403114b..240bc6cb6c0 100644 --- a/frontend/src/metabase/admin/permissions/utils/data-entity-id.ts +++ b/frontend/src/metabase/admin/permissions/utils/data-entity-id.ts @@ -1,7 +1,8 @@ -import Database from "metabase-lib/lib/metadata/Database"; -import Schema from "metabase-lib/lib/metadata/Schema"; -import Table from "metabase-lib/lib/metadata/Table"; -import { EntityId, PermissionSubject } from "../types"; +import type Database from "metabase-lib/lib/metadata/Database"; +import type Schema from "metabase-lib/lib/metadata/Schema"; +import type Table from "metabase-lib/lib/metadata/Table"; +import type { EntityId, PermissionSubject } from "../types"; +import type { ConcreteTableId } from "metabase-types/api"; export const getDatabaseEntityId = (databaseEntity: Database) => ({ databaseId: databaseEntity.id, @@ -15,7 +16,7 @@ export const getSchemaEntityId = (schemaEntity: Schema) => ({ export const getTableEntityId = (tableEntity: Table) => ({ databaseId: tableEntity.db_id, schemaName: tableEntity.schema_name, - tableId: tableEntity.id, + tableId: tableEntity.id as ConcreteTableId, }); export const isTableEntityId = (entityId: Partial<EntityId>) => diff --git a/frontend/src/metabase/admin/permissions/utils/graph/data-permissions.ts b/frontend/src/metabase/admin/permissions/utils/graph/data-permissions.ts index 7914b7e87e3..e5f37bfb0fe 100644 --- a/frontend/src/metabase/admin/permissions/utils/graph/data-permissions.ts +++ b/frontend/src/metabase/admin/permissions/utils/graph/data-permissions.ts @@ -5,10 +5,10 @@ import { PLUGIN_ADMIN_PERMISSIONS_TABLE_FIELDS_PERMISSION_VALUE, PLUGIN_ADVANCED_PERMISSIONS, } from "metabase/plugins"; -import { GroupsPermissions } from "metabase-types/api"; -import Database from "metabase-lib/lib/metadata/Database"; -import Table from "metabase-lib/lib/metadata/Table"; -import { +import type { GroupsPermissions, ConcreteTableId } from "metabase-types/api"; +import type Database from "metabase-lib/lib/metadata/Database"; +import type Table from "metabase-lib/lib/metadata/Table"; +import type { DatabaseEntityId, DataPermission, EntityId, @@ -202,7 +202,7 @@ export function downgradeNativePermissionsIfNeeded( const metadataTableToTableEntityId = (table: Table) => ({ databaseId: table.db_id, schemaName: table.schema_name || "", - tableId: table.id, + tableId: table.id as ConcreteTableId, }); // TODO Atte Keinänen 6/24/17 See if this method could be simplified diff --git a/frontend/src/metabase/admin/permissions/utils/graph/permissions-diff.ts b/frontend/src/metabase/admin/permissions/utils/graph/permissions-diff.ts index 0d18ccd3b2c..fb6ab6a738d 100644 --- a/frontend/src/metabase/admin/permissions/utils/graph/permissions-diff.ts +++ b/frontend/src/metabase/admin/permissions/utils/graph/permissions-diff.ts @@ -1,7 +1,11 @@ import _ from "underscore"; -import { Group, GroupsPermissions } from "metabase-types/api"; -import Database from "metabase-lib/lib/metadata/Database"; +import type { + Group, + GroupsPermissions, + ConcreteTableId, +} from "metabase-types/api"; +import type Database from "metabase-lib/lib/metadata/Database"; import { getFieldsPermission, getNativePermission, @@ -46,7 +50,7 @@ function diffDatabasePermissions( { databaseId: database.id, schemaName: table.schema_name || "", - tableId: table.id, + tableId: table.id as ConcreteTableId, }, "data", ); @@ -56,7 +60,7 @@ function diffDatabasePermissions( { databaseId: database.id, schemaName: table.schema_name || "", - tableId: table.id, + tableId: table.id as ConcreteTableId, }, "data", ); diff --git a/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.tsx b/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.tsx index bfb09b51525..5bb61f47cb6 100644 --- a/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.tsx +++ b/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.tsx @@ -20,7 +20,7 @@ import ConnectedTables from "./ConnectedTables"; type OwnProps = { className?: string; - tableId: number; + tableId: Table["id"]; onConnectedTableClick?: (table: Table) => void; }; @@ -33,8 +33,8 @@ const mapStateToProps = (state: any, props: OwnProps): { table?: Table } => { }; const mapDispatchToProps: { - fetchForeignKeys: (args: { id: number }) => Promise<any>; - fetchMetadata: (args: { id: number }) => Promise<any>; + fetchForeignKeys: (args: { id: Table["id"] }) => Promise<any>; + fetchMetadata: (args: { id: Table["id"] }) => Promise<any>; } = { fetchForeignKeys: Tables.actions.fetchForeignKeys, fetchMetadata: Tables.actions.fetchMetadata, 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 8f448a882a8..da8a66ab179 100644 --- a/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.unit.spec.tsx +++ b/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.unit.spec.tsx @@ -30,7 +30,7 @@ const tableWithoutDescription = new Table({ const fetchForeignKeys = jest.fn(); const fetchMetadata = jest.fn(); -function setup({ id, table }: { table: Table | undefined; id: number }) { +function setup({ id, table }: { table: Table | undefined; id: Table["id"] }) { fetchForeignKeys.mockReset(); fetchMetadata.mockReset(); diff --git a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx index aa02c3d4bc0..9492d27ca48 100644 --- a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx +++ b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx @@ -7,7 +7,7 @@ import { isPK } from "metabase/lib/schema_metadata"; import Table from "metabase-lib/lib/metadata/Table"; import { State } from "metabase-types/store"; -import { ForeignKey } from "metabase-types/api"; +import type { ForeignKey, ConcreteTableId } from "metabase-types/api"; import { DatasetData } from "metabase-types/types/Dataset"; import { ObjectId, OnVisualizationClickType } from "./types"; @@ -35,6 +35,7 @@ import { getCanZoomNextRow, } from "metabase/query_builder/selectors"; import { columnSettings } from "metabase/visualizations/lib/settings/column"; +import { isVirtualCardId } from "metabase/lib/saved-questions"; import { getObjectName, @@ -159,8 +160,8 @@ export function ObjectDetailFn({ return; } - if (table && table.fks == null) { - fetchTableFks(table.id); + if (table && table.fks == null && !isVirtualCardId(table.id)) { + fetchTableFks(table.id as ConcreteTableId); } // load up FK references if (tableForeignKeys) { -- GitLab