From 4fe7d93a5fb130972262c6234e9cac8750d0b8df Mon Sep 17 00:00:00 2001 From: Matthew Todd Date: Thu, 22 Sep 2022 10:41:58 -0400 Subject: [PATCH] ui: sql activity pages depend on node info Fixes #85249. Previously, the statements, transactions, and transaction details pages had an implicit dependency on the `state.adminUI.nodes` list in the redux store, and without that list, the Regions/Nodes columns and filters would be incorrectly hidden for multiregion clusters, unless the user happened to have already browsed through a page (such as statement details) that loaded the list. (This bug was affecting dedicated cloud clusters only. Self-hosted clusters were working properly because their global mechanism to sync alert data happened to [refresh the node list][1].) This change makes the dependency explicit and refreshes the list of nodes on entering those three pages. [1]: https://github.com/cockroachdb/cockroach/blob/6a25c6f4b985ddcd08ed51772f9e8a2645aa4582/pkg/ui/workspaces/db-console/src/redux/alerts.ts#L678-L682 Release note (bug fix): The statements, transactions, and transaction details pages in the admin console now properly show the Regions/Nodes columns and filters for multiregion clusters. --- .../src/statementsPage/statementsPage.fixture.ts | 1 + .../src/statementsPage/statementsPage.tsx | 15 +++++++++++---- .../statementsPage/statementsPageConnected.tsx | 2 ++ .../src/transactionDetails/transactionDetails.tsx | 7 +++++++ .../transactionDetailsConnected.tsx | 2 ++ .../transactionsPage/transactionsPage.stories.tsx | 5 +++++ .../src/transactionsPage/transactionsPage.tsx | 8 ++++++++ .../transactionsPageConnected.tsx | 2 ++ .../src/views/statements/statementsPage.tsx | 2 ++ .../src/views/transactions/transactionDetails.tsx | 7 ++++++- .../src/views/transactions/transactionsPage.tsx | 3 ++- 11 files changed, 48 insertions(+), 6 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts index 1e426f90ef26..6dac499217e0 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts @@ -929,6 +929,7 @@ const statementsPagePropsFixture: StatementsPageProps = { refreshStatementDiagnosticsRequests: noop, refreshStatements: noop, refreshUserSQLRoles: noop, + refreshNodes: noop, resetSQLStats: noop, onTimeScaleChange: noop, onActivateStatementDiagnostics: noop, diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx index 798c4eeb0e59..af806653a969 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx @@ -95,6 +95,7 @@ const sortableTableCx = classNames.bind(sortableTableStyles); export interface StatementsPageDispatchProps { refreshStatements: (req: StatementsRequest) => void; refreshStatementDiagnosticsRequests: () => void; + refreshNodes: () => void; refreshUserSQLRoles: () => void; resetSQLStats: (req: StatementsRequest) => void; dismissAlertMessage: () => void; @@ -343,8 +344,11 @@ export class StatementsPage extends React.Component< } this.props.refreshUserSQLRoles(); - if (!this.props.isTenant && !this.props.hasViewActivityRedactedRole) { - this.props.refreshStatementDiagnosticsRequests(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + if (!this.props.hasViewActivityRedactedRole) { + this.props.refreshStatementDiagnosticsRequests(); + } } } @@ -382,8 +386,11 @@ export class StatementsPage extends React.Component< componentDidUpdate = (): void => { this.updateQueryParams(); - if (!this.props.isTenant && !this.props.hasViewActivityRedactedRole) { - this.props.refreshStatementDiagnosticsRequests(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + if (!this.props.hasViewActivityRedactedRole) { + this.props.refreshStatementDiagnosticsRequests(); + } } }; diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPageConnected.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPageConnected.tsx index 926164bad035..5a3ee380d82b 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPageConnected.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPageConnected.tsx @@ -17,6 +17,7 @@ import { actions as statementDiagnosticsActions } from "src/store/statementDiagn import { actions as analyticsActions } from "src/store/analytics"; import { actions as localStorageActions } from "src/store/localStorage"; import { actions as sqlStatsActions } from "src/store/sqlStats"; +import { actions as nodesActions } from "../store/nodes"; import { StatementsPageDispatchProps, StatementsPageStateProps, @@ -119,6 +120,7 @@ export const ConnectedStatementsPage = withRouter( }, refreshStatementDiagnosticsRequests: () => dispatch(statementDiagnosticsActions.refresh()), + refreshNodes: () => dispatch(nodesActions.refresh()), refreshUserSQLRoles: () => dispatch(uiConfigActions.refreshUserSQLRoles()), resetSQLStats: (req: StatementsRequest) => diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx index 048b076a81ea..7d649a2def23 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx @@ -95,6 +95,7 @@ export interface TransactionDetailsStateProps { export interface TransactionDetailsDispatchProps { refreshData: (req?: StatementsRequest) => void; + refreshNodes: () => void; refreshUserSQLRoles: () => void; onTimeScaleChange: (ts: TimeScale) => void; } @@ -195,10 +196,16 @@ export class TransactionDetails extends React.Component< componentDidMount(): void { this.refreshData(""); this.props.refreshUserSQLRoles(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + } } componentDidUpdate(prevProps: TransactionDetailsProps): void { this.getTransactionStateInfo(prevProps.transactionFingerprintId); + if (!this.props.isTenant) { + this.props.refreshNodes(); + } } onChangeSortSetting = (ss: SortSetting): void => { diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsConnected.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsConnected.tsx index cef17ea0bbba..5d7bf675d2b0 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsConnected.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsConnected.tsx @@ -14,6 +14,7 @@ import { RouteComponentProps, withRouter } from "react-router-dom"; import { Dispatch } from "redux"; import { AppState, uiConfigActions } from "src/store"; +import { actions as nodesActions } from "../store/nodes"; import { actions as sqlStatsActions } from "src/store/sqlStats"; import { TransactionDetails, @@ -89,6 +90,7 @@ const mapDispatchToProps = ( ): TransactionDetailsDispatchProps => ({ refreshData: (req?: StatementsRequest) => dispatch(sqlStatsActions.refresh(req)), + refreshNodes: () => dispatch(nodesActions.refresh()), refreshUserSQLRoles: () => dispatch(uiConfigActions.refreshUserSQLRoles()), onTimeScaleChange: (ts: TimeScale) => { dispatch( diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.stories.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.stories.tsx index 6b57ab16bddf..039727dec10d 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.stories.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.stories.tsx @@ -45,6 +45,7 @@ storiesOf("Transactions Page", module) onFilterChange={noop} onSortingChange={noop} refreshData={noop} + refreshNodes={noop} resetSQLStats={noop} search={""} sortSetting={sortSetting} @@ -63,6 +64,7 @@ storiesOf("Transactions Page", module) onFilterChange={noop} onSortingChange={noop} refreshData={noop} + refreshNodes={noop} resetSQLStats={noop} search={""} sortSetting={sortSetting} @@ -89,6 +91,7 @@ storiesOf("Transactions Page", module) onFilterChange={noop} onSortingChange={noop} refreshData={noop} + refreshNodes={noop} resetSQLStats={noop} search={""} sortSetting={sortSetting} @@ -108,6 +111,7 @@ storiesOf("Transactions Page", module) onFilterChange={noop} onSortingChange={noop} refreshData={noop} + refreshNodes={noop} resetSQLStats={noop} search={""} sortSetting={sortSetting} @@ -134,6 +138,7 @@ storiesOf("Transactions Page", module) onFilterChange={noop} onSortingChange={noop} refreshData={noop} + refreshNodes={noop} resetSQLStats={noop} search={""} sortSetting={sortSetting} diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx index a7c48d7ef701..92d6c174eee8 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx @@ -98,6 +98,7 @@ export interface TransactionsPageStateProps { export interface TransactionsPageDispatchProps { refreshData: (req: StatementsRequest) => void; + refreshNodes: () => void; resetSQLStats: (req: StatementsRequest) => void; onTimeScaleChange?: (ts: TimeScale) => void; onColumnsChange?: (selectedColumns: string[]) => void; @@ -234,6 +235,10 @@ export class TransactionsPage extends React.Component< Math.max(0, nextRefresh.diff(now, "milliseconds")), ); } + + if (!this.props.isTenant) { + this.props.refreshNodes(); + } } componentWillUnmount(): void { @@ -274,6 +279,9 @@ export class TransactionsPage extends React.Component< componentDidUpdate(): void { this.updateQueryParams(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + } } onChangeSortSetting = (ss: SortSetting): void => { diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPageConnected.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPageConnected.tsx index ac8555ab7a94..fe5fbb064ebb 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPageConnected.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPageConnected.tsx @@ -13,6 +13,7 @@ import { RouteComponentProps, withRouter } from "react-router-dom"; import { Dispatch } from "redux"; import { AppState } from "src/store"; +import { actions as nodesActions } from "src/store/nodes"; import { actions as sqlStatsActions } from "src/store/sqlStats"; import { TransactionsPageStateProps, @@ -87,6 +88,7 @@ export const TransactionsPageConnected = withRouter( fingerprintsPageProps: { refreshData: (req: StatementsRequest) => dispatch(sqlStatsActions.refresh(req)), + refreshNodes: () => dispatch(nodesActions.refresh()), resetSQLStats: (req: StatementsRequest) => dispatch(sqlStatsActions.reset(req)), onTimeScaleChange: (ts: TimeScale) => { diff --git a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx index 9597e6cf8aa3..c7e7d28b67d6 100644 --- a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx +++ b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx @@ -14,6 +14,7 @@ import { createSelector } from "reselect"; import { RouteComponentProps, withRouter } from "react-router-dom"; import * as protos from "src/js/protos"; import { + refreshNodes, refreshStatementDiagnosticsRequests, refreshStatements, refreshUserSQLRoles, @@ -287,6 +288,7 @@ const fingerprintsPageActions = { refreshStatements, onTimeScaleChange: setGlobalTimeScaleAction, refreshStatementDiagnosticsRequests, + refreshNodes, refreshUserSQLRoles, resetSQLStats: resetSQLStatsAction, dismissAlertMessage: () => { diff --git a/pkg/ui/workspaces/db-console/src/views/transactions/transactionDetails.tsx b/pkg/ui/workspaces/db-console/src/views/transactions/transactionDetails.tsx index ab785e201257..b009b6b864a8 100644 --- a/pkg/ui/workspaces/db-console/src/views/transactions/transactionDetails.tsx +++ b/pkg/ui/workspaces/db-console/src/views/transactions/transactionDetails.tsx @@ -11,7 +11,11 @@ import { connect } from "react-redux"; import { createSelector } from "reselect"; import { RouteComponentProps, withRouter } from "react-router-dom"; -import { refreshStatements, refreshUserSQLRoles } from "src/redux/apiReducers"; +import { + refreshNodes, + refreshStatements, + refreshUserSQLRoles, +} from "src/redux/apiReducers"; import { AdminUIState } from "src/redux/state"; import { txnFingerprintIdAttr } from "src/util/constants"; import { getMatchParamByName } from "src/util/query"; @@ -80,6 +84,7 @@ export default withRouter( }, { refreshData: refreshStatements, + refreshNodes, refreshUserSQLRoles, onTimeScaleChange: setGlobalTimeScaleAction, }, diff --git a/pkg/ui/workspaces/db-console/src/views/transactions/transactionsPage.tsx b/pkg/ui/workspaces/db-console/src/views/transactions/transactionsPage.tsx index 60d59cdc2b5c..95caac82d3eb 100644 --- a/pkg/ui/workspaces/db-console/src/views/transactions/transactionsPage.tsx +++ b/pkg/ui/workspaces/db-console/src/views/transactions/transactionsPage.tsx @@ -11,7 +11,7 @@ import { connect } from "react-redux"; import { createSelector } from "reselect"; import { RouteComponentProps, withRouter } from "react-router-dom"; -import { refreshStatements } from "src/redux/apiReducers"; +import { refreshNodes, refreshStatements } from "src/redux/apiReducers"; import { resetSQLStatsAction } from "src/redux/sqlStats"; import { CachedDataReducerState } from "src/redux/cachedDataReducer"; import { AdminUIState } from "src/redux/state"; @@ -95,6 +95,7 @@ export const transactionColumnsLocalSetting = new LocalSetting( const fingerprintsPageActions = { refreshData: refreshStatements, + refreshNodes, resetSQLStats: resetSQLStatsAction, onTimeScaleChange: setGlobalTimeScaleAction, // We use `null` when the value was never set and it will show all columns.