From e7a4dd2618015614caa4cc7f01d594f87fc1a867 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Wed, 2 Feb 2022 18:48:56 +0100 Subject: [PATCH 01/11] feat(explore): Allow using time formatter on temporal columns in data table --- .../src/explore/actions/exploreActions.ts | 26 +++ .../components/DataTableControl/index.tsx | 210 +++++++++++++++-- .../components/DataTablesPane/index.tsx | 217 ++++++++---------- .../src/explore/reducers/exploreReducer.js | 40 ++++ .../src/explore/reducers/getInitialState.ts | 5 + .../src/utils/localStorageHelpers.ts | 2 + 6 files changed, 357 insertions(+), 143 deletions(-) diff --git a/superset-frontend/src/explore/actions/exploreActions.ts b/superset-frontend/src/explore/actions/exploreActions.ts index 2528bc06027b..395df7aa13c0 100644 --- a/superset-frontend/src/explore/actions/exploreActions.ts +++ b/superset-frontend/src/explore/actions/exploreActions.ts @@ -140,6 +140,30 @@ export function sliceUpdated(slice: Slice) { return { type: SLICE_UPDATED, slice }; } +export const SET_TIME_FORMATTED_COLUMN = 'SET_TIME_FORMATTED_COLUMN'; +export function setTimeFormattedColumn( + datasourceId: string, + columnName: string, +) { + return { + type: SET_TIME_FORMATTED_COLUMN, + datasourceId, + columnName, + }; +} + +export const UNSET_TIME_FORMATTED_COLUMN = 'UNSET_TIME_FORMATTED_COLUMN'; +export function unsetTimeFormattedColumn( + datasourceId: string, + columnIndex: number, +) { + return { + type: UNSET_TIME_FORMATTED_COLUMN, + datasourceId, + columnIndex, + }; +} + export const exploreActions = { ...toastActions, setDatasourceType, @@ -155,6 +179,8 @@ export const exploreActions = { updateChartTitle, createNewSlice, sliceUpdated, + setTimeFormattedColumn, + unsetTimeFormattedColumn, }; export type ExploreActions = typeof exploreActions; diff --git a/superset-frontend/src/explore/components/DataTableControl/index.tsx b/superset-frontend/src/explore/components/DataTableControl/index.tsx index a1383cf4a9e1..91f90e271dc9 100644 --- a/superset-frontend/src/explore/components/DataTableControl/index.tsx +++ b/superset-frontend/src/explore/components/DataTableControl/index.tsx @@ -16,20 +16,37 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useMemo } from 'react'; -import { styled, t } from '@superset-ui/core'; +import React, { useCallback, useMemo } from 'react'; +import { + css, + GenericDataType, + getTimeFormatter, + styled, + t, + useTheme, +} from '@superset-ui/core'; +import { Global } from '@emotion/react'; import { Column } from 'react-table'; import debounce from 'lodash/debounce'; -import { Input } from 'src/common/components'; +import { useDispatch, useSelector } from 'react-redux'; +import { Input, Space } from 'src/common/components'; import { BOOL_FALSE_DISPLAY, BOOL_TRUE_DISPLAY, SLOW_DEBOUNCE, } from 'src/constants'; +import { Radio } from 'src/components/Radio'; +import Icons from 'src/components/Icons'; import Button from 'src/components/Button'; +import Popover from 'src/components/Popover'; import { prepareCopyToClipboardTabularData } from 'src/utils/common'; import CopyToClipboard from 'src/components/CopyToClipboard'; import RowCountLabel from 'src/explore/components/RowCountLabel'; +import { ExplorePageState } from 'src/explore/reducers/getInitialState'; +import { + setTimeFormattedColumn, + unsetTimeFormattedColumn, +} from 'src/explore/actions/exploreActions'; export const CopyButton = styled(Button)` font-size: ${({ theme }) => theme.typography.sizes.s}px; @@ -97,6 +114,128 @@ export const RowCount = ({ /> ); +enum FormatPickerValue { + formatted, + epoch, +} + +const FormatPicker = ({ + onChange, + value, +}: { + onChange: any; + value: FormatPickerValue; +}) => ( + + + {t('Epoch')} + {t('Formatted date')} + + +); + +const FormatPickerContainer = styled.div` + display: flex; + flex-direction: column; + + padding: ${({ theme }) => `${theme.gridUnit * 4}px`}; +`; + +const FormatPickerLabel = styled.span` + font-size: ${({ theme }) => theme.typography.sizes.s}px; + color: ${({ theme }) => theme.colors.grayscale.base}; + margin-bottom: ${({ theme }) => theme.gridUnit * 2}px; + text-transform: uppercase; +`; + +const DataTableHeaderCell = ({ + columnName, + type, + datasourceId, + timeFormattedColumnIndex, +}: { + columnName: string; + type?: GenericDataType; + datasourceId?: string; + timeFormattedColumnIndex: number; +}) => { + const theme = useTheme(); + const dispatch = useDispatch(); + const isColumnTimeFormatted = timeFormattedColumnIndex > -1; + + const onChange = useCallback( + e => { + if (!datasourceId) { + return; + } + if (e.target.value === FormatPickerValue.epoch && isColumnTimeFormatted) { + dispatch( + unsetTimeFormattedColumn(datasourceId, timeFormattedColumnIndex), + ); + } else if ( + e.target.value === FormatPickerValue.formatted && + !isColumnTimeFormatted + ) { + dispatch(setTimeFormattedColumn(datasourceId, columnName)); + } + }, + [ + timeFormattedColumnIndex, + columnName, + datasourceId, + dispatch, + isColumnTimeFormatted, + ], + ); + const overlayContent = useMemo( + () => + datasourceId ? ( // eslint-disable-next-line jsx-a11y/no-static-element-interactions + e.stopPropagation()}> + {/* hack to disable click propagation from popover content to table header, which triggers sorting column */} + + {t('Column Formatting')} + + + ) : null, + [datasourceId, isColumnTimeFormatted, onChange], + ); + + return type === GenericDataType.TEMPORAL && datasourceId ? ( + + + e.stopPropagation()} + /> + + {columnName} + + ) : ( + {columnName} + ); +}; + export const useFilteredTableData = ( filterText: string, data?: Record[], @@ -121,34 +260,57 @@ export const useFilteredTableData = ( }, [data, filterText, rowsAsStrings]); }; +const timeFormatter = getTimeFormatter('%Y-%m-%d %H:%M:%S'); + export const useTableColumns = ( colnames?: string[], + coltypes?: GenericDataType[], data?: Record[], + datasourceId?: string, moreConfigs?: { [key: string]: Partial }, -) => - useMemo( +) => { + const timeFormattedColumns = useSelector(state => + datasourceId ? state.explore.timeFormattedColumns[datasourceId] ?? [] : [], + ); + + return useMemo( () => colnames && data?.length ? colnames .filter((column: string) => Object.keys(data[0]).includes(column)) - .map( - key => - ({ - accessor: row => row[key], - // When the key is empty, have to give a string of length greater than 0 - Header: key || ' ', - Cell: ({ value }) => { - if (value === true) { - return BOOL_TRUE_DISPLAY; - } - if (value === false) { - return BOOL_FALSE_DISPLAY; - } - return String(value); - }, - ...moreConfigs?.[key], - } as Column), - ) + .map((key, index) => { + const timeFormattedColumnIndex = + coltypes?.[index] === GenericDataType.TEMPORAL + ? timeFormattedColumns.indexOf(key) + : -1; + return { + id: key, + accessor: row => row[key], + // When the key is empty, have to give a string of length greater than 0 + Header: ( + + ), + Cell: ({ value }) => { + if (value === true) { + return BOOL_TRUE_DISPLAY; + } + if (value === false) { + return BOOL_FALSE_DISPLAY; + } + if (timeFormattedColumnIndex > -1) { + return timeFormatter(value); + } + return String(value); + }, + ...moreConfigs?.[key], + } as Column; + }) : [], - [data, colnames, moreConfigs], + [colnames, data, coltypes, datasourceId, moreConfigs, timeFormattedColumns], ); +}; diff --git a/superset-frontend/src/explore/components/DataTablesPane/index.tsx b/superset-frontend/src/explore/components/DataTablesPane/index.tsx index be0409ffeb92..bf43b972cd3f 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/index.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/index.tsx @@ -17,7 +17,7 @@ * under the License. */ import React, { useCallback, useEffect, useMemo, useState } from 'react'; -import { JsonObject, styled, t } from '@superset-ui/core'; +import { GenericDataType, JsonObject, styled, t } from '@superset-ui/core'; import Collapse from 'src/components/Collapse'; import Tabs from 'src/components/Tabs'; import Loading from 'src/components/Loading'; @@ -43,10 +43,10 @@ const RESULT_TYPES = { samples: 'samples' as const, }; -const NULLISH_RESULTS_STATE = { - [RESULT_TYPES.results]: undefined, - [RESULT_TYPES.samples]: undefined, -}; +const getDefaultDataTablesState = (value: any) => ({ + [RESULT_TYPES.results]: value, + [RESULT_TYPES.samples]: value, +}); const DATA_TABLE_PAGE_SIZE = 50; @@ -105,6 +105,8 @@ const Error = styled.pre` interface DataTableProps { columnNames: string[]; + columnTypes: GenericDataType[] | undefined; + datasource: string | undefined; filterText: string; data: object[] | undefined; isLoading: boolean; @@ -114,6 +116,8 @@ interface DataTableProps { const DataTable = ({ columnNames, + columnTypes, + datasource, filterText, data, isLoading, @@ -122,7 +126,7 @@ const DataTable = ({ }: DataTableProps) => { // this is to preserve the order of the columns, even if there are integer values, // while also only grabbing the first column's keys - const columns = useTableColumns(columnNames, data); + const columns = useTableColumns(columnNames, columnTypes, data, datasource); const filteredData = useFilteredTableData(filterText, data); if (isLoading) { @@ -172,22 +176,11 @@ export const DataTablesPane = ({ errorMessage?: JSX.Element; queriesResponse: Record; }) => { - const [data, setData] = useState<{ - [RESULT_TYPES.results]?: Record[]; - [RESULT_TYPES.samples]?: Record[]; - }>(NULLISH_RESULTS_STATE); - const [isLoading, setIsLoading] = useState({ - [RESULT_TYPES.results]: true, - [RESULT_TYPES.samples]: true, - }); - const [columnNames, setColumnNames] = useState<{ - [RESULT_TYPES.results]: string[]; - [RESULT_TYPES.samples]: string[]; - }>({ - [RESULT_TYPES.results]: [], - [RESULT_TYPES.samples]: [], - }); - const [error, setError] = useState(NULLISH_RESULTS_STATE); + const [data, setData] = useState(getDefaultDataTablesState([])); + const [isLoading, setIsLoading] = useState(getDefaultDataTablesState(true)); + const [columnNames, setColumnNames] = useState(getDefaultDataTablesState([])); + const [columnTypes, setColumnTypes] = useState(getDefaultDataTablesState([])); + const [error, setError] = useState(getDefaultDataTablesState('')); const [filterText, setFilterText] = useState(''); const [activeTabKey, setActiveTabKey] = useState( RESULT_TYPES.results, @@ -195,7 +188,7 @@ export const DataTablesPane = ({ const [isRequestPending, setIsRequestPending] = useState<{ [RESULT_TYPES.results]?: boolean; [RESULT_TYPES.samples]?: boolean; - }>(NULLISH_RESULTS_STATE); + }>(getDefaultDataTablesState(false)); const [panelOpen, setPanelOpen] = useState( getItem(LocalStorageKeys.is_datapanel_open, false), ); @@ -212,127 +205,109 @@ export const DataTablesPane = ({ [data], ); - const getData = useCallback( - (resultType: string) => { - setIsLoading(prevIsLoading => ({ - ...prevIsLoading, - [resultType]: true, - })); - return getChartDataRequest({ - formData: queryFormData, - resultFormat: 'json', - resultType, - ownState, - }) - .then(({ json }) => { - // Only displaying the first query is currently supported - if (json.result.length > 1) { - const data: any[] = []; - json.result.forEach((item: { data: any[] }) => { - item.data.forEach((row, i) => { - if (data[i] !== undefined) { - data[i] = { ...data[i], ...row }; - } else { - data[i] = row; - } - }); + const getSamplesData = useCallback(() => { + setIsLoading(prevIsLoading => ({ + ...prevIsLoading, + [RESULT_TYPES.samples]: true, + })); + return getChartDataRequest({ + formData: queryFormData, + resultFormat: 'json', + resultType: 'samples', + ownState, + }) + .then(({ json }) => { + // Only displaying the first query is currently supported + if (json.result.length > 1) { + const data: any[] = []; + json.result.forEach((item: { data: any[] }) => { + item.data.forEach((row, i) => { + if (data[i] !== undefined) { + data[i] = { ...data[i], ...row }; + } else { + data[i] = row; + } }); - setData(prevData => ({ - ...prevData, - [resultType]: data, - })); - } else { - setData(prevData => ({ - ...prevData, - [resultType]: json.result[0].data, - })); - } - const checkCols = json?.result[0]?.data?.length - ? Object.keys(json.result[0].data[0]) - : null; - setColumnNames(prevColumnNames => ({ - ...prevColumnNames, - [resultType]: json.result[0].columns || checkCols, + }); + setData(prevData => ({ + ...prevData, + [RESULT_TYPES.samples]: data, })); - setIsLoading(prevIsLoading => ({ - ...prevIsLoading, - [resultType]: false, + } else { + setData(prevData => ({ + ...prevData, + [RESULT_TYPES.samples]: json.result[0].data, })); + } + const checkCols = json?.result[0]?.data?.length + ? Object.keys(json.result[0].data[0]) + : null; + setColumnNames(prevColumnNames => ({ + ...prevColumnNames, + [RESULT_TYPES.samples]: json.result[0].columns || checkCols, + })); + setColumnTypes(prevColumnTypes => ({ + ...prevColumnTypes, + [RESULT_TYPES.samples]: json.result[0].coltypes || [], + })); + setIsLoading(prevIsLoading => ({ + ...prevIsLoading, + [RESULT_TYPES.samples]: false, + })); + setError(prevError => ({ + ...prevError, + [RESULT_TYPES.samples]: undefined, + })); + }) + .catch(response => { + getClientErrorObject(response).then(({ error, message }) => { setError(prevError => ({ ...prevError, - [resultType]: null, + [RESULT_TYPES.samples]: + error || message || t('Sorry, An error occurred'), + })); + setIsLoading(prevIsLoading => ({ + ...prevIsLoading, + [RESULT_TYPES.samples]: false, })); - }) - .catch(response => { - getClientErrorObject(response).then(({ error, message }) => { - setError(prevError => ({ - ...prevError, - [resultType]: error || message || t('Sorry, An error occurred'), - })); - setIsLoading(prevIsLoading => ({ - ...prevIsLoading, - [resultType]: false, - })); - }); }); - }, - [queryFormData, columnNames], - ); + }); + }, [queryFormData, columnNames]); useEffect(() => { setItem(LocalStorageKeys.is_datapanel_open, panelOpen); }, [panelOpen]); - useEffect(() => { - setIsRequestPending(prevState => ({ - ...prevState, - [RESULT_TYPES.results]: true, - })); - }, [queryFormData]); - useEffect(() => { setIsRequestPending(prevState => ({ ...prevState, [RESULT_TYPES.samples]: true, })); - }, [queryFormData?.adhoc_filters, queryFormData?.datasource]); + }, [queryFormData?.datasource]); useEffect(() => { if (queriesResponse && chartStatus === 'success') { - const { colnames } = queriesResponse[0]; + const { colnames, coltypes, data } = queriesResponse[0]; setColumnNames(prevColumnNames => ({ ...prevColumnNames, - [RESULT_TYPES.results]: colnames ? [...colnames] : [], + [RESULT_TYPES.results]: colnames ?? [], + })); + setColumnTypes(prevColumnTypes => ({ + ...prevColumnTypes, + [RESULT_TYPES.results]: coltypes ?? [], + })); + setData(prevData => ({ + ...prevData, + [RESULT_TYPES.results]: data, + })); + setIsLoading(prevIsLoading => ({ + ...prevIsLoading, + [RESULT_TYPES.results]: false, })); } }, [queriesResponse, chartStatus]); useEffect(() => { - if (panelOpen && isRequestPending[RESULT_TYPES.results]) { - if (errorMessage) { - setIsRequestPending(prevState => ({ - ...prevState, - [RESULT_TYPES.results]: false, - })); - setIsLoading(prevIsLoading => ({ - ...prevIsLoading, - [RESULT_TYPES.results]: false, - })); - return; - } - if (chartStatus === 'loading') { - setIsLoading(prevIsLoading => ({ - ...prevIsLoading, - [RESULT_TYPES.results]: true, - })); - } else { - setIsRequestPending(prevState => ({ - ...prevState, - [RESULT_TYPES.results]: false, - })); - getData(RESULT_TYPES.results); - } - } if ( panelOpen && isRequestPending[RESULT_TYPES.samples] && @@ -342,12 +317,12 @@ export const DataTablesPane = ({ ...prevState, [RESULT_TYPES.samples]: false, })); - getData(RESULT_TYPES.samples); + getSamplesData(); } }, [ panelOpen, isRequestPending, - getData, + getSamplesData, activeTabKey, chartStatus, errorMessage, @@ -396,7 +371,9 @@ export const DataTablesPane = ({ ; }; export function getItem( From f1977806ee98d3f49df928a84cbca85722420491 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Wed, 2 Feb 2022 19:28:20 +0100 Subject: [PATCH 02/11] Fix data table loading --- .../components/DataTablesPane/index.tsx | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/explore/components/DataTablesPane/index.tsx b/superset-frontend/src/explore/components/DataTablesPane/index.tsx index bf43b972cd3f..934b84ed853c 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/index.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/index.tsx @@ -176,7 +176,7 @@ export const DataTablesPane = ({ errorMessage?: JSX.Element; queriesResponse: Record; }) => { - const [data, setData] = useState(getDefaultDataTablesState([])); + const [data, setData] = useState(getDefaultDataTablesState(undefined)); const [isLoading, setIsLoading] = useState(getDefaultDataTablesState(true)); const [columnNames, setColumnNames] = useState(getDefaultDataTablesState([])); const [columnTypes, setColumnTypes] = useState(getDefaultDataTablesState([])); @@ -264,7 +264,7 @@ export const DataTablesPane = ({ setError(prevError => ({ ...prevError, [RESULT_TYPES.samples]: - error || message || t('Sorry, An error occurred'), + error || message || t('Sorry, an error occurred'), })); setIsLoading(prevIsLoading => ({ ...prevIsLoading, @@ -286,6 +286,17 @@ export const DataTablesPane = ({ }, [queryFormData?.datasource]); useEffect(() => { + if (chartStatus === 'loading' && !isLoading[RESULT_TYPES.results]) { + setIsLoading(prevIsLoading => ({ + ...prevIsLoading, + [RESULT_TYPES.results]: true, + })); + } else if (chartStatus !== 'loading' && isLoading[RESULT_TYPES.results]) { + setIsLoading(prevIsLoading => ({ + ...prevIsLoading, + [RESULT_TYPES.results]: false, + })); + } if (queriesResponse && chartStatus === 'success') { const { colnames, coltypes, data } = queriesResponse[0]; setColumnNames(prevColumnNames => ({ @@ -300,9 +311,17 @@ export const DataTablesPane = ({ ...prevData, [RESULT_TYPES.results]: data, })); - setIsLoading(prevIsLoading => ({ - ...prevIsLoading, - [RESULT_TYPES.results]: false, + } + if (queriesResponse && chartStatus === 'failed') { + const { error, message } = queriesResponse[0]; + setData(prevData => ({ + ...prevData, + [RESULT_TYPES.results]: undefined, + })); + setError(prevError => ({ + ...prevError, + [RESULT_TYPES.results]: + error || message || t('Sorry, an error occured'), })); } }, [queriesResponse, chartStatus]); @@ -375,7 +394,10 @@ export const DataTablesPane = ({ columnNames={columnNames[RESULT_TYPES.results]} columnTypes={columnTypes[RESULT_TYPES.results]} filterText={filterText} - error={error[RESULT_TYPES.results]} + error={ + queriesResponse?.[0]?.error || + queriesResponse?.[0]?.message + } errorMessage={errorMessage} /> From 74976db8805fce7a0c2888033af410762e6e857a Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 3 Feb 2022 13:52:16 +0100 Subject: [PATCH 03/11] Return colnames and coltypes from results request --- .../components/DataTablesPane/index.tsx | 224 ++++++++++-------- superset/common/query_actions.py | 8 +- superset/views/core.py | 6 +- superset/viz.py | 10 +- 4 files changed, 141 insertions(+), 107 deletions(-) diff --git a/superset-frontend/src/explore/components/DataTablesPane/index.tsx b/superset-frontend/src/explore/components/DataTablesPane/index.tsx index 934b84ed853c..0976bcf8be0c 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/index.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/index.tsx @@ -17,7 +17,14 @@ * under the License. */ import React, { useCallback, useEffect, useMemo, useState } from 'react'; -import { GenericDataType, JsonObject, styled, t } from '@superset-ui/core'; +import { + ensureIsArray, + GenericDataType, + JsonObject, + styled, + t, +} from '@superset-ui/core'; +import { ColumnMeta } from '@superset-ui/chart-controls'; import Collapse from 'src/components/Collapse'; import Tabs from 'src/components/Tabs'; import Loading from 'src/components/Loading'; @@ -37,6 +44,8 @@ import { useTableColumns, } from 'src/explore/components/DataTableControl'; import { applyFormattingToTabularData } from 'src/utils/common'; +import { useSelector } from 'react-redux'; +import { ExplorePageState } from '../../reducers/getInitialState'; const RESULT_TYPES = { results: 'results' as const, @@ -192,6 +201,9 @@ export const DataTablesPane = ({ const [panelOpen, setPanelOpen] = useState( getItem(LocalStorageKeys.is_datapanel_open, false), ); + const datasourceColumns = useSelector( + state => state.explore.datasource?.columns, + ); const formattedData = useMemo( () => ({ @@ -205,79 +217,100 @@ export const DataTablesPane = ({ [data], ); - const getSamplesData = useCallback(() => { - setIsLoading(prevIsLoading => ({ - ...prevIsLoading, - [RESULT_TYPES.samples]: true, - })); - return getChartDataRequest({ - formData: queryFormData, - resultFormat: 'json', - resultType: 'samples', - ownState, - }) - .then(({ json }) => { - // Only displaying the first query is currently supported - if (json.result.length > 1) { - const data: any[] = []; - json.result.forEach((item: { data: any[] }) => { - item.data.forEach((row, i) => { - if (data[i] !== undefined) { - data[i] = { ...data[i], ...row }; - } else { - data[i] = row; - } + const getData = useCallback( + (resultType: 'samples' | 'results') => { + setIsLoading(prevIsLoading => ({ + ...prevIsLoading, + [resultType]: true, + })); + return getChartDataRequest({ + formData: queryFormData, + resultFormat: 'json', + resultType, + ownState, + }) + .then(({ json }) => { + // Only displaying the first query is currently supported + if (json.result.length > 1) { + const data: any[] = []; + json.result.forEach((item: { data: any[] }) => { + item.data.forEach((row, i) => { + if (data[i] !== undefined) { + data[i] = { ...data[i], ...row }; + } else { + data[i] = row; + } + }); }); - }); - setData(prevData => ({ - ...prevData, - [RESULT_TYPES.samples]: data, - })); - } else { - setData(prevData => ({ - ...prevData, - [RESULT_TYPES.samples]: json.result[0].data, + setData(prevData => ({ + ...prevData, + [resultType]: data, + })); + } else { + setData(prevData => ({ + ...prevData, + [resultType]: json.result[0].data, + })); + } + + const colNames = ensureIsArray(json.result[0].colnames); + + const getColTypesFromDatasourceColumns = () => + colNames + .map( + name => + datasourceColumns.find(column => column.column_name === name) + ?.type_generic, + ) + .filter(type => type !== undefined); + + setColumnNames(prevColumnNames => ({ + ...prevColumnNames, + [resultType]: colNames, })); - } - const checkCols = json?.result[0]?.data?.length - ? Object.keys(json.result[0].data[0]) - : null; - setColumnNames(prevColumnNames => ({ - ...prevColumnNames, - [RESULT_TYPES.samples]: json.result[0].columns || checkCols, - })); - setColumnTypes(prevColumnTypes => ({ - ...prevColumnTypes, - [RESULT_TYPES.samples]: json.result[0].coltypes || [], - })); - setIsLoading(prevIsLoading => ({ - ...prevIsLoading, - [RESULT_TYPES.samples]: false, - })); - setError(prevError => ({ - ...prevError, - [RESULT_TYPES.samples]: undefined, - })); - }) - .catch(response => { - getClientErrorObject(response).then(({ error, message }) => { - setError(prevError => ({ - ...prevError, - [RESULT_TYPES.samples]: - error || message || t('Sorry, an error occurred'), + setColumnTypes(prevColumnTypes => ({ + ...prevColumnTypes, + [resultType]: + json.result[0].coltypes || + getColTypesFromDatasourceColumns() || + [], })); setIsLoading(prevIsLoading => ({ ...prevIsLoading, - [RESULT_TYPES.samples]: false, + [resultType]: false, })); + setError(prevError => ({ + ...prevError, + [resultType]: undefined, + })); + }) + .catch(response => { + getClientErrorObject(response).then(({ error, message }) => { + setError(prevError => ({ + ...prevError, + [resultType]: error || message || t('Sorry, an error occurred'), + })); + setIsLoading(prevIsLoading => ({ + ...prevIsLoading, + [resultType]: false, + })); + }); }); - }); - }, [queryFormData, columnNames]); + }, + [queryFormData, columnNames], + ); useEffect(() => { setItem(LocalStorageKeys.is_datapanel_open, panelOpen); }, [panelOpen]); + useEffect(() => { + setIsRequestPending(prevState => ({ + ...prevState, + [RESULT_TYPES.results]: true, + })); + }, [queryFormData]); + useEffect(() => { setIsRequestPending(prevState => ({ ...prevState, @@ -286,47 +319,41 @@ export const DataTablesPane = ({ }, [queryFormData?.datasource]); useEffect(() => { - if (chartStatus === 'loading' && !isLoading[RESULT_TYPES.results]) { - setIsLoading(prevIsLoading => ({ - ...prevIsLoading, - [RESULT_TYPES.results]: true, - })); - } else if (chartStatus !== 'loading' && isLoading[RESULT_TYPES.results]) { - setIsLoading(prevIsLoading => ({ - ...prevIsLoading, - [RESULT_TYPES.results]: false, - })); - } if (queriesResponse && chartStatus === 'success') { - const { colnames, coltypes, data } = queriesResponse[0]; + const { colnames } = queriesResponse[0]; setColumnNames(prevColumnNames => ({ ...prevColumnNames, [RESULT_TYPES.results]: colnames ?? [], })); - setColumnTypes(prevColumnTypes => ({ - ...prevColumnTypes, - [RESULT_TYPES.results]: coltypes ?? [], - })); - setData(prevData => ({ - ...prevData, - [RESULT_TYPES.results]: data, - })); - } - if (queriesResponse && chartStatus === 'failed') { - const { error, message } = queriesResponse[0]; - setData(prevData => ({ - ...prevData, - [RESULT_TYPES.results]: undefined, - })); - setError(prevError => ({ - ...prevError, - [RESULT_TYPES.results]: - error || message || t('Sorry, an error occured'), - })); } }, [queriesResponse, chartStatus]); useEffect(() => { + if (panelOpen && isRequestPending[RESULT_TYPES.results]) { + if (errorMessage) { + setIsRequestPending(prevState => ({ + ...prevState, + [RESULT_TYPES.results]: false, + })); + setIsLoading(prevIsLoading => ({ + ...prevIsLoading, + [RESULT_TYPES.results]: false, + })); + return; + } + if (chartStatus === 'loading') { + setIsLoading(prevIsLoading => ({ + ...prevIsLoading, + [RESULT_TYPES.results]: true, + })); + } else { + setIsRequestPending(prevState => ({ + ...prevState, + [RESULT_TYPES.results]: false, + })); + getData(RESULT_TYPES.results); + } + } if ( panelOpen && isRequestPending[RESULT_TYPES.samples] && @@ -336,12 +363,12 @@ export const DataTablesPane = ({ ...prevState, [RESULT_TYPES.samples]: false, })); - getSamplesData(); + getData(RESULT_TYPES.samples); } }, [ panelOpen, isRequestPending, - getSamplesData, + getData, activeTabKey, chartStatus, errorMessage, @@ -394,10 +421,7 @@ export const DataTablesPane = ({ columnNames={columnNames[RESULT_TYPES.results]} columnTypes={columnTypes[RESULT_TYPES.results]} filterText={filterText} - error={ - queriesResponse?.[0]?.error || - queriesResponse?.[0]?.message - } + error={error[RESULT_TYPES.results]} errorMessage={errorMessage} /> diff --git a/superset/common/query_actions.py b/superset/common/query_actions.py index 6ed18d195820..a09cc39f1d6c 100644 --- a/superset/common/query_actions.py +++ b/superset/common/query_actions.py @@ -129,7 +129,11 @@ def _get_full( ] + rejected_time_columns if result_type == ChartDataResultType.RESULTS and status != QueryStatus.FAILED: - return {"data": payload.get("data")} + return { + "data": payload.get("data"), + "colnames": payload.get("colnames"), + "coltypes": payload.get("coltypes"), + } return payload @@ -152,7 +156,7 @@ def _get_results( query_context: "QueryContext", query_obj: "QueryObject", force_cached: bool = False ) -> Dict[str, Any]: payload = _get_full(query_context, query_obj, force_cached) - return {"data": payload.get("data"), "error": payload.get("error")} + return payload _result_type_functions: Dict[ diff --git a/superset/views/core.py b/superset/views/core.py index f2368e344158..08f72a85d5c6 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -449,10 +449,12 @@ def get_raw_results(self, viz_obj: BaseViz) -> FlaskResponse: payload = viz_obj.get_df_payload() if viz_obj.has_error(payload): return json_error_response(payload=payload, status=400) - return self.json_response({"data": payload["df"].to_dict("records")}) + return self.json_response( + {"data": payload["df"].to_dict("records"), "colnames": payload["colnames"]} + ) def get_samples(self, viz_obj: BaseViz) -> FlaskResponse: - return self.json_response({"data": viz_obj.get_samples()}) + return self.json_response(viz_obj.get_samples()) @staticmethod def send_data_payload_response(viz_obj: BaseViz, payload: Any) -> FlaskResponse: diff --git a/superset/viz.py b/superset/viz.py index c1b506e25675..26868f169738 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -244,7 +244,7 @@ def apply_rolling(self, df: pd.DataFrame) -> pd.DataFrame: ) return df - def get_samples(self) -> List[Dict[str, Any]]: + def get_samples(self) -> Dict[str, Any]: query_obj = self.query_obj() query_obj.update( { @@ -258,8 +258,11 @@ def get_samples(self) -> List[Dict[str, Any]]: "to_dttm": None, } ) - df = self.get_df_payload(query_obj)["df"] # leverage caching logic - return df.to_dict(orient="records") + payload = self.get_df_payload(query_obj) # leverage caching logic + return { + "data": payload["df"].to_dict(orient="records"), + "colnames": payload["colnames"], + } def get_df(self, query_obj: Optional[QueryObjectDict] = None) -> pd.DataFrame: """Returns a pandas dataframe based on the query object""" @@ -621,6 +624,7 @@ def get_df_payload( # pylint: disable=too-many-statements "status": self.status, "stacktrace": stacktrace, "rowcount": len(df.index) if df is not None else 0, + "colnames": list(df.columns) if df is not None else None, } @staticmethod From f692a187ee2cfda581df00458487f1570d6fa709 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 3 Feb 2022 15:03:12 +0100 Subject: [PATCH 04/11] Fix types --- .../DataTableControl/useTableColumns.test.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts b/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts index 537f12bc0cb4..b42ccd308d60 100644 --- a/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts +++ b/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import { GenericDataType } from '@superset-ui/core'; import { renderHook } from '@testing-library/react-hooks'; import { BOOL_FALSE_DISPLAY, BOOL_TRUE_DISPLAY } from 'src/constants'; import { useTableColumns } from '.'; @@ -43,9 +44,15 @@ const data = [ }, ]; const all_columns = ['col01', 'col02', 'col03', asciiKey, unicodeKey]; +const coltypes = [ + GenericDataType.BOOLEAN, + GenericDataType.BOOLEAN, + GenericDataType.STRING, + GenericDataType.STRING, +]; test('useTableColumns with no options', () => { - const hook = renderHook(() => useTableColumns(all_columns, data)); + const hook = renderHook(() => useTableColumns(all_columns, coltypes, data)); expect(hook.result.current).toEqual([ { Cell: expect.any(Function), @@ -84,7 +91,9 @@ test('useTableColumns with no options', () => { test('use only the first record columns', () => { const newData = [data[3], data[0]]; - const hook = renderHook(() => useTableColumns(all_columns, newData)); + const hook = renderHook(() => + useTableColumns(all_columns, coltypes, newData), + ); expect(hook.result.current).toEqual([ { Cell: expect.any(Function), @@ -136,7 +145,9 @@ test('use only the first record columns', () => { test('useTableColumns with options', () => { const hook = renderHook(() => - useTableColumns(all_columns, data, { col01: { id: 'ID' } }), + useTableColumns(all_columns, coltypes, data, undefined, { + col01: { id: 'ID' }, + }), ); expect(hook.result.current).toEqual([ { From 410adc368c594ec0514e5952f4469c9f6b7d878c Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 3 Feb 2022 15:45:42 +0100 Subject: [PATCH 05/11] Fix tests --- .../spec/helpers/testing-library.tsx | 6 +++- .../components/DataTableControl/index.tsx | 28 ++++++++-------- .../DataTableControl/useTableColumns.test.ts | 33 ++++++++++++++----- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index 56489cce8471..5afc8539d093 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -19,6 +19,7 @@ import '@testing-library/jest-dom/extend-expect'; import React, { ReactNode, ReactElement } from 'react'; import { render, RenderOptions } from '@testing-library/react'; +import { renderHook } from '@testing-library/react-hooks'; import { ThemeProvider, supersetTheme } from '@superset-ui/core'; import { BrowserRouter } from 'react-router-dom'; import { Provider } from 'react-redux'; @@ -82,6 +83,9 @@ function createWrapper(options?: Options) { const customRender = (ui: ReactElement, options?: Options) => render(ui, { wrapper: createWrapper(options), ...options }); +const customRenderHook = (callback: (props: any) => any, options?: Options) => + renderHook(callback, { wrapper: createWrapper(options), ...options }); + export function sleep(time: number) { return new Promise(resolve => { setTimeout(resolve, time); @@ -89,4 +93,4 @@ export function sleep(time: number) { } export * from '@testing-library/react'; -export { customRender as render }; +export { customRender as render, customRenderHook as renderHook }; diff --git a/superset-frontend/src/explore/components/DataTableControl/index.tsx b/superset-frontend/src/explore/components/DataTableControl/index.tsx index 91f90e271dc9..a3b1d82c9910 100644 --- a/superset-frontend/src/explore/components/DataTableControl/index.tsx +++ b/superset-frontend/src/explore/components/DataTableControl/index.tsx @@ -148,14 +148,12 @@ const FormatPickerLabel = styled.span` text-transform: uppercase; `; -const DataTableHeaderCell = ({ +const DataTableTemporalHeaderCell = ({ columnName, - type, datasourceId, timeFormattedColumnIndex, }: { columnName: string; - type?: GenericDataType; datasourceId?: string; timeFormattedColumnIndex: number; }) => { @@ -213,7 +211,7 @@ const DataTableHeaderCell = ({ [datasourceId, isColumnTimeFormatted, onChange], ); - return type === GenericDataType.TEMPORAL && datasourceId ? ( + return datasourceId ? ( }, ) => { const timeFormattedColumns = useSelector(state => - datasourceId ? state.explore.timeFormattedColumns[datasourceId] ?? [] : [], + datasourceId + ? state.explore.timeFormattedColumns?.[datasourceId] ?? [] + : [], ); return useMemo( @@ -287,14 +287,16 @@ export const useTableColumns = ( id: key, accessor: row => row[key], // When the key is empty, have to give a string of length greater than 0 - Header: ( - - ), + Header: + coltypes?.[index] === GenericDataType.TEMPORAL ? ( + + ) : ( + key + ), Cell: ({ value }) => { if (value === true) { return BOOL_TRUE_DISPLAY; diff --git a/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts b/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts index b42ccd308d60..24bceb78d15a 100644 --- a/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts +++ b/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts @@ -17,7 +17,7 @@ * under the License. */ import { GenericDataType } from '@superset-ui/core'; -import { renderHook } from '@testing-library/react-hooks'; +import { renderHook } from 'spec/helpers/testing-library'; import { BOOL_FALSE_DISPLAY, BOOL_TRUE_DISPLAY } from 'src/constants'; import { useTableColumns } from '.'; @@ -52,27 +52,33 @@ const coltypes = [ ]; test('useTableColumns with no options', () => { - const hook = renderHook(() => useTableColumns(all_columns, coltypes, data)); + const hook = renderHook(() => useTableColumns(all_columns, coltypes, data), { + useRedux: true, + }); expect(hook.result.current).toEqual([ { Cell: expect.any(Function), Header: 'col01', accessor: expect.any(Function), + id: 'col01', }, { Cell: expect.any(Function), Header: 'col02', accessor: expect.any(Function), + id: 'col02', }, { Cell: expect.any(Function), Header: asciiKey, accessor: expect.any(Function), + id: asciiKey, }, { Cell: expect.any(Function), Header: unicodeKey, accessor: expect.any(Function), + id: unicodeKey, }, ]); hook.result.current.forEach((col: JsonObject) => { @@ -91,34 +97,40 @@ test('useTableColumns with no options', () => { test('use only the first record columns', () => { const newData = [data[3], data[0]]; - const hook = renderHook(() => - useTableColumns(all_columns, coltypes, newData), + const hook = renderHook( + () => useTableColumns(all_columns, coltypes, newData), + { useRedux: true }, ); expect(hook.result.current).toEqual([ { Cell: expect.any(Function), Header: 'col01', accessor: expect.any(Function), + id: 'col01', }, { Cell: expect.any(Function), Header: 'col02', accessor: expect.any(Function), + id: 'col02', }, { Cell: expect.any(Function), Header: 'col03', accessor: expect.any(Function), + id: 'col03', }, { Cell: expect.any(Function), Header: asciiKey, accessor: expect.any(Function), + id: asciiKey, }, { Cell: expect.any(Function), Header: unicodeKey, accessor: expect.any(Function), + id: unicodeKey, }, ]); @@ -144,10 +156,12 @@ test('use only the first record columns', () => { }); test('useTableColumns with options', () => { - const hook = renderHook(() => - useTableColumns(all_columns, coltypes, data, undefined, { - col01: { id: 'ID' }, - }), + const hook = renderHook( + () => + useTableColumns(all_columns, coltypes, data, undefined, { + col01: { id: 'ID' }, + }), + { useRedux: true }, ); expect(hook.result.current).toEqual([ { @@ -160,16 +174,19 @@ test('useTableColumns with options', () => { Cell: expect.any(Function), Header: 'col02', accessor: expect.any(Function), + id: 'col02', }, { Cell: expect.any(Function), Header: asciiKey, accessor: expect.any(Function), + id: asciiKey, }, { Cell: expect.any(Function), Header: unicodeKey, accessor: expect.any(Function), + id: unicodeKey, }, ]); hook.result.current.forEach((col: JsonObject) => { From 9a3a26a8f29fc4fc7343d041e7ab7d4f660f04d6 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 4 Feb 2022 16:33:21 +0100 Subject: [PATCH 06/11] Fix copy button --- .../components/DataTableControl/index.tsx | 29 ++++---- .../DataTablesPane/DataTablesPane.test.tsx | 9 ++- .../components/DataTablesPane/index.tsx | 34 +++------- .../components/useTimeFormattedColumns.ts | 27 ++++++++ superset-frontend/src/utils/common.js | 21 ++++-- superset-frontend/src/utils/common.test.jsx | 67 +++++++++++++++---- superset/views/core.py | 6 +- superset/viz.py | 2 + 8 files changed, 135 insertions(+), 60 deletions(-) create mode 100644 superset-frontend/src/explore/components/useTimeFormattedColumns.ts diff --git a/superset-frontend/src/explore/components/DataTableControl/index.tsx b/superset-frontend/src/explore/components/DataTableControl/index.tsx index a3b1d82c9910..95973b14193b 100644 --- a/superset-frontend/src/explore/components/DataTableControl/index.tsx +++ b/superset-frontend/src/explore/components/DataTableControl/index.tsx @@ -23,12 +23,13 @@ import { getTimeFormatter, styled, t, + TimeFormats, useTheme, } from '@superset-ui/core'; import { Global } from '@emotion/react'; import { Column } from 'react-table'; import debounce from 'lodash/debounce'; -import { useDispatch, useSelector } from 'react-redux'; +import { useDispatch } from 'react-redux'; import { Input, Space } from 'src/common/components'; import { BOOL_FALSE_DISPLAY, @@ -42,11 +43,11 @@ import Popover from 'src/components/Popover'; import { prepareCopyToClipboardTabularData } from 'src/utils/common'; import CopyToClipboard from 'src/components/CopyToClipboard'; import RowCountLabel from 'src/explore/components/RowCountLabel'; -import { ExplorePageState } from 'src/explore/reducers/getInitialState'; import { setTimeFormattedColumn, unsetTimeFormattedColumn, } from 'src/explore/actions/exploreActions'; +import { useTimeFormattedColumns } from '../useTimeFormattedColumns'; export const CopyButton = styled(Button)` font-size: ${({ theme }) => theme.typography.sizes.s}px; @@ -115,8 +116,8 @@ export const RowCount = ({ ); enum FormatPickerValue { - formatted, - epoch, + Formatted, + Epoch, } const FormatPicker = ({ @@ -128,8 +129,8 @@ const FormatPicker = ({ }) => ( - {t('Epoch')} - {t('Formatted date')} + {t('Original value')} + {t('Formatted date')} ); @@ -166,12 +167,12 @@ const DataTableTemporalHeaderCell = ({ if (!datasourceId) { return; } - if (e.target.value === FormatPickerValue.epoch && isColumnTimeFormatted) { + if (e.target.value === FormatPickerValue.Epoch && isColumnTimeFormatted) { dispatch( unsetTimeFormattedColumn(datasourceId, timeFormattedColumnIndex), ); } else if ( - e.target.value === FormatPickerValue.formatted && + e.target.value === FormatPickerValue.Formatted && !isColumnTimeFormatted ) { dispatch(setTimeFormattedColumn(datasourceId, columnName)); @@ -202,8 +203,8 @@ const DataTableTemporalHeaderCell = ({ onChange={onChange} value={ isColumnTimeFormatted - ? FormatPickerValue.formatted - : FormatPickerValue.epoch + ? FormatPickerValue.Formatted + : FormatPickerValue.Epoch } /> @@ -258,7 +259,7 @@ export const useFilteredTableData = ( }, [data, filterText, rowsAsStrings]); }; -const timeFormatter = getTimeFormatter('%Y-%m-%d %H:%M:%S'); +const timeFormatter = getTimeFormatter(TimeFormats.DATABASE_DATETIME); export const useTableColumns = ( colnames?: string[], @@ -267,11 +268,7 @@ export const useTableColumns = ( datasourceId?: string, moreConfigs?: { [key: string]: Partial }, ) => { - const timeFormattedColumns = useSelector(state => - datasourceId - ? state.explore.timeFormattedColumns?.[datasourceId] ?? [] - : [], - ); + const timeFormattedColumns = useTimeFormattedColumns(datasourceId); return useMemo( () => diff --git a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx index 42f996ae0a0b..1a7c6a1c3fd9 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx @@ -105,7 +105,13 @@ test('Should copy data table content correctly', async () => { fetchMock.post( 'glob:*/api/v1/chart/data?form_data=%7B%22slice_id%22%3A456%7D', { - result: [{ data: [{ __timestamp: 1230768000000, genre: 'Action' }] }], + result: [ + { + data: [{ __timestamp: 1230768000000, genre: 'Action' }], + colnames: ['__timestamp', 'genre'], + coltypes: [2, 1], + }, + ], }, ); const copyToClipboardSpy = jest.spyOn(copyUtils, 'default'); @@ -118,6 +124,7 @@ test('Should copy data table content correctly', async () => { queriesResponse: [ { colnames: ['__timestamp', 'genre'], + coltypes: [2, 1], }, ], }} diff --git a/superset-frontend/src/explore/components/DataTablesPane/index.tsx b/superset-frontend/src/explore/components/DataTablesPane/index.tsx index 0976bcf8be0c..b81871a3b38a 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/index.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/index.tsx @@ -24,7 +24,6 @@ import { styled, t, } from '@superset-ui/core'; -import { ColumnMeta } from '@superset-ui/chart-controls'; import Collapse from 'src/components/Collapse'; import Tabs from 'src/components/Tabs'; import Loading from 'src/components/Loading'; @@ -44,8 +43,7 @@ import { useTableColumns, } from 'src/explore/components/DataTableControl'; import { applyFormattingToTabularData } from 'src/utils/common'; -import { useSelector } from 'react-redux'; -import { ExplorePageState } from '../../reducers/getInitialState'; +import { useTimeFormattedColumns } from '../useTimeFormattedColumns'; const RESULT_TYPES = { results: 'results' as const, @@ -194,27 +192,29 @@ export const DataTablesPane = ({ const [activeTabKey, setActiveTabKey] = useState( RESULT_TYPES.results, ); - const [isRequestPending, setIsRequestPending] = useState<{ - [RESULT_TYPES.results]?: boolean; - [RESULT_TYPES.samples]?: boolean; - }>(getDefaultDataTablesState(false)); + const [isRequestPending, setIsRequestPending] = useState( + getDefaultDataTablesState(false), + ); const [panelOpen, setPanelOpen] = useState( getItem(LocalStorageKeys.is_datapanel_open, false), ); - const datasourceColumns = useSelector( - state => state.explore.datasource?.columns, + + const timeFormattedColumns = useTimeFormattedColumns( + queryFormData?.datasource, ); const formattedData = useMemo( () => ({ [RESULT_TYPES.results]: applyFormattingToTabularData( data[RESULT_TYPES.results], + timeFormattedColumns, ), [RESULT_TYPES.samples]: applyFormattingToTabularData( data[RESULT_TYPES.samples], + timeFormattedColumns, ), }), - [data], + [data, timeFormattedColumns], ); const getData = useCallback( @@ -255,25 +255,13 @@ export const DataTablesPane = ({ const colNames = ensureIsArray(json.result[0].colnames); - const getColTypesFromDatasourceColumns = () => - colNames - .map( - name => - datasourceColumns.find(column => column.column_name === name) - ?.type_generic, - ) - .filter(type => type !== undefined); - setColumnNames(prevColumnNames => ({ ...prevColumnNames, [resultType]: colNames, })); setColumnTypes(prevColumnTypes => ({ ...prevColumnTypes, - [resultType]: - json.result[0].coltypes || - getColTypesFromDatasourceColumns() || - [], + [resultType]: json.result[0].coltypes || [], })); setIsLoading(prevIsLoading => ({ ...prevIsLoading, diff --git a/superset-frontend/src/explore/components/useTimeFormattedColumns.ts b/superset-frontend/src/explore/components/useTimeFormattedColumns.ts new file mode 100644 index 000000000000..ed72205c7e8a --- /dev/null +++ b/superset-frontend/src/explore/components/useTimeFormattedColumns.ts @@ -0,0 +1,27 @@ +/** + * 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 { useSelector } from 'react-redux'; +import { ExplorePageState } from '../reducers/getInitialState'; + +export const useTimeFormattedColumns = (datasourceId?: string) => + useSelector(state => + datasourceId + ? state.explore.timeFormattedColumns?.[datasourceId] ?? [] + : [], + ); diff --git a/superset-frontend/src/utils/common.js b/superset-frontend/src/utils/common.js index 3e077075406e..26fdfb4bd6e4 100644 --- a/superset-frontend/src/utils/common.js +++ b/superset-frontend/src/utils/common.js @@ -20,6 +20,7 @@ import { SupersetClient, getTimeFormatter, TimeFormats, + ensureIsArray, } from '@superset-ui/core'; // ATTENTION: If you change any constants, make sure to also change constants.py @@ -107,18 +108,24 @@ export function prepareCopyToClipboardTabularData(data, columns) { return result; } -export function applyFormattingToTabularData(data) { - if (!data || data.length === 0 || !('__timestamp' in data[0])) { +export function applyFormattingToTabularData(data, timeFormattedColumns) { + if ( + !data || + data.length === 0 || + ensureIsArray(timeFormattedColumns).length === 0 + ) { return data; } + return data.map(row => ({ ...row, /* eslint-disable no-underscore-dangle */ - __timestamp: - row.__timestamp === 0 || row.__timestamp - ? DATETIME_FORMATTER(new Date(row.__timestamp)) - : row.__timestamp, - /* eslint-enable no-underscore-dangle */ + ...timeFormattedColumns.reduce((acc, colName) => { + if (row[colName] !== null && row[colName] !== undefined) { + acc[colName] = DATETIME_FORMATTER(row[colName]); + } + return acc; + }, {}), })); } diff --git a/superset-frontend/src/utils/common.test.jsx b/superset-frontend/src/utils/common.test.jsx index 56b9500d5158..6c73b1011cd9 100644 --- a/superset-frontend/src/utils/common.test.jsx +++ b/superset-frontend/src/utils/common.test.jsx @@ -63,29 +63,72 @@ describe('utils/common', () => { describe('applyFormattingToTabularData', () => { it('does not mutate empty array', () => { const data = []; - expect(applyFormattingToTabularData(data)).toEqual(data); + expect(applyFormattingToTabularData(data, [])).toEqual(data); }); it('does not mutate array without temporal column', () => { const data = [ { column1: 'lorem', column2: 'ipsum' }, { column1: 'dolor', column2: 'sit', column3: 'amet' }, ]; - expect(applyFormattingToTabularData(data)).toEqual(data); + expect(applyFormattingToTabularData(data, [])).toEqual(data); }); - it('changes formatting of temporal column', () => { + it('changes formatting of columns selected for formatting', () => { const originalData = [ - { __timestamp: null, column1: 'lorem' }, - { __timestamp: 0, column1: 'ipsum' }, - { __timestamp: 1594285437771, column1: 'dolor' }, - { __timestamp: 1594285441675, column1: 'sit' }, + { + __timestamp: null, + column1: 'lorem', + column2: 1590014060000, + column3: 1507680000000, + }, + { + __timestamp: 0, + column1: 'ipsum', + column2: 1590075817000, + column3: 1513641600000, + }, + { + __timestamp: 1594285437771, + column1: 'dolor', + column2: 1591062977000, + column3: 1516924800000, + }, + { + __timestamp: 1594285441675, + column1: 'sit', + column2: 1591397351000, + column3: 1518566400000, + }, ]; + const timeFormattedColumns = ['__timestamp', 'column3']; const expectedData = [ - { __timestamp: null, column1: 'lorem' }, - { __timestamp: '1970-01-01 00:00:00', column1: 'ipsum' }, - { __timestamp: '2020-07-09 09:03:57', column1: 'dolor' }, - { __timestamp: '2020-07-09 09:04:01', column1: 'sit' }, + { + __timestamp: null, + column1: 'lorem', + column2: 1590014060000, + column3: '2017-10-11 00:00:00', + }, + { + __timestamp: '1970-01-01 00:00:00', + column1: 'ipsum', + column2: 1590075817000, + column3: '2017-12-19 00:00:00', + }, + { + __timestamp: '2020-07-09 09:03:57', + column1: 'dolor', + column2: 1591062977000, + column3: '2018-01-26 00:00:00', + }, + { + __timestamp: '2020-07-09 09:04:01', + column1: 'sit', + column2: 1591397351000, + column3: '2018-02-14 00:00:00', + }, ]; - expect(applyFormattingToTabularData(originalData)).toEqual(expectedData); + expect( + applyFormattingToTabularData(originalData, timeFormattedColumns), + ).toEqual(expectedData); }); }); }); diff --git a/superset/views/core.py b/superset/views/core.py index 08f72a85d5c6..e7fdaf226d5f 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -450,7 +450,11 @@ def get_raw_results(self, viz_obj: BaseViz) -> FlaskResponse: if viz_obj.has_error(payload): return json_error_response(payload=payload, status=400) return self.json_response( - {"data": payload["df"].to_dict("records"), "colnames": payload["colnames"]} + { + "data": payload["df"].to_dict("records"), + "colnames": payload["colnames"], + "coltypes": payload.get("coltypes"), + }, ) def get_samples(self, viz_obj: BaseViz) -> FlaskResponse: diff --git a/superset/viz.py b/superset/viz.py index 26868f169738..768c7d176cf7 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -262,6 +262,7 @@ def get_samples(self) -> Dict[str, Any]: return { "data": payload["df"].to_dict(orient="records"), "colnames": payload["colnames"], + "coltypes": payload.get("coltypes"), } def get_df(self, query_obj: Optional[QueryObjectDict] = None) -> pd.DataFrame: @@ -625,6 +626,7 @@ def get_df_payload( # pylint: disable=too-many-statements "stacktrace": stacktrace, "rowcount": len(df.index) if df is not None else 0, "colnames": list(df.columns) if df is not None else None, + "coltypes": utils.extract_dataframe_dtypes(df, self.datasource), } @staticmethod From ef2f7f5ca6cb545fe56fd197b9df7e5a5ce8a29d Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 4 Feb 2022 17:12:44 +0100 Subject: [PATCH 07/11] Fix df is none --- superset/viz.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/superset/viz.py b/superset/viz.py index 768c7d176cf7..6c039b5e3105 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -626,7 +626,9 @@ def get_df_payload( # pylint: disable=too-many-statements "stacktrace": stacktrace, "rowcount": len(df.index) if df is not None else 0, "colnames": list(df.columns) if df is not None else None, - "coltypes": utils.extract_dataframe_dtypes(df, self.datasource), + "coltypes": utils.extract_dataframe_dtypes(df, self.datasource) + if df is not None + else None, } @staticmethod From 150392503b95faa76d049e4d664d01c974669c6c Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Mon, 7 Feb 2022 14:17:01 +0100 Subject: [PATCH 08/11] Fix test --- .../components/DataTablesPane/DataTablesPane.test.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx index 1a7c6a1c3fd9..380285b81156 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx @@ -131,6 +131,13 @@ test('Should copy data table content correctly', async () => { />, { useRedux: true, + initialState: { + explore: { + timeFormattedColumns: { + '34__table': ['__timestamp'], + }, + }, + }, }, ); userEvent.click(await screen.findByText('Data')); From 18ebae0f49ae378f328734148bd562bcac4fc378 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Mon, 7 Feb 2022 14:47:10 +0100 Subject: [PATCH 09/11] Address comments --- .../src/explore/components/DataTableControl/index.tsx | 11 +++++++---- superset/views/core.py | 2 +- superset/viz.py | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/explore/components/DataTableControl/index.tsx b/superset-frontend/src/explore/components/DataTableControl/index.tsx index 95973b14193b..508e926ca4e8 100644 --- a/superset-frontend/src/explore/components/DataTableControl/index.tsx +++ b/superset-frontend/src/explore/components/DataTableControl/index.tsx @@ -117,7 +117,7 @@ export const RowCount = ({ enum FormatPickerValue { Formatted, - Epoch, + Original, } const FormatPicker = ({ @@ -129,7 +129,7 @@ const FormatPicker = ({ }) => ( - {t('Original value')} + {t('Original value')} {t('Formatted date')} @@ -167,7 +167,10 @@ const DataTableTemporalHeaderCell = ({ if (!datasourceId) { return; } - if (e.target.value === FormatPickerValue.Epoch && isColumnTimeFormatted) { + if ( + e.target.value === FormatPickerValue.Original && + isColumnTimeFormatted + ) { dispatch( unsetTimeFormattedColumn(datasourceId, timeFormattedColumnIndex), ); @@ -204,7 +207,7 @@ const DataTableTemporalHeaderCell = ({ value={ isColumnTimeFormatted ? FormatPickerValue.Formatted - : FormatPickerValue.Epoch + : FormatPickerValue.Original } /> diff --git a/superset/views/core.py b/superset/views/core.py index e7fdaf226d5f..fa5cd0cb5f86 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -452,7 +452,7 @@ def get_raw_results(self, viz_obj: BaseViz) -> FlaskResponse: return self.json_response( { "data": payload["df"].to_dict("records"), - "colnames": payload["colnames"], + "colnames": payload.get("colnames"), "coltypes": payload.get("coltypes"), }, ) diff --git a/superset/viz.py b/superset/viz.py index 6c039b5e3105..26c77c115a40 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -261,7 +261,7 @@ def get_samples(self) -> Dict[str, Any]: payload = self.get_df_payload(query_obj) # leverage caching logic return { "data": payload["df"].to_dict(orient="records"), - "colnames": payload["colnames"], + "colnames": payload.get("colnames"), "coltypes": payload.get("coltypes"), } From 9a77d01b132f001686041fd82bf529bb1f3f4ee6 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Mon, 7 Feb 2022 18:02:13 +0100 Subject: [PATCH 10/11] Move useTimeFormattedColumns out of useTableColumns --- .../spec/helpers/testing-library.tsx | 6 +----- .../components/DataTableControl/index.tsx | 9 +++----- .../DataTableControl/useTableColumns.test.ts | 21 +++++++------------ .../components/DataTablesPane/index.tsx | 12 ++++++++++- 4 files changed, 23 insertions(+), 25 deletions(-) diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index 5afc8539d093..56489cce8471 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -19,7 +19,6 @@ import '@testing-library/jest-dom/extend-expect'; import React, { ReactNode, ReactElement } from 'react'; import { render, RenderOptions } from '@testing-library/react'; -import { renderHook } from '@testing-library/react-hooks'; import { ThemeProvider, supersetTheme } from '@superset-ui/core'; import { BrowserRouter } from 'react-router-dom'; import { Provider } from 'react-redux'; @@ -83,9 +82,6 @@ function createWrapper(options?: Options) { const customRender = (ui: ReactElement, options?: Options) => render(ui, { wrapper: createWrapper(options), ...options }); -const customRenderHook = (callback: (props: any) => any, options?: Options) => - renderHook(callback, { wrapper: createWrapper(options), ...options }); - export function sleep(time: number) { return new Promise(resolve => { setTimeout(resolve, time); @@ -93,4 +89,4 @@ export function sleep(time: number) { } export * from '@testing-library/react'; -export { customRender as render, customRenderHook as renderHook }; +export { customRender as render }; diff --git a/superset-frontend/src/explore/components/DataTableControl/index.tsx b/superset-frontend/src/explore/components/DataTableControl/index.tsx index 508e926ca4e8..68e83bc1a647 100644 --- a/superset-frontend/src/explore/components/DataTableControl/index.tsx +++ b/superset-frontend/src/explore/components/DataTableControl/index.tsx @@ -47,7 +47,6 @@ import { setTimeFormattedColumn, unsetTimeFormattedColumn, } from 'src/explore/actions/exploreActions'; -import { useTimeFormattedColumns } from '../useTimeFormattedColumns'; export const CopyButton = styled(Button)` font-size: ${({ theme }) => theme.typography.sizes.s}px; @@ -269,11 +268,10 @@ export const useTableColumns = ( coltypes?: GenericDataType[], data?: Record[], datasourceId?: string, + timeFormattedColumns: string[] = [], moreConfigs?: { [key: string]: Partial }, -) => { - const timeFormattedColumns = useTimeFormattedColumns(datasourceId); - - return useMemo( +) => + useMemo( () => colnames && data?.length ? colnames @@ -315,4 +313,3 @@ export const useTableColumns = ( : [], [colnames, data, coltypes, datasourceId, moreConfigs, timeFormattedColumns], ); -}; diff --git a/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts b/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts index 24bceb78d15a..bfc4b6d96468 100644 --- a/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts +++ b/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts @@ -17,7 +17,7 @@ * under the License. */ import { GenericDataType } from '@superset-ui/core'; -import { renderHook } from 'spec/helpers/testing-library'; +import { renderHook } from '@testing-library/react-hooks'; import { BOOL_FALSE_DISPLAY, BOOL_TRUE_DISPLAY } from 'src/constants'; import { useTableColumns } from '.'; @@ -52,9 +52,7 @@ const coltypes = [ ]; test('useTableColumns with no options', () => { - const hook = renderHook(() => useTableColumns(all_columns, coltypes, data), { - useRedux: true, - }); + const hook = renderHook(() => useTableColumns(all_columns, coltypes, data)); expect(hook.result.current).toEqual([ { Cell: expect.any(Function), @@ -97,9 +95,8 @@ test('useTableColumns with no options', () => { test('use only the first record columns', () => { const newData = [data[3], data[0]]; - const hook = renderHook( - () => useTableColumns(all_columns, coltypes, newData), - { useRedux: true }, + const hook = renderHook(() => + useTableColumns(all_columns, coltypes, newData), ); expect(hook.result.current).toEqual([ { @@ -156,12 +153,10 @@ test('use only the first record columns', () => { }); test('useTableColumns with options', () => { - const hook = renderHook( - () => - useTableColumns(all_columns, coltypes, data, undefined, { - col01: { id: 'ID' }, - }), - { useRedux: true }, + const hook = renderHook(() => + useTableColumns(all_columns, coltypes, data, undefined, [], { + col01: { id: 'ID' }, + }), ); expect(hook.result.current).toEqual([ { diff --git a/superset-frontend/src/explore/components/DataTablesPane/index.tsx b/superset-frontend/src/explore/components/DataTablesPane/index.tsx index b81871a3b38a..d6cfcc257e24 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/index.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/index.tsx @@ -116,6 +116,7 @@ interface DataTableProps { datasource: string | undefined; filterText: string; data: object[] | undefined; + timeFormattedColumns: string[] | undefined; isLoading: boolean; error: string | undefined; errorMessage: React.ReactElement | undefined; @@ -127,13 +128,20 @@ const DataTable = ({ datasource, filterText, data, + timeFormattedColumns, isLoading, error, errorMessage, }: DataTableProps) => { // this is to preserve the order of the columns, even if there are integer values, // while also only grabbing the first column's keys - const columns = useTableColumns(columnNames, columnTypes, data, datasource); + const columns = useTableColumns( + columnNames, + columnTypes, + data, + datasource, + timeFormattedColumns, + ); const filteredData = useFilteredTableData(filterText, data); if (isLoading) { @@ -406,6 +414,7 @@ export const DataTablesPane = ({ isLoading={isLoading[RESULT_TYPES.results]} data={data[RESULT_TYPES.results]} datasource={queryFormData?.datasource} + timeFormattedColumns={timeFormattedColumns} columnNames={columnNames[RESULT_TYPES.results]} columnTypes={columnTypes[RESULT_TYPES.results]} filterText={filterText} @@ -421,6 +430,7 @@ export const DataTablesPane = ({ isLoading={isLoading[RESULT_TYPES.samples]} data={data[RESULT_TYPES.samples]} datasource={queryFormData?.datasource} + timeFormattedColumns={timeFormattedColumns} columnNames={columnNames[RESULT_TYPES.samples]} columnTypes={columnTypes[RESULT_TYPES.samples]} filterText={filterText} From 618a3ef91a8d4065fb1d532bc80ebaa57d03d762 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 8 Feb 2022 17:22:09 +0100 Subject: [PATCH 11/11] Make reducer more readable --- .../src/explore/reducers/exploreReducer.js | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/superset-frontend/src/explore/reducers/exploreReducer.js b/superset-frontend/src/explore/reducers/exploreReducer.js index 7f328153e057..e2ec3a74dcdc 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.js +++ b/superset-frontend/src/explore/reducers/exploreReducer.js @@ -238,17 +238,15 @@ export default function exploreReducer(state = {}, action) { }; }, [actions.SET_TIME_FORMATTED_COLUMN]() { - const newTimeFormattedColumns = { ...state.timeFormattedColumns }; const { datasourceId, columnName } = action; + const newTimeFormattedColumns = { ...state.timeFormattedColumns }; + const newTimeFormattedColumnsForDatasource = ensureIsArray( + newTimeFormattedColumns[datasourceId], + ).slice(); - if (Array.isArray(newTimeFormattedColumns[action.datasourceId])) { - newTimeFormattedColumns[datasourceId] = [ - ...newTimeFormattedColumns[datasourceId], - columnName, - ]; - } else { - newTimeFormattedColumns[datasourceId] = [columnName]; - } + newTimeFormattedColumnsForDatasource.push(columnName); + newTimeFormattedColumns[datasourceId] = + newTimeFormattedColumnsForDatasource; setItem( LocalStorageKeys.explore__data_table_time_formatted_columns, newTimeFormattedColumns, @@ -256,18 +254,18 @@ export default function exploreReducer(state = {}, action) { return { ...state, timeFormattedColumns: newTimeFormattedColumns }; }, [actions.UNSET_TIME_FORMATTED_COLUMN]() { - const newTimeFormattedColumns = { ...state.timeFormattedColumns }; const { datasourceId, columnIndex } = action; + const newTimeFormattedColumns = { ...state.timeFormattedColumns }; + const newTimeFormattedColumnsForDatasource = ensureIsArray( + newTimeFormattedColumns[datasourceId], + ).slice(); - if (Array.isArray(newTimeFormattedColumns[datasourceId])) { - newTimeFormattedColumns[datasourceId] = [ - ...newTimeFormattedColumns[datasourceId].slice(0, columnIndex), - ...newTimeFormattedColumns[datasourceId].slice(columnIndex + 1), - ]; + newTimeFormattedColumnsForDatasource.splice(columnIndex, 1); + newTimeFormattedColumns[datasourceId] = + newTimeFormattedColumnsForDatasource; - if (newTimeFormattedColumns[datasourceId].length === 0) { - delete newTimeFormattedColumns[datasourceId]; - } + if (newTimeFormattedColumnsForDatasource.length === 0) { + delete newTimeFormattedColumns[datasourceId]; } setItem( LocalStorageKeys.explore__data_table_time_formatted_columns,