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: extend diagnostics column to allow activate and download reports #55164

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

koorosh
Copy link
Collaborator

@koorosh koorosh commented Oct 2, 2020

Resolves #50824
Depends on cockroachdb/admin-ui-components#31
Depends on cockroachdb/yarn-vendored#42

Previously, Statements table had a Diagnostics column which allowed
users to request diagnostics reports for the first time and then
displayed status for requested report only. As result it wasn't
possible to download already generated report or request new one
report from the statements table.

With current changes, Diagnostics column allows requesting new
reports every time when previous reports are generated.
Also, it provides a list with links to download previous reports.

The main change is to provide a list of available (or requested)
reports for every statement (instead of a single, most recent
report as it was before). Then extracted StatementsPage component
(from admin-ui-components package) handles all rendering logic
for this list of reports.

Minor changes:

  • WAITING FOR QUERY status is renamed to WAITING for new design
  • getDiagnosticsStatus utility function is reused to reduce code
    duplication

Release note (admin ui change): Diagnostics column (on statements table)
has been changed and includes Activate button and dropdown list to
download completed reports. Also, diagnostics badge status is changed from
WAITING FOR QUERY to WAITING

Screen Shot 2020-10-01 at 4 55 32 PM

@koorosh koorosh requested a review from a team October 2, 2020 10:45
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

@koorosh really appreciate the detail in the commit message. Nice work!

Reviewed 7 of 7 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

Copy link
Collaborator

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

@koorosh I am expecting this PR to bump the admin-ui-components version, am I missing something?

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

Previously, Statements table had a Diagnostics column which allowed
users to request diagnostics reports for the first time and then
displayed status for requested report only. As result it wasn't
possible to download already generated report or request new one
report from statements table.

With current changes, Diagnostics column allows to request new
reports everytime when previous reports are generated.
Also it provides a list with links to download previous reports.

The main change is to provide a list of available (or requested)
reports for every statement (instead of a single, most recent
report as it was before). Then extracted `StatementsPage` component
(from `admin-ui-components` package) handles all rendering logic
for this list of reports.

Minor changes:
- `WAITING FOR QUERY` status is renamed to `WAITING` for new design
- `getDiagnosticsStatus` utility function is reused to reduce code
duplication

Resolves: cockroachdb#50824

Release note (admin ui change): Diagnostics column (on statements table)
has been changed and includes `Activate` button and dropdown list to
download completed reports. Also diagnostics badge status is changed from
`WAITING FOR QUERY` to `WAITING`.
@koorosh
Copy link
Collaborator Author

koorosh commented Oct 16, 2020

@koorosh I am expecting this PR to bump the admin-ui-components version, am I missing something?

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@dhartunian , you're definitely right, already updated with bumped version. Thanks!

Copy link
Collaborator

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r2.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@koorosh
Copy link
Collaborator Author

koorosh commented Oct 20, 2020

bors r+

@craig
Copy link
Contributor

craig bot commented Oct 20, 2020

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.

ui: previous diagnostic bundle activations will prevent new triggers on statements list page
3 participants