From 7b5c1ac5a220c21a504012df0a9e2f105f66b79d Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Fri, 4 Nov 2022 18:45:37 -0700 Subject: [PATCH 01/19] feat: add tabs to edit dataset page --- .../dataset/AddDataset/EditDataset/index.tsx | 76 +++++++++++++++++++ .../CRUD/data/dataset/AddDataset/index.tsx | 19 ++++- .../CRUD/data/dataset/DatasetLayout/index.tsx | 15 ++-- 3 files changed, 102 insertions(+), 8 deletions(-) create mode 100644 superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx new file mode 100644 index 0000000000000..52358818a7c65 --- /dev/null +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx @@ -0,0 +1,76 @@ +/** + * 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 { css, styled, SupersetClient, t } from '@superset-ui/core'; +import React, { useEffect, useState } from 'react'; +import Badge from 'src/components/Badge'; +import Tabs from 'src/components/Tabs'; + +const StyledTabs = styled(Tabs)` + margin-top: 34px; + padding-left: 16px; + div[role='tablist'] { + width: 307px; + } +`; + +const TabStyles = styled.div` + .ant-badge { + width: 32px; + } +`; + +const EditPage = ({ id }) => { + const [usageCount, setUsageCount] = useState(0); + useEffect(() => { + if (id) + SupersetClient.get({ + endpoint: `/api/v1/dataset/${id}/related_objects`, + }) + .then(({ json = {} }) => { + console.log('JSON', json); + }) + .catch(err => console.log(err)); + }, [id]); + + const Tab = ( + + {t('Usage')} + + + ); + + return ( + <> + + +
placeholder
+
+ +
placeholder
+
+ +
placeholder
+
+ +
+ + ); +}; + +export default EditPage; diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx index a6d84ff10ee60..e17bd65f8d84c 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx @@ -27,6 +27,7 @@ import { logging, t } from '@superset-ui/core'; import { UseGetDatasetsList } from 'src/views/CRUD/data/hooks'; import { addDangerToast } from 'src/components/MessageToasts/actions'; import Header from './Header'; +import EditPage from './EditDataset'; import DatasetPanel from './DatasetPanel'; import LeftPanel from './LeftPanel'; import Footer from './Footer'; @@ -111,6 +112,15 @@ export default function AddDataset() { getDatasetsList(); } }, [dataset?.schema, getDatasetsList]); + const [showEditPage, setShowEditPage] = useState(false); + + const id = window.location.pathname.split('/')[2]; + useEffect(() => { + if (typeof Number(id) === 'number') { + console.log('showeditpage set', Number(id)); + setShowEditPage(true); + } + }, [id]); const HeaderComponent = () => (
@@ -124,6 +134,8 @@ export default function AddDataset() { /> ); + const EditPageComponent = () => ; + const DatasetPanelComponent = () => ( ); } diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx index b5691fe5cb1d8..992072e2b3b96 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx @@ -37,6 +37,7 @@ interface DatasetLayoutProps { datasetPanel?: ReactElement> | null; rightPanel?: ReactElement> | null; footer?: ReactElement> | null; + showEditPage: boolean; } export default function DatasetLayout({ @@ -45,17 +46,19 @@ export default function DatasetLayout({ datasetPanel, rightPanel, footer, + showEditPage, }: DatasetLayoutProps) { return ( {header && {header}} - - {leftPanel && ( - {leftPanel} - )} - - + {!showEditPage && ( + + {leftPanel && ( + {leftPanel} + )} + + )} {datasetPanel && ( From 3b42fc5dcca8c86377f3ee60687a34ffb94d0575 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 8 Nov 2022 04:01:08 -0800 Subject: [PATCH 02/19] add test, fix css, and add comments --- .../EditDataset/EditDataset.test.tsx | 43 +++++++++++++++++++ .../dataset/AddDataset/EditDataset/index.tsx | 38 +++++++++------- .../CRUD/data/dataset/AddDataset/index.tsx | 3 +- .../CRUD/data/dataset/DatasetLayout/index.tsx | 2 +- 4 files changed, 68 insertions(+), 18 deletions(-) create mode 100644 superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx new file mode 100644 index 0000000000000..1f885fc5082ff --- /dev/null +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx @@ -0,0 +1,43 @@ +/** + * 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 React from 'react'; +import fetchMock from 'fetch-mock'; +import { render, screen } from 'spec/helpers/testing-library'; +import EditDataset from './index'; + +const DATASET_ENDPOINT = 'glob:*api/v1/dataset/1/related_objects'; + +const mockedProps = { + id: 1, +}; + +fetchMock.get(DATASET_ENDPOINT, { charts: { results: [], count: 2 } }); + +test('should render edit dataset view with tabs', () => { + fetchMock.calls(DATASET_ENDPOINT); + render(); + + const columnTab = screen.getByRole('tab', { name: /columns/i }); + const metricsTab = screen.getByRole('tab', { name: /metrics/i }); + const UsagesTab = screen.getByText(/usage/i); + + expect(columnTab).toBeInTheDocument(); + expect(metricsTab).toBeInTheDocument(); + expect(UsagesTab).toBeInTheDocument(); +}); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx index 52358818a7c65..9defc1a0c62f3 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { css, styled, SupersetClient, t } from '@superset-ui/core'; +import { styled, SupersetClient, t } from '@superset-ui/core'; import React, { useEffect, useState } from 'react'; import Badge from 'src/components/Badge'; import Tabs from 'src/components/Tabs'; @@ -24,26 +24,39 @@ import Tabs from 'src/components/Tabs'; const StyledTabs = styled(Tabs)` margin-top: 34px; padding-left: 16px; - div[role='tablist'] { - width: 307px; + // display: inline-block; + .ant-tabs-top > .ant-tabs-nav::before, + .ant-tabs-top > .ant-tabs-nav::before { + width: 200px; + } + .ant-tabs-nav-list > div:nth-last-child(2) { + // margin-right: 0px; } `; const TabStyles = styled.div` .ant-badge { width: 32px; + margin-left: 10px; } `; -const EditPage = ({ id }) => { +interface EditPageProps { + id: number; +} + +const EditPage = ({ id }: EditPageProps) => { const [usageCount, setUsageCount] = useState(0); useEffect(() => { + // Todo: this useEffect should be used to call all count methods conncurently + // when we populate data for the new tabs. For right separating out this + // api call for building the usage page. if (id) SupersetClient.get({ endpoint: `/api/v1/dataset/${id}/related_objects`, }) .then(({ json = {} }) => { - console.log('JSON', json); + setUsageCount(json.charts.count); }) .catch(err => console.log(err)); }, [id]); @@ -51,23 +64,18 @@ const EditPage = ({ id }) => { const Tab = ( {t('Usage')} - + ); return ( <> - - -
placeholder
-
- + + + +
placeholder
- -
placeholder
-
-
); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx index e17bd65f8d84c..1ca10ecd10e7f 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx @@ -116,8 +116,7 @@ export default function AddDataset() { const id = window.location.pathname.split('/')[2]; useEffect(() => { - if (typeof Number(id) === 'number') { - console.log('showeditpage set', Number(id)); + if (!Number.isNaN(parseFloat(id))) { setShowEditPage(true); } }, [id]); diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx index 992072e2b3b96..a0e642d81a91b 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx @@ -37,7 +37,7 @@ interface DatasetLayoutProps { datasetPanel?: ReactElement> | null; rightPanel?: ReactElement> | null; footer?: ReactElement> | null; - showEditPage: boolean; + showEditPage?: boolean; } export default function DatasetLayout({ From 0b6af234d62be1b10d3afe5f9a3c42e9c499bf53 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 8 Nov 2022 04:04:47 -0800 Subject: [PATCH 03/19] fix type --- .../data/dataset/AddDataset/EditDataset/EditDataset.test.tsx | 2 +- .../views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx index 1f885fc5082ff..f0cebc24f98b4 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx @@ -24,7 +24,7 @@ import EditDataset from './index'; const DATASET_ENDPOINT = 'glob:*api/v1/dataset/1/related_objects'; const mockedProps = { - id: 1, + id: '1', }; fetchMock.get(DATASET_ENDPOINT, { charts: { results: [], count: 2 } }); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx index 9defc1a0c62f3..fb4e45c459938 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx @@ -42,7 +42,7 @@ const TabStyles = styled.div` `; interface EditPageProps { - id: number; + id: string; } const EditPage = ({ id }: EditPageProps) => { From 3b59aabd57f80b4e34aa2e230b4d92829118d47f Mon Sep 17 00:00:00 2001 From: AAfghahi <48933336+AAfghahi@users.noreply.github.com> Date: Tue, 15 Nov 2022 11:31:44 -0500 Subject: [PATCH 04/19] Update superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com> --- .../data/dataset/AddDataset/EditDataset/EditDataset.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx index f0cebc24f98b4..8959bcb2537a6 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx @@ -29,11 +29,11 @@ const mockedProps = { fetchMock.get(DATASET_ENDPOINT, { charts: { results: [], count: 2 } }); -test('should render edit dataset view with tabs', () => { +test('should render edit dataset view with tabs', async () => { fetchMock.calls(DATASET_ENDPOINT); render(); - const columnTab = screen.getByRole('tab', { name: /columns/i }); + const columnTab = await screen.findByRole('tab', { name: /columns/i }); const metricsTab = screen.getByRole('tab', { name: /metrics/i }); const UsagesTab = screen.getByText(/usage/i); From 435edca4695e3c54868ba989547445d040ecff27 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Mon, 12 Dec 2022 21:45:52 -0600 Subject: [PATCH 05/19] Rebase and make usage tab accessible --- .../views/CRUD/data/dataset/AddDataset/Footer/index.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx index e0853cdc9d587..8e9b13a5e7a52 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx @@ -73,6 +73,14 @@ function Footer({ return LOG_ACTIONS[value]; }; +<<<<<<< HEAD +======= + const goToPreviousUrl = (newurl = url) => { + // this is a placeholder url until the final feature gets implemented + // at that point we will be passing in the url of the previous location. + window.location.href = newurl; + }; +>>>>>>> 9dbecbebdd (Rebase and make usage tab accessible) const cancelButtonOnClick = () => { if (!datasetObject) { From e65b480db3b87a2f6a2bcac827a6d2c66b9a199d Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Mon, 12 Dec 2022 22:06:15 -0600 Subject: [PATCH 06/19] Address review comments --- .../dataset/AddDataset/EditDataset/index.tsx | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx index fb4e45c459938..7a57dbe64ac69 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx @@ -22,29 +22,35 @@ import Badge from 'src/components/Badge'; import Tabs from 'src/components/Tabs'; const StyledTabs = styled(Tabs)` - margin-top: 34px; - padding-left: 16px; - // display: inline-block; - .ant-tabs-top > .ant-tabs-nav::before, + ${({ theme }) => ` + margin-top: ${theme.gridUnit * 8.5}px; + padding-left: ${theme.gridUnit * 4}px; + .ant-tabs-top > .ant-tabs-nav::before { - width: 200px; - } - .ant-tabs-nav-list > div:nth-last-child(2) { - // margin-right: 0px; + width: ${theme.gridUnit * 50}px; } + `} `; const TabStyles = styled.div` + ${({ theme }) => ` .ant-badge { - width: 32px; - margin-left: 10px; + width: ${theme.gridUnit * 8}px; + margin-left: ${theme.gridUnit * 2.5}px; } + `} `; interface EditPageProps { id: string; } +const TRANSLATED = { + USAGE_TEXT: t('Usage'), + COLUMNS_TEXT: t('Columns'), + METRICS_TEXT: t('Metrics'), +}; + const EditPage = ({ id }: EditPageProps) => { const [usageCount, setUsageCount] = useState(0); useEffect(() => { @@ -61,23 +67,19 @@ const EditPage = ({ id }: EditPageProps) => { .catch(err => console.log(err)); }, [id]); - const Tab = ( + const UsageTab = ( - {t('Usage')} + {TRANSLATED.USAGE_TEXT} ); return ( - <> - - - - -
placeholder
-
-
- + + + + + ); }; From fb08fac059c81e87ff4ff00df73d8a37a67a683d Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Mon, 12 Dec 2022 22:54:40 -0600 Subject: [PATCH 07/19] Fix EditDataset test --- .../data/dataset/AddDataset/EditDataset/EditDataset.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx index 8959bcb2537a6..b65c255d255e5 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx @@ -35,9 +35,9 @@ test('should render edit dataset view with tabs', async () => { const columnTab = await screen.findByRole('tab', { name: /columns/i }); const metricsTab = screen.getByRole('tab', { name: /metrics/i }); - const UsagesTab = screen.getByText(/usage/i); + const usageTab = screen.getByRole('tab', { name: /usage/i }); expect(columnTab).toBeInTheDocument(); expect(metricsTab).toBeInTheDocument(); - expect(UsagesTab).toBeInTheDocument(); + expect(usageTab).toBeInTheDocument(); }); From 3c435f23caf2dae0912f19fa4df65b4374c7db17 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Tue, 13 Dec 2022 12:04:10 -0600 Subject: [PATCH 08/19] Change naming of TRANSLATED to TRANSLATIONS and add logging --- .../data/dataset/AddDataset/EditDataset/index.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx index 7a57dbe64ac69..7bc35f7cdacee 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { styled, SupersetClient, t } from '@superset-ui/core'; +import { styled, SupersetClient, t, logging } from '@superset-ui/core'; import React, { useEffect, useState } from 'react'; import Badge from 'src/components/Badge'; import Tabs from 'src/components/Tabs'; @@ -45,7 +45,7 @@ interface EditPageProps { id: string; } -const TRANSLATED = { +const TRANSLATiONS = { USAGE_TEXT: t('Usage'), COLUMNS_TEXT: t('Columns'), METRICS_TEXT: t('Metrics'), @@ -64,20 +64,20 @@ const EditPage = ({ id }: EditPageProps) => { .then(({ json = {} }) => { setUsageCount(json.charts.count); }) - .catch(err => console.log(err)); + .catch(err => logging.log(err)); }, [id]); const UsageTab = ( - {TRANSLATED.USAGE_TEXT} + {TRANSLATiONS.USAGE_TEXT} ); return ( - - + + ); From b62857cf1c7fff4db7c9b16b912b7294c7d9609a Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Tue, 3 Jan 2023 17:42:49 -0600 Subject: [PATCH 09/19] Address review comments --- .../dataset/AddDataset/EditDataset/index.tsx | 21 +++++++++---------- .../data/dataset/AddDataset/Footer/index.tsx | 8 ------- .../CRUD/data/dataset/AddDataset/index.tsx | 10 ++++----- .../CRUD/data/dataset/DatasetLayout/index.tsx | 6 +++--- .../src/views/CRUD/data/hooks.ts | 7 +++++++ 5 files changed, 25 insertions(+), 27 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx index 7bc35f7cdacee..3c674c69d963e 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx @@ -16,8 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { styled, SupersetClient, t, logging } from '@superset-ui/core'; +import { styled, t, logging } from '@superset-ui/core'; import React, { useEffect, useState } from 'react'; +import { UseGetDatasetRelatedObjects } from 'src/views/CRUD/data/hooks'; import Badge from 'src/components/Badge'; import Tabs from 'src/components/Tabs'; @@ -45,7 +46,7 @@ interface EditPageProps { id: string; } -const TRANSLATiONS = { +const TRANSLATIONS = { USAGE_TEXT: t('Usage'), COLUMNS_TEXT: t('Columns'), METRICS_TEXT: t('Metrics'), @@ -58,26 +59,24 @@ const EditPage = ({ id }: EditPageProps) => { // when we populate data for the new tabs. For right separating out this // api call for building the usage page. if (id) - SupersetClient.get({ - endpoint: `/api/v1/dataset/${id}/related_objects`, - }) - .then(({ json = {} }) => { - setUsageCount(json.charts.count); + UseGetDatasetRelatedObjects(id) + .then(json => { + setUsageCount(json?.charts.count); }) .catch(err => logging.log(err)); }, [id]); const UsageTab = ( - {TRANSLATiONS.USAGE_TEXT} - + {TRANSLATIONS.USAGE_TEXT} + {usageCount > 0 && } ); return ( - - + + ); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx index 8e9b13a5e7a52..e0853cdc9d587 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx @@ -73,14 +73,6 @@ function Footer({ return LOG_ACTIONS[value]; }; -<<<<<<< HEAD -======= - const goToPreviousUrl = (newurl = url) => { - // this is a placeholder url until the final feature gets implemented - // at that point we will be passing in the url of the previous location. - window.location.href = newurl; - }; ->>>>>>> 9dbecbebdd (Rebase and make usage tab accessible) const cancelButtonOnClick = () => { if (!datasetObject) { diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx index 1ca10ecd10e7f..2b6afa6a37e9d 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx @@ -84,6 +84,7 @@ export default function AddDataset() { >(datasetReducer, null); const [hasColumns, setHasColumns] = useState(false); const [datasets, setDatasets] = useState([]); + const [editPageIsVisible, setEditPageIsVisible] = useState(false); const datasetNames = datasets.map(dataset => dataset.table_name); const encodedSchema = dataset?.schema ? encodeURIComponent(dataset?.schema) @@ -112,12 +113,11 @@ export default function AddDataset() { getDatasetsList(); } }, [dataset?.schema, getDatasetsList]); - const [showEditPage, setShowEditPage] = useState(false); const id = window.location.pathname.split('/')[2]; useEffect(() => { if (!Number.isNaN(parseFloat(id))) { - setShowEditPage(true); + setEditPageIsVisible(true); } }, [id]); @@ -157,12 +157,12 @@ export default function AddDataset() { return ( ); } diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx index a0e642d81a91b..d7c335e7b39ba 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx @@ -37,7 +37,7 @@ interface DatasetLayoutProps { datasetPanel?: ReactElement> | null; rightPanel?: ReactElement> | null; footer?: ReactElement> | null; - showEditPage?: boolean; + editPageIsVisible?: boolean; } export default function DatasetLayout({ @@ -46,13 +46,13 @@ export default function DatasetLayout({ datasetPanel, rightPanel, footer, - showEditPage, + editPageIsVisible, }: DatasetLayoutProps) { return ( {header && {header}} - {!showEditPage && ( + {!editPageIsVisible && ( {leftPanel && ( {leftPanel} diff --git a/superset-frontend/src/views/CRUD/data/hooks.ts b/superset-frontend/src/views/CRUD/data/hooks.ts index 0c1528f07f437..de8cf2a964f6f 100644 --- a/superset-frontend/src/views/CRUD/data/hooks.ts +++ b/superset-frontend/src/views/CRUD/data/hooks.ts @@ -113,3 +113,10 @@ export const UseGetDatasetsList = async (filters: object[]) => { } return results; }; + +export const UseGetDatasetRelatedObjects = (id: string) => + SupersetClient.get({ + endpoint: `/api/v1/dataset/${id}/related_objects`, + }) + .then(({ json }) => json) + .catch(error => logging.error(error)); From c47f25dd78e5b754dab5d16e5cec8182eda99444 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Tue, 3 Jan 2023 22:22:14 -0600 Subject: [PATCH 10/19] Fix dataset hooks --- .../dataset/AddDataset/EditDataset/index.tsx | 18 ++-- .../CRUD/data/dataset/AddDataset/index.tsx | 42 +++------ .../src/views/CRUD/data/hooks.ts | 93 +++++++++++-------- 3 files changed, 73 insertions(+), 80 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx index 3c674c69d963e..80c1847f52591 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx @@ -16,9 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { styled, t, logging } from '@superset-ui/core'; -import React, { useEffect, useState } from 'react'; -import { UseGetDatasetRelatedObjects } from 'src/views/CRUD/data/hooks'; +import { styled, t } from '@superset-ui/core'; +import React, { useEffect } from 'react'; +import { useGetDatasetRelatedObjects } from 'src/views/CRUD/data/hooks'; import Badge from 'src/components/Badge'; import Tabs from 'src/components/Tabs'; @@ -53,17 +53,15 @@ const TRANSLATIONS = { }; const EditPage = ({ id }: EditPageProps) => { - const [usageCount, setUsageCount] = useState(0); + const { getDatasetRelatedObjects, usageCount } = + useGetDatasetRelatedObjects(id); useEffect(() => { // Todo: this useEffect should be used to call all count methods conncurently // when we populate data for the new tabs. For right separating out this // api call for building the usage page. - if (id) - UseGetDatasetRelatedObjects(id) - .then(json => { - setUsageCount(json?.charts.count); - }) - .catch(err => logging.log(err)); + if (id) { + getDatasetRelatedObjects(); + } }, [id]); const UsageTab = ( diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx index 2b6afa6a37e9d..f7eb66a25e2c3 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx @@ -16,16 +16,8 @@ * specific language governing permissions and limitations * under the License. */ -import React, { - useReducer, - Reducer, - useEffect, - useState, - useCallback, -} from 'react'; -import { logging, t } from '@superset-ui/core'; -import { UseGetDatasetsList } from 'src/views/CRUD/data/hooks'; -import { addDangerToast } from 'src/components/MessageToasts/actions'; +import React, { useReducer, Reducer, useEffect, useState } from 'react'; +import { useGetDatasetsList } from 'src/views/CRUD/data/hooks'; import Header from './Header'; import EditPage from './EditDataset'; import DatasetPanel from './DatasetPanel'; @@ -83,36 +75,24 @@ export default function AddDataset() { Reducer | null, DSReducerActionType> >(datasetReducer, null); const [hasColumns, setHasColumns] = useState(false); - const [datasets, setDatasets] = useState([]); const [editPageIsVisible, setEditPageIsVisible] = useState(false); - const datasetNames = datasets.map(dataset => dataset.table_name); const encodedSchema = dataset?.schema ? encodeURIComponent(dataset?.schema) : undefined; - const getDatasetsList = useCallback(async () => { - if (dataset?.schema) { - const filters = [ - { col: 'database', opr: 'rel_o_m', value: dataset?.db?.id }, - { col: 'schema', opr: 'eq', value: encodedSchema }, - { col: 'sql', opr: 'dataset_is_null_or_empty', value: true }, - ]; - await UseGetDatasetsList(filters) - .then(results => { - setDatasets(results); - }) - .catch(error => { - addDangerToast(t('There was an error fetching dataset')); - logging.error(t('There was an error fetching dataset'), error); - }); - } - }, [dataset?.db?.id, dataset?.schema, encodedSchema]); + const { getDatasetsList, datasets, datasetNames } = useGetDatasetsList(); useEffect(() => { + const filters = [ + { col: 'database', opr: 'rel_o_m', value: dataset?.db?.id }, + { col: 'schema', opr: 'eq', value: encodedSchema }, + { col: 'sql', opr: 'dataset_is_null_or_empty', value: true }, + ]; + if (dataset?.schema) { - getDatasetsList(); + getDatasetsList(filters); } - }, [dataset?.schema, getDatasetsList]); + }, [dataset?.db?.id, dataset?.schema, encodedSchema, getDatasetsList]); const id = window.location.pathname.split('/')[2]; useEffect(() => { diff --git a/superset-frontend/src/views/CRUD/data/hooks.ts b/superset-frontend/src/views/CRUD/data/hooks.ts index de8cf2a964f6f..cf3c63789e448 100644 --- a/superset-frontend/src/views/CRUD/data/hooks.ts +++ b/superset-frontend/src/views/CRUD/data/hooks.ts @@ -78,45 +78,60 @@ export function useQueryPreviewState({ }; } -/** - * Retrieves all pages of dataset results - */ -export const UseGetDatasetsList = async (filters: object[]) => { - let results: DatasetObject[] = []; - let page = 0; - let count; - - // If count is undefined or less than results, we need to - // asynchronously retrieve a page of dataset results - while (count === undefined || results.length < count) { - const queryParams = rison.encode_uri({ filters, page }); - try { - // eslint-disable-next-line no-await-in-loop - const response = await SupersetClient.get({ - endpoint: `/api/v1/dataset/?q=${queryParams}`, - }); - - // Reassign local count to response's count - ({ count } = response.json); - - const { - json: { result }, - } = response; - - results = [...results, ...result]; - - page += 1; - } catch (error) { - addDangerToast(t('There was an error fetching dataset')); - logging.error(t('There was an error fetching dataset'), error); +export const useGetDatasetsList = () => { + const [datasets, setDatasets] = useState([]); + + const getDatasetsList = async (filters: object[]) => { + let results: DatasetObject[] = []; + let page = 0; + let count; + + // If count is undefined or less than results, we need to + // asynchronously retrieve a page of dataset results + while (count === undefined || results.length < count) { + const queryParams = rison.encode_uri({ filters, page }); + try { + // eslint-disable-next-line no-await-in-loop + const response = await SupersetClient.get({ + endpoint: `/api/v1/dataset/?q=${queryParams}`, + }); + + // Reassign local count to response's count + ({ count } = response.json); + + const { + json: { result }, + } = response; + + results = [...results, ...result]; + + page += 1; + } catch (error) { + addDangerToast(t('There was an error fetching dataset')); + logging.error(t('There was an error fetching dataset'), error); + } } - } - return results; + + setDatasets(results); + return results; + }; + + const datasetNames = datasets?.map(dataset => dataset.table_name); + + return { getDatasetsList, datasets, datasetNames }; }; -export const UseGetDatasetRelatedObjects = (id: string) => - SupersetClient.get({ - endpoint: `/api/v1/dataset/${id}/related_objects`, - }) - .then(({ json }) => json) - .catch(error => logging.error(error)); +export const useGetDatasetRelatedObjects = (id: string) => { + const [usageCount, setUsageCount] = useState(0); + + const getDatasetRelatedObjects = () => + SupersetClient.get({ + endpoint: `/api/v1/dataset/${id}/related_objects`, + }) + .then(({ json }) => { + setUsageCount(json?.charts.count); + }) + .catch(error => logging.error(error)); + + return { getDatasetRelatedObjects, usageCount }; +}; From 1db6c4b732722908116809b85632e545a7dee7db Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Wed, 4 Jan 2023 12:11:36 -0600 Subject: [PATCH 11/19] Add getDatasetRelatedObjects to dependency array --- .../views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx index 80c1847f52591..d6f5a6208d9f4 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx @@ -62,7 +62,7 @@ const EditPage = ({ id }: EditPageProps) => { if (id) { getDatasetRelatedObjects(); } - }, [id]); + }, [id, getDatasetRelatedObjects]); const UsageTab = ( From febe947e16695f9775a409d25d57874e6a5703e2 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Thu, 5 Jan 2023 16:24:54 -0600 Subject: [PATCH 12/19] Change casing of usageTab --- .../views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx index d6f5a6208d9f4..acdf3264ee444 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx @@ -64,7 +64,7 @@ const EditPage = ({ id }: EditPageProps) => { } }, [id, getDatasetRelatedObjects]); - const UsageTab = ( + const usageTab = ( {TRANSLATIONS.USAGE_TEXT} {usageCount > 0 && } @@ -75,7 +75,7 @@ const EditPage = ({ id }: EditPageProps) => { - + ); }; From bc436af03fb1c86ecaf54eec37ee06420ee259b7 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Tue, 17 Jan 2023 13:40:25 -0600 Subject: [PATCH 13/19] Address review comments --- .../dataset/AddDataset/EditDataset/EditDataset.test.tsx | 2 +- .../src/views/CRUD/data/dataset/AddDataset/index.tsx | 5 +++-- superset-frontend/src/views/CRUD/data/hooks.ts | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx index b65c255d255e5..c24496facc871 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/EditDataset.test.tsx @@ -30,13 +30,13 @@ const mockedProps = { fetchMock.get(DATASET_ENDPOINT, { charts: { results: [], count: 2 } }); test('should render edit dataset view with tabs', async () => { - fetchMock.calls(DATASET_ENDPOINT); render(); const columnTab = await screen.findByRole('tab', { name: /columns/i }); const metricsTab = screen.getByRole('tab', { name: /metrics/i }); const usageTab = screen.getByRole('tab', { name: /usage/i }); + expect(fetchMock.calls(DATASET_ENDPOINT)).toBeTruthy(); expect(columnTab).toBeInTheDocument(); expect(metricsTab).toBeInTheDocument(); expect(usageTab).toBeInTheDocument(); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx index f7eb66a25e2c3..f0902d76d5a60 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx @@ -17,6 +17,7 @@ * under the License. */ import React, { useReducer, Reducer, useEffect, useState } from 'react'; +import { useParams } from 'react-router-dom'; import { useGetDatasetsList } from 'src/views/CRUD/data/hooks'; import Header from './Header'; import EditPage from './EditDataset'; @@ -94,9 +95,9 @@ export default function AddDataset() { } }, [dataset?.db?.id, dataset?.schema, encodedSchema, getDatasetsList]); - const id = window.location.pathname.split('/')[2]; + const { datasetId: id } = useParams<{ datasetId: string }>(); useEffect(() => { - if (!Number.isNaN(parseFloat(id))) { + if (!Number.isNaN(parseInt(id, 10))) { setEditPageIsVisible(true); } }, [id]); diff --git a/superset-frontend/src/views/CRUD/data/hooks.ts b/superset-frontend/src/views/CRUD/data/hooks.ts index cf3c63789e448..35f7c4f800b3f 100644 --- a/superset-frontend/src/views/CRUD/data/hooks.ts +++ b/superset-frontend/src/views/CRUD/data/hooks.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useState, useEffect } from 'react'; +import { useState, useEffect, useCallback } from 'react'; import { SupersetClient, logging, t } from '@superset-ui/core'; import { addDangerToast } from 'src/components/MessageToasts/actions'; import { DatasetObject } from 'src/views/CRUD/data/dataset/AddDataset/types'; @@ -81,7 +81,7 @@ export function useQueryPreviewState({ export const useGetDatasetsList = () => { const [datasets, setDatasets] = useState([]); - const getDatasetsList = async (filters: object[]) => { + const getDatasetsList = useCallback(async (filters: object[]) => { let results: DatasetObject[] = []; let page = 0; let count; @@ -114,7 +114,7 @@ export const useGetDatasetsList = () => { setDatasets(results); return results; - }; + }, []); const datasetNames = datasets?.map(dataset => dataset.table_name); From 9723a540c62b872f98c69cbfc3ce26257d6bc2d2 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Tue, 17 Jan 2023 15:38:14 -0600 Subject: [PATCH 14/19] Fix AddDataset test by mocking useParams --- .../src/views/CRUD/data/dataset/AddDataset/AddDataset.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/AddDataset.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/AddDataset.test.tsx index 7e5a7de12ab3a..ea595c0f901a0 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/AddDataset.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/AddDataset.test.tsx @@ -26,6 +26,7 @@ jest.mock('react-router-dom', () => ({ useHistory: () => ({ push: mockHistoryPush, }), + useParams: () => ({ datasetId: undefined }), })); describe('AddDataset', () => { From 786ecd0c883d19c3c3e76bac424a598eb2de093d Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Wed, 18 Jan 2023 15:11:25 -0600 Subject: [PATCH 15/19] Fix rebase loss --- superset-frontend/src/views/CRUD/data/hooks.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/views/CRUD/data/hooks.ts b/superset-frontend/src/views/CRUD/data/hooks.ts index 35f7c4f800b3f..17512b1b1de67 100644 --- a/superset-frontend/src/views/CRUD/data/hooks.ts +++ b/superset-frontend/src/views/CRUD/data/hooks.ts @@ -131,7 +131,12 @@ export const useGetDatasetRelatedObjects = (id: string) => { .then(({ json }) => { setUsageCount(json?.charts.count); }) - .catch(error => logging.error(error)); + .catch(error => { + addDangerToast( + t(`There was an error fetching dataset's related objects`), + ); + logging.error(error); + }); return { getDatasetRelatedObjects, usageCount }; }; From bbe35c8e682c5b000c197706bbabf4720a5b2b8e Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Wed, 1 Feb 2023 10:54:40 -0600 Subject: [PATCH 16/19] Fix rebase issues --- .../CRUD/data/dataset/AddDataset/LeftPanel/LeftPanel.test.tsx | 4 ++-- .../views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/LeftPanel.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/LeftPanel.test.tsx index b6c33cad3733b..3996dc0fec5c6 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/LeftPanel.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/LeftPanel.test.tsx @@ -137,8 +137,8 @@ fetchMock.get(schemasEndpoint, { }); fetchMock.get(tablesEndpoint, { - tableLength: 3, - options: [ + count: 3, + result: [ { value: 'Sheet1', type: 'table', extra: null }, { value: 'Sheet2', type: 'table', extra: null }, { value: 'Sheet3', type: 'table', extra: null }, diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx index 07ed32973ac1b..1d1d3847a2daf 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx @@ -190,7 +190,7 @@ export default function LeftPanel({ (url: string) => { SupersetClient.get({ url }) .then(({ json }) => { - const options: TableOption[] = json.options.map((table: Table) => { + const options: TableOption[] = json.result.map((table: Table) => { const option: TableOption = { value: table.value, label: , From 8b78b6a2bff032952f55630873a4ee90406ab1b4 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Wed, 1 Feb 2023 11:14:20 -0600 Subject: [PATCH 17/19] Add function description back to useGetDatasetsList --- superset-frontend/src/views/CRUD/data/hooks.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/superset-frontend/src/views/CRUD/data/hooks.ts b/superset-frontend/src/views/CRUD/data/hooks.ts index 17512b1b1de67..1c162d7ef8ab3 100644 --- a/superset-frontend/src/views/CRUD/data/hooks.ts +++ b/superset-frontend/src/views/CRUD/data/hooks.ts @@ -78,6 +78,9 @@ export function useQueryPreviewState({ }; } +/** + * Retrieves all pages of dataset results + */ export const useGetDatasetsList = () => { const [datasets, setDatasets] = useState([]); From c6a9b0766df0321ff07e78973aeef82dc32ffbef Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Thu, 2 Feb 2023 14:27:45 -0600 Subject: [PATCH 18/19] Optimize dataset hooks and LeftColumn visibility logic --- .../dataset/AddDataset/EditDataset/index.tsx | 15 +--- .../CRUD/data/dataset/AddDataset/index.tsx | 23 ++---- .../CRUD/data/dataset/DatasetLayout/index.tsx | 8 +-- .../src/views/CRUD/data/hooks.ts | 70 ++++++++++++++----- 4 files changed, 62 insertions(+), 54 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx index acdf3264ee444..7abc676fcab5b 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/index.tsx @@ -17,8 +17,8 @@ * under the License. */ import { styled, t } from '@superset-ui/core'; -import React, { useEffect } from 'react'; -import { useGetDatasetRelatedObjects } from 'src/views/CRUD/data/hooks'; +import React from 'react'; +import { useGetDatasetRelatedCounts } from 'src/views/CRUD/data/hooks'; import Badge from 'src/components/Badge'; import Tabs from 'src/components/Tabs'; @@ -53,16 +53,7 @@ const TRANSLATIONS = { }; const EditPage = ({ id }: EditPageProps) => { - const { getDatasetRelatedObjects, usageCount } = - useGetDatasetRelatedObjects(id); - useEffect(() => { - // Todo: this useEffect should be used to call all count methods conncurently - // when we populate data for the new tabs. For right separating out this - // api call for building the usage page. - if (id) { - getDatasetRelatedObjects(); - } - }, [id, getDatasetRelatedObjects]); + const { usageCount } = useGetDatasetRelatedCounts(id); const usageTab = ( diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx index f0902d76d5a60..67b108ab366f1 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/index.tsx @@ -18,7 +18,7 @@ */ import React, { useReducer, Reducer, useEffect, useState } from 'react'; import { useParams } from 'react-router-dom'; -import { useGetDatasetsList } from 'src/views/CRUD/data/hooks'; +import { useDatasetsList } from 'src/views/CRUD/data/hooks'; import Header from './Header'; import EditPage from './EditDataset'; import DatasetPanel from './DatasetPanel'; @@ -77,23 +77,11 @@ export default function AddDataset() { >(datasetReducer, null); const [hasColumns, setHasColumns] = useState(false); const [editPageIsVisible, setEditPageIsVisible] = useState(false); - const encodedSchema = dataset?.schema - ? encodeURIComponent(dataset?.schema) - : undefined; - const { getDatasetsList, datasets, datasetNames } = useGetDatasetsList(); - - useEffect(() => { - const filters = [ - { col: 'database', opr: 'rel_o_m', value: dataset?.db?.id }, - { col: 'schema', opr: 'eq', value: encodedSchema }, - { col: 'sql', opr: 'dataset_is_null_or_empty', value: true }, - ]; - - if (dataset?.schema) { - getDatasetsList(filters); - } - }, [dataset?.db?.id, dataset?.schema, encodedSchema, getDatasetsList]); + const { datasets, datasetNames } = useDatasetsList( + dataset?.db, + dataset?.schema, + ); const { datasetId: id } = useParams<{ datasetId: string }>(); useEffect(() => { @@ -143,7 +131,6 @@ export default function AddDataset() { editPageIsVisible ? EditPageComponent() : DatasetPanelComponent() } footer={FooterComponent()} - editPageIsVisible={editPageIsVisible} /> ); } diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx index d7c335e7b39ba..7702efcb59075 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/index.tsx @@ -37,7 +37,6 @@ interface DatasetLayoutProps { datasetPanel?: ReactElement> | null; rightPanel?: ReactElement> | null; footer?: ReactElement> | null; - editPageIsVisible?: boolean; } export default function DatasetLayout({ @@ -46,17 +45,14 @@ export default function DatasetLayout({ datasetPanel, rightPanel, footer, - editPageIsVisible, }: DatasetLayoutProps) { return ( {header && {header}} - {!editPageIsVisible && ( + {leftPanel && ( - {leftPanel && ( - {leftPanel} - )} + {leftPanel} )} diff --git a/superset-frontend/src/views/CRUD/data/hooks.ts b/superset-frontend/src/views/CRUD/data/hooks.ts index 1c162d7ef8ab3..4f4d20f73b03b 100644 --- a/superset-frontend/src/views/CRUD/data/hooks.ts +++ b/superset-frontend/src/views/CRUD/data/hooks.ts @@ -18,13 +18,15 @@ */ import { useState, useEffect, useCallback } from 'react'; import { SupersetClient, logging, t } from '@superset-ui/core'; +import rison from 'rison'; import { addDangerToast } from 'src/components/MessageToasts/actions'; import { DatasetObject } from 'src/views/CRUD/data/dataset/AddDataset/types'; -import rison from 'rison'; +import { DatabaseObject } from 'src/components/DatabaseSelector'; type BaseQueryObject = { id: number; }; + export function useQueryPreviewState({ queries, fetchData, @@ -81,8 +83,16 @@ export function useQueryPreviewState({ /** * Retrieves all pages of dataset results */ -export const useGetDatasetsList = () => { +export const useDatasetsList = ( + db: + | (DatabaseObject & { + owners: [number]; + }) + | undefined, + schema: string | null | undefined, +) => { const [datasets, setDatasets] = useState([]); + const encodedSchema = schema ? encodeURIComponent(schema) : undefined; const getDatasetsList = useCallback(async (filters: object[]) => { let results: DatasetObject[] = []; @@ -119,27 +129,51 @@ export const useGetDatasetsList = () => { return results; }, []); + useEffect(() => { + const filters = [ + { col: 'database', opr: 'rel_o_m', value: db?.id }, + { col: 'schema', opr: 'eq', value: encodedSchema }, + { col: 'sql', opr: 'dataset_is_null_or_empty', value: true }, + ]; + + if (schema) { + getDatasetsList(filters); + } + }, [db?.id, schema, encodedSchema, getDatasetsList]); + const datasetNames = datasets?.map(dataset => dataset.table_name); - return { getDatasetsList, datasets, datasetNames }; + return { datasets, datasetNames }; }; -export const useGetDatasetRelatedObjects = (id: string) => { +export const useGetDatasetRelatedCounts = (id: string) => { const [usageCount, setUsageCount] = useState(0); - const getDatasetRelatedObjects = () => - SupersetClient.get({ - endpoint: `/api/v1/dataset/${id}/related_objects`, - }) - .then(({ json }) => { - setUsageCount(json?.charts.count); + const getDatasetRelatedObjects = useCallback( + () => + SupersetClient.get({ + endpoint: `/api/v1/dataset/${id}/related_objects`, }) - .catch(error => { - addDangerToast( - t(`There was an error fetching dataset's related objects`), - ); - logging.error(error); - }); - - return { getDatasetRelatedObjects, usageCount }; + .then(({ json }) => { + setUsageCount(json?.charts.count); + }) + .catch(error => { + addDangerToast( + t(`There was an error fetching dataset's related objects`), + ); + logging.error(error); + }), + [id], + ); + + useEffect(() => { + // Todo: this useEffect should be used to call all count methods conncurently + // when we populate data for the new tabs. For right separating out this + // api call for building the usage page. + if (id) { + getDatasetRelatedObjects(); + } + }, [id, getDatasetRelatedObjects]); + + return { usageCount }; }; From 57ccf2c8b32a7aa34585228a6084c32010bd5b9c Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Thu, 2 Feb 2023 18:19:55 -0600 Subject: [PATCH 19/19] remove unneeded return statement --- superset-frontend/src/views/CRUD/data/hooks.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/src/views/CRUD/data/hooks.ts b/superset-frontend/src/views/CRUD/data/hooks.ts index 4f4d20f73b03b..4e7a51de35213 100644 --- a/superset-frontend/src/views/CRUD/data/hooks.ts +++ b/superset-frontend/src/views/CRUD/data/hooks.ts @@ -126,7 +126,6 @@ export const useDatasetsList = ( } setDatasets(results); - return results; }, []); useEffect(() => {