ui:app filter is has multi select option

Previously you could only select one App on the filter for Transaction and Statement page. This commits introduces a multi select option, making possible for the user select several apps at the same time and exclude internal results. This commits also properly sets the filter value for database with no values to (unset). Partially addresses #70544 Not included in this commit: - Select all apps except internal by default - Add app filter to Session tab Release note (ui change): App filter on Transaction and Statement pages is now multi select.
cockroachdb · Oct 25, 2021 · b963f54 · b963f54
1 parent ba04636
commit b963f54
Show file tree

Hide file tree

Showing 10 changed files with 154 additions and 87 deletions.
diff --git a/pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx b/pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx
@@ -29,7 +29,7 @@ import { MultiSelectCheckbox } from "../multiSelectCheckbox/multiSelectCheckbox"
 interface QueryFilter {
   onSubmitFilters: (filters: Filters) => void;
   smth?: string;
-  appNames: SelectOptions[];
+  appNames: string[];
   activeFilters: number;
   filters: Filters;
   dbNames?: string[];
@@ -69,7 +69,7 @@ const timeUnit = [
 ];
 
 export const defaultFilters: Filters = {
-  app: "All",
+  app: "",
   timeNumber: "0",
   timeUnit: "seconds",
   fullScan: false,
@@ -116,7 +116,7 @@ export const getFiltersFromQueryString = (
  * we want to consider 0 active Filters
  */
 export const inactiveFiltersState: Filters = {
-  app: "All",
+  app: "",
   timeNumber: "0",
   fullScan: false,
   sqlType: "",
@@ -289,6 +289,27 @@ export class Filter extends React.Component<QueryFilter, FilterState> {
       border: "none",
     });
 
+    const appsOptions = appNames.map(app => ({
+      label: app,
+      value: app,
+      isSelected: this.isOptionSelected(app, filters.app),
+    }));
+    const appValue = appsOptions.filter(option => {
+      return filters.app.split(",").includes(option.label);
+    });
+    const appFilter = (
+      <div>
+        <div className={filterLabel.margin}>App</div>
+        <MultiSelectCheckbox
+          options={appsOptions}
+          placeholder="All"
+          field="app"
+          parent={this}
+          value={appValue}
+        />
+      </div>
+    );
+
     const databasesOptions = showDB
       ? dbNames.map(db => ({
           label: db,
@@ -413,14 +434,6 @@ export class Filter extends React.Component<QueryFilter, FilterState> {
     );
     // TODO replace all onChange actions in Selects and Checkboxes with one onSubmit in <form />
 
-    // Some app names could be empty strings, so we're adding " " to those names,
-    // this way it's easier for the user to recognize the blank name.
-    const apps = appNames.map(app => {
-      const label =
-        app.label.trim().length === 0 ? '"' + app.label + '"' : app.label;
-      return { label: label, value: app.value };
-    });
-
     return (
       <div onClick={this.insideClick} ref={this.dropdownRef}>
         <div className={dropdownButton} onClick={this.toggleFilters}>
@@ -429,14 +442,7 @@ export class Filter extends React.Component<QueryFilter, FilterState> {
         </div>
         <div className={dropdownArea}>
           <div className={dropdownContentWrapper}>
-            <div className={filterLabel.top}>App</div>
-            <Select
-              options={apps}
-              onChange={e => this.handleSelectChange(e, "app")}
-              value={apps.filter(app => app.value === filters.app)}
-              placeholder="All"
-              styles={customStyles}
-            />
+            {appFilter}
             {showDB ? dbFilter : ""}
             {showSqlType ? sqlTypeFilter : ""}
             {showRegions ? regionsFilter : ""}

diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.selectors.ts
@@ -105,8 +105,13 @@ function filterByRouterParamsPredicate(
 ): (stat: ExecutionStatistics) => boolean {
   const statement = getMatchParamByName(match, statementAttr);
   const implicitTxn = getMatchParamByName(match, implicitTxnAttr) === "true";
-  const database = queryByName(location, databaseAttr);
-  let app = queryByName(location, appAttr);
+  const database =
+    queryByName(location, databaseAttr) === "(unset)"
+      ? ""
+      : queryByName(location, databaseAttr);
+  const apps = queryByName(location, appAttr)
+    ? queryByName(location, appAttr).split(",")
+    : null;
   // If the aggregatedTs is unset, we will aggregate across the current date range.
   const aggregatedTs = queryByName(location, aggregatedTsAttr);
   const aggInterval = queryByName(location, aggregationIntervalAttr);
@@ -119,20 +124,21 @@ function filterByRouterParamsPredicate(
     stmt.implicit_txn === implicitTxn &&
     (stmt.database === database || database === null);
 
-  if (!app) {
+  if (!apps) {
     return filterByKeys;
   }
-
-  if (app === "(unset)") {
-    app = "";
+  if (apps.includes("(unset)")) {
+    apps.push("");
   }
-
-  if (app === internalAppNamePrefix) {
-    return (stmt: ExecutionStatistics) =>
-      filterByKeys(stmt) && stmt.app.startsWith(internalAppNamePrefix);
+  let showInternal = false;
+  if (apps.includes(internalAppNamePrefix)) {
+    showInternal = true;
   }
 
-  return (stmt: ExecutionStatistics) => filterByKeys(stmt) && stmt.app === app;
+  return (stmt: ExecutionStatistics) =>
+    filterByKeys(stmt) &&
+    ((showInternal && stmt.app.startsWith(internalAppNamePrefix)) ||
+      apps.includes(stmt.app));
 }
 
 export const selectStatement = createSelector(

diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx
@@ -176,15 +176,15 @@ function statementsRequestFromProps(
 
 function AppLink(props: { app: string }) {
   if (!props.app) {
-    return <span className={cx("app-name", "app-name__unset")}>(unset)</span>;
+    return <Text className={cx("app-name", "app-name__unset")}>(unset)</Text>;
   }
 
   const searchParams = new URLSearchParams({ [appAttr]: props.app });
 
   return (
     <Link
       className={cx("app-name")}
-      to={`/statements/?${searchParams.toString()}`}
+      to={`/sql-activity?tab=statements&${searchParams.toString()}`}
     >
       {props.app}
     </Link>
@@ -542,6 +542,12 @@ export class StatementDetails extends React.Component<
             .utc()
         : this.props.dateRange[1];
 
+    const db = database ? (
+      <Text>{database}</Text>
+    ) : (
+      <Text className={cx("app-name", "app-name__unset")}>(unset)</Text>
+    );
+
     return (
       <Tabs
         defaultActiveKey="1"
@@ -700,7 +706,7 @@ export class StatementDetails extends React.Component<
 
                 <div className={summaryCardStylesCx("summary--card__item")}>
                   <Text>Database</Text>
-                  <Text>{database}</Text>
+                  {db}
                 </div>
                 <p
                   className={summaryCardStylesCx(

diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts
@@ -105,7 +105,9 @@ export const selectDatabases = createSelector(
 
     return Array.from(
       new Set(
-        statementsState.data.statements.map(s => s.key.key_data.database),
+        statementsState.data.statements.map(s =>
+          s.key.key_data.database ? s.key.key_data.database : "(unset)",
+        ),
       ),
     ).filter((dbName: string) => dbName !== null && dbName.length > 0);
   },
@@ -153,17 +155,19 @@ export const selectStatements = createSelector(
       statement.app.startsWith(state.data.internal_app_name_prefix);
 
     if (app && app !== "All") {
-      let criteria = decodeURIComponent(app);
+      const criteria = decodeURIComponent(app).split(",");
       let showInternal = false;
-      if (criteria === "(unset)") {
-        criteria = "";
-      } else if (criteria === state.data.internal_app_name_prefix) {
+      if (criteria.includes(state.data.internal_app_name_prefix)) {
         showInternal = true;
       }
+      if (criteria.includes("(unset)")) {
+        criteria.push("");
+      }
 
       statements = statements.filter(
         (statement: ExecutionStatistics) =>
-          (showInternal && isInternal(statement)) || statement.app === criteria,
+          (showInternal && isInternal(statement)) ||
+          criteria.includes(statement.app),
       );
     }
 

diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx
@@ -339,6 +339,9 @@ export class StatementsPage extends React.Component<
         : [];
     const databases =
       filters.database.length > 0 ? filters.database.split(",") : [];
+    if (databases.includes("(unset)")) {
+      databases.push("");
+    }
     const regions =
       filters.regions.length > 0 ? filters.regions.split(",") : [];
     const nodes = filters.nodes.length > 0 ? filters.nodes.split(",") : [];
@@ -409,6 +412,7 @@ export class StatementsPage extends React.Component<
     const { pagination, search, filters, activeFilters } = this.state;
     const {
       statements,
+      apps,
       databases,
       location,
       onDiagnosticsReportDownload,
@@ -421,8 +425,6 @@ export class StatementsPage extends React.Component<
     } = this.props;
     const appAttrValue = queryByName(location, appAttr);
     const selectedApp = appAttrValue || "";
-    const appOptions = [{ value: "", label: "All" }];
-    this.props.apps.forEach(app => appOptions.push({ value: app, label: app }));
     const data = this.filteredStatementsData();
     const totalWorkload = calculateTotalWorkload(data);
     const totalCount = data.length;
@@ -495,7 +497,7 @@ export class StatementsPage extends React.Component<
           <PageConfigItem>
             <Filter
               onSubmitFilters={this.onSubmitFilters}
-              appNames={appOptions}
+              appNames={apps}
               dbNames={databases}
               regions={regions}
               nodes={nodes.map(n => "n" + n)}

diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTableContent.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTableContent.tsx
@@ -46,7 +46,7 @@ export const StatementTableCell = {
     search?: string,
     selectedApp?: string,
     onStatementClick?: (statement: string) => void,
-  ) => (stmt: any) => (
+  ) => (stmt: AggregateStatistics): React.ReactElement => (
     <StatementLink
       statement={stmt.label}
       statementSummary={stmt.summary}
@@ -62,7 +62,7 @@ export const StatementTableCell = {
   diagnostics: (
     activateDiagnosticsRef: React.RefObject<ActivateDiagnosticsModalRef>,
     onDiagnosticsDownload: (report: IStatementDiagnosticsReport) => void = noop,
-  ) => (stmt: AggregateStatistics) => {
+  ) => (stmt: AggregateStatistics): React.ReactElement => {
     /*
      * Diagnostics cell might display different components depending
      * on following states:
@@ -127,7 +127,9 @@ export const StatementTableCell = {
       </div>
     );
   },
-  nodeLink: (nodeNames: NodeNames) => (stmt: any) => (
+  nodeLink: (nodeNames: NodeNames) => (
+    stmt: AggregateStatistics,
+  ): React.ReactElement => (
     <NodeLink nodeId={stmt.label} nodeNames={nodeNames} />
   ),
 };
@@ -229,7 +231,10 @@ export const StatementLink = ({
   );
 };
 
-export const NodeLink = (props: { nodeId: string; nodeNames?: NodeNames }) => (
+export const NodeLink = (props: {
+  nodeId: string;
+  nodeNames?: NodeNames;
+}): React.ReactElement => (
   <Link to={`/node/${props.nodeId}`}>
     <div className={cx("node-name-tooltip__info-icon")}>
       {props.nodeNames ? props.nodeNames[props.nodeId] : "N" + props.nodeId}

diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts
@@ -14,7 +14,7 @@ import {
   getStatementsByFingerprintIdAndTime,
   statementFingerprintIdsToText,
 } from "./utils";
-import { Filters } from "../queryFilter/filter";
+import { Filters } from "../queryFilter";
 import { data, nodeRegions, timestamp } from "./transactions.fixture";
 import Long from "long";
 import * as protos from "@cockroachlabs/crdb-protobuf-client";
@@ -46,7 +46,7 @@ const txData = (data.transactions as any) as Transaction[];
 describe("Filter transactions", () => {
   it("show all if no filters applied", () => {
     const filter: Filters = {
-      app: "All",
+      app: "",
       timeNumber: "0",
       timeUnit: "seconds",
       nodes: "",
@@ -107,6 +107,27 @@ describe("Filter transactions", () => {
     );
   });
 
+  it("filters by 2 apps", () => {
+    const filter: Filters = {
+      app: "$ TEST EXACT,$ TEST",
+      timeNumber: "0",
+      timeUnit: "seconds",
+      nodes: "",
+      regions: "",
+    };
+    assert.equal(
+      filterTransactions(
+        txData,
+        filter,
+        "$ internal",
+        data.statements,
+        nodeRegions,
+        false,
+      ).transactions.length,
+      4,
+    );
+  });
+
   it("filters by internal prefix", () => {
     const filter: Filters = {
       app: data.internal_app_name_prefix,
@@ -130,7 +151,7 @@ describe("Filter transactions", () => {
 
   it("filters by time", () => {
     const filter: Filters = {
-      app: "All",
+      app: "",
       timeNumber: "40",
       timeUnit: "miliseconds",
       nodes: "",
@@ -151,7 +172,7 @@ describe("Filter transactions", () => {
 
   it("filters by one node", () => {
     const filter: Filters = {
-      app: "All",
+      app: "",
       timeNumber: "0",
       timeUnit: "seconds",
       nodes: "n1",
@@ -172,7 +193,7 @@ describe("Filter transactions", () => {
 
   it("filters by multiple nodes", () => {
     const filter: Filters = {
-      app: "All",
+      app: "",
       timeNumber: "0",
       timeUnit: "seconds",
       nodes: "n2,n4",
@@ -193,7 +214,7 @@ describe("Filter transactions", () => {
 
   it("filters by one region", () => {
     const filter: Filters = {
-      app: "All",
+      app: "",
       timeNumber: "0",
       timeUnit: "seconds",
       nodes: "",
@@ -214,7 +235,7 @@ describe("Filter transactions", () => {
 
   it("filters by multiple regions", () => {
     const filter: Filters = {
-      app: "All",
+      app: "",
       timeNumber: "0",
       timeUnit: "seconds",
       nodes: "",