cluster-ui: show database info and ability to choose statement columns

[maryliag]

Information about database is now displayed on Statements page and Statements Details page.
By default the column database is not displayed, but with the new column selector option, it can be selected.

New filter allows to select statements based on database name.

New filter option for databases:

New column selector:

New column

Info on Statement Details

Closes: cockroachdb/cockroach#33316
Closes: cockroachdb/cockroach#59210

Release note (ui change): Display database name information on Statements
page (hidden by default) and Statements Details page.

New filter option for statements based on databases name.

New option to select which columns to display on statements table.

Release note (bug fix): Transaction page showing correct value for implicit txn

---

This change is 

[koorosh]

Reviewed 21 of 22 files at r1.
Reviewable status: 21 of 22 files reviewed, 9 unresolved discussions (waiting on @j-low, @laurenbarker, @maryliag, and @nathanstilwell)

a discussion (no related file):
@maryliag ,tried to unselect "All" checkbox and it didn't work. I assume it should unselect all options and allow users to select particular columns.
Also, when I try manually unselect all options, clicking on the last option - toggles all options back to selected state.

---

---

packages/cluster-ui/src/columnsSelector/columnsSelector.tsx, line 42 at r1 (raw file):

  return (
    <components.Option {...props}>
      <input

Is it possible to reuse CheckboxInput component from ui-components package (packages/ui-components/src/Input/CheckboxInput.tsx)?

---

packages/cluster-ui/src/columnsSelector/columnsSelector.tsx, line 133 at r1 (raw file):

    allOptions: OptionsType<SelectOption>,
  ) => {
    let selected = selectedOptions

@maryliag , this logic with strings manipulations looks quite complex. Could you explain the main idea behind this?

---

packages/cluster-ui/src/columnsSelector/columnsSelector.module.scss, line 11 at r1 (raw file):

    height: 32px;
    width: 67px;
    font-size: 12px;

Default font-sizes and spacing can be reused from packages/cluster-ui/src/core/index.module.scss (.lavel class uses it already)

---

packages/cluster-ui/src/statementsPage/statementsPage.tsx, line 76 at r1 (raw file):

  totalFingerprints: number;
  lastReset: string;
  columns: string;

@maryliag , what is the main intend to store columns as a string? Would it be better to keep it in array of strings?

---

packages/cluster-ui/src/statementsPage/statementsPage.tsx, line 327 at r1 (raw file):

      this.state.tableColumnsSelected == undefined ||
      this.state.tableColumnsSelected.length == 0 ||
      (this.state.tableColumnsSelected == "default" &&

What "default" value means?

---

packages/cluster-ui/src/statementsPage/statementsPage.selectors.ts, line 97 at r1 (raw file):

      },
    );
    return Object.keys(databases);

nit. It might be more clear to rewrite it in more declarative way with filter + map functions and then get rid of duplicate values with Set.

return [
  ...new Set(statementsState.data.statements
    .filter(s => !!s.key.key_data.database)
    .map(s => s.key.key_data.database)
  ).values()
]
  

---

packages/cluster-ui/src/statementsTable/statementsTable.tsx, line 155 at r1 (raw file):

    displayColumns == "" ||
    displayColumns == "default"
  )

nit. wrap if statement in { ... } to comply with linting rules

---

packages/cluster-ui/src/statementsTable/statementsTable.tsx, line 159 at r1 (raw file):

  return columns.filter(option => {
    return displayColumns.split(",").includes(option.name);

it might be refactored out from filter predicate:

const displayCol = displayColumns.split(",");
return columns.filter(option => {
    return displayCol.includes(option.name);
}

[maryliag]

Reviewable status: 15 of 23 files reviewed, 9 unresolved discussions (waiting on @j-low, @koorosh, @laurenbarker, and @nathanstilwell)

a discussion (no related file):

Previously, koorosh (Andrii Vorobiov) wrote…

@maryliag ,tried to unselect "All" checkbox and it didn't work. I assume it should unselect all options and allow users to select particular columns.
Also, when I try manually unselect all options, clicking on the last option - toggles all options back to selected state.

Good point, I forgot that case! It's fixed now!

---

---

packages/cluster-ui/src/columnsSelector/columnsSelector.tsx, line 42 at r1 (raw file):

Previously, koorosh (Andrii Vorobiov) wrote…

Is it possible to reuse CheckboxInput component from ui-components package (packages/ui-components/src/Input/CheckboxInput.tsx)?

Weird as it sounds the CheckboxInput is not really a checkbox, is just a regular input that I can then force to be checkbox. The problem is that it includes a wrapper div that I can't change the class and is messing up all alignments of the option, so I opted for a simple input instead.

---

packages/cluster-ui/src/columnsSelector/columnsSelector.tsx, line 133 at r1 (raw file):

Previously, koorosh (Andrii Vorobiov) wrote…

@maryliag , this logic with strings manipulations looks quite complex. Could you explain the main idea behind this?

I realized from your comment that was indeed confusing, so I restructured a little and used better names to help this issue. I also change to use array instead of string, so there is less manipulation. The idea here is: when a value is selected/deselected I don't have access to the value itself, only the array of the current selected ones, so what I'm doing is:

• check if the value "all" was the one clicked, if it was deselected, remove everything, if was selected, add everything
• if another value was selected and because of it all the values are now selected, update the "all" to be selected too
• otherwise just add/remove the value

Let me know if is better now :)

---

packages/cluster-ui/src/columnsSelector/columnsSelector.module.scss, line 11 at r1 (raw file):

Previously, koorosh (Andrii Vorobiov) wrote…

Default font-sizes and spacing can be reused from packages/cluster-ui/src/core/index.module.scss (.lavel class uses it already)

Done.

---

packages/cluster-ui/src/statementsPage/statementsPage.tsx, line 76 at r1 (raw file):

Previously, koorosh (Andrii Vorobiov) wrote…

@maryliag , what is the main intend to store columns as a string? Would it be better to keep it in array of strings?

I thought it would be better to save as string on cache to save space. I did refactor the code to get the value from cache (which is a string) and then transform to array. This way the code is easier to understand.

---

packages/cluster-ui/src/statementsPage/statementsPage.tsx, line 327 at r1 (raw file):

Previously, koorosh (Andrii Vorobiov) wrote…

What "default" value means?

on the majority of cases "default" means select everything, but you can have some columns you want to ignore. For example, we don't want to show the column "database" by default, so if the value is "default" we will hide that column.

---

packages/cluster-ui/src/statementsPage/statementsPage.selectors.ts, line 97 at r1 (raw file):

Previously, koorosh (Andrii Vorobiov) wrote…

nit. It might be more clear to rewrite it in more declarative way with filter + map functions and then get rid of duplicate values with Set.

return [
  ...new Set(statementsState.data.statements
    .filter(s => !!s.key.key_data.database)
    .map(s => s.key.key_data.database)
  ).values()
]
  

Done.

---

packages/cluster-ui/src/statementsTable/statementsTable.tsx, line 155 at r1 (raw file):

Previously, koorosh (Andrii Vorobiov) wrote…

nit. wrap if statement in { ... } to comply with linting rules

Done.

---

packages/cluster-ui/src/statementsTable/statementsTable.tsx, line 159 at r1 (raw file):

Previously, koorosh (Andrii Vorobiov) wrote…

it might be refactored out from filter predicate:

const displayCol = displayColumns.split(",");
return columns.filter(option => {
    return displayCol.includes(option.name);
}

Done.

[nathanstilwell]

Reviewed 2 of 8 files at r2, 11 of 11 files at r4.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @j-low, @koorosh, and @laurenbarker)