Skip to content

Commit 3318c24

Browse files
Mike Bridgeclaude
andcommitted
feat(soft-delete): gate soft delete behind a temporary SOFT_DELETE flag
Adds a temporary rollout / kill-switch feature flag (`SOFT_DELETE`, default off) so the soft-delete substrate can ship dark and be activated per-deploy once validated — separating deploy from release for a behavior change with cross-entity blast radius (sc-111230). This is deployment scaffolding, not a permanent toggle: it is removed once soft delete is stable, leaving the unconditional "always active" end state. Compatible with the earlier "no permanent feature flag" decision, not a reversal. Two gate points (both no-op to legacy behavior when off): - `BaseDAO.delete()` routes to soft_delete only when the flag is on; otherwise hard_delete. - The `do_orm_execute` visibility listener attaches no criteria when off. Both write- and read-path gate on the same flag, so they cannot diverge. Restore needs no explicit gate: with the flag off no row carries `deleted_at`, so a restore call has nothing to act on. The flag docstring documents the kill-switch semantics (flipping off after rows are soft-deleted resurrects them — emergency stop, not a clean rollback) and that the import pipeline's existing-row detection is gated transitively via the listener. The `deleted_at` migration stays un-gated (additive). Existing soft-delete unit tests are updated to enable the flag (they assert the on-behavior); new tests pin the gate-off path: a mixin model hard-deletes and the listener does not filter. Removal is a small follow-up once soaked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 464c67d commit 3318c24

6 files changed

Lines changed: 101 additions & 9 deletions

File tree

docs/static/feature-flags.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@
8787
"lifecycle": "development",
8888
"description": "Enable semantic layers and show semantic views alongside datasets"
8989
},
90+
{
91+
"name": "SOFT_DELETE",
92+
"default": false,
93+
"lifecycle": "development",
94+
"description": "Temporary rollout / kill-switch gate for soft delete (origin: sc-103157; gate: sc-111230). Default off. When off: DELETE hard-deletes (legacy) and the read-path visibility filter is inactive, so the substrate ships dark; the import pipeline's existing-row detection inherits this via the same listener (no separate check); restore has nothing to act on because no row carries deleted_at. Flipping ON->OFF after rows were soft-deleted RESURRECTS them (they become visible/active again) \u2014 an emergency stop, not a clean rollback. Not a permanent toggle: REMOVE this flag and its two gate points (BaseDAO.delete routing; the do_orm_execute visibility listener) once soft delete is stable."
95+
},
9096
{
9197
"name": "TABLE_V2_TIME_COMPARISON_ENABLED",
9298
"default": false,

superset/config.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,18 @@ class D3TimeFormat(TypedDict, total=False):
630630
# can_copy_clipboard) instead of the single can_csv permission
631631
# @lifecycle: development
632632
"GRANULAR_EXPORT_CONTROLS": False,
633+
# Temporary rollout / kill-switch gate for soft delete (origin: sc-103157;
634+
# gate: sc-111230). Default off. When off: DELETE hard-deletes (legacy) and
635+
# the read-path visibility filter is inactive, so the substrate ships dark;
636+
# the import pipeline's existing-row detection inherits this via the same
637+
# listener (no separate check); restore has nothing to act on because no row
638+
# carries deleted_at. Flipping ON->OFF after rows were soft-deleted RESURRECTS
639+
# them (they become visible/active again) — an emergency stop, not a clean
640+
# rollback. Not a permanent toggle: REMOVE this flag and its two gate points
641+
# (BaseDAO.delete routing; the do_orm_execute visibility listener) once soft
642+
# delete is stable.
643+
# @lifecycle: development
644+
"SOFT_DELETE": False,
633645
# Enable semantic layers and show semantic views alongside datasets
634646
# @lifecycle: development
635647
"SEMANTIC_LAYERS": False,

superset/daos/base.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
from superset_core.common.daos import BaseDAO as CoreBaseDAO
4646
from superset_core.common.models import CoreModel
4747

48+
from superset import is_feature_enabled
4849
from superset.constants import SKIP_VISIBILITY_FILTER_CLASSES
4950
from superset.daos.exceptions import (
5051
DAOFindFailedError,
@@ -516,16 +517,22 @@ def delete(cls, items: list[T]) -> None:
516517
soft delete.
517518
518519
For models that include ``SoftDeleteMixin``, this calls
519-
``soft_delete()``. For all other models, this calls ``hard_delete()``
520-
(the original behaviour).
520+
``soft_delete()`` — but only while the temporary ``SOFT_DELETE`` rollout
521+
gate is enabled (sc-111230). When the gate is off, every model
522+
hard-deletes (the original behaviour), so the substrate can ship dark.
523+
For all other models, this always calls ``hard_delete()``.
521524
522525
:param items: The items to delete
523526
"""
524527
from superset.models.helpers import (
525-
SoftDeleteMixin, # pylint: disable=import-outside-toplevel
528+
SoftDeleteMixin, # avoid circular import: models.helpers <-> daos
526529
)
527530

528-
if cls.model_cls is not None and issubclass(cls.model_cls, SoftDeleteMixin):
531+
if (
532+
cls.model_cls is not None
533+
and issubclass(cls.model_cls, SoftDeleteMixin)
534+
and is_feature_enabled("SOFT_DELETE")
535+
):
529536
cls.soft_delete(items)
530537
else:
531538
cls.hard_delete(items)

superset/models/helpers.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -792,8 +792,17 @@ def _should_attach_soft_delete_criteria(execute_state: ORMExecuteState) -> bool:
792792
relationship-load event closes that gap. The resulting WHERE
793793
clause may have ``deleted_at IS NULL`` twice when propagation
794794
DOES work — harmless redundancy, idempotent SQL.
795+
796+
Gated by the temporary ``SOFT_DELETE`` rollout flag (sc-111230): while it is
797+
off, no criteria are attached, so soft-deleted rows (which the delete path
798+
does not create while the flag is off) are not filtered. The flag and this
799+
gate are removed once soft delete is stable.
795800
"""
796-
return execute_state.is_select and not execute_state.is_column_load
801+
return (
802+
execute_state.is_select
803+
and not execute_state.is_column_load
804+
and is_feature_enabled("SOFT_DELETE")
805+
)
797806

798807

799808
def _all_soft_delete_subclasses() -> list[type[SoftDeleteMixin]]:

tests/unit_tests/daos/test_base_dao_soft_delete.py

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,37 @@ class _PlainDAO(BaseDAO[_Plain]):
5959
model_cls = _Plain
6060

6161

62-
def test_delete_routes_to_soft_delete_for_mixin_models(app_context: None) -> None:
63-
"""delete() calls soft_delete() when model_cls includes SoftDeleteMixin."""
62+
@patch("superset.daos.base.is_feature_enabled", return_value=True)
63+
def test_delete_routes_to_soft_delete_for_mixin_models(
64+
mock_flag: MagicMock, app_context: None
65+
) -> None:
66+
"""delete() soft-deletes a mixin model when the SOFT_DELETE gate is ON."""
6467
items = [MagicMock(), MagicMock()]
6568

6669
with patch.object(_SoftDeletableDAO, "soft_delete") as mock_soft:
6770
_SoftDeletableDAO.delete(items)
6871
mock_soft.assert_called_once_with(items)
6972

7073

71-
def test_delete_routes_to_hard_delete_for_non_mixin_models(app_context: None) -> None:
72-
"""delete() calls hard_delete() for non-SoftDeleteMixin models."""
74+
@patch("superset.daos.base.is_feature_enabled", return_value=False)
75+
def test_delete_hard_deletes_mixin_model_when_gate_off(
76+
mock_flag: MagicMock, app_context: None
77+
) -> None:
78+
"""With the SOFT_DELETE gate OFF (default), even a mixin model hard-deletes
79+
— the substrate ships dark (sc-111230)."""
80+
items = [MagicMock(), MagicMock()]
81+
82+
with patch.object(_SoftDeletableDAO, "hard_delete") as mock_hard:
83+
_SoftDeletableDAO.delete(items)
84+
mock_hard.assert_called_once_with(items)
85+
86+
87+
@patch("superset.daos.base.is_feature_enabled", return_value=True)
88+
def test_delete_routes_to_hard_delete_for_non_mixin_models(
89+
mock_flag: MagicMock, app_context: None
90+
) -> None:
91+
"""delete() calls hard_delete() for non-SoftDeleteMixin models — regardless
92+
of the gate (here ON, to show the gate doesn't make a plain model soft)."""
7393
items = [MagicMock(), MagicMock()]
7494

7595
with patch.object(_PlainDAO, "hard_delete") as mock_hard:

tests/unit_tests/models/test_soft_delete_mixin.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
from collections.abc import Generator
2929
from datetime import datetime
30+
from unittest.mock import patch
3031

3132
import pytest
3233
from sqlalchemy import Column, ForeignKey, Integer, String
@@ -90,6 +91,18 @@ def _synthetic_tables(session: Session) -> Generator[None, None, None]:
9091
_TestBase.metadata.drop_all(session.get_bind())
9192

9293

94+
@pytest.fixture(autouse=True)
95+
def _soft_delete_gate_on() -> Generator[None, None, None]:
96+
"""The ``do_orm_execute`` visibility listener is gated by the temporary
97+
``SOFT_DELETE`` rollout flag (sc-111230), default off. These tests exercise
98+
the listener's filtering, so enable the gate for the whole module. The
99+
gate-off (listener-noop) behaviour is pinned separately by
100+
``test_listener_noop_when_gate_off``.
101+
"""
102+
with patch("superset.models.helpers.is_feature_enabled", return_value=True):
103+
yield
104+
105+
93106
@pytest.mark.usefixtures("_synthetic_tables")
94107
def test_soft_delete_sets_deleted_at(app_context: None, session: Session) -> None:
95108
"""soft_delete() sets deleted_at to a non-null datetime."""
@@ -167,6 +180,31 @@ def test_global_filter_excludes_soft_deleted_rows(
167180
assert result is None
168181

169182

183+
@pytest.mark.usefixtures("_synthetic_tables")
184+
def test_listener_noop_when_gate_off(app_context: None, session: Session) -> None:
185+
"""With the ``SOFT_DELETE`` gate OFF, the listener attaches no criteria, so a
186+
soft-deleted row is NOT hidden — the substrate is dark (sc-111230). (While
187+
the gate is off the delete path also doesn't create such rows; this pins the
188+
listener side.)"""
189+
obj = _SoftDeletable(name="visible_when_gate_off")
190+
session.add(obj)
191+
session.flush()
192+
obj_id = obj.id
193+
194+
obj.soft_delete()
195+
session.flush()
196+
session.expire_all()
197+
198+
with patch("superset.models.helpers.is_feature_enabled", return_value=False):
199+
result = (
200+
session.query(_SoftDeletable)
201+
.filter(_SoftDeletable.id == obj_id)
202+
.one_or_none()
203+
)
204+
assert result is not None
205+
assert result.id == obj_id
206+
207+
170208
@pytest.mark.usefixtures("_synthetic_tables")
171209
def test_listener_adapts_criteria_to_aliased_table_in_joins(
172210
app_context: None, session: Session

0 commit comments

Comments
 (0)