Skip to content

Commit aff01be

Browse files
fix(monitors): Allow editing cron monitor detectors with existing slug (#103846)
Fixed [NEW-648: Cron detectors can't be edited due to error](https://linear.app/getsentry/issue/NEW-648/cron-detectors-cant-be-edited-due-to-error) When updating a cron monitor detector with dataSources in the request body, the slug validation was incorrectly rejecting the existing slug as "already in use". This occurred because the MonitorDataSourceValidator nested in a ListField didn't have its instance property set during validation. Created MonitorDataSourceListField to bind the Monitor instance to child validators before validation runs, allowing the slug check to recognize updates vs creates.
1 parent 4b341f6 commit aff01be

File tree

2 files changed

+59
-1
lines changed

2 files changed

+59
-1
lines changed

src/sentry/monitors/validators.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,28 @@ def update(self, instance: Monitor, validated_data: dict[str, Any]) -> Monitor:
686686
return monitor_validator.update(instance, monitor_validator.validated_data)
687687

688688

689+
class MonitorDataSourceListField(serializers.ListField):
690+
"""
691+
Custom ListField that properly binds the Monitor instance to child validators.
692+
693+
When updating a detector, we need to ensure the MonitorDataSourceValidator
694+
knows about the existing Monitor so slug validation works correctly.
695+
"""
696+
697+
def to_internal_value(self, data):
698+
# If we're updating (parent has instance), bind the Monitor instance to child validator
699+
if hasattr(self.parent, "instance") and self.parent.instance:
700+
detector = self.parent.instance
701+
monitor = get_cron_monitor(detector)
702+
703+
# Bind the monitor instance so slug validation recognizes this as an update
704+
# Type ignore: self.child is typed as Field but is actually MonitorDataSourceValidator
705+
self.child.instance = monitor # type: ignore[attr-defined]
706+
self.child.partial = self.parent.partial # type: ignore[attr-defined]
707+
708+
return super().to_internal_value(data)
709+
710+
689711
class MonitorIncidentDetectorValidator(BaseDetectorTypeValidator):
690712
"""
691713
Validator for monitor incident detection configuration.
@@ -694,7 +716,7 @@ class MonitorIncidentDetectorValidator(BaseDetectorTypeValidator):
694716
data_source field (MonitorDataSourceValidator).
695717
"""
696718

697-
data_sources = serializers.ListField(child=MonitorDataSourceValidator(), required=False)
719+
data_sources = MonitorDataSourceListField(child=MonitorDataSourceValidator(), required=False)
698720

699721
def validate_enabled(self, value: bool) -> bool:
700722
"""

tests/sentry/monitors/endpoints/test_organization_detector_details.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,42 @@ def test_update_monitor_incident_detector(self):
138138
assert self.monitor.name == original_monitor_name
139139
assert self.monitor.config == original_monitor_config
140140

141+
def test_update_monitor_incident_detector_with_data_sources(self):
142+
"""Test updating a monitor detector with dataSources field (including existing slug)."""
143+
data = {
144+
"name": "Updated Name",
145+
"dataSources": [
146+
{
147+
"name": "Updated Monitor Name",
148+
"slug": self.monitor.slug, # Keep the existing slug
149+
"config": {
150+
"checkin_margin": 1,
151+
"failure_issue_threshold": 1,
152+
"max_runtime": 30,
153+
"recovery_threshold": 1,
154+
"timezone": "UTC",
155+
"schedule": "0 0 * * 5",
156+
"schedule_type": "crontab",
157+
},
158+
}
159+
],
160+
}
161+
162+
response = self.get_success_response(
163+
self.organization.slug,
164+
self.detector.id,
165+
**data,
166+
status_code=200,
167+
method="PUT",
168+
)
169+
170+
assert response.data["name"] == "Updated Name"
171+
172+
self.monitor.refresh_from_db()
173+
assert self.monitor.name == "Updated Monitor Name"
174+
assert self.monitor.slug == "test-monitor" # Slug unchanged
175+
assert self.monitor.config["schedule"] == "0 0 * * 5"
176+
141177
def test_delete_monitor_incident_detector(self):
142178
"""Test deleting a monitor incident detector."""
143179
with outbox_runner():

0 commit comments

Comments
 (0)