From 67ebff69cc82118c864f689606785cb55aa86e49 Mon Sep 17 00:00:00 2001 From: Anton Kulyk <kuliks.anton@gmail.com> Date: Wed, 15 Jun 2022 14:23:13 +0100 Subject: [PATCH] Convert `initializeQB` to TypeScript (#23265) * Convert `initializeQB` to TypeScript * Extract parameter utils * Minor move * Simplify typings * Move cljs `normalize` to js file * Make optional parameter explicit --- frontend/src/metabase-types/store/state.ts | 4 + frontend/src/metabase-types/types/Card.ts | 5 + frontend/src/metabase/lib/query/normalize.js | 1 + .../src/metabase/parameters/utils/cards.ts | 2 +- .../core/{initializeQB.js => initializeQB.ts} | 251 +++++++----------- .../actions/core/parameterUtils.ts | 152 +++++++++++ .../query_builder/actions/navigation.js | 4 +- .../query_builder/actions/querying.js | 2 +- .../src/metabase/query_builder/typed-utils.ts | 27 ++ frontend/src/metabase/query_builder/utils.js | 20 -- 10 files changed, 295 insertions(+), 173 deletions(-) create mode 100644 frontend/src/metabase/lib/query/normalize.js rename frontend/src/metabase/query_builder/actions/core/{initializeQB.js => initializeQB.ts} (58%) create mode 100644 frontend/src/metabase/query_builder/actions/core/parameterUtils.ts create mode 100644 frontend/src/metabase/query_builder/typed-utils.ts diff --git a/frontend/src/metabase-types/store/state.ts b/frontend/src/metabase-types/store/state.ts index 9a7613ef12d..14bcca8d9a8 100644 --- a/frontend/src/metabase-types/store/state.ts +++ b/frontend/src/metabase-types/store/state.ts @@ -19,3 +19,7 @@ export interface State { settings: SettingsState; setup: SetupState; } + +export type Dispatch<T = unknown> = (action: T) => void; + +export type GetState = () => State; diff --git a/frontend/src/metabase-types/types/Card.ts b/frontend/src/metabase-types/types/Card.ts index e81ebbdc93f..a49e8f7e60a 100644 --- a/frontend/src/metabase-types/types/Card.ts +++ b/frontend/src/metabase-types/types/Card.ts @@ -11,6 +11,10 @@ export type UnsavedCard<Query = DatasetQuery> = { visualization_settings: VisualizationSettings; parameters?: Array<Parameter>; + // If coming from dashboard + dashboardId?: number; + dashcardId?: number; + // Not part of the card API contract, a field used by query builder for showing lineage original_card_id?: CardId; }; @@ -22,6 +26,7 @@ export type SavedCard<Query = DatasetQuery> = UnsavedCard<Query> & { dataset?: boolean; can_write: boolean; public_uuid: string; + archived?: boolean; }; export type Card<Query = DatasetQuery> = SavedCard<Query> | UnsavedCard<Query>; diff --git a/frontend/src/metabase/lib/query/normalize.js b/frontend/src/metabase/lib/query/normalize.js new file mode 100644 index 00000000000..a9f9052324f --- /dev/null +++ b/frontend/src/metabase/lib/query/normalize.js @@ -0,0 +1 @@ +export { normalize } from "cljs/metabase.mbql.js"; diff --git a/frontend/src/metabase/parameters/utils/cards.ts b/frontend/src/metabase/parameters/utils/cards.ts index 2bfbf5acc27..feeadb84f68 100644 --- a/frontend/src/metabase/parameters/utils/cards.ts +++ b/frontend/src/metabase/parameters/utils/cards.ts @@ -92,7 +92,7 @@ export function getParametersFromCard( export function getValueAndFieldIdPopulatedParametersFromCard( card: Card, metadata: Metadata, - parameterValues: { [key: string]: any }, + parameterValues: { [key: string]: any } = {}, parameters = getParametersFromCard(card), ) { if (!card) { diff --git a/frontend/src/metabase/query_builder/actions/core/initializeQB.js b/frontend/src/metabase/query_builder/actions/core/initializeQB.ts similarity index 58% rename from frontend/src/metabase/query_builder/actions/core/initializeQB.js rename to frontend/src/metabase/query_builder/actions/core/initializeQB.ts index 2f219bddc44..9b179ca7cc4 100644 --- a/frontend/src/metabase/query_builder/actions/core/initializeQB.js +++ b/frontend/src/metabase/query_builder/actions/core/initializeQB.ts @@ -1,15 +1,14 @@ import _ from "underscore"; import querystring from "querystring"; -import { normalize } from "cljs/metabase.mbql.js"; +import { LocationDescriptorObject } from "history"; import * as MetabaseAnalytics from "metabase/lib/analytics"; import { deserializeCardFromUrl, loadCard } from "metabase/lib/card"; +import { normalize } from "metabase/lib/query/normalize"; import * as Urls from "metabase/lib/urls"; import { cardIsEquivalent } from "metabase/meta/Card"; -import { DashboardApi } from "metabase/services"; - import { setErrorPage } from "metabase/redux/app"; import { getMetadata } from "metabase/selectors/metadata"; import { getUser } from "metabase/selectors/user"; @@ -17,17 +16,41 @@ import { getUser } from "metabase/selectors/user"; import Snippets from "metabase/entities/snippets"; import { fetchAlertsForQuestion } from "metabase/alert/alert"; -import { getValueAndFieldIdPopulatedParametersFromCard } from "metabase/parameters/utils/cards"; -import { hasMatchingParameters } from "metabase/parameters/utils/dashboards"; -import { getParameterValuesByIdFromQueryParams } from "metabase/parameters/utils/parameter-values"; - import Question from "metabase-lib/lib/Question"; -import { getQueryBuilderModeFromLocation } from "../../utils"; +import NativeQuery from "metabase-lib/lib/queries/NativeQuery"; +import StructuredQuery from "metabase-lib/lib/queries/StructuredQuery"; + +import { + Dispatch, + GetState, + QueryBuilderUIControls, +} from "metabase-types/store"; +import { Card, SavedCard } from "metabase-types/types/Card"; + +import { getQueryBuilderModeFromLocation } from "../../typed-utils"; import { redirectToNewQuestionFlow, updateUrl } from "../navigation"; import { cancelQuery, runQuestionQuery } from "../querying"; import { loadMetadataForCard, resetQB } from "./core"; +import { + handleDashboardParameters, + getParameterValuesForQuestion, +} from "./parameterUtils"; + +type BlankQueryOptions = { + db?: string; + table?: string; + segment?: string; + metric?: string; +}; + +type QueryParams = BlankQueryOptions & { + slug?: string; + objectId?: string; +}; + +type UIControls = Partial<QueryBuilderUIControls>; const ARCHIVED_ERROR = { data: { @@ -43,97 +66,12 @@ const NOT_FOUND_ERROR = { context: "query-builder", }; -function checkShouldPropagateDashboardParameters({ - cardId, - deserializedCard, - originalCard, -}) { - if (!deserializedCard) { - return false; - } - if (cardId && deserializedCard.parameters) { - return true; - } - if (!originalCard) { - return false; - } - const equalCards = cardIsEquivalent(deserializedCard, originalCard, { - checkParameters: false, - }); - const differentParameters = !cardIsEquivalent( - deserializedCard, - originalCard, - { checkParameters: true }, - ); - return equalCards && differentParameters; -} - -async function verifyMatchingDashcardAndParameters({ - dispatch, - dashboardId, - dashcardId, - cardId, - parameters, - metadata, -}) { - try { - const dashboard = await DashboardApi.get({ dashId: dashboardId }); - if ( - !hasMatchingParameters({ - dashboard, - dashcardId, - cardId, - parameters, - metadata, - }) - ) { - dispatch(setErrorPage({ status: 403 })); - } - } catch (error) { - dispatch(setErrorPage(error)); - } -} - -function getParameterValuesForQuestion({ card, queryParams, metadata }) { - const parameters = getValueAndFieldIdPopulatedParametersFromCard( - card, - metadata, - ); - return getParameterValuesByIdFromQueryParams( - parameters, - queryParams, - metadata, - ); -} - -async function handleDashboardParameters( - card, - { cardId, deserializedCard, originalCard, dispatch, getState }, -) { - const shouldPropagateParameters = checkShouldPropagateDashboardParameters({ - cardId, - deserializedCard, - originalCard, - }); - if (shouldPropagateParameters) { - const { dashboardId, dashcardId, parameters } = deserializedCard; - const metadata = getMetadata(getState()); - await verifyMatchingDashcardAndParameters({ - dispatch, - dashboardId, - dashcardId, - cardId: card.id, - parameters, - metadata, - }); - - card.parameters = parameters; - card.dashboardId = dashboardId; - card.dashcardId = dashcardId; - } -} - -function getCardForBlankQuestion({ db, table, segment, metric }) { +function getCardForBlankQuestion({ + db, + table, + segment, + metric, +}: BlankQueryOptions) { const databaseId = db ? parseInt(db) : undefined; const tableId = table ? parseInt(table) : undefined; @@ -141,14 +79,12 @@ function getCardForBlankQuestion({ db, table, segment, metric }) { if (databaseId && tableId) { if (segment) { - question = question - .query() + question = (question.query() as StructuredQuery) .filter(["segment", parseInt(segment)]) .question(); } if (metric) { - question = question - .query() + question = (question.query() as StructuredQuery) .aggregate(["metric", parseInt(metric)]) .question(); } @@ -157,7 +93,7 @@ function getCardForBlankQuestion({ db, table, segment, metric }) { return question.card(); } -function deserializeCard(serializedCard) { +function deserializeCard(serializedCard: string) { const card = deserializeCardFromUrl(serializedCard); if (card.dataset_query.database != null) { // Ensure older MBQL is supported @@ -166,7 +102,7 @@ function deserializeCard(serializedCard) { return card; } -async function fetchAndPrepareSavedQuestionCards(cardId) { +async function fetchAndPrepareSavedQuestionCards(cardId: number) { const card = await loadCard(cardId); const originalCard = { ...card }; @@ -177,7 +113,7 @@ async function fetchAndPrepareSavedQuestionCards(cardId) { return { card, originalCard }; } -async function fetchAndPrepareAdHocQuestionCards(deserializedCard) { +async function fetchAndPrepareAdHocQuestionCards(deserializedCard: Card) { if (!deserializedCard.original_card_id) { return { card: deserializedCard, @@ -200,7 +136,20 @@ async function fetchAndPrepareAdHocQuestionCards(deserializedCard) { }; } -function resolveCards({ cardId, deserializedCard, options }) { +type ResolveCardsResult = { + card: Card; + originalCard?: Card; +}; + +async function resolveCards({ + cardId, + deserializedCard, + options, +}: { + cardId?: number; + deserializedCard?: Card; + options: BlankQueryOptions; +}): Promise<ResolveCardsResult> { if (!cardId && !deserializedCard) { return { card: getCardForBlankQuestion(options), @@ -208,17 +157,11 @@ function resolveCards({ cardId, deserializedCard, options }) { } return cardId ? fetchAndPrepareSavedQuestionCards(cardId) - : fetchAndPrepareAdHocQuestionCards(deserializedCard); + : fetchAndPrepareAdHocQuestionCards(deserializedCard as Card); } -function getInitialUIControls(location) { - const { mode, ...uiControls } = getQueryBuilderModeFromLocation(location); - uiControls.queryBuilderMode = mode; - return uiControls; -} - -function parseHash(hash) { - let options = {}; +function parseHash(hash?: string) { + let options: BlankQueryOptions = {}; let serializedCard; // hash can contain either query params starting with ? or a base64 serialized card @@ -234,14 +177,26 @@ function parseHash(hash) { return { options, serializedCard }; } +function isSavedCard(card: Card): card is SavedCard { + return !!(card as SavedCard).id; +} + export const INITIALIZE_QB = "metabase/qb/INITIALIZE_QB"; -async function handleQBInit(dispatch, getState, { location, params }) { +async function handleQBInit( + dispatch: Dispatch, + getState: GetState, + { + location, + params, + }: { location: LocationDescriptorObject; params: QueryParams }, +) { dispatch(resetQB()); dispatch(cancelQuery()); + const queryParams = location.query; const cardId = Urls.extractEntityId(params.slug); - const uiControls = getInitialUIControls(location); + const uiControls: UIControls = getQueryBuilderModeFromLocation(location); const { options, serializedCard } = parseHash(location.hash); const hasCard = cardId || serializedCard; @@ -266,19 +221,22 @@ async function handleQBInit(dispatch, getState, { location, params }) { options, }); - if (card.archived) { + if (isSavedCard(card) && card.archived) { dispatch(setErrorPage(ARCHIVED_ERROR)); return; } - if (!card?.dataset && location.pathname.startsWith("/model")) { + if ( + isSavedCard(card) && + !card?.dataset && + location.pathname?.startsWith("/model") + ) { dispatch(setErrorPage(NOT_FOUND_ERROR)); return; } if (hasCard) { await handleDashboardParameters(card, { - cardId, deserializedCard, originalCard, dispatch, @@ -296,16 +254,15 @@ async function handleQBInit(dispatch, getState, { location, params }) { card.dataset_query.type, ); - if (card && card.id != null) { + if (isSavedCard(card)) { dispatch(fetchAlertsForQuestion(card.id)); } - if (card) { - await dispatch(loadMetadataForCard(card)); - } + await dispatch(loadMetadataForCard(card)); + const metadata = getMetadata(getState()); - let question = card && new Question(card, getMetadata(getState())); - if (question && question.isSaved()) { + let question = new Question(card, metadata); + if (question.isSaved()) { // Don't set viz automatically for saved questions question = question.lockDisplay(); @@ -316,25 +273,21 @@ async function handleQBInit(dispatch, getState, { location, params }) { } } - if ( - question && - question.isNative() && - question.query().hasSnippets() && - !question.query().readOnly() - ) { - await dispatch(Snippets.actions.fetchList()); - const snippets = Snippets.selectors.getList(getState()); - question = question.setQuery( - question.query().updateQueryTextWithNewSnippetNames(snippets), - ); + if (question && question.isNative()) { + const query = question.query() as NativeQuery; + if (query.hasSnippets() && !query.readOnly()) { + await dispatch(Snippets.actions.fetchList()); + const snippets = Snippets.selectors.getList(getState()); + question = question.setQuery( + query.updateQueryTextWithNewSnippetNames(snippets), + ); + } } - const queryParams = location.query; - const freshCard = question && question.card(); + const finalCard = question.card(); - const metadata = getMetadata(getState()); const parameterValues = getParameterValuesForQuestion({ - card, + card: finalCard, queryParams, metadata, }); @@ -344,7 +297,7 @@ async function handleQBInit(dispatch, getState, { location, params }) { dispatch({ type: INITIALIZE_QB, payload: { - card: freshCard, + card: finalCard, originalCard, uiControls, parameterValues, @@ -352,7 +305,7 @@ async function handleQBInit(dispatch, getState, { location, params }) { }, }); - if (question && uiControls.queryBuilderMode !== "notebook") { + if (uiControls.queryBuilderMode !== "notebook") { if (question.canRun()) { // Timeout to allow Parameters widget to set parameterValues setTimeout( @@ -361,7 +314,7 @@ async function handleQBInit(dispatch, getState, { location, params }) { ); } dispatch( - updateUrl(freshCard, { + updateUrl(finalCard, { replaceState: true, preserveParameters: hasCard, objectId, @@ -370,10 +323,10 @@ async function handleQBInit(dispatch, getState, { location, params }) { } } -export const initializeQB = (location, params) => async ( - dispatch, - getState, -) => { +export const initializeQB = ( + location: LocationDescriptorObject, + params: QueryParams, +) => async (dispatch: Dispatch, getState: GetState) => { try { await handleQBInit(dispatch, getState, { location, params }); } catch (error) { diff --git a/frontend/src/metabase/query_builder/actions/core/parameterUtils.ts b/frontend/src/metabase/query_builder/actions/core/parameterUtils.ts new file mode 100644 index 00000000000..94b1ce9f2f7 --- /dev/null +++ b/frontend/src/metabase/query_builder/actions/core/parameterUtils.ts @@ -0,0 +1,152 @@ +import _ from "underscore"; + +import { cardIsEquivalent } from "metabase/meta/Card"; + +import { DashboardApi } from "metabase/services"; + +import { setErrorPage } from "metabase/redux/app"; +import { getMetadata } from "metabase/selectors/metadata"; + +import { getValueAndFieldIdPopulatedParametersFromCard } from "metabase/parameters/utils/cards"; +import { hasMatchingParameters } from "metabase/parameters/utils/dashboards"; +import { getParameterValuesByIdFromQueryParams } from "metabase/parameters/utils/parameter-values"; + +import Metadata from "metabase-lib/lib/metadata/Metadata"; + +import { Dispatch, GetState } from "metabase-types/store"; + +import { Card, SavedCard } from "metabase-types/types/Card"; +import { Parameter } from "metabase-types/types/Parameter"; + +type BlankQueryOptions = { + db?: string; + table?: string; + segment?: string; + metric?: string; +}; + +type QueryParams = BlankQueryOptions & { + slug?: string; + objectId?: string; +}; + +function checkShouldPropagateDashboardParameters({ + cardId, + deserializedCard, + originalCard, +}: { + cardId?: number; + deserializedCard?: Card; + originalCard?: Card; +}) { + if (!deserializedCard) { + return false; + } + if (cardId && deserializedCard.parameters) { + return true; + } + if (!originalCard) { + return false; + } + const equalCards = cardIsEquivalent(deserializedCard, originalCard, { + checkParameters: false, + }); + const differentParameters = !cardIsEquivalent( + deserializedCard, + originalCard, + { checkParameters: true }, + ); + return equalCards && differentParameters; +} + +async function verifyMatchingDashcardAndParameters({ + dispatch, + dashboardId, + dashcardId, + cardId, + parameters, + metadata, +}: { + dispatch: Dispatch; + dashboardId: number; + dashcardId: number; + cardId: number; + parameters: Parameter[]; + metadata: Metadata; +}) { + try { + const dashboard = await DashboardApi.get({ dashId: dashboardId }); + if ( + !hasMatchingParameters({ + dashboard, + dashcardId, + cardId, + parameters, + metadata, + }) + ) { + dispatch(setErrorPage({ status: 403 })); + } + } catch (error) { + dispatch(setErrorPage(error)); + } +} + +export function getParameterValuesForQuestion({ + card, + queryParams, + metadata, +}: { + card: Card; + queryParams?: QueryParams; + metadata: Metadata; +}) { + const parameters = getValueAndFieldIdPopulatedParametersFromCard( + card, + metadata, + ); + return getParameterValuesByIdFromQueryParams( + parameters, + queryParams, + metadata, + ); +} + +export async function handleDashboardParameters( + card: Card, + { + deserializedCard, + originalCard, + dispatch, + getState, + }: { + cardId?: number; + deserializedCard?: Card; + originalCard?: Card; + dispatch: Dispatch; + getState: GetState; + }, +) { + const cardId = (card as SavedCard).id; + const shouldPropagateParameters = checkShouldPropagateDashboardParameters({ + cardId, + deserializedCard, + originalCard, + }); + if (shouldPropagateParameters && deserializedCard) { + const { dashboardId, dashcardId, parameters } = deserializedCard; + const metadata = getMetadata(getState()); + await verifyMatchingDashcardAndParameters({ + dispatch, + cardId, + metadata, + dashboardId: dashboardId as number, + dashcardId: dashcardId as number, + parameters: parameters as Parameter[], + }); + + card.parameters = parameters; + card.dashboardId = dashboardId; + card.dashcardId = dashcardId; + } +} diff --git a/frontend/src/metabase/query_builder/actions/navigation.js b/frontend/src/metabase/query_builder/actions/navigation.js index f83705cbe7b..4f04de7e507 100644 --- a/frontend/src/metabase/query_builder/actions/navigation.js +++ b/frontend/src/metabase/query_builder/actions/navigation.js @@ -20,10 +20,10 @@ import { getQueryBuilderMode, getQuestion, } from "../selectors"; +import { getQueryBuilderModeFromLocation } from "../typed-utils"; import { getCurrentQueryParams, getPathNameFromQueryBuilderMode, - getQueryBuilderModeFromLocation, getURLForCardState, } from "../utils"; @@ -68,7 +68,7 @@ export const popState = createThunkAction( } const { - mode: queryBuilderModeFromURL, + queryBuilderMode: queryBuilderModeFromURL, ...uiControls } = getQueryBuilderModeFromLocation(location); diff --git a/frontend/src/metabase/query_builder/actions/querying.js b/frontend/src/metabase/query_builder/actions/querying.js index dc1637c4949..307502d8bac 100644 --- a/frontend/src/metabase/query_builder/actions/querying.js +++ b/frontend/src/metabase/query_builder/actions/querying.js @@ -84,7 +84,7 @@ export const RUN_QUERY = "metabase/qb/RUN_QUERY"; export const runQuestionQuery = ({ shouldUpdateUrl = true, ignoreCache = false, - overrideWithCard, + overrideWithCard = null, } = {}) => { return async (dispatch, getState) => { dispatch(loadStartUIControls()); diff --git a/frontend/src/metabase/query_builder/typed-utils.ts b/frontend/src/metabase/query_builder/typed-utils.ts new file mode 100644 index 00000000000..27331d270f3 --- /dev/null +++ b/frontend/src/metabase/query_builder/typed-utils.ts @@ -0,0 +1,27 @@ +import { LocationDescriptorObject } from "history"; +import { QueryBuilderMode, DatasetEditorTab } from "metabase-types/store"; + +type LocationQBModeResult = { + queryBuilderMode: QueryBuilderMode; + datasetEditorTab?: DatasetEditorTab; +}; + +export function getQueryBuilderModeFromLocation( + location: LocationDescriptorObject, +): LocationQBModeResult { + const { pathname } = location; + if (pathname?.endsWith("/notebook")) { + return { + queryBuilderMode: "notebook", + }; + } + if (pathname?.endsWith("/query") || pathname?.endsWith("/metadata")) { + return { + queryBuilderMode: "dataset", + datasetEditorTab: pathname.endsWith("/query") ? "query" : "metadata", + }; + } + return { + queryBuilderMode: "view", + }; +} diff --git a/frontend/src/metabase/query_builder/utils.js b/frontend/src/metabase/query_builder/utils.js index 48ae8571811..7ae3f902fed 100644 --- a/frontend/src/metabase/query_builder/utils.js +++ b/frontend/src/metabase/query_builder/utils.js @@ -3,26 +3,6 @@ import { isSupportedTemplateTagForModel } from "metabase/lib/data-modeling/utils import * as Urls from "metabase/lib/urls"; import NativeQuery from "metabase-lib/lib/queries/NativeQuery"; -// Query Builder Mode - -export function getQueryBuilderModeFromLocation(location) { - const { pathname } = location; - if (pathname.endsWith("/notebook")) { - return { - mode: "notebook", - }; - } - if (pathname.endsWith("/query") || pathname.endsWith("/metadata")) { - return { - mode: "dataset", - datasetEditorTab: pathname.endsWith("/query") ? "query" : "metadata", - }; - } - return { - mode: "view", - }; -} - export function getPathNameFromQueryBuilderMode({ pathname, queryBuilderMode, -- GitLab