mikebridge opened a new pull request, #43724:
URL: https://github.com/apache/superset/pull/43724

   ### SUMMARY
   
   With `SOFT_DELETE` enabled, archiving a dataset warns about the charts and 
dashboards that depend on it — but permanently deleting that same dataset from 
**Settings → Recently archived** showed only a generic type-DELETE 
confirmation. The irreversible action carried less warning than the recoverable 
one, and confirming it silently orphaned every dependent chart (the non-cascade 
itself is intended behaviour: `ForcePurgeCommand` preserves independently owned 
entities).
   
   This PR makes the permanent-delete confirmation for an **archived dataset** 
show the full dependency impact before the user can confirm:
   
   - A new protected endpoint, `GET /api/v1/dataset/<uuid>/purge-impact`, 
returns the authoritative dependent-object counts (live **and** 
recoverable-archived charts and dashboards), the names/links the caller may 
see, an aggregate `restricted_count` for objects the caller cannot access (no 
names, UUIDs, or links leak), and a versioned SHA-256 `impact_token` 
fingerprinting the exact affected set.
   - The modal names and links the affected charts and dashboards (archived 
ones badged), shows explicit zero-states, keeps the type-DELETE gate, and 
disables **Delete** until the impact is known — if impact cannot be determined, 
permanent deletion is unavailable (Cancel/Retry only, fail closed).
   - The purge request must carry the confirmed `impact_token`. The purge 
re-collects impact at submit time and again under the dataset row lock inside 
the cascade; if the affected set changed since the user read the warning, the 
request mutates nothing and returns **409** with the refreshed impact, and the 
modal asks for a renewed confirmation (same TOCTOU shape as the version-restore 
guard). A post-write-ahead mismatch finalises its force-purge audit row as 
`failed` with zero removals.
   - Chart and dashboard purges keep the existing generic confirmation — the 
impact preview is dataset-only, where the orphaning harm is.
   - A reversible migration (`8f31c5d726ab`) adds two small indexes used by the 
impact queries: `slices(datasource_type, datasource_id)` and 
`dashboard_slices(slice_id)`.
   
   Design notes: dependents are deliberately **not** globally serialised — the 
guarantee is two submit-time rechecks, not an immutable set after the final 
snapshot (documented in the spec; a 7-lens review's serialisation finding was 
evaluated and rejected by design). Inaccessible dependents are counted but 
never named.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Archived dataset `purge_demo` with two live dependent charts.
   
   **Before** — no mention of the two charts this purge will orphan:
   
   ![Before: bare type-DELETE 
confirmation](https://raw.githubusercontent.com/mikebridge/superset/sc-118271-screenshots/purge-modal-before-light.png)
   
   **After** — authoritative counts, named and linked charts, explicit 
zero-state for dashboards, Delete gated on the typed confirmation:
   
   ![After: impact-aware confirmation, 
light](https://raw.githubusercontent.com/mikebridge/superset/sc-118271-screenshots/purge-modal-light.png)
   
   ![After: impact-aware confirmation, 
dark](https://raw.githubusercontent.com/mikebridge/superset/sc-118271-screenshots/purge-modal-dark.png)
   
   ### TESTING INSTRUCTIONS
   
   1. Enable `SOFT_DELETE`. Create a dataset and build two charts on it.
   2. Archive the dataset from the Datasets list, then open **Settings → 
Recently archived**, set Type to Dataset, and click **Delete permanently** on 
the row.
   3. The modal lists "2 Charts" with links (and "0 Dashboards" explicitly); 
**Delete** stays disabled until the impact has loaded and DELETE is typed.
   4. Stale-impact path: with the modal open, create another chart on the 
dataset in a second tab, then confirm — the purge is refused, nothing is 
deleted, and the modal shows the refreshed impact and asks to confirm again.
   5. Failure path: block `/purge-impact` in devtools and open the modal — an 
error state with Retry/Cancel appears and **Delete** cannot be reached.
   6. Confirm with a matching token: the dataset is purged; the charts remain 
(intended) and the force-purge audit row records the dangling chart UUIDs from 
the confirmed snapshot.
   7. Chart/dashboard rows in Recently archived keep the previous generic 
confirmation.
   
   Automated: `test_purge_impact.py` (canonical ordering, fingerprint 
stability, dashboard dedup, archived inclusion), `soft_delete_tests.py` 
(authorization incl. a caller with a restricted dependent; contract fields), 
`force_purge_tests.py` / `purge_tests.py` (matching/mismatched/same-count 
tokens, locked recheck, failed audit finalisation, token-free scheduled 
retention), `ArchivedList.test.tsx` (27 tests: loading, counts, badges, 
restricted aggregates, bounded lists, 409 renewal, error/retry, unchanged 
chart/dashboard flows), `DeleteModal.test.tsx` (external 
disable/loading/reset). Migration verified by upgrade→downgrade→re-upgrade on a 
fresh metadata DB, re-run after re-chaining onto `39097d124752`.
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [x] Required feature flags: `SOFT_DELETE`
   - [x] 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
     - [x] Confirm DB migration upgrade and downgrade tested
     - [x] Runtime estimates and downtime expectations provided: two `CREATE 
INDEX` statements on `slices` and `dashboard_slices`; seconds on typical 
metadata volumes, no table rewrites, no downtime expected
   - [x] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   _This description was drafted with Claude (AI) assistance on behalf of 
@mikebridge._
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to