Skip to content

Commit

Permalink
ui: hide reset options for non-admins
Browse files Browse the repository at this point in the history
Previously, we were showing reset sql stats and reset
index stats options for all users. If a non-admin user tried
to reset, it wasn't doing the reset as expected, but no
message was being displayed.
This commit now hides these options for non-admin so it's
no longer confusing to see the options but not being
able to use it.

Fixes cockroachdb#95213

Release note (ui change): Remove `reset sql stats` and `reset index stats`
from the Console when the user is a non-admin.
  • Loading branch information
maryliag committed Jan 17, 2023
1 parent 0efbdb0 commit 422176b
Show file tree
Hide file tree
Showing 20 changed files with 120 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const withLoadingIndicator: DatabaseTablePageProps = {
refreshIndexStats: () => {},
resetIndexUsageStats: () => {},
refreshSettings: () => {},
refreshUserSQLRoles: () => {},
};

const name = randomName();
Expand Down Expand Up @@ -167,6 +168,7 @@ const withData: DatabaseTablePageProps = {
refreshIndexStats: () => {},
resetIndexUsageStats: () => {},
refreshSettings: () => {},
refreshUserSQLRoles: () => {},
};

storiesOf("Database Table Page", module)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import { Loading } from "../loading";
import { CockroachCloudContext } from "../contexts";
import IdxRecAction from "../insights/indexActionBtn";
import { RecommendationType } from "../indexDetailsPage";
import { UIConfigState } from "../store";

const cx = classNames.bind(styles);
const booleanSettingCx = classnames.bind(booleanSettingStyles);
Expand Down Expand Up @@ -108,6 +109,7 @@ export interface DatabaseTablePageData {
indexStats: DatabaseTablePageIndexStats;
showNodeRegionsSection?: boolean;
automaticStatsCollectionEnabled?: boolean;
hasAdminRole?: UIConfigState["hasAdminRole"];
}

export interface DatabaseTablePageDataDetails {
Expand Down Expand Up @@ -166,6 +168,7 @@ export interface DatabaseTablePageActions {
refreshIndexStats?: (database: string, table: string) => void;
resetIndexUsageStats?: (database: string, table: string) => void;
refreshNodes?: () => void;
refreshUserSQLRoles: () => void;
}

export type DatabaseTablePageProps = DatabaseTablePageData &
Expand Down Expand Up @@ -247,6 +250,7 @@ export class DatabaseTablePage extends React.Component<
}

private refresh() {
this.props.refreshUserSQLRoles();
if (this.props.refreshNodes != null) {
this.props.refreshNodes();
}
Expand Down Expand Up @@ -493,6 +497,7 @@ export class DatabaseTablePage extends React.Component<
};

render(): React.ReactElement {
const { hasAdminRole } = this.props;
return (
<div className="root table-area">
<section className={baseHeadingClasses.wrapper}>
Expand Down Expand Up @@ -644,23 +649,25 @@ export class DatabaseTablePage extends React.Component<
{this.getLastResetString()}
</div>
</Tooltip>
<div>
<a
className={cx(
"action",
"separator",
"index-stats__reset-btn",
)}
onClick={() =>
this.props.resetIndexUsageStats(
this.props.databaseName,
this.props.name,
)
}
>
Reset all index stats
</a>
</div>
{hasAdminRole && (
<div>
<a
className={cx(
"action",
"separator",
"index-stats__reset-btn",
)}
onClick={() =>
this.props.resetIndexUsageStats(
this.props.databaseName,
this.props.name,
)
}
>
Reset all index stats
</a>
</div>
)}
</div>
</div>
<IndexUsageStatsTable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
} from "../util";
import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
import { IndexDetailsPageData } from "./indexDetailsPage";
import { selectIsTenant } from "../store/uiConfig";
import { selectHasAdminRole, selectIsTenant } from "../store/uiConfig";
import { BreadcrumbItem } from "../breadcrumbs";
import { RecommendationType as RecType } from "./indexDetailsPage";
const { RecommendationType } = cockroach.sql.IndexRecommendation;
Expand All @@ -39,7 +39,16 @@ export const selectIndexDetails = createSelector(
getMatchParamByName(props.match, indexNameAttr),
(state: AppState) => state.adminUI.indexStats.cachedData,
(state: AppState) => selectIsTenant(state),
(database, schema, table, index, indexStats): IndexDetailsPageData => {
(state: AppState) => selectHasAdminRole(state),
(
database,
schema,
table,
index,
indexStats,
isTenant,
hasAdminRole,
): IndexDetailsPageData => {
const stats = indexStats[generateTableID(database, table)];
const details = stats?.data?.statistics.filter(
stat => stat.index_name === index, // index names must be unique for a table
Expand Down Expand Up @@ -70,6 +79,7 @@ export const selectIndexDetails = createSelector(
table,
index,
),
hasAdminRole: hasAdminRole,
details: {
loading: !!stats?.inFlight,
loaded: !!stats?.valid,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

import { AppState } from "../store";
import { AppState, uiConfigActions } from "../store";
import { RouteComponentProps, withRouter } from "react-router-dom";
import { selectIndexDetails } from "./indexDetails.selectors";
import { Dispatch } from "redux";
Expand Down Expand Up @@ -42,6 +42,7 @@ const mapDispatchToProps = (dispatch: Dispatch): IndexDetailPageActions => ({
);
},
refreshNodes: () => dispatch(nodesActions.refresh()),
refreshUserSQLRoles: () => dispatch(uiConfigActions.refreshUserSQLRoles()),
});

export const ConnectedIndexDetailsPage = withRouter<any, any>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const withData: IndexDetailsPageProps = {
refreshIndexStats: () => {},
resetIndexUsageStats: () => {},
refreshNodes: () => {},
refreshUserSQLRoles: () => {},
};

storiesOf("Index Details Page", module)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import React from "react";
import classNames from "classnames/bind";
import { SortSetting } from "src/sortedtable";

import { UIConfigState } from "../store";
import styles from "./indexDetailsPage.module.scss";
import { baseHeadingClasses } from "src/transactionsPage/transactionsPageClasses";
import { CaretRight } from "../icon/caretRight";
Expand Down Expand Up @@ -58,6 +58,7 @@ export interface IndexDetailsPageData {
indexName: string;
details: IndexDetails;
breadcrumbItems: BreadcrumbItem[];
hasAdminRole?: UIConfigState["hasAdminRole"];
}

interface IndexDetails {
Expand All @@ -81,6 +82,7 @@ export interface IndexDetailPageActions {
refreshIndexStats?: (database: string, table: string) => void;
resetIndexUsageStats?: (database: string, table: string) => void;
refreshNodes?: () => void;
refreshUserSQLRoles: () => void;
}

export type IndexDetailsPageProps = IndexDetailsPageData &
Expand Down Expand Up @@ -113,6 +115,7 @@ export class IndexDetailsPage extends React.Component<
}

private refresh() {
this.props.refreshUserSQLRoles();
if (this.props.refreshNodes != null) {
this.props.refreshNodes();
}
Expand Down Expand Up @@ -210,6 +213,8 @@ export class IndexDetailsPage extends React.Component<
}

render(): React.ReactElement {
const { hasAdminRole } = this.props;

return (
<div className={cx("page-container")}>
<div className="root table-area">
Expand All @@ -235,23 +240,25 @@ export class IndexDetailsPage extends React.Component<
{this.getTimestampString(this.props.details.lastReset)}
</div>
</Tooltip>
<div>
<a
className={cx(
"action",
"separator",
"index-stats__reset-btn",
)}
onClick={() =>
this.props.resetIndexUsageStats(
this.props.databaseName,
this.props.tableName,
)
}
>
Reset all index stats
</a>
</div>
{hasAdminRole && (
<div>
<a
className={cx(
"action",
"separator",
"index-stats__reset-btn",
)}
onClick={() =>
this.props.resetIndexUsageStats(
this.props.databaseName,
this.props.tableName,
)
}
>
Reset all index stats
</a>
</div>
)}
</div>
</div>
<section className={baseHeadingClasses.wrapper}>
Expand Down
4 changes: 0 additions & 4 deletions pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,6 @@ export const MemoryUsageItem: React.FC<{
export class SessionDetails extends React.Component<SessionDetailsProps> {
terminateSessionRef: React.RefObject<TerminateSessionModalRef>;
terminateQueryRef: React.RefObject<TerminateQueryModalRef>;
static defaultProps = {
uiConfig: { showGatewayNodeLink: true },
isTenant: false,
};

componentDidMount(): void {
if (!this.props.isTenant) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,6 @@ export class DiagnosticsView extends React.Component<
DiagnosticsViewProps,
DiagnosticsViewState
> {
static defaultProps: Partial<DiagnosticsViewProps> = {
showDiagnosticsViewLink: true,
};
columns: ColumnsConfig<IStatementDiagnosticsReport> = [
{
key: "activatedOn",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,6 @@ export class StatementDetails extends React.Component<
}
}

static defaultProps: Partial<StatementDetailsProps> = {
onDiagnosticBundleDownload: _.noop,
uiConfig: {
showStatementDiagnosticsLink: true,
},
isTenant: false,
hasViewActivityRedactedRole: false,
};

hasDiagnosticReports = (): boolean =>
this.props.diagnosticsReports.length > 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,7 @@ const statementsPagePropsFixture: StatementsPageProps = {
columns: null,
isTenant: false,
hasViewActivityRedactedRole: false,
hasAdminRole: true,
dismissAlertMessage: noop,
refreshStatementDiagnosticsRequests: noop,
refreshStatements: noop,
Expand Down
27 changes: 14 additions & 13 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ export interface StatementsPageStateProps {
search: string;
isTenant?: UIConfigState["isTenant"];
hasViewActivityRedactedRole?: UIConfigState["hasViewActivityRedactedRole"];
hasAdminRole?: UIConfigState["hasAdminRole"];
}

export interface StatementsPageState {
Expand Down Expand Up @@ -213,11 +214,6 @@ export class StatementsPage extends React.Component<
}
}

static defaultProps: Partial<StatementsPageProps> = {
isTenant: false,
hasViewActivityRedactedRole: false,
};

getStateFromHistory = (): Partial<StatementsPageState> => {
const {
history,
Expand Down Expand Up @@ -673,6 +669,7 @@ export class StatementsPage extends React.Component<
search,
isTenant,
nodeRegions,
hasAdminRole,
} = this.props;

const nodes = isTenant
Expand Down Expand Up @@ -728,14 +725,18 @@ export class StatementsPage extends React.Component<
setTimeScale={this.changeTimeScale}
/>
</PageConfigItem>
<PageConfigItem
className={`${commonStyles("separator")} ${cx("reset-btn-area")} `}
>
<ClearStats
resetSQLStats={this.resetSQLStats}
tooltipType="statement"
/>
</PageConfigItem>
{hasAdminRole && (
<PageConfigItem
className={`${commonStyles("separator")} ${cx(
"reset-btn-area",
)} `}
>
<ClearStats
resetSQLStats={this.resetSQLStats}
tooltipType="statement"
/>
</PageConfigItem>
)}
</PageConfig>
<div className={cx("table-area")}>
<Loading
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
import {
selectIsTenant,
selectHasViewActivityRedactedRole,
selectHasAdminRole,
} from "../store/uiConfig";
import { nodeRegionsByIDSelector } from "../store/nodes";
import { StatementsRequest } from "src/api/statementsApi";
Expand Down Expand Up @@ -94,6 +95,7 @@ export const ConnectedStatementsPage = withRouter(
filters: selectFilters(state),
isTenant: selectIsTenant(state),
hasViewActivityRedactedRole: selectHasViewActivityRedactedRole(state),
hasAdminRole: selectHasAdminRole(state),
lastReset: selectLastReset(state),
nodeRegions: selectIsTenant(state)
? {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export type UIConfigState = {
isTenant: boolean;
userSQLRoles: string[];
hasViewActivityRedactedRole: boolean;
hasAdminRole: boolean;
pages: {
statementDetails: {
showStatementDiagnosticsLink: boolean;
Expand All @@ -32,6 +33,7 @@ const initialState: UIConfigState = {
isTenant: false,
userSQLRoles: [],
hasViewActivityRedactedRole: false,
hasAdminRole: false,
pages: {
statementDetails: {
showStatementDiagnosticsLink: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,7 @@ export const selectHasViewActivityRedactedRole = createSelector(
selectUIConfig,
uiConfig => uiConfig.userSQLRoles.includes("VIEWACTIVITYREDACTED"),
);

export const selectHasAdminRole = createSelector(selectUIConfig, uiConfig =>
uiConfig.userSQLRoles.includes("ADMIN"),
);
Loading

0 comments on commit 422176b

Please sign in to comment.