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

[Dashboard] Show empty screen with error message if no permissions #58584

Merged
merged 3 commits into from
Feb 27, 2020

Conversation

majagrubic
Copy link
Contributor

Summary

Fix for: #55118.

If the user has only permissions to create the dashboard but not the visualizations, the new empty panel is confusing. This PR hides the panel if user doesn't have permissions and shows an existing empty screen:
Screenshot 2020-02-26 at 10 36 32

Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials
- [ ] Unit or functional tests were updated or added to match the most common scenarios
- [ ] This was checked for keyboard-only and screenreader accessibility

For maintainers

@majagrubic majagrubic added v8.0.0 v7.7.0 Feature:Dashboard Dashboard related features :KibanaApp/fix-it-week Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Feb 26, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@majagrubic majagrubic marked this pull request as ready for review February 26, 2020 11:50
@majagrubic majagrubic requested review from a team and kobelb February 26, 2020 11:50
@majagrubic majagrubic changed the title Show empty screen with error message if no permissions [Dashboad] Show empty screen with error message if no permissions Feb 26, 2020
@majagrubic majagrubic changed the title [Dashboad] Show empty screen with error message if no permissions [Dashboard] Show empty screen with error message if no permissions Feb 26, 2020
Copy link
Contributor

@myasonik myasonik left a comment

Choose a reason for hiding this comment

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

Didn't run locally but code changes look pretty straight forward — LGTM

!getShouldShowEditHelp() &&
!getShouldShowViewHelp() &&
dashboardConfig.getHideWriteControls();
const getIsEmptyInReadonlyModeOrUserHasNoPermissions = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method name is getting long enough that it's difficult to read...

Would changing it to something like shouldShowErrorMessage() be an accurate alternative?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, it's not an error state though, it's a valid state, somewhat edge-casey, but still valid.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe shouldShowUnauthorizedEmptyState?

Also, not really critical to make this change if you like longer function names 🤷‍♂

@majagrubic
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@majagrubic majagrubic merged commit c97bc86 into elastic:master Feb 27, 2020
@majagrubic majagrubic deleted the fix-55118 branch February 27, 2020 18:47
majagrubic pushed a commit that referenced this pull request Feb 28, 2020
…58584) (#58761)

* Show empty screen with error message if no permissions

* Renaming function

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants