mikebridge commented on code in PR #40128:
URL: https://github.com/apache/superset/pull/40128#discussion_r3501511516
##########
superset/commands/dashboard/exceptions.py:
##########
@@ -70,6 +71,24 @@ class
DashboardColorsConfigUpdateFailedError(UpdateFailedError):
message = _("Dashboard color configuration could not be updated.")
+class DashboardRestoreFailedError(UpdateFailedError):
+ # Restore semantically clears ``deleted_at``; it is an UPDATE, not a new
+ # row. ``UpdateFailedError`` is the nearest typed middle-tier base in the
+ # codebase. A dedicated ``RestoreFailedError`` in
+ # ``superset/commands/exceptions.py`` would be more precise across the
+ # entity rollouts but lives in already-merged infrastructure (#39977);
Review Comment:
Fixed in d3059265 — removed the inaccurate "#39977" claim.
`DashboardRestoreFailedError` already inherits `UpdateFailedError`; the comment
now just notes that extracting a shared `RestoreFailedError` into
`superset/commands/exceptions.py` is a cross-entity follow-up.
_— posted by Claude (AI agent) on behalf of @mikebridge_
##########
superset/daos/dashboard.py:
##########
@@ -245,13 +245,26 @@ def get_dashboard_and_datasets_changed_on( # pylint:
disable=invalid-name
@staticmethod
def validate_slug_uniqueness(slug: str) -> bool:
+ # The ``SoftDeleteMixin`` listener auto-appends ``deleted_at IS NULL``
+ # to this query, so soft-deleted rows are correctly ignored on
+ # PostgreSQL and MySQL 8.0+ where slug uniqueness is enforced by a
+ # partial index (``ix_dashboards_active_slug``). On SQLite, MariaDB,
+ # and MySQL <8.0 the database retains a full unique constraint on
+ # ``slug``; a soft-deleted row will still block insertion of a new
+ # row with the same slug at flush time, even though this check
+ # passes. The fallback constraint is documented in UPDATING.md and
+ # in the 9e1f3b8c4d2a migration.
if not slug:
Review Comment:
Fixed in d3059265 — `validate_slug_uniqueness` now guards on `slug is None`
(param widened to `str | None`), so an empty-string slug runs the uniqueness
check, matching `validate_update_slug_uniqueness`.
_— posted by Claude (AI agent) on behalf of @mikebridge_
##########
superset/dashboards/filters.py:
##########
@@ -265,3 +266,38 @@ def apply(self, query: Query, value: Any) -> Query:
if value is False:
return query.filter(and_(Dashboard.created_by_fk.is_(None)))
return query
+
+
+class DashboardDeletedStateFilter( # pylint: disable=too-few-public-methods
+ BaseDeletedStateFilter
+):
+ """Rison filter for the GET list that exposes soft-deleted dashboards.
+
+ Soft-deleted rows are additionally scoped to the **restore audience**:
+ only the dashboard's owners (or admins) may enumerate them. This mirrors
+ ``RestoreDashboardCommand``'s ``raise_for_ownership`` check, so a
+ read-access non-owner (who can see the dashboard via published-datasource
+ access or dashboard RBAC) cannot list soft-deleted dashboards they could
+ never restore. Live rows are unaffected — they keep their normal
+ ``DashboardAccessFilter`` visibility.
+ """
+
+ arg_name = "dashboard_deleted_state"
+ model = Dashboard
+
+ def apply(self, query: Query, value: Any) -> Query:
+ query = super().apply(query, value)
+ normalized = str(value).lower().strip() if value is not None else ""
Review Comment:
Fixed in d3059265 — added a `_normalize` staticmethod on
`BaseDeletedStateFilter`; the base `apply()` and `DashboardDeletedStateFilter`
both use it now, so there is a single normalization implementation that cannot
drift.
_— posted by Claude (AI agent) on behalf of @mikebridge_
--
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]
