fix(semantic-layer): block deletion of sources with dependent assets

[mikebridge]

SUMMARY

Reject hard deletion of semantic views or layers while live charts, live dashboards containing those charts or targeting the views in native filters or display controls, or active reports/alerts depend on them. Return 409 with total, up to 20 dependents visible through corresponding list-access filters, and inaccessible_count for the rest. Each dependent's id is the integer key that the existing chart, dashboard and report REST endpoints accept, so a client can act on it directly. Schedule types are lowercase.

The dashboard target check uses a SQL LIKE prefilter followed by exact Python parsing of typed (datasourceType, datasetId) targets in both dashboard control lists; legacy id-only targets remain SQL datasets. Malformed dashboard metadata is ignored with debug logging of the dashboard ID only.

Limitations:

• The guard is best-effort: a dependent created or re-pointed between the check and the delete can still be orphaned. No chart/dashboard-write locking is added.
• Deliberately raw, unicode-escaped datasourceType text can evade the LIKE prefilter; normal dashboard writers serialize the literal ASCII value.
• The active report_schedule reference lookup is intentionally unindexed, since it only runs on a protected delete; an index can follow if volumes warrant.
• The dashboard candidate scan is bounded by workspace data, not a hard cap; the candidate query requests 1,000-row streaming batches where the database driver supports them.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable; API behavior only.

TESTING INSTRUCTIONS

Create a semantic view with a live chart, a dashboard containing it, a native-filter-only dashboard targeting it, a display-control-only dashboard targeting it, and active alert/report schedules. Verify single-view, bulk-view and layer deletes return 409 with lowercase types. Give a source editor no dashboard read access and verify the response counts but does not name it. Verify same-ID table/legacy targets, malformed metadata, archived dashboards/charts and inactive schedules do not block deletion; a string semantic-view ID does. Run the focused command and API tests and the semantic-layer coverage gate.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags: SEMANTIC_LAYERS
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

🤖 Generated with Claude Code

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	7abd4ca
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6ac81a8e4a13c30008e0d05c
😎 Deploy Preview	https://deploy-preview-45071--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codecov]

Codecov Report

❌ Patch coverage is 95.45455% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.75%. Comparing base (34b3d65) to head (7be89d9).
⚠️ Report is 16 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/commands/semantic_layer/delete.py	94.87%	2 Missing and 2 partials ⚠️
superset-frontend/src/pages/DatasetList/index.tsx	91.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #45071      +/-   ##
==========================================
+ Coverage   82.64%   82.75%   +0.10%     
==========================================
  Files        3007     3013       +6     
  Lines      187318   191989    +4671     
  Branches    43399    44509    +1110     
==========================================
+ Hits       154811   158878    +4067     
- Misses      29742    30076     +334     
- Partials     2765     3035     +270     

Flag	Coverage Δ
hive	35.36% <0.00%> (-0.83%)	⬇️
javascript	78.39% <91.66%> (+<0.01%)	⬆️
mysql	53.74% <0.00%> (-1.38%)	⬇️
postgres	53.74% <0.00%> (-1.38%)	⬇️
presto	37.14% <0.00%> (-0.91%)	⬇️
python	86.47% <95.91%> (+0.02%)	⬆️
sqlite	53.49% <0.00%> (-1.36%)	⬇️
unit	80.09% <95.91%> (+0.37%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mikebridge]

@aminghadersohi @rebenitez1802 this is ready for review: deleting a semantic layer or view is now blocked while charts, dashboards (including native filters that target it) or alerts/reports still depend on it, with a message listing the dependents.

[codeant-ai-for-open-source]

CodeAnt PR Risk: Low Risk

• The PR appears safe to merge; semantic-source deletes now protect live dependent assets and return structured conflict details.
• Focused tests cover dependency detection, visibility, API conflicts, and user-facing errors.
• The dashboard metadata scan can grow with workspace size, but runs only on semantic-source deletion and checks targets stored in unindexed metadata.

Assessed commit: 7be89d9d3349

[fitzee]

Verdict: COMMENT. I found no blocking bugs. The dependency guard does what the PR says on all three delete entry points. One design question is worth settling before merge: archived assets don't block the delete. The rest are follow-ups and nits.

Reviewed head 729053a6. The PR's 153 unit tests pass locally. I also ran _dependent_assets through the real ORM session, not the raw Connection the tests patch in, against SQLite, Postgres 16 and MySQL 8. Both the admin and non-admin visibility paths compile and return the expected (total, dependents, inaccessible_count) on all three. The yield_per streaming and SUM(CASE … IN (subquery)) work on each backend.

What I checked and found correct:

• Charts: matched on datasource_type == "semantic_view" plus the id. A table chart with the same numeric id does not block.
• Dashboards: caught both through chart membership and through typed native-filter / display-control targets.
• Reports and alerts: active ones block, whether they point at a chart or a dashboard. lower() on the Alert/Report enum values gives the documented lowercase types.
• Layer delete: covers every view in the layer, which the ORM/FK cascade would otherwise remove.
• Bulk delete: rejects the whole batch if any one view has dependents.
• Visibility: names come only from the list-API base filters (ChartFilter / DashboardAccessFilter / ReportScheduleFilter). Hidden dependents are only counted.
• Queries: no N+1. It is one count statement plus one limited fetch.

Should address / discuss

1. Archived charts and dashboards don't block a hard delete, and restoring them later gives broken assets. superset/commands/semantic_layer/delete.py:123,131,137 filter deleted_at IS NULL. The view delete is a hard delete. BaseRestoreCommand.validate (superset/commands/restore.py:76-104) does not re-check the datasource.

   • Scenario (with SOFT_DELETE on): archive chart 7, which uses view 42. DELETE /api/v1/semantic_view/42 returns 200. POST /api/v1/chart/<uuid>/restore then succeeds, and the restored chart has no datasource.
   • The same applies to an archived dashboard whose native filter targets the view.
   • Precedent: deleting a database blocks on soft-deleted datasets and tells the operator to purge them first (superset/commands/database/delete.py:82-102). That precedent is FK-driven, but users see the same contract: "archived = recoverable".
   • The limitation is documented (docs/.../importing-exporting-datasources.mdx:158). Still, it would be better either to count archived dependents (perhaps in a separate field, with a purge hint) or to make restore reject a chart whose semantic view is gone.

2. The frontend ignores the structured 409.

   • Single view (superset-frontend/src/pages/DatasetList/index.tsx:823-839) and layer (superset-frontend/src/pages/DatabaseList/index.tsx:784-797): the toast shows only the generic "Semantic source has dependent assets and cannot be deleted." It does not show total or dependents, so the user can't tell what to fix.
   • Bulk (DatasetList/index.tsx:1458-1490): a mixed selection of datasets and semantic views archives the datasets and then gets a 409 on the views. The user sees only "There was an issue deleting the selected datasets", with no hint that the views were refused or why.
   • Fine as a follow-up. A count in message (e.g. "used by 3 charts, 1 dashboard") would help with no UI change.

Nits

• delete.py:98-113: every view or layer delete scans and JSON-parses each live dashboard whose metadata contains semantic_view. It does this even when the view has no charts. It's acceptable for a delete-only path (the bot's perf flag is real but not blocking). If it ever matters, skip the scan when nothing else matched and no dashboard uses the view type.
• delete.py:61-68: str.isdecimal() accepts non-ASCII digits ("٤٢" → 42), and the try/except ValueError can't be reached after that check. Harmless over-match. raw_id.isascii() and raw_id.isdigit() would be tighter.
• delete.py:71-95 re-implements the target walk in superset/semantic_layers/import_export.py:44 (dashboard_targets), with lenient instead of raising semantics. A shared walker with a strict flag would keep the two from drifting when a third control list is added.
• delete.py:231: ORDER BY type, id sorts alert before chart. A source with 20 or more alerts lists only alerts and never the charts, which are usually what the user needs to act on.
• superset/semantic_layers/api.py: the 409 OpenAPI block is pasted three times (around lines 487, 580 and 1041), and the handler three times (lines 528, 629 and 1079). A shared components/responses entry plus a helper would keep them in sync.
• Status code: 409 is reasonable. Note that the closest existing case, a database with datasets attached, returns 422 (superset/databases/api.py:652). Clients that treat "has dependents" as 422 would need a second case.
• Tests: delete_test.py patches db.session.execute with a raw Connection (e.g. around line 343). That skips ORM execution, so the soft-delete do_orm_execute listener and ORM yield_per handling never run in tests. One test that uses the session fixture directly would cover the production path. I did this locally and it passes.

The race between check and delete (TOCTOU), the LIKE prefilter evasion and the unindexed schedule lookup are acknowledged in the PR body. I agree they're acceptable for a best-effort guard.

[aminghadersohi]

Two non-blocking test gaps inline; nothing here blocks.

[mikebridge]

Thanks for spelling out the archived-asset case. We're keeping archived dependents non-blocking for source deletion; dashboard restore is tracked separately in SC-125586. To be precise about charts: #44925 landed after the head you reviewed, and this update merges it. It makes chart version-history restore refuse a version whose datasource no longer exists. Restoring an archived chart from Trash is a different path, and it still succeeds. With #44926, also merged here, the restored chart's datasource-derived permissions are cleared because its source is gone, so it fails closed rather than keeping stale access. It still comes back as a chart without a datasource, though. So this doesn't resolve your example end to end; making Trash restore refuse or warn is a follow-up, not part of this PR.

[aminghadersohi]

The _semantic_target_matches guards are now pinned; the Dashboard/ReportSchedule can_read else-arms still are not (replied in that thread), and one new test gap is inline. The red sharded-jest-tests (2) leg is the Chart.test.tsx export-menu flake, unrelated to this PR.

[mikebridge]

Thanks for the follow-up review. Both remaining test gaps are addressed in 7be89d9d33.

• The unreadable-dependent case includes a dashboard and an active report. Independently changing either permission else-arm to sa.true() fails the no-names assertion.
• The mixed bulk-delete case asserts exactly one danger toast. Removing the rejected-status guard fails with two calls.
• Verification: 900 focused Python tests and all 14 DatasetList behavior tests passed; after restoring the mutations, 31 delete tests and all 14 Jest tests passed again. Changed-file hooks, including frontend type-checking, also passed.

This follow-up changes only tests.