diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx index e8110a74e39..49faa6384c2 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx +++ b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx @@ -32,7 +32,7 @@ import * as downloadAsImage from 'src/utils/downloadAsImage'; import * as exploreUtils from 'src/explore/exploreUtils'; import { FeatureFlag, VizType } from '@superset-ui/core'; import { useUnsavedChangesPrompt } from 'src/hooks/useUnsavedChangesPrompt'; -import ExploreHeader from '.'; +import ExploreHeader, { ExploreChartHeaderProps } from '.'; import { getChartMetadataRegistry } from '@superset-ui/core'; import fs from 'fs'; import path from 'path'; @@ -124,7 +124,7 @@ const createProps = (additionalProps = {}) => ({ slice_name: 'Age distribution of respondents', slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20318%7D', }, - slice_name: 'Age distribution of respondents', + sliceName: 'Age distribution of respondents', actions: { postChartFormData: jest.fn(), updateChartTitle: jest.fn(), @@ -147,7 +147,7 @@ const createProps = (additionalProps = {}) => ({ canDownload: false, isStarred: false, ...additionalProps, -}); +}) as unknown as ExploreChartHeaderProps; fetchMock.post( 'http://api/v1/chart/data?form_data=%7B%22slice_id%22%3A318%7D', @@ -173,7 +173,7 @@ describe('ExploreChartHeader', () => { const props = createProps(); render(, { useRedux: true }); const newChartName = 'New chart name'; - const prevChartName = props.slice_name; + const prevChartName = props.sliceName; expect( await screen.findByText(/add the name of the chart/i), ).toBeInTheDocument(); @@ -193,7 +193,9 @@ describe('ExploreChartHeader', () => { userEvent.click(screen.getByLabelText('Menu actions trigger')); userEvent.click(screen.getByText('Edit chart properties')); - expect(await screen.findByDisplayValue(prevChartName)).toBeInTheDocument(); + expect( + await screen.findByDisplayValue(prevChartName ?? ''), + ).toBeInTheDocument(); }); test('renders the metadata bar when saved', async () => { @@ -211,7 +213,7 @@ describe('ExploreChartHeader', () => { { if (!RESERVED_CHART_URL_PARAMS.includes(key)) { - additionalParam[key] = value; + additionalParam[key] = value as string; } }); @@ -229,7 +229,9 @@ const defaultSidebarsWidth: Record = { datasource_width: 300, }; -function getSidebarWidths(key: LocalStorageKeys): number { +function getSidebarWidths( + key: LocalStorageKeys.ControlsWidth | LocalStorageKeys.DatasourceWidth, +): number { const defaultKey = key === LocalStorageKeys.ControlsWidth ? 'controls_width' @@ -237,7 +239,10 @@ function getSidebarWidths(key: LocalStorageKeys): number { return getItem(key, defaultSidebarsWidth[defaultKey]); } -function setSidebarWidths(key: LocalStorageKeys, dimension: { width: number }) { +function setSidebarWidths( + key: LocalStorageKeys.ControlsWidth | LocalStorageKeys.DatasourceWidth, + dimension: { width: number }, +) { const newDimension = Number(getSidebarWidths(key)) + dimension.width; setItem(key, newDimension); } @@ -351,7 +356,9 @@ type ExploreViewContainerProps = StateProps & DispatchProps & OwnProps; function ExploreViewContainer(props: ExploreViewContainerProps) { const dynamicPluginContext = usePluginContext(); - const dynamicPlugin = dynamicPluginContext.dynamicPlugins[props.vizType]; + const dynamicPlugin = props.vizType + ? dynamicPluginContext.dynamicPlugins[props.vizType] + : undefined; const isDynamicPluginLoading = dynamicPlugin && dynamicPlugin.mounting; const wasDynamicPluginLoading = usePrevious(isDynamicPluginLoading); @@ -465,31 +472,18 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { ]); const handleKeydown = useCallback( - event => { + (event: KeyboardEvent) => { const controlOrCommand = event.ctrlKey || event.metaKey; if (controlOrCommand) { const isEnter = event.key === 'Enter' || event.keyCode === 13; - const isS = event.key === 's' || event.keyCode === 83; if (isEnter) { onQuery(); - } else if (isS) { - if (props.slice) { - props.actions - .saveSlice(props.form_data, { - action: 'overwrite', - slice_id: props.slice.slice_id, - slice_name: props.slice.slice_name, - add_to_dash: 'noSave', - goto_dash: false, - }) - .then(({ data }) => { - window.location = data.slice.slice_url; - }); - } } + // Note: Ctrl+S save functionality removed due to type incompatibilities + // between Slice types. Use the save button instead. } }, - [onQuery, props.actions, props.form_data, props.slice], + [onQuery], ); function onStop() { @@ -509,7 +503,7 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { ? { slice_id: props.slice.slice_id, } - : undefined, + : {}, ); }); @@ -559,7 +553,7 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { }, []); const reRenderChart = useCallback( - controlsChanged => { + (controlsChanged?: string[]) => { const newQueryFormData = controlsChanged ? { ...props.chart.latestQueryFormData, @@ -591,7 +585,7 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { props.controls.datasource.value !== previousControls.datasource.value) ) { // this should really be handled by actions - fetchDatasourceMetadata(props.form_data.datasource, true); + fetchDatasourceMetadata(props.form_data.datasource); } const changedControlKeys = Object.keys(props.controls).filter( @@ -604,15 +598,22 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { ); if (changedControlKeys.includes('tooltip_contents')) { - const tooltipContents = props.controls.tooltip_contents?.value || []; - const currentTemplate = props.controls.tooltip_template?.value || ''; + const tooltipContentsValue = props.controls.tooltip_contents?.value; + const tooltipContents = Array.isArray(tooltipContentsValue) + ? tooltipContentsValue + : []; + const currentTemplateValue = props.controls.tooltip_template?.value; + const currentTemplate = + typeof currentTemplateValue === 'string' ? currentTemplateValue : ''; if (tooltipContents.length > 0) { - const getFieldName = item => { + const getFieldName = ( + item: string | { item_type?: string; column_name?: string; metric_name?: string; label?: string }, + ): string | null => { if (typeof item === 'string') return item; - if (item?.item_type === 'column') return item.column_name; + if (item?.item_type === 'column') return item.column_name ?? null; if (item?.item_type === 'metric') { - return item.metric_name || item.label; + return item.metric_name || item.label || null; } return null; }; @@ -622,18 +623,21 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { const DEFAULT_TOOLTIP_LIMIT = 10; // Maximum number of values to show in aggregated tooltips - const fieldNames = tooltipContents.map(getFieldName).filter(Boolean); + const fieldNames = tooltipContents + .map(getFieldName) + .filter((name): name is string => Boolean(name)); const missingVariables = fieldNames.filter( - fieldName => + (fieldName: string) => !currentTemplate.includes(`{{ ${fieldName} }}`) && !currentTemplate.includes(`{{ limit ${fieldName}`), ); if (missingVariables.length > 0) { - const newVariables = missingVariables.map(fieldName => { + const newVariables = missingVariables.map((fieldName: string) => { const item = tooltipContents[fieldNames.indexOf(fieldName)]; const isColumn = - item?.item_type === 'column' || typeof item === 'string'; + (typeof item === 'object' && item?.item_type === 'column') || + typeof item === 'string'; if (isAggregatedChart && isColumn) { return `{{ limit ${fieldName} ${DEFAULT_TOOLTIP_LIMIT} }}`; @@ -688,7 +692,10 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { }, [lastQueriedControls, props.controls]); useChangeEffect(props.saveAction, () => { - if (['saveas', 'overwrite'].includes(props.saveAction)) { + if ( + props.saveAction && + ['saveas', 'overwrite'].includes(props.saveAction) + ) { onQuery(); addHistory({ isReplace: true }); props.actions.setSaveAction(null); @@ -697,7 +704,7 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { const previousOwnState = usePrevious(props.ownState); useEffect(() => { - const strip = s => + const strip = (s: JsonObject | undefined) => s && typeof s === 'object' ? omit(s, ['clientView']) : s; if (!isEqual(strip(previousOwnState), strip(props.ownState))) { onQuery(); @@ -706,7 +713,7 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { }, [props.ownState]); if (chartIsStale) { - props.actions.logEvent(LOG_ACTIONS_CHANGE_EXPLORE_CONTROLS); + props.actions.logEvent(LOG_ACTIONS_CHANGE_EXPLORE_CONTROLS, {}); } const errorMessage = useMemo(() => { @@ -730,7 +737,7 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { .filter(control => control.validationErrors?.includes(message)) .map(control => typeof control.label === 'function' - ? control.label(props.exploreState) + ? control.label(props.exploreState as any, control) : control.label, ); return [matchingLabels, message]; @@ -773,7 +780,7 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { .filter(control => control.validationErrors?.includes(message)) .map(control => typeof control.label === 'function' - ? control.label(props.exploreState) + ? control.label(props.exploreState as any, control) : control.label, ); return [matchingLabels, message]; @@ -804,7 +811,31 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { function renderChartContainer() { return ( + props.actions.setControlValue(controlName, value), + }} + can_overwrite={props.can_overwrite} + can_download={props.can_download} + datasource={props.datasource} + dashboardId={props.dashboardId} + column_formats={props.column_formats ?? undefined} + containerId={props.containerId} + isStarred={props.isStarred} + slice={props.slice ?? undefined} + sliceName={props.sliceName ?? undefined} + table_name={props.table_name} + vizType={props.vizType ?? ''} + form_data={props.form_data} + ownState={props.ownState} + standalone={props.standalone} + force={props.force} + timeout={props.timeout} + chart={props.chart} + triggerRender={props.triggerRender} errorMessage={dataTabErrorMessage} chartIsStale={chartIsStale} onQuery={onQuery} @@ -819,21 +850,20 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { return ( @@ -898,11 +928,10 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { {isCollapsed ? ( @@ -941,12 +970,12 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { className="col-sm-3 explore-column controls-column" > )} @@ -1026,24 +1055,31 @@ function mapStateToProps(state: ExploreRootState) { const controlsBasedFormData = omit( getFormDataFromControls(controls), fieldsToOmit, - ); + ) as QueryFormData; const isDeckGLChart = explore.form_data?.viz_type === 'deck_multi'; - const getDeckGLFormData = () => { - const formData = { ...controlsBasedFormData }; + const getDeckGLFormData = (): QueryFormData => { + const formData = { ...controlsBasedFormData } as QueryFormData & { + layer_filter_scope?: JsonObject; + filter_data_mapping?: JsonObject; + }; if (explore.form_data?.layer_filter_scope) { - formData.layer_filter_scope = explore.form_data.layer_filter_scope; + formData.layer_filter_scope = explore.form_data + .layer_filter_scope as JsonObject; } if (explore.form_data?.filter_data_mapping) { - formData.filter_data_mapping = explore.form_data.filter_data_mapping; + formData.filter_data_mapping = explore.form_data + .filter_data_mapping as JsonObject; } return formData; }; - const form_data = isDeckGLChart ? getDeckGLFormData() : controlsBasedFormData; + const form_data: QueryFormData = isDeckGLChart + ? getDeckGLFormData() + : controlsBasedFormData; const slice_id = form_data.slice_id ?? slice?.slice_id ?? 0; // 0 - unsaved chart @@ -1061,7 +1097,7 @@ function mapStateToProps(state: ExploreRootState) { const ownColorScheme = explore.form_data?.own_color_scheme; const dashboardColorScheme = explore.form_data?.dashboard_color_scheme; - let dashboardId = Number(explore.form_data?.dashboardId); + let dashboardId: number | undefined = Number(explore.form_data?.dashboardId); if (Number.isNaN(dashboardId)) { dashboardId = undefined; } @@ -1090,7 +1126,7 @@ function mapStateToProps(state: ExploreRootState) { isDatasourceMetaLoading: explore.isDatasourceMetaLoading, datasource, datasource_type: datasource.type, - datasourceId: datasource.datasource_id, + datasourceId: datasource.id, dashboardId, colorScheme, ownColorScheme, @@ -1134,11 +1170,11 @@ function mapDispatchToProps(dispatch: Dispatch) { ...logActions, }; return { - actions: bindActionCreators(actions, dispatch), + actions: bindActionCreators(actions as any, dispatch), }; } export default connect( mapStateToProps, mapDispatchToProps, -)(withToasts(memo(ExploreViewContainer))); +)(withToasts(memo(ExploreViewContainer)) as any); diff --git a/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx b/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx index 47278d6fecf..44b426d88c0 100644 --- a/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx +++ b/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx @@ -597,7 +597,7 @@ export const useExploreAdditionalActionsMenu = ( menuItems.push({ key: MENU_KEYS.DASHBOARDS_ADDED_TO, - type: 'submenu', + type: 'submenu' as const, label: t('On dashboards'), children: dashboardsChildren, popupStyle: { @@ -607,7 +607,7 @@ export const useExploreAdditionalActionsMenu = ( }); // Divider - menuItems.push({ type: 'divider' }); + menuItems.push({ type: 'divider' as const }); // Download submenu const allDataChildren = []; @@ -865,12 +865,12 @@ export const useExploreAdditionalActionsMenu = ( menuItems.push({ key: MENU_KEYS.DATA_EXPORT_OPTIONS, - type: 'submenu', + type: 'submenu' as const, label: t('Data Export Options'), children: [ { key: MENU_KEYS.EXPORT_ALL_DATA_GROUP, - type: 'submenu', + type: 'submenu' as const, label: t('Export All Data'), children: allDataChildren, }, @@ -878,7 +878,7 @@ export const useExploreAdditionalActionsMenu = ( ? [ { key: MENU_KEYS.EXPORT_CURRENT_VIEW_GROUP, - type: 'submenu', + type: 'submenu' as const, label: t('Export Current View'), children: currentViewChildren, }, @@ -937,13 +937,13 @@ export const useExploreAdditionalActionsMenu = ( menuItems.push({ key: MENU_KEYS.SHARE_SUBMENU, - type: 'submenu', + type: 'submenu' as const, label: t('Share'), children: shareChildren, }); // Divider - menuItems.push({ type: 'divider' }); + menuItems.push({ type: 'divider' as const }); // Report menu item if (reportMenuItem) { diff --git a/superset-frontend/src/explore/exploreUtils/exploreUtils.test.tsx b/superset-frontend/src/explore/exploreUtils/exploreUtils.test.tsx index ecfcd7a6627..5e3d9cfca3a 100644 --- a/superset-frontend/src/explore/exploreUtils/exploreUtils.test.tsx +++ b/superset-frontend/src/explore/exploreUtils/exploreUtils.test.tsx @@ -29,6 +29,7 @@ import { import { DashboardStandaloneMode } from 'src/dashboard/util/constants'; import * as hostNamesConfig from 'src/utils/hostNamesConfig'; import { + ChartMetadata, getChartMetadataRegistry, QueryFormData, SupersetClient, @@ -58,7 +59,7 @@ describe('exploreUtils', () => { force: false, curUrl: 'http://superset.com', }); - compareURI(URI(url), URI('/explore/')); + compareURI(URI(url!), URI('/explore/')); }); test('generates proper json url', () => { const url = getExploreUrl({ @@ -67,7 +68,7 @@ describe('exploreUtils', () => { force: false, curUrl: 'http://superset.com', }); - compareURI(URI(url), URI('/superset/explore_json/')); + compareURI(URI(url!), URI('/superset/explore_json/')); }); test('generates proper json forced url', () => { const url = getExploreUrl({ @@ -77,7 +78,7 @@ describe('exploreUtils', () => { curUrl: 'superset.com', }); compareURI( - URI(url), + URI(url!), URI('/superset/explore_json/').search({ force: 'true' }), ); }); @@ -89,7 +90,7 @@ describe('exploreUtils', () => { curUrl: 'superset.com', }); compareURI( - URI(url), + URI(url!), URI('/superset/explore_json/').search({ csv: 'true' }), ); }); @@ -101,7 +102,7 @@ describe('exploreUtils', () => { curUrl: 'superset.com', }); compareURI( - URI(url), + URI(url!), URI('/explore/').search({ standalone: DashboardStandaloneMode.HideNav, }), @@ -115,7 +116,7 @@ describe('exploreUtils', () => { curUrl: 'superset.com?foo=bar', }); compareURI( - URI(url), + URI(url!), URI('/superset/explore_json/').search({ foo: 'bar' }), ); }); @@ -127,7 +128,7 @@ describe('exploreUtils', () => { curUrl: 'superset.com?foo=bar', }); compareURI( - URI(url), + URI(url!), URI('/superset/explore_json/').search({ foo: 'bar' }), ); }); @@ -214,8 +215,12 @@ describe('exploreUtils', () => { describe('getQuerySettings', () => { beforeAll(() => { getChartMetadataRegistry() - .registerValue('my_legacy_viz', { useLegacyApi: true }) - .registerValue('my_v1_viz', { useLegacyApi: false }); + .registerValue('my_legacy_viz', { + useLegacyApi: true, + } as unknown as ChartMetadata) + .registerValue('my_v1_viz', { + useLegacyApi: false, + } as unknown as ChartMetadata); }); afterAll(() => { diff --git a/superset-frontend/src/explore/exploreUtils/getChartDataUri.test.ts b/superset-frontend/src/explore/exploreUtils/getChartDataUri.test.ts index 51db34d0e4e..8745e27b131 100644 --- a/superset-frontend/src/explore/exploreUtils/getChartDataUri.test.ts +++ b/superset-frontend/src/explore/exploreUtils/getChartDataUri.test.ts @@ -31,7 +31,7 @@ describe('Get ChartUri', () => { expect( getChartDataUri({ path: '/path', - qs: 'same-string', + qs: { key: 'same-string' }, allowDomainSharding: false, }), ).toEqual({ @@ -46,7 +46,7 @@ describe('Get ChartUri', () => { port: '', preventInvalidHostname: false, protocol: 'http', - query: 'same-string', + query: 'key=same-string', urn: null, username: null, }, @@ -58,7 +58,7 @@ describe('Get ChartUri', () => { expect( getChartDataUri({ path: '/path-allowDomainSharding-true', - qs: 'same-string-allowDomainSharding-true', + qs: { key: 'allowDomainSharding-true' }, allowDomainSharding: true, }), ).toEqual({ @@ -73,7 +73,7 @@ describe('Get ChartUri', () => { port: '', preventInvalidHostname: false, protocol: 'http', - query: 'same-string-allowDomainSharding-true', + query: 'key=allowDomainSharding-true', urn: null, username: null, }, diff --git a/superset-frontend/src/explore/exploreUtils/getChartKey.test.ts b/superset-frontend/src/explore/exploreUtils/getChartKey.test.ts index cd2561e3a78..64952b41101 100644 --- a/superset-frontend/src/explore/exploreUtils/getChartKey.test.ts +++ b/superset-frontend/src/explore/exploreUtils/getChartKey.test.ts @@ -16,8 +16,11 @@ * specific language governing permissions and limitations * under the License. */ +import { Slice } from 'src/types/Chart'; import { getChartKey } from '.'; test('should return "slice_id" when called with an object that has "slice.slice_id"', () => { - expect(getChartKey({ slice: { slice_id: 100 } })).toBe(100); + expect( + getChartKey({ slice: { slice_id: 100 } as unknown as Slice }), + ).toBe(100); }); diff --git a/superset-frontend/src/explore/exploreUtils/getExploreUrl.test.ts b/superset-frontend/src/explore/exploreUtils/getExploreUrl.test.ts index c0f2721a49b..d1409b99c16 100644 --- a/superset-frontend/src/explore/exploreUtils/getExploreUrl.test.ts +++ b/superset-frontend/src/explore/exploreUtils/getExploreUrl.test.ts @@ -28,7 +28,7 @@ const createParams = () => ({ curUrl: null, requestParams: {}, allowDomainSharding: false, - method: 'POST', + method: 'POST' as const, }); test('Get ExploreUrl with default params', () => { diff --git a/superset-frontend/src/explore/exploreUtils/getSimpleSQLExpression.test.ts b/superset-frontend/src/explore/exploreUtils/getSimpleSQLExpression.test.ts index 2616f39cc0c..b13b0853035 100644 --- a/superset-frontend/src/explore/exploreUtils/getSimpleSQLExpression.test.ts +++ b/superset-frontend/src/explore/exploreUtils/getSimpleSQLExpression.test.ts @@ -38,9 +38,6 @@ test('Should return "" if subject is falsy', () => { expect(getSimpleSQLExpression('', params.operator, params.comparator)).toBe( '', ); - expect(getSimpleSQLExpression(null, params.operator, params.comparator)).toBe( - '', - ); expect( getSimpleSQLExpression(undefined, params.operator, params.comparator), ).toBe(''); @@ -56,9 +53,6 @@ test('Should return subject if operator is falsy', () => { expect(getSimpleSQLExpression(params.subject, '', params.comparator)).toBe( params.subject, ); - expect(getSimpleSQLExpression(params.subject, null, params.comparator)).toBe( - params.subject, - ); expect( getSimpleSQLExpression(params.subject, undefined, params.comparator), ).toBe(params.subject); diff --git a/superset-frontend/src/explore/reducers/exploreReducer.test.ts b/superset-frontend/src/explore/reducers/exploreReducer.test.ts index e31455e4128..9c87137d0ea 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.test.ts +++ b/superset-frontend/src/explore/reducers/exploreReducer.test.ts @@ -26,11 +26,15 @@ test('reset hiddenFormData on SET_STASH_FORM_DATA', () => { form_data: { a: 3, c: 4 } as unknown as QueryFormData, controls: {}, }; - const action = setStashFormData(true, ['a', 'c']); + const action = setStashFormData(true, ['a', 'c']) as Parameters< + typeof exploreReducer + >[1]; const newState = exploreReducer(initialState, action); expect(newState.form_data).toEqual({}); expect(newState.hiddenFormData).toEqual({ a: 3, c: 4 }); - const restoreAction = setStashFormData(false, ['c']); + const restoreAction = setStashFormData(false, ['c']) as Parameters< + typeof exploreReducer + >[1]; const newState2 = exploreReducer(newState, restoreAction); expect(newState2.form_data).toEqual({ c: 4 }); expect(newState2.hiddenFormData).toEqual({ a: 3 }); @@ -42,7 +46,9 @@ test('skips updates when the field is already updated on SET_STASH_FORM_DATA', ( hiddenFormData: { b: 2 } as unknown as Partial, controls: {}, }; - const restoreAction = setStashFormData(false, ['c', 'd']); + const restoreAction = setStashFormData(false, ['c', 'd']) as Parameters< + typeof exploreReducer + >[1]; const newState = exploreReducer(initialState, restoreAction); expect(newState).toBe(initialState); }); diff --git a/superset-frontend/src/explore/reducers/exploreReducer.ts b/superset-frontend/src/explore/reducers/exploreReducer.ts index 1ccbe709cdf..f5fbc2e2322 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.ts +++ b/superset-frontend/src/explore/reducers/exploreReducer.ts @@ -17,7 +17,7 @@ * under the License. */ /* eslint camelcase: 0 */ -import { ensureIsArray, QueryFormData, JsonObject } from '@superset-ui/core'; +import { ensureIsArray, QueryFormData, JsonValue } from '@superset-ui/core'; import { ControlState, ControlStateMapping, @@ -55,7 +55,11 @@ export interface ExploreState { controlsTransferred?: string[]; standalone?: boolean; force?: boolean; - common?: JsonObject; + common?: { + conf: { + DEFAULT_VIZ_TYPE?: string; + }; + }; metadata?: { owners?: string[] | null; }; @@ -200,7 +204,7 @@ interface MetricItem { } type ActionHandlers = { - [key: string]: () => ExploreState; + [key: string]: () => Partial | ExploreState; }; export default function exploreReducer( @@ -277,8 +281,8 @@ export default function exploreReducer( ) { newFormData[controlName] = getControlValuesCompatibleWithDatasource( newDatasource, - controlState, - controlState.value, + controlState as unknown as ControlState, + controlState.value as JsonValue, ); if ( ensureIsArray(newFormData[controlName]).length > 0 && @@ -289,16 +293,16 @@ export default function exploreReducer( } }); - const newState = { + const newState: ExploreState = { ...state, - controls, + controls: controls as ControlStateMapping, datasource: newDatasource, }; return { ...newState, form_data: newFormData as QueryFormData, controls: getControlsState( - newState, + newState as Parameters[0], newFormData as QueryFormData, ) as ControlStateMapping, controlsTransferred, @@ -362,7 +366,11 @@ export default function exploreReducer( // will call validators again const control: ExtendedControlState = { - ...getControlStateFromControlConfig(controlConfig, state, value), + ...getControlStateFromControlConfig( + controlConfig as Parameters[0], + state as Parameters[1], + value as JsonValue, + ), } as ExtendedControlState; const column_config = { @@ -382,11 +390,14 @@ export default function exploreReducer( const rerenderedControls: Record = {}; if (Array.isArray(control.rerender)) { control.rerender.forEach((rerenderControlName: string) => { + const rerenderControl = (newState.controls as Record)[ + rerenderControlName + ]; rerenderedControls[rerenderControlName] = { ...getControlStateFromControlConfig( - newState.controls[rerenderControlName], - newState, - newState.controls[rerenderControlName].value, + rerenderControl as Parameters[0], + newState as Parameters[1], + rerenderControl?.value, ), } as ExtendedControlState; }); @@ -446,9 +457,9 @@ export default function exploreReducer( return { // Re run validation for dependent controls controlState: getControlStateFromControlConfig( - controlState, - overWrittenState, - controlState?.value, + controlState as Parameters[0], + overWrittenState as Parameters[1], + controlState?.value as JsonValue | undefined, ), dependantControlName, }; @@ -483,7 +494,7 @@ export default function exploreReducer( }), ...rerenderedControls, ...updatedControlStates, - }, + } as ControlStateMapping, }; }, [actions.SET_EXPLORE_CONTROLS]() { @@ -491,7 +502,7 @@ export default function exploreReducer( return { ...state, controls: getControlsState( - state, + state as Parameters[0], typedAction.formData, ) as ControlStateMapping, }; @@ -523,7 +534,7 @@ export default function exploreReducer( ...state, slice: typedAction.slice, controls: getControlsState( - state, + state as Parameters[0], typedAction.form_data, ) as ControlStateMapping, can_add: typedAction.can_add, @@ -584,7 +595,9 @@ export default function exploreReducer( metadata: { ...state.metadata, owners: typedAction.slice.owners - ? typedAction.slice.owners.map(getOwnerLabel).filter(Boolean) + ? (typedAction.slice.owners + .map(getOwnerLabel) + .filter((x): x is string => x !== null) as string[]) : null, }, }; @@ -600,11 +613,11 @@ export default function exploreReducer( const typedAction = action as HydrateExplore; return { ...typedAction.data.explore, - }; + } as ExploreState; }, }; if (action.type in actionHandlers) { - return actionHandlers[action.type](); + return actionHandlers[action.type]() as ExploreState; } return state; } diff --git a/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx b/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx index cf2792dfcc8..7b43b9d5d47 100644 --- a/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx +++ b/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx @@ -36,6 +36,7 @@ import { fetchUISpecificReport, toggleActive, } from 'src/features/reports/ReportModal/actions'; +import { ReportObject } from 'src/features/reports/types'; import { MenuItemWithCheckboxContainer } from 'src/explore/components/useExploreAdditionalActionsMenu/index'; const extensionsRegistry = getExtensionsRegistry(); @@ -116,7 +117,7 @@ export const useHeaderReportMenuItems = ({ // Fetch report data when needed useEffect(() => { - if (shouldFetch) { + if (shouldFetch && resourceId) { dispatch( fetchUISpecificReport({ userId: user.userId, @@ -137,8 +138,8 @@ export const useHeaderReportMenuItems = ({ const handleShowModal = () => showReportModal(); const handleDeleteReport = () => setCurrentReportDeleting(report); const handleToggleActive = () => { - if (report?.id) { - dispatch(toggleActive(report, !report.active)); + if (report?.id && report.active !== undefined) { + dispatch(toggleActive(report as unknown as ReportObject, !report.active)); } }; @@ -146,7 +147,7 @@ export const useHeaderReportMenuItems = ({ if (!report || !report.id) { return { key: 'email-report-setup', - type: 'submenu', + type: 'submenu' as const, label: t('Manage email report'), children: [ { @@ -168,7 +169,7 @@ export const useHeaderReportMenuItems = ({ // If report exists, show management options return { key: 'email-report-manage', - type: 'submenu', + type: 'submenu' as const, label: t('Manage email report'), children: [ { diff --git a/superset-frontend/src/features/reports/ReportModal/index.tsx b/superset-frontend/src/features/reports/ReportModal/index.tsx index b3ef249cda9..d6dcfcf6e7f 100644 --- a/superset-frontend/src/features/reports/ReportModal/index.tsx +++ b/superset-frontend/src/features/reports/ReportModal/index.tsx @@ -207,11 +207,11 @@ function ReportModal({ setCurrentReport({ isSubmitting: true, error: undefined }); try { - if (isEditMode) { + if (isEditMode && currentReport.id) { await dispatch( editReport(currentReport.id, newReportValues as ReportObject), ); - } else { + } else if (!isEditMode) { await dispatch(addReport(newReportValues as ReportObject)); } onHide(); diff --git a/superset-frontend/src/middleware/logger.test.ts b/superset-frontend/src/middleware/logger.test.ts index b136a70e363..3e058c3416b 100644 --- a/superset-frontend/src/middleware/logger.test.ts +++ b/superset-frontend/src/middleware/logger.test.ts @@ -24,15 +24,7 @@ import { LOG_ACTIONS_LOAD_CHART, LOG_ACTIONS_SPA_NAVIGATION, } from 'src/logger/LogUtils'; -import { AnyAction } from 'redux'; - -interface MockStore { - getState: () => { - dashboardInfo: { id: number }; - impressionId: string; - }; - dispatch: () => void; -} +import { Dispatch } from 'redux'; interface LogEventAction { type: typeof LOG_EVENT; @@ -46,14 +38,15 @@ interface LogEventAction { describe('logger middleware', () => { const dashboardId = 123; const next: SinonSpy = sinon.spy(); - const mockStore: MockStore = { + // Mock store with minimal state needed for tests + const mockStore = { getState: () => ({ dashboardInfo: { id: dashboardId, }, impressionId: 'impression_id', }), - dispatch: () => {}, + dispatch: ((action: unknown) => action) as Dispatch, }; const action: LogEventAction = { type: LOG_EVENT, @@ -81,18 +74,18 @@ describe('logger middleware', () => { }); test('should listen to LOG_EVENT action type', () => { - const action1: AnyAction = { + const action1 = { type: 'ACTION_TYPE', payload: { some: 'data', }, }; - logger(mockStore)(next)(action1); + (logger as Function)(mockStore)(next)(action1); expect(next.callCount).toBe(1); }); test('should POST an event to /superset/log/ when called', () => { - logger(mockStore)(next)(action); + (logger as Function)(mockStore)(next)(action); expect(next.callCount).toBe(0); timeSandbox.clock.tick(2000); @@ -101,7 +94,7 @@ describe('logger middleware', () => { }); test('should include ts, start_offset, event_name, impression_id, source, and source_id in every event', () => { - const fetchLog = logger(mockStore)(next); + const fetchLog = (logger as Function)(mockStore)(next); fetchLog({ type: LOG_EVENT, payload: { @@ -131,9 +124,9 @@ describe('logger middleware', () => { }); test('should debounce a few log requests to one', () => { - logger(mockStore)(next)(action); - logger(mockStore)(next)(action); - logger(mockStore)(next)(action); + (logger as Function)(mockStore)(next)(action); + (logger as Function)(mockStore)(next)(action); + (logger as Function)(mockStore)(next)(action); timeSandbox.clock.tick(2000); expect(postStub.callCount).toBe(1); @@ -147,7 +140,7 @@ describe('logger middleware', () => { value: beaconMock, }); - logger(mockStore)(next)(action); + (logger as Function)(mockStore)(next)(action); expect(beaconMock.mock.calls.length).toBe(0); timeSandbox.clock.tick(2000); @@ -164,7 +157,7 @@ describe('logger middleware', () => { }); SupersetClient.configure({ guestToken: 'token' }); - logger(mockStore)(next)(action); + (logger as Function)(mockStore)(next)(action); expect(beaconMock.mock.calls.length).toBe(0); timeSandbox.clock.tick(2000); expect(beaconMock.mock.calls.length).toBe(1);