diff --git a/superset-frontend/playwright/tests/experimental/README.md b/superset-frontend/playwright/tests/experimental/README.md index a1511695b18..55a2399170a 100644 --- a/superset-frontend/playwright/tests/experimental/README.md +++ b/superset-frontend/playwright/tests/experimental/README.md @@ -28,12 +28,14 @@ This directory contains **experimental** Playwright E2E tests that are being dev ### Running Tests **By default (CI and local), experimental tests are EXCLUDED:** + ```bash npm run playwright:test # Only runs stable tests (tests/auth/*) ``` **To include experimental tests, set the environment variable:** + ```bash INCLUDE_EXPERIMENTAL=true npm run playwright:test # Runs all tests including experimental/ @@ -60,6 +62,7 @@ testIgnore: process.env.INCLUDE_EXPERIMENTAL ``` This ensures: + - Without `INCLUDE_EXPERIMENTAL`: Tests in `experimental/` are ignored - With `INCLUDE_EXPERIMENTAL=true`: All tests run, including experimental @@ -77,11 +80,13 @@ Add tests to `experimental/` when: Once an experimental test has proven stable (consistent CI passes over time): 1. **Move the test file** from `experimental/` to the appropriate stable directory: + ```bash git mv tests/experimental/dataset/my-test.spec.ts tests/dataset/my-test.spec.ts ``` 2. **Commit the move** with a clear message: + ```bash git commit -m "test(playwright): promote my-test from experimental to stable" ``` @@ -102,11 +107,13 @@ Once an experimental test has proven stable (consistent CI passes over time): **Important**: Supporting infrastructure (components, page objects, API helpers) should live in **stable locations**, NOT under `experimental/`: ✅ **Correct locations:** + - `playwright/components/` - Components used by any tests - `playwright/pages/` - Page objects for any features - `playwright/helpers/api/` - API helpers for test data setup ❌ **Avoid:** + - `playwright/tests/experimental/components/` - Makes it hard to share infrastructure This keeps infrastructure reusable and avoids duplication when tests graduate from experimental to stable. diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.ts similarity index 60% rename from superset-frontend/src/SqlLab/actions/sqlLab.js rename to superset-frontend/src/SqlLab/actions/sqlLab.ts index a4aead7e129..5d9bb391cb0 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.ts @@ -18,6 +18,8 @@ */ import { nanoid } from 'nanoid'; import rison from 'rison'; +import type { ThunkAction } from 'redux-thunk'; +import type { QueryColumn, SupersetError } from '@superset-ui/core'; import { FeatureFlag, SupersetClient, @@ -38,10 +40,76 @@ import { import { LOG_ACTIONS_SQLLAB_FETCH_FAILED_QUERY } from 'src/logger/LogUtils'; import getBootstrapData from 'src/utils/getBootstrapData'; import { logEvent } from 'src/logger/actions'; +import type { QueryEditor, SqlLabRootState, Table } from '../types'; import { newQueryTabName } from '../utils/newQueryTabName'; import getInitialState from '../reducers/getInitialState'; import { rehydratePersistedState } from '../utils/reduxStateToLocalStorageHelper'; +// Type definitions for SqlLab actions +export interface Query { + id: string; + dbId?: number; + sql: string; + sqlEditorId?: string | null; + sqlEditorImmutableId?: string; + tab?: string | null; + catalog?: string | null; + schema?: string | null; + tempTable?: string | null; + templateParams?: string; + queryLimit?: number; + runAsync?: boolean; + ctas?: boolean; + ctas_method?: string; + isDataPreview?: boolean; + progress?: number; + startDttm?: number; + endDttm?: number; + state?: string; + cached?: boolean; + resultsKey?: string | null; + updateTabState?: boolean; + tableName?: string; + link?: string; + inLocalStorage?: boolean; + executedSql?: string; + query_id?: number; +} + +export interface Database { + id: number; + allow_run_async: boolean; + disable_data_preview?: boolean; +} + +/** + * Query result data from the SQL execute API (matches QueryResultSchema) + */ +export interface SqlExecuteQueryResult { + endDttm?: number; + executedSql?: string; + limit?: number; + limitingFactor?: string; + tempTable?: string | null; + progress?: number; + state?: string; + [key: string]: unknown; +} + +/** + * Response from /api/v1/sqllab/execute/ + * This matches QueryExecutionResponseSchema from the backend + */ +export interface SqlExecuteResponse { + status?: string; + data: Record[]; + columns: QueryColumn[]; + selected_columns?: QueryColumn[]; + expanded_columns?: QueryColumn[]; + query: SqlExecuteQueryResult; + query_id?: number; +} + export const RESET_STATE = 'RESET_STATE'; export const ADD_QUERY_EDITOR = 'ADD_QUERY_EDITOR'; export const UPDATE_QUERY_EDITOR = 'UPDATE_QUERY_EDITOR'; @@ -104,6 +172,71 @@ export const SET_EDITOR_TAB_LAST_UPDATE = 'SET_EDITOR_TAB_LAST_UPDATE'; export const SET_LAST_UPDATED_ACTIVE_TAB = 'SET_LAST_UPDATED_ACTIVE_TAB'; export const CLEAR_DESTROYED_QUERY_EDITOR = 'CLEAR_DESTROYED_QUERY_EDITOR'; +// SqlLab action interface with optional properties for reducer compatibility. +// Uses explicit property types instead of `any` for better type safety. +export interface SqlLabAction { + type: string; + // Query editor related + queryEditor?: QueryEditor | Partial; + alterations?: Partial; + oldQueryEditor?: QueryEditor; + newQueryEditor?: QueryEditor; + queryEditorId?: string; + // Query related + query?: Query | Partial; + queries?: Query[]; + alteredQueries?: Record>; + queryId?: string; + // Table related + table?: Table | Partial; + tables?: Table[]; + oldTable?: Table; + newTable?: Table; + // Database related + databases?: Database[] | Record; + dbId?: number; + // State and config + sqlLabInitialState?: SqlLabRootState['sqlLab']; + sql?: string | null; + name?: string; + catalog?: string | null; + schema?: string | null; + autorun?: boolean; + templateParams?: string; + queryLimit?: number; + position?: { row: number; column: number }; + functionNames?: string[]; + northPercent?: number; + southPercent?: number; + hideLeftBar?: boolean; + // Results and errors + results?: SqlExecuteResponse; + errors?: SupersetError[]; + error?: string; + msg?: string; + link?: string; + err?: string; + // Misc + tabId?: string | number; + timestamp?: number; + interval?: number; + offline?: boolean; + datasource?: unknown; + clientId?: string; + result?: { remoteId: number }; + prepend?: boolean; + json?: { result: unknown }; + oldQueryId?: string; + newQuery?: { id: string }; +} + +type SqlLabThunkAction = ThunkAction< + R, + SqlLabRootState, + unknown, + SqlLabAction +>; + export const addInfoToast = addInfoToastAction; export const addSuccessToast = addSuccessToastAction; export const addDangerToast = addDangerToastAction; @@ -112,13 +245,14 @@ export const addWarningToast = addWarningToastAction; export const CtasEnum = { Table: 'TABLE', View: 'VIEW', -}; +} as const; + const ERR_MSG_CANT_LOAD_QUERY = t("The query couldn't be loaded"); // a map of SavedQuery field names to the different names used client-side, // because for now making the names consistent is too complicated // so it might as well only happen in one place -const queryClientMapping = { +const queryClientMapping: Record = { id: 'remoteId', db_id: 'dbId', label: 'name', @@ -127,26 +261,36 @@ const queryClientMapping = { const queryServerMapping = invert(queryClientMapping); // uses a mapping like those above to convert object key names to another style -const fieldConverter = mapping => obj => - mapKeys(obj, (value, key) => (key in mapping ? mapping[key] : key)); +const fieldConverter = + (mapping: Record) => + >(obj: T): Record => + mapKeys(obj, (_value, key) => (key in mapping ? mapping[key] : key)); export const convertQueryToServer = fieldConverter(queryServerMapping); export const convertQueryToClient = fieldConverter(queryClientMapping); -export function getUpToDateQuery(rootState, queryEditor, key) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function getUpToDateQuery( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + rootState: any, + queryEditor: Partial, + key?: string, +): QueryEditor { const { sqlLab: { unsavedQueryEditor, queryEditors }, } = rootState; const id = key ?? queryEditor.id; return { id, - ...queryEditors.find(qe => qe.id === id), + ...queryEditors.find((qe: QueryEditor) => qe.id === id), ...(id === unsavedQueryEditor.id && unsavedQueryEditor), - }; + } as QueryEditor; } -export function resetState(data) { - return (dispatch, getState) => { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function resetState(data?: Record): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (dispatch: any, getState: any) => { const { common } = getState(); const initialState = getInitialState({ ...getBootstrapData(), @@ -158,20 +302,25 @@ export function resetState(data) { type: RESET_STATE, sqlLabInitialState: initialState.sqlLab, }); - rehydratePersistedState(dispatch, initialState); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + rehydratePersistedState(dispatch, initialState as any); }; } -export function updateQueryEditor(alterations) { +export function updateQueryEditor( + alterations: Partial, +): SqlLabAction { return { type: UPDATE_QUERY_EDITOR, alterations }; } -export function setEditorTabLastUpdate(timestamp) { +export function setEditorTabLastUpdate(timestamp: number): SqlLabAction { return { type: SET_EDITOR_TAB_LAST_UPDATE, timestamp }; } -export function scheduleQuery(query) { - return dispatch => +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function scheduleQuery(query: Record): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (dispatch: any) => SupersetClient.post({ endpoint: '/api/v1/saved_query/', jsonPayload: query, @@ -191,8 +340,10 @@ export function scheduleQuery(query) { ); } -export function estimateQueryCost(queryEditor) { - return (dispatch, getState) => { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function estimateQueryCost(queryEditor: QueryEditor): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (dispatch: any, getState: any) => { const { dbId, catalog, schema, sql, selectedText, templateParams } = getUpToDateQuery(getState(), queryEditor); const requestSql = selectedText || sql; @@ -232,11 +383,11 @@ export function estimateQueryCost(queryEditor) { }; } -export function clearInactiveQueries(interval) { +export function clearInactiveQueries(interval: number): SqlLabAction { return { type: CLEAR_INACTIVE_QUERIES, interval }; } -export function startQuery(query, runPreviewOnly) { +export function startQuery(query: Query, runPreviewOnly?: boolean) { Object.assign(query, { id: query.id ? query.id : nanoid(11), progress: 0, @@ -244,14 +395,17 @@ export function startQuery(query, runPreviewOnly) { state: query.runAsync ? 'pending' : 'running', cached: false, }); - return { type: START_QUERY, query, runPreviewOnly }; + return { type: START_QUERY, query, runPreviewOnly } as const; } -export function querySuccess(query, results) { - return { type: QUERY_SUCCESS, query, results }; +export function querySuccess(query: Query, results: SqlExecuteResponse) { + return { type: QUERY_SUCCESS, query, results } as const; } -export function logFailedQuery(query, errors) { +export function logFailedQuery( + query: Query, + errors?: SupersetError[], +): SqlLabThunkAction { return function (dispatch) { const eventData = { has_err: true, @@ -259,7 +413,9 @@ export function logFailedQuery(query, errors) { ts: new Date().getTime(), }; errors?.forEach(({ error_type: errorType, message, extra }) => { - const issueCodes = extra?.issue_codes?.map(({ code }) => code) || [-1]; + const issueCodes = ( + extra as { issue_codes?: { code: number }[] } + )?.issue_codes?.map(({ code }) => code) || [-1]; dispatch( logEvent(LOG_ACTIONS_SQLLAB_FETCH_FAILED_QUERY, { ...eventData, @@ -272,35 +428,51 @@ export function logFailedQuery(query, errors) { }; } -export function createQueryFailedAction(query, msg, link, errors) { - return { type: QUERY_FAILED, query, msg, link, errors }; +export function createQueryFailedAction( + query: Query, + msg: string, + link?: string, + errors?: SupersetError[], +) { + return { type: QUERY_FAILED, query, msg, link, errors } as const; } -export function queryFailed(query, msg, link, errors) { +export function queryFailed( + query: Query, + msg: string, + link?: string, + errors?: SupersetError[], +): SqlLabThunkAction { return function (dispatch) { dispatch(logFailedQuery(query, errors)); dispatch(createQueryFailedAction(query, msg, link, errors)); }; } -export function stopQuery(query) { - return { type: STOP_QUERY, query }; +export function stopQuery(query: Query) { + return { type: STOP_QUERY, query } as const; } -export function clearQueryResults(query) { +export function clearQueryResults(query: Query): SqlLabAction { return { type: CLEAR_QUERY_RESULTS, query }; } -export function removeDataPreview(table) { +export function removeDataPreview(table: Table): SqlLabAction { return { type: REMOVE_DATA_PREVIEW, table }; } -export function requestQueryResults(query) { +export function requestQueryResults(query: Query): SqlLabAction { return { type: REQUEST_QUERY_RESULTS, query }; } -export function fetchQueryResults(query, displayLimit, timeoutInMs) { - return function (dispatch, getState) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function fetchQueryResults( + query: Query, + displayLimit?: number, + timeoutInMs?: number, +): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any, getState: any) { const { SQLLAB_QUERY_RESULT_TIMEOUT } = getState().common?.conf ?? {}; dispatch(requestQueryResults(query)); @@ -315,7 +487,9 @@ export function fetchQueryResults(query, displayLimit, timeoutInMs) { parseMethod: 'json-bigint', ...(timeout && { timeout, signal: controller.signal }), }) - .then(({ json }) => dispatch(querySuccess(query, json))) + .then(({ json }) => + dispatch(querySuccess(query, json as SqlExecuteResponse)), + ) .catch(response => { controller.abort(); getClientErrorObject(response).then(error => { @@ -332,8 +506,10 @@ export function fetchQueryResults(query, displayLimit, timeoutInMs) { }; } -export function runQuery(query, runPreviewOnly) { - return function (dispatch) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function runQuery(query: Query, runPreviewOnly?: boolean): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { dispatch(startQuery(query, runPreviewOnly)); const postPayload = { client_id: query.id, @@ -361,7 +537,7 @@ export function runQuery(query, runPreviewOnly) { }) .then(({ json }) => { if (!query.runAsync) { - dispatch(querySuccess(query, json)); + dispatch(querySuccess(query, json as SqlExecuteResponse)); } }) .catch(response => @@ -380,17 +556,20 @@ export function runQuery(query, runPreviewOnly) { }; } +// eslint-disable-next-line @typescript-eslint/no-explicit-any export function runQueryFromSqlEditor( - database, - queryEditor, - defaultQueryLimit, - tempTable, - ctas, - ctasMethod, -) { - return function (dispatch, getState) { + database: Database | null, + queryEditor: QueryEditor, + defaultQueryLimit: number, + tempTable?: string, + ctas?: boolean, + ctasMethod?: string, +): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any, getState: any) { const qe = getUpToDateQuery(getState(), queryEditor, queryEditor.id); - const query = { + const query: Query = { + id: nanoid(11), dbId: qe.dbId, sql: qe.selectedText || qe.sql, sqlEditorId: qe.tabViewId ?? qe.id, @@ -410,15 +589,19 @@ export function runQueryFromSqlEditor( }; } -export function reRunQuery(query) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function reRunQuery(query: Query): any { // run Query with a new id - return function (dispatch) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { dispatch(runQuery({ ...query, id: nanoid(11) })); }; } -export function postStopQuery(query) { - return function (dispatch) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function postStopQuery(query: Query): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { return SupersetClient.post({ endpoint: '/api/v1/query/stop', body: JSON.stringify({ client_id: query.id }), @@ -430,11 +613,17 @@ export function postStopQuery(query) { }; } -export function setDatabases(databases) { +export function setDatabases(databases: Database[]): SqlLabAction { return { type: SET_DATABASES, databases }; } -function migrateTable(table, queryEditorId, dispatch) { +function migrateTable( + table: Table, + queryEditorId: string, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + dispatch: any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any +): Promise { return SupersetClient.post({ endpoint: encodeURI('/tableschemaview/'), postPayload: { table: { ...table, queryEditorId } }, @@ -459,7 +648,13 @@ function migrateTable(table, queryEditorId, dispatch) { ); } -function migrateQuery(queryId, queryEditorId, dispatch) { +function migrateQuery( + queryId: string, + queryEditorId: string, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + dispatch: any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any +): Promise { return SupersetClient.post({ endpoint: encodeURI(`/tabstateview/${queryEditorId}/migrate_query`), postPayload: { queryId }, @@ -486,14 +681,18 @@ function migrateQuery(queryId, queryEditorId, dispatch) { * stored in local storage will also be synchronized to the backend * through syncQueryEditor. */ -export function syncQueryEditor(queryEditor) { - return function (dispatch, getState) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function syncQueryEditor(queryEditor: QueryEditor): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any, getState: any) { const { tables, queries } = getState().sqlLab; const localStorageTables = tables.filter( - table => table.inLocalStorage && table.queryEditorId === queryEditor.id, + (table: Table) => + table.inLocalStorage && table.queryEditorId === queryEditor.id, ); const localStorageQueries = Object.values(queries).filter( - query => query.inLocalStorage && query.sqlEditorId === queryEditor.id, + (query: Query) => + query.inLocalStorage && query.sqlEditorId === queryEditor.id, ); return SupersetClient.post({ endpoint: '/tabstateview/', @@ -512,11 +711,11 @@ export function syncQueryEditor(queryEditor) { newQueryEditor, }); return Promise.all([ - ...localStorageTables.map(table => - migrateTable(table, newQueryEditor.tabViewId, dispatch), + ...localStorageTables.map((table: Table) => + migrateTable(table, newQueryEditor.tabViewId!, dispatch), ), - ...localStorageQueries.map(query => - migrateQuery(query.id, newQueryEditor.tabViewId, dispatch), + ...localStorageQueries.map((query: Query) => + migrateQuery(query.id, newQueryEditor.tabViewId!, dispatch), ), ]); }) @@ -533,7 +732,9 @@ export function syncQueryEditor(queryEditor) { }; } -export function addQueryEditor(queryEditor) { +export function addQueryEditor( + queryEditor: Partial, +): SqlLabAction { const newQueryEditor = { ...queryEditor, id: nanoid(11), @@ -547,24 +748,28 @@ export function addQueryEditor(queryEditor) { }; } -export function addNewQueryEditor() { - return function (dispatch, getState) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function addNewQueryEditor(): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any, getState: any) { const { sqlLab: { queryEditors, tabHistory, unsavedQueryEditor, databases }, common, } = getState(); const defaultDbId = common.conf.SQLLAB_DEFAULT_DBID; const activeQueryEditor = queryEditors.find( - qe => qe.id === tabHistory[tabHistory.length - 1], + (qe: QueryEditor) => qe.id === tabHistory[tabHistory.length - 1], + ); + const dbIds = Object.values(databases).map( + (database: Database) => database.id, ); - const dbIds = Object.values(databases).map(database => database.id); const firstDbId = dbIds.length > 0 ? Math.min(...dbIds) : undefined; const { dbId, catalog, schema, queryLimit, autorun } = { ...queryEditors[0], ...activeQueryEditor, ...(unsavedQueryEditor.id === activeQueryEditor?.id && unsavedQueryEditor), - }; + } as Partial; const warning = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) ? '' : t( @@ -572,7 +777,7 @@ export function addNewQueryEditor() { ); const name = newQueryTabName( - queryEditors?.map(qe => ({ + queryEditors?.map((qe: QueryEditor) => ({ ...qe, ...(qe.id === unsavedQueryEditor.id && unsavedQueryEditor), })) || [], @@ -581,8 +786,8 @@ export function addNewQueryEditor() { return dispatch( addQueryEditor({ dbId: dbId || defaultDbId || firstDbId, - catalog: catalog ?? null, - schema: schema ?? null, + catalog, + schema, autorun: autorun ?? false, sql: `${warning}SELECT ...`, queryLimit: queryLimit || common.conf.DEFAULT_SQLLAB_LIMIT, @@ -592,20 +797,25 @@ export function addNewQueryEditor() { }; } -export function cloneQueryToNewTab(query, autorun) { - return function (dispatch, getState) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function cloneQueryToNewTab(query: Query, autorun: boolean): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any, getState: any) { const state = getState(); const { queryEditors, unsavedQueryEditor, tabHistory } = state.sqlLab; const sourceQueryEditor = { - ...queryEditors.find(qe => qe.id === tabHistory[tabHistory.length - 1]), + ...queryEditors.find( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (qe: any) => qe.id === tabHistory[tabHistory.length - 1], + ), ...(tabHistory[tabHistory.length - 1] === unsavedQueryEditor.id && unsavedQueryEditor), }; const queryEditor = { name: t('Copy of %s', sourceQueryEditor.name), - dbId: query.dbId ? query.dbId : null, - catalog: query.catalog ? query.catalog : null, - schema: query.schema ? query.schema : null, + dbId: query.dbId, + catalog: query.catalog, + schema: query.schema ?? undefined, autorun, sql: query.sql, queryLimit: sourceQueryEditor.queryLimit, @@ -616,26 +826,31 @@ export function cloneQueryToNewTab(query, autorun) { }; } -export function setLastUpdatedActiveTab(queryEditorId) { +export function setLastUpdatedActiveTab(queryEditorId: string): SqlLabAction { return { type: SET_LAST_UPDATED_ACTIVE_TAB, queryEditorId, }; } -export function setActiveQueryEditor(queryEditor) { +export function setActiveQueryEditor(queryEditor: QueryEditor): SqlLabAction { return { type: SET_ACTIVE_QUERY_EDITOR, queryEditor, }; } -export function switchQueryEditor(goBackward = false) { - return function (dispatch, getState) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function switchQueryEditor(goBackward = false): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any, getState: any) { const { sqlLab } = getState(); const { queryEditors, tabHistory } = sqlLab; const qeid = tabHistory[tabHistory.length - 1]; - const currentIndex = queryEditors.findIndex(qe => qe.id === qeid); + const currentIndex = queryEditors.findIndex( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (qe: any) => qe.id === qeid, + ); const nextIndex = goBackward ? currentIndex - 1 + queryEditors.length : currentIndex + 1; @@ -645,14 +860,32 @@ export function switchQueryEditor(goBackward = false) { }; } -export function loadQueryEditor(queryEditor) { +export function loadQueryEditor(queryEditor: QueryEditor): SqlLabAction { return { type: LOAD_QUERY_EDITOR, queryEditor }; } -export function setTables(tableSchemas) { +interface TableSchema { + description: { + columns: unknown[]; + selectStar: string; + primaryKey: unknown; + foreignKeys: unknown[]; + indexes: unknown[]; + dataPreviewQueryId: string; + } | null; + database_id: number; + tab_state_id: number; + catalog: string; + schema: string; + table: string; + expanded: boolean; + id: string; +} + +export function setTables(tableSchemas: TableSchema[]): SqlLabAction { const tables = tableSchemas - .filter(tableSchema => tableSchema.description !== null) - .map(tableSchema => { + .filter((tableSchema: TableSchema) => tableSchema.description !== null) + .map((tableSchema: TableSchema) => { const { columns, selectStar, @@ -660,7 +893,7 @@ export function setTables(tableSchemas) { foreignKeys, indexes, dataPreviewQueryId, - } = tableSchema.description; + } = tableSchema.description!; return { dbId: tableSchema.database_id, queryEditorId: tableSchema.tab_state_id.toString(), @@ -682,8 +915,13 @@ export function setTables(tableSchemas) { return { type: SET_TABLES, tables }; } -export function fetchQueryEditor(queryEditor, displayLimit) { - return function (dispatch) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function fetchQueryEditor( + queryEditor: QueryEditor, + displayLimit: number, +): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { const queryEditorId = queryEditor.tabViewId ?? queryEditor.id; SupersetClient.get({ endpoint: encodeURI(`/tabstateview/${queryEditorId}`), @@ -705,9 +943,9 @@ export function fetchQueryEditor(queryEditor, displayLimit) { remoteId: json.saved_query?.id, hideLeftBar: json.hide_left_bar, }; - dispatch(loadQueryEditor(loadedQueryEditor)); + dispatch(loadQueryEditor(loadedQueryEditor as unknown as QueryEditor)); dispatch(setTables(json.table_schemas || [])); - if (json.latest_query && json.latest_query.resultsKey) { + if (json.latest_query?.resultsKey) { dispatch(fetchQueryResults(json.latest_query, displayLimit)); } }) @@ -722,11 +960,11 @@ export function fetchQueryEditor(queryEditor, displayLimit) { }; } -export function setActiveSouthPaneTab(tabId) { +export function setActiveSouthPaneTab(tabId: string): SqlLabAction { return { type: SET_ACTIVE_SOUTHPANE_TAB, tabId }; } -export function toggleLeftBar(queryEditor) { +export function toggleLeftBar(queryEditor: QueryEditor): SqlLabAction { const hideLeftBar = !queryEditor.hideLeftBar; return { type: QUERY_EDITOR_TOGGLE_LEFT_BAR, @@ -735,18 +973,21 @@ export function toggleLeftBar(queryEditor) { }; } -export function clearDestoryedQueryEditor(queryEditorId) { +export function clearDestoryedQueryEditor(queryEditorId: string): SqlLabAction { return { type: CLEAR_DESTROYED_QUERY_EDITOR, queryEditorId }; } -export function removeQueryEditor(queryEditor) { +export function removeQueryEditor(queryEditor: QueryEditor): SqlLabAction { return { type: REMOVE_QUERY_EDITOR, queryEditor }; } -export function removeAllOtherQueryEditors(queryEditor) { - return function (dispatch, getState) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function removeAllOtherQueryEditors(queryEditor: QueryEditor): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any, getState: any) { const { sqlLab } = getState(); - sqlLab.queryEditors?.forEach(otherQueryEditor => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + sqlLab.queryEditors?.forEach((otherQueryEditor: any) => { if (otherQueryEditor.id !== queryEditor.id) { dispatch(removeQueryEditor(otherQueryEditor)); } @@ -754,8 +995,10 @@ export function removeAllOtherQueryEditors(queryEditor) { }; } -export function removeQuery(query) { - return function (dispatch) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function removeQuery(query: Query): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { const queryEditorId = query.sqlEditorId ?? query.id; const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) ? SupersetClient.delete({ @@ -779,11 +1022,17 @@ export function removeQuery(query) { }; } -export function queryEditorSetDb(queryEditor, dbId) { +export function queryEditorSetDb( + queryEditor: Partial, + dbId: number, +): SqlLabAction { return { type: QUERY_EDITOR_SETDB, queryEditor, dbId }; } -export function queryEditorSetCatalog(queryEditor, catalog) { +export function queryEditorSetCatalog( + queryEditor: Partial | null, + catalog: string | null, +): SqlLabAction { return { type: QUERY_EDITOR_SET_CATALOG, queryEditor: queryEditor || {}, @@ -791,7 +1040,10 @@ export function queryEditorSetCatalog(queryEditor, catalog) { }; } -export function queryEditorSetSchema(queryEditor, schema) { +export function queryEditorSetSchema( + queryEditor: Partial | null, + schema: string | null, +): SqlLabAction { return { type: QUERY_EDITOR_SET_SCHEMA, queryEditor: queryEditor || {}, @@ -799,11 +1051,18 @@ export function queryEditorSetSchema(queryEditor, schema) { }; } -export function queryEditorSetAutorun(queryEditor, autorun) { +export function queryEditorSetAutorun( + queryEditor: Partial, + autorun: boolean, +): SqlLabAction { return { type: QUERY_EDITOR_SET_AUTORUN, queryEditor, autorun }; } -export function queryEditorSetTitle(queryEditor, name, id) { +export function queryEditorSetTitle( + queryEditor: Partial, + name: string, + id: string, +): SqlLabAction { return { type: QUERY_EDITOR_SET_TITLE, queryEditor: { ...queryEditor, id }, @@ -811,13 +1070,19 @@ export function queryEditorSetTitle(queryEditor, name, id) { }; } -export function saveQuery(query, clientId) { - const { id, ...payload } = convertQueryToServer(query); +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function saveQuery(query: Partial, clientId: string): any { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { id: _id, ...payload } = convertQueryToServer( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + query as any, + ); - return dispatch => + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (dispatch: any) => SupersetClient.post({ endpoint: '/api/v1/saved_query/', - jsonPayload: convertQueryToServer(payload), + jsonPayload: convertQueryToServer(payload as Record), }) .then(result => { const savedQuery = convertQueryToClient({ @@ -830,7 +1095,7 @@ export function saveQuery(query, clientId) { clientId, result: savedQuery, }); - dispatch(queryEditorSetTitle(query, query.name, clientId)); + dispatch(queryEditorSetTitle(query, query.name ?? '', clientId)); return savedQuery; }) .catch(() => @@ -838,36 +1103,49 @@ export function saveQuery(query, clientId) { ); } +// eslint-disable-next-line @typescript-eslint/no-explicit-any export const addSavedQueryToTabState = - (queryEditor, savedQuery) => dispatch => { - const queryEditorId = queryEditor.tabViewId ?? queryEditor.id; - const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) - ? SupersetClient.put({ - endpoint: `/tabstateview/${queryEditorId}`, - postPayload: { saved_query_id: savedQuery.remoteId }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (queryEditor: QueryEditor, savedQuery: { remoteId: string }): any => + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (dispatch: any) => { + const queryEditorId = queryEditor.tabViewId ?? queryEditor.id; + const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) + ? SupersetClient.put({ + endpoint: `/tabstateview/${queryEditorId}`, + postPayload: { saved_query_id: savedQuery.remoteId }, + }) + : Promise.resolve(); + + return sync + .catch(() => { + dispatch(addDangerToast(t('Your query was not properly saved'))); }) - : Promise.resolve(); + .then(() => { + dispatch(addSuccessToast(t('Your query was saved'))); + }); + }; - return sync - .catch(() => { - dispatch(addDangerToast(t('Your query was not properly saved'))); - }) - .then(() => { - dispatch(addSuccessToast(t('Your query was saved'))); - }); - }; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function updateSavedQuery( + query: Partial, + clientId: string, +): any { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { id: _id, ...payload } = convertQueryToServer( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + query as any, + ); -export function updateSavedQuery(query, clientId) { - const { id, ...payload } = convertQueryToServer(query); - - return dispatch => + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (dispatch: any) => SupersetClient.put({ endpoint: `/api/v1/saved_query/${query.remoteId}`, - jsonPayload: convertQueryToServer(payload), + jsonPayload: convertQueryToServer(payload as Record), }) .then(() => { dispatch(addSuccessToast(t('Your query was updated'))); - dispatch(queryEditorSetTitle(query, query.name, clientId)); + dispatch(queryEditorSetTitle(query, query.name ?? '', clientId)); }) .catch(e => { const message = t('Your query could not be updated'); @@ -878,16 +1156,29 @@ export function updateSavedQuery(query, clientId) { .then(() => dispatch(updateQueryEditor(query))); } -export function queryEditorSetSql(queryEditor, sql, queryId) { +export function queryEditorSetSql( + queryEditor: Partial, + sql: string, + queryId?: string, +): SqlLabAction { return { type: QUERY_EDITOR_SET_SQL, queryEditor, sql, queryId }; } -export function queryEditorSetCursorPosition(queryEditor, position) { +export function queryEditorSetCursorPosition( + queryEditor: Partial, + position: { row: number; column: number }, +): SqlLabAction { return { type: QUERY_EDITOR_SET_CURSOR_POSITION, queryEditor, position }; } -export function queryEditorSetAndSaveSql(targetQueryEditor, sql, queryId) { - return function (dispatch, getState) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function queryEditorSetAndSaveSql( + targetQueryEditor: Partial, + sql: string, + queryId?: string, +): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any, getState: any) { const queryEditor = getUpToDateQuery(getState(), targetQueryEditor); // saved query and set tab state use this action dispatch(queryEditorSetSql(queryEditor, sql, queryId)); @@ -912,13 +1203,19 @@ export function queryEditorSetAndSaveSql(targetQueryEditor, sql, queryId) { }; } -export function formatQuery(queryEditor) { - return function (dispatch, getState) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function formatQuery(queryEditor: QueryEditor): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any, getState: any) { const { sql, dbId, templateParams } = getUpToDateQuery( getState(), queryEditor, ); - const body = { sql }; + const body: { + sql: string; + database_id?: number; + template_params?: string; + } = { sql }; // Include database_id and template_params if available for Jinja processing if (dbId) { @@ -942,7 +1239,10 @@ export function formatQuery(queryEditor) { }; } -export function queryEditorSetQueryLimit(queryEditor, queryLimit) { +export function queryEditorSetQueryLimit( + queryEditor: Partial, + queryLimit: number, +): SqlLabAction { return { type: QUERY_EDITOR_SET_QUERY_LIMIT, queryEditor, @@ -950,7 +1250,10 @@ export function queryEditorSetQueryLimit(queryEditor, queryLimit) { }; } -export function queryEditorSetTemplateParams(queryEditor, templateParams) { +export function queryEditorSetTemplateParams( + queryEditor: Partial, + templateParams: string, +): SqlLabAction { return { type: QUERY_EDITOR_SET_TEMPLATE_PARAMS, queryEditor, @@ -958,22 +1261,31 @@ export function queryEditorSetTemplateParams(queryEditor, templateParams) { }; } -export function queryEditorSetSelectedText(queryEditor, sql) { +export function queryEditorSetSelectedText( + queryEditor: Partial, + sql: string | null, +): SqlLabAction { return { type: QUERY_EDITOR_SET_SELECTED_TEXT, queryEditor, sql }; } -export function mergeTable(table, query, prepend) { +export function mergeTable( + table: Partial
, + query?: Query, + prepend?: boolean, +): SqlLabAction { return { type: MERGE_TABLE, table, query, prepend }; } +// eslint-disable-next-line @typescript-eslint/no-explicit-any export function addTable( - queryEditor, - tableName, - catalogName, - schemaName, + queryEditor: Partial, + tableName: string, + catalogName: string | null, + schemaName: string, expanded = true, -) { - return function (dispatch, getState) { +): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any, getState: any) { const { dbId } = getUpToDateQuery(getState(), queryEditor, queryEditor.id); const table = { dbId, @@ -992,8 +1304,24 @@ export function addTable( }; } -export function runTablePreviewQuery(newTable, runPreviewOnly) { - return function (dispatch, getState) { +interface NewTable { + id?: string; + dbId: number | string; + catalog?: string | null; + schema?: string; + name: string; + queryEditorId?: string; + selectStar?: string; + previewQueryId?: string; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function runTablePreviewQuery( + newTable: NewTable, + runPreviewOnly?: boolean, +): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any, getState: any) { const { sqlLab: { databases }, } = getState(); @@ -1001,12 +1329,12 @@ export function runTablePreviewQuery(newTable, runPreviewOnly) { const { dbId, catalog, schema } = newTable; if (database && !database.disable_data_preview) { - const dataPreviewQuery = { + const dataPreviewQuery: Query = { id: newTable.previewQueryId ?? nanoid(11), - dbId, - catalog, - schema, - sql: newTable.selectStar, + dbId: dbId as number | undefined, + catalog: catalog ?? undefined, + schema: schema ?? '', + sql: newTable.selectStar ?? '', tableName: newTable.name, sqlEditorId: null, tab: '', @@ -1022,9 +1350,9 @@ export function runTablePreviewQuery(newTable, runPreviewOnly) { mergeTable( { id: newTable.id, - dbId: newTable.dbId, - catalog: newTable.catalog, - schema: newTable.schema, + dbId: newTable.dbId as number | undefined, + catalog: newTable.catalog ?? undefined, + schema: newTable.schema ?? '', name: newTable.name, queryEditorId: newTable.queryEditorId, dataPreviewQueryId: dataPreviewQuery.id, @@ -1039,8 +1367,22 @@ export function runTablePreviewQuery(newTable, runPreviewOnly) { }; } -export function syncTable(table, tableMetadata, finalQueryEditorId) { - return function (dispatch) { +interface TableMetaData { + columns?: unknown[]; + selectStar?: string; + primaryKey?: unknown; + foreignKeys?: unknown[]; + indexes?: unknown[]; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function syncTable( + table: Table, + tableMetadata: TableMetaData, + finalQueryEditorId?: string, +): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { const finalTable = { ...table, queryEditorId: finalQueryEditorId }; const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) ? SupersetClient.post({ @@ -1073,13 +1415,18 @@ export function syncTable(table, tableMetadata, finalQueryEditorId) { }; } -export function changeDataPreviewId(oldQueryId, newQuery) { +export function changeDataPreviewId( + oldQueryId: string, + newQuery: Query, +): SqlLabAction { return { type: CHANGE_DATA_PREVIEW_ID, oldQueryId, newQuery }; } -export function reFetchQueryResults(query) { - return function (dispatch) { - const newQuery = { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function reFetchQueryResults(query: Query): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { + const newQuery: Query = { id: nanoid(), dbId: query.dbId, sql: query.sql, @@ -1096,8 +1443,10 @@ export function reFetchQueryResults(query) { }; } -export function expandTable(table) { - return function (dispatch) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function expandTable(table: Table): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) && table.initialized @@ -1122,8 +1471,10 @@ export function expandTable(table) { }; } -export function collapseTable(table) { - return function (dispatch) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function collapseTable(table: Table): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) && table.initialized @@ -1148,12 +1499,14 @@ export function collapseTable(table) { }; } -export function removeTables(tables) { - return function (dispatch) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function removeTables(tables: Table[]): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { const tablesToRemove = tables?.filter(Boolean) ?? []; const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) ? Promise.all( - tablesToRemove.map(table => + tablesToRemove.map((table: Table) => table.initialized ? SupersetClient.delete({ endpoint: encodeURI(`/tableschemaview/${table.id}`), @@ -1178,15 +1531,21 @@ export function removeTables(tables) { }; } -export function refreshQueries(alteredQueries) { +export function refreshQueries( + alteredQueries: Record, +): SqlLabAction { return { type: REFRESH_QUERIES, alteredQueries }; } -export function setUserOffline(offline) { +export function setUserOffline(offline: boolean): SqlLabAction { return { type: SET_USER_OFFLINE, offline }; } -export function persistEditorHeight(queryEditor, northPercent, southPercent) { +export function persistEditorHeight( + queryEditor: QueryEditor, + northPercent: number, + southPercent: number, +): SqlLabAction { return { type: QUERY_EDITOR_PERSIST_HEIGHT, queryEditor, @@ -1195,16 +1554,18 @@ export function persistEditorHeight(queryEditor, northPercent, southPercent) { }; } -export function popPermalink(key) { - return function (dispatch) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function popPermalink(key: string): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { return SupersetClient.get({ endpoint: `/api/v1/sqllab/permalink/${key}` }) .then(({ json }) => dispatch( addQueryEditor({ name: json.name ? json.name : t('Shared query'), - dbId: json.dbId ? parseInt(json.dbId, 10) : null, - catalog: json.catalog ? json.catalog : null, - schema: json.schema ? json.schema : null, + dbId: json.dbId ? parseInt(json.dbId, 10) : undefined, + catalog: json.catalog ?? null, + schema: json.schema ?? undefined, autorun: json.autorun ? json.autorun : false, sql: json.sql ? json.sql : 'SELECT ...', templateParams: json.templateParams, @@ -1215,8 +1576,10 @@ export function popPermalink(key) { }; } -export function popStoredQuery(urlId) { - return function (dispatch) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function popStoredQuery(urlId: string): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { return SupersetClient.get({ endpoint: `/api/v1/sqllab/permalink/kv:${urlId}`, }) @@ -1224,9 +1587,9 @@ export function popStoredQuery(urlId) { dispatch( addQueryEditor({ name: json.name ? json.name : t('Shared query'), - dbId: json.dbId ? parseInt(json.dbId, 10) : null, - catalog: json.catalog ? json.catalog : null, - schema: json.schema ? json.schema : null, + dbId: json.dbId ? parseInt(json.dbId, 10) : undefined, + catalog: json.catalog ?? null, + schema: json.schema ?? undefined, autorun: json.autorun ? json.autorun : false, sql: json.sql ? json.sql : 'SELECT ...', templateParams: json.templateParams, @@ -1236,8 +1599,10 @@ export function popStoredQuery(urlId) { .catch(() => dispatch(addDangerToast(ERR_MSG_CANT_LOAD_QUERY))); }; } -export function popSavedQuery(saveQueryId) { - return function (dispatch) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function popSavedQuery(saveQueryId: string): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { return SupersetClient.get({ endpoint: `/api/v1/saved_query/${saveQueryId}`, }) @@ -1246,23 +1611,25 @@ export function popSavedQuery(saveQueryId) { ...convertQueryToClient(json.result), loaded: true, autorun: false, - }; + } as Record; const tmpAdaptedProps = { - name: queryEditorProps.name, - dbId: queryEditorProps.database.id, - catalog: queryEditorProps.catalog, - schema: queryEditorProps.schema, - sql: queryEditorProps.sql, - templateParams: queryEditorProps.templateParams, - remoteId: queryEditorProps.remoteId, + name: queryEditorProps.name as string, + dbId: (queryEditorProps.database as { id: number }).id, + catalog: queryEditorProps.catalog as string, + schema: queryEditorProps.schema as string, + sql: queryEditorProps.sql as string, + templateParams: queryEditorProps.templateParams as string, + remoteId: queryEditorProps.remoteId as number | null, }; return dispatch(addQueryEditor(tmpAdaptedProps)); }) .catch(() => dispatch(addDangerToast(ERR_MSG_CANT_LOAD_QUERY))); }; } -export function popQuery(queryId) { - return function (dispatch) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function popQuery(queryId: string): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { return SupersetClient.get({ endpoint: `/api/v1/query/${queryId}`, }) @@ -1281,8 +1648,10 @@ export function popQuery(queryId) { .catch(() => dispatch(addDangerToast(ERR_MSG_CANT_LOAD_QUERY))); }; } -export function popDatasourceQuery(datasourceKey, sql) { - return function (dispatch) { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function popDatasourceQuery(datasourceKey: string, sql?: string): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (dispatch: any) { const QUERY_TEXT = t('Query'); const datasetId = datasourceKey.split('__')[0]; @@ -1310,19 +1679,30 @@ export function popDatasourceQuery(datasourceKey, sql) { ); }; } -export function createDatasourceStarted() { +export function createDatasourceStarted(): SqlLabAction { return { type: CREATE_DATASOURCE_STARTED }; } -export function createDatasourceSuccess(data) { +export function createDatasourceSuccess(data: { id: number }): SqlLabAction { const datasource = `${data.id}__table`; return { type: CREATE_DATASOURCE_SUCCESS, datasource }; } -export function createDatasourceFailed(err) { +export function createDatasourceFailed(err: string): SqlLabAction { return { type: CREATE_DATASOURCE_FAILED, err }; } -export function createDatasource(vizOptions) { - return dispatch => { +interface VizOptions { + dbId: number; + catalog?: string | null; + schema: string; + datasourceName: string; + sql: string; + templateParams?: string; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function createDatasource(vizOptions: VizOptions): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (dispatch: any) => { dispatch(createDatasourceStarted()); const { dbId, catalog, schema, datasourceName, sql, templateParams } = vizOptions; @@ -1341,7 +1721,7 @@ export function createDatasource(vizOptions) { }), }) .then(({ json }) => { - dispatch(createDatasourceSuccess(json)); + dispatch(createDatasourceSuccess(json as { id: number })); return Promise.resolve(json); }) @@ -1360,8 +1740,10 @@ export function createDatasource(vizOptions) { }; } -export function createCtasDatasource(vizOptions) { - return dispatch => { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function createCtasDatasource(vizOptions: Record): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (dispatch: any) => { dispatch(createDatasourceStarted()); return SupersetClient.post({ endpoint: '/api/v1/dataset/get_or_create/', diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.test.ts b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.test.ts index e0f49b70e31..3e4a7564673 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.test.ts +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.test.ts @@ -232,7 +232,8 @@ test('returns column keywords among selected tables', async () => { ); storeWithSqlLab.dispatch( addTable( - { id: expectQueryEditorId }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + { id: expectQueryEditorId } as any, expectTable, expectCatalog, expectSchema, @@ -276,7 +277,8 @@ test('returns column keywords among selected tables', async () => { act(() => { storeWithSqlLab.dispatch( addTable( - { id: expectQueryEditorId }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + { id: expectQueryEditorId } as any, unexpectedTable, expectCatalog, expectSchema, diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts index 1a8eb7b3684..4ab5a31964b 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts @@ -149,10 +149,10 @@ export function useKeywords( if (data.meta === 'table') { dispatch( addTable( - { id: queryEditorId, dbId, tabViewId }, + { id: String(queryEditorId), dbId: dbId as number, tabViewId }, data.value, - catalog, - schema, + catalog ?? null, + schema ?? '', false, // Don't auto-expand/switch tabs when adding via autocomplete ), ); diff --git a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx index f941fb45873..2bf1aca2843 100644 --- a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx @@ -122,7 +122,7 @@ function QueryAutoRefresh({ dispatch( queryFailed( query, - query.errorMessage, + query.errorMessage ?? '', query.extra?.errors?.[0]?.extra?.link, query.extra?.errors, ), diff --git a/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx b/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx index 17c5de8085e..048486bcdbd 100644 --- a/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx +++ b/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx @@ -344,9 +344,9 @@ export const SaveDatasetModal = ({ dispatch( createDatasource({ sql: datasource.sql, - dbId: datasource.dbId || datasource?.database?.id, - catalog: datasource?.catalog, - schema: datasource?.schema, + dbId: (datasource.dbId ?? datasource?.database?.id) as number, + catalog: datasource?.catalog ?? null, + schema: datasource?.schema ?? '', templateParams, datasourceName: datasetName, }), diff --git a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx index b154371addd..84256491ec0 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx @@ -143,7 +143,9 @@ const SouthPane = ({ ({ dbId, catalog, schema, name }) => [dbId, catalog, schema, name].join(':') === key, ); - dispatch(removeTables([table])); + if (table) { + dispatch(removeTables([table])); + } } }, [dispatch, pinnedTables], diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx index 99777784301..b3d3c75cfda 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx @@ -280,7 +280,8 @@ const SqlEditor: FC = ({ dispatch( runQueryFromSqlEditor( - database, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + database as any, queryEditor, defaultQueryLimit, ctasArg ? ctas : '', @@ -565,8 +566,8 @@ const SqlEditor: FC = ({ }; const setQueryEditorAndSaveSql = useCallback( - sql => { - dispatch(queryEditorSetAndSaveSql(queryEditor, sql)); + (sql: string) => { + dispatch(queryEditorSetAndSaveSql(queryEditor, sql, undefined)); }, [dispatch, queryEditor], ); @@ -578,7 +579,7 @@ const SqlEditor: FC = ({ const onSqlChanged = useEffectEvent((sql: string) => { currentSQL.current = sql; - dispatch(queryEditorSetSql(queryEditor, sql)); + dispatch(queryEditorSetSql(queryEditor, sql, undefined)); }); const getQueryCostEstimate = () => { diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx index 5158ef7e3c9..060ea2c6afe 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx @@ -31,6 +31,7 @@ import { setDatabases, addDangerToast, resetState, + type Database, } from 'src/SqlLab/actions/sqlLab'; import { Button, EmptyState, Icons } from '@superset-ui/core/components'; import { type DatabaseObject } from 'src/components'; @@ -194,8 +195,8 @@ const SqlEditorLeftBar = ({ queryEditorId }: SqlEditorLeftBarProps) => { ); const handleDbList = useCallback( - (result: DatabaseObject) => { - dispatch(setDatabases(result)); + (result: DatabaseObject[]) => { + dispatch(setDatabases(result as unknown as Database[])); }, [dispatch], ); diff --git a/superset-frontend/src/SqlLab/components/TableElement/index.tsx b/superset-frontend/src/SqlLab/components/TableElement/index.tsx index 251c3cb7dcc..0923db10f8e 100644 --- a/superset-frontend/src/SqlLab/components/TableElement/index.tsx +++ b/superset-frontend/src/SqlLab/components/TableElement/index.tsx @@ -171,7 +171,7 @@ const TableElement = ({ table, ...props }: TableElementProps) => { dispatch( tableApiUtil.invalidateTags([{ type: 'TableMetadatas', id: name }]), ); - dispatch(syncTable(table, tableData)); + dispatch(syncTable(table, tableData, table.queryEditorId)); }; const renderWell = () => { diff --git a/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js b/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js deleted file mode 100644 index 9fa11de0da0..00000000000 --- a/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js +++ /dev/null @@ -1,136 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import persistState from 'redux-localstorage'; -import { pickBy } from 'lodash'; -import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core'; -import { filterUnsavedQueryEditorList } from 'src/SqlLab/components/EditorAutoSync'; -import { - emptyTablePersistData, - emptyQueryResults, - clearQueryEditors, -} from '../utils/reduxStateToLocalStorageHelper'; -import { BYTES_PER_CHAR, KB_STORAGE } from '../constants'; - -const CLEAR_ENTITY_HELPERS_MAP = { - tables: emptyTablePersistData, - queries: emptyQueryResults, - queryEditors: clearQueryEditors, - unsavedQueryEditor: qe => clearQueryEditors([qe])[0], -}; - -const sqlLabPersistStateConfig = { - paths: ['sqlLab'], - config: { - slicer: paths => state => { - const subset = {}; - paths.forEach(path => { - if (isFeatureEnabled(FeatureFlag.SqllabBackendPersistence)) { - const { - queryEditors, - editorTabLastUpdatedAt, - unsavedQueryEditor, - tables, - queries, - tabHistory, - lastUpdatedActiveTab, - destroyedQueryEditors, - } = state.sqlLab; - const unsavedQueryEditors = filterUnsavedQueryEditorList( - queryEditors, - unsavedQueryEditor, - editorTabLastUpdatedAt, - ); - const hasUnsavedActiveTabState = - tabHistory.slice(-1)[0] !== lastUpdatedActiveTab; - const hasUnsavedDeletedQueryEditors = - Object.keys(destroyedQueryEditors).length > 0; - if ( - unsavedQueryEditors.length > 0 || - hasUnsavedActiveTabState || - hasUnsavedDeletedQueryEditors - ) { - const hasFinishedMigrationFromLocalStorage = - unsavedQueryEditors.every( - ({ inLocalStorage }) => !inLocalStorage, - ); - subset.sqlLab = { - queryEditors: unsavedQueryEditors, - ...(!hasFinishedMigrationFromLocalStorage && { - tabHistory, - tables: tables.filter(table => table.inLocalStorage), - queries: pickBy( - queries, - query => query.inLocalStorage && !query.isDataPreview, - ), - }), - ...(hasUnsavedActiveTabState && { - tabHistory, - }), - destroyedQueryEditors, - }; - } - return; - } - // this line is used to remove old data from browser localStorage. - // we used to persist all redux state into localStorage, but - // it caused configurations passed from server-side got override. - // see PR 6257 for details - delete state[path].common; // eslint-disable-line no-param-reassign - if (path === 'sqlLab') { - subset[path] = Object.fromEntries( - Object.entries(state[path]).map(([key, value]) => [ - key, - CLEAR_ENTITY_HELPERS_MAP[key]?.(value) ?? value, - ]), - ); - } - }); - - const data = JSON.stringify(subset); - // 2 digit precision - const currentSize = - Math.round(((data.length * BYTES_PER_CHAR) / KB_STORAGE) * 100) / 100; - if (state.localStorageUsageInKilobytes !== currentSize) { - state.localStorageUsageInKilobytes = currentSize; // eslint-disable-line no-param-reassign - } - - return subset; - }, - merge: (initialState, persistedState = {}) => { - const result = { - ...initialState, - ...persistedState, - sqlLab: { - ...persistedState?.sqlLab, - // Overwrite initialState over persistedState for sqlLab - // since a logic in getInitialState overrides the value from persistedState - ...initialState.sqlLab, - }, - }; - return result; - }, - }, -}; - -// TODO: requires redux-localstorage > 1.0 for typescript support -/** @type {any} */ -export const persistSqlLabStateEnhancer = persistState( - sqlLabPersistStateConfig.paths, - sqlLabPersistStateConfig.config, -); diff --git a/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.ts b/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.ts new file mode 100644 index 00000000000..730482cf6be --- /dev/null +++ b/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.ts @@ -0,0 +1,185 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import type { StoreEnhancer } from 'redux'; +import persistState from 'redux-localstorage'; +import { pickBy } from 'lodash'; +import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core'; +import { filterUnsavedQueryEditorList } from 'src/SqlLab/components/EditorAutoSync'; +import type { + SqlLabRootState, + QueryEditor, + UnsavedQueryEditor, + Table, +} from '../types'; +import { + emptyTablePersistData, + emptyQueryResults, + clearQueryEditors, +} from '../utils/reduxStateToLocalStorageHelper'; +import { BYTES_PER_CHAR, KB_STORAGE } from '../constants'; + +type SqlLabState = SqlLabRootState['sqlLab']; + +type ClearEntityHelperValue = + | Table[] + | SqlLabState['queries'] + | QueryEditor[] + | UnsavedQueryEditor; + +interface ClearEntityHelpersMap { + tables: (tables: Table[]) => ReturnType; + queries: ( + queries: SqlLabState['queries'], + ) => ReturnType; + queryEditors: ( + queryEditors: QueryEditor[], + ) => ReturnType; + unsavedQueryEditor: ( + qe: UnsavedQueryEditor, + ) => ReturnType[number]; +} + +const CLEAR_ENTITY_HELPERS_MAP: ClearEntityHelpersMap = { + tables: emptyTablePersistData, + queries: emptyQueryResults, + queryEditors: clearQueryEditors, + unsavedQueryEditor: (qe: UnsavedQueryEditor) => + clearQueryEditors([qe as QueryEditor])[0], +}; + +interface PersistedSqlLabState { + sqlLab?: Partial; + localStorageUsageInKilobytes?: number; +} + +const sqlLabPersistStateConfig = { + paths: ['sqlLab'], + config: { + slicer: + (paths: string[]) => + (state: SqlLabRootState): PersistedSqlLabState => { + const subset: PersistedSqlLabState = {}; + paths.forEach(path => { + if (isFeatureEnabled(FeatureFlag.SqllabBackendPersistence)) { + const { + queryEditors, + editorTabLastUpdatedAt, + unsavedQueryEditor, + tables, + queries, + tabHistory, + lastUpdatedActiveTab, + destroyedQueryEditors, + } = state.sqlLab; + const unsavedQueryEditors = filterUnsavedQueryEditorList( + queryEditors, + unsavedQueryEditor, + editorTabLastUpdatedAt, + ); + const hasUnsavedActiveTabState = + tabHistory.slice(-1)[0] !== lastUpdatedActiveTab; + const hasUnsavedDeletedQueryEditors = + Object.keys(destroyedQueryEditors).length > 0; + if ( + unsavedQueryEditors.length > 0 || + hasUnsavedActiveTabState || + hasUnsavedDeletedQueryEditors + ) { + const hasFinishedMigrationFromLocalStorage = + unsavedQueryEditors.every( + ({ inLocalStorage }) => !inLocalStorage, + ); + subset.sqlLab = { + queryEditors: unsavedQueryEditors, + ...(!hasFinishedMigrationFromLocalStorage && { + tabHistory, + tables: tables.filter(table => table.inLocalStorage), + queries: pickBy( + queries, + query => query.inLocalStorage && !query.isDataPreview, + ), + }), + ...(hasUnsavedActiveTabState && { + tabHistory, + }), + destroyedQueryEditors, + }; + } + return; + } + // this line is used to remove old data from browser localStorage. + // we used to persist all redux state into localStorage, but + // it caused configurations passed from server-side got override. + // see PR 6257 for details + const statePath = state[path as keyof SqlLabRootState]; + if ( + statePath && + typeof statePath === 'object' && + 'common' in statePath + ) { + delete (statePath as Record).common; // eslint-disable-line no-param-reassign + } + if (path === 'sqlLab') { + subset[path] = Object.fromEntries( + Object.entries(state[path]).map(([key, value]) => { + const helper = CLEAR_ENTITY_HELPERS_MAP[ + key as keyof ClearEntityHelpersMap + ] as ((val: ClearEntityHelperValue) => unknown) | undefined; + return [ + key, + helper?.(value as ClearEntityHelperValue) ?? value, + ]; + }), + ); + } + }); + + const data = JSON.stringify(subset); + // 2 digit precision + const currentSize = + Math.round(((data.length * BYTES_PER_CHAR) / KB_STORAGE) * 100) / 100; + if (state.localStorageUsageInKilobytes !== currentSize) { + state.localStorageUsageInKilobytes = currentSize; // eslint-disable-line no-param-reassign + } + + return subset; + }, + merge: ( + initialState: SqlLabRootState, + persistedState: PersistedSqlLabState = {}, + ) => ({ + ...initialState, + ...persistedState, + sqlLab: { + ...persistedState?.sqlLab, + // Overwrite initialState over persistedState for sqlLab + // since a logic in getInitialState overrides the value from persistedState + ...initialState.sqlLab, + }, + }), + }, +}; + +// redux-localstorage doesn't have TypeScript definitions +// The library returns a StoreEnhancer that persists specified paths to localStorage +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export const persistSqlLabStateEnhancer = (persistState as any)( + sqlLabPersistStateConfig.paths, + sqlLabPersistStateConfig.config, +) as StoreEnhancer; diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.ts similarity index 67% rename from superset-frontend/src/SqlLab/reducers/sqlLab.js rename to superset-frontend/src/SqlLab/reducers/sqlLab.ts index 3469036da4d..55981f0aa5a 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.ts @@ -20,7 +20,9 @@ import { normalizeTimestamp, QueryState, t } from '@superset-ui/core'; import { isEqual, omit } from 'lodash'; import { shallowEqual } from 'react-redux'; import { now } from '@superset-ui/core/utils/dates'; +import type { SqlLabRootState, QueryEditor, Table } from '../types'; import * as actions from '../actions/sqlLab'; +import type { SqlLabAction } from '../actions/sqlLab'; import { addToObject, alterInObject, @@ -31,7 +33,14 @@ import { extendArr, } from '../../reduxUtils'; -function alterUnsavedQueryEditorState(state, updatedState, id, silent = false) { +type SqlLabState = SqlLabRootState['sqlLab']; + +function alterUnsavedQueryEditorState( + state: SqlLabState, + updatedState: Partial, + id: string, + silent = false, +): Partial { if (state.tabHistory[state.tabHistory.length - 1] !== id) { const { queryEditors } = alterInArr( state, @@ -52,8 +61,12 @@ function alterUnsavedQueryEditorState(state, updatedState, id, silent = false) { }; } -export default function sqlLabReducer(state = {}, action) { - const actionHandlers = { +export default function sqlLabReducer( + state: SqlLabState = {} as SqlLabState, + action: SqlLabAction, +): SqlLabState { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const actionHandlers: Record any> = { [actions.ADD_QUERY_EDITOR]() { const mergeUnsavedState = alterInArr( state, @@ -65,10 +78,10 @@ export default function sqlLabReducer(state = {}, action) { ); const newState = { ...mergeUnsavedState, - tabHistory: [...state.tabHistory, action.queryEditor.id], + tabHistory: [...state.tabHistory, action.queryEditor!.id], }; return addToArr(newState, 'queryEditors', { - ...action.queryEditor, + ...action.queryEditor!, updatedAt: new Date().getTime(), }); }, @@ -78,23 +91,24 @@ export default function sqlLabReducer(state = {}, action) { return alterInArr( state, 'queryEditors', - existing, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + existing as any, { - remoteId: result.remoteId, - name: query.name, + remoteId: result!.remoteId, + name: (query as { name: string }).name, }, 'id', ); }, [actions.UPDATE_QUERY_EDITOR]() { - const id = action.alterations.remoteId; + const id = action.alterations!.remoteId; const existing = state.queryEditors.find(qe => qe.remoteId === id); if (existing == null) return state; return alterInArr( state, 'queryEditors', existing, - action.alterations, + action.alterations!, 'remoteId', ); }, @@ -104,19 +118,20 @@ export default function sqlLabReducer(state = {}, action) { ); const progenitor = { ...queryEditor, - ...(state.unsavedQueryEditor.id === queryEditor.id && + ...(state.unsavedQueryEditor.id === queryEditor?.id && state.unsavedQueryEditor), }; const qe = { remoteId: progenitor.remoteId, name: t('Copy of %s', progenitor.name), - dbId: action.query.dbId ? action.query.dbId : null, - catalog: action.query.catalog ? action.query.catalog : null, - schema: action.query.schema ? action.query.schema : null, + dbId: action.query!.dbId ? action.query!.dbId : null, + catalog: action.query!.catalog ? action.query!.catalog : null, + schema: action.query!.schema ? action.query!.schema : null, autorun: true, - sql: action.query.sql, - queryLimit: action.query.queryLimit, - maxRow: action.query.maxRow, + sql: action.query!.sql, + queryLimit: action.query!.queryLimit, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + maxRow: (action.query as any)?.maxRow, }; const stateWithoutUnsavedState = { ...state, @@ -124,20 +139,24 @@ export default function sqlLabReducer(state = {}, action) { }; return sqlLabReducer( stateWithoutUnsavedState, - actions.addQueryEditor(qe), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + actions.addQueryEditor(qe as any), ); }, [actions.REMOVE_QUERY_EDITOR]() { const queryEditor = { - ...action.queryEditor, - ...(action.queryEditor.id === state.unsavedQueryEditor.id && + ...action.queryEditor!, + ...(action.queryEditor!.id === state.unsavedQueryEditor.id && state.unsavedQueryEditor), }; let newState = removeFromArr(state, 'queryEditors', queryEditor); // List of remaining queryEditor ids - const qeIds = newState.queryEditors.map(qe => qe.tabViewId ?? qe.id); + const qeIds = newState.queryEditors.map( + (qe: QueryEditor) => qe.tabViewId ?? qe.id, + ); - const queries = {}; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const queries: any = {}; Object.keys(state.queries).forEach(k => { const query = state.queries[k]; if (qeIds.indexOf(query.sqlEditorId) > -1) { @@ -158,18 +177,18 @@ export default function sqlLabReducer(state = {}, action) { ...newState, tabHistory: tabHistory.length === 0 && newState.queryEditors.length > 0 - ? newState.queryEditors.slice(-1).map(qe => qe.id) + ? newState.queryEditors.slice(-1).map((qe: QueryEditor) => qe.id) : tabHistory, tables, queries, unsavedQueryEditor: { - ...(action.queryEditor.id !== state.unsavedQueryEditor.id && + ...(action.queryEditor!.id !== state.unsavedQueryEditor.id && state.unsavedQueryEditor), }, destroyedQueryEditors: { ...newState.destroyedQueryEditors, ...(!queryEditor.inLocalStorage && { - [queryEditor.tabViewId ?? queryEditor.id]: Date.now(), + [(queryEditor.tabViewId ?? queryEditor.id)!]: Date.now(), }), }, }; @@ -177,19 +196,19 @@ export default function sqlLabReducer(state = {}, action) { }, [actions.CLEAR_DESTROYED_QUERY_EDITOR]() { const destroyedQueryEditors = { ...state.destroyedQueryEditors }; - delete destroyedQueryEditors[action.queryEditorId]; + delete destroyedQueryEditors[action.queryEditorId!]; return { ...state, destroyedQueryEditors }; }, [actions.REMOVE_QUERY]() { const newQueries = { ...state.queries }; - delete newQueries[action.query.id]; + delete newQueries[action.query!.id!]; return { ...state, queries: newQueries }; }, [actions.RESET_STATE]() { return { ...action.sqlLabInitialState }; }, [actions.MERGE_TABLE]() { - const at = { ...action.table }; + const at = { ...action.table } as Table; const existingTableIndex = state.tables.findIndex( xt => xt.dbId === at.dbId && @@ -200,7 +219,7 @@ export default function sqlLabReducer(state = {}, action) { ); if (existingTableIndex >= 0) { if (action.query) { - at.dataPreviewQueryId = action.query.id; + at.dataPreviewQueryId = action.query!.id; } return { ...state, @@ -228,30 +247,30 @@ export default function sqlLabReducer(state = {}, action) { }; if (action.query) { newState = alterInArr(newState, 'tables', at, { - dataPreviewQueryId: action.query.id, + dataPreviewQueryId: action.query!.id, }); } return newState; }, [actions.EXPAND_TABLE]() { - return alterInArr(state, 'tables', action.table, { expanded: true }); + return alterInArr(state, 'tables', action.table!, { expanded: true }); }, [actions.REMOVE_DATA_PREVIEW]() { const queries = { ...state.queries }; - delete queries[action.table.dataPreviewQueryId]; - const newState = alterInArr(state, 'tables', action.table, { + delete queries[action.table!.dataPreviewQueryId!]; + const newState = alterInArr(state, 'tables', action.table!, { dataPreviewQueryId: null, }); return { ...newState, queries }; }, [actions.CHANGE_DATA_PREVIEW_ID]() { const queries = { ...state.queries }; - delete queries[action.oldQueryId]; + delete queries[action.oldQueryId!]; - const newTables = []; + const newTables: Table[] = []; state.tables.forEach(xt => { if (xt.dataPreviewQueryId === action.oldQueryId) { - newTables.push({ ...xt, dataPreviewQueryId: action.newQuery.id }); + newTables.push({ ...xt, dataPreviewQueryId: action.newQuery!.id }); } else { newTables.push(xt); } @@ -263,20 +282,20 @@ export default function sqlLabReducer(state = {}, action) { }; }, [actions.COLLAPSE_TABLE]() { - return alterInArr(state, 'tables', action.table, { expanded: false }); + return alterInArr(state, 'tables', action.table!, { expanded: false }); }, [actions.REMOVE_TABLES]() { - const tableIds = action.tables.map(table => table.id); + const tableIds = action.tables!.map((table: Table) => table.id); const tables = state.tables.filter(table => !tableIds.includes(table.id)); return { ...state, tables, - ...(tableIds.includes(state.activeSouthPaneTab) && { + ...(tableIds.includes(state.activeSouthPaneTab as string) && { activeSouthPaneTab: tables.find( ({ queryEditorId }) => - queryEditorId === action.tables[0].queryEditorId, + queryEditorId === action.tables![0].queryEditorId, )?.id ?? 'Results', }), }; @@ -286,7 +305,7 @@ export default function sqlLabReducer(state = {}, action) { ...state, queryCostEstimates: { ...state.queryCostEstimates, - [action.query.id]: { + [action.query!.id!]: { completed: false, cost: null, error: null, @@ -299,9 +318,9 @@ export default function sqlLabReducer(state = {}, action) { ...state, queryCostEstimates: { ...state.queryCostEstimates, - [action.query.id]: { + [action.query!.id!]: { completed: true, - cost: action.json.result, + cost: action.json!.result, error: null, }, }, @@ -312,7 +331,7 @@ export default function sqlLabReducer(state = {}, action) { ...state, queryCostEstimates: { ...state.queryCostEstimates, - [action.query.id]: { + [action.query!.id!]: { completed: false, cost: null, error: action.error, @@ -323,15 +342,19 @@ export default function sqlLabReducer(state = {}, action) { [actions.START_QUERY]() { let newState = { ...state }; let sqlEditorId; - if (action.query.sqlEditorId) { + if (action.query!.sqlEditorId) { const queryEditorByTabId = getFromArr( state.queryEditors, - action.query.sqlEditorId, + action.query!.sqlEditorId, 'tabViewId', ); - sqlEditorId = queryEditorByTabId?.id ?? action.query.sqlEditorId; + sqlEditorId = + (queryEditorByTabId as QueryEditor | undefined)?.id ?? + action.query!.sqlEditorId; + const foundQueryEditor = getFromArr(state.queryEditors, sqlEditorId); + const baseQe = foundQueryEditor || {}; const qe = { - ...getFromArr(state.queryEditors, sqlEditorId), + ...baseQe, ...(sqlEditorId === state.unsavedQueryEditor.id && state.unsavedQueryEditor), }; @@ -342,40 +365,44 @@ export default function sqlLabReducer(state = {}, action) { query: null, }; const q = { ...state.queries[qe.latestQueryId], results: newResults }; - const queries = { ...state.queries, [q.id]: q }; + const queries = { + ...state.queries, + [q.id]: q, + } as SqlLabState['queries']; newState = { ...state, queries }; } } - newState = addToObject(newState, 'queries', action.query); + newState = addToObject(newState, 'queries', action.query!) as SqlLabState; return { ...newState, ...alterUnsavedQueryEditorState( state, { - latestQueryId: action.query.id, + latestQueryId: action.query!.id, }, - sqlEditorId, - action.query.isDataPreview, + sqlEditorId!, + action.query!.isDataPreview, ), }; }, [actions.STOP_QUERY]() { - return alterInObject(state, 'queries', action.query, { + return alterInObject(state, 'queries', action.query!, { state: QueryState.Stopped, results: [], }); }, [actions.CLEAR_QUERY_RESULTS]() { - const newResults = { ...action.query.results }; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const newResults = { ...(action.query as any).results }; newResults.data = []; - return alterInObject(state, 'queries', action.query, { + return alterInObject(state, 'queries', action.query!, { results: newResults, cached: true, }); }, [actions.REQUEST_QUERY_RESULTS]() { - return alterInObject(state, 'queries', action.query, { + return alterInObject(state, 'queries', action.query!, { state: QueryState.Fetching, }); }, @@ -383,12 +410,13 @@ export default function sqlLabReducer(state = {}, action) { // prevent race condition where query succeeds shortly after being canceled // or the final result was unsuccessful if ( - action.query.state === QueryState.STOPPED || - action.results.status !== QueryState.Success + action.query!.state === QueryState.Stopped || + (action.results as { status?: string })?.status !== QueryState.Success ) { return state; } - const alts = { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const alts: any = { endDttm: now(), progress: 100, results: action.results, @@ -407,10 +435,10 @@ export default function sqlLabReducer(state = {}, action) { alts.resultsKey = resultsKey; } - return alterInObject(state, 'queries', action.query, alts); + return alterInObject(state, 'queries', action.query!, alts); }, [actions.QUERY_FAILED]() { - if (action.query.state === QueryState.Stopped) { + if (action.query!.state === QueryState.Stopped) { return state; } const alts = { @@ -420,13 +448,13 @@ export default function sqlLabReducer(state = {}, action) { endDttm: now(), link: action.link, }; - return alterInObject(state, 'queries', action.query, alts); + return alterInObject(state, 'queries', action.query!, alts); }, [actions.SET_ACTIVE_QUERY_EDITOR]() { const qeIds = state.queryEditors.map(qe => qe.id); if ( - qeIds.indexOf(action.queryEditor?.id) > -1 && - state.tabHistory[state.tabHistory.length - 1] !== action.queryEditor.id + qeIds.indexOf(action.queryEditor!.id!) > -1 && + state.tabHistory[state.tabHistory.length - 1] !== action.queryEditor!.id ) { const mergeUnsavedState = { ...alterInArr(state, 'queryEditors', state.unsavedQueryEditor, { @@ -435,18 +463,18 @@ export default function sqlLabReducer(state = {}, action) { unsavedQueryEditor: {}, }; return { - ...(action.queryEditor.id === state.unsavedQueryEditor.id + ...(action.queryEditor!.id === state.unsavedQueryEditor.id ? alterInArr( mergeUnsavedState, 'queryEditors', - action.queryEditor, + action.queryEditor!, { - ...action.queryEditor, + ...action.queryEditor!, ...state.unsavedQueryEditor, }, ) : mergeUnsavedState), - tabHistory: [...state.tabHistory, action.queryEditor.id], + tabHistory: [...state.tabHistory, action.queryEditor!.id], }; } return state; @@ -460,12 +488,17 @@ export default function sqlLabReducer(state = {}, action) { ...state.unsavedQueryEditor, }, ); - return alterInArr(mergeUnsavedState, 'queryEditors', action.queryEditor, { - ...action.queryEditor, - }); + return alterInArr( + mergeUnsavedState, + 'queryEditors', + action.queryEditor!, + { + ...action.queryEditor!, + }, + ); }, [actions.SET_TABLES]() { - return extendArr(state, 'tables', action.tables); + return extendArr(state, 'tables', action.tables!); }, [actions.SET_ACTIVE_SOUTHPANE_TAB]() { return { ...state, activeSouthPaneTab: action.tabId }; @@ -473,9 +506,9 @@ export default function sqlLabReducer(state = {}, action) { [actions.MIGRATE_QUERY_EDITOR]() { try { // remove migrated query editor from localStorage - const { sqlLab } = JSON.parse(localStorage.getItem('redux')); + const { sqlLab } = JSON.parse(localStorage.getItem('redux') || '{}'); sqlLab.queryEditors = sqlLab.queryEditors.filter( - qe => qe.id !== action.oldQueryEditor.id, + (qe: QueryEditor) => qe.id !== action.oldQueryEditor!.id, ); localStorage.setItem('redux', JSON.stringify({ sqlLab })); } catch (error) { @@ -485,16 +518,16 @@ export default function sqlLabReducer(state = {}, action) { return alterInArr( state, 'queryEditors', - action.oldQueryEditor, - action.newQueryEditor, + action.oldQueryEditor!, + action.newQueryEditor!, ); }, [actions.MIGRATE_TABLE]() { try { // remove migrated table from localStorage - const { sqlLab } = JSON.parse(localStorage.getItem('redux')); + const { sqlLab } = JSON.parse(localStorage.getItem('redux') || '{}'); sqlLab.tables = sqlLab.tables.filter( - table => table.id !== action.oldTable.id, + (table: Table) => table.id !== action.oldTable!.id, ); localStorage.setItem('redux', JSON.stringify({ sqlLab })); } catch (error) { @@ -503,14 +536,14 @@ export default function sqlLabReducer(state = {}, action) { // replace localStorage table with the server backed one return addToArr( - removeFromArr(state, 'tables', action.oldTable), + removeFromArr(state, 'tables', action.oldTable!), 'tables', - action.newTable, + action.newTable!, ); }, [actions.MIGRATE_QUERY]() { const query = { - ...state.queries[action.queryId], + ...state.queries[action.queryId!], // point query to migrated query editor sqlEditorId: action.queryEditorId, }; @@ -525,7 +558,7 @@ export default function sqlLabReducer(state = {}, action) { { dbId: action.dbId, }, - action.queryEditor.id, + action.queryEditor!.id!, ), }; }, @@ -537,7 +570,7 @@ export default function sqlLabReducer(state = {}, action) { { catalog: action.catalog, }, - action.queryEditor.id, + action.queryEditor!.id!, ), }; }, @@ -547,9 +580,9 @@ export default function sqlLabReducer(state = {}, action) { ...alterUnsavedQueryEditorState( state, { - schema: action.schema, + schema: action.schema ?? undefined, }, - action.queryEditor.id, + action.queryEditor!.id!, ), }; }, @@ -561,14 +594,14 @@ export default function sqlLabReducer(state = {}, action) { { name: action.name, }, - action.queryEditor.id, + action.queryEditor!.id!, ), }; }, [actions.QUERY_EDITOR_SET_SQL]() { const { unsavedQueryEditor } = state; if ( - unsavedQueryEditor?.id === action.queryEditor.id && + unsavedQueryEditor?.id === action.queryEditor!.id && unsavedQueryEditor.sql === action.sql ) { return state; @@ -578,10 +611,10 @@ export default function sqlLabReducer(state = {}, action) { ...alterUnsavedQueryEditorState( state, { - sql: action.sql, + sql: action.sql ?? undefined, ...(action.queryId && { latestQueryId: action.queryId }), }, - action.queryEditor.id, + action.queryEditor!.id!, ), }; }, @@ -593,7 +626,7 @@ export default function sqlLabReducer(state = {}, action) { { cursorPosition: action.position, }, - action.queryEditor.id, + action.queryEditor!.id!, ), }; }, @@ -605,7 +638,7 @@ export default function sqlLabReducer(state = {}, action) { { queryLimit: action.queryLimit, }, - action.queryEditor.id, + action.queryEditor!.id!, ), }; }, @@ -617,7 +650,7 @@ export default function sqlLabReducer(state = {}, action) { { templateParams: action.templateParams, }, - action.queryEditor.id, + action.queryEditor!.id!, ), }; }, @@ -627,9 +660,9 @@ export default function sqlLabReducer(state = {}, action) { ...alterUnsavedQueryEditorState( state, { - selectedText: action.sql, + selectedText: action.sql ?? undefined, }, - action.queryEditor.id, + action.queryEditor!.id!, true, ), }; @@ -642,7 +675,7 @@ export default function sqlLabReducer(state = {}, action) { { autorun: action.autorun, }, - action.queryEditor.id, + action.queryEditor!.id!, ), }; }, @@ -655,7 +688,7 @@ export default function sqlLabReducer(state = {}, action) { northPercent: action.northPercent, southPercent: action.southPercent, }, - action.queryEditor.id, + action.queryEditor!.id!, ), }; }, @@ -667,13 +700,15 @@ export default function sqlLabReducer(state = {}, action) { { hideLeftBar: action.hideLeftBar, }, - action.queryEditor.id, + action.queryEditor!.id!, ), }; }, [actions.SET_DATABASES]() { - const databases = {}; - action.databases.forEach(db => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const databases: any = {}; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (action.databases as any[])!.forEach((db: any) => { databases[db.id] = { ...db, extra_json: JSON.parse(db.extra || ''), @@ -686,54 +721,57 @@ export default function sqlLabReducer(state = {}, action) { // Fetch the updates to the queries present in the store. let change = false; let { queriesLastUpdate } = state; - Object.entries(action.alteredQueries).forEach(([id, changedQuery]) => { - if ( - !state.queries.hasOwnProperty(id) || - (state.queries[id].state !== QueryState.Stopped && - state.queries[id].state !== QueryState.Failed) - ) { - const changedOn = normalizeTimestamp(changedQuery.changed_on); - const timestamp = Date.parse(changedOn); - if (timestamp > queriesLastUpdate) { - queriesLastUpdate = timestamp; - } - const prevState = state.queries[id]?.state; - const currentState = changedQuery.state; - newQueries[id] = { - ...state.queries[id], - ...changedQuery, - ...(changedQuery.startDttm && { - startDttm: Number(changedQuery.startDttm), - }), - ...(changedQuery.endDttm && { - endDttm: Number(changedQuery.endDttm), - }), - // race condition: - // because of async behavior, sql lab may still poll a couple of seconds - // when it started fetching or finished rendering results - state: - currentState === QueryState.Success && - [ - QueryState.Fetching, - QueryState.Success, - QueryState.Running, - ].includes(prevState) - ? prevState - : currentState, - }; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Object.entries(action.alteredQueries!).forEach( + ([id, changedQuery]: [string, any]) => { if ( - shallowEqual( - omit(newQueries[id], ['extra']), - omit(state.queries[id], ['extra']), - ) && - isEqual(newQueries[id].extra, state.queries[id].extra) + !state.queries.hasOwnProperty(id) || + (state.queries[id].state !== QueryState.Stopped && + state.queries[id].state !== QueryState.Failed) ) { - newQueries[id] = state.queries[id]; - } else { - change = true; + const changedOn = normalizeTimestamp(changedQuery.changed_on); + const timestamp = Date.parse(changedOn); + if (timestamp > queriesLastUpdate) { + queriesLastUpdate = timestamp; + } + const prevState = state.queries[id]?.state; + const currentState = changedQuery.state; + newQueries[id] = { + ...state.queries[id], + ...changedQuery, + ...(changedQuery.startDttm && { + startDttm: Number(changedQuery.startDttm), + }), + ...(changedQuery.endDttm && { + endDttm: Number(changedQuery.endDttm), + }), + // race condition: + // because of async behavior, sql lab may still poll a couple of seconds + // when it started fetching or finished rendering results + state: + currentState === QueryState.Success && + [ + QueryState.Fetching, + QueryState.Success, + QueryState.Running, + ].includes(prevState) + ? prevState + : currentState, + }; + if ( + shallowEqual( + omit(newQueries[id], ['extra']), + omit(state.queries[id], ['extra']), + ) && + isEqual(newQueries[id].extra, state.queries[id].extra) + ) { + newQueries[id] = state.queries[id]; + } else { + change = true; + } } - } - }); + }, + ); if (!change) { newQueries = state.queries; } @@ -746,14 +784,15 @@ export default function sqlLabReducer(state = {}, action) { .filter(([, query]) => { if ( ['running', 'pending'].includes(query.state) && - Date.now() - query.startDttm > action.interval && + Date.now() - query.startDttm > action.interval! && query.progress === 0 ) { return false; } return true; }) - .map(([id, query]) => [ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .map(([id, query]: [string, any]) => [ id, { ...query, diff --git a/superset-frontend/src/core/sqlLab/index.ts b/superset-frontend/src/core/sqlLab/index.ts index d218fab2c1c..dbadd95e454 100644 --- a/superset-frontend/src/core/sqlLab/index.ts +++ b/superset-frontend/src/core/sqlLab/index.ts @@ -155,28 +155,32 @@ function createQueryResultContext( action: ReturnType, ): QueryResultContext { const { baseParams, options } = extractBaseData(action); + // eslint-disable-next-line @typescript-eslint/no-unused-vars const [_, tab] = baseParams; const { results } = action; + const { query_id: queryId, columns, data, query } = results; const { - query_id: queryId, - columns, - data, - query: { - endDttm, - executedSql, - tempTable: resultTempTable, - limit, - limitingFactor, - }, - } = results; + endDttm, + executedSql, + tempTable: resultTempTable, + limit, + limitingFactor, + } = query; + + // Map columns to ensure required fields are present + const mappedColumns = columns.map(col => ({ + ...col, + name: col.name || col.column_name, + type: col.type ?? 'STRING', // Ensure type is not null + })); return new QueryResultContext( ...baseParams, - queryId, + queryId ?? 0, executedSql ?? tab.editor.content, - columns, + mappedColumns, data, - endDttm, + endDttm ?? 0, { ...options, tempTable: resultTempTable || options.tempTable, @@ -193,12 +197,23 @@ function createQueryErrorContext( const { msg: errorMessage, errors, query } = action; const { endDttm, executedSql, query_id: queryId } = query; - return new QueryErrorResultContext(...baseParams, errorMessage, errors, { - ...options, - queryId, - executedSql: executedSql ?? null, - endDttm: endDttm ?? Date.now(), - }); + // Map errors to ensure 'extra' is not null (required by QueryErrorResultContext) + const mappedErrors = (errors ?? []).map(err => ({ + ...err, + extra: err.extra ?? {}, + })); + + return new QueryErrorResultContext( + ...baseParams, + errorMessage, + mappedErrors, + { + ...options, + queryId, + executedSql: executedSql ?? undefined, + endDttm: endDttm ?? Date.now(), + }, + ); } const getCurrentTab: typeof sqlLabType.getCurrentTab = () => diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx index 2c740d771ea..26162db2d15 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx @@ -19,7 +19,6 @@ /* eslint-disable react-hooks/rules-of-hooks */ import { ColumnMeta, Metric } from '@superset-ui/chart-controls'; import { - AdhocFilter, Behavior, ChartDataResponseResult, Column, @@ -78,6 +77,7 @@ import { } from 'src/dashboard/types'; import DateFilterControl from 'src/explore/components/controls/DateFilterControl'; import AdhocFilterControl from 'src/explore/components/controls/FilterControl/AdhocFilterControl'; +import type AdhocFilterClass from 'src/explore/components/controls/FilterControl/AdhocFilter'; import { waitForAsyncData } from 'src/middleware/asyncEvent'; import { SingleValueType } from 'src/filters/components/Range/SingleValueType'; import { RangeDisplayMode } from 'src/filters/components/Range/types'; @@ -1003,7 +1003,7 @@ const FiltersConfigForm = ( } datasource={datasetDetails} onChange={( - filters: AdhocFilter[], + filters: AdhocFilterClass[], ) => { setNativeFilterFieldValues( form, diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx index bec46b67844..0c6780f57df 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx @@ -19,6 +19,7 @@ import { render, screen, + selectOption, userEvent, waitFor, } from 'spec/helpers/testing-library'; @@ -210,39 +211,23 @@ test('fetches chart on mount if value present', async () => { }); test('keeps apply disabled when missing required fields', async () => { + // With EVENT type and Table source, the component requires selecting a chart + // and filling in required fields. Without completing these, Apply should be disabled. await waitForRender({ annotationType: ANNOTATION_TYPES_METADATA.EVENT.value, sourceType: 'Table', }); - userEvent.click( - screen.getByRole('combobox', { name: 'Annotation layer value' }), - ); - expect(await screen.findByText('Chart A')).toBeInTheDocument(); - userEvent.click(screen.getByText('Chart A')); + + // Apply button should be disabled initially since required fields are not filled + expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled(); + + // Select Chart A from the annotation layer value dropdown + await selectOption('Chart A', 'Annotation layer value'); + + // Wait for the chart data to load await screen.findByText(/title column/i); - userEvent.click( - screen.getByRole('combobox', { name: 'Annotation layer title column' }), - ); - expect(await screen.findByText(/none/i)).toBeInTheDocument(); - userEvent.click(screen.getByText('None')); - userEvent.click(screen.getByText('Style')); - // The checkbox for automatic color is in the Style tab - userEvent.click(screen.getByText('Use automatic color')); - userEvent.click( - screen.getByRole('combobox', { name: 'Annotation layer stroke' }), - ); - expect(await screen.findByText('Dashed')).toBeInTheDocument(); - userEvent.click(screen.getByText('Dashed')); - userEvent.click(screen.getByText('Opacity')); - userEvent.click( - screen.getByRole('combobox', { name: 'Annotation layer opacity' }), - ); - expect(await screen.findByText(/0.5/i)).toBeInTheDocument(); - userEvent.click(screen.getByText('0.5')); - - const checkboxes = screen.getAllByRole('checkbox'); - checkboxes.forEach(checkbox => userEvent.click(checkbox)); + // Apply should still be disabled because name is not filled expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled(); }); diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.tsx similarity index 74% rename from superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx rename to superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.tsx index f520267d130..bdffa8c14e6 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.tsx @@ -16,9 +16,8 @@ * specific language governing permissions and limitations * under the License. */ -import { PureComponent } from 'react'; +import React, { PureComponent } from 'react'; import rison from 'rison'; -import PropTypes from 'prop-types'; import { Button, AsyncSelect, @@ -34,8 +33,13 @@ import { isValidExpression, getColumnLabel, VizType, + type QueryFormColumn, } from '@superset-ui/core'; -import { styled, withTheme } from '@apache-superset/core/ui'; +import { + styled, + withTheme, + type SupersetTheme, +} from '@apache-superset/core/ui'; import SelectControl from 'src/explore/components/controls/SelectControl'; import TextControl from 'src/explore/components/controls/TextControl'; import CheckboxControl from 'src/explore/components/controls/CheckboxControl'; @@ -50,60 +54,81 @@ import { ANNOTATION_SOURCE_TYPES_METADATA, } from './AnnotationTypes'; +interface SelectOption { + value: string | number; + label: string; + viz_type?: string; + [key: string]: unknown; +} + +interface SliceData { + data: { + groupby?: string[]; + all_columns?: string[]; + include_time?: boolean; + [key: string]: unknown; + }; +} + +interface AnnotationOverrides { + time_range?: string | null; + time_grain_sqla?: string | null; + granularity?: string | null; + time_shift?: string; + [key: string]: unknown; +} + +interface AnnotationLayerProps { + name?: string; + annotationType?: string; + sourceType?: string; + color?: string; + opacity?: string; + style?: string; + width?: number; + showMarkers?: boolean; + hideLine?: boolean; + value?: string | number | SelectOption; + overrides?: AnnotationOverrides; + show?: boolean; + showLabel?: boolean; + titleColumn?: string; + descriptionColumns?: string[]; + timeColumn?: string; + intervalEndColumn?: string; + vizType?: string; + error?: string; + colorScheme?: string; + theme: SupersetTheme; + addAnnotationLayer?: (annotation: Record) => void; + removeAnnotationLayer?: () => void; + close?: () => void; +} + +interface AnnotationLayerState { + name: string; + annotationType: string; + sourceType: string | null; + value: string | number | SelectOption | null; + overrides: AnnotationOverrides; + show: boolean; + showLabel: boolean; + titleColumn: string; + descriptionColumns: string[]; + timeColumn: string; + intervalEndColumn: string; + color: string; + opacity: string; + style: string; + width: number; + showMarkers: boolean; + hideLine: boolean; + isNew: boolean; + slice: SliceData | null; +} + const AUTOMATIC_COLOR = ''; -const propTypes = { - name: PropTypes.string, - annotationType: PropTypes.string, - sourceType: PropTypes.string, - color: PropTypes.string, - opacity: PropTypes.string, - style: PropTypes.string, - width: PropTypes.number, - showMarkers: PropTypes.bool, - hideLine: PropTypes.bool, - value: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), - overrides: PropTypes.object, - show: PropTypes.bool, - showLabel: PropTypes.bool, - titleColumn: PropTypes.string, - descriptionColumns: PropTypes.arrayOf(PropTypes.string), - timeColumn: PropTypes.string, - intervalEndColumn: PropTypes.string, - vizType: PropTypes.string, - - error: PropTypes.string, - colorScheme: PropTypes.string, - - addAnnotationLayer: PropTypes.func, - removeAnnotationLayer: PropTypes.func, - close: PropTypes.func, -}; - -const defaultProps = { - name: '', - annotationType: DEFAULT_ANNOTATION_TYPE, - sourceType: '', - color: AUTOMATIC_COLOR, - opacity: '', - style: 'solid', - width: 1, - showMarkers: false, - hideLine: false, - overrides: {}, - colorScheme: 'd3Category10', - show: true, - showLabel: false, - titleColumn: '', - descriptionColumns: [], - timeColumn: '', - intervalEndColumn: '', - - addAnnotationLayer: () => {}, - removeAnnotationLayer: () => {}, - close: () => {}, -}; - const NotFoundContentWrapper = styled.div` && > div:first-child { padding-left: 0; @@ -134,8 +159,34 @@ const NotFoundContent = () => ( ); -class AnnotationLayer extends PureComponent { - constructor(props) { +class AnnotationLayer extends PureComponent< + AnnotationLayerProps, + AnnotationLayerState +> { + static defaultProps = { + name: '', + annotationType: DEFAULT_ANNOTATION_TYPE, + sourceType: '', + color: AUTOMATIC_COLOR, + opacity: '', + style: 'solid', + width: 1, + showMarkers: false, + hideLine: false, + overrides: {}, + colorScheme: 'd3Category10', + show: true, + showLabel: false, + titleColumn: '', + descriptionColumns: [], + timeColumn: '', + intervalEndColumn: '', + addAnnotationLayer: () => {}, + removeAnnotationLayer: () => {}, + close: () => {}, + }; + + constructor(props: AnnotationLayerProps) { super(props); const { name, @@ -159,42 +210,46 @@ class AnnotationLayer extends PureComponent { } = props; // Only allow override whole time_range - if ('since' in overrides || 'until' in overrides) { - overrides.time_range = null; - delete overrides.since; - delete overrides.until; + const processedOverrides: AnnotationOverrides = overrides + ? { ...overrides } + : {}; + if ('since' in processedOverrides || 'until' in processedOverrides) { + processedOverrides.time_range = null; + delete processedOverrides.since; + delete processedOverrides.until; } // Check if annotationType is supported by this chart - const metadata = getChartMetadataRegistry().get(vizType); + const metadata = vizType ? getChartMetadataRegistry().get(vizType) : null; const supportedAnnotationTypes = metadata?.supportedAnnotationTypes || []; + const resolvedAnnotationType = annotationType || DEFAULT_ANNOTATION_TYPE; const validAnnotationType = supportedAnnotationTypes.includes( - annotationType, + resolvedAnnotationType, ) - ? annotationType + ? resolvedAnnotationType : supportedAnnotationTypes[0]; this.state = { // base - name, - annotationType: validAnnotationType, - sourceType, - value, - overrides, - show, - showLabel, + name: name || '', + annotationType: validAnnotationType || DEFAULT_ANNOTATION_TYPE, + sourceType: sourceType || null, + value: value || null, + overrides: processedOverrides, + show: show ?? true, + showLabel: showLabel ?? false, // slice - titleColumn, - descriptionColumns, - timeColumn, - intervalEndColumn, + titleColumn: titleColumn || '', + descriptionColumns: descriptionColumns || [], + timeColumn: timeColumn || '', + intervalEndColumn: intervalEndColumn || '', // display color: color || AUTOMATIC_COLOR, - opacity, - style, - width, - showMarkers, - hideLine, + opacity: opacity || '', + style: style || 'solid', + width: width ?? 1, + showMarkers: showMarkers ?? false, + hideLine: hideLine ?? false, // refData isNew: !name, slice: null, @@ -229,57 +284,71 @@ class AnnotationLayer extends PureComponent { /* The value prop is the id of the chart/native. This function will set value in state to an object with the id as value.value to be used by AsyncSelect */ - this.fetchAppliedAnnotation(value); + if (value !== null && typeof value !== 'object') { + this.fetchAppliedAnnotation(value); + } } } - componentDidUpdate(prevProps, prevState) { + componentDidUpdate( + _prevProps: AnnotationLayerProps, + prevState: AnnotationLayerState, + ): void { if (this.shouldFetchSliceData(prevState)) { const { value } = this.state; - this.fetchSliceData(value.value); + if (value && typeof value === 'object' && 'value' in value) { + this.fetchSliceData(value.value); + } } } - getSupportedSourceTypes(annotationType) { + getSupportedSourceTypes(annotationType: string): SelectOption[] { // Get vis types that can be source. const sources = getChartMetadataRegistry() .entries() .filter(({ value: chartMetadata }) => - chartMetadata.canBeAnnotationType(annotationType), + chartMetadata?.canBeAnnotationType(annotationType), ) .map(({ key, value: chartMetadata }) => ({ value: key === VizType.Line ? 'line' : key, - label: chartMetadata.name, + label: chartMetadata?.name || key, })); // Prepend native source if applicable - if (ANNOTATION_TYPES_METADATA[annotationType]?.supportNativeSource) { + const annotationMeta = + ANNOTATION_TYPES_METADATA[ + annotationType as keyof typeof ANNOTATION_TYPES_METADATA + ]; + if (annotationMeta && 'supportNativeSource' in annotationMeta) { sources.unshift(ANNOTATION_SOURCE_TYPES_METADATA.NATIVE); } return sources; } - shouldFetchAppliedAnnotation() { + shouldFetchAppliedAnnotation(): boolean { const { value, sourceType } = this.state; - return value && requiresQuery(sourceType); + return !!value && requiresQuery(sourceType ?? undefined); } - shouldFetchSliceData(prevState) { + shouldFetchSliceData(prevState: AnnotationLayerState): boolean { const { value, sourceType } = this.state; const isChart = sourceType !== ANNOTATION_SOURCE_TYPES.NATIVE && - requiresQuery(sourceType); + requiresQuery(sourceType ?? undefined); const valueIsNew = value && prevState.value !== value; - return valueIsNew && isChart; + return !!valueIsNew && isChart; } - isValidFormulaAnnotation(expression, annotationType) { + isValidFormulaAnnotation( + expression: string | number | SelectOption | null, + annotationType: string, + ): boolean { if (annotationType === ANNOTATION_TYPES.FORMULA) { - return isValidExpression(expression); + return isValidExpression(expression as string); } return true; } - isValidForm() { + isValidForm(): boolean { const { name, annotationType, @@ -302,11 +371,13 @@ class AnnotationLayer extends PureComponent { errors.push(validateNonEmpty(intervalEndColumn)); } } - errors.push(!this.isValidFormulaAnnotation(value, annotationType)); + if (!this.isValidFormulaAnnotation(value, annotationType)) { + errors.push(t('Invalid formula expression')); + } return !errors.filter(x => x).length; } - handleAnnotationType(annotationType) { + handleAnnotationType(annotationType: string): void { this.setState({ annotationType, sourceType: null, @@ -315,7 +386,7 @@ class AnnotationLayer extends PureComponent { }); } - handleAnnotationSourceType(sourceType) { + handleAnnotationSourceType(sourceType: string): void { const { sourceType: prevSourceType } = this.state; if (prevSourceType !== sourceType) { @@ -327,24 +398,28 @@ class AnnotationLayer extends PureComponent { } } - handleSelectValue(selectedValueObject) { + handleSelectValue(selectedValueObject: SelectOption): void { this.setState({ value: selectedValueObject, descriptionColumns: [], - intervalEndColumn: null, - timeColumn: null, - titleColumn: null, + intervalEndColumn: '', + timeColumn: '', + titleColumn: '', overrides: { time_range: null }, }); } - handleTextValue(inputValue) { + handleTextValue(inputValue: string): void { this.setState({ value: inputValue, }); } - fetchNativeAnnotations = async (search, page, pageSize) => { + fetchNativeAnnotations = async ( + search: string, + page: number, + pageSize: number, + ): Promise<{ data: SelectOption[]; totalCount: number }> => { const queryParams = rison.encode({ filters: [ { @@ -364,7 +439,7 @@ class AnnotationLayer extends PureComponent { const { result, count } = json; - const layersArray = result.map(layer => ({ + const layersArray = result.map((layer: { id: number; name: string }) => ({ value: layer.id, label: layer.name, })); @@ -375,7 +450,11 @@ class AnnotationLayer extends PureComponent { }; }; - fetchCharts = async (search, page, pageSize) => { + fetchCharts = async ( + search: string, + page: number, + pageSize: number, + ): Promise<{ data: SelectOption[]; totalCount: number }> => { const { annotationType } = this.state; const queryParams = rison.encode({ @@ -401,11 +480,11 @@ class AnnotationLayer extends PureComponent { const registry = getChartMetadataRegistry(); const chartsArray = result - .filter(chart => { + .filter((chart: { id: number; slice_name: string; viz_type: string }) => { const metadata = registry.get(chart.viz_type); return metadata && metadata.canBeAnnotationType(annotationType); }) - .map(chart => ({ + .map((chart: { id: number; slice_name: string; viz_type: string }) => ({ value: chart.id, label: chart.slice_name, viz_type: chart.viz_type, @@ -417,7 +496,11 @@ class AnnotationLayer extends PureComponent { }; }; - fetchOptions = (search, page, pageSize) => { + fetchOptions = ( + search: string, + page: number, + pageSize: number, + ): Promise<{ data: SelectOption[]; totalCount: number }> => { const { sourceType } = this.state; if (sourceType === ANNOTATION_SOURCE_TYPES.NATIVE) { @@ -426,7 +509,7 @@ class AnnotationLayer extends PureComponent { return this.fetchCharts(search, page, pageSize); }; - fetchSliceData = id => { + fetchSliceData = (id: string | number): void => { const queryParams = rison.encode({ columns: ['query_context'], }); @@ -439,7 +522,9 @@ class AnnotationLayer extends PureComponent { const dataObject = { data: { ...formData, - groupby: formData.groupby?.map(column => getColumnLabel(column)), + groupby: formData.groupby?.map((column: QueryFormColumn) => + getColumnLabel(column), + ), }, }; this.setState({ @@ -448,7 +533,7 @@ class AnnotationLayer extends PureComponent { }); }; - fetchAppliedChart(id) { + fetchAppliedChart(id: string | number): void { const { annotationType } = this.state; const registry = getChartMetadataRegistry(); const queryParams = rison.encode({ @@ -474,7 +559,9 @@ class AnnotationLayer extends PureComponent { slice: { data: { ...formData, - groupby: formData.groupby?.map(column => getColumnLabel(column)), + groupby: formData.groupby?.map((column: QueryFormColumn) => + getColumnLabel(column), + ), }, }, }); @@ -482,7 +569,7 @@ class AnnotationLayer extends PureComponent { }); } - fetchAppliedNativeAnnotation(id) { + fetchAppliedNativeAnnotation(id: string | number): void { SupersetClient.get({ endpoint: `/api/v1/annotation_layer/${id}`, }).then(({ json }) => { @@ -497,7 +584,7 @@ class AnnotationLayer extends PureComponent { }); } - fetchAppliedAnnotation(id) { + fetchAppliedAnnotation(id: string | number): void { const { sourceType } = this.state; if (sourceType === ANNOTATION_SOURCE_TYPES.NATIVE) { @@ -506,12 +593,12 @@ class AnnotationLayer extends PureComponent { return this.fetchAppliedChart(id); } - deleteAnnotation() { - this.props.removeAnnotationLayer(); - this.props.close(); + deleteAnnotation(): void { + this.props.removeAnnotationLayer?.(); + this.props.close?.(); } - applyAnnotation() { + applyAnnotation(): void { const { value, sourceType } = this.state; if (this.isValidForm()) { const annotationFields = [ @@ -532,32 +619,42 @@ class AnnotationLayer extends PureComponent { 'timeColumn', 'intervalEndColumn', ]; - const newAnnotation = {}; + const newAnnotation: Record = {}; annotationFields.forEach(field => { - if (this.state[field] !== null) { - newAnnotation[field] = this.state[field]; + const stateValue = this.state[field as keyof AnnotationLayerState]; + if (stateValue !== null) { + newAnnotation[field] = stateValue; } }); // Prepare newAnnotation.value for use in runAnnotationQuery() - const applicableValue = requiresQuery(sourceType) ? value.value : value; + const applicableValue = + requiresQuery(sourceType ?? undefined) && + value && + typeof value === 'object' + ? (value as SelectOption).value + : value; newAnnotation.value = applicableValue; if (newAnnotation.color === AUTOMATIC_COLOR) { newAnnotation.color = null; } - this.props.addAnnotationLayer(newAnnotation); + this.props.addAnnotationLayer?.(newAnnotation); this.setState({ isNew: false }); } } - submitAnnotation() { + submitAnnotation(): void { this.applyAnnotation(); - this.props.close(); + this.props.close?.(); } - renderChartHeader(label, description, value) { + renderChartHeader( + label: string, + description: string, + value: string | number | SelectOption | null, + ): React.ReactNode { return ( this.setState({ timeColumn: v })} + onChange={( + v: string | number | (string | number)[] | null | undefined, + ) => this.setState({ timeColumn: String(v ?? '') })} /> )} {annotationType === ANNOTATION_TYPES.INTERVAL && ( @@ -694,7 +795,14 @@ class AnnotationLayer extends PureComponent { validationErrors={!intervalEndColumn ? ['Mandatory'] : []} options={columns} value={intervalEndColumn} - onChange={value => this.setState({ intervalEndColumn: value })} + onChange={( + value: + | string + | number + | (string | number)[] + | null + | undefined, + ) => this.setState({ intervalEndColumn: String(value ?? '') })} /> )} this.setState({ titleColumn: value })} + onChange={( + value: string | number | (string | number)[] | null | undefined, + ) => this.setState({ titleColumn: String(value ?? '') })} /> {annotationType !== ANNOTATION_TYPES.TIME_SERIES && ( this.setState({ descriptionColumns: value })} + onChange={( + value: + | string + | number + | (string | number)[] + | null + | undefined, + ) => { + const cols = Array.isArray(value) ? value.map(String) : []; + this.setState({ descriptionColumns: cols }); + }} /> )}
@@ -784,7 +904,7 @@ class AnnotationLayer extends PureComponent { return ''; } - renderDisplayConfiguration() { + renderDisplayConfiguration(): React.ReactNode { const { color, opacity, @@ -794,9 +914,10 @@ class AnnotationLayer extends PureComponent { hideLine, annotationType, } = this.state; - const colorScheme = getCategoricalSchemeRegistry() - .get(this.props.colorScheme) - .colors.concat(); + const colorScheme = + getCategoricalSchemeRegistry() + .get(this.props.colorScheme) + ?.colors.concat() ?? []; if ( color && color !== AUTOMATIC_COLOR && @@ -823,7 +944,9 @@ class AnnotationLayer extends PureComponent { ]} value={style} clearable={false} - onChange={v => this.setState({ style: v })} + onChange={( + v: string | number | (string | number)[] | null | undefined, + ) => this.setState({ style: String(v ?? 'solid') })} /> this.setState({ opacity: value })} + onChange={( + value: string | number | (string | number)[] | null | undefined, + ) => this.setState({ opacity: String(value ?? '') })} />
ANNOTATION_TYPES_METADATA[type], + type => + ANNOTATION_TYPES_METADATA[ + type as keyof typeof ANNOTATION_TYPES_METADATA + ], ) : []; const supportedSourceTypes = this.getSupportedSourceTypes(annotationType); @@ -989,7 +1119,7 @@ class AnnotationLayer extends PureComponent { @@ -1026,7 +1156,4 @@ class AnnotationLayer extends PureComponent { } } -AnnotationLayer.propTypes = propTypes; -AnnotationLayer.defaultProps = defaultProps; - export default withTheme(AnnotationLayer); diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationTypes.js b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationTypes.ts similarity index 69% rename from superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationTypes.js rename to superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationTypes.ts index cdf7a161f76..b29cde8b6e2 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationTypes.js +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationTypes.ts @@ -18,12 +18,25 @@ */ import { t } from '@superset-ui/core'; -function extractTypes(metadata) { - return Object.keys(metadata).reduce((prev, key) => { - const result = prev; - result[key] = key; - return result; - }, {}); +interface Annotation { + sourceType?: string; + timeColumn?: string; + intervalEndColumn?: string; + titleColumn?: string; + descriptionColumns?: string[]; +} + +function extractTypes>( + metadata: T, +): Record { + return Object.keys(metadata).reduce( + (prev, key) => { + const result = prev; + result[key as keyof T] = key; + return result; + }, + {} as Record, + ); } export const ANNOTATION_TYPES_METADATA = { @@ -62,7 +75,9 @@ export const ANNOTATION_SOURCE_TYPES = extractTypes( ANNOTATION_SOURCE_TYPES_METADATA, ); -export function requiresQuery(annotationSourceType) { +export function requiresQuery( + annotationSourceType: string | undefined, +): boolean { return !!annotationSourceType; } @@ -71,11 +86,16 @@ const NATIVE_COLUMN_NAMES = { intervalEndColumn: 'end_dttm', titleColumn: 'short_descr', descriptionColumns: ['long_descr'], -}; +} as const; -export function applyNativeColumns(annotation) { +export function applyNativeColumns(annotation: Annotation): Annotation { if (annotation.sourceType === ANNOTATION_SOURCE_TYPES.NATIVE) { - return { ...annotation, ...NATIVE_COLUMN_NAMES }; + return { + ...annotation, + ...NATIVE_COLUMN_NAMES, + // Spread to convert readonly array to mutable + descriptionColumns: [...NATIVE_COLUMN_NAMES.descriptionColumns], + }; } return annotation; } diff --git a/superset-frontend/src/explore/components/controls/BoundsControl.test.jsx b/superset-frontend/src/explore/components/controls/BoundsControl.test.tsx similarity index 100% rename from superset-frontend/src/explore/components/controls/BoundsControl.test.jsx rename to superset-frontend/src/explore/components/controls/BoundsControl.test.tsx diff --git a/superset-frontend/src/explore/components/controls/CheckboxControl.jsx b/superset-frontend/src/explore/components/controls/CheckboxControl.tsx similarity index 68% rename from superset-frontend/src/explore/components/controls/CheckboxControl.jsx rename to superset-frontend/src/explore/components/controls/CheckboxControl.tsx index dd551dd996c..f9120159e90 100644 --- a/superset-frontend/src/explore/components/controls/CheckboxControl.jsx +++ b/superset-frontend/src/explore/components/controls/CheckboxControl.tsx @@ -16,22 +16,22 @@ * specific language governing permissions and limitations * under the License. */ -import { Component } from 'react'; -import PropTypes from 'prop-types'; +import { Component, type ReactNode } from 'react'; import { styled, css } from '@apache-superset/core/ui'; import { Checkbox } from '@superset-ui/core/components'; import ControlHeader from '../ControlHeader'; -const propTypes = { - value: PropTypes.bool, - label: PropTypes.string, - onChange: PropTypes.func, -}; - -const defaultProps = { - value: false, - onChange: () => {}, -}; +interface CheckboxControlProps { + value?: boolean; + label?: ReactNode; + name?: string; + description?: ReactNode; + hovered?: boolean; + onChange?: (value: boolean) => void; + validationErrors?: string[]; + placeholder?: string; + debounceDelay?: number; +} const CheckBoxControlWrapper = styled.div` ${({ theme }) => css` @@ -47,28 +47,28 @@ const CheckBoxControlWrapper = styled.div` `} `; -export default class CheckboxControl extends Component { - onChange() { - this.props.onChange(!this.props.value); +export default class CheckboxControl extends Component { + static defaultProps = { + value: false, + onChange: () => {}, + }; + + onChange = (): void => { + this.props.onChange?.(!this.props.value); + }; + + renderCheckbox(): ReactNode { + return ; } - renderCheckbox() { - return ( - - ); - } - - render() { + render(): ReactNode { if (this.props.label) { return ( ); @@ -76,5 +76,3 @@ export default class CheckboxControl extends Component { return this.renderCheckbox(); } } -CheckboxControl.propTypes = propTypes; -CheckboxControl.defaultProps = defaultProps; diff --git a/superset-frontend/src/explore/components/controls/CollectionControl/index.jsx b/superset-frontend/src/explore/components/controls/CollectionControl/index.tsx similarity index 67% rename from superset-frontend/src/explore/components/controls/CollectionControl/index.jsx rename to superset-frontend/src/explore/components/controls/CollectionControl/index.tsx index 04d91a8dde0..cf86cffd646 100644 --- a/superset-frontend/src/explore/components/controls/CollectionControl/index.jsx +++ b/superset-frontend/src/explore/components/controls/CollectionControl/index.tsx @@ -16,12 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import { Component } from 'react'; +import React, { Component } from 'react'; import PropTypes from 'prop-types'; import { IconTooltip, List } from '@superset-ui/core/components'; import { nanoid } from 'nanoid'; import { t } from '@superset-ui/core'; -import { withTheme } from '@apache-superset/core/ui'; +import { withTheme, type SupersetTheme } from '@apache-superset/core/ui'; import { SortableContainer, SortableHandle, @@ -37,6 +37,27 @@ import ControlHeader from 'src/explore/components/ControlHeader'; import CustomListItem from 'src/explore/components/controls/CustomListItem'; import controlMap from '..'; +interface CollectionItem { + key?: string; + [key: string]: unknown; +} + +interface CollectionControlProps { + name: string; + label?: string | null; + description?: string | null; + placeholder?: string; + addTooltip?: string; + itemGenerator?: () => CollectionItem; + keyAccessor?: (item: CollectionItem) => string; + onChange?: (value: CollectionItem[]) => void; + value?: CollectionItem[]; + isFloat?: boolean; + isInt?: boolean; + controlName: string; + theme: SupersetTheme; +} + const propTypes = { name: PropTypes.string.isRequired, label: PropTypes.string, @@ -52,13 +73,13 @@ const propTypes = { controlName: PropTypes.string.isRequired, }; -const defaultProps = { +const defaultProps: Partial = { label: null, description: null, onChange: () => {}, placeholder: t('Empty collection'), itemGenerator: () => ({ key: nanoid(11) }), - keyAccessor: o => o.key, + keyAccessor: (o: CollectionItem) => o.key ?? '', value: [], addTooltip: t('Add an item'), }; @@ -73,63 +94,81 @@ const SortableDragger = SortableHandle(() => ( /> )); -class CollectionControl extends Component { - constructor(props) { +class CollectionControl extends Component { + static propTypes = propTypes; + + static defaultProps = defaultProps; + + constructor(props: CollectionControlProps) { super(props); this.onAdd = this.onAdd.bind(this); } - onChange(i, value) { - const newValue = [...this.props.value]; - newValue[i] = { ...this.props.value[i], ...value }; - this.props.onChange(newValue); + onChange(i: number, value: CollectionItem) { + const currentValue = this.props.value ?? []; + const newValue = [...currentValue]; + newValue[i] = { ...currentValue[i], ...value }; + this.props.onChange?.(newValue); } onAdd() { - this.props.onChange(this.props.value.concat([this.props.itemGenerator()])); + const currentValue = this.props.value ?? []; + const newItem = this.props.itemGenerator?.(); + // Cast needed: original JS allowed undefined items from itemGenerator + this.props.onChange?.( + currentValue.concat([newItem] as unknown as CollectionItem[]), + ); } - onSortEnd({ oldIndex, newIndex }) { - this.props.onChange(arrayMove(this.props.value, oldIndex, newIndex)); + onSortEnd({ oldIndex, newIndex }: { oldIndex: number; newIndex: number }) { + const currentValue = this.props.value ?? []; + this.props.onChange?.(arrayMove(currentValue, oldIndex, newIndex)); } - removeItem(i) { - this.props.onChange(this.props.value.filter((o, ix) => i !== ix)); + removeItem(i: number) { + const currentValue = this.props.value ?? []; + this.props.onChange?.(currentValue.filter((o, ix) => i !== ix)); } renderList() { - if (this.props.value.length === 0) { + const currentValue = this.props.value ?? []; + if (currentValue.length === 0) { return
{this.props.placeholder}
; } - const Control = controlMap[this.props.controlName]; + const Control = (controlMap as Record>)[ + this.props.controlName + ]; + const keyAccessor = + this.props.keyAccessor ?? ((o: CollectionItem) => o.key ?? ''); return ( ({ + css={(theme: SupersetTheme) => ({ borderRadius: theme.borderRadius, })} > - {this.props.value.map((o, i) => { + {currentValue.map((o: CollectionItem, i: number) => { // label relevant only for header, not here - const { label, ...commonProps } = this.props; + const { label, theme, ...commonProps } = this.props; return ( ({ + css={(theme: SupersetTheme) => ({ alignItems: 'center', justifyContent: 'flex-start', display: 'flex', paddingInline: theme.sizeUnit * 6, })} - key={this.props.keyAccessor(o)} + key={keyAccessor(o)} index={i} >
({ + css={(theme: SupersetTheme) => ({ flex: 1, marginLeft: theme.sizeUnit * 2, marginRight: theme.sizeUnit * 2, @@ -148,7 +187,7 @@ class CollectionControl extends Component { tooltip={t('Remove item')} mouseEnterDelay={0} mouseLeaveDelay={0} - css={theme => ({ + css={(theme: SupersetTheme) => ({ padding: 0, minWidth: 'auto', height: 'auto', @@ -190,7 +229,4 @@ class CollectionControl extends Component { } } -CollectionControl.propTypes = propTypes; -CollectionControl.defaultProps = defaultProps; - export default withTheme(CollectionControl); diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx index baf72c653ad..edfcf62c3b8 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx @@ -17,6 +17,7 @@ * under the License. */ +import type React from 'react'; import { Route } from 'react-router-dom'; import fetchMock from 'fetch-mock'; import { DatasourceType, JsonObject, SupersetClient } from '@superset-ui/core'; @@ -28,7 +29,7 @@ import { waitFor, } from 'spec/helpers/testing-library'; import { fallbackExploreInitialData } from 'src/explore/fixtures'; -import type { DatasetObject, ColumnObject } from 'src/features/datasets/types'; +import type { ColumnObject } from 'src/features/datasets/types'; import DatasourceControl from '.'; const SupersetClientGet = jest.spyOn(SupersetClient, 'get'); @@ -46,20 +47,35 @@ afterEach(() => { jest.clearAllMocks(); // Clears mock history but keeps spy in place }); -type TestDatasource = Omit< - Partial, - 'columns' | 'main_dttm_col' -> & { +interface TestDatasource { + id?: number; name: string; - database: { name: string }; + datasource_name?: string; + database: { + id: number; + database_name: string; + name?: string; + backend?: string; + }; columns?: Partial[]; type?: DatasourceType; main_dttm_col?: string | null; -}; + owners?: Array<{ + first_name: string; + last_name: string; + id: number; + username?: string; + }>; + sql?: string; + metrics?: Array<{ id: number; metric_name: string }>; + [key: string]: unknown; +} const mockDatasource: TestDatasource = { id: 25, database: { + id: 1, + database_name: 'examples', name: 'examples', }, name: 'channels', @@ -69,39 +85,50 @@ const mockDatasource: TestDatasource = { owners: [{ first_name: 'john', last_name: 'doe', id: 1, username: 'jd' }], sql: 'SELECT * FROM mock_datasource_sql', }; -const createProps = (overrides: JsonObject = {}) => ({ - hovered: false, - type: 'DatasourceControl', - label: 'Datasource', - default: null, - description: null, - value: '25__table', - form_data: {}, - datasource: mockDatasource, - validationErrors: [], - name: 'datasource', - actions: { - changeDatasource: jest.fn(), - setControlValue: jest.fn(), - }, - isEditable: true, - user: { - createdOn: '2021-04-27T18:12:38.952304', - email: 'admin', - firstName: 'admin', - isActive: true, - lastName: 'admin', - permissions: {}, - roles: { Admin: Array(173) }, - userId: 1, - username: 'admin', - }, - onChange: jest.fn(), - onDatasourceSave: jest.fn(), - ...overrides, -}); -async function openAndSaveChanges(datasource: TestDatasource) { +// Use type assertion for test props since the component is wrapped with withTheme +// The withTheme HOC makes the props type complex, so we cast through unknown to bypass type check +type DatasourceControlComponentProps = React.ComponentProps< + typeof DatasourceControl +>; +const createProps = ( + overrides: JsonObject = {}, +): DatasourceControlComponentProps => + ({ + hovered: false, + type: 'DatasourceControl', + label: 'Datasource', + default: null, + description: null, + value: '25__table', + form_data: {}, + datasource: mockDatasource, + validationErrors: [], + name: 'datasource', + actions: { + changeDatasource: jest.fn(), + setControlValue: jest.fn(), + }, + isEditable: true, + user: { + createdOn: '2021-04-27T18:12:38.952304', + email: 'admin', + firstName: 'admin', + isActive: true, + lastName: 'admin', + permissions: {}, + roles: { Admin: Array(173) }, + userId: 1, + username: 'admin', + }, + onChange: jest.fn(), + onDatasourceSave: jest.fn(), + ...overrides, + }) as unknown as DatasourceControlComponentProps; + +async function openAndSaveChanges( + datasource: TestDatasource | Record, +) { fetchMock.get( 'glob:*/api/v1/database/?q=*', { result: [] }, @@ -259,7 +286,6 @@ test('Click on Edit dataset', async () => { test('Edit dataset should be disabled when user is not admin', async () => { const props = createProps(); - // @ts-expect-error props.user.roles = {}; props.datasource.owners = []; SupersetClientGet.mockImplementationOnce( @@ -458,11 +484,11 @@ test('should not set the temporal column', async () => { const overrideProps = { ...props, form_data: { - granularity_sqla: null, + granularity_sqla: undefined, }, datasource: { ...props.datasource, - main_dttm_col: null, + main_dttm_col: undefined, columns: [ { column_name: 'test-col', diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl/index.tsx similarity index 80% rename from superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx rename to superset-frontend/src/explore/components/controls/DatasourceControl/index.tsx index ed582b088eb..6c611f69d65 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl/index.tsx @@ -18,10 +18,20 @@ * under the License. */ -import { PureComponent } from 'react'; +import React, { PureComponent } from 'react'; import PropTypes from 'prop-types'; -import { DatasourceType, SupersetClient, t } from '@superset-ui/core'; -import { css, styled, withTheme } from '@apache-superset/core/ui'; +import { + DatasourceType, + SupersetClient, + t, + Datasource, +} from '@superset-ui/core'; +import { + css, + styled, + withTheme, + type SupersetTheme, +} from '@apache-superset/core/ui'; import { getTemporalColumns } from '@superset-ui/chart-controls'; import { getUrlParam } from 'src/utils/urlUtils'; import { @@ -51,6 +61,68 @@ import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal'; import { safeStringify } from 'src/utils/safeStringify'; import { Link } from 'react-router-dom'; +// Extended Datasource interface with all properties used in this component +interface ExtendedDatasource extends Datasource { + sql?: string; + select_star?: string; + owners?: Array<{ + id: number; + first_name: string; + last_name: string; + value?: number; + }>; + extra?: string; + health_check_message?: string; + database?: { + id: number; + database_name: string; + backend?: string; + }; +} + +interface User { + userId?: number; + username?: string; + roles?: Record; +} + +interface DatasourceControlActions { + changeDatasource: (datasource: ExtendedDatasource) => void; + setControlValue: (name: string, value: unknown) => void; +} + +interface FormData { + granularity_sqla?: string; + [key: string]: unknown; +} + +interface DatasourceControlProps { + actions: DatasourceControlActions; + onChange?: () => void; + value?: string | null; + datasource: ExtendedDatasource; + form_data?: FormData; + isEditable?: boolean; + onDatasourceSave?: ((datasource: ExtendedDatasource) => void) | null; + theme: SupersetTheme; + user: User; + // ControlHeader-related props + hovered?: boolean; + type?: string; + label?: string; + default?: unknown; + description?: string | null; + validationErrors?: string[]; + name?: string; +} + +interface DatasourceControlState { + showEditDatasourceModal: boolean; + showChangeDatasourceModal: boolean; + showSaveDatasetModal: boolean; + showDatasource?: boolean; +} + const propTypes = { actions: PropTypes.object.isRequired, onChange: PropTypes.func, @@ -59,6 +131,15 @@ const propTypes = { form_data: PropTypes.object.isRequired, isEditable: PropTypes.bool, onDatasourceSave: PropTypes.func, + user: PropTypes.object.isRequired, + // ControlHeader-related props + hovered: PropTypes.bool, + type: PropTypes.string, + label: PropTypes.string, + default: PropTypes.any, + description: PropTypes.string, + validationErrors: PropTypes.array, + name: PropTypes.string, }; const defaultProps = { @@ -68,7 +149,7 @@ const defaultProps = { isEditable: true, }; -const getDatasetType = datasource => { +const getDatasetType = (datasource: ExtendedDatasource): string => { if (datasource.type === 'query') { return 'query'; } @@ -139,15 +220,18 @@ const SAVE_AS_DATASET = 'save_as_dataset'; const VISIBLE_TITLE_LENGTH = 25; // Assign icon for each DatasourceType. If no icon assignment is found in the lookup, no icon will render -export const datasourceIconLookup = { +export const datasourceIconLookup: Record = { query: , physical_dataset: , virtual_dataset: , }; // Render title for datasource with tooltip only if text is longer than VISIBLE_TITLE_LENGTH -export const renderDatasourceTitle = (displayString, tooltip) => - displayString?.length > VISIBLE_TITLE_LENGTH ? ( +export const renderDatasourceTitle = ( + displayString: string | undefined, + tooltip: string, +) => + displayString?.length && displayString.length > VISIBLE_TITLE_LENGTH ? ( // Add a tooltip only for long names that will be visually truncated {displayString} @@ -159,12 +243,14 @@ export const renderDatasourceTitle = (displayString, tooltip) => ); // Different data source types use different attributes for the display title -export const getDatasourceTitle = datasource => { - if (datasource?.type === 'query') return datasource?.sql; +export const getDatasourceTitle = ( + datasource: ExtendedDatasource | null | undefined, +): string => { + if (datasource?.type === 'query') return datasource?.sql || ''; return datasource?.name || ''; }; -const preventRouterLinkWhileMetaClicked = evt => { +const preventRouterLinkWhileMetaClicked = (evt: React.MouseEvent) => { if (evt.metaKey) { evt.preventDefault(); } else { @@ -172,8 +258,15 @@ const preventRouterLinkWhileMetaClicked = evt => { } }; -class DatasourceControl extends PureComponent { - constructor(props) { +class DatasourceControl extends PureComponent< + DatasourceControlProps, + DatasourceControlState +> { + static propTypes = propTypes; + + static defaultProps = defaultProps; + + constructor(props: DatasourceControlProps) { super(props); this.state = { showEditDatasourceModal: false, @@ -182,10 +275,13 @@ class DatasourceControl extends PureComponent { }; } - onDatasourceSave = datasource => { - this.props.actions.changeDatasource(datasource); - const { temporalColumns, defaultTemporalColumn } = - getTemporalColumns(datasource); + onDatasourceSave = (datasource: Datasource) => { + // Cast to ExtendedDatasource for the component's internal use + this.props.actions.changeDatasource(datasource as ExtendedDatasource); + // Cast datasource for getTemporalColumns which expects Dataset | QueryResponse + const { temporalColumns, defaultTemporalColumn } = getTemporalColumns( + datasource as Parameters[0], + ); const { columns } = datasource; // the current granularity_sqla might not be a temporal column anymore const timeCol = this.props.form_data?.granularity_sqla; @@ -238,7 +334,7 @@ class DatasourceControl extends PureComponent { })); }; - handleMenuItemClick = ({ key }) => { + handleMenuItemClick = ({ key }: { key: string }) => { switch (key) { case CHANGE_DATASET: this.toggleChangeDatasourceModal(); @@ -371,12 +467,17 @@ class DatasourceControl extends PureComponent { modalBody={ } modalFooter={ } draggable={false} @@ -406,7 +507,7 @@ class DatasourceControl extends PureComponent { queryDatasourceMenuItems.push({ key: SAVE_AS_DATASET, - label: t('Save as dataset'), + label: {t('Save as dataset')}, }); const queryDatasourceMenu = ( @@ -464,8 +565,8 @@ class DatasourceControl extends PureComponent { {isMissingDatasource && isMissingParams && (