From c86d40f0e1ce54c3f8a1d0b5f2745d22b75e9717 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Mon, 27 Sep 2021 19:53:56 -0700 Subject: [PATCH 1/5] initial fix --- .../components/DataTableControl/index.tsx | 2 +- .../components/DataTablesPane/index.tsx | 152 +++++++++++------- 2 files changed, 91 insertions(+), 63 deletions(-) diff --git a/superset-frontend/src/explore/components/DataTableControl/index.tsx b/superset-frontend/src/explore/components/DataTableControl/index.tsx index dc29adc377b4f..52cbeabac2a46 100644 --- a/superset-frontend/src/explore/components/DataTableControl/index.tsx +++ b/superset-frontend/src/explore/components/DataTableControl/index.tsx @@ -144,5 +144,5 @@ export const useTableColumns = ( } as Column), ) : [], - [data, moreConfigs], + [data, colnames, moreConfigs], ); diff --git a/superset-frontend/src/explore/components/DataTablesPane/index.tsx b/superset-frontend/src/explore/components/DataTablesPane/index.tsx index 0cb68d33c398b..4fd34352076dd 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/index.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/index.tsx @@ -105,6 +105,58 @@ const Error = styled.pre` margin-top: ${({ theme }) => `${theme.gridUnit * 4}px`}; `; +interface DataTableProps { + columnNames: string[]; + filterText: string; + data: object[] | undefined; + isLoading: boolean; + error: string | undefined; + errorMessage: React.ReactElement | undefined; +} + +const DataTable = ({ + columnNames, + filterText, + data, + 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, data); + const filteredData = useFilteredTableData(filterText, data); + + if (isLoading) { + return ; + } + if (error) { + return {error}; + } + if (data) { + if (data.length === 0) { + return No data; + } + return ( + + ); + } + if (errorMessage) { + return {errorMessage}; + } + return null; +}; + export const DataTablesPane = ({ queryFormData, tableSectionHeight, @@ -130,7 +182,13 @@ export const DataTablesPane = ({ [RESULT_TYPES.results]: true, [RESULT_TYPES.samples]: true, }); - const [columnNames, setColumnNames] = useState([]); + 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 [filterText, setFilterText] = useState(''); const [activeTabKey, setActiveTabKey] = useState( @@ -179,7 +237,12 @@ export const DataTablesPane = ({ [resultType]: json.result[0].data, })); } - + if (json.result[0].colnames) { + setColumnNames({ + ...columnNames, + [resultType]: json.result[0].colnames, + }); + } setIsLoading(prevIsLoading => ({ ...prevIsLoading, [resultType]: false, @@ -202,7 +265,7 @@ export const DataTablesPane = ({ }); }); }, - [queryFormData], + [queryFormData, columnNames], ); useEffect(() => { @@ -226,7 +289,10 @@ export const DataTablesPane = ({ useEffect(() => { if (queriesResponse && chartStatus === 'success') { const { colnames } = queriesResponse[0]; - setColumnNames([...colnames]); + setColumnNames({ + ...columnNames, + [RESULT_TYPES.results]: [...colnames], + }); } }, [queriesResponse]); @@ -276,65 +342,13 @@ export const DataTablesPane = ({ errorMessage, ]); - const filteredData = { - [RESULT_TYPES.results]: useFilteredTableData( - filterText, - data[RESULT_TYPES.results], - ), - [RESULT_TYPES.samples]: useFilteredTableData( - filterText, - data[RESULT_TYPES.samples], - ), - }; - - // 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 = { - [RESULT_TYPES.results]: useTableColumns( - columnNames, - data[RESULT_TYPES.results], - ), - [RESULT_TYPES.samples]: useTableColumns( - columnNames, - data[RESULT_TYPES.samples], - ), - }; - - const renderDataTable = (type: string) => { - if (isLoading[type]) { - return ; - } - if (error[type]) { - return {error[type]}; - } - if (data[type]) { - if (data[type]?.length === 0) { - return No data; - } - return ( - - ); - } - if (errorMessage) { - return {errorMessage}; - } - return null; - }; - const TableControls = ( - + ); @@ -368,13 +382,27 @@ export const DataTablesPane = ({ tab={t('View results')} key={RESULT_TYPES.results} > - {renderDataTable(RESULT_TYPES.results)} + - {renderDataTable(RESULT_TYPES.samples)} + From 7132189dc31817a124523a4faedebb954ca3ee25 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Fri, 5 Nov 2021 15:26:41 -0700 Subject: [PATCH 2/5] use data keys for cols --- .../explore/components/DataTablesPane/index.tsx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/superset-frontend/src/explore/components/DataTablesPane/index.tsx b/superset-frontend/src/explore/components/DataTablesPane/index.tsx index 1168ccd197724..337f0fe963f19 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/index.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/index.tsx @@ -247,9 +247,23 @@ export const DataTablesPane = ({ } else { setData(prevData => ({ ...prevData, + // [{....}] [resultType]: json.result[0].data, })); } + const checkCols = json?.result[0]?.data?.length + ? Object.keys(json.result[0].data[0]) + : null; + if (!json.result[0].colnames && checkCols?.length) { + let tabKey: string = RESULT_TYPES.results; + if (resultType === RESULT_TYPES.samples) { + tabKey = RESULT_TYPES.samples; + } + setColumnNames({ + ...columnNames, + [tabKey]: checkCols, + }); + } if (json.result[0].colnames) { setColumnNames({ ...columnNames, From 844109bc3bb54656a6807d0c461c0bd9101b3285 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Thu, 11 Nov 2021 22:46:13 -0800 Subject: [PATCH 3/5] add e2e test --- .../integration/explore/control.test.ts | 21 ++++++++++++++++++- .../components/DataTablesPane/index.tsx | 3 +-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts index c57369e89177e..116e521f290a9 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts @@ -88,7 +88,7 @@ describe('Datasource control', () => { }); }); -describe('VizType control', () => { +describe.only('VizType control', () => { beforeEach(() => { cy.login(); interceptChart({ legacy: false }).as('tableChartData'); @@ -113,6 +113,25 @@ describe('VizType control', () => { }); }); +describe.only('Test datatable', () => { + beforeEach(() => { + cy.login(); + interceptChart({ legacy: false }).as('tableChartData'); + interceptChart({ legacy: true }).as('lineChartData'); + }); + it('Data Pane opens and loads results', () => { + cy.get('[data-test="data-tab"]').click(); + cy.get('[data-test="row-count-label"]').contains('27 rows retrieved'); + cy.contains('View results'); + cy.get('.ant-empty-description').should('not.exist'); + }); + it('Datapane loads view samples', () => { + cy.contains('View samples').click(); + cy.get('[data-test="row-count-label"]').contains('1k rows retrieved'); + cy.get('.ant-empty-description').should('not.exist'); + }); +}); + describe('Time range filter', () => { beforeEach(() => { cy.login(); diff --git a/superset-frontend/src/explore/components/DataTablesPane/index.tsx b/superset-frontend/src/explore/components/DataTablesPane/index.tsx index 337f0fe963f19..94d354148ba78 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/index.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/index.tsx @@ -247,7 +247,6 @@ export const DataTablesPane = ({ } else { setData(prevData => ({ ...prevData, - // [{....}] [resultType]: json.result[0].data, })); } @@ -388,7 +387,7 @@ export const DataTablesPane = ({ return ( - + Date: Thu, 11 Nov 2021 22:48:15 -0800 Subject: [PATCH 4/5] remove code --- .../cypress-base/cypress/integration/explore/control.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts index 116e521f290a9..b316d5c12e899 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts @@ -88,7 +88,7 @@ describe('Datasource control', () => { }); }); -describe.only('VizType control', () => { +describe('VizType control', () => { beforeEach(() => { cy.login(); interceptChart({ legacy: false }).as('tableChartData'); @@ -113,7 +113,7 @@ describe.only('VizType control', () => { }); }); -describe.only('Test datatable', () => { +describe('Test datatable', () => { beforeEach(() => { cy.login(); interceptChart({ legacy: false }).as('tableChartData'); From e7ea74aa487288367006074092bfa53d056308e4 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Fri, 12 Nov 2021 11:36:33 -0800 Subject: [PATCH 5/5] trim logic --- .../components/DataTablesPane/index.tsx | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/superset-frontend/src/explore/components/DataTablesPane/index.tsx b/superset-frontend/src/explore/components/DataTablesPane/index.tsx index 94d354148ba78..67a07f905fed6 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/index.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/index.tsx @@ -253,22 +253,10 @@ export const DataTablesPane = ({ const checkCols = json?.result[0]?.data?.length ? Object.keys(json.result[0].data[0]) : null; - if (!json.result[0].colnames && checkCols?.length) { - let tabKey: string = RESULT_TYPES.results; - if (resultType === RESULT_TYPES.samples) { - tabKey = RESULT_TYPES.samples; - } - setColumnNames({ - ...columnNames, - [tabKey]: checkCols, - }); - } - if (json.result[0].colnames) { - setColumnNames({ - ...columnNames, - [resultType]: json.result[0].colnames, - }); - } + setColumnNames(prevColumnNames => ({ + ...prevColumnNames, + [resultType]: json.result[0].columns || checkCols, + })); setIsLoading(prevIsLoading => ({ ...prevIsLoading, [resultType]: false,