diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.module.scss b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.module.scss index 77c1abf2dc2e..6293d67aa9ae 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.module.scss +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.module.scss @@ -30,6 +30,15 @@ &__no-result { @include text--body-strong; } + + &__cell-error { + display: inline-flex; + align-items: center; + gap: 10px; + svg { + fill: $colors--warning; + } + } } .sorted-table { diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx index c25c12b755a9..6a593a87dff4 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx @@ -59,7 +59,7 @@ import { TableSchemaDetailsRow, TableSpanStatsRow, } from "../api"; -import { checkInfoAvailable } from "../databases"; +import { LoadingCell } from "../databases"; import { InlineAlert } from "@cockroachlabs/ui-components"; import { ViewMode } from "./types"; @@ -561,12 +561,16 @@ export class DatabaseDetailsPage extends React.Component< Ranges ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.spanStats?.error, - table.details.spanStats?.range_count, - ), + cell: table => ( + + {table.details.spanStats?.range_count} + + ), sort: table => table.details.spanStats?.range_count, className: cx("database-table__col-range-count"), name: "rangeCount", @@ -580,12 +584,16 @@ export class DatabaseDetailsPage extends React.Component< Columns ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.schemaDetails?.error, - table.details.schemaDetails?.columns?.length, - ), + cell: table => ( + + {table.details.schemaDetails?.columns?.length} + + ), sort: table => table.details.schemaDetails?.columns?.length, className: cx("database-table__col-column-count"), name: "columnCount", @@ -618,15 +626,19 @@ export class DatabaseDetailsPage extends React.Component< Regions ), - cell: table => - checkInfoAvailable( - table.requestError, - null, - table.details.nodesByRegionString && - table.details.nodesByRegionString.length > 0 + cell: table => ( + + {table.details.nodesByRegionString && + table.details.nodesByRegionString.length > 0 ? table.details.nodesByRegionString - : null, - ), + : null} + + ), sort: table => table.details.nodesByRegionString, className: cx("database-table__col--regions"), name: "regions", @@ -651,16 +663,19 @@ export class DatabaseDetailsPage extends React.Component< % of Live Data ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.spanStats?.error, - table.details.spanStats ? ( + cell: table => ( + + {table.details.spanStats ? ( - ) : null, - ), + ) : null} + + ), sort: table => table.details.spanStats?.live_percentage, - className: cx("database-table__col-column-count"), name: "livePercentage", }, { @@ -672,16 +687,20 @@ export class DatabaseDetailsPage extends React.Component< Table Stats Last Updated ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.statsLastUpdated?.error, + cell: table => ( + , - ), + /> + + ), sort: table => table.details.statsLastUpdated, className: cx("database-table__col--table-stats"), name: "tableStatsUpdated", @@ -720,12 +739,16 @@ export class DatabaseDetailsPage extends React.Component< Users ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.grants?.error, - table.details.grants?.roles.length, - ), + cell: table => ( + + {table.details.grants?.roles.length} + + ), sort: table => table.details.grants?.roles.length, className: cx("database-table__col-user-count"), name: "userCount", @@ -736,12 +759,16 @@ export class DatabaseDetailsPage extends React.Component< Roles ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.grants?.error, - table.details.grants?.roles.join(", "), - ), + cell: table => ( + + {table.details.grants?.roles.join(", ")} + + ), sort: table => table.details.grants?.roles.join(", "), className: cx("database-table__col-roles"), name: "roles", @@ -752,12 +779,16 @@ export class DatabaseDetailsPage extends React.Component< Grants ), - cell: table => - checkInfoAvailable( - table.requestError, - table.details.grants?.error, - table.details.grants?.privileges.join(", "), - ), + cell: table => ( + + {table.details.grants?.privileges.join(", ")} + + ), sort: table => table.details.grants?.privileges?.join(", "), className: cx("database-table__col-grants"), name: "grants", diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/tableCells.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/tableCells.tsx index e10c4d30dfa4..fda96404d2a0 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/tableCells.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/tableCells.tsx @@ -32,7 +32,7 @@ import * as format from "../util/format"; import { Breadcrumbs } from "../breadcrumbs"; import { CaretRight } from "../icon/caretRight"; import { CockroachCloudContext } from "../contexts"; -import { checkInfoAvailable, getNetworkErrorMessage } from "../databases"; +import { LoadingCell, getNetworkErrorMessage } from "../databases"; import { ViewMode } from "./types"; const cx = classNames.bind(styles); @@ -44,13 +44,18 @@ export const DiskSizeCell = ({ }): JSX.Element => { return ( <> - {checkInfoAvailable( - table.requestError, - table.details?.spanStats?.error, - table.details?.spanStats?.approximate_disk_bytes - ? format.Bytes(table.details?.spanStats?.approximate_disk_bytes) - : null, - )} + { + + {table.details?.spanStats?.approximate_disk_bytes + ? format.Bytes(table.details?.spanStats?.approximate_disk_bytes) + : null} + + } ); }; @@ -110,11 +115,16 @@ export const IndexesCell = ({ }): JSX.Element => { const elem = ( <> - {checkInfoAvailable( - table.requestError, - table.details?.schemaDetails?.error, - table.details?.schemaDetails?.indexes?.length, - )} + { + + {table.details?.schemaDetails?.indexes?.length} + + } ); // If index recommendations are not enabled or we don't have any index recommendations, diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.module.scss b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.module.scss index f685f2248f52..af6569d122af 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.module.scss +++ b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.module.scss @@ -40,6 +40,15 @@ @include text--body; overflow-wrap: anywhere; } + + &__error-cell{ + display: inline-flex; + align-items: center; + gap: 10px; + svg { + fill: $colors--warning; + } + } } .icon { diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx index 925ab3a1101b..3d005c7500a8 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx @@ -68,7 +68,7 @@ import { TableSchemaDetailsRow, TableSpanStatsRow, } from "../api"; -import { checkInfoAvailable } from "../databases"; +import { LoadingCell } from "../databases"; const cx = classNames.bind(styles); const booleanSettingCx = classnames.bind(booleanSettingStyles); @@ -473,55 +473,82 @@ export class DatabaseTablePage extends React.Component< + {details.spanStats?.approximate_disk_bytes + ? format.Bytes( + details.spanStats?.approximate_disk_bytes, + ) + : null} + + } /> + {details.replicaData?.replicaCount} + + } /> + {details.spanStats?.range_count} + + } /> , - )} + value={ + + + + } /> {details.statsLastUpdated && ( , - )} + value={ + + + + } /> )} {this.props.automaticStatsCollectionEnabled != @@ -556,14 +583,21 @@ export class DatabaseTablePage extends React.Component< {this.props.showNodeRegionsSection && ( + {details.nodesByRegionString && details.nodesByRegionString?.length - ? details.nodesByRegionString - : null, - )} + ? details.nodesByRegionString + : null} + + } /> )} + {details.indexData?.indexes?.join(", ")} + + } className={cx( "database-table-page__indexes--value", )} diff --git a/pkg/ui/workspaces/cluster-ui/src/databases/util.spec.ts b/pkg/ui/workspaces/cluster-ui/src/databases/util.spec.tsx similarity index 71% rename from pkg/ui/workspaces/cluster-ui/src/databases/util.spec.ts rename to pkg/ui/workspaces/cluster-ui/src/databases/util.spec.tsx index e9c0c04e528d..7f4ee73954b5 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databases/util.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/databases/util.spec.tsx @@ -8,6 +8,9 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. +import React from "react"; +import { render } from "@testing-library/react"; + import { INodeStatus } from "../util"; import { Nodes, @@ -16,6 +19,7 @@ import { getNodeIdsFromStoreIds, normalizePrivileges, normalizeRoles, + LoadingCell, } from "./util"; describe("Getting nodes by region string", () => { @@ -173,3 +177,74 @@ describe("Normalize roles", () => { expect(result).toEqual(["admin", "public"]); }); }); + +describe("LoadingCell", () => { + it("renders empty data", () => { + const { getByText } = render( + + {null} + , + ); + + expect(getByText("No data")).not.toBeNull(); + }); + it("renders with undefined children", () => { + const { getByText } = render( + , + ); + + expect(getByText("No data")).not.toBeNull(); + }); + it("renders skeleton heading when loading", () => { + const { getByRole } = render( + + {null} + , + ); + + expect(getByRole("heading")).not.toBeNull(); + }); + it("renders error name and status icon", () => { + const { getByRole } = render( + + {null} + , + ); + + // TODO(davidh): rendering of antd Tooltip component doesn't work + // here and hence can't be directly tested to contain the error + // name. + expect(getByRole("status")).not.toBeNull(); + }); + it("renders children with no error", () => { + const { getByText } = render( + +
inner data
+ , + ); + + expect(getByText("inner data")).not.toBeNull(); + }); + it("renders children with error together", () => { + const { getByText, getByRole } = render( + +
inner data
+ , + ); + + expect(getByRole("status")).not.toBeNull(); + expect(getByText("inner data")).not.toBeNull(); + }); +}); diff --git a/pkg/ui/workspaces/cluster-ui/src/databases/util.tsx b/pkg/ui/workspaces/cluster-ui/src/databases/util.tsx index 5a9340ce97f8..0664af6c511e 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databases/util.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databases/util.tsx @@ -10,8 +10,11 @@ import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; import React from "react"; -import { Tooltip } from "antd"; +import { Skeleton, Tooltip } from "antd"; import "antd/lib/tooltip/style"; +import "antd/lib/skeleton/style"; +import { Caution } from "@cockroachlabs/icons"; + import { isMaxSizeError, isPrivilegeError, @@ -240,34 +243,56 @@ export function buildIndexStatToRecommendationsMap( return recommendationsMap; } -export function checkInfoAvailable( - requestError: Error, - queryError: Error, - cell: React.ReactNode, -): React.ReactNode { +interface LoadingCellProps { + requestError: Error; + queryError?: Error; + loading: boolean; + errorClassName: string; +} + +export const LoadingCell: React.FunctionComponent = ({ + loading, + requestError, + queryError, + errorClassName, + children, +}) => { + if (loading) { + return ( + + ); + } + let tooltipMsg = ""; if (requestError) { tooltipMsg = `Encountered a network error fetching data for this cell: ${requestError.name}`; } else if (queryError) { tooltipMsg = getQueryErrorMessage(queryError); - } else if (cell == null) { - tooltipMsg = "Empty result"; } + + let childrenOrNoData = <>{children}; + if (children == null) { + childrenOrNoData = <>{"No data"}; + } + // If we encounter an error gathering data for this cell, - // render it "unavailable" with a tooltip message for the error. + // render a warning icon with a tooltip message for the error. if (tooltipMsg !== "") { return ( - (unavailable) + + {childrenOrNoData} ); + } else { + return childrenOrNoData; } - return cell; -} +}; export const getNetworkErrorMessage = (requestError: Error): string => { return `Encountered a network error: ${requestError.message}`; diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databaseTableCells.tsx b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databaseTableCells.tsx index e66c1a5a14ca..f18008d53d98 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databaseTableCells.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databaseTableCells.tsx @@ -20,7 +20,7 @@ import { Link } from "react-router-dom"; import { StackIcon } from "../icon/stackIcon"; import { CockroachCloudContext } from "../contexts"; import { - checkInfoAvailable, + LoadingCell, getNetworkErrorMessage, getQueryErrorMessage, } from "../databases"; @@ -33,19 +33,18 @@ interface CellProps { database: DatabasesPageDataDatabase; } -export const DiskSizeCell = ({ database }: CellProps): JSX.Element => { - return ( - <> - {checkInfoAvailable( - database.spanStatsRequestError, - database.spanStats?.error, - database.spanStats?.approximate_disk_bytes - ? format.Bytes(database.spanStats?.approximate_disk_bytes) - : null, - )} - - ); -}; +export const DiskSizeCell = ({ database }: CellProps) => ( + + {database.spanStats?.approximate_disk_bytes + ? format.Bytes(database.spanStats?.approximate_disk_bytes) + : null} + +); export const IndexRecCell = ({ database }: CellProps): JSX.Element => { const text = diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.module.scss b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.module.scss index 4e076c6faeb2..f1e8981fd471 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.module.scss +++ b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.module.scss @@ -31,6 +31,15 @@ &__no-result { @include text--body-strong; } + + &__cell-error { + display: inline-flex; + align-items: center; + gap: 10px; + svg { + fill: $colors--warning; + } + } } .sorted-table { diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx index e24d839a583b..755022c74bbb 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx @@ -57,8 +57,8 @@ import { SqlApiQueryResponse, SqlExecutionErrorMessage, } from "../api"; +import { LoadingCell } from "../databases"; import { InlineAlert } from "@cockroachlabs/ui-components"; -import { checkInfoAvailable } from "../databases"; const cx = classNames.bind(styles); const sortableTableCx = classNames.bind(sortableTableStyles); @@ -562,12 +562,16 @@ export class DatabasesPage extends React.Component< Tables ), - cell: database => - checkInfoAvailable( - database.detailsRequestError, - database.tables?.error, - database.tables?.tables?.length, - ), + cell: database => ( + + {database.tables?.tables?.length} + + ), sort: database => database.tables?.tables.length ?? 0, className: cx("databases-table__col-table-count"), name: "tableCount", @@ -581,12 +585,16 @@ export class DatabasesPage extends React.Component< Range Count ), - cell: database => - checkInfoAvailable( - database.spanStatsRequestError, - database.spanStats?.error, - database.spanStats?.range_count, - ), + cell: database => ( + + {database.spanStats?.range_count} + + ), sort: database => database.spanStats?.range_count, className: cx("databases-table__col-range-count"), name: "rangeCount", @@ -600,12 +608,15 @@ export class DatabasesPage extends React.Component< {this.props.isTenant ? "Regions" : "Regions/Nodes"} ), - cell: database => - checkInfoAvailable( - database.detailsRequestError, - null, - database.nodesByRegionString ? database.nodesByRegionString : null, - ), + cell: database => ( + + {database.nodesByRegionString ? database.nodesByRegionString : null} + + ), sort: database => database.nodesByRegionString, className: cx("databases-table__col-node-regions"), name: "nodeRegions",