diff --git a/src/sentry/monitors/models.py b/src/sentry/monitors/models.py index 5212ee901c7a6f..6cd308ee829d98 100644 --- a/src/sentry/monitors/models.py +++ b/src/sentry/monitors/models.py @@ -325,7 +325,7 @@ def get_audit_log_data(self): "name": self.name, "status": self.status, "config": self.config, - "is_muted": self.is_muted, + "is_muted": is_monitor_muted(self), "slug": self.slug, "owner_user_id": self.owner_user_id, "owner_team_id": self.owner_team_id, @@ -433,22 +433,22 @@ def normalize_before_relocation_import( self.guid = uuid4() return old_pk - @property - def is_muted(self) -> bool: - """ - A monitor is considered muted if ALL of its environments are muted. - If a monitor has no environments, it is considered unmuted. - """ - env_counts = MonitorEnvironment.objects.filter(monitor_id=self.id).aggregate( - total=models.Count("id"), muted=models.Count("id", filter=Q(is_muted=True)) - ) - # If no environments exist, monitor is not muted - if env_counts["total"] == 0: - return False +def is_monitor_muted(monitor: Monitor) -> bool: + """ + A monitor is considered muted if ALL of its environments are muted. + If a monitor has no environments, it is considered unmuted. + """ + env_counts = MonitorEnvironment.objects.filter(monitor_id=monitor.id).aggregate( + total=models.Count("id"), muted=models.Count("id", filter=Q(is_muted=True)) + ) + + # If no environments exist, monitor is not muted + if env_counts["total"] == 0: + return False - # Monitor is muted only if ALL environments are muted - return env_counts["total"] == env_counts["muted"] + # Monitor is muted only if ALL environments are muted + return env_counts["total"] == env_counts["muted"] def check_organization_monitor_limit(organization_id: int) -> None: @@ -637,7 +637,7 @@ def ensure_environment( monitor_env, created = MonitorEnvironment.objects.get_or_create( monitor=monitor, environment_id=environment.id, - defaults={"status": MonitorStatus.ACTIVE, "is_muted": monitor.is_muted}, + defaults={"status": MonitorStatus.ACTIVE, "is_muted": is_monitor_muted(monitor)}, ) # recompute per-project monitor check-in rate limit quota diff --git a/src/sentry/monitors/serializers.py b/src/sentry/monitors/serializers.py index c3badbf4f0cdc9..5e7317907daf9f 100644 --- a/src/sentry/monitors/serializers.py +++ b/src/sentry/monitors/serializers.py @@ -1,6 +1,8 @@ from collections import defaultdict from collections.abc import MutableMapping, Sequence from datetime import datetime +from itertools import groupby +from operator import attrgetter from typing import Any, Literal, TypedDict, cast from django.db.models import prefetch_related_objects @@ -198,6 +200,30 @@ def get_attrs(self, item_list, user, **kwargs): for actor, serialized_actor in zip(filtered_actors, actors_serialized) } + # Query ALL environments (unfiltered) to determine muted status + # A monitor is muted only if ALL its environments are muted + all_monitor_environments = ( + MonitorEnvironment.objects.filter(monitor__in=item_list) + .exclude( + status__in=[MonitorStatus.PENDING_DELETION, MonitorStatus.DELETION_IN_PROGRESS] + ) + .order_by("monitor_id") + ) + + # Group environments by monitor + monitor_envs_by_id = { + monitor_id: list(envs) + for monitor_id, envs in groupby(all_monitor_environments, key=attrgetter("monitor_id")) + } + + # A monitor is muted only if it has environments AND all of them are muted + is_muted_data = { + item.id: bool(monitor_envs_by_id.get(item.id, [])) + and not any(not env.is_muted for env in monitor_envs_by_id.get(item.id, [])) + for item in item_list + } + + # Now query the filtered environments for serialization monitor_environments_qs = ( MonitorEnvironment.objects.filter(monitor__in=item_list) .annotate(status_ordering=MONITOR_ENVIRONMENT_ORDERING) @@ -213,6 +239,7 @@ def get_attrs(self, item_list, user, **kwargs): monitor_environments = list(monitor_environments_qs) serialized_monitor_environments = defaultdict(list) + for monitor_env, serialized in zip( monitor_environments, serialize(monitor_environments, user) ): @@ -227,6 +254,7 @@ def get_attrs(self, item_list, user, **kwargs): "project": projects_data[item.project_id] if item.project_id else None, "environments": environment_data[item.id], "owner": actor_data.get(item.owner_actor), + "is_muted": is_muted_data[item.id], } for item in item_list } @@ -245,7 +273,7 @@ def serialize(self, obj, attrs, user, **kwargs) -> MonitorSerializerResponse: result: MonitorSerializerResponse = { "id": str(obj.guid), "status": obj.get_status_display(), - "isMuted": obj.is_muted, + "isMuted": attrs["is_muted"], "isUpserting": obj.is_upserting, "name": obj.name, "slug": obj.slug, diff --git a/tests/sentry/monitors/endpoints/test_base_monitor_details.py b/tests/sentry/monitors/endpoints/test_base_monitor_details.py index 96d466a010494f..d19bedb81466e3 100644 --- a/tests/sentry/monitors/endpoints/test_base_monitor_details.py +++ b/tests/sentry/monitors/endpoints/test_base_monitor_details.py @@ -18,6 +18,7 @@ MonitorEnvironment, MonitorIncident, ScheduleType, + is_monitor_muted, ) from sentry.monitors.types import DATA_SOURCE_CRON_MONITOR from sentry.monitors.utils import ensure_cron_detector, get_timeout_at @@ -354,7 +355,7 @@ def test_can_mute(self) -> None: assert resp.data["slug"] == monitor.slug monitor = Monitor.objects.get(id=monitor.id) - assert monitor.is_muted + assert is_monitor_muted(monitor) def test_can_unmute(self) -> None: monitor = self._create_monitor() @@ -367,7 +368,7 @@ def test_can_unmute(self) -> None: assert resp.data["slug"] == monitor.slug monitor = Monitor.objects.get(id=monitor.id) - assert not monitor.is_muted + assert not is_monitor_muted(monitor) def test_timezone(self) -> None: monitor = self._create_monitor() diff --git a/tests/sentry/monitors/endpoints/test_base_monitor_environment_details.py b/tests/sentry/monitors/endpoints/test_base_monitor_environment_details.py index 5dac3dc336a5c5..67ae7cc20a910f 100644 --- a/tests/sentry/monitors/endpoints/test_base_monitor_environment_details.py +++ b/tests/sentry/monitors/endpoints/test_base_monitor_environment_details.py @@ -1,6 +1,6 @@ from sentry.constants import ObjectStatus from sentry.deletions.models.scheduleddeletion import RegionScheduledDeletion -from sentry.monitors.models import Monitor, MonitorEnvironment, MonitorStatus +from sentry.monitors.models import Monitor, MonitorEnvironment, MonitorStatus, is_monitor_muted from sentry.testutils.cases import MonitorTestCase from sentry.testutils.helpers.datetime import freeze_time @@ -72,7 +72,7 @@ def test_muting_all_environments_mutes_monitor(self) -> None: # Initially monitor should be unmuted monitor.refresh_from_db() - assert monitor.is_muted is False + assert is_monitor_muted(monitor) is False # Mute first environment self.get_success_response( @@ -85,7 +85,7 @@ def test_muting_all_environments_mutes_monitor(self) -> None: # Monitor should still be unmuted (one environment is unmuted) monitor.refresh_from_db() - assert monitor.is_muted is False + assert is_monitor_muted(monitor) is False # Mute second environment self.get_success_response( @@ -98,7 +98,7 @@ def test_muting_all_environments_mutes_monitor(self) -> None: # Now monitor should be muted (all environments are muted) monitor.refresh_from_db() - assert monitor.is_muted is True + assert is_monitor_muted(monitor) is True def test_unmuting_one_environment_unmutes_monitor(self) -> None: """Test that unmuting one environment when all were muted also unmutes the monitor.""" @@ -111,7 +111,7 @@ def test_unmuting_one_environment_unmutes_monitor(self) -> None: # Verify initial state monitor.refresh_from_db() - assert monitor.is_muted is True + assert is_monitor_muted(monitor) is True # Unmute one environment via the endpoint self.get_success_response( @@ -124,7 +124,7 @@ def test_unmuting_one_environment_unmutes_monitor(self) -> None: # Monitor should now be unmuted monitor = Monitor.objects.get(id=monitor.id) - assert monitor.is_muted is False + assert is_monitor_muted(monitor) is False env1.refresh_from_db() assert env1.is_muted is False env2.refresh_from_db() diff --git a/tests/sentry/monitors/endpoints/test_organization_detector_index.py b/tests/sentry/monitors/endpoints/test_organization_detector_index.py index 4f30e3727f3ee1..1d26ceb2fa6e62 100644 --- a/tests/sentry/monitors/endpoints/test_organization_detector_index.py +++ b/tests/sentry/monitors/endpoints/test_organization_detector_index.py @@ -3,7 +3,7 @@ from sentry.api.serializers import serialize from sentry.constants import ObjectStatus from sentry.monitors.grouptype import MonitorIncidentType -from sentry.monitors.models import Monitor, ScheduleType +from sentry.monitors.models import Monitor, ScheduleType, is_monitor_muted from sentry.monitors.serializers import MonitorSerializer from sentry.monitors.types import DATA_SOURCE_CRON_MONITOR from sentry.testutils.cases import APITestCase @@ -205,7 +205,7 @@ def test_create_monitor_with_optional_fields(self): ) assert monitor.name == "Full Config Monitor" assert monitor.status == ObjectStatus.DISABLED - assert monitor.is_muted is False + assert is_monitor_muted(monitor) is False assert monitor.config["schedule"] == "*/30 * * * *" assert monitor.config["checkin_margin"] == 15 assert monitor.config["max_runtime"] == 120 diff --git a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py index f79691377bb415..812fe1a3ce7a26 100644 --- a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py +++ b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py @@ -12,7 +12,7 @@ from sentry.analytics.events.cron_monitor_created import CronMonitorCreated, FirstCronMonitorCreated from sentry.constants import DataCategory, ObjectStatus from sentry.models.rule import Rule, RuleSource -from sentry.monitors.models import Monitor, MonitorStatus, ScheduleType +from sentry.monitors.models import Monitor, MonitorStatus, ScheduleType, is_monitor_muted from sentry.monitors.utils import get_detector_for_monitor from sentry.quotas.base import SeatAssignmentResult from sentry.testutils.asserts import assert_org_audit_log_exists @@ -150,7 +150,7 @@ def test_sort_muted(self) -> None: self._create_monitor(name="A monitor"), self._create_monitor(name="ZA monitor"), ] - monitors.sort(key=lambda m: (m.is_muted, m.name)) + monitors.sort(key=lambda m: (is_monitor_muted(m), m.name)) response = self.get_success_response(self.organization.slug, sort="muted") self.check_valid_response(response, monitors) @@ -389,6 +389,45 @@ def test_ignore_pending_deletion_environments(self) -> None: assert len(response.data[0]["environments"]) == 1 assert response.data[0]["environments"][0]["status"] == "ok" + def test_is_muted_calculated_from_all_envs_not_filtered(self) -> None: + """Test that isMuted field is calculated from ALL environments, not just filtered ones""" + # Monitor with prod unmuted, dev muted + monitor = self._create_monitor(name="Test Monitor") + self._create_monitor_environment(monitor, name="prod", is_muted=False) + self._create_monitor_environment(monitor, name="dev", is_muted=True) + + # When viewing all environments, monitor should NOT be muted + # (because not ALL environments are muted) + response = self.get_success_response(self.organization.slug) + assert response.data[0]["isMuted"] is False + + # When filtering to only "prod" environment, monitor should STILL not be muted + # even though we're only displaying prod in the environments array + response = self.get_success_response(self.organization.slug, environment=["prod"]) + assert response.data[0]["isMuted"] is False + assert len(response.data[0]["environments"]) == 1 + assert response.data[0]["environments"][0]["name"] == "prod" + + # When filtering to only "dev" environment (which is muted), + # monitor should STILL not be muted because prod is unmuted + response = self.get_success_response(self.organization.slug, environment=["dev"]) + assert response.data[0]["isMuted"] is False + assert len(response.data[0]["environments"]) == 1 + assert response.data[0]["environments"][0]["name"] == "dev" + + # Now mute ALL environments + monitor.monitorenvironment_set.update(is_muted=True) + + # Monitor should now be muted regardless of environment filter + response = self.get_success_response(self.organization.slug) + assert response.data[0]["isMuted"] is True + + response = self.get_success_response(self.organization.slug, environment=["prod"]) + assert response.data[0]["isMuted"] is True + + response = self.get_success_response(self.organization.slug, environment=["dev"]) + assert response.data[0]["isMuted"] is True + class CreateOrganizationMonitorTest(MonitorTestCase): endpoint = "sentry-api-0-organization-monitor-index" @@ -655,8 +694,8 @@ def test_bulk_mute_unmute(self) -> None: monitor_two.refresh_from_db() env_one.refresh_from_db() env_two.refresh_from_db() - assert monitor_one.is_muted - assert monitor_two.is_muted + assert is_monitor_muted(monitor_one) + assert is_monitor_muted(monitor_two) assert env_one.is_muted assert env_two.is_muted assert_org_audit_log_exists( @@ -681,8 +720,8 @@ def test_bulk_mute_unmute(self) -> None: monitor_two.refresh_from_db() env_one.refresh_from_db() env_two.refresh_from_db() - assert not monitor_one.is_muted - assert not monitor_two.is_muted + assert not is_monitor_muted(monitor_one) + assert not is_monitor_muted(monitor_two) assert not env_one.is_muted assert not env_two.is_muted @@ -763,5 +802,5 @@ def test_disallow_when_no_open_membership(self) -> None: monitor.refresh_from_db() env.refresh_from_db() - assert not monitor.is_muted + assert not is_monitor_muted(monitor) assert not env.is_muted diff --git a/tests/sentry/monitors/logic/test_mark_failed.py b/tests/sentry/monitors/logic/test_mark_failed.py index de16c502d2136f..436c85a2daa384 100644 --- a/tests/sentry/monitors/logic/test_mark_failed.py +++ b/tests/sentry/monitors/logic/test_mark_failed.py @@ -16,6 +16,7 @@ MonitorIncident, MonitorStatus, ScheduleType, + is_monitor_muted, ) from sentry.testutils.cases import TestCase @@ -100,7 +101,7 @@ def test_mark_failed_muted(self, mock_dispatch_incident_occurrence: mock.MagicMo monitor.refresh_from_db() monitor_environment.refresh_from_db() - assert monitor.is_muted + assert is_monitor_muted(monitor) assert monitor_environment.status == MonitorStatus.ERROR assert mock_dispatch_incident_occurrence.call_count == 0 @@ -342,7 +343,7 @@ def test_mark_failed_issue_threshold_disabled( monitor.refresh_from_db() monitor_environment.refresh_from_db() - assert monitor.is_muted + assert is_monitor_muted(monitor) assert monitor_environment.status == MonitorStatus.ERROR assert mock_dispatch_incident_occurrence.call_count == 0 diff --git a/tests/sentry/monitors/test_models.py b/tests/sentry/monitors/test_models.py index f22ba43b5a6895..4e06985b2ad785 100644 --- a/tests/sentry/monitors/test_models.py +++ b/tests/sentry/monitors/test_models.py @@ -13,6 +13,7 @@ MonitorEnvironmentLimitsExceeded, MonitorLimitsExceeded, ScheduleType, + is_monitor_muted, ) from sentry.monitors.types import DATA_SOURCE_CRON_MONITOR from sentry.monitors.validators import ConfigValidator @@ -454,7 +455,7 @@ def test_is_muted_all_environments_muted(self): ) # Verify monitor.is_muted is True - assert monitor.is_muted is True + assert is_monitor_muted(monitor) is True def test_is_muted_some_environments_unmuted(self): """Test that monitor.is_muted returns False when any environment is unmuted.""" @@ -475,7 +476,7 @@ def test_is_muted_some_environments_unmuted(self): ) # Verify monitor.is_muted is False - assert monitor.is_muted is False + assert is_monitor_muted(monitor) is False def test_is_muted_all_environments_unmuted(self): """Test that monitor.is_muted returns False when all environments are unmuted.""" @@ -496,12 +497,12 @@ def test_is_muted_all_environments_unmuted(self): ) # Verify monitor.is_muted is False - assert monitor.is_muted is False + assert is_monitor_muted(monitor) is False def test_is_muted_no_environments(self): """Test that monitor.is_muted returns False when there are no environments.""" monitor = self.create_monitor() - assert monitor.is_muted is False + assert is_monitor_muted(monitor) is False def test_is_muted_single_environment(self): """Test is_muted works correctly with a single environment.""" @@ -514,7 +515,7 @@ def test_is_muted_single_environment(self): is_muted=True, ) - assert monitor.is_muted is True + assert is_monitor_muted(monitor) is True # Test with unmuted environment monitor2 = self.create_monitor() @@ -525,4 +526,4 @@ def test_is_muted_single_environment(self): is_muted=False, ) - assert monitor2.is_muted is False + assert is_monitor_muted(monitor2) is False diff --git a/tests/sentry/monitors/test_validators.py b/tests/sentry/monitors/test_validators.py index e6d776f1d08e2b..b33ac1cac66547 100644 --- a/tests/sentry/monitors/test_validators.py +++ b/tests/sentry/monitors/test_validators.py @@ -11,7 +11,13 @@ from sentry.analytics.events.cron_monitor_created import CronMonitorCreated, FirstCronMonitorCreated from sentry.constants import DataCategory, ObjectStatus from sentry.models.rule import Rule, RuleSource -from sentry.monitors.models import Monitor, MonitorStatus, ScheduleType, get_cron_monitor +from sentry.monitors.models import ( + Monitor, + MonitorStatus, + ScheduleType, + get_cron_monitor, + is_monitor_muted, +) from sentry.monitors.types import DATA_SOURCE_CRON_MONITOR from sentry.monitors.validators import ( MonitorDataSourceValidator, @@ -326,7 +332,7 @@ def test_create_with_is_muted_noop(self): monitor = validator.save() # Monitor has no environments, so is_muted returns False regardless of input - assert monitor.is_muted is False + assert is_monitor_muted(monitor) is False class MonitorValidatorUpdateTest(MonitorTestCase): @@ -536,7 +542,7 @@ def test_update_is_muted(self): assert validator.is_valid() updated_monitor = validator.save() - assert updated_monitor.is_muted is True + assert is_monitor_muted(updated_monitor) is True # Verify the environment was also muted env.refresh_from_db() @@ -570,7 +576,7 @@ def test_update_is_muted_propagates_to_environments(self): ) assert validator.is_valid() updated_monitor = validator.save() - assert updated_monitor.is_muted is True + assert is_monitor_muted(updated_monitor) is True # Verify both environments are now muted env1.refresh_from_db() @@ -594,7 +600,7 @@ def test_update_is_muted_false_propagates_to_environments(self): ) # Verify monitor is muted (all environments are muted) - assert self.monitor.is_muted is True + assert is_monitor_muted(self.monitor) is True # Unmute the monitor via validator validator = MonitorValidator( @@ -609,7 +615,7 @@ def test_update_is_muted_false_propagates_to_environments(self): ) assert validator.is_valid() updated_monitor = validator.save() - assert updated_monitor.is_muted is False + assert is_monitor_muted(updated_monitor) is False # Verify both environments are now unmuted env1.refresh_from_db() @@ -708,7 +714,7 @@ def test_update_multiple_fields(self): updated_monitor = validator.save() assert updated_monitor.name == "New Name" assert updated_monitor.slug == "new-slug" - assert updated_monitor.is_muted is False + assert is_monitor_muted(updated_monitor) is False assert updated_monitor.owner_team_id == self.team.id def test_update_slug_already_exists(self): @@ -1064,7 +1070,7 @@ def _assert_monitor_attributes(self, monitor, name, slug, schedule, status=Objec assert monitor.project_id == self.project.id assert monitor.config["schedule"] == schedule assert monitor.status == status - assert monitor.is_muted is False + assert is_monitor_muted(monitor) is False assert monitor.owner_user_id is None assert monitor.owner_team_id is None