From 834bb9409d5ab4028a469db85dba28ff353c4c09 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 10 Jun 2021 03:05:33 +0100 Subject: [PATCH] fix(datasets): consistent dataset list (#15014) --- .../datasource/ChangeDatasourceModal_spec.jsx | 2 +- .../src/components/TableView/TableView.tsx | 47 ++++++-- .../src/components/TableView/types.ts | 12 +- .../src/datasource/ChangeDatasourceModal.tsx | 104 ++++++++++++------ .../views/CRUD/data/dataset/DatasetList.tsx | 22 ++-- .../src/views/CRUD/data/dataset/constants.ts | 34 ++++++ 6 files changed, 158 insertions(+), 63 deletions(-) create mode 100644 superset-frontend/src/views/CRUD/data/dataset/constants.ts diff --git a/superset-frontend/spec/javascripts/datasource/ChangeDatasourceModal_spec.jsx b/superset-frontend/spec/javascripts/datasource/ChangeDatasourceModal_spec.jsx index 4e7e9346f8..77a35eba89 100644 --- a/superset-frontend/spec/javascripts/datasource/ChangeDatasourceModal_spec.jsx +++ b/superset-frontend/spec/javascripts/datasource/ChangeDatasourceModal_spec.jsx @@ -48,7 +48,7 @@ const datasourceData = { }; const DATASOURCES_ENDPOINT = - 'glob:*/api/v1/dataset/?q=(order_column:changed_on_delta_humanized,order_direction:asc,page:0,page_size:20)'; + 'glob:*/api/v1/dataset/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25)'; const DATASOURCE_ENDPOINT = `glob:*/datasource/get/${datasourceData.type}/${datasourceData.id}`; const DATASOURCE_PAYLOAD = { new: 'data' }; diff --git a/superset-frontend/src/components/TableView/TableView.tsx b/superset-frontend/src/components/TableView/TableView.tsx index bc3517f5bd..07dc18a9db 100644 --- a/superset-frontend/src/components/TableView/TableView.tsx +++ b/superset-frontend/src/components/TableView/TableView.tsx @@ -16,12 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; +import React, { useEffect } from 'react'; +import isEqual from 'lodash/isEqual'; import { styled, t } from '@superset-ui/core'; import { useFilters, usePagination, useSortBy, useTable } from 'react-table'; import { Empty } from 'src/common/components'; import { TableCollection, Pagination } from 'src/components/dataViewCommon'; -import { SortColumns } from './types'; +import { SortByType, ServerPagination } from './types'; const DEFAULT_PAGE_SIZE = 10; @@ -34,8 +35,11 @@ export interface TableViewProps { columns: any[]; data: any[]; pageSize?: number; + totalCount?: number; + serverPagination?: boolean; + onServerPagination?: (args: ServerPagination) => void; initialPageIndex?: number; - initialSortBy?: SortColumns; + initialSortBy?: SortByType; loading?: boolean; withPagination?: boolean; emptyWrapperType?: EmptyWrapperType; @@ -57,13 +61,17 @@ const TableViewStyles = styled.div<{ ${({ scrollTable, theme }) => scrollTable && ` - height: 300px; + height: 380px; margin-bottom: ${theme.gridUnit * 4}px; overflow: auto; `} - .table-cell.table-cell { - vertical-align: top; + .table-row { + height: 43px; + } + + th[role='columnheader'] { + z-index: 1; } .pagination-container { @@ -92,6 +100,7 @@ const TableView = ({ columns, data, pageSize: initialPageSize, + totalCount = data.length, initialPageIndex, initialSortBy = [], loading = false, @@ -99,6 +108,8 @@ const TableView = ({ emptyWrapperType = EmptyWrapperType.Default, noDataText, showRowCount = true, + serverPagination = false, + onServerPagination = () => {}, ...props }: TableViewProps) => { const initialState = { @@ -116,18 +127,38 @@ const TableView = ({ prepareRow, pageCount, gotoPage, - state: { pageIndex, pageSize }, + state: { pageIndex, pageSize, sortBy }, } = useTable( { columns, data, initialState, + manualPagination: serverPagination, + manualSortBy: serverPagination, + pageCount: Math.ceil(totalCount / initialState.pageSize), }, useFilters, useSortBy, usePagination, ); + useEffect(() => { + if (serverPagination && pageIndex !== initialState.pageIndex) { + onServerPagination({ + pageIndex, + }); + } + }, [pageIndex]); + + useEffect(() => { + if (serverPagination && !isEqual(sortBy, initialState.sortBy)) { + onServerPagination({ + pageIndex: 0, + sortBy, + }); + } + }, [sortBy]); + const content = withPagination ? page : rows; let EmptyWrapperComponent; @@ -182,7 +213,7 @@ const TableView = ({ '%s-%s of %s', pageSize * pageIndex + (page.length && 1), pageSize * pageIndex + page.length, - data.length, + totalCount, )} )} diff --git a/superset-frontend/src/components/TableView/types.ts b/superset-frontend/src/components/TableView/types.ts index 974cb24962..dceb27573b 100644 --- a/superset-frontend/src/components/TableView/types.ts +++ b/superset-frontend/src/components/TableView/types.ts @@ -16,9 +16,11 @@ * specific language governing permissions and limitations * under the License. */ -export interface SortColumn { - id: string; - desc?: boolean; -} +import { SortingRule } from 'react-table'; -export type SortColumns = SortColumn[]; +export type SortByType = SortingRule[]; + +export interface ServerPagination { + pageIndex: number; + sortBy?: SortByType; +} diff --git a/superset-frontend/src/datasource/ChangeDatasourceModal.tsx b/superset-frontend/src/datasource/ChangeDatasourceModal.tsx index 78dd1c2afb..d5357510d1 100644 --- a/superset-frontend/src/datasource/ChangeDatasourceModal.tsx +++ b/superset-frontend/src/datasource/ChangeDatasourceModal.tsx @@ -26,15 +26,22 @@ import React, { import Alert from 'src/components/Alert'; import { SupersetClient, t, styled } from '@superset-ui/core'; import TableView, { EmptyWrapperType } from 'src/components/TableView'; +import { ServerPagination, SortByType } from 'src/components/TableView/types'; import StyledModal from 'src/components/Modal'; import Button from 'src/components/Button'; import { useListViewResource } from 'src/views/CRUD/hooks'; import Dataset from 'src/types/Dataset'; import { useDebouncedEffect } from 'src/explore/exploreUtils'; +import { SLOW_DEBOUNCE } from 'src/constants'; import { getClientErrorObject } from 'src/utils/getClientErrorObject'; import Loading from 'src/components/Loading'; import withToasts from 'src/messageToasts/enhancers/withToasts'; import { Input, AntdInput } from 'src/common/components'; +import { + PAGE_SIZE as DATASET_PAGE_SIZE, + SORT_BY as DATASET_SORT_BY, +} from 'src/views/CRUD/data/dataset/constants'; +import FacePile from '../components/FacePile'; const CONFIRM_WARNING_MESSAGE = t( 'Warning! Changing the dataset may break the chart if the metadata does not exist.', @@ -81,21 +88,6 @@ const StyledSpan = styled.span` } `; -const TABLE_COLUMNS = [ - 'name', - 'type', - 'schema', - 'connection', - 'creator', -].map(col => ({ accessor: col, Header: col })); - -const emptyRequest = { - pageIndex: 0, - pageSize: 20, - filters: [], - sortBy: [{ id: 'changed_on_delta_humanized' }], -}; - const ChangeDatasourceModal: FunctionComponent = ({ addDangerToast, addSuccessToast, @@ -105,12 +97,14 @@ const ChangeDatasourceModal: FunctionComponent = ({ show, }) => { const [filter, setFilter] = useState(undefined); + const [pageIndex, setPageIndex] = useState(0); + const [sortBy, setSortBy] = useState(DATASET_SORT_BY); const [confirmChange, setConfirmChange] = useState(false); const [confirmedDataset, setConfirmedDataset] = useState(); const searchRef = useRef(null); const { - state: { loading, resourceCollection }, + state: { loading, resourceCollection, resourceCount }, fetchData, } = useListViewResource('dataset', t('dataset'), addDangerToast); @@ -119,10 +113,17 @@ const ChangeDatasourceModal: FunctionComponent = ({ setConfirmedDataset(datasource); }, []); + const fetchDatasetPayload = { + pageIndex, + pageSize: DATASET_PAGE_SIZE, + filters: [], + sortBy, + }; + useDebouncedEffect( () => { fetchData({ - ...emptyRequest, + ...fetchDatasetPayload, ...(filter && { filters: [ { @@ -134,8 +135,8 @@ const ChangeDatasourceModal: FunctionComponent = ({ }), }); }, - 300, - [filter], + SLOW_DEBOUNCE, + [filter, pageIndex, sortBy], ); useEffect(() => { @@ -159,6 +160,7 @@ const ChangeDatasourceModal: FunctionComponent = ({ const changeSearch = (event: React.ChangeEvent) => { const searchValue = event.target.value ?? ''; setFilter(searchValue); + setPageIndex(0); }; const handleChangeConfirm = () => { @@ -187,25 +189,53 @@ const ChangeDatasourceModal: FunctionComponent = ({ setConfirmChange(false); }; - const renderTableView = () => { - const data = resourceCollection.map((ds: any) => ({ - rawName: ds.table_name, - connection: ds.database.database_name, - schema: ds.schema, - name: ( + const columns = [ + { + Cell: ({ row: { original } }: any) => ( selectDatasource({ type: 'table', ...ds })} + onClick={() => selectDatasource({ type: 'table', ...original })} > - {ds.table_name} + {original?.table_name} ), - type: ds.kind, - })); + Header: t('Name'), + accessor: 'table_name', + }, + { + Header: t('Type'), + accessor: 'kind', + disableSortBy: true, + }, + { + Header: t('Schema'), + accessor: 'schema', + }, + { + Header: t('Connection'), + accessor: 'database.database_name', + disableSortBy: true, + }, + { + Cell: ({ + row: { + original: { owners = [] }, + }, + }: any) => , + Header: t('Owners'), + id: 'owners', + disableSortBy: true, + }, + ]; - return data; + const onServerPagination = (args: ServerPagination) => { + setPageIndex(args.pageIndex); + if (args.sortBy) { + // ensure default sort by + setSortBy(args.sortBy.length > 0 ? args.sortBy : DATASET_SORT_BY); + } }; return ( @@ -215,7 +245,7 @@ const ChangeDatasourceModal: FunctionComponent = ({ responsive title={t('Change dataset')} width={confirmChange ? '432px' : ''} - height={confirmChange ? 'auto' : '480px'} + height={confirmChange ? 'auto' : '540px'} hideFooter={!confirmChange} footer={ <> @@ -259,11 +289,17 @@ const ChangeDatasourceModal: FunctionComponent = ({ {loading && } {!loading && ( )} diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index 7b1da36214..3f3ab8d397 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -53,20 +53,12 @@ import ImportModelsModal from 'src/components/ImportModal/index'; import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip'; import AddDatasetModal from './AddDatasetModal'; - -const PAGE_SIZE = 25; -const PASSWORDS_NEEDED_MESSAGE = t( - 'The passwords for the databases below are needed in order to ' + - 'import them together with the datasets. Please note that the ' + - '"Secure Extra" and "Certificate" sections of ' + - 'the database configuration are not present in export files, and ' + - 'should be added manually after the import if they are needed.', -); -const CONFIRM_OVERWRITE_MESSAGE = t( - 'You are importing one or more datasets that already exist. ' + - 'Overwriting might cause you to lose some of your work. Are you ' + - 'sure you want to overwrite?', -); +import { + PAGE_SIZE, + SORT_BY, + PASSWORDS_NEEDED_MESSAGE, + CONFIRM_OVERWRITE_MESSAGE, +} from './constants'; const FlexRowContainer = styled.div` align-items: center; @@ -158,7 +150,7 @@ const DatasetList: FunctionComponent = ({ const canCreate = hasPerm('can_write'); const canExport = hasPerm('can_read'); - const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; + const initialSort = SORT_BY; const openDatasetEditModal = useCallback( ({ id }: Dataset) => { diff --git a/superset-frontend/src/views/CRUD/data/dataset/constants.ts b/superset-frontend/src/views/CRUD/data/dataset/constants.ts new file mode 100644 index 0000000000..cb0b5d3c0a --- /dev/null +++ b/superset-frontend/src/views/CRUD/data/dataset/constants.ts @@ -0,0 +1,34 @@ +/** + * 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 { t } from '@superset-ui/core'; + +export const PAGE_SIZE = 25; +export const SORT_BY = [{ id: 'changed_on_delta_humanized', desc: true }]; +export const PASSWORDS_NEEDED_MESSAGE = t( + 'The passwords for the databases below are needed in order to ' + + 'import them together with the datasets. Please note that the ' + + '"Secure Extra" and "Certificate" sections of ' + + 'the database configuration are not present in export files, and ' + + 'should be added manually after the import if they are needed.', +); +export const CONFIRM_OVERWRITE_MESSAGE = t( + 'You are importing one or more datasets that already exist. ' + + 'Overwriting might cause you to lose some of your work. Are you ' + + 'sure you want to overwrite?', +);