mikebridge opened a new pull request, #40128: URL: https://github.com/apache/superset/pull/40128
### SUMMARY Wire **dashboards** to the soft-delete infrastructure shipped in #39977 (sc-106188). Second of four PRs decomposing the soft-delete work for charts, dashboards, and datasets per [SIP-208](https://github.com/apache/superset/issues/39464) (passed 2026-05-08). **Depends on #39977 — do not merge until that PR is in master.** This PR consumes the abstractions (`SoftDeleteMixin`, `BaseRestoreCommand`, `BaseDeletedStateFilter`, `SoftDeleteApiMixin`, `find_existing_for_import` / `clear_soft_deleted_for_import`) and wires them to `Dashboard`. #### What this PR ships **Migration** (`superset/migrations/versions/2026-05-08_12-05_9e1f3b8c4d2a_add_deleted_at_to_dashboards.py`) - `revision = 9e1f3b8c4d2a`, `down_revision = 33d7e0e21daa`. Adds a nullable `deleted_at` column and `ix_dashboards_deleted_at` index to `dashboards`. Reversible. Uses `superset.migrations.shared.utils` helpers (`add_columns`, `create_index`, `drop_index`, `drop_columns`) so the migration is idempotent. - **Note**: this PR and the chart (sc-106189) / dataset (sc-106191) PRs share `down_revision = 33d7e0e21daa`. Whichever lands second/third will need a rebase or merge migration. **Model + DAO** - `Dashboard` inherits `SoftDeleteMixin` (`superset/models/dashboard.py`). The `do_orm_execute` listener from the infrastructure PR begins filtering `Dashboard` queries (including relationship lazy loads, via `with_loader_criteria(..., propagate_to_loaders=True)`). - `DeleteDashboardCommand` needs **no source change** — `BaseDAO.delete()` routing in sc-106188 detects the mixin and dispatches to `soft_delete()`. **API** - `RestoreDashboardCommand` (`superset/commands/dashboard/restore.py`) — 4 ClassVar declarations subclassing `BaseRestoreCommand[Dashboard]`. No method overrides; the base class owns validation, ownership checks, and the transactional wrapper. - `POST /api/v1/dashboard/<uuid>/restore` endpoint (`superset/dashboards/api.py`) with the standard decorator stack (`@protect`, `@safe`, `@statsd_metrics`, `@event_logger.log_this_with_context`). Permission model mirrors delete exactly: endpoint-level `can_write`, resource-level `raise_for_ownership`. UUIDs in the path per the new-endpoints convention. - `DashboardDeletedStateFilter` (`superset/dashboards/filters.py`) — 2-line subclass of `BaseDeletedStateFilter` setting `arg_name = "dashboard_deleted_state"` and `model = Dashboard`. Wired into `search_filters` on `DashboardRestApi`. - `DashboardRestApi` gains `SoftDeleteApiMixin` and `"restore"` is added to `include_route_methods`. - `DashboardRestoreFailedError` exception type added (`superset/commands/dashboard/exceptions.py`). **Import pipeline** (`superset/commands/dashboard/importers/v1/utils.py`) - Updates `import_dashboard` to use the infrastructure's `find_existing_for_import` (pure lookup) + `clear_soft_deleted_for_import` (explicit destructive cleanup). The destructive call happens **after** the overwrite + permission check passes, so a soft-deleted match can't be quietly hard-deleted before the import command decides whether it has the right to proceed. **Embedded-dashboard regression coverage** - `tests/integration_tests/dashboards/soft_delete_tests.py` covers the case where a parent dashboard is soft-deleted: the embedded iframe URL still returns 200 (the embedded handler doesn't dereference the parent during render), and the dashboard API returns 404 cleanly. The assertion order is deliberate — the API check runs before the embedded GET, because the embedded handler clears the test-client session in CI; reordering keeps both assertions in scope. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF No UI changes in this PR — REST endpoints only. The frontend "Restore" button is a follow-up PR. | Endpoint | Before | After | |---|---|---| | `DELETE /api/v1/dashboard/<pk>` | Hard-deletes the row | Sets `deleted_at`; row remains but is hidden from default queries | | `GET /api/v1/dashboard/?...&dashboard_deleted_state=only` | Filter doesn't exist (400) | Returns soft-deleted dashboards with `deleted_at` populated | | `GET /api/v1/dashboard/?...&dashboard_deleted_state=include` | Filter doesn't exist (400) | Returns live + soft-deleted; `deleted_at` populated on deleted rows | | `POST /api/v1/dashboard/<uuid>/restore` | 404 (no such endpoint) | Clears `deleted_at` on the soft-deleted row | ### TESTING INSTRUCTIONS ```bash # Unit tests pytest tests/unit_tests/commands/dashboard/restore_test.py -v # Integration tests pytest tests/integration_tests/dashboards/soft_delete_tests.py -v ``` The integration suite covers: - DELETE sets `deleted_at` instead of removing the row - Soft-deleted dashboards excluded from default list - `dashboard_deleted_state=only` returns soft-deleted with `deleted_at` populated - `dashboard_deleted_state=include` returns both, response augmented - POST restore clears `deleted_at` (admin and owner paths) - Restore returns 403 for non-owners (via `raise_for_ownership`) - Restore returns 404 for non-existent or already-live dashboards - Re-import of a soft-deleted UUID via the v1 importer succeeds (and only after permission check) - Embedded iframe URL still loads when the parent dashboard is soft-deleted Manual verification was performed end-to-end against a running container — `deleted_state=only`, `include`, default list, and restore all behave as specified. ### ADDITIONAL INFORMATION - [X] Has associated issue: [sc-106190](https://app.shortcut.com/preset/story/106190), [SIP-208 #39464](https://github.com/apache/superset/issues/39464) - [ ] Required feature flags: - [ ] Changes UI - [X] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [X] Migration is atomic, supports rollback & is backwards-compatible (nullable column + index; downgrade reverses both) - [X] Confirm DB migration upgrade and downgrade tested - [X] Runtime estimates and downtime expectations provided — *additive only: nullable column with no default + a single-column index on a typically small table (`dashboards`). Instant on Postgres/MySQL/SQLite for any realistic deployment size; no lock-holding.* - [X] Introduces new feature or API — soft-delete behaviour, `dashboard_deleted_state` rison filter, `POST /api/v1/dashboard/<uuid>/restore` endpoint, `deleted_at` field on list responses when augmentation is opted into - [ ] Removes existing feature or API #### Behaviour changes worth flagging - **`DELETE /api/v1/dashboard/<pk>` is now soft.** Existing API consumers that called DELETE expecting permanent removal will see the dashboard reappear if it's restored. The row is no longer findable via the default API after DELETE (the listener hides it), so most downstream behaviour is preserved — but anyone doing direct DB queries on the `dashboards` table will see the row. - **Embedded dashboards on a soft-deleted parent return 404 from the dashboard API** but the iframe URL itself is unaffected (the embed handler renders without dereferencing the parent during initial load). Covered by integration tests; doc note may be worth adding to `UPDATING.md` if reviewers think this needs surfacing for external consumers. #### Depends on - #39977 (infrastructure) #### Coordination with sibling PRs This PR's migration shares `down_revision = 33d7e0e21daa` with #39977's sibling rollouts: - sc-106189 (charts soft-delete) — `7c4a8d09ca37` - sc-106191 (datasets soft-delete) — `3a8e6f2c1b95` The first of the three to merge keeps its `down_revision`; the others will need a rebase or a merge migration. -- 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]
