Merge pull request #70804 from xinhaoz/backport21.2-70600

release-21.2: ui/cluster-ui: fix routing to statement details page

pkg/ui/workspaces/cluster-ui/src/api/fetchData.ts

@@ -11,7 +11,6 @@
 import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
 import { RequestError } from "../util";
 import { getBasePath } from "./basePath";
-import { stringify } from "querystring";
 
 interface ProtoBuilder<
   P extends ConstructorType,
@@ -30,18 +29,6 @@ export function toArrayBuffer(encodedRequest: Uint8Array): ArrayBuffer {
   );
 }
 
-// propsToQueryString is a helper function that converts a set of object
-// properties to a query string
-// - keys with null or undefined values will be skipped
-// - non-string values will be toString'd
-export function propsToQueryString(props: { [k: string]: any }) {
-  const params = new URLSearchParams();
-  Object.entries(props).forEach(
-    ([k, v]: [string, any]) => v != null && params.set(k, v.toString()),
-  );
-  return params.toString();
-}
-
 /**
  * @param RespBuilder expects protobuf stub class to build decode response;
  * @param path relative URL path for requested resource;

pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts

@@ -9,7 +9,8 @@
 // licenses/APL.txt.
 
 import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
-import { fetchData, propsToQueryString } from "src/api";
+import { fetchData } from "src/api";
+import { propsToQueryString } from "src/util";
 
 const STATEMENTS_PATH = "/_status/statements";
 
@@ -28,7 +29,7 @@ export const getCombinedStatements = (
   const queryStr = propsToQueryString({
     start: req.start.toInt(),
     end: req.end.toInt(),
-    combined: true,
+    combined: req.combined,
   });
   return fetchData(
     cockroach.server.serverpb.StatementsResponse,

pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx

@@ -14,7 +14,7 @@ import { sessionAttr } from "src/util/constants";
 import { Helmet } from "react-helmet";
 import { Loading } from "../loading";
 import _ from "lodash";
-import { Link, RouteComponentProps, withRouter } from "react-router-dom";
+import { Link, RouteComponentProps } from "react-router-dom";
 
 import { SessionInfo } from "./sessionsTable";
 
@@ -320,7 +320,6 @@ export class SessionDetails extends React.Component<SessionDetailsProps> {
                     statementNoConstants: stmt.sql_no_constants,
                     implicitTxn: session.active_txn?.implicit,
                     app: "",
-                    search: "",
                   })}
                   onClick={() =>
                     this.props.onStatementClick && this.props.onStatementClick()

pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.selectors.ts

@@ -10,6 +10,7 @@
 
 import { createSelector } from "@reduxjs/toolkit";
 import { RouteComponentProps, match as Match } from "react-router-dom";
+import { Location } from "history";
 import _ from "lodash";
 import { AppState } from "../store";
 import {
@@ -24,6 +25,7 @@ import {
   databaseAttr,
   StatementStatistics,
   statementKey,
+  queryByName,
 } from "../util";
 import { AggregateStatistics } from "../statementsTable";
 import { Fraction } from "./statementDetails";
@@ -87,12 +89,13 @@ function fractionMatching(
 
 function filterByRouterParamsPredicate(
   match: Match<any>,
+  location: Location,
   internalAppNamePrefix: string,
 ): (stat: ExecutionStatistics) => boolean {
   const statement = getMatchParamByName(match, statementAttr);
   const implicitTxn = getMatchParamByName(match, implicitTxnAttr) === "true";
-  const database = getMatchParamByName(match, databaseAttr);
-  let app = getMatchParamByName(match, appAttr);
+  const database = queryByName(location, databaseAttr);
+  let app = queryByName(location, appAttr);
 
   const filterByKeys = (stmt: ExecutionStatistics) =>
     stmt.statement === statement &&
@@ -129,15 +132,19 @@ export const selectStatement = createSelector(
     const flattened = flattenStatementStats(statements);
     const results = _.filter(
       flattened,
-      filterByRouterParamsPredicate(props.match, internalAppNamePrefix),
+      filterByRouterParamsPredicate(
+        props.match,
+        props.location,
+        internalAppNamePrefix,
+      ),
     );
     const statement = getMatchParamByName(props.match, statementAttr);
     return {
       statement,
       stats: combineStatementStats(results.map(s => s.stats)),
       byNode: coalesceNodeStats(results),
       app: _.uniq(results.map(s => s.app)),
-      database: getMatchParamByName(props.match, databaseAttr),
+      database: queryByName(props.location, databaseAttr),
       distSQL: fractionMatching(results, s => s.distSQL),
       vec: fractionMatching(results, s => s.vec),
       opt: fractionMatching(results, s => s.opt),

pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx

@@ -30,10 +30,10 @@ import {
   NumericStat,
   StatementStatistics,
   stdDev,
-  getMatchParamByName,
   formatNumberForDisplay,
   calculateTotalWorkload,
   unique,
+  queryByName,
 } from "src/util";
 import { Loading } from "src/loading";
 import { Button } from "src/button";
@@ -389,7 +389,7 @@ export class StatementDetails extends React.Component<
   };
 
   render(): React.ReactElement {
-    const app = getMatchParamByName(this.props.match, appAttr);
+    const app = queryByName(this.props.location, appAttr);
     return (
       <div className={cx("root")}>
         <Helmet title={`Details | ${app ? `${app} App |` : ""} Statements`} />
@@ -445,7 +445,7 @@ export class StatementDetails extends React.Component<
     } = this.props.statement;
 
     if (!stats) {
-      const sourceApp = getMatchParamByName(this.props.match, appAttr);
+      const sourceApp = queryByName(this.props.location, appAttr);
       const listUrl = "/statements" + (sourceApp ? "/" + sourceApp : "");
 
       return (

pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts

@@ -17,7 +17,7 @@ import {
   ExecutionStatistics,
   flattenStatementStats,
   formatDate,
-  getMatchParamByName,
+  queryByName,
   statementKey,
   StatementStatistics,
   TimestampToMoment,
@@ -142,7 +142,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);
 

pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx

@@ -36,6 +36,7 @@ import {
   calculateTotalWorkload,
   unique,
   containAny,
+  queryByName,
 } from "src/util";
 import {
   AggregateStatistics,
@@ -433,7 +434,7 @@ export class StatementsPage extends React.Component<
     } = this.props;
     const appAttrValue = getMatchParamByName(match, appAttr);
     const selectedApp = appAttrValue || "";
-    const appOptions = [{ value: "All", label: "All" }];
+    const appOptions = [{ value: "all", label: "All" }];
     this.props.apps.forEach(app => appOptions.push({ value: app, label: app }));
     const data = this.filteredStatementsData();
     const totalWorkload = calculateTotalWorkload(data);
@@ -577,12 +578,12 @@ export class StatementsPage extends React.Component<
 
   render() {
     const {
-      match,
+      location,
       refreshStatementDiagnosticsRequests,
       onActivateStatementDiagnostics,
       onDiagnosticsModalOpen,
     } = this.props;
-    const app = getMatchParamByName(match, appAttr);
+    const app = queryByName(location, appAttr);
     return (
       <div className={cx("root", "table-area")}>
         <Helmet title={app ? `${app} App | Statements` : "Statements"} />

pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTableContent.tsx

@@ -22,7 +22,13 @@ import { Dropdown } from "src/dropdown";
 import { Button } from "src/button";
 
 import { Tooltip } from "@cockroachlabs/ui-components";
-import { summarize, TimestampToMoment } from "src/util";
+import {
+  appAttr,
+  databaseAttr,
+  propsToQueryString,
+  summarize,
+  TimestampToMoment,
+} from "src/util";
 import { shortStatement } from "./statementsTable";
 import styles from "./statementsTableContent.module.scss";
 import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
@@ -121,39 +127,43 @@ export const StatementTableCell = {
   ),
 };
 
-interface StatementLinkProps {
+type StatementLinkTargetProps = {
   statement: string;
   app: string;
   implicitTxn: boolean;
-  search: string;
   statementNoConstants?: string;
   database?: string;
-  onClick?: (statement: string) => void;
-}
+};
 
 // StatementLinkTarget returns the link to the relevant statement page, given
 // the input statement details.
-export const StatementLinkTarget = (props: StatementLinkProps) => {
-  let base: string;
-  if (props.app && props.app.length > 0) {
-    base = `/statements/${props.app}`;
-  } else {
-    base = `/statement`;
-  }
-  if (props.database && props.database.length > 0) {
-    base = base + `/${props.database}/${props.implicitTxn}`;
-  } else {
-    base = base + `/${props.implicitTxn}`;
-  }
+export const StatementLinkTarget = (
+  props: StatementLinkTargetProps,
+): string => {
+  const base = `/statement/${props.implicitTxn}`;
+  const linkStatement = props.statementNoConstants || props.statement;
+
+  const searchParams = propsToQueryString({
+    [databaseAttr]: props.database,
+    [appAttr]: props.app,
+  });
 
-  let linkStatement = props.statement;
-  if (props.statementNoConstants) {
-    linkStatement = props.statementNoConstants;
-  }
-  return `${base}/${encodeURIComponent(linkStatement)}`;
+  return `${base}/${encodeURIComponent(linkStatement)}?${searchParams}`;
 };
 
-export const StatementLink = (props: StatementLinkProps) => {
+interface StatementLinkProps {
+  statement: string;
+  app: string;
+  implicitTxn: boolean;
+  search: string;
+  statementNoConstants?: string;
+  database?: string;
+  onClick?: (statement: string) => void;
+}
+
+export const StatementLink = (
+  props: StatementLinkProps,
+): React.ReactElement => {
   const summary = summarize(props.statement);
   const { onClick, statement } = props;
   const onStatementClick = React.useCallback(() => {
@@ -162,8 +172,16 @@ export const StatementLink = (props: StatementLinkProps) => {
     }
   }, [onClick, statement]);
 
+  const linkProps = {
+    statement: props.statement,
+    app: props.app,
+    implicitTxn: props.implicitTxn,
+    statementNoConstants: props.statementNoConstants,
+    database: props.database,
+  };
+
   return (
-    <Link to={StatementLinkTarget(props)} onClick={onStatementClick}>
+    <Link to={StatementLinkTarget(linkProps)} onClick={onStatementClick}>
       <div>
         <Tooltip
           placement="bottom"

pkg/ui/workspaces/cluster-ui/src/util/query/query.spec.ts

@@ -9,7 +9,7 @@
 // licenses/APL.txt.
 
 import { assert } from "chai";
-import { queryToString, queryByName } from "./query";
+import { propsToQueryString, queryByName } from "./query";
 import { Location } from "history";
 
 const location: Location = {
@@ -22,11 +22,29 @@ const location: Location = {
 };
 
 describe("Query utils", () => {
-  describe("queryToString", () => {
-    it("make query to string", () => {
-      assert.equal(queryToString({ a: "test" }), "a=test");
-      assert.equal(queryToString({ a: "test", b: "test" }), "a=test&b=test");
-      assert.equal(queryToString({ a: undefined }), "a=undefined");
+  describe("propsToQueryString", () => {
+    it("creates query string from object", () => {
+      const obj = {
+        start: 100,
+        end: 200,
+        strParam: "hello",
+        bool: false,
+      };
+      const expected = "start=100&end=200&strParam=hello&bool=false";
+      const res = propsToQueryString(obj);
+      expect(res).toEqual(expected);
+    });
+
+    it("skips entries with nullish values", () => {
+      const obj = {
+        start: 100,
+        end: 200,
+        strParam: null as any,
+        hello: undefined as any,
+      };
+      const expected = "start=100&end=200";
+      const res = propsToQueryString(obj);
+      expect(res).toEqual(expected);
     });
   });
   describe("queryByName", () => {