diff --git a/frontend/src/metabase-lib/actions/utils.unit.spec.ts b/frontend/src/metabase-lib/actions/utils.unit.spec.ts index 47608f3b2bbc93d864253b9ac3a177127239a273..ea139ffad14520e6d9e4e197194e9ae97223dcff 100644 --- a/frontend/src/metabase-lib/actions/utils.unit.spec.ts +++ b/frontend/src/metabase-lib/actions/utils.unit.spec.ts @@ -1,37 +1,54 @@ +import { WritebackAction, Database } from "metabase-types/api"; import { createMockDatabase, createMockQueryAction, } from "metabase-types/api/mocks"; -import Database from "metabase-lib/metadata/Database"; +import { createMockMetadata } from "__support__/metadata"; import { canRunAction } from "./utils"; +interface SetupOpts { + database?: Database; + action?: WritebackAction; +} + +const setup = ({ + database = createMockDatabase(), + action = createMockQueryAction({ database_id: database.id }), +}: SetupOpts = {}) => { + const metadata = createMockMetadata({ databases: [database] }); + const db = metadata.database(database.id); + if (!db) { + throw new Error(); + } + + return { action, database: db }; +}; + describe("canRunAction", () => { it("should not be able to run an action if the user has no access to the database", () => { - const action = createMockQueryAction(); + const { action } = setup(); expect(canRunAction(action, [])).toBe(false); }); it("should not be able to run an action if the database has actions disabled", () => { - const database = new Database( - createMockDatabase({ + const { database, action } = setup({ + database: createMockDatabase({ native_permissions: "write", settings: { "database-enable-actions": false }, }), - ); - const action = createMockQueryAction({ database_id: database.id }); + }); expect(canRunAction(action, [database])).toBe(false); }); it("should be able to run an action if the database has actions enabled", () => { - const database = new Database( - createMockDatabase({ + const { database, action } = setup({ + database: createMockDatabase({ native_permissions: "read", settings: { "database-enable-actions": true }, }), - ); - const action = createMockQueryAction({ database_id: database.id }); + }); expect(canRunAction(action, [database])).toBe(true); }); diff --git a/frontend/src/metabase-lib/metadata/Database.ts b/frontend/src/metabase-lib/metadata/Database.ts index c76dea6b086190318383636b0462c0e31d2e7cd1..773a3aba76589db0be374887be3485a7b9fc52db 100644 --- a/frontend/src/metabase-lib/metadata/Database.ts +++ b/frontend/src/metabase-lib/metadata/Database.ts @@ -1,86 +1,62 @@ -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-nocheck +import _ from "underscore"; import { - Database as IDatabase, - DatabaseFeature, - DatabaseSettings, - NativePermissions, + NativeQuery, + NormalizedDatabase, StructuredQuery, } from "metabase-types/api"; import { generateSchemaId } from "metabase-lib/metadata/utils/schema"; -import { createLookupByProperty, memoizeClass } from "metabase-lib/utils"; import Question from "../Question"; -import Base from "./Base"; import Table from "./Table"; import Schema from "./Schema"; import Metadata from "./Metadata"; -/** - * @typedef { import("./Metadata").SchemaName } SchemaName - */ - -/** - * Wrapper class for database metadata objects. Contains {@link Schema}s, {@link Table}s, {@link Metric}s, {@link Segment}s. - * - * Backed by types/Database data structure which matches the backend API contract - */ - -class DatabaseInner extends Base { - id: number; - name: string; - engine: string; - description: string; - creator_id?: number; - is_sample: boolean; - is_saved_questions: boolean; - tables: Table[]; - schemas: Schema[]; - metadata: Metadata; - features: DatabaseFeature[]; - details: Record<string, unknown>; - settings?: DatabaseSettings; - native_permissions: NativePermissions; - - // Only appears in GET /api/database/:id - "can-manage"?: boolean; - - getPlainObject(): IDatabase { +interface Database extends Omit<NormalizedDatabase, "tables" | "schemas"> { + tables?: Table[]; + schemas?: Schema[]; + metadata?: Metadata; +} + +class Database { + private readonly _plainObject: NormalizedDatabase; + + constructor(database: NormalizedDatabase) { + this._plainObject = database; + this.tablesLookup = _.memoize(this.tablesLookup); + Object.assign(this, database); + } + + getPlainObject(): NormalizedDatabase { return this._plainObject; } - // TODO Atte Keinänen 6/11/17: List all fields here (currently only in types/Database) displayName() { return this.name; } - // SCHEMAS - - /** - * @param {SchemaName} [schemaName] - */ - schema(schemaName) { - return this.metadata.schema(generateSchemaId(this.id, schemaName)); + schema(schemaName: string | undefined) { + return this.metadata?.schema(generateSchemaId(this.id, schemaName)); } schemaNames() { - return this.schemas.map(s => s.name).sort((a, b) => a.localeCompare(b)); + return this.getSchemas() + .map(s => s.name) + .sort((a, b) => a.localeCompare(b)); } getSchemas() { - return this.schemas; + return this.schemas ?? []; } schemasCount() { - return this.schemas.length; + return this.getSchemas().length; } getTables() { - return this.tables; + return this.tables ?? []; } - // TABLES tablesLookup() { - return createLookupByProperty(this.tables, "id"); + return Object.fromEntries(this.getTables().map(table => [table.id, table])); } // @deprecated: use tablesLookup @@ -88,19 +64,12 @@ class DatabaseInner extends Base { return this.tablesLookup(); } - // FEATURES - - /** - * @typedef {import("./Metadata").DatabaseFeature} DatabaseFeature - * @typedef {"join"} VirtualDatabaseFeature - * @param {DatabaseFeature | VirtualDatabaseFeature} [feature] - */ - hasFeature(feature) { + hasFeature(feature: string | undefined) { if (!feature) { return true; } - const set = new Set(this.features); + const set = new Set<string>(this.features); if (feature === "join") { return ( @@ -142,14 +111,13 @@ class DatabaseInner extends Base { return Boolean(this.settings?.["database-enable-actions"]); } - // QUESTIONS newQuestion() { return this.question().setDefaultQuery().setDefaultDisplay(); } question( query: StructuredQuery = { - "source-table": null, + "source-table": undefined, }, ) { return Question.create({ @@ -162,7 +130,7 @@ class DatabaseInner extends Base { }); } - nativeQuestion(native = {}) { + nativeQuestion(native: Partial<NativeQuery> = {}) { return Question.create({ metadata: this.metadata, dataset_query: { @@ -177,17 +145,14 @@ class DatabaseInner extends Base { }); } - nativeQuery(native) { + nativeQuery(native: Partial<NativeQuery>) { return this.nativeQuestion(native).query(); } - /** Returns a database containing only the saved questions from the same database, if any */ savedQuestionsDatabase() { - return this.metadata.databasesList().find(db => db.is_saved_questions); + return this.metadata?.databasesList().find(db => db.is_saved_questions); } } // eslint-disable-next-line import/no-default-export -- deprecated usage -export default class Database extends memoizeClass<DatabaseInner>( - "tablesLookup", -)(DatabaseInner) {} +export default Database; diff --git a/frontend/src/metabase-lib/metadata/Database.unit.spec.ts b/frontend/src/metabase-lib/metadata/Database.unit.spec.ts index 0a94467a3edcac8748d0eab9d9259bb68cb32c5e..7157cf0c04899f89c2aa5f414db95e1b55cba2f6 100644 --- a/frontend/src/metabase-lib/metadata/Database.unit.spec.ts +++ b/frontend/src/metabase-lib/metadata/Database.unit.spec.ts @@ -5,20 +5,12 @@ import Database from "./Database"; import Schema from "./Schema"; import Metadata from "./Metadata"; import Table from "./Table"; -import Base from "./Base"; + describe("Database", () => { describe("instantiation", () => { it("should create an instance of Schema", () => { expect(new Database()).toBeInstanceOf(Database); }); - it("should add `object` props to the instance (because it extends Base)", () => { - expect(new Database()).toBeInstanceOf(Base); - expect( - new Database({ - foo: "bar", - }), - ).toHaveProperty("foo", "bar"); - }); }); describe("displayName", () => { it("should return the name prop", () => { @@ -167,7 +159,7 @@ describe("Database", () => { expect(database.question().datasetQuery()).toEqual({ database: 123, query: { - "source-table": null, + "source-table": undefined, }, type: "query", }); diff --git a/frontend/src/metabase-lib/parameters/utils/targets.unit.spec.ts b/frontend/src/metabase-lib/parameters/utils/targets.unit.spec.ts index 2be8f350fa5a3ddd72261a4ede26a8322934fdda..e063e3989ebb40d17fd16bdbe63dcfe027840762 100644 --- a/frontend/src/metabase-lib/parameters/utils/targets.unit.spec.ts +++ b/frontend/src/metabase-lib/parameters/utils/targets.unit.spec.ts @@ -5,6 +5,7 @@ import { } from "__support__/sample_database_fixture"; import { isDimensionTarget } from "metabase-types/guards"; import type { Card, ParameterDimensionTarget } from "metabase-types/api"; +import { createMockTemplateTag } from "metabase-types/api/mocks"; import Database from "metabase-lib/metadata/Database"; import { getParameterTargetField, @@ -74,9 +75,9 @@ describe("parameters/utils/targets", () => { const question = SAMPLE_DATABASE.nativeQuestion({ query: "select * from PRODUCTS where CATEGORY = {{foo}}", "template-tags": { - foo: { + foo: createMockTemplateTag({ type: "text", - }, + }), }, }); @@ -97,10 +98,10 @@ describe("parameters/utils/targets", () => { const question = SAMPLE_DATABASE.nativeQuestion({ query: "select * from PRODUCTS where {{foo}}", "template-tags": { - foo: { + foo: createMockTemplateTag({ type: "dimension", dimension: ["field", PRODUCTS.CATEGORY.id, null], - }, + }), }, }); diff --git a/frontend/src/metabase-lib/queries/utils/native-query-table.ts b/frontend/src/metabase-lib/queries/utils/native-query-table.ts index 9c4b87ab1a893fb3a543519a426b508858cb35ef..09267b7ed1dbad45a5f56c80b438bcd964391b6c 100644 --- a/frontend/src/metabase-lib/queries/utils/native-query-table.ts +++ b/frontend/src/metabase-lib/queries/utils/native-query-table.ts @@ -17,7 +17,7 @@ export function getNativeQueryTable(nativeQuery: NativeQuery): Table | null { const collection = nativeQuery.collection(); if (database && collection) { return ( - _.findWhere(database.tables, { + _.findWhere(database.getTables(), { name: collection, }) || null ); diff --git a/frontend/src/metabase/admin/databases/components/DatabaseEditApp/Sidebar/Sidebar.unit.spec.tsx b/frontend/src/metabase/admin/databases/components/DatabaseEditApp/Sidebar/Sidebar.unit.spec.tsx index f0cdff37be6f1f7e353534fe0b05db7d9c688bef..e629433ed1b057b6d39c82bf5349047d068aaa99 100644 --- a/frontend/src/metabase/admin/databases/components/DatabaseEditApp/Sidebar/Sidebar.unit.spec.tsx +++ b/frontend/src/metabase/admin/databases/components/DatabaseEditApp/Sidebar/Sidebar.unit.spec.tsx @@ -1,21 +1,21 @@ import React from "react"; import _ from "underscore"; -import { - render, - screen, - waitForElementToBeRemoved, - within, -} from "@testing-library/react"; import userEvent from "@testing-library/user-event"; - -import type { InitialSyncStatus } from "metabase-types/api"; - +import { checkNotNull } from "metabase/core/utils/types"; +import { getMetadata } from "metabase/selectors/metadata"; +import type { Database, InitialSyncStatus } from "metabase-types/api"; import { createMockDatabase, COMMON_DATABASE_FEATURES, } from "metabase-types/api/mocks"; -import Database from "metabase-lib/metadata/Database"; - +import { createMockState } from "metabase-types/store/mocks"; +import { createMockEntitiesState } from "__support__/store"; +import { + renderWithProviders, + screen, + waitForElementToBeRemoved, + within, +} from "__support__/ui"; import Sidebar from "./Sidebar"; const NOT_SYNCED_DB_STATUSES: InitialSyncStatus[] = ["aborted", "incomplete"]; @@ -24,11 +24,24 @@ function getModal() { return document.querySelector(".Modal") as HTMLElement; } +interface SetupOpts { + database?: Database; + isAdmin?: boolean; + isModelPersistenceEnabled?: boolean; +} + function setup({ database = createMockDatabase(), isAdmin = true, isModelPersistenceEnabled = false, -} = {}) { +}: SetupOpts = {}) { + const state = createMockState({ + entities: createMockEntitiesState({ + databases: [database], + }), + }); + const metadata = getMetadata(state); + // Using mockResolvedValue since `ActionButton` component // the Sidebar is using is expecting these callbacks to be async const updateDatabase = jest.fn().mockResolvedValue({}); @@ -38,9 +51,9 @@ function setup({ const dismissSyncSpinner = jest.fn().mockResolvedValue({}); const deleteDatabase = jest.fn().mockResolvedValue({}); - const utils = render( + const utils = renderWithProviders( <Sidebar - database={new Database(database)} + database={checkNotNull(metadata.database(database.id))} isAdmin={isAdmin} isModelPersistenceEnabled={isModelPersistenceEnabled} updateDatabase={updateDatabase} @@ -50,6 +63,7 @@ function setup({ dismissSyncSpinner={dismissSyncSpinner} deleteDatabase={deleteDatabase} />, + { storeInitialState: state }, ); return { @@ -184,11 +198,6 @@ describe("DatabaseEditApp/Sidebar", () => { expect(screen.queryByText(/Model actions/i)).not.toBeInTheDocument(); }); - it("isn't shown when adding a new database", () => { - setup({ database: createMockDatabase({ id: undefined }) }); - expect(screen.queryByText(/Model actions/i)).not.toBeInTheDocument(); - }); - it("enables actions", () => { const { database, updateDatabase } = setup(); diff --git a/frontend/src/metabase/admin/permissions/selectors/confirmations.ts b/frontend/src/metabase/admin/permissions/selectors/confirmations.ts index b302c15b43069bb8e5ddbe39b145a1f07464ddca..da1cbb13efe580ce0c4c5c232eb5516f3e17c122 100644 --- a/frontend/src/metabase/admin/permissions/selectors/confirmations.ts +++ b/frontend/src/metabase/admin/permissions/selectors/confirmations.ts @@ -140,7 +140,7 @@ export function getRevokingAccessToAllTablesWarningModal( getNativePermission(permissions, groupId, entityId) !== "none" ) { // allTableEntityIds contains tables from all schemas - const allTableEntityIds = database.tables.map(table => ({ + const allTableEntityIds = database.getTables().map(table => ({ databaseId: table.db_id, schemaName: table.schema_name || "", tableId: table.id as ConcreteTableId, diff --git a/frontend/src/metabase/metabot/components/Metabot/Metabot.unit.spec.tsx b/frontend/src/metabase/metabot/components/Metabot/Metabot.unit.spec.tsx index 1d7aec9010862123a03c5a285fc991d5d38b3a09..6218648d9c3c762efe489170820f4e5130a77ef8 100644 --- a/frontend/src/metabase/metabot/components/Metabot/Metabot.unit.spec.tsx +++ b/frontend/src/metabase/metabot/components/Metabot/Metabot.unit.spec.tsx @@ -1,20 +1,18 @@ import React from "react"; -import { screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import fetchMock from "fetch-mock"; -import { renderWithProviders } from "__support__/ui"; -import { - MetabotEntityId, - MetabotEntityType, -} from "metabase-types/store/metabot"; +import { checkNotNull } from "metabase/core/utils/types"; +import { getMetadata } from "metabase/selectors/metadata"; +import { Card, Database } from "metabase-types/api"; import { createMockCard, createMockDatabase, createMockField, - createMockStructuredDatasetQuery, - createMockStructuredQuery, createMockTable, } from "metabase-types/api/mocks"; +import { createStructuredModelCard } from "metabase-types/api/mocks/presets"; +import { MetabotEntityId, MetabotEntityType } from "metabase-types/store"; +import { createMockState } from "metabase-types/store/mocks"; import { setupCardDataset, setupDatabaseEndpoints, @@ -26,9 +24,8 @@ import { setupMetabotDatabaseEndpoint, setupMetabotModelEndpoint, } from "__support__/server-mocks/metabot"; -import Question from "metabase-lib/Question"; -import Database from "metabase-lib/metadata/Database"; -import { getStructuredModel } from "metabase-lib/mocks"; +import { createMockEntitiesState } from "__support__/store"; +import { renderWithProviders, screen } from "__support__/ui"; import Metabot from "./Metabot"; const PROMPT = "average orders total"; @@ -55,13 +52,15 @@ const ORDERS_DATABASE = createMockDatabase({ tables: [ORDERS_TABLE], }); -const MODEL = getStructuredModel({ +const MODEL = createStructuredModelCard({ id: 1, + name: "Q1", result_metadata: [FIELD], - dataset_query: createMockStructuredDatasetQuery({ + dataset_query: { database: ORDERS_DATABASE_ID, - query: createMockStructuredQuery({ "source-table": ORDERS_TABLE_ID }), - }), + type: "query", + query: { "source-table": ORDERS_TABLE_ID }, + }, }); const GENERATED_CARD = createMockCard({ id: undefined, display: "table" }); @@ -84,7 +83,7 @@ const setupMetabotDatabaseEndpoints = (couldGenerateCard = true) => { const setupMetabotModelEndpoints = (couldGenerateCard = true) => { if (couldGenerateCard) { - setupMetabotModelEndpoint(MODEL.id(), GENERATED_CARD, true); + setupMetabotModelEndpoint(MODEL.id, GENERATED_CARD, true); setupCardDataset( { row_count: 1, @@ -93,7 +92,7 @@ const setupMetabotModelEndpoints = (couldGenerateCard = true) => { true, ); } else { - setupBadRequestMetabotModelEndpoint(MODEL.id()); + setupBadRequestMetabotModelEndpoint(MODEL.id); } }; @@ -101,7 +100,7 @@ interface SetupOpts { entityId?: MetabotEntityId; entityType: MetabotEntityType; initialPrompt?: string; - model?: Question; + model?: Card; database?: Database; databases?: Database[]; } @@ -111,17 +110,26 @@ const setup = ({ entityType, initialPrompt, model, - database = new Database(ORDERS_DATABASE), - databases = [new Database(ORDERS_DATABASE)], + database = ORDERS_DATABASE, + databases = [ORDERS_DATABASE], }: SetupOpts) => { + const state = createMockState({ + entities: createMockEntitiesState({ + databases: databases, + questions: model ? [model] : [], + }), + }); + + const metadata = getMetadata(state); + renderWithProviders( <Metabot entityId={entityId} entityType={entityType} initialPrompt={initialPrompt} - model={model} - database={database} - databases={databases} + model={model ? checkNotNull(metadata.question(model.id)) : undefined} + database={checkNotNull(metadata.database(database.id))} + databases={[checkNotNull(metadata.database(database.id))]} />, ); }; diff --git a/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorDatabaseSchemaPicker/DataSelectorDatabaseSchemaPicker.tsx b/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorDatabaseSchemaPicker/DataSelectorDatabaseSchemaPicker.tsx index 28228e7e011f2735f03a8360063e78240aa8c4f2..8cfbf32988db42d9b9e77a7c45ab967d3de93482 100644 --- a/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorDatabaseSchemaPicker/DataSelectorDatabaseSchemaPicker.tsx +++ b/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorDatabaseSchemaPicker/DataSelectorDatabaseSchemaPicker.tsx @@ -59,8 +59,8 @@ const DataSelectorDatabaseSchemaPicker = ({ const sections: Sections = databases.map(database => ({ name: database.is_saved_questions ? t`Saved Questions` : database.name, items: - !database.is_saved_questions && database.schemas.length > 1 - ? database.schemas.map(schema => ({ + !database.is_saved_questions && database.getSchemas().length > 1 + ? database.getSchemas().map(schema => ({ schema, name: schema.displayName() ?? "", })) @@ -69,7 +69,7 @@ const DataSelectorDatabaseSchemaPicker = ({ icon: database.is_saved_questions ? "collection" : "database", loading: selectedDatabase?.id === database.id && - database.schemas.length === 0 && + database.getSchemas().length === 0 && isLoading, active: database.is_saved_questions || isSyncCompleted(database), })); @@ -112,7 +112,7 @@ const DataSelectorDatabaseSchemaPicker = ({ ? databases.findIndex(db => db.id === selectedDatabase.id) : -1; - if (openSection >= 0 && databases[openSection]?.schemas.length === 1) { + if (openSection >= 0 && databases[openSection]?.getSchemas().length === 1) { openSection = -1; } diff --git a/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorDatabaseSchemaPicker/DataSelectorDatabaseSchemaPicker.unit.spec.js b/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorDatabaseSchemaPicker/DataSelectorDatabaseSchemaPicker.unit.spec.js index d9ebeeb0a208a23ffa5b62bf9167478ade6bccde..793067bb05455cae562d24e9d50d01f6995552b5 100644 --- a/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorDatabaseSchemaPicker/DataSelectorDatabaseSchemaPicker.unit.spec.js +++ b/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorDatabaseSchemaPicker/DataSelectorDatabaseSchemaPicker.unit.spec.js @@ -19,7 +19,7 @@ describe("DataSelectorDatabaseSchemaPicker", () => { { id: 1, name: databaseName, - schemas: [ + getSchemas: () => [ { displayName: () => schemaName, }, @@ -45,7 +45,7 @@ describe("DataSelectorDatabaseSchemaPicker", () => { id: 1, is_saved_questions: true, name: databaseName, - schemas: [ + getSchemas: () => [ { displayName: () => schemaName, }, diff --git a/frontend/src/metabase/query_builder/components/dataref/DatabaseSchemasPane.tsx b/frontend/src/metabase/query_builder/components/dataref/DatabaseSchemasPane.tsx index 9b6b66d3d8081d776edbaa58a61a75b013d4059b..fa2ec282a07bf3a9b79d5c5f72019bcda33de38b 100644 --- a/frontend/src/metabase/query_builder/components/dataref/DatabaseSchemasPane.tsx +++ b/frontend/src/metabase/query_builder/components/dataref/DatabaseSchemasPane.tsx @@ -37,7 +37,7 @@ const DatabaseSchemasPane = ({ () => models.sort((a, b) => a.name.localeCompare(b.name)), [models], ); - const schemas = database.schemas; + const schemas = database.getSchemas(); return ( <SidebarContent title={database.name} diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorHelpText.stories.tsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorHelpText.stories.tsx index 87768e791c3511e7f241a7f14a75039ae8b5f6c4..0b3dce9ec667b8e5d8d44564b54fc5f0a7602368 100644 --- a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorHelpText.stories.tsx +++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorHelpText.stories.tsx @@ -1,9 +1,8 @@ import React, { useRef } from "react"; import type { ComponentStory } from "@storybook/react"; - +import { checkNotNull } from "metabase/core/utils/types"; import { createMockDatabase } from "metabase-types/api/mocks"; -import Database from "metabase-lib/metadata/Database"; - +import { createMockMetadata } from "__support__/metadata"; import { getHelpText } from "./ExpressionEditorTextfield/helper-text-strings"; import ExpressionEditorHelpText, { ExpressionEditorHelpTextProps, @@ -17,11 +16,13 @@ export default { const Template: ComponentStory<typeof ExpressionEditorHelpText> = args => { const target = useRef(null); + const database = createMockDatabase(); + const metadata = createMockMetadata({ databases: [database] }); const props: ExpressionEditorHelpTextProps = { helpText: getHelpText( "datetime-diff", - new Database(createMockDatabase()), + checkNotNull(metadata.database(database.id)), "UTC", ), width: 397, diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorHelpText.unit.spec.tsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorHelpText.unit.spec.tsx index b2b8e187848d8d8c2303160a6be0ba438b272e22..ad562362bb6daa871ae30d3c7a6fb1ead4c683d6 100644 --- a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorHelpText.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorHelpText.unit.spec.tsx @@ -1,18 +1,23 @@ import React from "react"; import { render, screen, within } from "@testing-library/react"; +import { checkNotNull } from "metabase/core/utils/types"; +import { createMockMetadata } from "__support__/metadata"; import { getBrokenUpTextMatcher } from "__support__/ui"; -import { createMockDatabase } from "metabase-types/api/mocks"; -import Database from "metabase-lib/metadata/Database"; +import { + createSampleDatabase, + SAMPLE_DB_ID, +} from "metabase-types/api/mocks/presets"; import { getHelpText } from "./ExpressionEditorTextfield/helper-text-strings"; import ExpressionEditorHelpText, { ExpressionEditorHelpTextProps, } from "./ExpressionEditorHelpText"; -const DATABASE = new Database(createMockDatabase()); - describe("ExpressionEditorHelpText", () => { + const metadata = createMockMetadata({ databases: [createSampleDatabase()] }); + const database = checkNotNull(metadata.database(SAMPLE_DB_ID)); + it("should render expression function info, example and documentation link", async () => { - await setup(); + await setup({ helpText: getHelpText("datetime-diff", database, "UTC") }); expect( screen.getByText('datetimeDiff([Created At], [Shipped At], "month")'), @@ -39,7 +44,7 @@ describe("ExpressionEditorHelpText", () => { }); it("should handle expression function without arguments", async () => { - await setup({ helpText: getHelpText("cum-count", DATABASE, "UTC") }); + await setup({ helpText: getHelpText("cum-count", database, "UTC") }); expect(screen.getAllByText("CumulativeCount")).toHaveLength(2); @@ -58,7 +63,7 @@ describe("ExpressionEditorHelpText", () => { it("should render function arguments", async () => { const { props: { helpText }, - } = await setup({ helpText: getHelpText("concat", DATABASE, "UTC") }); + } = await setup({ helpText: getHelpText("concat", database, "UTC") }); const argumentsBlock = screen.getByTestId( "expression-helper-popover-arguments", @@ -75,9 +80,7 @@ async function setup(additionalProps?: Partial<ExpressionEditorHelpTextProps>) { const target = { current: null }; const props: ExpressionEditorHelpTextProps = { - helpText: - additionalProps?.helpText || - getHelpText("datetime-diff", DATABASE, "UTC"), + helpText: additionalProps?.helpText, width: 397, target, ...additionalProps, diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/helper-text-strings.unit.spec.ts b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/helper-text-strings.unit.spec.ts index 127aa3f17c139b015ba410c4091770e232cd731f..04410c12473de9ca345cff0a73f0f9477bd621e0 100644 --- a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/helper-text-strings.unit.spec.ts +++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/helper-text-strings.unit.spec.ts @@ -1,6 +1,7 @@ +import { checkNotNull } from "metabase/core/utils/types"; import { createMockDatabase } from "metabase-types/api/mocks/database"; -import { Database as DatabaseApi } from "metabase-types/api"; -import Database from "metabase-lib/metadata/Database"; +import { Database } from "metabase-types/api"; +import { createMockMetadata } from "__support__/metadata"; import { getHelpText } from "./helper-text-strings"; describe("getHelpText", () => { @@ -72,8 +73,9 @@ describe("getHelpText", () => { }); }); -function setup(dbOpts?: Partial<DatabaseApi>) { - const database = new Database(createMockDatabase(dbOpts)); +function setup(dbOpts?: Partial<Database>) { + const database = createMockDatabase(dbOpts); + const metadata = createMockMetadata({ databases: [database] }); - return { database }; + return { database: checkNotNull(metadata.database(database.id)) }; } diff --git a/frontend/src/metabase/selectors/metadata.ts b/frontend/src/metabase/selectors/metadata.ts index 421754b23823c8721630b27183368bbda2619f88..ea6350b80539994f36a4ad7c007e5efc4b1486e6 100644 --- a/frontend/src/metabase/selectors/metadata.ts +++ b/frontend/src/metabase/selectors/metadata.ts @@ -113,8 +113,9 @@ export const getMetadata: ( // database hydrate(metadata.databases, "tables", database => { - if (database.tables?.length > 0) { - return database.tables + const tableIds = database.getPlainObject().tables ?? []; + if (tableIds.length > 0) { + return tableIds .map(tableId => metadata.table(tableId)) .filter(table => table != null); } @@ -141,9 +142,11 @@ export const getMetadata: ( hydrate(metadata.tables, "schema", table => metadata.schema(table.schema)); hydrate(metadata.databases, "schemas", database => { - if (database.schemas) { - return database.schemas.map(s => metadata.schema(s)); + const schemaIds = database.getPlainObject().schemas; + if (schemaIds) { + return schemaIds.map(s => metadata.schema(s)); } + return Object.values(metadata.schemas).filter( s => s.database && s.database.id === database.id, ); @@ -154,11 +157,11 @@ export const getMetadata: ( return tableIds ? // use the schema tables if they exist tableIds.map(table => metadata.table(table)) - : schema.database && schema.database.tables.length > 0 + : schema.database && schema.database.getTables().length > 0 ? // if the schema has a database with tables, use those - schema.database.tables.filter( - table => table.schema_name === schema.name, - ) + schema.database + .getTables() + .filter(table => table.schema_name === schema.name) : // otherwise use any loaded tables that match the schema id Object.values(metadata.tables).filter( table => table.schema && table.schema.id === schema.id, diff --git a/frontend/test/__support__/sample_database_fixture.ts b/frontend/test/__support__/sample_database_fixture.ts index 549eb1a7ce743510e6edfd4fe0285e0ae7c18080..930e1a59e758aa088314a4ffc53e7b07e6ba397e 100644 --- a/frontend/test/__support__/sample_database_fixture.ts +++ b/frontend/test/__support__/sample_database_fixture.ts @@ -33,7 +33,7 @@ function aliasTablesAndFields(metadata: Metadata) { // NOTE: this assume names don't conflict with other properties in Database/Table which I think is safe for Sample Database /* eslint-disable @typescript-eslint/ban-ts-comment */ for (const database of Object.values(metadata.databases)) { - for (const table of database.tables) { + for (const table of database.getTables()) { if (!(table.name in database)) { // @ts-ignore database[table.name] = table; diff --git a/frontend/test/metabase-lib/lib/queries/StructuredQuery-clean.unit.spec.js b/frontend/test/metabase-lib/lib/queries/StructuredQuery-clean.unit.spec.js index c6fd18c7a674d4a4905ee700e56a6cd8f5671db7..b0d3fa58a63812fa77d059dfbe533fb9256f82df 100644 --- a/frontend/test/metabase-lib/lib/queries/StructuredQuery-clean.unit.spec.js +++ b/frontend/test/metabase-lib/lib/queries/StructuredQuery-clean.unit.spec.js @@ -250,7 +250,7 @@ describe("StructuredQuery", () => { ).toEqual({ type: "query", database: SAMPLE_DATABASE.id, - query: { "source-table": null }, + query: { "source-table": undefined }, }); }); it("should remove outer empty queries", () => {