gabotorresruiz commented on code in PR #45130:
URL: https://github.com/apache/superset/pull/45130#discussion_r4252446969
##########
docs/docs/using-superset/recently-archived.mdx:
##########
@@ -67,6 +67,18 @@ you can view but not edit still appear in the list. The
**Recover** and
that object type; recovering a specific object can still be refused if you
are not one of its editors.
+If a dashboard filter or display control references a semantic view that was
+deleted while the dashboard was archived, recovery removes that entire control
+and its cascade links. Unaffected controls, including table filters, are
preserved.
+The REST and MCP restore responses include a warning in their `message` field;
review the
Review Comment:
Not a blocker, and you do call this out in the PR body, but since this
paragraph lands in the user-facing guide I think it reads as though someone
recovering from the Archive page will see the warning. They will not:
`performRowAction` in `superset-frontend/src/pages/ArchivedList/index.tsx`
discards the `SupersetClient.post` response and builds the success toast from
the row name alone, so the only signal a UI user gets is that the dashboard
came back.
Could we say that plainly here, so the "review the remaining filters" advice
does not rest on a channel the reader does not have? Something like:
```
Recovery through the UI does not report the removal, so review the
dashboard's
filters after recovering. The REST and MCP restore responses carry the
warning
in their `message` field.
```
##########
superset/commands/dashboard/restore.py:
##########
@@ -44,6 +101,81 @@ class
RestoreDashboardCommand(BaseRestoreCommand[Dashboard]):
forbidden_exc = DashboardForbiddenError
restore_failed_exc = DashboardRestoreFailedError
+ def prepare_restore(self, model: Dashboard) -> None:
+ """Remove controls with missing semantic targets before making them
live.
+
+ A missing target invalidates the entire control, including its
defaults.
+ Do not confuse unavailable providers or lost datasource grants with
deletion:
+ only metadata existence matters after dashboard editorship was
validated.
+ """
+ metadata: dict[str, Any] | None =
_parse_restore_metadata(model.json_metadata)
+ if metadata is None:
+ self.warnings.append(
+ "Warning: dashboard filter cleanup was skipped because its
metadata "
+ "is malformed. Review the dashboard filters before using its
results."
+ )
+ return
+ keys: tuple[str, str] = (
+ "native_filter_configuration",
+ "chart_customization_config",
+ )
+ controls: list[dict[str, Any]] = [
+ control for key in keys for control in metadata.get(key) or []
+ ]
+ semantic_ids: dict[int, set[int | None]] = {
+ id(control): {
+ _semantic_target_id(target)
+ for target in control.get("targets") or []
+ if target.get("datasourceType") == "semantic_view"
Review Comment:
@rebenitez1802 asked whether legacy `chart_customization_config` entries can
reference a semantic view and so slip past this check. I went looking and I
believe the answer is no, so I do not think there is a carve-out to document:
- The legacy shape has no field that can carry a datasource type.
`LegacyChartCustomizationConfig.dataset` is `string | number | {value, label,
table_name?} | null`, and `migrateChartCustomization` emits `targets:
[{datasetId, column}]` with no `datasourceType` at all.
- It also predates the feature. The legacy format already had its migration
path in #37176 (Feb 2026), while `datasourceType` only appeared on filter
targets in #40475 (Jul 2026), so a row written in the legacy shape predates any
way to point a control at a semantic view.
And `== "semantic_view"` here agrees with every other reader of these
targets in the repo, all of which treat an absent `datasourceType` as `table`:
`superset/security/manager.py:918` and `:5417`,
`superset/semantic_layers/import_export.py:159`, and
`getFormDataWithExtraFilters.ts:89`. I might be missing a path, but I could not
find one, so I would leave this as is.
--
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]
