From d2f172dc6d7ec8a9080483b26c632e7a0eb7f461 Mon Sep 17 00:00:00 2001 From: Xin Hao Zhang Date: Tue, 5 Oct 2021 13:55:37 -0400 Subject: [PATCH] ui/cluster-ui: make app name a query search parameter in stmts page Fixes: #70790 Previously, the selected app was derived from a route param on the statements page. All other filters are derived from query search parameters on the page. This commit makes the app name a query search parameter, as is the case in the transactions page. Release note: None --- .../cluster-ui/src/queryFilter/filter.tsx | 30 ++++++++++++----- .../statementDetails.selectors.ts | 6 +++- .../src/statementDetails/statementDetails.tsx | 4 ++- .../statementsPage.selectors.ts | 4 +-- .../src/statementsPage/statementsPage.tsx | 33 ++++++------------- pkg/ui/workspaces/db-console/src/app.tsx | 9 ++--- .../src/views/statements/statementDetails.tsx | 6 +++- .../src/views/statements/statements.spec.tsx | 17 +++++++++- .../src/views/statements/statementsPage.tsx | 4 +-- 9 files changed, 70 insertions(+), 43 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx b/pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx index 808e834698bf..37e3011a5473 100644 --- a/pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx @@ -69,7 +69,7 @@ const timeUnit = [ ]; export const defaultFilters: Filters = { - app: "All", + app: "", timeNumber: "0", timeUnit: "seconds", fullScan: false, @@ -89,7 +89,9 @@ export const defaultFilters: Filters = { * @return Filters: the default filters with updated keys existing on * queryString */ -export const getFiltersFromQueryString = (queryString: string) => { +export const getFiltersFromQueryString = ( + queryString: string, +): Record => { const searchParams = new URLSearchParams(queryString); return Object.keys(defaultFilters).reduce( @@ -97,7 +99,7 @@ export const getFiltersFromQueryString = (queryString: string) => { const defaultValue = defaultFilters[filter]; const queryStringFilter = searchParams.get(filter); const filterValue = - queryStringFilter === null + queryStringFilter == null ? defaultValue : defaultValue.constructor(searchParams.get(filter)); return { [filter]: filterValue, ...filters }; @@ -114,7 +116,7 @@ export const getFiltersFromQueryString = (queryString: string) => { * we want to consider 0 active Filters */ export const inactiveFiltersState: Filters = { - app: "All", + app: "", timeNumber: "0", fullScan: false, sqlType: "", @@ -123,7 +125,7 @@ export const inactiveFiltersState: Filters = { nodes: "", }; -export const calculateActiveFilters = (filters: Filters) => { +export const calculateActiveFilters = (filters: Filters): number => { return Object.keys(inactiveFiltersState).reduce( (active, filter: keyof Filters) => { return inactiveFiltersState[filter] !== filters[filter] @@ -185,7 +187,19 @@ export class Filter extends React.Component { this.setState({ hide: true }); }; - handleChange = (event: any, field: string) => { + handleSelectChange = ( + event: { label: string; value: string }, + field: string, + ): void => { + this.setState({ + filters: { + ...this.state.filters, + [field]: event.value, + }, + }); + }; + + handleChange = (event: any, field: string): void => { this.setState({ filters: { ...this.state.filters, @@ -419,7 +433,7 @@ export class Filter extends React.Component {
App
unit.label == filters.timeUnit)} - onChange={e => this.handleChange(e, "timeUnit")} + onChange={e => this.handleSelectChange(e, "timeUnit")} className={timePair.timeUnit} styles={customStylesSmall} /> diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.selectors.ts index 89c28d965465..92d293707193 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.selectors.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.selectors.ts @@ -150,7 +150,11 @@ export const selectStatement = createSelector( statement, stats: combineStatementStats(results.map(s => s.stats)), byNode: coalesceNodeStats(results), - app: _.uniq(results.map(s => s.app)), + app: _.uniq( + results.map(s => + s.app.startsWith(internalAppNamePrefix) ? "(internal)" : s.app, + ), + ), database: queryByName(props.location, databaseAttr), distSQL: fractionMatching(results, s => s.distSQL), vec: fractionMatching(results, s => s.vec), diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx index b9841a001393..70fc658fac57 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx @@ -177,10 +177,12 @@ function AppLink(props: { app: string }) { return (unset); } + const searchParams = new URLSearchParams({ [appAttr]: props.app }); + return ( {props.app} diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts index 4e39104e0c49..adcc70464717 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts @@ -17,7 +17,7 @@ import { ExecutionStatistics, flattenStatementStats, formatDate, - getMatchParamByName, + queryByName, statementKey, StatementStatistics, TimestampToMoment, @@ -144,7 +144,7 @@ export const selectStatements = createSelector( return null; } let statements = flattenStatementStats(state.data.statements); - const app = getMatchParamByName(props.match, appAttr); + const app = queryByName(props.location, appAttr); const isInternal = (statement: ExecutionStatistics) => statement.app.startsWith(state.data.internal_app_name_prefix); diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx index f3dcddfb2192..71acddb3d2e9 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx @@ -10,7 +10,7 @@ import React from "react"; import { RouteComponentProps } from "react-router-dom"; -import { isNil, merge, forIn } from "lodash"; +import { isNil, merge } from "lodash"; import Helmet from "react-helmet"; import moment, { Moment } from "moment"; import classNames from "classnames/bind"; @@ -179,19 +179,19 @@ export class StatementsPage extends React.Component< }; }; - syncHistory = (params: Record) => { + syncHistory = (params: Record): void => { const { history } = this.props; - const currentSearchParams = new URLSearchParams(history.location.search); + const nextSearchParams = new URLSearchParams(history.location.search); - forIn(params, (value, key) => { + Object.entries(params).forEach(([key, value]) => { if (!value) { - currentSearchParams.delete(key); + nextSearchParams.delete(key); } else { - currentSearchParams.set(key, value); + nextSearchParams.set(key, value); } }); - history.location.search = currentSearchParams.toString(); + history.location.search = nextSearchParams.toString(); history.replace(history.location); }; @@ -223,17 +223,6 @@ export class StatementsPage extends React.Component< ); }; - selectApp = (value: string): void => { - if (value == "All") value = ""; - const { history, onFilterChange } = this.props; - history.location.pathname = `/statements/${encodeURIComponent(value)}`; - history.replace(history.location); - this.resetPagination(); - if (onFilterChange) { - onFilterChange(value); - } - }; - resetPagination = (): void => { this.setState(prevState => { return { @@ -307,7 +296,6 @@ export class StatementsPage extends React.Component< regions: filters.regions, nodes: filters.nodes, }); - this.selectApp(filters.app); }; onClearSearchField = (): void => { @@ -334,7 +322,6 @@ export class StatementsPage extends React.Component< regions: undefined, nodes: undefined, }); - this.selectApp(""); }; filteredStatementsData = (): AggregateStatistics[] => { @@ -422,8 +409,8 @@ export class StatementsPage extends React.Component< const { statements, databases, - match, lastReset, + location, onDiagnosticsReportDownload, onStatementClick, resetSQLStats, @@ -432,9 +419,9 @@ export class StatementsPage extends React.Component< nodeRegions, isTenant, } = this.props; - const appAttrValue = getMatchParamByName(match, appAttr); + const appAttrValue = queryByName(location, appAttr); const selectedApp = appAttrValue || ""; - const appOptions = [{ value: "all", label: "All" }]; + const appOptions = [{ value: "", label: "All" }]; this.props.apps.forEach(app => appOptions.push({ value: app, label: app })); const data = this.filteredStatementsData(); const totalWorkload = calculateTotalWorkload(data); diff --git a/pkg/ui/workspaces/db-console/src/app.tsx b/pkg/ui/workspaces/db-console/src/app.tsx index a4a3a14177ba..91844302bd8b 100644 --- a/pkg/ui/workspaces/db-console/src/app.tsx +++ b/pkg/ui/workspaces/db-console/src/app.tsx @@ -13,7 +13,7 @@ import { History } from "history"; import "nvd3/build/nv.d3.min.css"; import React from "react"; import { Provider } from "react-redux"; -import { Redirect, Route, Switch } from "react-router-dom"; +import { Redirect, Route, RouteChildrenProps, Switch } from "react-router-dom"; import "react-select/dist/react-select.css"; import { Action, Store } from "redux"; import { AdminUIState } from "src/redux/state"; @@ -66,6 +66,7 @@ import SessionDetails from "src/views/sessions/sessionDetails"; import TransactionsPage from "src/views/transactions/transactionsPage"; import StatementsDiagnosticsHistoryView from "src/views/reports/containers/statementDiagnosticsHistory"; import { RedirectToStatementDetails } from "src/routes/RedirectToStatementDetails"; +import { getMatchParamByName } from "./util/query"; import "styl/app.styl"; // NOTE: If you are adding a new path to the router, and that path contains any @@ -179,10 +180,10 @@ export const App: React.FC = (props: AppProps) => { {/* statement statistics */} - s.stats)), byNode: coalesceNodeStats(results), - app: _.uniq(results.map(s => s.app)), + app: _.uniq( + results.map(s => + s.app.startsWith(internalAppNamePrefix) ? "(internal)" : s.app, + ), + ), database: queryByName(props.location, databaseAttr), distSQL: fractionMatching(results, s => s.distSQL), vec: fractionMatching(results, s => s.vec), diff --git a/pkg/ui/workspaces/db-console/src/views/statements/statements.spec.tsx b/pkg/ui/workspaces/db-console/src/views/statements/statements.spec.tsx index 54c5a369920c..f63be5eacdd9 100644 --- a/pkg/ui/workspaces/db-console/src/views/statements/statements.spec.tsx +++ b/pkg/ui/workspaces/db-console/src/views/statements/statements.spec.tsx @@ -588,6 +588,21 @@ function makeRoutePropsWithParams(params: { [key: string]: string }) { }; } +function makeRoutePropsWithSearchParams(params: { [key: string]: string }) { + const history = H.createHashHistory(); + history.location.search = new URLSearchParams(params).toString(); + return { + location: history.location, + history, + match: { + url: "", + path: history.location.pathname, + isExact: false, + params: {}, + }, + }; +} + function makeEmptyRouteProps(): RouteComponentProps { const history = H.createHashHistory(); return { @@ -603,7 +618,7 @@ function makeEmptyRouteProps(): RouteComponentProps { } function makeRoutePropsWithApp(app: string) { - return makeRoutePropsWithParams({ + return makeRoutePropsWithSearchParams({ [appAttr]: app, }); } 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 1f0e46f2b6e9..dbd55813b905 100644 --- a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx +++ b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx @@ -34,7 +34,7 @@ import { PrintTime } from "src/views/reports/containers/range/print"; import { selectDiagnosticsReportsPerStatement } from "src/redux/statements/statementsSelectors"; import { createStatementDiagnosticsAlertLocalSetting } from "src/redux/alerts"; import { statementsDateRangeLocalSetting } from "src/redux/statementsDateRange"; -import { getMatchParamByName } from "src/util/query"; +import { queryByName } from "src/util/query"; import { StatementsPage, AggregateStatistics } from "@cockroachlabs/cluster-ui"; import { @@ -79,7 +79,7 @@ export const selectStatements = createSelector( return null; } let statements = flattenStatementStats(state.data.statements); - const app = getMatchParamByName(props.match, appAttr); + const app = queryByName(props.location, appAttr); const isInternal = (statement: ExecutionStatistics) => statement.app.startsWith(state.data.internal_app_name_prefix);