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

Adding visible field to visualization saved object #59134

Closed
wants to merge 10 commits into from

Conversation

majagrubic
Copy link
Contributor

@majagrubic majagrubic commented Mar 3, 2020

Summary

Context

When adding a visualization to the dashboard, we'd like to hide this visualization from the end user. From the end user perspective, this visualization doesn't exist in the Visualize app, but only in the context of the dashboard. However, embedding visualization directly to the dashboard is still in PoC phase (see this PR and this PR). In the meantime, we will add a visible flag to the visualization saved object. All visualizations will have this set to true, except the ones created by a dashboard. If the flag is set to false, the visualization won't be shown in the visualize editor.

Changes

This PR includes changes to:

  1. adds a visible flag to the visualize saved object
  2. does a saved object migration
  3. hide non-visible visualizations from visualize listing
  4. hide non-visible visualizations from saved object finder

This PR doesn't include changes to Lens saved object, it will be done in the subsequent PR.

Example of a hidden visualization:
vis_hidden

Limitations

The visualization will still be shown in the saved objects management console. (See more details in this issue). I've made the visible checkbox in the saved object overview as non-editable, so that we at least don't allow customer to tamper with what is basically a system setting. Any concerns around this?

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

For maintainers

@majagrubic
Copy link
Contributor Author

Jenkins, test this

@majagrubic majagrubic added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Mar 6, 2020
@elasticmachine
Copy link
Contributor

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

@majagrubic
Copy link
Contributor Author

Jenkins, test this

@lizozom
Copy link
Contributor

lizozom commented Mar 8, 2020

Looking at this feature, I'm curious why we don't want visualizations created from within a dashboard to be available elsewhere. @AlonaNadler

For example, what happens if I accidentally remove the visualization and then wish to re-add it? What will happen when I clone a dashboard? etc.

Should automatically generated visualizations (beats dashboard for example) also have this property turned on?

@majagrubic
Copy link
Contributor Author

@lizozom the current way of adding visualizations to the dashboard is very cumbersome. the users need to create the visualization, name it, save it and then add it to the dashboard. we'd like to remove the last two steps from the end user. from user's point of view, they will be create the visualization for the dashboard and that's it. we will still save it in the background with some autogenerated title (for now), but as far as the end user is concerned, this doesn't exist outside of the dashboard. if they delete it, yes, they'll need to recreate it. existing visualizations shouldn't be affected, no need to hide them.

@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

@ryankeairns
Copy link
Contributor

ryankeairns commented Mar 9, 2020

Perhaps we could do something like show a modal confirmation to confirm the deletion with an alternative action being to add it to the Visualization library for future use.

So the modal might read:
"Removing this panel will permanently delete this object since it does not exist in the Visualization library. If you would like to save it for future use, then you can add it the library now."

With button actions like:
Cancel | Add to library | Delete permanently

I can mock this up later if we like this route.

@majagrubic
Copy link
Contributor Author

majagrubic commented Mar 9, 2020

Just to be clear: This PR does not include any integration with the dashboard yet. All visualizations will be marked as visible and stored/saved as usual. It is an important thing to discuss, but could we move this discussion to a separate issue?

@ryankeairns ryankeairns mentioned this pull request Mar 9, 2020
14 tasks
@ryankeairns
Copy link
Contributor

Moved discussion to original issue: #51318 (comment)

@AlonaNadler
Copy link

I've made the visible checkbox in the saved object overview as non-editable, so that we at least don't allow customer to tamper with what is basically a system setting. Any concerns around this?

why do we show the hidden visualizations in the saved object? Hiding visualization is the technical short term and fastest solution for removing the hundreds of visualizations users need to manage constantly . In a perfect world, a dashboard will be one object.

having every visualization as a separate object in visualize is only useful when users use a single visualization in multiple dashboards.A use case that we know exists but not as common as using visualization in only single dashboard.

@AlonaNadler
Copy link

Looking at this feature, I'm curious why we don't want visualizations created from within a dashboard to be available elsewhere. @AlonaNadler

  • most visualizations are only used in one dashboard. Saving them explicitly in visualize in overwhelming (companies quickly have thousands of visualization) without the ability to delete them or know where they are being used.
  • Hiding the visualization by default is optimizing for most people use case. We will still allow users to explicitly add to Visualize in case they want to use visualization in a multiple dashboards

For example, what happens if I accidentally remove the visualization and then wish to re-add it? What will happen when I clone a dashboard? etc.

  • In the new dashboard experience we encourage users to create the visualization within the dashboard instead of going and searching for the visualization they need among thousands of other visualizations.
  • Yes if users created a visualization from a dashboard and removed it is will be deleted. That is ok and expected.
  • Clone the dashboard is a really good use case. Ideally, It will create a new dashboard object with new visualization objects (@majagrubic what do you think). Im also open to removing the clone dashboard which honestly creates a lot of problems even before this change in favor of cloning panel. However, people will still be able to "save as" so an approach will anyway need to be determined

Should automatically generated visualizations (beats dashboard for example) also have this property turned on?

Ideally yes, that can have a separate timeline. As an example, Metricbeat alone adds 200 visualizations. That doesn't make sense and a bad experience. The experience will improve if these will be part of a dashboard and hidden

@majagrubic
Copy link
Contributor Author

majagrubic commented Mar 9, 2020

why do we show the hidden visualizations in the saved object?

Changing the saved object client to allow for the ES query that would need to be put in place to support this would be a massive overhaul and something platform team is not keen on doing. Additonally, when exporting saved objects, they would still be visible in the export. So two main arguments:

  1. Saved object management console is something for advanced users only and it's ok we show things that are "under the hood" there
  2. Regarding the export: it's better if users see many objects that they are not sure where they are coming from, than not see them and be suddenly surprised with them in the export.

Detailed convo with the platform team about this is in the linked issue.

Also, there is an initiative that the inspect view could go away soon: #59588

@flash1293
Copy link
Contributor

@majagrubic

I'm pretty sure this was discussed, but I couldn't find a reference anywhere.

Why aren't we using a hidden field instead? It seems easier to handle (e.g. no migration necessary). Plus I'm concerned that the visible field will break a lot of automated integrations in a non-obvious way (I know some customers are creating visualizations via script, those would not have the visible flag and thus be hidden by default - not for the ones already in Kibana, the migration would catch those, but all future visualizations added via automation). It seems like this is a breaking change at some level which is easy to avoid so I'm curious to learn why we are not going for it.

@majagrubic
Copy link
Contributor Author

I'm gonna close this PR as this is not the approach we're going to take.

@majagrubic majagrubic closed this Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Saved Objects 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.

7 participants