From 89687a8a134045e20979dd245dd5176344fbba5f Mon Sep 17 00:00:00 2001 From: Celia La Date: Sat, 17 Nov 2018 23:24:38 -0500 Subject: [PATCH] ui: render errors in Loading component Previously, data fetching errors were not being consistently surfaced to the user. This commit updates all existing uses of the Loading component to surface these errors: - Some parent components render data from multiple sources, potentially having multiple errors to show the user. This commit also extends the Loading component to handle multiple errors. - The Certificates page had custom loading logic. This commit replaces logic with Loading component. Also in this commit is some minor cleanup (copy, layout). Fixes: #24011 Release note (admin ui change): This commit wires up data errors to all existing uses of the Loading component. Previously, data errors weren't consistently surfaced. --- .../containers/map/nodeCanvasContainer.tsx | 10 +++++++ .../containers/dataDistribution/index.tsx | 30 ++++++++++++++----- .../reports/containers/localities/index.tsx | 1 + .../reports/containers/network/index.tsx | 18 ++++++++++- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/pkg/ui/ccl/src/views/clusterviz/containers/map/nodeCanvasContainer.tsx b/pkg/ui/ccl/src/views/clusterviz/containers/map/nodeCanvasContainer.tsx index 390cacbb526f..c7c2daa2259a 100644 --- a/pkg/ui/ccl/src/views/clusterviz/containers/map/nodeCanvasContainer.tsx +++ b/pkg/ui/ccl/src/views/clusterviz/containers/map/nodeCanvasContainer.tsx @@ -41,6 +41,7 @@ interface NodeCanvasContainerProps { livenesses: { [id: string]: Liveness }; dataExists: boolean; dataIsValid: boolean; + dataErrors: Error[]; refreshNodes: typeof refreshNodes; refreshLiveness: typeof refreshLiveness; refreshLocations: typeof refreshLocations; @@ -72,6 +73,7 @@ class NodeCanvasContainer extends React.Component ( nodes.valid && locations.valid && liveness.valid, ); +const dataErrors = createSelector( + selectNodeRequestStatus, + selectLocationsRequestStatus, + selectLivenessRequestStatus, + (nodes, locations, liveness) => [nodes.lastError, locations.lastError, liveness.lastError], +); + export default connect( (state: AdminUIState, _ownProps: NodeCanvasContainerOwnProps) => ({ nodesSummary: nodesSummarySelector(state), @@ -109,6 +118,7 @@ export default connect( livenesses: livenessByNodeIDSelector(state), dataIsValid: selectDataIsValid(state), dataExists: selectDataExists(state), + dataErrors: dataErrors(state), }), { refreshNodes, diff --git a/pkg/ui/src/views/cluster/containers/dataDistribution/index.tsx b/pkg/ui/src/views/cluster/containers/dataDistribution/index.tsx index a8894c97df06..e0f0a0df54a5 100644 --- a/pkg/ui/src/views/cluster/containers/dataDistribution/index.tsx +++ b/pkg/ui/src/views/cluster/containers/dataDistribution/index.tsx @@ -24,11 +24,17 @@ import * as docsURL from "src/util/docs"; import { FixLong } from "src/util/fixLong"; import { cockroach } from "src/js/protos"; import { AdminUIState } from "src/redux/state"; -import { refreshDataDistribution, refreshNodes, refreshLiveness } from "src/redux/apiReducers"; +import { + refreshDataDistribution, + refreshNodes, + refreshLiveness, + CachedDataReducerState, +} from "src/redux/apiReducers"; import { LocalityTree, selectLocalityTree } from "src/redux/localities"; import ReplicaMatrix, { SchemaObject } from "./replicaMatrix"; import { TreeNode, TreePath } from "./tree"; import "./index.styl"; +import {selectLivenessRequestStatus, selectNodeRequestStatus} from "src/redux/nodes"; type DataDistributionResponse = cockroach.server.serverpb.DataDistributionResponse; type NodeDescriptor = cockroach.roachpb.INodeDescriptor; @@ -43,7 +49,7 @@ const ZONE_CONFIG_TEXT = ( ); interface DataDistributionProps { - dataDistribution: DataDistributionResponse; + dataDistribution: CachedDataReducerState; localityTree: LocalityTree; sortedZoneConfigs: ZoneConfig$Properties[]; } @@ -69,7 +75,7 @@ class DataDistribution extends React.Component { getCellValue = (dbPath: TreePath, nodePath: TreePath): number => { const [dbName, tableName] = dbPath; const nodeID = nodePath[nodePath.length - 1]; - const databaseInfo = this.props.dataDistribution.database_info; + const databaseInfo = this.props.dataDistribution.data.database_info; const res = databaseInfo[dbName].table_info[tableName].replica_count_by_node_id[nodeID]; if (!res) { @@ -81,7 +87,7 @@ class DataDistribution extends React.Component { render() { const nodeTree = nodeTreeFromLocalityTree("Cluster", this.props.localityTree); - const databaseInfo = this.props.dataDistribution.database_info; + const databaseInfo = this.props.dataDistribution.data.database_info; const dbTree: TreeNode = { name: "Cluster", data: { @@ -137,8 +143,9 @@ class DataDistribution extends React.Component { } interface DataDistributionPageProps { - dataDistribution: DataDistributionResponse; + dataDistribution: CachedDataReducerState; localityTree: LocalityTree; + localityTreeErrors: Error[]; sortedZoneConfigs: ZoneConfig$Properties[]; refreshDataDistribution: typeof refreshDataDistribution; refreshNodes: typeof refreshNodes; @@ -170,8 +177,8 @@ class DataDistributionPage extends React.Component {
( [nodes.lastError, liveness.lastError], +); + // tslint:disable-next-line:variable-name const DataDistributionPageConnected = connect( (state: AdminUIState) => ({ - dataDistribution: state.cachedData.dataDistribution.data, + dataDistribution: state.cachedData.dataDistribution, sortedZoneConfigs: sortedZoneConfigs(state), localityTree: selectLocalityTree(state), + localityTreeErrors: localityTreeErrors(state), }), { refreshDataDistribution, diff --git a/pkg/ui/src/views/reports/containers/localities/index.tsx b/pkg/ui/src/views/reports/containers/localities/index.tsx index f59e772efe50..e48011847e37 100644 --- a/pkg/ui/src/views/reports/containers/localities/index.tsx +++ b/pkg/ui/src/views/reports/containers/localities/index.tsx @@ -111,6 +111,7 @@ class Localities extends React.Component {

Localities

(
diff --git a/pkg/ui/src/views/reports/containers/network/index.tsx b/pkg/ui/src/views/reports/containers/network/index.tsx index 746e4ad2e1b1..a9609b0a86a2 100644 --- a/pkg/ui/src/views/reports/containers/network/index.tsx +++ b/pkg/ui/src/views/reports/containers/network/index.tsx @@ -20,9 +20,16 @@ import React from "react"; import { Helmet } from "react-helmet"; import { connect } from "react-redux"; import { RouterState } from "react-router"; +import { createSelector } from "reselect"; import { refreshLiveness, refreshNodes } from "src/redux/apiReducers"; -import { LivenessStatus, NodesSummary, nodesSummarySelector } from "src/redux/nodes"; +import { + LivenessStatus, + NodesSummary, + nodesSummarySelector, + selectLivenessRequestStatus, + selectNodeRequestStatus, +} from "src/redux/nodes"; import { AdminUIState } from "src/redux/state"; import { LongToMoment, NanoToMilli } from "src/util/convert"; import { FixLong } from "src/util/fixLong"; @@ -36,6 +43,7 @@ import Loading from "src/views/shared/components/loading"; interface NetworkOwnProps { nodesSummary: NodesSummary; + nodeSummaryErrors: Error[]; refreshNodes: typeof refreshNodes; refreshLiveness: typeof refreshLiveness; } @@ -456,6 +464,7 @@ class Network extends React.Component {

Network Diagnostics

(
@@ -469,9 +478,16 @@ class Network extends React.Component { } } +const nodeSummaryErrors = createSelector( + selectNodeRequestStatus, + selectLivenessRequestStatus, + (nodes, liveness) => [nodes.lastError, liveness.lastError], +); + function mapStateToProps(state: AdminUIState) { return { nodesSummary: nodesSummarySelector(state), + nodeSummaryErrors: nodeSummaryErrors(state), }; }