From d776224bcf4b7dda78ed7fd0f90d1c257a0c6e5b Mon Sep 17 00:00:00 2001 From: Aniket Katkar Date: Wed, 25 Jan 2023 11:35:40 +0530 Subject: [PATCH] fix(ui)#9846: row count not populating on table details page (#9896) * fixed rows not populating in table info added skeleton loaders for rowCount and columnCount styling and localization fixes * added unit tests for checking rowCount and columnCount --- .../DatasetDetails.component.tsx | 106 ++++++++++++------ .../DatasetDetails.interface.ts | 1 + .../EntitySummaryDetails.tsx | 6 +- .../resources/ui/src/interface/types.d.ts | 1 + .../ui/src/locale/languages/en-us.json | 2 +- .../DatasetDetailsPage.component.tsx | 35 +++++- .../DatasetDetailsPage.test.tsx | 68 ++++++++++- .../datasetDetailsPage.mock.ts | 9 ++ .../resources/ui/src/utils/EntityUtils.tsx | 2 +- 9 files changed, 186 insertions(+), 44 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DatasetDetails/DatasetDetails.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DatasetDetails/DatasetDetails.component.tsx index ef5641ef220..fcdb2030cb6 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DatasetDetails/DatasetDetails.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DatasetDetails/DatasetDetails.component.tsx @@ -11,7 +11,7 @@ * limitations under the License. */ -import { Col, Row } from 'antd'; +import { Col, Row, Skeleton, Space, Typography } from 'antd'; import { AxiosError } from 'axios'; import classNames from 'classnames'; import { isEqual, isNil, isUndefined } from 'lodash'; @@ -31,6 +31,7 @@ import { JoinedWith, Table, TableJoins, + TableProfile, TypeUsedToReturnUsageDetailsOfAnEntity, } from '../../generated/entity/data/table'; import { ThreadType } from '../../generated/entity/feed/thread'; @@ -127,6 +128,7 @@ const DatasetDetails: React.FC = ({ handleExtensionUpdate, updateThreadHandler, entityFieldTaskCount, + isTableProfileLoading, }: DatasetDetailsProps) => { const { t } = useTranslation(); const history = useHistory(); @@ -338,37 +340,62 @@ const DatasetDetails: React.FC = ({ .sort((a, b) => b.joinCount - a.joinCount); }; - const prepareTableRowInfo = () => { - const rowData = - ([ - { - date: new Date(tableProfile?.timestamp || 0), - value: tableProfile?.rowCount ?? 0, - }, - ] as Array<{ - date: Date; - value: number; - }>) ?? []; - - if (!isUndefined(tableProfile)) { + const prepareExtraInfoValues = ( + key: EntityInfo, + isTableProfileLoading?: boolean, + tableProfile?: TableProfile, + numberOfColumns?: number + ) => { + if (isTableProfileLoading) { return ( -
- {rowData.length > 1 && ( - - )} - 1, - })}>{`${tableProfile?.rowCount?.toLocaleString() || 0} rows`} -
+ ); - } else { - return ''; + } + switch (key) { + case EntityInfo.COLUMNS: { + const columnCount = + tableProfile && tableProfile?.columnCount + ? tableProfile?.columnCount + : numberOfColumns + ? numberOfColumns + : undefined; + + return columnCount + ? `${columns.length} ${t('label.column-plural')}` + : null; + } + + case EntityInfo.ROWS: { + const rowData = + ([ + { + date: new Date(tableProfile?.timestamp || 0), + value: tableProfile?.rowCount ?? 0, + }, + ] as Array<{ + date: Date; + value: number; + }>) ?? []; + + return isUndefined(tableProfile) ? null : ( + + {rowData.length > 1 && ( + + )} + {`${ + tableProfile?.rowCount?.toLocaleString() || 0 + } rows`} + + ); + } + default: + return null; } }; @@ -395,16 +422,21 @@ const DatasetDetails: React.FC = ({ { value: `${weeklyUsageCount} ${t('label.query-plural')}` }, { key: EntityInfo.COLUMNS, - value: - tableProfile && tableProfile?.columnCount - ? `${tableProfile.columnCount} ${t('label.columns-plural')}` - : columns.length - ? `${columns.length} ${t('label.columns-plural')}` - : '', + localizationKey: 'column-plural', + value: prepareExtraInfoValues( + EntityInfo.COLUMNS, + isTableProfileLoading, + tableProfile, + columns.length + ), }, { key: EntityInfo.ROWS, - value: prepareTableRowInfo(), + value: prepareExtraInfoValues( + EntityInfo.ROWS, + isTableProfileLoading, + tableProfile + ), }, ]; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DatasetDetails/DatasetDetails.interface.ts b/openmetadata-ui/src/main/resources/ui/src/components/DatasetDetails/DatasetDetails.interface.ts index 974ee102bf9..17745302ff1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DatasetDetails/DatasetDetails.interface.ts +++ b/openmetadata-ui/src/main/resources/ui/src/components/DatasetDetails/DatasetDetails.interface.ts @@ -66,6 +66,7 @@ export interface DatasetDetailsProps { slashedTableName: TitleBreadcrumbProps['titleLinks']; entityThread: Thread[]; deleted?: boolean; + isTableProfileLoading?: boolean; isLineageLoading?: boolean; isSampleDataLoading?: boolean; isQueriesLoading?: boolean; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/EntitySummaryDetails/EntitySummaryDetails.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/EntitySummaryDetails/EntitySummaryDetails.tsx index 60d43af5c37..354a7b40268 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/EntitySummaryDetails/EntitySummaryDetails.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/EntitySummaryDetails/EntitySummaryDetails.tsx @@ -203,7 +203,11 @@ const EntitySummaryDetails = ({ ? `${t(`label.${toLower(data.key)}`)} - ` : null : `${t('label.no-entity', { - entity: t(`label.${toLower(data.key)}`), + entity: t( + `label.${toLower( + data.localizationKey ? data.localizationKey : data.key + )}` + ), })}` : null} diff --git a/openmetadata-ui/src/main/resources/ui/src/interface/types.d.ts b/openmetadata-ui/src/main/resources/ui/src/interface/types.d.ts index a88dab1a4ea..3bf45c2b8cc 100644 --- a/openmetadata-ui/src/main/resources/ui/src/interface/types.d.ts +++ b/openmetadata-ui/src/main/resources/ui/src/interface/types.d.ts @@ -175,6 +175,7 @@ declare module 'Models' { key?: string; value: string | number | React.ReactNode; id?: string; + localizationKey?: string; isLink?: boolean; placeholderText?: string; openInNewTab?: boolean; diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json index 7ff48b05661..232aaba182c 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json @@ -84,7 +84,7 @@ "collapse-all": "Collapse All", "column": "Column", "column-entity": "Column {{entity}}", - "columns-plural": "Columns", + "column-plural": "Columns", "comment-lowercase": "comment", "completed": "Completed", "completed-entity": "Completed {{entity}}", diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/DatasetDetailsPage/DatasetDetailsPage.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/DatasetDetailsPage/DatasetDetailsPage.component.tsx index 444fe6432c6..553a8cc0ed2 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/DatasetDetailsPage/DatasetDetailsPage.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/DatasetDetailsPage/DatasetDetailsPage.component.tsx @@ -33,12 +33,14 @@ import { isEmpty, isUndefined } from 'lodash'; import { observer } from 'mobx-react'; import { EntityTags } from 'Models'; import React, { FunctionComponent, useEffect, useState } from 'react'; +import { useTranslation } from 'react-i18next'; import { useHistory, useParams } from 'react-router-dom'; import { getAllFeeds, postFeedById, postThread } from 'rest/feedsAPI'; import { getLineageByFQN } from 'rest/lineageAPI'; import { addLineage, deleteLineageEdge } from 'rest/miscAPI'; import { addFollower, + getLatestTableProfileByFqn, getTableDetailsByFQN, patchTableDetails, removeFollower, @@ -95,6 +97,7 @@ import { showErrorToast } from '../../utils/ToastUtils'; const DatasetDetailsPage: FunctionComponent = () => { const history = useHistory(); + const { t } = useTranslation(); const { getEntityPermissionByFqn } = usePermissionProvider(); const [isLoading, setIsLoading] = useState(true); const [isLineageLoading, setIsLineageLoading] = useState(false); @@ -104,6 +107,8 @@ const DatasetDetailsPage: FunctionComponent = () => { useState(false); const [isentityThreadLoading, setIsentityThreadLoading] = useState(false); + const [isTableProfileLoading, setIsTableProfileLoading] = + useState(false); const USERId = getCurrentUserId(); const [tableId, setTableId] = useState(''); const [tier, setTier] = useState(); @@ -289,7 +294,6 @@ const DatasetDetailsPage: FunctionComponent = () => { joins, tags, sampleData, - profile, tableType, version, service, @@ -354,7 +358,6 @@ const DatasetDetailsPage: FunctionComponent = () => { setDescription(description ?? ''); setColumns(columns || []); setSampleData(sampleData as TableData); - setTableProfile(profile); setTableTags(getTagsWithoutTier(tags || [])); setUsageSummary( usageSummary as TypeUsedToReturnUsageDetailsOfAnEntity @@ -382,6 +385,29 @@ const DatasetDetailsPage: FunctionComponent = () => { }); }; + const fetchTableProfileDetails = async () => { + if (!isEmpty(tableDetails)) { + setIsTableProfileLoading(true); + try { + const { profile } = await getLatestTableProfileByFqn( + tableDetails.fullyQualifiedName ?? '' + ); + + setTableProfile(profile); + } catch (err) { + showErrorToast( + err as AxiosError, + t('server.entity-details-fetch-error', { + entityType: t('label.table'), + entityName: tableDetails.displayName ?? tableDetails.name, + }) + ); + } finally { + setIsTableProfileLoading(false); + } + } + }; + const fetchTabSpecificData = (tabField = '') => { switch (tabField) { case TabSpecificField.SAMPLE_DATA: { @@ -798,6 +824,10 @@ const DatasetDetailsPage: FunctionComponent = () => { } }, [tablePermissions]); + useEffect(() => { + fetchTableProfileDetails(); + }, [tableDetails]); + useEffect(() => { fetchResourcePermission(tableFQN); }, [tableFQN]); @@ -851,6 +881,7 @@ const DatasetDetailsPage: FunctionComponent = () => { isNodeLoading={isNodeLoading} isQueriesLoading={isTableQueriesLoading} isSampleDataLoading={isSampleDataLoading} + isTableProfileLoading={isTableProfileLoading} isentityThreadLoading={isentityThreadLoading} joins={joins} lineageLeafNodes={leafNodes} diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/DatasetDetailsPage/DatasetDetailsPage.test.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/DatasetDetailsPage/DatasetDetailsPage.test.tsx index 1362c4e82ea..4623389b603 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/DatasetDetailsPage/DatasetDetailsPage.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/DatasetDetailsPage/DatasetDetailsPage.test.tsx @@ -17,6 +17,7 @@ import { findByText, fireEvent, render, + screen, } from '@testing-library/react'; import React from 'react'; import { MemoryRouter } from 'react-router'; @@ -30,6 +31,7 @@ import { getLineageByFQN } from 'rest/lineageAPI'; import { addLineage, deleteLineageEdge } from 'rest/miscAPI'; import { addFollower, + getLatestTableProfileByFqn, getTableDetailsByFQN, patchTableDetails, removeFollower, @@ -40,10 +42,13 @@ import { createPostRes, mockFollowRes, mockLineageRes, + mockTableProfileResponse, mockUnfollowRes, updateTagRes, } from './datasetDetailsPage.mock'; +const mockShowErrorToast = jest.fn(); + const mockUseParams = { datasetFQN: 'bigquery_gcp:shopify:dim_address', tab: 'schema', @@ -68,8 +73,7 @@ jest.mock('../../AppState', () => ({ jest.mock('components/PermissionProvider/PermissionProvider', () => ({ usePermissionProvider: jest.fn().mockImplementation(() => ({ - permissions: {}, - getEntityPermission: jest.fn().mockResolvedValue({ + getEntityPermissionByFqn: jest.fn().mockResolvedValue({ Create: true, Delete: true, EditAll: true, @@ -146,6 +150,7 @@ jest.mock('components/DatasetDetails/DatasetDetails.component', () => { handleRemoveColumnTest, deletePostHandler, entityLineageHandler, + tableProfile, }) => (
+ {tableProfile && ( + <> +
{tableProfile.rowCount}
+
{tableProfile.columnCount}
+ + )}
) ); @@ -265,6 +276,19 @@ jest.mock('rest/tableAPI', () => ({ removeFollower: jest .fn() .mockImplementation(() => Promise.resolve(mockUnfollowRes)), + getLatestTableProfileByFqn: jest + .fn() + .mockImplementation(() => Promise.resolve(mockTableProfileResponse)), +})); + +jest.mock('react-i18next', () => ({ + useTranslation: jest.fn().mockImplementation(() => ({ + t: jest.fn().mockImplementation((str) => str), + })), +})); + +jest.mock('../../utils/ToastUtils', () => ({ + showErrorToast: jest.fn().mockImplementation(() => mockShowErrorToast()), })); jest.mock('../../utils/FeedUtils', () => ({ @@ -1159,5 +1183,45 @@ describe('Test DatasetDetails page', () => { fireEvent.click(deletePostHandler); }); + + it('Table profile details should be passed correctly after successful API response', async () => { + await act(async () => { + render(, { + wrapper: MemoryRouter, + }); + }); + + const rowCount = screen.getByTestId('rowCount'); + const columnCount = screen.getByTestId('columnCount'); + + expect(rowCount).toBeInTheDocument(); + expect(columnCount).toBeInTheDocument(); + + expect(rowCount).toContainHTML( + `${mockTableProfileResponse.profile.rowCount}` + ); + expect(columnCount).toContainHTML( + `${mockTableProfileResponse.profile.columnCount}` + ); + }); + + it('An error should be thrown if table profile API throws error', async () => { + (getLatestTableProfileByFqn as jest.Mock).mockImplementationOnce(() => + Promise.reject() + ); + + await act(async () => { + render(, { + wrapper: MemoryRouter, + }); + }); + + const rowCount = screen.queryByTestId('rowCount'); + const columnCount = screen.queryByTestId('columnCount'); + + expect(rowCount).toBeNull(); + expect(columnCount).toBeNull(); + expect(mockShowErrorToast).toHaveBeenCalledTimes(1); + }); }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/DatasetDetailsPage/datasetDetailsPage.mock.ts b/openmetadata-ui/src/main/resources/ui/src/pages/DatasetDetailsPage/datasetDetailsPage.mock.ts index 03aae0c1ca0..9acdadc80c2 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/DatasetDetailsPage/datasetDetailsPage.mock.ts +++ b/openmetadata-ui/src/main/resources/ui/src/pages/DatasetDetailsPage/datasetDetailsPage.mock.ts @@ -489,3 +489,12 @@ export const mockLineageRes = { upstreamEdges: [], downstreamEdges: [], }; + +export const mockTableProfileResponse = { + profile: { + timestamp: 1674466560, + profileSampleType: 'PERCENTAGE', + columnCount: 12, + rowCount: 14567, + }, +}; diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/EntityUtils.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/EntityUtils.tsx index 49e9bc4a63b..882d09a9c1b 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/EntityUtils.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/EntityUtils.tsx @@ -172,7 +172,7 @@ export const getEntityOverview = ( isLink: false, }, { - name: i18next.t('label.columns-plural'), + name: i18next.t('label.column-plural'), value: columns ? columns.length : '--', isLink: false, },