mikebridge opened a new pull request, #44259:
URL: https://github.com/apache/superset/pull/44259
### SUMMARY
Pure tidy-first refactor (SC-115506 + SC-115607): the `restore()` and
`purge()` soft-delete routes were triplicated across `ChartRestApi`,
`DashboardRestApi`, and `DatasetRestApi` with identical bodies that differed
only in the command class and the entity-specific exception types. This hoists
those bodies onto `SoftDeleteApiMixin`.
Two commits:
1. **restore** (all three APIs) →
`SoftDeleteApiMixin._restore_soft_deleted()`
2. **purge** (chart + dashboard only) →
`SoftDeleteApiMixin._purge_soft_deleted()`
`DatasetRestApi.purge()` is deliberately **not** hoisted — it takes a JSON
body with a confirmed impact token, has 400/409 responses, and a `purge_impact`
companion route, so it shares no body with the chart/dashboard twins. It and
`purge_impact` are left untouched.
Each concrete route keeps its `@expose` path, full decorator stack (same
order), and per-entity OpenAPI docstring verbatim; only the `try/except` body
moves. The helper is parametrized by class attributes each API declares
(`restore_command_cls`, the not-found / forbidden / restore-failed /
restore-conflict exception tuples, `purge_binding`, `purge_failed_errors`, and
the module logger). `raise_for_access`/`@protect` enforcement lives in the
commands and decorators and is unchanged.
No new features, no behaviour change, no public contract change, no
`UPDATING.md` entry.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — internal refactor, no user-facing or API-contract change.
### TESTING INSTRUCTIONS
Behavioural equivalence was verified locally:
- **OpenAPI spec is byte-identical** — regenerating on the pinned
requirements produces zero diff:
```
SUPERSET__SQLALCHEMY_DATABASE_URI='sqlite:///:memory:'
FLASK_APP='superset.app:create_app()' superset update-api-docs
git diff --exit-code docs/static/resources/openapi.json # clean
```
So `check-openapi-spec-drift` stays green.
- **Existing restore/purge integration tests pass unchanged** (not edited):
`tests/integration_tests/{charts,dashboards,datasets}/soft_delete_tests.py` —
70 passed, 3 skipped.
- **New parametrized unit test**
`tests/unit_tests/soft_delete/test_restore_purge_hoist.py` (42 cases) pins, per
route: the exact ordered decorator chain, FAB `_urls`/`_permission_name` and
runtime wrapper chain, the generated OpenAPI operation compared against the
published spec, and the complete success/404/403/422/unexpected mapping
including exception precedence and the not-logged extra-422 leg.
- ruff, ruff-format, and mypy (no new errors vs. baseline) clean on the
changed files.
### ADDITIONAL INFORMATION
- [x] Has associated issue: SC-115506, SC-115607
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01C7U5oFfrqUyM6hJTZiW2uf
--
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]
