From 8666bf0fde2dab298feb232c828954136d28e4e0 Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Wed, 11 Jun 2025 15:38:48 -0700 Subject: [PATCH 1/6] add a registry to the activity model to allow for hooks when the activity is created. --- src/sentry/models/activity.py | 11 ++++++++++- tests/sentry/models/test_activity.py | 23 +++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/sentry/models/activity.py b/src/sentry/models/activity.py index 363c6cd7dd7f1a..a3a56fc9a051d4 100644 --- a/src/sentry/models/activity.py +++ b/src/sentry/models/activity.py @@ -1,7 +1,7 @@ from __future__ import annotations import logging -from collections.abc import Mapping, Sequence +from collections.abc import Callable, Mapping, Sequence from enum import Enum from typing import TYPE_CHECKING, Any, ClassVar @@ -27,6 +27,7 @@ from sentry.tasks import activity from sentry.types.activity import CHOICES, STATUS_CHANGE_ACTIVITY_TYPES, ActivityType from sentry.types.group import PriorityLevel +from sentry.utils.registry import Registry if TYPE_CHECKING: from sentry.models.group import Group @@ -98,9 +99,13 @@ def create_group_activity( if user_id is not None: activity_args["user_id"] = user_id activity = self.create(**activity_args) + if send_notification: activity.send_notification() + for handler in activity_creation_registry.registrations.values(): + handler(activity) + return activity @@ -218,3 +223,7 @@ class ActivityIntegration(Enum): MSTEAMS = "msteams" DISCORD = "discord" SUSPECT_COMMITTER = "suspectCommitter" + + +ActivityCreationHandler = Callable[[Activity], None] +activity_creation_registry = Registry[ActivityCreationHandler](enable_reverse_lookup=False) diff --git a/tests/sentry/models/test_activity.py b/tests/sentry/models/test_activity.py index 61527f1fb618d6..0315ed3e3ff5ad 100644 --- a/tests/sentry/models/test_activity.py +++ b/tests/sentry/models/test_activity.py @@ -1,4 +1,5 @@ import logging +from unittest import mock from unittest.mock import MagicMock, patch from sentry.event_manager import EventManager @@ -344,3 +345,25 @@ def test_skips_status_change_notifications_if_disabled( ) mock_send_activity_notifications.assert_not_called() + + +class TestActivityCreationHandlers(TestCase): + def test_create_group_activity(self): + project = self.create_project(name="test_create_group_activity") + group = self.create_group(project) + user = self.create_user() + + with mock.patch("sentry.models.activity.activity_creation_registry") as mock_registry: + mock_handler = mock.Mock() + mock_registry.registrations = {"test_handler": mock_handler} + + activity = Activity.objects.create_group_activity( + group=group, + type=ActivityType.SET_UNRESOLVED, + user=user, + data=None, + send_notification=False, + ) + + assert mock_handler.called + assert mock_handler.call_args[0][0] == activity From 030acf265011e4a3cc0560747244a104c32c5c06 Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Fri, 13 Jun 2025 12:21:50 -0700 Subject: [PATCH 2/6] Add the detector id to the status change message, so we can track what created the change --- src/sentry/issues/status_change_consumer.py | 4 ++++ src/sentry/issues/status_change_message.py | 3 +++ src/sentry/models/activity.py | 6 ++++-- src/sentry/models/group.py | 3 +++ src/sentry/workflow_engine/handlers/detector/stateful.py | 1 + .../workflow_engine/handlers/detector/test_stateful.py | 1 + 6 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/sentry/issues/status_change_consumer.py b/src/sentry/issues/status_change_consumer.py index c3453161f077fc..0afedd0fbfc01c 100644 --- a/src/sentry/issues/status_change_consumer.py +++ b/src/sentry/issues/status_change_consumer.py @@ -39,6 +39,7 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: "fingerprint": status_change["fingerprint"], "new_status": new_status, "new_substatus": new_substatus, + "detector_id": status_change.get("detector_id", None), } # Validate the provided status and substatus - we only allow setting a substatus for unresolved or ignored groups. @@ -63,6 +64,7 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: status=new_status, substatus=new_substatus, activity_type=ActivityType.SET_RESOLVED, + detector_id=status_change.get("detector_id", None), ) remove_group_from_inbox(group, action=GroupInboxRemoveAction.RESOLVED) kick_off_status_syncs.apply_async( @@ -85,6 +87,7 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: status=new_status, substatus=new_substatus, activity_type=ActivityType.SET_IGNORED, + detector_id=status_change.get("detector_id", None), ) remove_group_from_inbox(group, action=GroupInboxRemoveAction.IGNORED) kick_off_status_syncs.apply_async( @@ -122,6 +125,7 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: substatus=new_substatus, activity_type=activity_type, from_substatus=group.substatus, + detector_id=status_change.get("detector_id", None), ) add_group_to_inbox(group, group_inbox_reason) kick_off_status_syncs.apply_async( diff --git a/src/sentry/issues/status_change_message.py b/src/sentry/issues/status_change_message.py index bc93044a252efa..439067c05c9239 100644 --- a/src/sentry/issues/status_change_message.py +++ b/src/sentry/issues/status_change_message.py @@ -12,6 +12,7 @@ class StatusChangeMessageData(TypedDict): new_status: int new_substatus: int | None id: str + detector_id: int | None @dataclass(frozen=True) @@ -20,6 +21,7 @@ class StatusChangeMessage: project_id: int new_status: int new_substatus: int | None + detector_id: int | None = None id: str = field(default_factory=lambda: uuid4().hex) def to_dict( @@ -30,5 +32,6 @@ def to_dict( "project_id": self.project_id, "new_status": self.new_status, "new_substatus": self.new_substatus, + "detector_id": self.detector_id, "id": self.id, } diff --git a/src/sentry/models/activity.py b/src/sentry/models/activity.py index a3a56fc9a051d4..64c77e14d94e8e 100644 --- a/src/sentry/models/activity.py +++ b/src/sentry/models/activity.py @@ -36,6 +36,7 @@ _default_logger = logging.getLogger(__name__) +DetectorId = int | None class ActivityManager(BaseManager["Activity"]): @@ -87,6 +88,7 @@ def create_group_activity( user_id: int | None = None, data: Mapping[str, Any] | None = None, send_notification: bool = True, + detector_id: DetectorId = None, ) -> Activity: if user: user_id = user.id @@ -104,7 +106,7 @@ def create_group_activity( activity.send_notification() for handler in activity_creation_registry.registrations.values(): - handler(activity) + handler(activity, detector_id) return activity @@ -225,5 +227,5 @@ class ActivityIntegration(Enum): SUSPECT_COMMITTER = "suspectCommitter" -ActivityCreationHandler = Callable[[Activity], None] +ActivityCreationHandler = Callable[[Activity, DetectorId], None] activity_creation_registry = Registry[ActivityCreationHandler](enable_reverse_lookup=False) diff --git a/src/sentry/models/group.py b/src/sentry/models/group.py index 5dfac6127c2ea7..48288b1650025b 100644 --- a/src/sentry/models/group.py +++ b/src/sentry/models/group.py @@ -442,6 +442,7 @@ def update_group_status( activity_data: Mapping[str, Any] | None = None, send_activity_notification: bool = True, from_substatus: int | None = None, + detector_id: int | None = None, ) -> None: """For each groups, update status to `status` and create an Activity.""" from sentry.models.activity import Activity @@ -486,6 +487,7 @@ def update_group_status( activity_type, data=activity_data, send_notification=send_activity_notification, + detector_id=detector_id, ) record_group_history_from_activity_type(group, activity_type.value) @@ -498,6 +500,7 @@ def update_group_status( "priority": new_priority.to_str(), "reason": PriorityChangeReason.ONGOING, }, + detector_id=detector_id, ) record_group_history(group, PRIORITY_TO_GROUP_HISTORY_STATUS[new_priority]) diff --git a/src/sentry/workflow_engine/handlers/detector/stateful.py b/src/sentry/workflow_engine/handlers/detector/stateful.py index 80273b8c237004..c4b8f43462eee3 100644 --- a/src/sentry/workflow_engine/handlers/detector/stateful.py +++ b/src/sentry/workflow_engine/handlers/detector/stateful.py @@ -365,6 +365,7 @@ def _create_resolve_message(self, group_key: DetectorGroupKey = None) -> StatusC project_id=self.detector.project_id, new_status=GroupStatus.RESOLVED, new_substatus=None, + detector_id=self.detector.id, ) def _extract_value_from_packet( diff --git a/tests/sentry/workflow_engine/handlers/detector/test_stateful.py b/tests/sentry/workflow_engine/handlers/detector/test_stateful.py index 108fe37113fd29..76a1c376a6681f 100644 --- a/tests/sentry/workflow_engine/handlers/detector/test_stateful.py +++ b/tests/sentry/workflow_engine/handlers/detector/test_stateful.py @@ -218,6 +218,7 @@ def test_evaluate__resolve(self): assert evaluation_result assert evaluation_result.priority == DetectorPriorityLevel.OK assert isinstance(evaluation_result.result, StatusChangeMessage) + assert evaluation_result.result.detector_id == self.detector.id def test_evaluate__resolve__detector_state(self): self.handler.evaluate(self.data_packet) From e590fa3536ae0b556f614ba6e39b96374459af6d Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Fri, 13 Jun 2025 13:01:46 -0700 Subject: [PATCH 3/6] fix type issue wehre detector_id wasn't set creating a statuschangemessagedata diretly --- tests/sentry/issues/test_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/sentry/issues/test_utils.py b/tests/sentry/issues/test_utils.py index 92d7bf8520dbb4..7c73e86e37863b 100644 --- a/tests/sentry/issues/test_utils.py +++ b/tests/sentry/issues/test_utils.py @@ -94,6 +94,7 @@ def build_statuschange_data(self, **overrides: Any) -> StatusChangeMessageData: "fingerprint": ["some-fingerprint"], "new_status": 1, "new_substatus": 1, + "detector_id": None, } kwargs.update(overrides) # type: ignore[typeddict-item] From 167e0567839c2e7cd1f533aaae40d58eba76149f Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Fri, 13 Jun 2025 14:13:56 -0700 Subject: [PATCH 4/6] rather than proxy the detector_id so much, make the hook more natural in update_status. if there's a new activity, get the latest event and trigger the registry --- src/sentry/issues/status_change_consumer.py | 31 ++++- src/sentry/models/activity.py | 12 +- src/sentry/models/group.py | 3 - .../issues/test_status_change_consumer.py | 128 +++++++++++++++++- tests/sentry/models/test_activity.py | 23 ---- 5 files changed, 148 insertions(+), 49 deletions(-) diff --git a/src/sentry/issues/status_change_consumer.py b/src/sentry/issues/status_change_consumer.py index 0afedd0fbfc01c..51632ed8bfc78a 100644 --- a/src/sentry/issues/status_change_consumer.py +++ b/src/sentry/issues/status_change_consumer.py @@ -2,7 +2,7 @@ import logging from collections import defaultdict -from collections.abc import Iterable, Mapping, Sequence +from collections.abc import Callable, Iterable, Mapping, Sequence from typing import Any from sentry_sdk.tracing import NoOpSpan, Span, Transaction @@ -10,6 +10,7 @@ from sentry.integrations.tasks.kick_off_status_syncs import kick_off_status_syncs from sentry.issues.escalating.escalating import manage_issue_states from sentry.issues.status_change_message import StatusChangeMessageData +from sentry.models.activity import Activity from sentry.models.group import Group, GroupStatus from sentry.models.grouphash import GroupHash from sentry.models.groupinbox import ( @@ -23,6 +24,7 @@ from sentry.types.activity import ActivityType from sentry.types.group import IGNORED_SUBSTATUS_CHOICES, GroupSubStatus from sentry.utils import metrics +from sentry.utils.registry import Registry logger = logging.getLogger(__name__) @@ -39,7 +41,6 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: "fingerprint": status_change["fingerprint"], "new_status": new_status, "new_substatus": new_substatus, - "detector_id": status_change.get("detector_id", None), } # Validate the provided status and substatus - we only allow setting a substatus for unresolved or ignored groups. @@ -59,12 +60,12 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: return if new_status == GroupStatus.RESOLVED: + activity_type = ActivityType.SET_RESOLVED Group.objects.update_group_status( groups=[group], status=new_status, substatus=new_substatus, - activity_type=ActivityType.SET_RESOLVED, - detector_id=status_change.get("detector_id", None), + activity_type=activity_type, ) remove_group_from_inbox(group, action=GroupInboxRemoveAction.RESOLVED) kick_off_status_syncs.apply_async( @@ -82,12 +83,12 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: ) return + activity_type = ActivityType.SET_IGNORED Group.objects.update_group_status( groups=[group], status=new_status, substatus=new_substatus, - activity_type=ActivityType.SET_IGNORED, - detector_id=status_change.get("detector_id", None), + activity_type=activity_type, ) remove_group_from_inbox(group, action=GroupInboxRemoveAction.IGNORED) kick_off_status_syncs.apply_async( @@ -125,7 +126,6 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: substatus=new_substatus, activity_type=activity_type, from_substatus=group.substatus, - detector_id=status_change.get("detector_id", None), ) add_group_to_inbox(group, group_inbox_reason) kick_off_status_syncs.apply_async( @@ -140,6 +140,19 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: f"Unsupported status: {status_change['new_status']} {status_change['new_substatus']}" ) + if activity_type is not None: + """ + If we have set created an activity, then we'll also notify any registered handlers + that the group status has changed. + + This is used to trigger the `workflow_engine` processing status changes + """ + latest_activity = Activity.objects.filter( + group_id=group.id, type=activity_type.value + ).order_by("-datetime") + for handler in group_status_update_registry.registrations.values(): + handler(group, status_change, latest_activity[0]) + def get_group_from_fingerprint(project_id: int, fingerprint: Sequence[str]) -> Group | None: results = bulk_get_groups_from_fingerprints([(project_id, fingerprint)]) @@ -259,3 +272,7 @@ def process_status_change_message( update_status(group, status_change_data) return group + + +GroupUpdateHandler = Callable[[Group, StatusChangeMessageData, Activity], None] +group_status_update_registry = Registry[GroupUpdateHandler](enable_reverse_lookup=False) diff --git a/src/sentry/models/activity.py b/src/sentry/models/activity.py index 64c77e14d94e8e..a66378b44139c4 100644 --- a/src/sentry/models/activity.py +++ b/src/sentry/models/activity.py @@ -1,7 +1,7 @@ from __future__ import annotations import logging -from collections.abc import Callable, Mapping, Sequence +from collections.abc import Mapping, Sequence from enum import Enum from typing import TYPE_CHECKING, Any, ClassVar @@ -27,7 +27,6 @@ from sentry.tasks import activity from sentry.types.activity import CHOICES, STATUS_CHANGE_ACTIVITY_TYPES, ActivityType from sentry.types.group import PriorityLevel -from sentry.utils.registry import Registry if TYPE_CHECKING: from sentry.models.group import Group @@ -36,7 +35,6 @@ _default_logger = logging.getLogger(__name__) -DetectorId = int | None class ActivityManager(BaseManager["Activity"]): @@ -88,7 +86,6 @@ def create_group_activity( user_id: int | None = None, data: Mapping[str, Any] | None = None, send_notification: bool = True, - detector_id: DetectorId = None, ) -> Activity: if user: user_id = user.id @@ -105,9 +102,6 @@ def create_group_activity( if send_notification: activity.send_notification() - for handler in activity_creation_registry.registrations.values(): - handler(activity, detector_id) - return activity @@ -225,7 +219,3 @@ class ActivityIntegration(Enum): MSTEAMS = "msteams" DISCORD = "discord" SUSPECT_COMMITTER = "suspectCommitter" - - -ActivityCreationHandler = Callable[[Activity, DetectorId], None] -activity_creation_registry = Registry[ActivityCreationHandler](enable_reverse_lookup=False) diff --git a/src/sentry/models/group.py b/src/sentry/models/group.py index 48288b1650025b..5dfac6127c2ea7 100644 --- a/src/sentry/models/group.py +++ b/src/sentry/models/group.py @@ -442,7 +442,6 @@ def update_group_status( activity_data: Mapping[str, Any] | None = None, send_activity_notification: bool = True, from_substatus: int | None = None, - detector_id: int | None = None, ) -> None: """For each groups, update status to `status` and create an Activity.""" from sentry.models.activity import Activity @@ -487,7 +486,6 @@ def update_group_status( activity_type, data=activity_data, send_notification=send_activity_notification, - detector_id=detector_id, ) record_group_history_from_activity_type(group, activity_type.value) @@ -500,7 +498,6 @@ def update_group_status( "priority": new_priority.to_str(), "reason": PriorityChangeReason.ONGOING, }, - detector_id=detector_id, ) record_group_history(group, PRIORITY_TO_GROUP_HISTORY_STATUS[new_priority]) diff --git a/tests/sentry/issues/test_status_change_consumer.py b/tests/sentry/issues/test_status_change_consumer.py index 494077d4622ad5..65fe0df2255744 100644 --- a/tests/sentry/issues/test_status_change_consumer.py +++ b/tests/sentry/issues/test_status_change_consumer.py @@ -4,7 +4,8 @@ from unittest.mock import MagicMock, patch from sentry.issues.occurrence_consumer import _process_message -from sentry.issues.status_change_consumer import bulk_get_groups_from_fingerprints +from sentry.issues.status_change_consumer import bulk_get_groups_from_fingerprints, update_status +from sentry.issues.status_change_message import StatusChangeMessageData from sentry.models.activity import Activity from sentry.models.group import Group, GroupStatus from sentry.models.grouphistory import GroupHistory, GroupHistoryStatus @@ -20,6 +21,7 @@ def get_test_message_status_change( fingerprint: list[str] | None = None, new_status: int = GroupStatus.RESOLVED, new_substatus: int | None = None, + detector_id: int | None = None, ) -> dict[str, Any]: payload = { "project_id": project_id, @@ -27,6 +29,7 @@ def get_test_message_status_change( "new_status": new_status, "new_substatus": new_substatus, "payload_type": "status_change", + "detector_id": detector_id, } return payload @@ -170,10 +173,9 @@ def test_valid_payload_auto_ongoing(self) -> None: self.group.save() message = get_test_message_status_change( - self.project.id, - fingerprint=self.fingerprint, - new_status=GroupStatus.UNRESOLVED, - new_substatus=GroupSubStatus.ONGOING, + project_id=self.project.id, + fingerprint="test-fingerprint", + new_status=GroupStatus.RESOLVED, ) result = _process_message(message) assert result is not None @@ -345,3 +347,119 @@ def test_bulk_get_single_project_multiple_hash(self) -> None: tuple([*other_occurrence.fingerprint, *self.occurrence.fingerprint]), ): other_group } + + +class TestStatusChangeRegistry(IssueOccurrenceTestBase): + def setUp(self) -> None: + super().setUp() + self.detector = self.create_detector() + self.group = self.create_group( + project=self.project, + status=GroupStatus.UNRESOLVED, + substatus=GroupSubStatus.ESCALATING, + ) + + status_change = get_test_message_status_change( + self.project.id, + new_status=GroupStatus.RESOLVED, + detector_id=self.detector.id, + ) + + self.message = StatusChangeMessageData( + id="test-id", + project_id=status_change["project_id"], + fingerprint=status_change["fingerprint"], + new_status=status_change["new_status"], + new_substatus=status_change.get("new_substatus"), + detector_id=status_change.get("detector_id"), + ) + + def get_latest_activity(self, activity_type: ActivityType) -> Activity: + latest_activity = ( + Activity.objects.filter(group_id=self.group.id, type=activity_type.value) + .order_by("-datetime") + .first() + ) + + if latest_activity is None: + raise AssertionError(f"No activity found for type {activity_type}") + + return latest_activity + + def test_handler_is_called__resolved(self) -> None: + with patch( + "sentry.issues.status_change_consumer.group_status_update_registry", + ) as mock_registry: + mock_handler = MagicMock() + mock_registry.registrations = { + "test_status_change": mock_handler, + } + + update_status(self.group, self.message) + latest_activity = self.get_latest_activity(ActivityType.SET_RESOLVED) + + mock_handler.assert_called_once_with(self.group, self.message, latest_activity) + + def test_handler_is_not_called__unresolved_escalating(self) -> None: + # There will be an issue occurrence that triggers this instead + + self.message["new_status"] = GroupStatus.UNRESOLVED + self.message["new_substatus"] = GroupSubStatus.ESCALATING + with patch( + "sentry.issues.status_change_consumer.group_status_update_registry", + ) as mock_registry: + mock_handler = MagicMock() + mock_registry.registrations = { + "test_status_change": mock_handler, + } + + update_status(self.group, self.message) + assert mock_handler.call_count == 0 + + def test_handler_is_called_unresolved_ongoing(self) -> None: + self.message["new_status"] = GroupStatus.UNRESOLVED + self.message["new_substatus"] = GroupSubStatus.ONGOING + + with patch( + "sentry.issues.status_change_consumer.group_status_update_registry", + ) as mock_registry: + mock_handler = MagicMock() + mock_registry.registrations = { + "test_status_change": mock_handler, + } + + update_status(self.group, self.message) + latest_activity = self.get_latest_activity(ActivityType.AUTO_SET_ONGOING) + mock_handler.assert_called_once_with(self.group, self.message, latest_activity) + + def test_handler_is_called__unresolved_regressed(self) -> None: + self.message["new_status"] = GroupStatus.UNRESOLVED + self.message["new_substatus"] = GroupSubStatus.REGRESSED + + with patch( + "sentry.issues.status_change_consumer.group_status_update_registry", + ) as mock_registry: + mock_handler = MagicMock() + mock_registry.registrations = { + "test_status_change": mock_handler, + } + + update_status(self.group, self.message) + latest_activity = self.get_latest_activity(ActivityType.SET_REGRESSION) + mock_handler.assert_called_once_with(self.group, self.message, latest_activity) + + def test_handler_is_called__ignored(self) -> None: + self.message["new_status"] = GroupStatus.IGNORED + self.message["new_substatus"] = GroupSubStatus.FOREVER + + with patch( + "sentry.issues.status_change_consumer.group_status_update_registry", + ) as mock_registry: + mock_handler = MagicMock() + mock_registry.registrations = { + "test_status_change": mock_handler, + } + + update_status(self.group, self.message) + latest_activity = self.get_latest_activity(ActivityType.SET_IGNORED) + mock_handler.assert_called_once_with(self.group, self.message, latest_activity) diff --git a/tests/sentry/models/test_activity.py b/tests/sentry/models/test_activity.py index 0315ed3e3ff5ad..61527f1fb618d6 100644 --- a/tests/sentry/models/test_activity.py +++ b/tests/sentry/models/test_activity.py @@ -1,5 +1,4 @@ import logging -from unittest import mock from unittest.mock import MagicMock, patch from sentry.event_manager import EventManager @@ -345,25 +344,3 @@ def test_skips_status_change_notifications_if_disabled( ) mock_send_activity_notifications.assert_not_called() - - -class TestActivityCreationHandlers(TestCase): - def test_create_group_activity(self): - project = self.create_project(name="test_create_group_activity") - group = self.create_group(project) - user = self.create_user() - - with mock.patch("sentry.models.activity.activity_creation_registry") as mock_registry: - mock_handler = mock.Mock() - mock_registry.registrations = {"test_handler": mock_handler} - - activity = Activity.objects.create_group_activity( - group=group, - type=ActivityType.SET_UNRESOLVED, - user=user, - data=None, - send_notification=False, - ) - - assert mock_handler.called - assert mock_handler.call_args[0][0] == activity From 0915b347edddebad17b95ae7f613220117c5efab Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Fri, 13 Jun 2025 14:15:48 -0700 Subject: [PATCH 5/6] prefer the positive check for activity_Type so it wil ensure .value is set as well --- src/sentry/issues/status_change_consumer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/issues/status_change_consumer.py b/src/sentry/issues/status_change_consumer.py index 51632ed8bfc78a..2eba0b916e4ce7 100644 --- a/src/sentry/issues/status_change_consumer.py +++ b/src/sentry/issues/status_change_consumer.py @@ -140,7 +140,7 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: f"Unsupported status: {status_change['new_status']} {status_change['new_substatus']}" ) - if activity_type is not None: + if isinstance(activity_type, ActivityType): """ If we have set created an activity, then we'll also notify any registered handlers that the group status has changed. From 1b03e2e0a71a746ba588af68d6a1e1c41bf9ee24 Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Fri, 13 Jun 2025 14:27:19 -0700 Subject: [PATCH 6/6] Whoops, fix a thing i didn't mean to change. add types / sentinel value for 'activity_type' --- src/sentry/issues/status_change_consumer.py | 5 +++-- tests/sentry/issues/test_status_change_consumer.py | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/sentry/issues/status_change_consumer.py b/src/sentry/issues/status_change_consumer.py index 2eba0b916e4ce7..c2287fc8edc069 100644 --- a/src/sentry/issues/status_change_consumer.py +++ b/src/sentry/issues/status_change_consumer.py @@ -30,6 +30,7 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: + activity_type: ActivityType | None = None new_status = status_change["new_status"] new_substatus = status_change["new_substatus"] @@ -140,12 +141,12 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None: f"Unsupported status: {status_change['new_status']} {status_change['new_substatus']}" ) - if isinstance(activity_type, ActivityType): + if activity_type is not None: """ If we have set created an activity, then we'll also notify any registered handlers that the group status has changed. - This is used to trigger the `workflow_engine` processing status changes + This is used to trigger the `workflow_engine` processing status changes. """ latest_activity = Activity.objects.filter( group_id=group.id, type=activity_type.value diff --git a/tests/sentry/issues/test_status_change_consumer.py b/tests/sentry/issues/test_status_change_consumer.py index 65fe0df2255744..f01920d1fcd698 100644 --- a/tests/sentry/issues/test_status_change_consumer.py +++ b/tests/sentry/issues/test_status_change_consumer.py @@ -173,9 +173,10 @@ def test_valid_payload_auto_ongoing(self) -> None: self.group.save() message = get_test_message_status_change( - project_id=self.project.id, - fingerprint="test-fingerprint", - new_status=GroupStatus.RESOLVED, + self.project.id, + fingerprint=self.fingerprint, + new_status=GroupStatus.UNRESOLVED, + new_substatus=GroupSubStatus.ONGOING, ) result = _process_message(message) assert result is not None