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

Partially addresses cockroachdb#68843

Previously, we used two different bases for the statement details page, which
depended on which route parameters were included. `/statements/` was used when
the app name was included in the path, and otherwise `/statement/` was used.
The database name was also optionally included in the path name, further
complicating routing to the statement details page as these optional route
params lead to the need to include all combinations of route parameters for
statement detail paths,

This commit turns all optional route parameters into query string parameters,
removing the necessity for different base paths and route param combinations.

Release note (ui change): For statement detail URLs, the app name and database
name are now query string parameters. The route to statement details is
now definitively `/statement/:implicitTxn/:statement?{queryStringParams}`.

e.g.
`statement/true/SELECT%20city%2C%20id%20FROM%20vehicles%20WHERE%20city%20%3D%20%241?database=movr&app=movr`

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 {
@@ -27,6 +28,7 @@ import {
 } from "../util";
 import { AggregateStatistics } from "../statementsTable";
 import { Fraction } from "./statementDetails";
+import { queryByName } from "src/util/query";
 
 interface StatementDetailsData {
   nodeId: number;
@@ -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,7 +132,11 @@ 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 {

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", () => {

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

@@ -15,15 +15,16 @@ interface ParamsObj {
   [key: string]: string;
 }
 
-/* eslint-disable no-prototype-builtins,no-restricted-syntax */
-export function queryToString(obj: any) {
-  const str = [];
-  for (const p in obj) {
-    if (obj.hasOwnProperty(p)) {
-      str.push(`${encodeURIComponent(p)}=${encodeURIComponent(obj[p])}`);
-    }
-  }
-  return str.join("&");
+// 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 }): string {
+  const params = new URLSearchParams();
+  Object.entries(props).forEach(
+    ([k, v]: [string, any]) => v != null && params.set(k, v.toString()),
+  );
+  return params.toString();
 }
 
 export function queryToObj(location: Location, key: string, value: string) {
@@ -44,12 +45,15 @@ export function queryToObj(location: Location, key: string, value: string) {
   return paramObj;
 }
 
-export function queryByName(location: Location, key: string) {
+export function queryByName(location: Location, key: string): string {
   const urlParams = new URLSearchParams(location.search);
   return urlParams.get(key);
 }
 
-export function getMatchParamByName(match: Match<any>, key: string) {
+export function getMatchParamByName(
+  match: Match<any>,
+  key: string,
+): string | null {
   const param = match.params[key];
   if (param) {
     return decodeURIComponent(param);

pkg/ui/workspaces/db-console/src/app.tsx

@@ -65,6 +65,7 @@ import SessionsPage from "src/views/sessions/sessionsPage";
 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 "styl/app.styl";
 
 // NOTE: If you are adding a new path to the router, and that path contains any
@@ -185,39 +186,38 @@ export const App: React.FC<AppProps> = (props: AppProps) => {
                 />
                 <Route
                   exact
-                  path={`/statements/:${appAttr}/:${statementAttr}`}
+                  path={`/statement/:${implicitTxnAttr}/:${statementAttr}`}
                   component={StatementDetails}
                 />
                 <Route
                   exact
-                  path={`/statements/:${appAttr}/:${implicitTxnAttr}/:${statementAttr}`}
-                  component={StatementDetails}
+                  path={`/statements/:${appAttr}/:${statementAttr}`}
+                  render={RedirectToStatementDetails}
                 />
                 <Route
                   exact
-                  path={`/statements/:${appAttr}/:${databaseAttr}/:${implicitTxnAttr}/:${statementAttr}`}
-                  component={StatementDetails}
+                  path={`/statements/:${appAttr}/:${implicitTxnAttr}/:${statementAttr}`}
+                  render={RedirectToStatementDetails}
                 />
-
                 <Route
                   exact
-                  path="/statement"
-                  component={() => <Redirect to="/statements" />}
+                  path={`/statements/:${appAttr}/:${databaseAttr}/:${implicitTxnAttr}/:${statementAttr}`}
+                  render={RedirectToStatementDetails}
                 />
                 <Route
                   exact
-                  path={`/statement/:${statementAttr}`}
-                  component={StatementDetails}
+                  path={`/statement/:${implicitTxnAttr}/:${statementAttr}`}
+                  render={RedirectToStatementDetails}
                 />
                 <Route
                   exact
-                  path={`/statement/:${implicitTxnAttr}/:${statementAttr}`}
-                  component={StatementDetails}
+                  path={`/statement/:${databaseAttr}/:${implicitTxnAttr}/:${statementAttr}`}
+                  render={RedirectToStatementDetails}
                 />
                 <Route
                   exact
-                  path={`/statement/:${databaseAttr}/:${implicitTxnAttr}/:${statementAttr}`}
-                  component={StatementDetails}
+                  path="/statement"
+                  component={() => <Redirect to="/statements" />}
                 />
 
                 {/* sessions */}