From 5b19999dfc9c05c663bbf8345299f35354aac511 Mon Sep 17 00:00:00 2001 From: Ryan Laurie <30528226+iethree@users.noreply.github.com> Date: Tue, 2 Apr 2024 10:49:46 -0600 Subject: [PATCH] Use RTK Query instead of entity framework for DatabaseCandidates (#40403) * use RTK Query instead of entity framework for DatabaseCandidates * use skipToken to tighten up API request types --- .../components/DatabaseList/DatabaseList.jsx | 2 +- frontend/src/metabase/api/api.ts | 3 +- .../src/metabase/api/database-candidates.ts | 13 +++++ frontend/src/metabase/api/index.ts | 1 + .../common/hooks/entity-framework/index.ts | 1 - .../index.ts | 1 - .../use-database-candidate-list-query.ts | 27 ---------- ...atabase-candidate-list-query.unit.spec.tsx | 53 ------------------- .../DatabaseSyncModal/DatabaseSyncModal.tsx | 39 ++++++++++++-- .../DatabaseSyncModal.unit.spec.tsx | 6 +-- .../components/DatabaseSyncModal/index.ts | 3 +- .../DatabaseSyncModal/DatabaseSyncModal.tsx | 52 ------------------ .../containers/DatabaseSyncModal/index.ts | 2 - .../metabase/entities/database-candidates.js | 16 ------ frontend/src/metabase/entities/index.js | 1 - .../HomeXraySection/HomeXraySection.tsx | 13 ++--- 16 files changed, 62 insertions(+), 171 deletions(-) create mode 100644 frontend/src/metabase/api/database-candidates.ts delete mode 100644 frontend/src/metabase/common/hooks/entity-framework/use-database-candidate-list-query/index.ts delete mode 100644 frontend/src/metabase/common/hooks/entity-framework/use-database-candidate-list-query/use-database-candidate-list-query.ts delete mode 100644 frontend/src/metabase/common/hooks/entity-framework/use-database-candidate-list-query/use-database-candidate-list-query.unit.spec.tsx delete mode 100644 frontend/src/metabase/databases/containers/DatabaseSyncModal/DatabaseSyncModal.tsx delete mode 100644 frontend/src/metabase/databases/containers/DatabaseSyncModal/index.ts delete mode 100644 frontend/src/metabase/entities/database-candidates.js diff --git a/frontend/src/metabase/admin/databases/components/DatabaseList/DatabaseList.jsx b/frontend/src/metabase/admin/databases/components/DatabaseList/DatabaseList.jsx index c7cb2e6f626..a028ab9955b 100644 --- a/frontend/src/metabase/admin/databases/components/DatabaseList/DatabaseList.jsx +++ b/frontend/src/metabase/admin/databases/components/DatabaseList/DatabaseList.jsx @@ -11,7 +11,7 @@ import FormMessage from "metabase/components/form/FormMessage"; import AdminS from "metabase/css/admin.module.css"; import ButtonsS from "metabase/css/components/buttons.module.css"; import CS from "metabase/css/core/index.css"; -import DatabaseSyncModal from "metabase/databases/containers/DatabaseSyncModal"; +import { DatabaseSyncModal } from "metabase/databases/components/DatabaseSyncModal"; import { isSyncCompleted } from "metabase/lib/syncing"; import { PLUGIN_FEATURE_LEVEL_PERMISSIONS } from "metabase/plugins"; diff --git a/frontend/src/metabase/api/api.ts b/frontend/src/metabase/api/api.ts index f16782be4f3..a888f8004e3 100644 --- a/frontend/src/metabase/api/api.ts +++ b/frontend/src/metabase/api/api.ts @@ -1,4 +1,5 @@ -import { createApi } from "@reduxjs/toolkit/query/react"; +import { createApi, skipToken } from "@reduxjs/toolkit/query/react"; +export { skipToken }; import { apiQuery } from "./query"; import { TAG_TYPES } from "./tags"; diff --git a/frontend/src/metabase/api/database-candidates.ts b/frontend/src/metabase/api/database-candidates.ts new file mode 100644 index 00000000000..c7d93392b39 --- /dev/null +++ b/frontend/src/metabase/api/database-candidates.ts @@ -0,0 +1,13 @@ +import { Api } from "metabase/api"; +import type { DatabaseCandidate, DatabaseId } from "metabase-types/api"; + +// a "database candidate" is a set of sample x-rays we suggest for new users +export const databaseCandidatesApi = Api.injectEndpoints({ + endpoints: builder => ({ + listDatabaseCandidates: builder.query<DatabaseCandidate[], DatabaseId>({ + query: id => `/api/automagic-dashboards/database/${id}/candidates`, + }), + }), +}); + +export const { useListDatabaseCandidatesQuery } = databaseCandidatesApi; diff --git a/frontend/src/metabase/api/index.ts b/frontend/src/metabase/api/index.ts index 868d5d6335b..50e0ae088f9 100644 --- a/frontend/src/metabase/api/index.ts +++ b/frontend/src/metabase/api/index.ts @@ -2,6 +2,7 @@ export * from "./api"; export * from "./api-key"; export * from "./card"; export * from "./database"; +export * from "./database-candidates"; export * from "./dataset"; export * from "./field"; export * from "./login-history"; diff --git a/frontend/src/metabase/common/hooks/entity-framework/index.ts b/frontend/src/metabase/common/hooks/entity-framework/index.ts index 2e1837c7037..cf956d375d2 100644 --- a/frontend/src/metabase/common/hooks/entity-framework/index.ts +++ b/frontend/src/metabase/common/hooks/entity-framework/index.ts @@ -9,7 +9,6 @@ export * from "./use-bookmark-list-query"; export * from "./use-collection-list-query"; export * from "./use-collection-query"; export * from "./use-dashboard-query"; -export * from "./use-database-candidate-list-query"; export * from "./use-database-list-query"; export * from "./use-database-query"; export * from "./use-group-list-query"; diff --git a/frontend/src/metabase/common/hooks/entity-framework/use-database-candidate-list-query/index.ts b/frontend/src/metabase/common/hooks/entity-framework/use-database-candidate-list-query/index.ts deleted file mode 100644 index 70cff8c35b0..00000000000 --- a/frontend/src/metabase/common/hooks/entity-framework/use-database-candidate-list-query/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from "./use-database-candidate-list-query"; diff --git a/frontend/src/metabase/common/hooks/entity-framework/use-database-candidate-list-query/use-database-candidate-list-query.ts b/frontend/src/metabase/common/hooks/entity-framework/use-database-candidate-list-query/use-database-candidate-list-query.ts deleted file mode 100644 index 15b4157797e..00000000000 --- a/frontend/src/metabase/common/hooks/entity-framework/use-database-candidate-list-query/use-database-candidate-list-query.ts +++ /dev/null @@ -1,27 +0,0 @@ -import DatabaseCandidates from "metabase/entities/database-candidates"; -import type { - DatabaseCandidateListQuery, - DatabaseCandidate, -} from "metabase-types/api"; - -import type { - UseEntityListQueryProps, - UseEntityListQueryResult, -} from "../use-entity-list-query"; -import { useEntityListQuery } from "../use-entity-list-query"; - -/** - * @deprecated use "metabase/api" instead - */ -export const useDatabaseCandidateListQuery = ( - props: UseEntityListQueryProps<DatabaseCandidateListQuery> = {}, -): UseEntityListQueryResult<DatabaseCandidate> => { - return useEntityListQuery(props, { - fetchList: DatabaseCandidates.actions.fetchList, - getList: DatabaseCandidates.selectors.getList, - getLoading: DatabaseCandidates.selectors.getLoading, - getLoaded: DatabaseCandidates.selectors.getLoaded, - getError: DatabaseCandidates.selectors.getError, - getListMetadata: DatabaseCandidates.selectors.getListMetadata, - }); -}; diff --git a/frontend/src/metabase/common/hooks/entity-framework/use-database-candidate-list-query/use-database-candidate-list-query.unit.spec.tsx b/frontend/src/metabase/common/hooks/entity-framework/use-database-candidate-list-query/use-database-candidate-list-query.unit.spec.tsx deleted file mode 100644 index 2c04233dcde..00000000000 --- a/frontend/src/metabase/common/hooks/entity-framework/use-database-candidate-list-query/use-database-candidate-list-query.unit.spec.tsx +++ /dev/null @@ -1,53 +0,0 @@ -import { setupDatabaseCandidatesEndpoint } from "__support__/server-mocks"; -import { - renderWithProviders, - screen, - waitForLoaderToBeRemoved, -} from "__support__/ui"; -import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper"; -import { createMockDatabaseCandidate } from "metabase-types/api/mocks"; - -import { useDatabaseCandidateListQuery } from "./use-database-candidate-list-query"; - -const TEST_DB_ID = 1; -const TEST_DB_CANDIDATE = createMockDatabaseCandidate(); - -const TestComponent = () => { - const { - data = [], - isLoading, - error, - } = useDatabaseCandidateListQuery({ - query: { id: TEST_DB_ID }, - }); - - if (isLoading || error) { - return <LoadingAndErrorWrapper loading={isLoading} error={error} />; - } - - return ( - <div> - {data.map((item, index) => ( - <div key={index}>{item.schema}</div> - ))} - </div> - ); -}; - -const setup = () => { - setupDatabaseCandidatesEndpoint(TEST_DB_ID, [TEST_DB_CANDIDATE]); - renderWithProviders(<TestComponent />); -}; - -describe("useDatabaseCandidateListQuery", () => { - it("should be initially loading", () => { - setup(); - expect(screen.getByTestId("loading-spinner")).toBeInTheDocument(); - }); - - it("should show data from the response", async () => { - setup(); - await waitForLoaderToBeRemoved(); - expect(screen.getByText(TEST_DB_CANDIDATE.schema)).toBeInTheDocument(); - }); -}); diff --git a/frontend/src/metabase/databases/components/DatabaseSyncModal/DatabaseSyncModal.tsx b/frontend/src/metabase/databases/components/DatabaseSyncModal/DatabaseSyncModal.tsx index 7eb826ab8a2..76dbca8f2c8 100644 --- a/frontend/src/metabase/databases/components/DatabaseSyncModal/DatabaseSyncModal.tsx +++ b/frontend/src/metabase/databases/components/DatabaseSyncModal/DatabaseSyncModal.tsx @@ -1,9 +1,13 @@ import cx from "classnames"; import { jt, t } from "ttag"; +import { useListDatabaseCandidatesQuery, skipToken } from "metabase/api"; +import { useDatabaseListQuery } from "metabase/common/hooks"; +import { useSetting } from "metabase/common/hooks/use-setting"; import Button from "metabase/core/components/Button"; import Link from "metabase/core/components/Link"; import ButtonsS from "metabase/css/components/buttons.module.css"; +import type { DatabaseCandidate, TableCandidate } from "metabase-types/api"; import { ModalMessage, @@ -14,12 +18,38 @@ import { ModalCloseIcon, } from "./DatabaseSyncModal.styled"; +const getSampleUrl = (candidates: DatabaseCandidate[]) => { + const tables = candidates.flatMap(d => d.tables); + const table = + tables.find((t: TableCandidate) => t.title.includes("Orders")) ?? tables[0]; + return table?.url; +}; + +const useSampleDatabaseLink = () => { + const { data: databases } = useDatabaseListQuery(); + const xraysEnabled = useSetting("enable-xrays"); + const sampleDatabase = databases?.find(d => d.is_sample); + + const { data: databaseCandidates } = useListDatabaseCandidatesQuery( + sampleDatabase?.id && xraysEnabled ? sampleDatabase.id : skipToken, + ); + + const sampleUrl = databaseCandidates + ? getSampleUrl(databaseCandidates) + : undefined; + + return sampleUrl; +}; + export interface DatabaseSyncModalProps { sampleUrl?: string; onClose?: () => void; } -const DatabaseSyncModal = ({ sampleUrl, onClose }: DatabaseSyncModalProps) => { +export const DatabaseSyncModalView = ({ + sampleUrl, + onClose, +}: DatabaseSyncModalProps) => { return ( <ModalRoot> <ModalBody> @@ -56,5 +86,8 @@ const DatabaseSyncModal = ({ sampleUrl, onClose }: DatabaseSyncModalProps) => { ); }; -// eslint-disable-next-line import/no-default-export -- deprecated usage -export default DatabaseSyncModal; +export const DatabaseSyncModal = ({ onClose }: { onClose: () => void }) => { + const sampleUrl = useSampleDatabaseLink(); + + return <DatabaseSyncModalView sampleUrl={sampleUrl} onClose={onClose} />; +}; diff --git a/frontend/src/metabase/databases/components/DatabaseSyncModal/DatabaseSyncModal.unit.spec.tsx b/frontend/src/metabase/databases/components/DatabaseSyncModal/DatabaseSyncModal.unit.spec.tsx index 30a77cac762..216c0194735 100644 --- a/frontend/src/metabase/databases/components/DatabaseSyncModal/DatabaseSyncModal.unit.spec.tsx +++ b/frontend/src/metabase/databases/components/DatabaseSyncModal/DatabaseSyncModal.unit.spec.tsx @@ -1,11 +1,11 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import DatabaseSyncModal from "./DatabaseSyncModal"; +import { DatabaseSyncModalView } from "./DatabaseSyncModal"; describe("DatabaseSyncModal", () => { it("should render with a table from the sample database", () => { - render(<DatabaseSyncModal sampleUrl={"/auto/table/1"} />); + render(<DatabaseSyncModalView sampleUrl={"/auto/table/1"} />); expect(screen.getByText("Explore sample data")).toBeInTheDocument(); }); @@ -13,7 +13,7 @@ describe("DatabaseSyncModal", () => { it("should render with no sample database", async () => { const onClose = jest.fn(); - render(<DatabaseSyncModal onClose={onClose} />); + render(<DatabaseSyncModalView onClose={onClose} />); await userEvent.click(screen.getByText("Got it")); expect(onClose).toHaveBeenCalled(); diff --git a/frontend/src/metabase/databases/components/DatabaseSyncModal/index.ts b/frontend/src/metabase/databases/components/DatabaseSyncModal/index.ts index 937585c38cf..c17a9ef0d87 100644 --- a/frontend/src/metabase/databases/components/DatabaseSyncModal/index.ts +++ b/frontend/src/metabase/databases/components/DatabaseSyncModal/index.ts @@ -1,2 +1 @@ -// eslint-disable-next-line import/no-default-export -- deprecated usage -export { default } from "./DatabaseSyncModal"; +export * from "./DatabaseSyncModal"; diff --git a/frontend/src/metabase/databases/containers/DatabaseSyncModal/DatabaseSyncModal.tsx b/frontend/src/metabase/databases/containers/DatabaseSyncModal/DatabaseSyncModal.tsx deleted file mode 100644 index b6ee6fb3246..00000000000 --- a/frontend/src/metabase/databases/containers/DatabaseSyncModal/DatabaseSyncModal.tsx +++ /dev/null @@ -1,52 +0,0 @@ -import { createSelector } from "@reduxjs/toolkit"; -import { connect } from "react-redux"; -import _ from "underscore"; - -import DatabaseCandidates from "metabase/entities/database-candidates"; -import Databases from "metabase/entities/databases"; -import { getSetting } from "metabase/selectors/settings"; -import type Database from "metabase-lib/v1/metadata/Database"; -import type { DatabaseCandidate } from "metabase-types/api"; -import type { State } from "metabase-types/store"; - -import DatabaseSyncModal from "../../components/DatabaseSyncModal"; - -interface DatabaseProps { - databases: Database[]; -} - -interface CandidatesProps { - databaseCandidates: DatabaseCandidate[]; -} - -const getSampleQuery = createSelector( - (state: State, props: DatabaseProps) => props.databases, - (state: State) => getSetting(state, "enable-xrays"), - (databases, enableXrays) => { - const sampleDatabase = databases.find(d => d.is_sample); - - if (sampleDatabase && enableXrays) { - return { id: sampleDatabase.id }; - } - }, -); - -const getSampleUrl = createSelector( - (state: unknown, props: CandidatesProps) => props.databaseCandidates, - candidates => { - const tables = candidates.flatMap(d => d.tables); - const table = tables.find(t => t.title.includes("Orders")) ?? tables[0]; - return table?.url; - }, -); - -const mapStateToProps = (state: unknown, props: CandidatesProps) => ({ - sampleUrl: getSampleUrl(state, props), -}); - -// eslint-disable-next-line import/no-default-export -- deprecated usage -export default _.compose( - Databases.loadList(), - DatabaseCandidates.loadList({ query: getSampleQuery }), - connect(mapStateToProps), -)(DatabaseSyncModal); diff --git a/frontend/src/metabase/databases/containers/DatabaseSyncModal/index.ts b/frontend/src/metabase/databases/containers/DatabaseSyncModal/index.ts deleted file mode 100644 index 937585c38cf..00000000000 --- a/frontend/src/metabase/databases/containers/DatabaseSyncModal/index.ts +++ /dev/null @@ -1,2 +0,0 @@ -// eslint-disable-next-line import/no-default-export -- deprecated usage -export { default } from "./DatabaseSyncModal"; diff --git a/frontend/src/metabase/entities/database-candidates.js b/frontend/src/metabase/entities/database-candidates.js deleted file mode 100644 index 91263ef756b..00000000000 --- a/frontend/src/metabase/entities/database-candidates.js +++ /dev/null @@ -1,16 +0,0 @@ -import { createEntity } from "metabase/lib/entities"; -import { AutoApi } from "metabase/services"; - -/** - * @deprecated use "metabase/api" instead - */ -const DatabaseCandidates = createEntity({ - name: "databaseCandidates", - api: { - list: async (query = {}) => { - return query.id ? AutoApi.db_candidates(query) : []; - }, - }, -}); - -export default DatabaseCandidates; diff --git a/frontend/src/metabase/entities/index.js b/frontend/src/metabase/entities/index.js index 30e02cb3139..0d6e1dd3fef 100644 --- a/frontend/src/metabase/entities/index.js +++ b/frontend/src/metabase/entities/index.js @@ -3,7 +3,6 @@ export { default as alerts } from "./alerts"; export { default as collections } from "./collections"; export { default as snippetCollections } from "./snippet-collections"; export { default as dashboards } from "./dashboards"; -export { default as databaseCandidates } from "./database-candidates"; export { default as pulses } from "./pulses"; export { default as questions } from "./questions"; export { ModelIndexes as modelIndexes } from "./model-indexes"; diff --git a/frontend/src/metabase/home/components/HomeXraySection/HomeXraySection.tsx b/frontend/src/metabase/home/components/HomeXraySection/HomeXraySection.tsx index 3dc7c70bfbd..a3b9d2497cf 100644 --- a/frontend/src/metabase/home/components/HomeXraySection/HomeXraySection.tsx +++ b/frontend/src/metabase/home/components/HomeXraySection/HomeXraySection.tsx @@ -3,10 +3,8 @@ import { useCallback, useMemo, useState } from "react"; import { t } from "ttag"; import _ from "underscore"; -import { - useDatabaseCandidateListQuery, - useDatabaseListQuery, -} from "metabase/common/hooks"; +import { skipToken, useListDatabaseCandidatesQuery } from "metabase/api"; +import { useDatabaseListQuery } from "metabase/common/hooks"; import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper"; import Select from "metabase/core/components/Select"; import { useSelector } from "metabase/lib/redux"; @@ -33,10 +31,9 @@ import { export const HomeXraySection = () => { const databaseListState = useDatabaseListQuery(); const database = getXrayDatabase(databaseListState.data); - const candidateListState = useDatabaseCandidateListQuery({ - query: database ? { id: database.id } : undefined, - enabled: database != null, - }); + const candidateListState = useListDatabaseCandidatesQuery( + database?.id ?? skipToken, + ); const isLoading = databaseListState.isLoading || candidateListState.isLoading; const error = databaseListState.error ?? candidateListState.error; -- GitLab