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

[Share dashboards] Deleting a shared dashboard #167959

Closed
Tracked by #167901 ...
teresaalvarezsoler opened this issue Oct 4, 2023 · 17 comments
Closed
Tracked by #167901 ...

[Share dashboards] Deleting a shared dashboard #167959

teresaalvarezsoler opened this issue Oct 4, 2023 · 17 comments
Assignees
Labels
Feature:Dashboard Dashboard related features impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Project:ShareToSpace Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@teresaalvarezsoler
Copy link

teresaalvarezsoler commented Oct 4, 2023

Describe the feature:
When users delete a dashboard, it will be deleted from all spaces that it leaves in.

We will keep the same UX that exist currently for dataviews and other saved objects:

  • Deleting from the Listing page: image

  • Deleting from the Saved Object management page: image

@teresaalvarezsoler teresaalvarezsoler added Feature:Dashboard Dashboard related features Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Oct 4, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@ThomThomson
Copy link
Contributor

I think we'll need a bit of triage here to really iron out how this will work. As I see it testing on saved queries there are two operations:

Deleting the saved object:

  • This is what happens when you hit delete in the SO management screen
  • This is what happens when you delete a Dashboard from the listing page.
    When deleting the saved object, it is deleted from all spaces it is assigned to. Right now it shows this warning:
Screenshot 2023-10-04 at 10 44 44 AM

Un-sharing the saved object:

From the saved object sharing screen, a user can open the share to spaces flyout, and unassign the object from that space.
Screenshot 2023-10-04 at 10 50 32 AM

Where / when do we think this modal should be used?

@ThomThomson ThomThomson added loe:medium Medium Level of Effort impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. labels Oct 4, 2023
@teresaalvarezsoler
Copy link
Author

I updated the description of this issue to reflect what we agreed upon.

@teresaalvarezsoler teresaalvarezsoler changed the title [Share dashboards] Deleting a shared dashboard will only delete it from the current space [Share dashboards] Deleting a shared dashboard Oct 6, 2023
@nickpeihl nickpeihl self-assigned this Oct 23, 2023
@cqliu1
Copy link
Contributor

cqliu1 commented Nov 1, 2023

When you delete a data view in Stack Management > Data views, it also deletes the selected data view(s) permanently from all spaces.

Screenshot 2023-10-31 at 4 07 25 PM

In all of the existing table list views that support sharing/unsharing saved objects, there is no way to unshare a saved object from the current space because selection of the current space is disabled to prevent a scenario where all spaces are deselected, and the users lose access to saved object that has no spaces assigned to it.

If we introduce this Remove from current space action in the dashboard listing page, will we need to align the other listing pages? Those potentially include the visualize listing page, maps listing page, data views management page, tags management page, and files management page.

Should we alternatively allow the user to deselect the current space in the spaces selection list, and instead check that at least 1 space is selected, whether it is the current space or not, and disable the Save & Close button when no spaces are selected?

@nickpeihl
Copy link
Member

If we introduce this Remove from current space action in the dashboard listing page, will we need to align the other listing pages? Those potentially include the visualize listing page, maps listing page, data views management page, tags management page, and files management page.

I think we do need to align this workflow. Files, Visualize, Maps, and Dashboards all use the same TableListView component, so we can align there. It looks like the Tags and Data View listing pages use custom components, but I think it should be possible to migrate to the TableListView component once it supports the Spaces list column.

Should we alternatively allow the user to deselect the current space in the spaces selection list, and instead check that at least 1 space is selected, whether it is the current space or not, and disable the Save & Close button when no spaces are selected?

You mean changing the Share to space flyout component from the Spaces plugin? We can do that, but there is no confirmation check for the user. So a user might unwittingly remove their access to a shared saved object. Probably should confirm with @elastic/kibana-security if that's something we want to allow.

@jeramysoucy
Copy link
Contributor

@nickpeihl We (Kibana Security) have an open issue for implementing this behavior.

@teresaalvarezsoler
Copy link
Author

If we introduce this Remove from current space action in the dashboard listing page, will we need to align the other listing pages? Those potentially include the visualize listing page, maps listing page, data views management page, tags management page, and files management page.

Thanks for bringing this up @cqliu1 . Yes, I think that's the desired behaviour. As per @nickpeihl comment it seems pretty straightforward since all saved objects use the same list view component, isn't it?

@nickpeihl
Copy link
Member

I have found what might be an easier and less confusing UI for handling deletion of shared saved objects.

We can disable the selector if the object is shared to multiple Spaces. The selector is currently only used for the "Delete" bulk action. We can detect if the item is shared to multiple Spaces and tell the listing table to disable selection of that item. We can also provide a tooltip explaining why the selector is disabled. Users with proper privileges can choose to unshare the item from other Spaces by clicking on the Spaces list icons to open the "Share to space flyout". And, once #169865 is fixed, users can use the "Share to space flyout" to remove the item from the current Space. I think this would simplify the workflow and we shouldn't need these extra modals.

Disabling item deletion is currently in use by the Files table list view and is also being considered for Managed saved objects (objects installed by Integrations).

Here's a demo video from a PoC.

disable-delete-shared-object.mp4

@teresaalvarezsoler
Copy link
Author

Thanks @nickpeihl , it looks like a good solution to me. It's also better in the case of a dashboard being shared in All spaces because the user has to make a concious decision of unsharing it.

I have only three concerns:

  • I don't know if it will be easy for the user to realize that he needs to "unshare" the dashboard from the current space in order to "remove" it. A tooltip will definetely help and I would consider it a must-have for this solution. Can you share this POC and I will test it with the users I'm meeting?
  • If a user wants to remove the dashboard from all spaces, he will need to first unshare it from all spaces except the current one and then delete it from the list view. I'm wondering whether it would be better to allow the deletion from all spaces instead of disabling it.
  • The dependency with the security team work, I ask them an ETA.

@cqliu1
Copy link
Contributor

cqliu1 commented Nov 9, 2023

  • If a user wants to remove the dashboard from all spaces, he will need to first unshare it from all spaces except the current one and then delete it from the list view. I'm wondering whether it would be better to allow the deletion from all spaces instead of disabling it.

We should only allow deleting a shared dashboard if that user has All access to Saved object management and All access to Dashboards in every space that shares that dashboard. If they don't have sufficient permissions in any one of the other spaces, deleting should be disabled, and that user will need to unshare before deleting.

But I'm worried that this permission check across spaces can get complicated/expensive if we have to do this check per dashboard in the listing page to disable the selection on the table every time a user loads the Dashboard listing page, whether or not they intended to delete any dashboards. By having the user manually unshare the dashboard first, we're delaying that permission check across space to only when it's necessary and the user is actively trying to unshare/delete a shared dashboard.

The user with All access to Saved object management regardless of their access to Dashboard will still be able to delete a shared dashboard without unsharing from the Saved object management page.

@cqliu1
Copy link
Contributor

cqliu1 commented Nov 9, 2023

@teresaalvarezsoler @andreadelrio We should align the deletion confirm modal for all of the listing pages, e.g. saved object management, data view management, dashboard listing page, etc.

These are the existing confirm modals for deleting shared objects:

Saved object management

Many different types of objects can be selected for deletion here, so the Type column makes sense here. I wonder if the ID column is necessary here as it's only useful differentiating between two objects with duplicate names. This confirm modal currently doesn't show any information on which spaces are impacted for each saved object selected. Ideally we would use the same Spaces column here. I do like the pagination for when you select many objects, but we should probably hide the pagination if there is only one page.

Screenshot 2023-11-08 at 11 30 25 AM

Data view management

Since only one type of saved object can be deleted on this page, and other listing pages, we can forgo the Type column here. This one shows the number of spaces that each object will be deleted from, but I think we should show the same Spaces column listing out every space for each data view here too.

Screenshot 2023-11-08 at 11 24 07 AM

@teresaalvarezsoler
Copy link
Author

Thanks @cqliu1

Saved object management

I wonder if the ID column is necessary here as it's only useful differentiating between two objects with duplicate names

Let's leave it for now since it has been there forever and there may be some use cases we don't know about it.

Ideally we would use the same Spaces column here.

Agree

Data view management

I think we should show the same Spaces column listing out every space for each data view here too

Agree. Could you share the final screenshot of how it looks like?

@nickpeihl
Copy link
Member

nickpeihl commented Dec 1, 2023

@teresaalvarezsoler Here are screenshots of the two proposals @cqliu1 identified for the delete modal. In these demos, the logged in user only has write access to the current Space. The data views are shared to many Spaces. Please ignore that in the screenshots I am are calling them "ContentItems". This is a placeholder that will change for each app (e.g. "Dashboard", "Visualizations", "Maps", etc).

The first proposal mimics the Data View modal. It shows a count of Spaces to which the item is shared.

Screenshot 2023-12-01 at 10 29 23 AM

The second proposal shows a SpacesList component similar to the component we would see in the listing view. The user in this video does not have any read privileges on some Spaces to which the item is shared. So the icons for some Spaces are hidden by the "+3" icon. Hovering over this icon presents a tooltip.

confirm_delete.mp4

In both cases, the user is still allowed to delete the object from all the Spaces. This is the established security model of Spaces to which we are following.

@andreadelrio
Copy link
Contributor

The second proposal shows a SpacesList component similar to the component we would see in the listing view. The user in this video does not have any read privileges on some Spaces to which the item is shared. So the icons for some Spaces are hidden by the "+3" icon. Hovering over this icon presents a tooltip.

confirm_delete.mp4

In both cases, the user is still allowed to delete the object from all the Spaces. This is the established security model of Spaces to which we are following.

@nickpeihl For the second proposal, I'd recommend we keep the spaces list to one-row max, that way we avoid having different heights in the table rows and the table looks cleaner. Would it be possible to display +3 in plain text instead of using EuiAvatar?

Frame 1368

@teresaalvarezsoler
Copy link
Author

thanks @nickpeihl I like the second proposals if it doesn't imply more effort on our side. I agree with @andreadelrio regarding the multi-line.

@nickpeihl
Copy link
Member

Also, here is original design issue for deleting Data Views on which these proposals are modeled. The first proposal above matches very closely the current delete modal from Data Views Management.

@mattkime Since you implemented the designs for the Data View Management table, I wonder if you have any thoughts on these two proposals? For example, your implementation lists a count of affected Spaces in the modal. My second proposal above uses the SpacesList component instead.

  1. In the original design had you considered displaying the SpacesList component in the Delete modal?
  2. Once we reach alignment here and Add spaces to TableListView #171461, I think we could consider migrating the Data Views Management table to the shared-ux TableListView component. So your feedback would be very pertinent.

@nreese
Copy link
Contributor

nreese commented Jul 17, 2024

Closing, issue iceboxed in #188573

@nreese nreese closed this as not planned Won't fix, can't repro, duplicate, stale Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Project:ShareToSpace Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

No branches or pull requests

8 participants