diff --git a/src/sentry/issues/issue_occurrence.py b/src/sentry/issues/issue_occurrence.py index 02c09800d1a8bb..97fbe57e764958 100644 --- a/src/sentry/issues/issue_occurrence.py +++ b/src/sentry/issues/issue_occurrence.py @@ -1,6 +1,7 @@ from __future__ import annotations import hashlib +import logging from collections.abc import Mapping, Sequence from dataclasses import dataclass from datetime import datetime @@ -10,6 +11,7 @@ from sentry import nodestore from sentry.issues.grouptype import GroupType, get_group_type_by_type_id +from sentry.utils.actor import ActorTuple from sentry.utils.dates import parse_timestamp DEFAULT_LEVEL = "info" @@ -36,6 +38,11 @@ class IssueOccurrenceData(TypedDict): level: str | None culprit: str | None initial_issue_priority: NotRequired[int | None] + assignee: NotRequired[str | None] + """ + Who to assign the issue to when creating a new issue. Has no effect on existing issues. + In the format of an Actor identifier, as defined in `ActorTuple.from_actor_identifier` + """ @dataclass(frozen=True) @@ -88,6 +95,7 @@ class IssueOccurrence: level: str culprit: str initial_issue_priority: int | None = None + assignee: ActorTuple | None = None def __post_init__(self) -> None: if not is_aware(self.detection_time): @@ -111,10 +119,13 @@ def to_dict( "level": self.level, "culprit": self.culprit, "initial_issue_priority": self.initial_issue_priority, + "assignee": self.assignee.identifier if self.assignee else None, } @classmethod def from_dict(cls, data: IssueOccurrenceData) -> IssueOccurrence: + from sentry.api.serializers.rest_framework import ValidationError + # Backwards compatibility - we used to not require this field, so set a default when `None` level = data.get("level") if not level: @@ -122,6 +133,18 @@ def from_dict(cls, data: IssueOccurrenceData) -> IssueOccurrence: culprit = data.get("culprit") if not culprit: culprit = "" + + assignee = None + try: + # Note that this can cause IO, but in practice this will happen only the first time that + # the occurrence is sent to the issue platform. We then translate to the id and store + # that, so subsequent fetches won't cause IO. + assignee = ActorTuple.from_actor_identifier(data.get("assignee")) + except ValidationError: + logging.exception("Failed to parse assignee actor identifier") + except Exception: + # We never want this to cause parsing an occurrence to fail + logging.exception("Unexpected error parsing assignee") return cls( data["id"], data["project_id"], @@ -141,6 +164,7 @@ def from_dict(cls, data: IssueOccurrenceData) -> IssueOccurrence: level, culprit, data.get("initial_issue_priority"), + assignee, ) @property diff --git a/src/sentry/issues/occurrence_consumer.py b/src/sentry/issues/occurrence_consumer.py index d4cae08db1e049..9a96aa0d05ec48 100644 --- a/src/sentry/issues/occurrence_consumer.py +++ b/src/sentry/issues/occurrence_consumer.py @@ -154,6 +154,7 @@ def _get_kwargs(payload: Mapping[str, Any]) -> Mapping[str, Any]: "type": payload["type"], "detection_time": payload["detection_time"], "level": payload.get("level", DEFAULT_LEVEL), + "assignee": payload.get("assignee"), } process_occurrence_data(occurrence_data) diff --git a/src/sentry/utils/actor.py b/src/sentry/utils/actor.py index 96fdb5d27f74df..2ed20142064dfc 100644 --- a/src/sentry/utils/actor.py +++ b/src/sentry/utils/actor.py @@ -47,7 +47,7 @@ def from_actor_identifier(cls, actor_identifier: int | str | None) -> ActorTuple "maisey@dogsrule.com" -> look up User by primary email """ - if actor_identifier is None: + if not actor_identifier: return None # If we have an integer, fall back to assuming it's a User diff --git a/tests/sentry/issues/test_issue_occurrence.py b/tests/sentry/issues/test_issue_occurrence.py index f1670e17b4f680..468fef018c96dd 100644 --- a/tests/sentry/issues/test_issue_occurrence.py +++ b/tests/sentry/issues/test_issue_occurrence.py @@ -1,5 +1,8 @@ from sentry.issues.issue_occurrence import DEFAULT_LEVEL, IssueEvidence, IssueOccurrence +from sentry.models.team import Team +from sentry.models.user import User from sentry.testutils.cases import TestCase +from sentry.utils.actor import ActorTuple from tests.sentry.issues.test_utils import OccurrenceTestMixin @@ -16,6 +19,35 @@ def test_level_default(self) -> None: occurrence = IssueOccurrence.from_dict(occurrence_data) assert occurrence.level == DEFAULT_LEVEL + def test_assignee(self) -> None: + occurrence_data = self.build_occurrence_data() + occurrence_data["assignee"] = f"user:{self.user.id}" + occurrence = IssueOccurrence.from_dict(occurrence_data) + assert occurrence.assignee == ActorTuple(self.user.id, User) + occurrence_data["assignee"] = f"{self.user.id}" + occurrence = IssueOccurrence.from_dict(occurrence_data) + assert occurrence.assignee == ActorTuple(self.user.id, User) + occurrence_data["assignee"] = f"{self.user.email}" + occurrence = IssueOccurrence.from_dict(occurrence_data) + assert occurrence.assignee == ActorTuple(self.user.id, User) + occurrence_data["assignee"] = f"{self.user.username}" + occurrence = IssueOccurrence.from_dict(occurrence_data) + assert occurrence.assignee == ActorTuple(self.user.id, User) + occurrence_data["assignee"] = f"team:{self.team.id}" + occurrence = IssueOccurrence.from_dict(occurrence_data) + assert occurrence.assignee == ActorTuple(self.team.id, Team) + + def test_assignee_none(self) -> None: + occurrence_data = self.build_occurrence_data() + occurrence = IssueOccurrence.from_dict(occurrence_data) + assert occurrence.assignee is None + occurrence_data["assignee"] = None + occurrence = IssueOccurrence.from_dict(occurrence_data) + assert occurrence.assignee is None + occurrence_data["assignee"] = "" + occurrence = IssueOccurrence.from_dict(occurrence_data) + assert occurrence.assignee is None + class IssueOccurrenceSaveAndFetchTest(OccurrenceTestMixin, TestCase): def test(self) -> None: diff --git a/tests/sentry/issues/test_occurrence_consumer.py b/tests/sentry/issues/test_occurrence_consumer.py index d151a3bb897ff6..34c534df8e2520 100644 --- a/tests/sentry/issues/test_occurrence_consumer.py +++ b/tests/sentry/issues/test_occurrence_consumer.py @@ -458,3 +458,21 @@ def test_priority_overrides_defaults(self) -> None: message["initial_issue_priority"] = PriorityLevel.HIGH kwargs = _get_kwargs(message) assert kwargs["occurrence_data"]["initial_issue_priority"] == PriorityLevel.HIGH + + def test_assignee(self) -> None: + message = deepcopy(get_test_message(self.project.id)) + message["assignee"] = f"user:{self.user.id}" + kwargs = _get_kwargs(message) + assert kwargs["occurrence_data"]["assignee"] == f"user:{self.user.id}" + + def test_assignee_none(self) -> None: + kwargs = _get_kwargs(deepcopy(get_test_message(self.project.id))) + assert kwargs["occurrence_data"]["assignee"] is None + message = deepcopy(get_test_message(self.project.id)) + message["assignee"] = None + kwargs = _get_kwargs(message) + assert kwargs["occurrence_data"]["assignee"] is None + message = deepcopy(get_test_message(self.project.id)) + message["assignee"] = "" + kwargs = _get_kwargs(message) + assert kwargs["occurrence_data"]["assignee"] == ""