Skip to content

Commit

Permalink
ui/cluster-ui: make app name a query search parameter in stmts page
Browse files Browse the repository at this point in the history
Fixes: cockroachdb#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
  • Loading branch information
xinhaoz committed Oct 7, 2021
1 parent 95affc1 commit d2f172d
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 43 deletions.
30 changes: 22 additions & 8 deletions pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const timeUnit = [
];

export const defaultFilters: Filters = {
app: "All",
app: "",
timeNumber: "0",
timeUnit: "seconds",
fullScan: false,
Expand All @@ -89,15 +89,17 @@ 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<string, string> => {
const searchParams = new URLSearchParams(queryString);

return Object.keys(defaultFilters).reduce(
(filters, filter: keyof Filters): Filters => {
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 };
Expand All @@ -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: "",
Expand All @@ -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]
Expand Down Expand Up @@ -185,7 +187,19 @@ export class Filter extends React.Component<QueryFilter, FilterState> {
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,
Expand Down Expand Up @@ -419,7 +433,7 @@ export class Filter extends React.Component<QueryFilter, FilterState> {
<div className={filterLabel.top}>App</div>
<Select
options={apps}
onChange={e => this.handleChange(e, "app")}
onChange={e => this.handleSelectChange(e, "app")}
value={apps.filter(app => app.value === filters.app)}
placeholder="All"
styles={customStyles}
Expand All @@ -441,7 +455,7 @@ export class Filter extends React.Component<QueryFilter, FilterState> {
<Select
options={timeUnit}
value={timeUnit.filter(unit => unit.label == filters.timeUnit)}
onChange={e => this.handleChange(e, "timeUnit")}
onChange={e => this.handleSelectChange(e, "timeUnit")}
className={timePair.timeUnit}
styles={customStylesSmall}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,12 @@ function AppLink(props: { app: string }) {
return <span className={cx("app-name", "app-name__unset")}>(unset)</span>;
}

const searchParams = new URLSearchParams({ [appAttr]: props.app });

return (
<Link
className={cx("app-name")}
to={`/statements/${encodeURIComponent(props.app)}`}
to={`/statements/?${searchParams.toString()}`}
>
{props.app}
</Link>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
ExecutionStatistics,
flattenStatementStats,
formatDate,
getMatchParamByName,
queryByName,
statementKey,
StatementStatistics,
TimestampToMoment,
Expand Down Expand Up @@ -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);

Expand Down
33 changes: 10 additions & 23 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -179,19 +179,19 @@ export class StatementsPage extends React.Component<
};
};

syncHistory = (params: Record<string, string | undefined>) => {
syncHistory = (params: Record<string, string | undefined>): 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);
};

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -307,7 +296,6 @@ export class StatementsPage extends React.Component<
regions: filters.regions,
nodes: filters.nodes,
});
this.selectApp(filters.app);
};

onClearSearchField = (): void => {
Expand All @@ -334,7 +322,6 @@ export class StatementsPage extends React.Component<
regions: undefined,
nodes: undefined,
});
this.selectApp("");
};

filteredStatementsData = (): AggregateStatistics[] => {
Expand Down Expand Up @@ -422,8 +409,8 @@ export class StatementsPage extends React.Component<
const {
statements,
databases,
match,
lastReset,
location,
onDiagnosticsReportDownload,
onStatementClick,
resetSQLStats,
Expand All @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions pkg/ui/workspaces/db-console/src/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -179,10 +180,10 @@ export const App: React.FC<AppProps> = (props: AppProps) => {

{/* statement statistics */}
<Route exact path="/statements" component={StatementsPage} />
<Route
<Redirect
exact
path={`/statements/:${appAttr}`}
component={StatementsPage}
from={`/statements/:${appAttr}`}
to={`/statements?${appAttr}=:${appAttr}`}
/>
<Route
exact
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<any> {
const history = H.createHashHistory();
return {
Expand All @@ -603,7 +618,7 @@ function makeEmptyRouteProps(): RouteComponentProps<any> {
}

function makeRoutePropsWithApp(app: string) {
return makeRoutePropsWithParams({
return makeRoutePropsWithSearchParams({
[appAttr]: app,
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit d2f172d

Please sign in to comment.