-
Notifications
You must be signed in to change notification settings - Fork 4
fix(ecosystem): Breaks issue sync cycles #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
0cf53ea
f665d92
94578b8
6eb81dc
40cfb76
cb2b417
cc67c4c
9501091
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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() | ||
|
|
||
| @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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| 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 | ||
|
|
@@ -24,7 +27,12 @@ | |
| 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) | ||
|
|
||
|
|
@@ -42,10 +50,15 @@ def sync_assignee_outbound(external_issue_id: int, user_id: int | None, assign: | |
| ): | ||
| return | ||
|
|
||
| if installation.should_sync("outbound_assignee"): | ||
| parsed_assignment_source = ( | ||
| AssignmentSource.from_dict(assignment_source_dict) if assignment_source_dict else None | ||
| ) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 AI 代码审查发现问题 📋 问题概述
line 48-50 处仅检查 当调用方传入 📍 问题详情🟡 问题 1 | 严重程度:
|
||
| 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, | ||
|
|
||
| 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): | ||
| 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 AI 代码审查发现问题
📋 问题概述
line 17 处
queued: datetime = timezone.now()在类定义时即求值,导致所有未显式传入queued的实例共享同一时间戳(类定义时的时间),而非实例创建时间。当通过 line 21-24from_integration()创建实例时,若调用方未提供queued参数,记录的时间将永远是模块首次加载时的时间,造成分配来源时间记录错误。📍 问题详情
🟡 问题 1 | 严重程度:
MEDIUM| 行号:17💬 详细说明:
queued: datetime = timezone.now()在类定义时即求值,导致所有未显式传入queued的实例共享同一时间戳(类定义时的时间),而非实例创建时间。当通过 line 21-24from_integration()创建实例时,若调用方未提供queued参数,记录的时间将永远是模块首次加载时的时间,造成分配来源时间记录错误。📝 问题代码:
💡 修复建议:
✅ 修复示例:
🔗 参考链接
无