michael-s-molina commented on PR #30016: URL: https://github.com/apache/superset/pull/30016#issuecomment-2315159952
> Also open to abusing some other components (Popovers with scrolling lists? Tabs? ¯\_(ツ)_/¯ ) but again was just looking to make a simple (hah!) safety improvement. I understand your objective but simple might mean different things depending on the environment. I'm just trying to reach a simple solution that works for organizations with a lot of assets. > I would also argue that if you don't even have time to scroll past the list of things you're about to break, you probably should not be allowed to click that button. > Is the scrolling the real problem here? I'll just limit the results to fit on a sensible screen. I was just looking for a quick improvement over a modal where you a user has no idea what they're breaking. I don't think the problem is just the scroll. Think about a scenario where a database has 40k charts. The admin will not review every single chart. They might review a sample of charts, maybe the "popular" ones to understand the impact of the change. This might be one way to resolve this, show the top 10 assets of each category. We would need to determine how to calculate the top. We also need to consider the payload size when returning the names for all results of each asset type. > I'm merely displaying the list that's already data the component has access to. This is really concerning because it means that this query returns thousands of names when we currently only display a count. If we go for the top 10 solution, this query should also be modified to just return the necessary data. > Alerts are not in the scope of the current data AFAIK. Alerts have a database connection associated to them. When you delete a database, you'll delete the alerts too because of cascading. > I could list the Saved Queries on the database modal if interested, but that seemed of less utility (including the links to them). Saved queries are really important at Airbnb as they might be really complex and take a long time to build. Rebuilding a dashboard or chart might be way faster than rebuilding a saved query. In summary, I think the top 10 approach might work for giving admins an idea of what's being affected by a database deletion. If you really want to show all values, we could introduce a configuration to limit the max number of assets displayed and show a message indicating the limit. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
