Skip to content

Conversation

@evanpurkhiser
Copy link
Member

Fixed NEW-648: Cron detectors can't 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.

@evanpurkhiser evanpurkhiser requested a review from a team as a code owner November 21, 2025 19:34
@linear
Copy link

linear bot commented Nov 21, 2025

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 21, 2025
@codecov
Copy link

codecov bot commented Nov 21, 2025

Codecov Report

❌ Patch coverage is 30.00000% with 7 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/monitors/validators.py 30.00% 7 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #103846      +/-   ##
===========================================
- Coverage   80.62%    80.61%   -0.01%     
===========================================
  Files        9287      9287              
  Lines      396357    396390      +33     
  Branches    25259     25259              
===========================================
+ Hits       319544    319550       +6     
- Misses      76353     76380      +27     
  Partials      460       460              

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.
@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/fix-monitors-allow-editing-cron-monitor-detectors-with-existing-slug branch from 1940b99 to cc636f4 Compare November 21, 2025 20:33

def to_internal_value(self, data):
# If we're updating (parent has instance), bind the Monitor instance to child validator
if hasattr(self.parent, "instance") and self.parent.instance:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could just be if getattr(self.parent, "instance", None)

@evanpurkhiser evanpurkhiser merged commit aff01be into master Nov 25, 2025
66 checks passed
@evanpurkhiser evanpurkhiser deleted the evanpurkhiser/fix-monitors-allow-editing-cron-monitor-detectors-with-existing-slug branch November 25, 2025 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants