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: Added informative warning for insufficient privileges #71960

Merged
merged 1 commit into from
Oct 26, 2021
Merged

ui: Added informative warning for insufficient privileges #71960

merged 1 commit into from
Oct 26, 2021

Conversation

nathanstilwell
Copy link
Contributor

@nathanstilwell nathanstilwell commented Oct 25, 2021

Screen Shot 2021-10-25 at 17 17 49

Previously, /#/reports/nodes would seem to hang in a loading state
indefinitely when a DB Console user wouldn't have admin privileges for
that endpoint. This was due to nodes data being empty from a 403
response.

An error is captured in the application state for this response, so
mapping this error as a prop to the component, the UI can distinguish
between the nodes data simply being empty and being empty due to a
restriction error.

Screen Shot 2021-10-25 at 17 16 07

Detecting this error, we render an <InlineAlert /> to notify the user
of their lack of privileges.

Release note (ui change): All Nodes report notifies user is they need
more privileges to view information.

Previously, `/#/reports/nodes` would seem to hang in a loading state
indefinitely when a DB Console user wouldn't have admin privileges for
that endpoint. This was due to nodes data being empty from a 403
response.

An error is captured in the application state for this response, so
mapping this error as a prop to the component, the UI can distinguish
between the nodes data simply being empty and being empty due to a
restriction error.

Detecting this error, we render an `<InlineAlert />` to notify the user
of their lack of privileges.

Release note (ui change): All Nodes report notifies user is they need
more privileges to view information.
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@nathanstilwell nathanstilwell marked this pull request as ready for review October 25, 2021 21:18
@thtruo
Copy link
Contributor

thtruo commented Oct 25, 2021

@nathanstilwell could we also include a backport into 21.1 and 20.2? Given that David's endpoint changes were backported to the same versions, we should do the same here

Copy link
Contributor

@Santamaura Santamaura left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian)

@nathanstilwell
Copy link
Contributor Author

@thtruo Perhaps I've lost the plot with backporting, but doesn't this PR need to be merged before I can backport the change to release branches?

@Santamaura
Copy link
Contributor

@nathanstilwell I believe you can add backport labels and blathers will auto create them after this is merged

@thtruo
Copy link
Contributor

thtruo commented Oct 26, 2021

Perhaps I've lost the plot with backporting, but doesn't this PR need to be merged before I can backport the change to release branches?

Sorry @nathanstilwell I jumped the gun, didn't realize this was the case

I believe you can add backport labels and blathers will auto create them after this is merged

TIL!

@nathanstilwell
Copy link
Contributor Author

@thtruo It turns out you are right! I haven't reviewed our backporting docs in a while and things seem much better now. 😄

Thanks for the nudge in the right direction @Santamaura

@nathanstilwell
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Oct 26, 2021

Build succeeded:

@craig craig bot merged commit ff2877c into cockroachdb:master Oct 26, 2021
@blathers-crl
Copy link

blathers-crl bot commented Oct 26, 2021

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from 3b4274d to blathers/backport-release-20.2-71960: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 20.2.x failed. See errors above.


error creating merge commit from 3b4274d to blathers/backport-release-21.1-71960: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 21.1.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

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.

4 participants