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

   ### SUMMARY
   
   An archived dashboard can outlive a semantic view used by its native filters 
or display controls. Restoring the dashboard previously kept those dangling 
references, leaving broken controls in the recovered dashboard.
   
   Restore now removes an entire control when any of its explicitly typed 
semantic-view targets is missing or has an invalid ID, and removes links to 
that control from the remaining controls' `cascadeParentIds`. Table filters and 
other unaffected controls are preserved, including table targets with the same 
numeric ID as a deleted semantic view.
   
   The check uses metadata existence, not provider availability or datasource 
permissions: an existing view is kept even when its provider is disabled or 
unavailable to the restoring user. Cleanup and unarchive share the existing 
transaction, after editorship and slug validation. If stored metadata cannot be 
safely parsed or traversed, restore leaves its bytes unchanged, skips cleanup 
and returns a warning instead of preventing recovery.
   
   Warnings are returned in the existing REST/MCP `message` field. **This PR 
does not add a UI warning toast**; the archive UI does not display successful 
response messages. Callers should review the remaining dashboard filters before 
relying on its results.
   
   This complements #45071, which guards semantic-source deletion for **live** 
dependent assets. Archived dashboards are intentionally excluded from that 
guard, so they need this recovery handling. This is not a new concurrency 
protocol: a concurrent or later source deletion can still invalidate a restored 
dashboard.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Not applicable: backend restore handling, with no UI change.
   
   | Scenario | Before | After |
   | --- | --- | --- |
   | Filter/display control targets a deleted semantic view | Dangling control 
survives restore | Whole control and incoming cascade links removed; response 
warns |
   | Existing view is inaccessible or its provider is disabled | Control 
retained | Control retained |
   | Malformed legacy dashboard metadata | Restorable without cleanup | Still 
restorable, metadata preserved byte-for-byte; response warns cleanup was 
skipped |
   | Ordinary table filter | Retained | Retained |
   
   ### TESTING INSTRUCTIONS
   
   1. Enable `SOFT_DELETE` and `SEMANTIC_LAYERS`. Create a dashboard with a 
semantic-view native filter or display control, an unaffected table filter, and 
a control cascading from the semantic control.
   2. Archive the dashboard, then delete the semantic view (with no other live 
dependents). Restore the dashboard as an editor through REST or MCP.
   3. Verify the broken control is removed, incoming cascade links are pruned, 
unrelated metadata/table filters remain, and the response `message` warns about 
the removed control. No UI warning toast is expected.
   4. Repeat while retaining the view but disabling its provider or removing 
datasource access. The control must remain.
   5. Regression tests cover persisted malformed/deep JSON, invalid IDs, 
rollback on commit failure, denied editorship, slug conflict and both REST/MCP 
warning propagation.
   
   ```bash
   pytest tests/unit_tests/commands/dashboard/restore_semantic_filters_test.py \
          tests/unit_tests/commands/dashboard/restore_test.py \
          tests/unit_tests/commands/test_base_restore_command.py \
          tests/unit_tests/soft_delete/test_restore_purge_hoist.py \
          tests/unit_tests/mcp_service/dashboard/tool/test_restore_dashboard.py 
\
          tests/unit_tests/commands/chart/restore_test.py \
          tests/unit_tests/commands/dataset/restore_test.py
   ```
   
   Local validation: the focused suite passed 124 tests. Both deeply nested 
JSON regression cases failed before the parser fix and passed after it. 
Changed-file pre-commit hooks passed, including MyPy, Ruff and pylint. 
PostgreSQL/MySQL and browser validation were not run locally.
   
   Whole unit suite (`TZ=UTC`): **22,668 passed, 40 skipped, 2 xfailed**, with 
exactly three known local-environment baseline failures in unchanged 
numeric-contract and translation-fixture tests. All 40 semantic-restore cases 
passed in that full run.
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [x] Required feature flags: `SOFT_DELETE`, `SEMANTIC_LAYERS`
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] 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
   
   Related: #45071. @aminghadersohi @rebenitez1802
   
   AI-assisted implementation and review with Codex and Claude.
   


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