ramiroaquinoromero commented on code in PR #37042:
URL: https://github.com/apache/superset/pull/37042#discussion_r2717472502
##########
superset/commands/chart/delete.py:
##########
@@ -68,3 +73,132 @@ def validate(self) -> None:
security_manager.raise_for_ownership(model)
except SupersetSecurityException as ex:
raise ChartForbiddenError() from ex
+
+ def _cleanup_dashboard_metadata( # noqa: C901
+ self, chart_id: int
+ ) -> None:
+ """
+ Remove references to this chart from all dashboard metadata.
+
+ When a chart is deleted, dashboards may still contain references to the
+ chart ID in various metadata fields (expanded_slices, filter_scopes,
etc.).
+ This method cleans up those references to prevent issues during
dashboard
+ export/import.
+ """
+ # Find all dashboards that contain this chart
+ dashboards = (
+ db.session.query(Dashboard)
+ .filter(Dashboard.slices.any(id=chart_id)) # type:
ignore[attr-defined]
+ .all()
+ )
Review Comment:
Good morning @reynoldmorel, you're right. I've optimized this:
Performance: Changed from loading full Dashboard objects to just (id,
json_metadata) tuples. Added a safety limit of 1000 dashboards with logging if
exceeded.
Soft-delete: Confirmed there's no soft-delete column on Dashboard - only
published boolean, so the query won't return deleted records.
The limit of 1000 is somewhat arbitrary - happy to adjust based on what
makes sense for your production usage. The error log will make it clear if we
ever hit that edge case.
--
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]
