Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: hide node/regions info on tenants #69444

Merged
merged 1 commit into from
Aug 28, 2021
Merged

Conversation

maryliag
Copy link
Contributor

@maryliag maryliag commented Aug 26, 2021

On tenant clusters we don't want to show information
about nodes and regions.

This commit hides that information on:

  • statements page (table, filter, column selector)
  • statement details page (overview, exec stats)
  • transactions page (table, filter)

Release justification: Category 4
Release note (ui change): Hide node and regions information
on the new tenant plan (serverless / free tier)

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@maryliag maryliag marked this pull request as draft August 26, 2021 23:45
@maryliag maryliag force-pushed the tenant-flag branch 2 times, most recently from 89b0207 to 6831767 Compare August 27, 2021 13:49
@maryliag maryliag requested review from a team and koorosh August 27, 2021 13:50
@maryliag maryliag changed the title [draft] ui: hide node/regions info on tenants ui: hide node/regions info on tenants Aug 27, 2021
@maryliag maryliag marked this pull request as ready for review August 27, 2021 13:52
Copy link
Contributor

@matthewtodd matthewtodd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @koorosh and @maryliag)


-- commits, line 4 at r1:
nit: s/cluster/clusters


pkg/ui/workspaces/cluster-ui/src/sortedtable/sortedtable.tsx, line 59 at r1 (raw file):

  // If true, the user can't overwrite the setting for this column. False if not defined.
  alwaysShow?: boolean;
  // If true, it will hide this column if it's tenant cluster. False is not defined.

nit: "If true, hide this column for tenant clusters. False if not defined."


pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx, line 351 at r1 (raw file):

        // list if the list is not empty.
        // If the cluster is a tenant cluster we don't care
        // about regions

nit: trailing period.


pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx, line 368 at r1 (raw file):

        // list if the list is not empty.
        // If the cluster is a tenant cluster we don't care
        // about nodes

nit: trailing period.


pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx, line 404 at r1 (raw file):

    const isEmptySearchResults = statements?.length > 0 && search?.length > 0;
    // If the cluster is a tenant cluster we don't show info
    // about nodes/regions

nit: trailing period.


pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx, line 482 at r1 (raw file):

              showScan={true}
              showRegions={regions.length > 1 && !isTenant}
              showNodes={nodes.length > 1 && !isTenant}

Are these clauses redundant with setting nodes and regions to empty arrays above?


pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx, line 237 at r1 (raw file):

            );
            // If the cluster is a tenant cluster we don't show info
            // about nodes/regions

nit: trailing period.


pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx, line 295 at r1 (raw file):

                      filters={filters}
                      showRegions={regions.length > 1 && !isTenant}
                      showNodes={nodes.length > 1 && !isTenant}

Are these clauses redundant with setting nodes and regions to empty arrays above?


pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts, line 144 at r1 (raw file):

      if (regions.length == 0 && nodes.length == 0) return true;
      // If the cluster is a tenant cluster we don't care
      // about node/regions

nit: trailing period.

Copy link
Contributor Author

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @koorosh and @matthewtodd)


pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx, line 482 at r1 (raw file):

Previously, matthewtodd (Matthew Todd) wrote…

Are these clauses redundant with setting nodes and regions to empty arrays above?

Good point, remove here and other cases.
Done


pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx, line 295 at r1 (raw file):

Previously, matthewtodd (Matthew Todd) wrote…

Are these clauses redundant with setting nodes and regions to empty arrays above?

Done.

Copy link
Contributor

@matthewtodd matthewtodd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @koorosh)

On tenant clusters we don't want to show information
about nodes and regions.
This commit hides that information on:
- statements page (table, filter, column selector)
- statement details page (overview, exec stats)
- transactions page (table, filter)

Release justification: Category 4
Release note (ui change): Hide node and regions information
on the new tenant plan (serverless / free tier)
@maryliag
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 28, 2021

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants