Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions src/python/src/sentry/integrations/mixins/issues.py
Original file line number Diff line number Diff line change
@@ -1,30 +1,45 @@
from sentry.eventstore.models import GroupEvent
from sentry.integrations.base import IntegrationInstallation
from sentry.integrations.models.external_issue import ExternalIssue
from sentry.integrations.services.assignment_source import AssignmentSource
from sentry.integrations.services.integration import integration_service
from sentry.integrations.tasks.sync_status_inbound import (
sync_status_inbound as sync_status_inbound_task,


class IssueBasicIntegration(IntegrationInstallation, ABC):
def should_sync(self, attribute):
def should_sync(self, attribute, sync_source: AssignmentSource | None = None):
return False

def get_group_title(self, group, event, **kwargs):
outbound_assignee_key: ClassVar[str | None] = None

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

🔵 Базовый метод should_sync игнорирует параметр sync_source

Сигнатура IssueBasicIntegration.should_sync расширена параметром sync_source: AssignmentSource | None = None, но тело метода безусловно возвращает False, не используя новый аргумент. Это допустимо как no-op базовая реализация, но отсутствует docstring или комментарий, поясняющий, что логика предотвращения циклов реализована только в подклассах. Будущие читатели могут ошибочно предположить, что базовый класс выполняет проверку циклов. Рекомендуется добавить комментарий с описанием намерения.

inbound_assignee_key: ClassVar[str | None] = None

def should_sync(self, attribute: str) -> bool:
def should_sync(self, attribute: str, sync_source: AssignmentSource | None = None) -> bool:
key = getattr(self, f"{attribute}_key", None)
if key is None or self.org_integration is None:
return False

# Check that the assignment source isn't this same integration in order to
# prevent sync-cycles from occurring. This should still allow other
# integrations to propagate changes outward.
if sync_source and sync_source.integration_id == self.org_integration.integration_id:
return False

value: bool = self.org_integration.config.get(key, False)
return value

raise NotImplementedError

@abstractmethod
def sync_status_outbound(self, external_issue, is_resolved, project_id, **kwargs):
def sync_status_outbound(
self,
external_issue,
is_resolved,
project_id,
assignment_source: AssignmentSource | None = None,
**kwargs,
):
"""
Propagate a sentry issue's status to a linked issue's status.
"""
35 changes: 35 additions & 0 deletions src/python/src/sentry/integrations/services/assignment_source.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
from __future__ import annotations

from dataclasses import asdict, dataclass
from datetime import datetime
from typing import TYPE_CHECKING, Any

from django.utils import timezone

if TYPE_CHECKING:
from sentry.integrations.models import Integration
from sentry.integrations.services.integration import RpcIntegration


@dataclass(frozen=True)
class AssignmentSource:
source_name: str
integration_id: int
queued: datetime = timezone.now()

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

🟠 Значение по умолчанию timezone.now() для поля queued вычисляется однократно при импорте модуля

В frozen-датаклассе AssignmentSource поле queued объявлено как queued: datetime = timezone.now(). В Python значения по умолчанию для полей dataclass вычисляются один раз — в момент определения класса (при импорте модуля), а не при создании каждого экземпляра. В результате все объекты, созданные через from_integration или from_dict без явного указания queued, получат один и тот же таймстамп — время первого импорта модуля. Следует использовать field(default_factory=timezone.now).


@classmethod
def from_integration(cls, integration: Integration | RpcIntegration) -> AssignmentSource:
return AssignmentSource(
source_name=integration.name,
integration_id=integration.id,
)

def to_dict(self) -> dict[str, Any]:
return asdict(self)

@classmethod
def from_dict(cls, input_dict: dict[str, Any]) -> AssignmentSource | None:
try:
return cls(**input_dict)
except (ValueError, TypeError):
return None
19 changes: 16 additions & 3 deletions src/python/src/sentry/integrations/tasks/sync_assignee_outbound.py
Original file line number Diff line number Diff line change
@@ -1,23 +1,36 @@
from typing import Any

from sentry import analytics, features
from sentry.integrations.models.external_issue import ExternalIssue
from sentry.integrations.models.integration import Integration
from sentry.integrations.services.assignment_source import AssignmentSource
from sentry.integrations.services.integration import integration_service
from sentry.models.organization import Organization
from sentry.silo.base import SiloMode
Organization.DoesNotExist,
)
)
def sync_assignee_outbound(external_issue_id: int, user_id: int | None, assign: bool) -> None:
def sync_assignee_outbound(
external_issue_id: int,
user_id: int | None,
assign: bool,
assignment_source_dict: dict[str, Any] | None = None,
) -> None:
# Sync Sentry assignee to an external issue.
external_issue = ExternalIssue.objects.get(id=external_issue_id)

):
return

if installation.should_sync("outbound_assignee"):
parsed_assignment_source = (
AssignmentSource.from_dict(assignment_source_dict) if assignment_source_dict else None
)
if installation.should_sync("outbound_assignee", parsed_assignment_source):
# Assume unassign if None.
user = user_service.get_user(user_id) if user_id else None
installation.sync_assignee_outbound(external_issue, user, assign=assign)
installation.sync_assignee_outbound(
external_issue, user, assign=assign, assignment_source=parsed_assignment_source
)
analytics.record(
"integration.issue.assignee.synced",
provider=integration.provider,
29 changes: 25 additions & 4 deletions src/python/src/sentry/integrations/utils/sync.py
Original file line number Diff line number Diff line change
@@ -1,32 +1,53 @@
from typing import TYPE_CHECKING

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

🟡 Потеря типа datetime при round-trip to_dict()/from_dict() через Celery kwargs

Функция sync_group_assignee_outbound передаёт результат assignment_source.to_dict() (где queued имеет тип datetime) как assignment_source_dict в sync_assignee_outbound.apply_async(kwargs=...). При JSON-сериализации Celery datetime превращается в строку. Метод from_dict вызывает cls(**input_dict), сохраняя строку вместо datetime. Объявленный тип поля нарушается, что может привести к ошибкам при сравнении или форматировании даты downstream. Рекомендуется явно парсить строку обратно в datetime в from_dict.


from sentry import features
from sentry.integrations.services.assignment_source import AssignmentSource
from sentry.integrations.services.integration import integration_service
from sentry.integrations.tasks.sync_assignee_outbound import sync_assignee_outbound
from sentry.models.group import Group

if not assign:
for group in affected_groups:
GroupAssignee.objects.deassign(group)
GroupAssignee.objects.deassign(
group,
assignment_source=AssignmentSource.from_integration(integration),
)

return affected_groups

users = user_service.get_many_by_email(emails=[email], is_verified=True)
user_id = get_user_id(projects_by_user, group)
user = users_by_id.get(user_id)
if user:
GroupAssignee.objects.assign(group, user)
GroupAssignee.objects.assign(
group,
user,
assignment_source=AssignmentSource.from_integration(integration),
)
groups_assigned.append(group)
else:
logger.info("assignee-not-found-inbound", extra=log_context)
return groups_assigned


def sync_group_assignee_outbound(group: Group, user_id: int | None, assign: bool = True) -> None:
def sync_group_assignee_outbound(
group: Group,
user_id: int | None,
assign: bool = True,
assignment_source: AssignmentSource | None = None,
) -> None:
from sentry.models.grouplink import GroupLink

external_issue_ids = GroupLink.objects.filter(

for external_issue_id in external_issue_ids:
sync_assignee_outbound.apply_async(
kwargs={"external_issue_id": external_issue_id, "user_id": user_id, "assign": assign}
kwargs={
"external_issue_id": external_issue_id,
"user_id": user_id,
"assign": assign,
"assignment_source_dict": assignment_source.to_dict()
if assignment_source
else None,
}
)
11 changes: 9 additions & 2 deletions src/python/src/sentry/models/groupassignee.py
Original file line number Diff line number Diff line change
@@ -1,32 +1,39 @@
from sentry.db.models import FlexibleForeignKey, Model, region_silo_model, sane_repr
from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
from sentry.db.models.manager.base import BaseManager
from sentry.integrations.services.assignment_source import AssignmentSource
from sentry.models.grouphistory import GroupHistoryStatus, record_group_history
from sentry.models.groupowner import GroupOwner
from sentry.models.groupsubscription import GroupSubscription
create_only: bool = False,
extra: dict[str, str] | None = None,
force_autoassign: bool = False,
assignment_source: AssignmentSource | None = None,
):
from sentry.integrations.utils import sync_group_assignee_outbound
from sentry.models.activity import Activity
if assignee_type == "user" and features.has(
"organizations:integrations-issue-sync", group.organization, actor=acting_user
):
sync_group_assignee_outbound(group, assigned_to.id, assign=True)
sync_group_assignee_outbound(
group, assigned_to.id, assign=True, assignment_source=assignment_source
)

if not created: # aka re-assignment
self.remove_old_assignees(group, assignee, assigned_to_id, assignee_type)
acting_user: User | RpcUser | None = None,
assigned_to: Team | RpcUser | None = None,
extra: dict[str, str] | None = None,
assignment_source: AssignmentSource | None = None,
) -> None:
from sentry.integrations.utils import sync_group_assignee_outbound
from sentry.models.activity import Activity
if features.has(
"organizations:integrations-issue-sync", group.organization, actor=acting_user
):
sync_group_assignee_outbound(group, None, assign=False)
sync_group_assignee_outbound(
group, None, assign=False, assignment_source=assignment_source
)

issue_unassigned.send_robust(
project=group.project, group=group, user=acting_user, sender=self.__class__
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
from typing import Any

from sentry.integrations.services.assignment_source import AssignmentSource
from sentry.testutils.cases import TestCase


class TestAssignmentSource(TestCase):
def test_from_dict_empty_array(self):
data: dict[str, Any] = {}
result = AssignmentSource.from_dict(data)
assert result is None

def test_from_dict_inalid_data(self):

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

🔵 Опечатка в имени теста: test_from_dict_inalid_data

Имя тестового метода содержит опечатку — inalid вместо invalid. Это снижает читаемость и обнаруживаемость теста при поиске.

data = {
"foo": "bar",
}

result = AssignmentSource.from_dict(data)
assert result is None

def test_from_dict_valid_data(self):
data = {"source_name": "foo-source", "integration_id": 123}

result = AssignmentSource.from_dict(data)
assert result is not None
assert result.source_name == "foo-source"
assert result.integration_id == 123

def test_to_dict(self):
source = AssignmentSource(
source_name="foo-source",
integration_id=123,
)

result = source.to_dict()
assert result.get("queued") is not None
assert result.get("source_name") == "foo-source"
assert result.get("integration_id") == 123
74 changes: 71 additions & 3 deletions src/python/tests/sentry/models/test_groupassignee.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,93 @@

from sentry.integrations.example.integration import ExampleIntegration
from sentry.integrations.models.external_issue import ExternalIssue
from sentry.integrations.services.assignment_source import AssignmentSource
from sentry.integrations.utils import sync_group_assignee_inbound
from sentry.models.activity import Activity
from sentry.models.groupassignee import GroupAssignee

with self.feature({"organizations:integrations-issue-sync": True}):
with self.tasks():
GroupAssignee.objects.assign(self.group, self.user)
GroupAssignee.objects.assign(
self.group,
self.user,
)

mock_sync_assignee_outbound.assert_called_with(
external_issue, user_service.get_user(self.user.id), assign=True
external_issue,
user_service.get_user(self.user.id),
assign=True,
assignment_source=None,
)

assert GroupAssignee.objects.filter(
project=self.group.project,
group=self.group,
user_id=self.user.id,
team__isnull=True,
).exists()

activity = Activity.objects.get(
project=self.group.project, group=self.group, type=ActivityType.ASSIGNED.value
)

assert activity.data["assignee"] == str(self.user.id)
assert activity.data["assigneeEmail"] == self.user.email
assert activity.data["assigneeType"] == "user"

@mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
def test_assignee_sync_outbound_assign_with_matching_source_integration(
self, mock_sync_assignee_outbound
):
group = self.group
integration = self.create_integration(
organization=group.organization,
external_id="123456",
provider="example",
oi_params={
"config": {
"sync_comments": True,
"sync_status_outbound": True,
"sync_status_inbound": True,
"sync_assignee_outbound": True,
"sync_assignee_inbound": True,
}
},
)

external_issue = ExternalIssue.objects.create(
organization_id=group.organization.id, integration_id=integration.id, key="APP-123"
)

GroupLink.objects.create(
group_id=group.id,
project_id=group.project_id,
linked_type=GroupLink.LinkedType.issue,
linked_id=external_issue.id,
relationship=GroupLink.Relationship.references,
)

with self.feature({"organizations:integrations-issue-sync": True}):
with self.tasks():
# Assert that we don't perform an outbound assignment if
# the source of the assignment is the same target integration
GroupAssignee.objects.assign(
self.group,
self.user,
assignment_source=AssignmentSource.from_integration(integration),
)

mock_sync_assignee_outbound.assert_not_called()

assert GroupAssignee.objects.filter(
project=self.group.project,
group=self.group,
with self.feature({"organizations:integrations-issue-sync": True}):
with self.tasks():
GroupAssignee.objects.deassign(self.group)
mock_sync_assignee_outbound.assert_called_with(external_issue, None, assign=False)
mock_sync_assignee_outbound.assert_called_with(
external_issue, None, assign=False, assignment_source=None
)

assert not GroupAssignee.objects.filter(
project=self.group.project,