From f8960c4ac3793b827b6cd38e8880a1307fafd39b Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Thu, 5 Jan 2023 00:28:05 +0100 Subject: [PATCH 01/11] Add empty table. --- .../src/pages/ChartList/index.tsx | 7 +- superset-frontend/src/types/Chart.ts | 14 ++++ .../DatasetUsage/DatasetUsage.test.tsx | 20 +++++ .../EditDataset/DatasetUsage/index.tsx | 84 +++++++++++++++++++ .../dataset/AddDataset/EditDataset/index.tsx | 6 +- 5 files changed, 124 insertions(+), 7 deletions(-) create mode 100644 superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/DatasetUsage.test.tsx create mode 100644 superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/index.tsx diff --git a/superset-frontend/src/pages/ChartList/index.tsx b/superset-frontend/src/pages/ChartList/index.tsx index d449321686bcc..38271e470db40 100644 --- a/superset-frontend/src/pages/ChartList/index.tsx +++ b/superset-frontend/src/pages/ChartList/index.tsx @@ -57,7 +57,7 @@ import { dangerouslyGetItemDoNotUse } from 'src/utils/localStorageHelpers'; import withToasts from 'src/components/MessageToasts/withToasts'; import PropertiesModal from 'src/explore/components/PropertiesModal'; import ImportModelsModal from 'src/components/ImportModal/index'; -import Chart from 'src/types/Chart'; +import Chart, { ChartLinkedDashboard } from 'src/types/Chart'; import { Tooltip } from 'src/components/Tooltip'; import Icons from 'src/components/Icons'; import { nativeFilterGate } from 'src/dashboard/components/nativeFilters/utils'; @@ -148,11 +148,6 @@ interface ChartListProps { }; } -type ChartLinkedDashboard = { - id: number; - dashboard_title: string; -}; - const Actions = styled.div` color: ${({ theme }) => theme.colors.grayscale.base}; `; diff --git a/superset-frontend/src/types/Chart.ts b/superset-frontend/src/types/Chart.ts index df6460080b34c..76ec04f60b7a9 100644 --- a/superset-frontend/src/types/Chart.ts +++ b/superset-frontend/src/types/Chart.ts @@ -24,6 +24,11 @@ import { QueryFormData } from '@superset-ui/core'; import Owner from './Owner'; +export type ChartLinkedDashboard = { + id: number; + dashboard_title: string; +}; + export interface Chart { id: number; url: string; @@ -39,11 +44,20 @@ export interface Chart { cache_timeout: number | null; thumbnail_url?: string; owners?: Owner[]; + last_saved_at?: string; + last_saved_by?: { + id: number; + first_name: string; + last_name: string; + }; datasource_name_text?: string; form_data: { viz_type: string; }; is_managed_externally: boolean; + + // TODO: Update API spec to describe `dashboards` key + dashboards: ChartLinkedDashboard[]; } export type Slice = { diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/DatasetUsage.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/DatasetUsage.test.tsx new file mode 100644 index 0000000000000..72fb2dc728084 --- /dev/null +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/DatasetUsage.test.tsx @@ -0,0 +1,20 @@ +/** + * 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. + */ + +test.todo('Sample test'); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/index.tsx new file mode 100644 index 0000000000000..ebc2d61821171 --- /dev/null +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/index.tsx @@ -0,0 +1,84 @@ +/** + * 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 { Link } from 'react-router-dom'; +import { ensureIsArray, t } from '@superset-ui/core'; +import CrossLinks from 'src/components/ListView/CrossLinks'; +import Chart from 'src/types/Chart'; +import Table, { ColumnsType, TableSize } from 'src/components/Table'; +import { alphabeticalSort } from 'src/components/Table/sorters'; + +const DEFAULT_PAGE_SIZE = 25; +const columns: ColumnsType = [ + { + key: 'slice_name', + title: t('Chart'), + width: '320px', + render: (value, record) => {record.slice_name}, + sorter: (a, b) => alphabeticalSort('slice_name', a, b), + }, + { + key: 'owners', + title: t('Chart owners'), + width: '242px', + render: (value, record) => + record.owners + ?.map(owner => `${owner.first_name} ${owner.last_name}`) + .join(', '), + }, + { + key: 'last-modified', + dataIndex: 'changed_on_delta_humanized', + title: t('Chart last modified'), + width: '209px', + sorter: (a, b) => alphabeticalSort('changed_on_utc', a, b), + }, + { + key: 'last-modified-by', + title: t('Chart last modified by'), + width: '216px', + dataIndex: 'changed_by_name', + sorter: (a, b) => alphabeticalSort('changed_by_name', a, b), + }, + { + key: 'dashboards', + title: t('Dashboard usage'), + width: '420px', + render: (value, record) => ( + ({ + title: d.dashboard_title, + id: d.id, + }))} + /> + ), + }, +]; + +const DatasetUsage = () => ( + +); + +export default DatasetUsage; 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 7abc676fcab5b..8c23b685c4489 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 @@ -21,11 +21,13 @@ import React from 'react'; import { useGetDatasetRelatedCounts } from 'src/views/CRUD/data/hooks'; import Badge from 'src/components/Badge'; import Tabs from 'src/components/Tabs'; +import DatasetUsage from './DatasetUsage'; const StyledTabs = styled(Tabs)` ${({ theme }) => ` margin-top: ${theme.gridUnit * 8.5}px; padding-left: ${theme.gridUnit * 4}px; + padding-right: ${theme.gridUnit * 4}px; .ant-tabs-top > .ant-tabs-nav::before { width: ${theme.gridUnit * 50}px; @@ -66,7 +68,9 @@ const EditPage = ({ id }: EditPageProps) => { - + + + ); }; From cf5de3f024b857a3850c63be7bf73297f22cd189 Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Thu, 5 Jan 2023 17:21:37 +0100 Subject: [PATCH 02/11] Add empty state. --- .../DatasetUsage/DatasetUsage.test.tsx | 19 ++++- .../EditDataset/DatasetUsage/index.tsx | 83 +++++++++++++++++-- .../dataset/AddDataset/EditDataset/index.tsx | 2 +- 3 files changed, 93 insertions(+), 11 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/DatasetUsage.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/DatasetUsage.test.tsx index 72fb2dc728084..b5c6f28a0d9a2 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/DatasetUsage.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/DatasetUsage.test.tsx @@ -17,4 +17,21 @@ * under the License. */ -test.todo('Sample test'); +import React from 'react'; +import { render, screen } from 'spec/helpers/testing-library'; +import DatasetUsage from '.'; + +const sampleDatasetId = '1'; + +test('shows empty state', () => { + render(); + expect(screen.getByText(/no charts/i)).toBeVisible(); + expect( + screen.getByText(/this dataset is not used to power any charts\./i), + ).toBeVisible(); +}); + +test.todo('shows loading state'); +test.todo('shows error state'); +test.todo('shows single-page results'); +test.todo('shows multi-page results and paginates'); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/index.tsx index ebc2d61821171..951e90f8f8528 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/index.tsx @@ -17,13 +17,26 @@ * under the License. */ -import React from 'react'; +import React, { useCallback } from 'react'; import { Link } from 'react-router-dom'; -import { ensureIsArray, t } from '@superset-ui/core'; +import { + css, + ensureIsArray, + styled, + SupersetTheme, + t, +} from '@superset-ui/core'; import CrossLinks from 'src/components/ListView/CrossLinks'; import Chart from 'src/types/Chart'; import Table, { ColumnsType, TableSize } from 'src/components/Table'; import { alphabeticalSort } from 'src/components/Table/sorters'; +import { EmptyStateBig } from 'src/components/EmptyState'; +import ChartImage from 'src/assets/images/chart.svg'; +import Icons from 'src/components/Icons'; + +interface DatasetUsageProps { + datasetId: string; +} const DEFAULT_PAGE_SIZE = 25; const columns: ColumnsType = [ @@ -72,13 +85,65 @@ const columns: ColumnsType = [ }, ]; -const DatasetUsage = () => ( -
+const emptyStateTableCSS = (theme: SupersetTheme) => css` + && th.ant-table-cell { + color: ${theme.colors.grayscale.light1}; + } + + .ant-table-placeholder { + display: none; + } +`; + +const emptyStateButtonText = ( + <> + .anticon { + line-height: 0; + } + `} + /> + {t('Create chart with dataset')} + ); +const StyledEmptyStateBig = styled(EmptyStateBig)` + margin: ${({ theme }) => 13 * theme.gridUnit}px 0; +`; + +const data: Chart[] = []; + +const DatasetUsage = ({ datasetId }: DatasetUsageProps) => { + const emptyStateButtonAction = useCallback( + () => + window.open( + `/explore/?dataset_type=table&dataset_id=${datasetId}`, + '_blank', + ), + [datasetId], + ); + + return ( +
+
+ {!data.length ? ( + } + title={t('No charts')} + description={t('This dataset is not used to power any charts.')} + buttonText={emptyStateButtonText} + buttonAction={emptyStateButtonAction} + /> + ) : null} + + ); +}; + export default DatasetUsage; 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 8c23b685c4489..45ec900c485aa 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 @@ -69,7 +69,7 @@ const EditPage = ({ id }: EditPageProps) => { - + ); From 079cc58a2cfaad4f6f9ab16a156b2e6bdb2d528a Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Mon, 9 Jan 2023 23:53:19 +0100 Subject: [PATCH 03/11] Rename to UsageTab, finish tests for empty state. --- .../DatasetUsage/DatasetUsage.test.tsx | 37 ------- .../EditDataset/UsageTab/UsageTab.test.tsx | 85 +++++++++++++++ .../{DatasetUsage => UsageTab}/index.tsx | 103 ++++++++++++++++-- .../dataset/AddDataset/EditDataset/index.tsx | 4 +- 4 files changed, 181 insertions(+), 48 deletions(-) delete mode 100644 superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/DatasetUsage.test.tsx create mode 100644 superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx rename superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/{DatasetUsage => UsageTab}/index.tsx (58%) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/DatasetUsage.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/DatasetUsage.test.tsx deleted file mode 100644 index b5c6f28a0d9a2..0000000000000 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/DatasetUsage.test.tsx +++ /dev/null @@ -1,37 +0,0 @@ -/** - * 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 { render, screen } from 'spec/helpers/testing-library'; -import DatasetUsage from '.'; - -const sampleDatasetId = '1'; - -test('shows empty state', () => { - render(); - expect(screen.getByText(/no charts/i)).toBeVisible(); - expect( - screen.getByText(/this dataset is not used to power any charts\./i), - ).toBeVisible(); -}); - -test.todo('shows loading state'); -test.todo('shows error state'); -test.todo('shows single-page results'); -test.todo('shows multi-page results and paginates'); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx new file mode 100644 index 0000000000000..59404bebc5abf --- /dev/null +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx @@ -0,0 +1,85 @@ +/** + * 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 ToastContainer from 'src/components/MessageToasts/ToastContainer'; +import DatasetUsage from '.'; + +const datasetId = '1'; +const emptyChartResponse = { count: 0, result: [] }; + +const mockChartsFetch = (response: any) => { + fetchMock.reset(); + fetchMock.get('glob:*/api/v1/chart/_info?*', { + permissions: ['can_export', 'can_read', 'can_write'], + }); + + fetchMock.get('glob:*/api/v1/chart/?*', response); +}; + +const renderDatasetUsage = () => + render( + <> + + + , + { useRedux: true }, + ); + +test('shows loading state', () => { + mockChartsFetch( + new Promise(resolve => setTimeout(() => resolve(emptyChartResponse), 250)), + ); + renderDatasetUsage(); + + const loadingIndicator = screen.getByRole('status', { + name: /loading/i, + }); + + expect(loadingIndicator).toBeVisible(); +}); + +test('shows error state', async () => { + mockChartsFetch(500); + renderDatasetUsage(); + + const errorMessage = await screen.findByText( + /an error occurred while fetching charts/i, + ); + + expect(errorMessage).toBeInTheDocument(); +}); + +test('shows empty state', () => { + mockChartsFetch(emptyChartResponse); + renderDatasetUsage(); + + const noChartsTitle = screen.getByText(/no charts/i); + const noChartsDescription = screen.getByText( + /this dataset is not used to power any charts\./i, + ); + + expect(noChartsTitle).toBeVisible(); + expect(noChartsDescription).toBeVisible(); +}); + +test.todo('shows and sorts single-page results'); +test.todo('shows and sorts multi-page results and paginates'); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx similarity index 58% rename from superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/index.tsx rename to superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx index 951e90f8f8528..0ca56651683d2 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/DatasetUsage/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx @@ -17,7 +17,7 @@ * under the License. */ -import React, { useCallback } from 'react'; +import React, { useCallback, useEffect, useMemo } from 'react'; import { Link } from 'react-router-dom'; import { css, @@ -28,11 +28,19 @@ import { } from '@superset-ui/core'; import CrossLinks from 'src/components/ListView/CrossLinks'; import Chart from 'src/types/Chart'; -import Table, { ColumnsType, TableSize } from 'src/components/Table'; +import Table, { + ColumnsType, + TableSize, + OnChangeFunction, +} from 'src/components/Table'; import { alphabeticalSort } from 'src/components/Table/sorters'; import { EmptyStateBig } from 'src/components/EmptyState'; import ChartImage from 'src/assets/images/chart.svg'; import Icons from 'src/components/Icons'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; +import { useListViewResource } from 'src/views/CRUD/hooks'; +import { FilterOperator } from 'src/components/ListView'; +import moment from 'moment'; interface DatasetUsageProps { datasetId: string; @@ -57,17 +65,18 @@ const columns: ColumnsType = [ .join(', '), }, { - key: 'last-modified', - dataIndex: 'changed_on_delta_humanized', + key: 'last_saved_at', title: t('Chart last modified'), width: '209px', - sorter: (a, b) => alphabeticalSort('changed_on_utc', a, b), + render: (value, record) => + record.last_saved_at ?? moment.utc(record.last_saved_at).fromNow(), + sorter: (a, b) => alphabeticalSort('last_saved_at', a, b), }, { - key: 'last-modified-by', + key: 'changed_by_name', + dataIndex: 'changed_by_name', title: t('Chart last modified by'), width: '216px', - dataIndex: 'changed_by_name', sorter: (a, b) => alphabeticalSort('changed_by_name', a, b), }, { @@ -113,9 +122,82 @@ const StyledEmptyStateBig = styled(EmptyStateBig)` margin: ${({ theme }) => 13 * theme.gridUnit}px 0; `; -const data: Chart[] = []; +/** + * Hook that uses the useListViewResource hook to retrieve records + * based on pagination state. + */ +const useDatasetChartRecords = (datasetId: string) => { + const { addDangerToast } = useToasts(); + + // Always filters charts by dataset + const baseFilters = useMemo( + () => [ + { + id: 'datasource_id', + operator: FilterOperator.equals, + value: datasetId, + }, + ], + [datasetId], + ); + + // Returns request status/results and function for re-fetching + const { + state: { loading, resourceCount, resourceCollection }, + fetchData, + } = useListViewResource( + 'chart', + t('chart'), + addDangerToast, + true, + [], + baseFilters, + ); + + // Adds `key` field + const resourceCollectionWithKey = useMemo( + () => resourceCollection.map(o => ({ ...o, key: o.id })), + [resourceCollection], + ); + + // Called by table with updated table state to fetch new data + const onChange: OnChangeFunction = useCallback( + (tablePagination, tableFilters, tableSorter) => { + const pageIndex = tablePagination.current ?? 0; + const pageSize = tablePagination.pageSize ?? 0; + const sortBy = ensureIsArray(tableSorter) + .filter(({ columnKey }) => typeof columnKey === 'string') + .map(({ columnKey, order }) => ({ + id: columnKey as string, + desc: order === 'descend', + })); + fetchData({ pageIndex, pageSize, sortBy, filters: [] }); + }, + [fetchData], + ); + + // Initial data request + useEffect(() => { + fetchData({ + pageIndex: 0, + pageSize: DEFAULT_PAGE_SIZE, + sortBy: [{ id: 'changed_on_utc', desc: true }], + filters: [], + }); + }, [fetchData]); + + return { + loading, + recordCount: resourceCount, + data: resourceCollectionWithKey, + onChange, + }; +}; const DatasetUsage = ({ datasetId }: DatasetUsageProps) => { + const { loading, recordCount, data, onChange } = + useDatasetChartRecords(datasetId); + const emptyStateButtonAction = useCallback( () => window.open( @@ -132,8 +214,11 @@ const DatasetUsage = ({ datasetId }: DatasetUsageProps) => { data={data} size={TableSize.MIDDLE} defaultPageSize={DEFAULT_PAGE_SIZE} + recordCount={recordCount} + loading={loading} + onChange={onChange} /> - {!data.length ? ( + {!data.length && !loading ? ( } title={t('No charts')} 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 45ec900c485aa..e8853cf043b19 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 @@ -21,7 +21,7 @@ import React from 'react'; import { useGetDatasetRelatedCounts } from 'src/views/CRUD/data/hooks'; import Badge from 'src/components/Badge'; import Tabs from 'src/components/Tabs'; -import DatasetUsage from './DatasetUsage'; +import UsageTab from './UsageTab'; const StyledTabs = styled(Tabs)` ${({ theme }) => ` @@ -69,7 +69,7 @@ const EditPage = ({ id }: EditPageProps) => { - + ); From afd559030b70387cb9f6258df7ae388559a6423d Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Tue, 10 Jan 2023 00:03:20 +0100 Subject: [PATCH 04/11] Fix tests. --- .../dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx | 4 ++-- .../data/dataset/AddDataset/EditDataset/UsageTab/index.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx index 59404bebc5abf..466ed1fdd5d1c 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx @@ -68,11 +68,11 @@ test('shows error state', async () => { expect(errorMessage).toBeInTheDocument(); }); -test('shows empty state', () => { +test('shows empty state', async () => { mockChartsFetch(emptyChartResponse); renderDatasetUsage(); - const noChartsTitle = screen.getByText(/no charts/i); + const noChartsTitle = await screen.findByText(/no charts/i); const noChartsDescription = screen.getByText( /this dataset is not used to power any charts\./i, ); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx index 0ca56651683d2..d761081fbe16f 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx @@ -181,7 +181,7 @@ const useDatasetChartRecords = (datasetId: string) => { fetchData({ pageIndex: 0, pageSize: DEFAULT_PAGE_SIZE, - sortBy: [{ id: 'changed_on_utc', desc: true }], + sortBy: [{ id: 'last_saved_at', desc: true }], filters: [], }); }, [fetchData]); From 5cd0ed80d6220978b406383f72be86b9f6ed5c68 Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Tue, 10 Jan 2023 14:55:18 +0100 Subject: [PATCH 05/11] Add more tests, fix pagination. --- superset-frontend/spec/fixtures/mockCharts.ts | 55 +++ .../EditDataset/UsageTab/UsageTab.test.tsx | 327 +++++++++++++++++- .../AddDataset/EditDataset/UsageTab/index.tsx | 19 +- 3 files changed, 383 insertions(+), 18 deletions(-) create mode 100644 superset-frontend/spec/fixtures/mockCharts.ts diff --git a/superset-frontend/spec/fixtures/mockCharts.ts b/superset-frontend/spec/fixtures/mockCharts.ts new file mode 100644 index 0000000000000..570d52cdc0612 --- /dev/null +++ b/superset-frontend/spec/fixtures/mockCharts.ts @@ -0,0 +1,55 @@ +/** + * 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. + */ + +export interface ChartListChart { + id: number; + slice_name: string; + url: string; + last_saved_at: null | string; + last_saved_by: null | { id: number; first_name: string; last_name: string }; + owners: { + id: number; + first_name: string; + last_name: string; + username: string; + }[]; + dashboards: { id: number; dashboard_title: string }[]; +} + +const CHART_ID = 1; +const MOCK_CHART: ChartListChart = { + id: CHART_ID, + slice_name: 'Sample chart', + url: `/explore/?slice_id=${CHART_ID}`, + last_saved_at: null, + dashboards: [], + last_saved_by: null, + owners: [], +}; + +/** + * Get mock charts as would be returned by the /api/v1/chart list endpoint. + */ +export const getMockChart = ( + overrides: Partial = {}, +): ChartListChart => ({ + ...MOCK_CHART, + ...(overrides.id ? { url: `/explore/?slice_id=${overrides.id}` } : null), + ...overrides, +}); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx index 466ed1fdd5d1c..a07e42ad4e369 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx @@ -19,35 +19,84 @@ import React from 'react'; import fetchMock from 'fetch-mock'; -import { render, screen } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; +import { render, screen, waitFor } from 'spec/helpers/testing-library'; +import { ChartListChart, getMockChart } from 'spec/fixtures/mockCharts'; import ToastContainer from 'src/components/MessageToasts/ToastContainer'; import DatasetUsage from '.'; -const datasetId = '1'; -const emptyChartResponse = { count: 0, result: [] }; +const DEFAULT_DATASET_ID = '1'; +const DEFAULT_ORDER_COLUMN = 'last_saved_at'; +const DEFAULT_ORDER_DIRECTION = 'desc'; +const DEFAULT_PAGE = 0; +const DEFAULT_PAGE_SIZE = 25; +const getChartResponse = (result: ChartListChart[]) => ({ + count: result.length, + result, +}); + +const CHARTS_ENDPOINT = 'glob:*/api/v1/chart/?*'; const mockChartsFetch = (response: any) => { fetchMock.reset(); fetchMock.get('glob:*/api/v1/chart/_info?*', { permissions: ['can_export', 'can_read', 'can_write'], }); - fetchMock.get('glob:*/api/v1/chart/?*', response); + fetchMock.get(CHARTS_ENDPOINT, response); }; const renderDatasetUsage = () => render( <> - + , - { useRedux: true }, + { useRedux: true, useRouter: true }, + ); + +const expectLastChartRequest = (params?: { + datasetId?: string; + orderColumn?: string; + orderDirection?: 'desc' | 'asc'; + page?: number; + pageSize?: number; +}) => { + const { datasetId, orderColumn, orderDirection, page, pageSize } = { + datasetId: DEFAULT_DATASET_ID, + orderColumn: DEFAULT_ORDER_COLUMN, + orderDirection: DEFAULT_ORDER_DIRECTION, + page: DEFAULT_PAGE, + pageSize: DEFAULT_PAGE_SIZE, + ...(params || {}), + }; + + const calls = fetchMock.calls(CHARTS_ENDPOINT); + expect(calls.length).toBeGreaterThan(0); + const lastChartRequestUrl = calls[calls.length - 1][0]; + expect(lastChartRequestUrl).toMatch( + new RegExp(`col:datasource_id,opr:eq,value:%27${datasetId}%27`), ); + expect(lastChartRequestUrl).toMatch( + new RegExp(`order_column:${orderColumn}`), + ); + + expect(lastChartRequestUrl).toMatch( + new RegExp(`order_direction:${orderDirection}`), + ); + + expect(lastChartRequestUrl).toMatch(new RegExp(`page:${page}`)); + expect(lastChartRequestUrl).toMatch(new RegExp(`page_size:${pageSize}`)); +}; + test('shows loading state', () => { mockChartsFetch( - new Promise(resolve => setTimeout(() => resolve(emptyChartResponse), 250)), + new Promise(resolve => + setTimeout(() => resolve(getChartResponse([])), 250), + ), ); + renderDatasetUsage(); const loadingIndicator = screen.getByRole('status', { @@ -69,7 +118,7 @@ test('shows error state', async () => { }); test('shows empty state', async () => { - mockChartsFetch(emptyChartResponse); + mockChartsFetch(getChartResponse([])); renderDatasetUsage(); const noChartsTitle = await screen.findByText(/no charts/i); @@ -79,7 +128,265 @@ test('shows empty state', async () => { expect(noChartsTitle).toBeVisible(); expect(noChartsDescription).toBeVisible(); + expect(fetchMock.calls(CHARTS_ENDPOINT)).toHaveLength(1); + expectLastChartRequest(); +}); + +test('show and sort by chart title', async () => { + mockChartsFetch( + getChartResponse([ + getMockChart({ id: 1, slice_name: 'Sample A' }), + getMockChart({ id: 2, slice_name: 'Sample C' }), + getMockChart({ id: 3, slice_name: 'Sample B' }), + ]), + ); + + renderDatasetUsage(); + + const chartNameColumnHeader = screen.getByText('Chart'); + const chartNameLinks = await screen.findAllByRole('link', { + name: /sample/i, + }); + + // Default sort + expect(chartNameLinks).toHaveLength(3); + expect(chartNameLinks[0]).toHaveTextContent('Sample A'); + expect(chartNameLinks[0]).toHaveAttribute('href', '/explore/?slice_id=1'); + expect(chartNameLinks[1]).toHaveTextContent('Sample C'); + expect(chartNameLinks[1]).toHaveAttribute('href', '/explore/?slice_id=2'); + expect(chartNameLinks[2]).toHaveTextContent('Sample B'); + expect(chartNameLinks[2]).toHaveAttribute('href', '/explore/?slice_id=3'); + expectLastChartRequest(); + + // Sort by name ascending + userEvent.click(chartNameColumnHeader); + waitFor(() => { + expectLastChartRequest({ + orderColumn: 'slice_name', + orderDirection: 'asc', + }); + }); + + // Sort by name descending + userEvent.click(chartNameColumnHeader); + waitFor(() => { + expectLastChartRequest({ + orderColumn: 'slice_name', + orderDirection: 'desc', + }); + }); +}); + +test('show chart owners', async () => { + mockChartsFetch( + getChartResponse([ + getMockChart({ + id: 1, + owners: [ + { id: 1, first_name: 'John', last_name: 'Doe', username: 'j1' }, + { id: 2, first_name: 'Jane', last_name: 'Doe', username: 'j2' }, + ], + }), + getMockChart({ id: 2 }), + getMockChart({ + id: 3, + owners: [ + { id: 3, first_name: 'John', last_name: 'Doe', username: 'j1' }, + ], + }), + ]), + ); + + renderDatasetUsage(); + + const chartOwners = await screen.findAllByText(/doe/i); + + expect(chartOwners).toHaveLength(2); + expect(chartOwners[0]).toHaveTextContent('John Doe, Jane Doe'); + expect(chartOwners[1]).toHaveTextContent('John Doe'); + expectLastChartRequest(); +}); + +const getDate = (msAgo: number) => { + const date = new Date(); + date.setMilliseconds(date.getMilliseconds() - msAgo); + return date; +}; + +test('show and sort by chart last modified', async () => { + mockChartsFetch( + getChartResponse([ + getMockChart({ id: 2, last_saved_at: getDate(10000).toISOString() }), + getMockChart({ id: 1, last_saved_at: getDate(1000000).toISOString() }), + getMockChart({ id: 3, last_saved_at: getDate(100000000).toISOString() }), + ]), + ); + + renderDatasetUsage(); + + const chartLastModifiedColumnHeader = screen.getByText('Chart last modified'); + const chartLastModifiedValues = await screen.findAllByText( + /a few seconds ago|17 minutes ago|a day ago/i, + ); + + // Default sort + expect(chartLastModifiedValues).toHaveLength(3); + expect(chartLastModifiedValues[0]).toHaveTextContent('a few seconds ago'); + expect(chartLastModifiedValues[1]).toHaveTextContent('17 minutes ago'); + expect(chartLastModifiedValues[2]).toHaveTextContent('a day ago'); + expectLastChartRequest(); + + // Sort by last modified ascending + userEvent.click(chartLastModifiedColumnHeader); + waitFor(() => { + expectLastChartRequest({ orderDirection: 'asc' }); + }); + + // Sort by last modified descending + userEvent.click(chartLastModifiedColumnHeader); + waitFor(() => { + expectLastChartRequest({ orderDirection: 'desc' }); + }); +}); + +test('show and sort by chart last modified by', async () => { + mockChartsFetch( + getChartResponse([ + getMockChart({ + id: 2, + last_saved_by: { id: 1, first_name: 'John', last_name: 'Doe' }, + }), + getMockChart({ + id: 1, + last_saved_by: null, + }), + getMockChart({ + id: 3, + last_saved_by: { id: 2, first_name: 'Jane', last_name: 'Doe' }, + }), + ]), + ); + + renderDatasetUsage(); + + const chartLastModifiedByColumnHeader = screen.getByText( + 'Chart last modified by', + ); + + const chartLastModifiedByValues = await screen.findAllByText(/doe/i); + + // Default sort + expect(chartLastModifiedByValues).toHaveLength(2); + expect(chartLastModifiedByValues[0]).toHaveTextContent('John Doe'); + expect(chartLastModifiedByValues[1]).toHaveTextContent('Jane Doe'); + expectLastChartRequest(); + + // Sort by last modified ascending + userEvent.click(chartLastModifiedByColumnHeader); + waitFor(() => { + expectLastChartRequest({ orderDirection: 'asc' }); + }); + + // Sort by last modified descending + userEvent.click(chartLastModifiedByColumnHeader); + waitFor(() => { + expectLastChartRequest({ orderDirection: 'desc' }); + }); +}); + +test('show chart dashboards', async () => { + mockChartsFetch( + getChartResponse([ + getMockChart({ + id: 1, + dashboards: [ + { id: 1, dashboard_title: 'Sample dashboard A' }, + { id: 2, dashboard_title: 'Sample dashboard B' }, + ], + }), + getMockChart({ id: 2 }), + getMockChart({ + id: 3, + dashboards: [{ id: 3, dashboard_title: 'Sample dashboard C' }], + }), + ]), + ); + + renderDatasetUsage(); + + const chartDashboards = await screen.findAllByRole('link', { + name: /sample dashboard/i, + }); + + expect(chartDashboards).toHaveLength(3); + expect(chartDashboards[0]).toHaveTextContent('Sample dashboard A'); + expect(chartDashboards[0]).toHaveAttribute('href', '/superset/dashboard/1'); + expect(chartDashboards[1]).toHaveTextContent('Sample dashboard B'); + expect(chartDashboards[1]).toHaveAttribute('href', '/superset/dashboard/2'); + expect(chartDashboards[0].parentNode).toBe(chartDashboards[1].parentNode); + expect(chartDashboards[2]).toHaveTextContent('Sample dashboard C'); + expect(chartDashboards[2]).toHaveAttribute('href', '/superset/dashboard/3'); + expect(chartDashboards[2].parentNode).not.toBe(chartDashboards[0].parentNode); + expect(chartDashboards[2].parentNode).not.toBe(chartDashboards[1].parentNode); + expectLastChartRequest(); + + expect( + screen.queryByRole('button', { + name: /right/i, + }), + ).not.toBeInTheDocument(); }); -test.todo('shows and sorts single-page results'); -test.todo('shows and sorts multi-page results and paginates'); +test('paginates', async () => { + const charts = []; + for (let i = 0; i < 65; i += 1) { + charts.push( + getMockChart({ + id: i + 1, + slice_name: `Sample chart ${i + 1}`, + }), + ); + } + + mockChartsFetch(getChartResponse(charts)); + renderDatasetUsage(); + + // First page + let chartNameValues = await screen.findAllByRole('cell', { + name: /sample chart/i, + }); + + expect(chartNameValues).toHaveLength(25); + expect(chartNameValues[0]).toHaveTextContent('Sample chart 1'); + expect(chartNameValues[24]).toHaveTextContent('Sample chart 25'); + + // Second page + userEvent.click( + screen.getByRole('button', { + name: /right/i, + }), + ); + + chartNameValues = await screen.findAllByRole('cell', { + name: /sample chart/i, + }); + + expect(chartNameValues).toHaveLength(25); + expect(chartNameValues[0]).toHaveTextContent('Sample chart 26'); + expect(chartNameValues[24]).toHaveTextContent('Sample chart 50'); + + // Third page + userEvent.click( + screen.getByRole('button', { + name: /right/i, + }), + ); + + chartNameValues = await screen.findAllByRole('cell', { + name: /sample chart/i, + }); + + expect(chartNameValues).toHaveLength(15); + expect(chartNameValues[0]).toHaveTextContent('Sample chart 51'); + expect(chartNameValues[14]).toHaveTextContent('Sample chart 65'); +}); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx index d761081fbe16f..743354359ad59 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx @@ -33,7 +33,6 @@ import Table, { TableSize, OnChangeFunction, } from 'src/components/Table'; -import { alphabeticalSort } from 'src/components/Table/sorters'; import { EmptyStateBig } from 'src/components/EmptyState'; import ChartImage from 'src/assets/images/chart.svg'; import Icons from 'src/components/Icons'; @@ -52,8 +51,8 @@ const columns: ColumnsType = [ key: 'slice_name', title: t('Chart'), width: '320px', + sorter: true, render: (value, record) => {record.slice_name}, - sorter: (a, b) => alphabeticalSort('slice_name', a, b), }, { key: 'owners', @@ -68,16 +67,20 @@ const columns: ColumnsType = [ key: 'last_saved_at', title: t('Chart last modified'), width: '209px', + sorter: true, + defaultSortOrder: 'descend', render: (value, record) => - record.last_saved_at ?? moment.utc(record.last_saved_at).fromNow(), - sorter: (a, b) => alphabeticalSort('last_saved_at', a, b), + record.last_saved_at ? moment.utc(record.last_saved_at).fromNow() : null, }, { - key: 'changed_by_name', - dataIndex: 'changed_by_name', + key: 'last_saved_by.first_name', title: t('Chart last modified by'), width: '216px', - sorter: (a, b) => alphabeticalSort('changed_by_name', a, b), + sorter: true, + render: (value, record) => + record.last_saved_by + ? `${record.last_saved_by.first_name} ${record.last_saved_by.last_name}` + : null, }, { key: 'dashboards', @@ -163,7 +166,7 @@ const useDatasetChartRecords = (datasetId: string) => { // Called by table with updated table state to fetch new data const onChange: OnChangeFunction = useCallback( (tablePagination, tableFilters, tableSorter) => { - const pageIndex = tablePagination.current ?? 0; + const pageIndex = (tablePagination.current ?? 1) - 1; const pageSize = tablePagination.pageSize ?? 0; const sortBy = ensureIsArray(tableSorter) .filter(({ columnKey }) => typeof columnKey === 'string') From c5b3c5525a668252b749011d1051ad1ef31001c8 Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Tue, 10 Jan 2023 21:23:39 +0100 Subject: [PATCH 06/11] Add & test truncation. --- .../src/components/TruncatedList/index.tsx | 167 ++++++++++++++++++ .../EditDataset/UsageTab/UsageTab.test.tsx | 10 +- .../AddDataset/EditDataset/UsageTab/index.tsx | 16 +- 3 files changed, 185 insertions(+), 8 deletions(-) create mode 100644 superset-frontend/src/components/TruncatedList/index.tsx diff --git a/superset-frontend/src/components/TruncatedList/index.tsx b/superset-frontend/src/components/TruncatedList/index.tsx new file mode 100644 index 0000000000000..c8b0337981dc4 --- /dev/null +++ b/superset-frontend/src/components/TruncatedList/index.tsx @@ -0,0 +1,167 @@ +/** + * 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, { ReactNode, useEffect, useMemo, useRef, useState } from 'react'; +import { styled, t } from '@superset-ui/core'; +import { useTruncation } from 'src/hooks/useTruncation'; +import { Tooltip } from '../Tooltip'; + +export type TruncatedListProps = { + /** + * Array of input items of type `ListItemType`. + */ + items: ListItemType[]; + + /** + * Renderer for items not overflowed into the tooltip. + * Required if `ListItemType` is not renderable by React. + */ + renderVisibleItem?: (item: ListItemType) => ReactNode; + + /** + * Renderer for items not overflowed into the tooltip. + * Required if `ListItemType` is not renderable by React. + */ + renderTooltipItem?: (item: ListItemType) => ReactNode; + + /** + * Returns the React key for an item. + */ + getKey?: (item: ListItemType) => React.Key; + + /** + * The max number of links that should appear in the tooltip. + */ + maxLinks?: number; +}; + +const StyledTruncatedList = styled.div` + & > span { + width: 100%; + display: flex; + + .ant-tooltip-open { + display: inline; + } + } +`; + +const StyledVisibleItems = styled.span` + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + display: inline-block; + width: 100%; + vertical-align: bottom; +`; + +const StyledVisibleItem = styled.span` + &:not(:last-child)::after { + content: ', '; + } +`; + +const StyledTooltipItem = styled.div` + .link { + color: ${({ theme }) => theme.colors.grayscale.light5}; + display: block; + text-decoration: underline; + } +`; + +const StyledPlus = styled.span` + ${({ theme }) => ` + cursor: pointer; + color: ${theme.colors.primary.dark1}; + font-weight: ${theme.typography.weights.normal}; + `} +`; + +export default function TruncatedList({ + items, + renderVisibleItem = item => item, + renderTooltipItem = item => item, + getKey = item => item as React.Key, + maxLinks = 20, +}: TruncatedListProps) { + const itemsNotInTooltipRef = useRef(null); + const plusRef = useRef(null); + const [elementsTruncated, hasHiddenElements] = useTruncation( + itemsNotInTooltipRef, + plusRef, + ) as [number, boolean]; + + const nMoreItems = useMemo( + () => (items.length > maxLinks ? items.length - maxLinks : undefined), + [items, maxLinks], + ); + + const itemsNotInTooltip = useMemo( + () => ( + + {items.map(item => ( + + {renderVisibleItem(item)} + + ))} + + ), + [getKey, items, renderVisibleItem], + ); + + const itemsInTooltip = useMemo( + () => + items + .slice(0, maxLinks) + .map(item => ( + + {renderTooltipItem(item)} + + )), + [getKey, items, maxLinks, renderTooltipItem], + ); + + const [timesRendered, setTimesRendered] = useState(0); + useEffect(() => { + if (timesRendered < 3) { + setTimesRendered(timesRendered + 1); + } + }, [timesRendered]); + + return ( + + + {itemsInTooltip} + {nMoreItems && {t('+ %s more', nMoreItems)}} + + ) : null + } + > + {itemsNotInTooltip} + {hasHiddenElements && ( + +{elementsTruncated} + )} + + + ); +} diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx index a07e42ad4e369..508a156cfcf1a 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx @@ -201,9 +201,13 @@ test('show chart owners', async () => { const chartOwners = await screen.findAllByText(/doe/i); - expect(chartOwners).toHaveLength(2); - expect(chartOwners[0]).toHaveTextContent('John Doe, Jane Doe'); - expect(chartOwners[1]).toHaveTextContent('John Doe'); + expect(chartOwners).toHaveLength(3); + expect(chartOwners[0]).toHaveTextContent('John Doe'); + expect(chartOwners[1]).toHaveTextContent('Jane Doe'); + expect(chartOwners[0].parentNode).toBe(chartOwners[1].parentNode); + expect(chartOwners[2]).toHaveTextContent('John Doe'); + expect(chartOwners[2].parentNode).not.toBe(chartOwners[0].parentNode); + expect(chartOwners[2].parentNode).not.toBe(chartOwners[1].parentNode); expectLastChartRequest(); }); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx index 743354359ad59..d223bbe46d1a2 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx @@ -40,12 +40,13 @@ import { useToasts } from 'src/components/MessageToasts/withToasts'; import { useListViewResource } from 'src/views/CRUD/hooks'; import { FilterOperator } from 'src/components/ListView'; import moment from 'moment'; +import TruncatedList from 'src/components/TruncatedList'; interface DatasetUsageProps { datasetId: string; } -const DEFAULT_PAGE_SIZE = 25; +const DEFAULT_PAGE_SIZE = 5; const columns: ColumnsType = [ { key: 'slice_name', @@ -58,10 +59,15 @@ const columns: ColumnsType = [ key: 'owners', title: t('Chart owners'), width: '242px', - render: (value, record) => - record.owners - ?.map(owner => `${owner.first_name} ${owner.last_name}`) - .join(', '), + render: (value, record) => ( + `${owner.first_name} ${owner.last_name}`, + ) ?? [] + } + /> + ), }, { key: 'last_saved_at', From a909995f8534aa2e694819f37101826185b83c9a Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Tue, 10 Jan 2023 21:56:07 +0100 Subject: [PATCH 07/11] Fix default page size. --- .../CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx index d223bbe46d1a2..66d2809279fb5 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx @@ -46,7 +46,7 @@ interface DatasetUsageProps { datasetId: string; } -const DEFAULT_PAGE_SIZE = 5; +const DEFAULT_PAGE_SIZE = 25; const columns: ColumnsType = [ { key: 'slice_name', From 16b0685316252053ab14195692d0992b2fed4b44 Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Tue, 10 Jan 2023 23:21:24 +0100 Subject: [PATCH 08/11] Change dashboard tooltip color/style. --- .../AddDataset/EditDataset/UsageTab/index.tsx | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx index 66d2809279fb5..99663d91e19dc 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/index.tsx @@ -26,8 +26,7 @@ import { SupersetTheme, t, } from '@superset-ui/core'; -import CrossLinks from 'src/components/ListView/CrossLinks'; -import Chart from 'src/types/Chart'; +import Chart, { ChartLinkedDashboard } from 'src/types/Chart'; import Table, { ColumnsType, TableSize, @@ -47,6 +46,23 @@ interface DatasetUsageProps { } const DEFAULT_PAGE_SIZE = 25; + +const getLinkProps = (dashboard: ChartLinkedDashboard) => ({ + key: dashboard.id, + to: `/superset/dashboard/${dashboard.id}`, + target: '_blank', + rel: 'noreferer noopener', + children: dashboard.dashboard_title, +}); + +const tooltipItemCSS = (theme: SupersetTheme) => css` + color: ${theme.colors.grayscale.light5}; + text-decoration: underline; + &:hover { + color: inherit; + } +`; + const columns: ColumnsType = [ { key: 'slice_name', @@ -93,11 +109,13 @@ const columns: ColumnsType = [ title: t('Dashboard usage'), width: '420px', render: (value, record) => ( - ({ - title: d.dashboard_title, - id: d.id, - }))} + + items={record.dashboards} + renderVisibleItem={dashboard => } + renderTooltipItem={dashboard => ( + + )} + getKey={dashboard => dashboard.id} /> ), }, From c1556347f59ed49860362b01067ef974a56f3b2d Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Fri, 3 Feb 2023 13:43:30 -0800 Subject: [PATCH 09/11] Fix typing, remove debug code. --- .../src/components/TruncatedList/index.tsx | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/superset-frontend/src/components/TruncatedList/index.tsx b/superset-frontend/src/components/TruncatedList/index.tsx index c8b0337981dc4..37d4fe0436564 100644 --- a/superset-frontend/src/components/TruncatedList/index.tsx +++ b/superset-frontend/src/components/TruncatedList/index.tsx @@ -17,7 +17,7 @@ * under the License. */ -import React, { ReactNode, useEffect, useMemo, useRef, useState } from 'react'; +import React, { ReactNode, useMemo, useRef } from 'react'; import { styled, t } from '@superset-ui/core'; import { useTruncation } from 'src/hooks/useTruncation'; import { Tooltip } from '../Tooltip'; @@ -35,7 +35,7 @@ export type TruncatedListProps = { renderVisibleItem?: (item: ListItemType) => ReactNode; /** - * Renderer for items not overflowed into the tooltip. + * Renderer for items that are overflowed into the tooltip. * Required if `ListItemType` is not renderable by React. */ renderTooltipItem?: (item: ListItemType) => ReactNode; @@ -93,11 +93,11 @@ const StyledPlus = styled.span` `} `; -export default function TruncatedList({ +export default function TruncatedList({ items, renderVisibleItem = item => item, renderTooltipItem = item => item, - getKey = item => item as React.Key, + getKey = item => item as unknown as React.Key, maxLinks = 20, }: TruncatedListProps) { const itemsNotInTooltipRef = useRef(null); @@ -137,13 +137,6 @@ export default function TruncatedList({ [getKey, items, maxLinks, renderTooltipItem], ); - const [timesRendered, setTimesRendered] = useState(0); - useEffect(() => { - if (timesRendered < 3) { - setTimesRendered(timesRendered + 1); - } - }, [timesRendered]); - return ( Date: Fri, 3 Feb 2023 14:44:08 -0800 Subject: [PATCH 10/11] Fix unit tests. --- .../EditDataset/UsageTab/UsageTab.test.tsx | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx index 508a156cfcf1a..60bb986a27476 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx @@ -327,11 +327,20 @@ test('show chart dashboards', async () => { expect(chartDashboards[0]).toHaveAttribute('href', '/superset/dashboard/1'); expect(chartDashboards[1]).toHaveTextContent('Sample dashboard B'); expect(chartDashboards[1]).toHaveAttribute('href', '/superset/dashboard/2'); - expect(chartDashboards[0].parentNode).toBe(chartDashboards[1].parentNode); + expect(chartDashboards[0].closest('.ant-table-cell')).toBe( + chartDashboards[1].closest('.ant-table-cell'), + ); + expect(chartDashboards[2]).toHaveTextContent('Sample dashboard C'); expect(chartDashboards[2]).toHaveAttribute('href', '/superset/dashboard/3'); - expect(chartDashboards[2].parentNode).not.toBe(chartDashboards[0].parentNode); - expect(chartDashboards[2].parentNode).not.toBe(chartDashboards[1].parentNode); + expect(chartDashboards[2].closest('.ant-table-cell')).not.toBe( + chartDashboards[0].closest('.ant-table-cell'), + ); + + expect(chartDashboards[2].closest('.ant-table-cell')).not.toBe( + chartDashboards[1].closest('.ant-table-cell'), + ); + expectLastChartRequest(); expect( From 578445fdea6a9b6402052f0204d190227868d303 Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Mon, 6 Feb 2023 15:46:41 -0800 Subject: [PATCH 11/11] Fix typing, errors in tests. --- .../AddDataset/EditDataset/UsageTab/UsageTab.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx index 60bb986a27476..7bbdbbfd52cfa 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx @@ -37,7 +37,7 @@ const getChartResponse = (result: ChartListChart[]) => ({ }); const CHARTS_ENDPOINT = 'glob:*/api/v1/chart/?*'; -const mockChartsFetch = (response: any) => { +const mockChartsFetch = (response: fetchMock.MockResponse) => { fetchMock.reset(); fetchMock.get('glob:*/api/v1/chart/_info?*', { permissions: ['can_export', 'can_read', 'can_write'], @@ -90,7 +90,7 @@ const expectLastChartRequest = (params?: { expect(lastChartRequestUrl).toMatch(new RegExp(`page_size:${pageSize}`)); }; -test('shows loading state', () => { +test('shows loading state', async () => { mockChartsFetch( new Promise(resolve => setTimeout(() => resolve(getChartResponse([])), 250), @@ -99,7 +99,7 @@ test('shows loading state', () => { renderDatasetUsage(); - const loadingIndicator = screen.getByRole('status', { + const loadingIndicator = await screen.findByRole('status', { name: /loading/i, });