feat(soft-delete): gate soft delete behind a temporary SOFT_DELETE release toggle (#41166)

Co-authored-by: Mike Bridge <michael.bridge@ext.preset.io>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

tests/unit_tests/daos/test_base_dao_soft_delete.py

@@ -59,18 +59,52 @@ class _PlainDAO(BaseDAO[_Plain]):
     model_cls = _Plain
 
 
-def test_delete_routes_to_soft_delete_for_mixin_models(app_context: None) -> None:
-    """delete() calls soft_delete() when model_cls includes SoftDeleteMixin."""
-    items = [MagicMock(), MagicMock()]
+@patch("superset.daos.base.is_feature_enabled", return_value=True)
+def test_delete_routes_to_soft_delete_for_mixin_models(
+    mock_flag: MagicMock, app_context: None
+) -> None:
+    """delete() soft-deletes a mixin model when the SOFT_DELETE gate is ON."""
+    items: list[MagicMock] = [MagicMock(), MagicMock()]
 
     with patch.object(_SoftDeletableDAO, "soft_delete") as mock_soft:
         _SoftDeletableDAO.delete(items)
         mock_soft.assert_called_once_with(items)
 
 
-def test_delete_routes_to_hard_delete_for_non_mixin_models(app_context: None) -> None:
-    """delete() calls hard_delete() for non-SoftDeleteMixin models."""
-    items = [MagicMock(), MagicMock()]
+@patch("superset.daos.base.is_feature_enabled", return_value=False)
+def test_delete_hard_deletes_mixin_model_when_gate_off(
+    mock_flag: MagicMock, app_context: None
+) -> None:
+    """With the SOFT_DELETE gate OFF (default), even a mixin model hard-deletes
+    — the substrate ships dark."""
+    items: list[MagicMock] = [MagicMock(), MagicMock()]
+
+    with patch.object(_SoftDeletableDAO, "hard_delete") as mock_hard:
+        _SoftDeletableDAO.delete(items)
+        mock_hard.assert_called_once_with(items)
+
+
+@patch("superset.daos.base.is_feature_enabled", return_value=True)
+def test_delete_routes_to_hard_delete_for_non_mixin_models(
+    mock_flag: MagicMock, app_context: None
+) -> None:
+    """delete() calls hard_delete() for non-SoftDeleteMixin models — regardless
+    of the gate (here ON, to show the gate doesn't make a plain model soft)."""
+    items: list[MagicMock] = [MagicMock(), MagicMock()]
+
+    with patch.object(_PlainDAO, "hard_delete") as mock_hard:
+        _PlainDAO.delete(items)
+        mock_hard.assert_called_once_with(items)
+
+
+@patch("superset.daos.base.is_feature_enabled", return_value=False)
+def test_delete_hard_deletes_non_mixin_model_when_gate_off(
+    mock_flag: MagicMock, app_context: None
+) -> None:
+    """A non-SoftDeleteMixin model hard-deletes with the gate OFF too — the
+    mixin check short-circuits to hard_delete before the gate is evaluated.
+    Completes the (gate, model_type) matrix's fourth cell."""
+    items: list[MagicMock] = [MagicMock(), MagicMock()]
 
     with patch.object(_PlainDAO, "hard_delete") as mock_hard:
         _PlainDAO.delete(items)
@@ -82,7 +116,7 @@ def test_hard_delete_calls_session_delete(
     mock_db: MagicMock, app_context: None
 ) -> None:
     """hard_delete() calls db.session.delete() on each item."""
-    items = [MagicMock(), MagicMock()]
+    items: list[MagicMock] = [MagicMock(), MagicMock()]
 
     BaseDAO.hard_delete(items)
 
@@ -93,7 +127,7 @@ def test_hard_delete_calls_session_delete(
 
 def test_soft_delete_calls_item_soft_delete(app_context: None) -> None:
     """soft_delete() calls soft_delete() on each item."""
-    items = [MagicMock(), MagicMock()]
+    items: list[MagicMock] = [MagicMock(), MagicMock()]
 
     BaseDAO.soft_delete(items)
     items[0].soft_delete.assert_called_once()

tests/unit_tests/models/test_soft_delete_mixin.py

@@ -27,6 +27,7 @@ from __future__ import annotations
 
 from collections.abc import Generator
 from datetime import datetime
+from unittest.mock import patch
 
 import pytest
 from sqlalchemy import Column, ForeignKey, Integer, String
@@ -90,6 +91,18 @@ def _synthetic_tables(session: Session) -> Generator[None, None, None]:
     _TestBase.metadata.drop_all(session.get_bind())
 
 
+@pytest.fixture(autouse=True)
+def _soft_delete_gate_on() -> Generator[None, None, None]:
+    """The ``do_orm_execute`` visibility listener is gated by the temporary
+    ``SOFT_DELETE`` rollout flag, default off. These tests exercise
+    the listener's filtering, so enable the gate for the whole module. The
+    gate-off (listener-noop) behaviour is pinned separately by
+    ``test_listener_noop_when_gate_off``.
+    """
+    with patch("superset.models.helpers.is_feature_enabled", return_value=True):
+        yield
+
+
 @pytest.mark.usefixtures("_synthetic_tables")
 def test_soft_delete_sets_deleted_at(app_context: None, session: Session) -> None:
     """soft_delete() sets deleted_at to a non-null datetime."""
@@ -167,6 +180,31 @@ def test_global_filter_excludes_soft_deleted_rows(
     assert result is None
 
 
+@pytest.mark.usefixtures("_synthetic_tables")
+def test_listener_noop_when_gate_off(app_context: None, session: Session) -> None:
+    """With the ``SOFT_DELETE`` gate OFF, the listener attaches no criteria, so a
+    soft-deleted row is NOT hidden — the substrate is dark. (While
+    the gate is off the delete path also doesn't create such rows; this pins the
+    listener side.)"""
+    obj: _SoftDeletable = _SoftDeletable(name="visible_when_gate_off")
+    session.add(obj)
+    session.flush()
+    obj_id: int = obj.id
+
+    obj.soft_delete()
+    session.flush()
+    session.expire_all()
+
+    with patch("superset.models.helpers.is_feature_enabled", return_value=False):
+        result: _SoftDeletable | None = (
+            session.query(_SoftDeletable)
+            .filter(_SoftDeletable.id == obj_id)
+            .one_or_none()
+        )
+    assert result is not None
+    assert result.id == obj_id
+
+
 @pytest.mark.usefixtures("_synthetic_tables")
 def test_listener_adapts_criteria_to_aliased_table_in_joins(
     app_context: None, session: Session