feat: disable buttons to try sample data or upload a file for a read-only user #197991
Conversation
paulinashakirova
added
release_note:fix
Team:SharedUX
|
Pinging @elastic/appex-sharedux (Team:SharedUX) |
| @@ -38,6 +38,8 @@ interface Props { | |||
| export const AddData: FC<Props> = ({ addBasePath, application, isDarkMode, isCloudEnabled }) => { | |||
| const { trackUiMetric, guidedOnboardingService } = getServices(); | |||
| const canAccessIntegrations = application.capabilities.navLinks.integrations; | |||
| const onlyReadAccess = !application.capabilities.rulesSettings.writeFlappingSettingsUI; | |||
There was a problem hiding this comment.
I don't have a lot of experience with capabilities, but I don't think this is a good flag for this readonly state. rulesSettings.writeFlappingSettingsUI seem to be a alerting framework
specific capability and we shouldn't use it outside of it.
I think this is the right direction, but I don't know if there is an existing capability that could work for us here (need to look for it) otherwise we might have to register a new one
There was a problem hiding this comment.
For the the sampel data, I think application.capabilities.indexPatterns.save would be a good capability. To create sample data you'd have to be able to create a (dataview) so it would at least be a required condition.
for the fileDataViz not sure. haven't worked with it before - need review the code an see if there anything we could use
There was a problem hiding this comment.
Also the screenshot from the issue #138547 is pointing out an issue on different component
There was a problem hiding this comment.
In the comment below the body of the issue there was this suggestion
#138547 (comment), which made sense, so I followed it.
Do you think it is less beneficial to disable the whole accordion?
There was a problem hiding this comment.
I was thinking a bit more about the issue and did some testing. I think the issue is not that simple and we will need to find some middleground between the effort and the result. Some investigation is needed, here are some pointers:
- In general, just disabling the buttons is not a good UX. It either need to have a reason (for example in a tooltip) explaining why it is disable or simply hidden completely.
- Hidding or disabling the buttons in
add_datacomponent is not enough, because there are other ways to navigate toapp/home#/tutorial_directory/sampleDatapage . Also maybe we shouldn't disable those buttons inadd_datacomponent at all , because there is an external link from that page to a demo environment that isn't restricted by permissions application.capabilities.savedObjectsManagement.editis a good guess for checking if the user is able to add sample data, but it might give false positives and negatives. Ideally we should investigate exact permissions that are needed for adding sample data and then decide what to do with them. This can be done by reviewing the code, analysing internal errors and trial and error.- I am not sure if it is correct to treat sample data and import together. They likely need different inner permissions. I was exploring the upload functionality and found that there is a more specific check inside . It might be that reviewing it might help to better understand how permissions work and what how to better fix it.
- Another problem is that when you don't have enough permissions and try to add sample data we show "Internal server error" If we'd start from showing the proper "Permissions" error that we get from elasticsearch, that would be better for a user and for us to debug and improve on the issue
It might be that after the investigation we will return to a simple fix like we currently have. But to do it with confidence we'd need to understand how it all works end to end
There was a problem hiding this comment.
Thank you for your detailed review.
I will convert this PR to draft for now, since it doesn't fall into the "papercut" category.
Might reopen in the future.
💚 Build Succeeded
Metrics [docs]Async chunks
History
|
|
Closed since it is not a papercut and demands additional investigation. |
|
🤖 Jobs for this PR can be triggered through checkboxes. 🚧
ℹ️ To trigger the CI, please tick the checkbox below 👇
|
Summary
This PR fixes [Home] Disable sample data actions for non-privileged users issue and updates corresponding tests.
Currently a user with read-only privileges sees the Home screen like this
This PR suggests for a user with read-only privileges to see the Home screen like this