You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by yo...@apache.org on 2021/06/10 02:06:41 UTC

[superset] branch master updated: fix(datasets): consistent dataset list (#15014)

This is an automated email from the ASF dual-hosted git repository.

yongjiezhao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 834bb94  fix(datasets): consistent dataset list (#15014)
834bb94 is described below

commit 834bb9409d5ab4028a469db85dba28ff353c4c09
Author: Yongjie Zhao <yo...@gmail.com>
AuthorDate: Thu Jun 10 03:05:33 2021 +0100

    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 ++++++++++++++-------
 .../src/views/CRUD/data/dataset/DatasetList.tsx    |  22 ++---
 .../CRUD/data/dataset/constants.ts}                |  20 +++-
 6 files changed, 139 insertions(+), 68 deletions(-)

diff --git a/superset-frontend/spec/javascripts/datasource/ChangeDatasourceModal_spec.jsx b/superset-frontend/spec/javascripts/datasource/ChangeDatasourceModal_spec.jsx
index 4e7e934..77a35eb 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 bc3517f..07dc18a 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,
                 )}
             </div>
           )}
diff --git a/superset-frontend/src/components/TableView/types.ts b/superset-frontend/src/components/TableView/types.ts
index 974cb24..dceb275 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 SortByType = SortingRule<string>[];
 
-export type SortColumns = SortColumn[];
+export interface ServerPagination {
+  pageIndex: number;
+  sortBy?: SortByType;
+}
diff --git a/superset-frontend/src/datasource/ChangeDatasourceModal.tsx b/superset-frontend/src/datasource/ChangeDatasourceModal.tsx
index 78dd1c2..d535751 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<ChangeDatasourceModalProps> = ({
   addDangerToast,
   addSuccessToast,
@@ -105,12 +97,14 @@ const ChangeDatasourceModal: FunctionComponent<ChangeDatasourceModalProps> = ({
   show,
 }) => {
   const [filter, setFilter] = useState<any>(undefined);
+  const [pageIndex, setPageIndex] = useState<number>(0);
+  const [sortBy, setSortBy] = useState<SortByType>(DATASET_SORT_BY);
   const [confirmChange, setConfirmChange] = useState(false);
   const [confirmedDataset, setConfirmedDataset] = useState<Datasource>();
   const searchRef = useRef<AntdInput>(null);
 
   const {
-    state: { loading, resourceCollection },
+    state: { loading, resourceCollection, resourceCount },
     fetchData,
   } = useListViewResource<Dataset>('dataset', t('dataset'), addDangerToast);
 
@@ -119,10 +113,17 @@ const ChangeDatasourceModal: FunctionComponent<ChangeDatasourceModalProps> = ({
     setConfirmedDataset(datasource);
   }, []);
 
+  const fetchDatasetPayload = {
+    pageIndex,
+    pageSize: DATASET_PAGE_SIZE,
+    filters: [],
+    sortBy,
+  };
+
   useDebouncedEffect(
     () => {
       fetchData({
-        ...emptyRequest,
+        ...fetchDatasetPayload,
         ...(filter && {
           filters: [
             {
@@ -134,8 +135,8 @@ const ChangeDatasourceModal: FunctionComponent<ChangeDatasourceModalProps> = ({
         }),
       });
     },
-    300,
-    [filter],
+    SLOW_DEBOUNCE,
+    [filter, pageIndex, sortBy],
   );
 
   useEffect(() => {
@@ -159,6 +160,7 @@ const ChangeDatasourceModal: FunctionComponent<ChangeDatasourceModalProps> = ({
   const changeSearch = (event: React.ChangeEvent<HTMLInputElement>) => {
     const searchValue = event.target.value ?? '';
     setFilter(searchValue);
+    setPageIndex(0);
   };
 
   const handleChangeConfirm = () => {
@@ -187,25 +189,53 @@ const ChangeDatasourceModal: FunctionComponent<ChangeDatasourceModalProps> = ({
     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) => (
         <StyledSpan
           role="button"
           tabIndex={0}
           data-test="datasource-link"
-          onClick={() => selectDatasource({ type: 'table', ...ds })}
+          onClick={() => selectDatasource({ type: 'table', ...original })}
         >
-          {ds.table_name}
+          {original?.table_name}
         </StyledSpan>
       ),
-      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) => <FacePile users={owners} />,
+      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<ChangeDatasourceModalProps> = ({
       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<ChangeDatasourceModalProps> = ({
             {loading && <Loading />}
             {!loading && (
               <TableView
-                columns={TABLE_COLUMNS}
-                data={renderTableView()}
-                pageSize={20}
+                columns={columns}
+                data={resourceCollection}
+                pageSize={DATASET_PAGE_SIZE}
+                initialPageIndex={pageIndex}
+                initialSortBy={sortBy}
+                totalCount={resourceCount}
+                onServerPagination={onServerPagination}
                 className="table-condensed"
                 emptyWrapperType={EmptyWrapperType.Small}
+                serverPagination
+                isPaginationSticky
                 scrollTable
               />
             )}
diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
index 7b1da36..3f3ab8d 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<DatasetListProps> = ({
   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/components/TableView/types.ts b/superset-frontend/src/views/CRUD/data/dataset/constants.ts
similarity index 52%
copy from superset-frontend/src/components/TableView/types.ts
copy to superset-frontend/src/views/CRUD/data/dataset/constants.ts
index 974cb24..cb0b5d3 100644
--- a/superset-frontend/src/components/TableView/types.ts
+++ b/superset-frontend/src/views/CRUD/data/dataset/constants.ts
@@ -16,9 +16,19 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-export interface SortColumn {
-  id: string;
-  desc?: boolean;
-}
+import { t } from '@superset-ui/core';
 
-export type SortColumns = SortColumn[];
+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?',
+);