Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/sentry/deletions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def load_defaults(manager: DeletionTaskManager) -> None:
manager.register(integrations.RepositoryProjectPathConfig, defaults.RepositoryProjectPathConfigDeletionTask)
manager.register(monitors.Monitor, defaults.MonitorDeletionTask)
manager.register(monitors.MonitorEnvironment, defaults.MonitorEnvironmentDeletionTask)
manager.register(monitors.MonitorCheckIn, BulkModelDeletionTask)
manager.register(monitors.MonitorCheckIn, defaults.MonitorCheckInDeletionTask)
manager.register(monitors.MonitorIncident, defaults.MonitorIncidentDeletionTask)
manager.register(monitors.MonitorEnvBrokenDetection, BulkModelDeletionTask)
manager.register(sentry_apps.PlatformExternalIssue, defaults.PlatformExternalIssueDeletionTask)
Expand Down
1 change: 1 addition & 0 deletions src/sentry/deletions/defaults/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from .grouphash import * # noqa: F401,F403
from .incident import * # noqa: F401,F403
from .monitor import * # noqa: F401,F403
from .monitor_checkin import * # noqa: F401,F403
from .monitor_environment import * # noqa: F401,F403
from .monitor_incident import * # noqa: F401,F403
from .organization import * # noqa: F401,F403
Expand Down
12 changes: 10 additions & 2 deletions src/sentry/deletions/defaults/monitor.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
from sentry.deletions.base import BaseRelation, ModelDeletionTask, ModelRelation
from sentry.deletions.base import (
BaseRelation,
BulkModelDeletionTask,
ModelDeletionTask,
ModelRelation,
)
from sentry.monitors.models import Monitor


Expand All @@ -8,6 +13,9 @@ def get_child_relations(self, instance: Monitor) -> list[BaseRelation]:

return [
ModelRelation(models.MonitorIncident, {"monitor_id": instance.id}),
ModelRelation(models.MonitorCheckIn, {"monitor_id": instance.id}),
# Use BulkModelDeletionTask here since MonitorIncidents are already handled above
ModelRelation(
models.MonitorCheckIn, {"monitor_id": instance.id}, BulkModelDeletionTask
),
ModelRelation(models.MonitorEnvironment, {"monitor_id": instance.id}),
]
20 changes: 20 additions & 0 deletions src/sentry/deletions/defaults/monitor_checkin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
from sentry.deletions.base import BaseRelation, ModelDeletionTask, ModelRelation
from sentry.monitors.models import MonitorCheckIn


class MonitorCheckInDeletionTask(ModelDeletionTask[MonitorCheckIn]):
def get_child_relations(self, instance: MonitorCheckIn) -> list[BaseRelation]:
from sentry.monitors import models

# When MonitorCheckIn is deleted directly, we need to delete MonitorIncidents
# that reference it. MonitorIncident has two FKs pointing to MonitorCheckIn.
return [
ModelRelation(
models.MonitorIncident,
{"starting_checkin_id": instance.id},
),
ModelRelation(
models.MonitorIncident,
{"resolving_checkin_id": instance.id},
),
]
9 changes: 8 additions & 1 deletion src/sentry/deletions/defaults/monitor_environment.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
from sentry.deletions.base import BaseRelation, ModelDeletionTask, ModelRelation
from sentry.deletions.base import (
BaseRelation,
BulkModelDeletionTask,
ModelDeletionTask,
ModelRelation,
)
from sentry.monitors.models import MonitorEnvironment


Expand All @@ -11,8 +16,10 @@ def get_child_relations(self, instance: MonitorEnvironment) -> list[BaseRelation
models.MonitorIncident,
{"monitor_environment_id": instance.id},
),
# Use BulkModelDeletionTask here since MonitorIncidents are already handled above
ModelRelation(
models.MonitorCheckIn,
{"monitor_environment_id": instance.id},
BulkModelDeletionTask,
),
]
117 changes: 104 additions & 13 deletions tests/sentry/deletions/test_monitor_checkin.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,106 @@


class DeleteMonitorCheckInTest(APITestCase, TransactionTestCase, HybridCloudTestMixin):
def test_delete_monitor_checkin_with_incidents_and_detections(self) -> None:

def test_delete_checkin_directly(self) -> None:
"""
Test that deleting a MonitorCheckIn directly (not via Monitor deletion)
properly handles MonitorIncident children via MonitorCheckInDeletionTask.
"""
project = self.create_project(name="test")
env = Environment.objects.create(organization_id=project.organization_id, name="prod")

monitor = Monitor.objects.create(
organization_id=project.organization.id,
project_id=project.id,
config={"schedule": "* * * * *", "schedule_type": ScheduleType.CRONTAB},
)
monitor_env = MonitorEnvironment.objects.create(
monitor=monitor,
environment_id=env.id,
)

# Create check-ins
checkin1 = MonitorCheckIn.objects.create(
monitor=monitor,
monitor_environment=monitor_env,
project_id=project.id,
status=CheckInStatus.ERROR,
)
checkin2 = MonitorCheckIn.objects.create(
monitor=monitor,
monitor_environment=monitor_env,
project_id=project.id,
status=CheckInStatus.OK,
)
checkin3 = MonitorCheckIn.objects.create(
monitor=monitor,
monitor_environment=monitor_env,
project_id=project.id,
status=CheckInStatus.ERROR,
)

# Create incidents referencing checkin1
incident1 = MonitorIncident.objects.create(
monitor=monitor,
monitor_environment=monitor_env,
starting_checkin=checkin1,
resolving_checkin=checkin2,
)
incident2 = MonitorIncident.objects.create(
monitor=monitor,
monitor_environment=monitor_env,
starting_checkin=checkin3,
resolving_checkin=checkin1, # checkin1 is also a resolving checkin
)

detection1 = MonitorEnvBrokenDetection.objects.create(
monitor_incident=incident1,
)
detection2 = MonitorEnvBrokenDetection.objects.create(
monitor_incident=incident2,
)

# Verify initial state
assert MonitorCheckIn.objects.count() == 3
assert MonitorIncident.objects.count() == 2
assert MonitorEnvBrokenDetection.objects.count() == 2

# Delete checkin1 directly (not via Monitor)
self.ScheduledDeletion.schedule(instance=checkin1, days=0)

with self.tasks():
run_scheduled_deletions()

# Verify checkin1 is deleted
assert not MonitorCheckIn.objects.filter(id=checkin1.id).exists()

# Verify both incidents are deleted (incident1 has checkin1 as starting_checkin,
# incident2 has checkin1 as resolving_checkin)
assert not MonitorIncident.objects.filter(id=incident1.id).exists()
assert not MonitorIncident.objects.filter(id=incident2.id).exists()

# Verify detections are deleted
assert not MonitorEnvBrokenDetection.objects.filter(id=detection1.id).exists()
assert not MonitorEnvBrokenDetection.objects.filter(id=detection2.id).exists()

# Verify other check-ins still exist
assert MonitorCheckIn.objects.filter(id=checkin2.id).exists()
assert MonitorCheckIn.objects.filter(id=checkin3.id).exists()
assert MonitorCheckIn.objects.count() == 2

# Verify monitor and environment still exist
assert Monitor.objects.filter(id=monitor.id).exists()
assert MonitorEnvironment.objects.filter(id=monitor_env.id).exists()

def test_delete_monitor_with_incidents_and_detections(self) -> None:
"""
Test that deleting MonitorCheckIns properly cascades to:
- MonitorIncidents (via starting_checkin_id and resolving_checkin_id)
- MonitorEnvBrokenDetection (via MonitorIncident)
Test that deleting a Monitor properly cascades to:
- MonitorIncidents (deleted first via child relations)
- MonitorCheckIns (bulk deleted after incidents)
- MonitorEnvBrokenDetection (via MonitorIncident deletion)

This tests the get_child_relations_bulk() implementation which should:
1. Use __in queries for MonitorIncidents pointing to multiple check-ins
2. Bulk delete MonitorEnvBrokenDetection via BulkModelDeletionTask
This verifies the ordered deletion: MonitorIncident → MonitorCheckIn → MonitorEnvironment
"""
project = self.create_project(name="test")
env = Environment.objects.create(organization_id=project.organization_id, name="prod")
Expand Down Expand Up @@ -107,10 +198,10 @@ def test_delete_monitor_checkin_with_incidents_and_detections(self) -> None:
assert Environment.objects.filter(id=env.id).exists()
assert self.project.__class__.objects.filter(id=self.project.id).exists()

def test_delete_multiple_checkins_with_shared_incident(self) -> None:
def test_delete_monitor_with_shared_incident(self) -> None:
"""
Test edge case where one incident references multiple check-ins
(starting_checkin != resolving_checkin from the same batch).
Test that deleting a Monitor handles edge case where one incident references
multiple check-ins (starting_checkin != resolving_checkin).
"""
project = self.create_project(name="test")
env = Environment.objects.create(organization_id=project.organization_id, name="prod")
Expand Down Expand Up @@ -162,10 +253,10 @@ def test_delete_multiple_checkins_with_shared_incident(self) -> None:
assert not MonitorIncident.objects.filter(id=incident.id).exists()
assert not MonitorEnvBrokenDetection.objects.filter(id=detection.id).exists()

def test_delete_monitor_only_affects_its_own_checkins(self) -> None:
def test_delete_monitor_only_affects_its_own_data(self) -> None:
"""
Test that deleting one monitor's check-ins doesn't affect another monitor's data.
This verifies that the __in queries are properly scoped.
Test that deleting one Monitor doesn't affect another Monitor's data.
This verifies that deletion queries are properly scoped by monitor_id.
"""
project = self.create_project(name="test")
env = Environment.objects.create(organization_id=project.organization_id, name="prod")
Expand Down
Loading