From 04b7a26365edac524cbeb7336bd64898cdf52835 Mon Sep 17 00:00:00 2001 From: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com> Date: Mon, 5 Dec 2022 15:43:54 -0600 Subject: [PATCH] feat: Flow for tables that already have a dataset (#22136) --- .../DatasetPanel/DatasetPanel.test.tsx | 31 ++- .../AddDataset/DatasetPanel/DatasetPanel.tsx | 201 +++++++++++++----- .../AddDataset/DatasetPanel/fixtures.ts | 14 ++ .../dataset/AddDataset/DatasetPanel/index.tsx | 6 +- .../dataset/AddDataset/Footer/Footer.test.tsx | 16 +- .../data/dataset/AddDataset/Footer/index.tsx | 12 +- .../AddDataset/LeftPanel/LeftPanel.test.tsx | 45 ++++ .../dataset/AddDataset/LeftPanel/index.tsx | 67 ++++-- .../CRUD/data/dataset/AddDataset/index.tsx | 44 +++- .../CRUD/data/dataset/AddDataset/types.tsx | 1 + .../src/views/CRUD/data/dataset/styles.ts | 1 + .../src/views/CRUD/data/hooks.ts | 10 + 12 files changed, 366 insertions(+), 82 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.test.tsx index 6800594bd8eb..4b066eb30b29 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.test.tsx @@ -24,7 +24,7 @@ import DatasetPanel, { tableColumnDefinition, COLUMN_TITLE, } from './DatasetPanel'; -import { exampleColumns } from './fixtures'; +import { exampleColumns, exampleDataset } from './fixtures'; import { SELECT_MESSAGE, CREATE_MESSAGE, @@ -44,7 +44,7 @@ jest.mock( ); describe('DatasetPanel', () => { - it('renders a blank state DatasetPanel', () => { + test('renders a blank state DatasetPanel', () => { render(); const blankDatasetImg = screen.getByRole('img', { name: /empty/i }); @@ -65,7 +65,7 @@ describe('DatasetPanel', () => { expect(sqlLabLink).toBeVisible(); }); - it('renders a no columns screen', () => { + test('renders a no columns screen', () => { render( { expect(noColumnsDescription).toBeVisible(); }); - it('renders a loading screen', () => { + test('renders a loading screen', () => { render( { expect(blankDatasetTitle).toBeVisible(); }); - it('renders an error screen', () => { + test('renders an error screen', () => { render( { expect(errorDescription).toBeVisible(); }); - it('renders a table with columns displayed', async () => { + test('renders a table with columns displayed', async () => { const tableName = 'example_name'; render( { expect(screen.getByText(row.type)).toBeInTheDocument(); }); }); + + test('renders an info banner if table already has a dataset', async () => { + render( + , + ); + + // This is text in the info banner + expect( + await screen.findByText( + /this table already has a dataset associated with it. you can only associate one dataset with a table./i, + ), + ).toBeVisible(); + }); }); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx index 2a5e12cea888..b8d0a469411f 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx @@ -17,12 +17,14 @@ * under the License. */ import React from 'react'; -import { supersetTheme, t, styled } from '@superset-ui/core'; +import { t, styled, useTheme } from '@superset-ui/core'; import Icons from 'src/components/Icons'; +import Alert from 'src/components/Alert'; import Table, { ColumnsType, TableSize } from 'src/components/Table'; import { alphabeticalSort } from 'src/components/Table/sorters'; // @ts-ignore import LOADING_GIF from 'src/assets/images/loading.gif'; +import { DatasetObject } from 'src/views/CRUD/data/dataset/AddDataset/types'; import { ITableColumn } from './types'; import MessageContent from './MessageContent'; @@ -53,43 +55,56 @@ const HALF = 0.5; const MARGIN_MULTIPLIER = 3; const StyledHeader = styled.div` + ${({ theme }) => ` position: ${(props: StyledHeaderProps) => props.position}; - margin: ${({ theme }) => theme.gridUnit * MARGIN_MULTIPLIER}px; - margin-top: ${({ theme }) => theme.gridUnit * (MARGIN_MULTIPLIER + 1)}px; - font-size: ${({ theme }) => theme.gridUnit * 6}px; - font-weight: ${({ theme }) => theme.typography.weights.medium}; - padding-bottom: ${({ theme }) => theme.gridUnit * MARGIN_MULTIPLIER}px; + margin: ${theme.gridUnit * (MARGIN_MULTIPLIER + 1)}px + ${theme.gridUnit * MARGIN_MULTIPLIER}px + ${theme.gridUnit * MARGIN_MULTIPLIER}px + ${theme.gridUnit * (MARGIN_MULTIPLIER + 3)}px; + font-size: ${theme.gridUnit * 6}px; + font-weight: ${theme.typography.weights.medium}; + padding-bottom: ${theme.gridUnit * MARGIN_MULTIPLIER}px; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; .anticon:first-of-type { - margin-right: ${({ theme }) => theme.gridUnit * (MARGIN_MULTIPLIER + 1)}px; + margin-right: ${theme.gridUnit * (MARGIN_MULTIPLIER + 1)}px; } .anticon:nth-of-type(2) { - margin-left: ${({ theme }) => theme.gridUnit * (MARGIN_MULTIPLIER + 1)}px; - } + margin-left: ${theme.gridUnit * (MARGIN_MULTIPLIER + 1)}px; + `} `; const StyledTitle = styled.div` - margin-left: ${({ theme }) => theme.gridUnit * MARGIN_MULTIPLIER}px; - margin-bottom: ${({ theme }) => theme.gridUnit * MARGIN_MULTIPLIER}px; - font-weight: ${({ theme }) => theme.typography.weights.bold}; + ${({ theme }) => ` + margin-left: ${theme.gridUnit * (MARGIN_MULTIPLIER + 3)}px; + margin-bottom: ${theme.gridUnit * MARGIN_MULTIPLIER}px; + font-weight: ${theme.typography.weights.bold}; + `} `; const LoaderContainer = styled.div` - padding: ${({ theme }) => theme.gridUnit * 8}px - ${({ theme }) => theme.gridUnit * 6}px; - + ${({ theme }) => ` + padding: ${theme.gridUnit * 8}px + ${theme.gridUnit * 6}px; + box-sizing: border-box; display: flex; align-items: center; justify-content: center; height: 100%; + position: absolute; + top: 0; + bottom: 0; + left: 0; + right: 0; + `} `; const StyledLoader = styled.div` + ${({ theme }) => ` max-width: 50%; width: ${LOADER_WIDTH}px; @@ -100,19 +115,23 @@ const StyledLoader = styled.div` div { width: 100%; - margin-top: ${({ theme }) => theme.gridUnit * MARGIN_MULTIPLIER}px; + margin-top: ${theme.gridUnit * MARGIN_MULTIPLIER}px; text-align: center; - font-weight: ${({ theme }) => theme.typography.weights.normal}; - font-size: ${({ theme }) => theme.typography.sizes.l}px; - color: ${({ theme }) => theme.colors.grayscale.light1}; + font-weight: ${theme.typography.weights.normal}; + font-size: ${theme.typography.sizes.l}px; + color: ${theme.colors.grayscale.light1}; } + `} `; const TableContainer = styled.div` + ${({ theme }) => ` position: relative; - margin: ${({ theme }) => theme.gridUnit * MARGIN_MULTIPLIER}px; + margin: ${theme.gridUnit * MARGIN_MULTIPLIER}px; + margin-left: ${theme.gridUnit * (MARGIN_MULTIPLIER + 3)}px; overflow: scroll; - height: calc(100% - ${({ theme }) => theme.gridUnit * 36}px); + height: calc(100% - ${theme.gridUnit * 36}px); + `} `; const StyledTable = styled(Table)` @@ -123,6 +142,26 @@ const StyledTable = styled(Table)` right: 0; `; +const StyledAlert = styled(Alert)` + ${({ theme }) => ` + border: 1px solid ${theme.colors.info.base}; + padding: ${theme.gridUnit * 4}px; + margin: ${theme.gridUnit * 6}px ${theme.gridUnit * 6}px + ${theme.gridUnit * 8}px; + .view-dataset-button { + position: absolute; + top: ${theme.gridUnit * 4}px; + right: ${theme.gridUnit * 4}px; + font-weight: ${theme.typography.weights.normal}; + + &:hover { + color: ${theme.colors.secondary.dark3}; + text-decoration: underline; + } + } + `} +`; + export const REFRESHING = t('Refreshing columns'); export const COLUMN_TITLE = t('Table columns'); export const ALT_LOADING = t('Loading'); @@ -168,19 +207,57 @@ export interface IDatasetPanelProps { * Boolean indicating if the component is in a loading state */ loading: boolean; + datasets?: DatasetObject[] | undefined; } +const EXISTING_DATASET_DESCRIPTION = t( + 'This table already has a dataset associated with it. You can only associate one dataset with a table.\n', +); +const VIEW_DATASET = t('View Dataset'); + +const renderExistingDatasetAlert = (dataset?: DatasetObject) => ( + + {EXISTING_DATASET_DESCRIPTION} + { + window.open( + dataset?.explore_url, + '_blank', + 'noreferrer noopener popup=false', + ); + }} + tabIndex={0} + className="view-dataset-button" + > + {VIEW_DATASET} + + + } + /> +); + const DatasetPanel = ({ tableName, columnList, loading, hasError, + datasets, }: IDatasetPanelProps) => { + const theme = useTheme(); const hasColumns = columnList?.length > 0 ?? false; + const datasetNames = datasets?.map(dataset => dataset.table_name); let component; + let loader; if (loading) { - component = ( + loader = ( {ALT_LOADING} @@ -188,48 +265,58 @@ const DatasetPanel = ({ ); - } else if (tableName && hasColumns && !hasError) { - component = ( - <> - {COLUMN_TITLE} - - - - - ); - } else { - component = ( - - ); + } + if (!loading) { + if (!loading && tableName && hasColumns && !hasError) { + component = ( + <> + {COLUMN_TITLE} + + + + + ); + } else { + component = ( + + ); + } } return ( <> {tableName && ( - - {tableName && ( - - )} - {tableName} - + <> + {datasetNames?.includes(tableName) && + renderExistingDatasetAlert( + datasets?.find(dataset => dataset.table_name === tableName), + )} + + {tableName && ( + + )} + {tableName} + + )} {component} + {loader} ); }; diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/fixtures.ts b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/fixtures.ts index 2199190c9906..d3ee58da14b9 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/fixtures.ts +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/fixtures.ts @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import { DatasetObject } from 'src/views/CRUD/data/dataset/AddDataset/types'; import { ITableColumn } from './types'; export const exampleColumns: ITableColumn[] = [ @@ -32,3 +33,16 @@ export const exampleColumns: ITableColumn[] = [ type: 'DATE', }, ]; + +export const exampleDataset: DatasetObject[] = [ + { + db: { + id: 1, + database_name: 'test_database', + owners: [1], + }, + schema: 'test_schema', + dataset_name: 'example_dataset', + table_name: 'example_table', + }, +]; diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/index.tsx index e390c781fd2e..15e6225e5612 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/index.tsx @@ -18,6 +18,7 @@ */ import React, { useEffect, useState, useRef } from 'react'; import { SupersetClient } from '@superset-ui/core'; +import { DatasetObject } from 'src/views/CRUD/data/dataset/AddDataset/types'; import DatasetPanel from './DatasetPanel'; import { ITableColumn, IDatabaseTable, isIDatabaseTable } from './types'; @@ -53,6 +54,7 @@ export interface IDatasetPanelWrapperProps { */ schema?: string | null; setHasColumns?: Function; + datasets?: DatasetObject[] | undefined; } const DatasetPanelWrapper = ({ @@ -60,6 +62,7 @@ const DatasetPanelWrapper = ({ dbId, schema, setHasColumns, + datasets, }: IDatasetPanelWrapperProps) => { const [columnList, setColumnList] = useState([]); const [loading, setLoading] = useState(false); @@ -110,7 +113,7 @@ const DatasetPanelWrapper = ({ if (tableName && schema && dbId) { getTableMetadata({ tableName, dbId, schema }); } - // getTableMetadata is a const and should not be independency array + // getTableMetadata is a const and should not be in dependency array // eslint-disable-next-line react-hooks/exhaustive-deps }, [tableName, dbId, schema]); @@ -120,6 +123,7 @@ const DatasetPanelWrapper = ({ hasError={hasError} loading={loading} tableName={tableName} + datasets={datasets} /> ); }; diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/Footer.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/Footer.test.tsx index bb51c4de8644..8fcbd83b15cc 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/Footer.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/Footer.test.tsx @@ -40,7 +40,7 @@ const mockPropsWithDataset = { }; describe('Footer', () => { - it('renders a Footer with a cancel button and a disabled create button', () => { + test('renders a Footer with a cancel button and a disabled create button', () => { render(