Skip to content

Commit f9919d7

Browse files
fix(ecosystem): Breaks issue sync cycles (#77754)
1 parent 19c92a4 commit f9919d7

File tree

7 files changed

+212
-15
lines changed

7 files changed

+212
-15
lines changed

src/sentry/integrations/mixins/issues.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from sentry.eventstore.models import GroupEvent
1313
from sentry.integrations.base import IntegrationInstallation
1414
from sentry.integrations.models.external_issue import ExternalIssue
15+
from sentry.integrations.services.assignment_source import AssignmentSource
1516
from sentry.integrations.services.integration import integration_service
1617
from sentry.integrations.tasks.sync_status_inbound import (
1718
sync_status_inbound as sync_status_inbound_task,
@@ -62,7 +63,7 @@ def from_resolve_unresolve(
6263

6364

6465
class IssueBasicIntegration(IntegrationInstallation, ABC):
65-
def should_sync(self, attribute):
66+
def should_sync(self, attribute, sync_source: AssignmentSource | None = None):
6667
return False
6768

6869
def get_group_title(self, group, event, **kwargs):
@@ -378,10 +379,17 @@ class IssueSyncIntegration(IssueBasicIntegration, ABC):
378379
outbound_assignee_key: ClassVar[str | None] = None
379380
inbound_assignee_key: ClassVar[str | None] = None
380381

381-
def should_sync(self, attribute: str) -> bool:
382+
def should_sync(self, attribute: str, sync_source: AssignmentSource | None = None) -> bool:
382383
key = getattr(self, f"{attribute}_key", None)
383384
if key is None or self.org_integration is None:
384385
return False
386+
387+
# Check that the assignment source isn't this same integration in order to
388+
# prevent sync-cycles from occurring. This should still allow other
389+
# integrations to propagate changes outward.
390+
if sync_source and sync_source.integration_id == self.org_integration.integration_id:
391+
return False
392+
385393
value: bool = self.org_integration.config.get(key, False)
386394
return value
387395

@@ -400,7 +408,14 @@ def sync_assignee_outbound(
400408
raise NotImplementedError
401409

402410
@abstractmethod
403-
def sync_status_outbound(self, external_issue, is_resolved, project_id, **kwargs):
411+
def sync_status_outbound(
412+
self,
413+
external_issue,
414+
is_resolved,
415+
project_id,
416+
assignment_source: AssignmentSource | None = None,
417+
**kwargs,
418+
):
404419
"""
405420
Propagate a sentry issue's status to a linked issue's status.
406421
"""
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
from __future__ import annotations
2+
3+
from dataclasses import asdict, dataclass
4+
from datetime import datetime
5+
from typing import TYPE_CHECKING, Any
6+
7+
from django.utils import timezone
8+
9+
if TYPE_CHECKING:
10+
from sentry.integrations.models import Integration
11+
from sentry.integrations.services.integration import RpcIntegration
12+
13+
14+
@dataclass(frozen=True)
15+
class AssignmentSource:
16+
source_name: str
17+
integration_id: int
18+
queued: datetime = timezone.now()
19+
20+
@classmethod
21+
def from_integration(cls, integration: Integration | RpcIntegration) -> AssignmentSource:
22+
return AssignmentSource(
23+
source_name=integration.name,
24+
integration_id=integration.id,
25+
)
26+
27+
def to_dict(self) -> dict[str, Any]:
28+
return asdict(self)
29+
30+
@classmethod
31+
def from_dict(cls, input_dict: dict[str, Any]) -> AssignmentSource | None:
32+
try:
33+
return cls(**input_dict)
34+
except (ValueError, TypeError):
35+
return None

src/sentry/integrations/tasks/sync_assignee_outbound.py

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
from typing import Any
2+
13
from sentry import analytics, features
24
from sentry.integrations.models.external_issue import ExternalIssue
35
from sentry.integrations.models.integration import Integration
6+
from sentry.integrations.services.assignment_source import AssignmentSource
47
from sentry.integrations.services.integration import integration_service
58
from sentry.models.organization import Organization
69
from sentry.silo.base import SiloMode
@@ -24,7 +27,12 @@
2427
Organization.DoesNotExist,
2528
)
2629
)
27-
def sync_assignee_outbound(external_issue_id: int, user_id: int | None, assign: bool) -> None:
30+
def sync_assignee_outbound(
31+
external_issue_id: int,
32+
user_id: int | None,
33+
assign: bool,
34+
assignment_source_dict: dict[str, Any] | None = None,
35+
) -> None:
2836
# Sync Sentry assignee to an external issue.
2937
external_issue = ExternalIssue.objects.get(id=external_issue_id)
3038

@@ -42,10 +50,15 @@ def sync_assignee_outbound(external_issue_id: int, user_id: int | None, assign:
4250
):
4351
return
4452

45-
if installation.should_sync("outbound_assignee"):
53+
parsed_assignment_source = (
54+
AssignmentSource.from_dict(assignment_source_dict) if assignment_source_dict else None
55+
)
56+
if installation.should_sync("outbound_assignee", parsed_assignment_source):
4657
# Assume unassign if None.
4758
user = user_service.get_user(user_id) if user_id else None
48-
installation.sync_assignee_outbound(external_issue, user, assign=assign)
59+
installation.sync_assignee_outbound(
60+
external_issue, user, assign=assign, assignment_source=parsed_assignment_source
61+
)
4962
analytics.record(
5063
"integration.issue.assignee.synced",
5164
provider=integration.provider,

src/sentry/integrations/utils/sync.py

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from typing import TYPE_CHECKING
66

77
from sentry import features
8+
from sentry.integrations.services.assignment_source import AssignmentSource
89
from sentry.integrations.services.integration import integration_service
910
from sentry.integrations.tasks.sync_assignee_outbound import sync_assignee_outbound
1011
from sentry.models.group import Group
@@ -92,7 +93,11 @@ def sync_group_assignee_inbound(
9293

9394
if not assign:
9495
for group in affected_groups:
95-
GroupAssignee.objects.deassign(group)
96+
GroupAssignee.objects.deassign(
97+
group,
98+
assignment_source=AssignmentSource.from_integration(integration),
99+
)
100+
96101
return affected_groups
97102

98103
users = user_service.get_many_by_email(emails=[email], is_verified=True)
@@ -104,14 +109,23 @@ def sync_group_assignee_inbound(
104109
user_id = get_user_id(projects_by_user, group)
105110
user = users_by_id.get(user_id)
106111
if user:
107-
GroupAssignee.objects.assign(group, user)
112+
GroupAssignee.objects.assign(
113+
group,
114+
user,
115+
assignment_source=AssignmentSource.from_integration(integration),
116+
)
108117
groups_assigned.append(group)
109118
else:
110119
logger.info("assignee-not-found-inbound", extra=log_context)
111120
return groups_assigned
112121

113122

114-
def sync_group_assignee_outbound(group: Group, user_id: int | None, assign: bool = True) -> None:
123+
def sync_group_assignee_outbound(
124+
group: Group,
125+
user_id: int | None,
126+
assign: bool = True,
127+
assignment_source: AssignmentSource | None = None,
128+
) -> None:
115129
from sentry.models.grouplink import GroupLink
116130

117131
external_issue_ids = GroupLink.objects.filter(
@@ -120,5 +134,12 @@ def sync_group_assignee_outbound(group: Group, user_id: int | None, assign: bool
120134

121135
for external_issue_id in external_issue_ids:
122136
sync_assignee_outbound.apply_async(
123-
kwargs={"external_issue_id": external_issue_id, "user_id": user_id, "assign": assign}
137+
kwargs={
138+
"external_issue_id": external_issue_id,
139+
"user_id": user_id,
140+
"assign": assign,
141+
"assignment_source_dict": assignment_source.to_dict()
142+
if assignment_source
143+
else None,
144+
}
124145
)

src/sentry/models/groupassignee.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from sentry.db.models import FlexibleForeignKey, Model, region_silo_model, sane_repr
1313
from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
1414
from sentry.db.models.manager.base import BaseManager
15+
from sentry.integrations.services.assignment_source import AssignmentSource
1516
from sentry.models.grouphistory import GroupHistoryStatus, record_group_history
1617
from sentry.models.groupowner import GroupOwner
1718
from sentry.models.groupsubscription import GroupSubscription
@@ -134,6 +135,7 @@ def assign(
134135
create_only: bool = False,
135136
extra: dict[str, str] | None = None,
136137
force_autoassign: bool = False,
138+
assignment_source: AssignmentSource | None = None,
137139
):
138140
from sentry.integrations.utils import sync_group_assignee_outbound
139141
from sentry.models.activity import Activity
@@ -187,7 +189,9 @@ def assign(
187189
if assignee_type == "user" and features.has(
188190
"organizations:integrations-issue-sync", group.organization, actor=acting_user
189191
):
190-
sync_group_assignee_outbound(group, assigned_to.id, assign=True)
192+
sync_group_assignee_outbound(
193+
group, assigned_to.id, assign=True, assignment_source=assignment_source
194+
)
191195

192196
if not created: # aka re-assignment
193197
self.remove_old_assignees(group, assignee, assigned_to_id, assignee_type)
@@ -200,6 +204,7 @@ def deassign(
200204
acting_user: User | RpcUser | None = None,
201205
assigned_to: Team | RpcUser | None = None,
202206
extra: dict[str, str] | None = None,
207+
assignment_source: AssignmentSource | None = None,
203208
) -> None:
204209
from sentry.integrations.utils import sync_group_assignee_outbound
205210
from sentry.models.activity import Activity
@@ -230,7 +235,9 @@ def deassign(
230235
if features.has(
231236
"organizations:integrations-issue-sync", group.organization, actor=acting_user
232237
):
233-
sync_group_assignee_outbound(group, None, assign=False)
238+
sync_group_assignee_outbound(
239+
group, None, assign=False, assignment_source=assignment_source
240+
)
234241

235242
issue_unassigned.send_robust(
236243
project=group.project, group=group, user=acting_user, sender=self.__class__
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
from typing import Any
2+
3+
from sentry.integrations.services.assignment_source import AssignmentSource
4+
from sentry.testutils.cases import TestCase
5+
6+
7+
class TestAssignmentSource(TestCase):
8+
def test_from_dict_empty_array(self):
9+
data: dict[str, Any] = {}
10+
result = AssignmentSource.from_dict(data)
11+
assert result is None
12+
13+
def test_from_dict_inalid_data(self):
14+
data = {
15+
"foo": "bar",
16+
}
17+
18+
result = AssignmentSource.from_dict(data)
19+
assert result is None
20+
21+
def test_from_dict_valid_data(self):
22+
data = {"source_name": "foo-source", "integration_id": 123}
23+
24+
result = AssignmentSource.from_dict(data)
25+
assert result is not None
26+
assert result.source_name == "foo-source"
27+
assert result.integration_id == 123
28+
29+
def test_to_dict(self):
30+
source = AssignmentSource(
31+
source_name="foo-source",
32+
integration_id=123,
33+
)
34+
35+
result = source.to_dict()
36+
assert result.get("queued") is not None
37+
assert result.get("source_name") == "foo-source"
38+
assert result.get("integration_id") == 123

tests/sentry/models/test_groupassignee.py

Lines changed: 71 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from sentry.integrations.example.integration import ExampleIntegration
66
from sentry.integrations.models.external_issue import ExternalIssue
7+
from sentry.integrations.services.assignment_source import AssignmentSource
78
from sentry.integrations.utils import sync_group_assignee_inbound
89
from sentry.models.activity import Activity
910
from sentry.models.groupassignee import GroupAssignee
@@ -148,12 +149,77 @@ def test_assignee_sync_outbound_assign(self, mock_sync_assignee_outbound):
148149

149150
with self.feature({"organizations:integrations-issue-sync": True}):
150151
with self.tasks():
151-
GroupAssignee.objects.assign(self.group, self.user)
152+
GroupAssignee.objects.assign(
153+
self.group,
154+
self.user,
155+
)
152156

153157
mock_sync_assignee_outbound.assert_called_with(
154-
external_issue, user_service.get_user(self.user.id), assign=True
158+
external_issue,
159+
user_service.get_user(self.user.id),
160+
assign=True,
161+
assignment_source=None,
162+
)
163+
164+
assert GroupAssignee.objects.filter(
165+
project=self.group.project,
166+
group=self.group,
167+
user_id=self.user.id,
168+
team__isnull=True,
169+
).exists()
170+
171+
activity = Activity.objects.get(
172+
project=self.group.project, group=self.group, type=ActivityType.ASSIGNED.value
173+
)
174+
175+
assert activity.data["assignee"] == str(self.user.id)
176+
assert activity.data["assigneeEmail"] == self.user.email
177+
assert activity.data["assigneeType"] == "user"
178+
179+
@mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
180+
def test_assignee_sync_outbound_assign_with_matching_source_integration(
181+
self, mock_sync_assignee_outbound
182+
):
183+
group = self.group
184+
integration = self.create_integration(
185+
organization=group.organization,
186+
external_id="123456",
187+
provider="example",
188+
oi_params={
189+
"config": {
190+
"sync_comments": True,
191+
"sync_status_outbound": True,
192+
"sync_status_inbound": True,
193+
"sync_assignee_outbound": True,
194+
"sync_assignee_inbound": True,
195+
}
196+
},
197+
)
198+
199+
external_issue = ExternalIssue.objects.create(
200+
organization_id=group.organization.id, integration_id=integration.id, key="APP-123"
201+
)
202+
203+
GroupLink.objects.create(
204+
group_id=group.id,
205+
project_id=group.project_id,
206+
linked_type=GroupLink.LinkedType.issue,
207+
linked_id=external_issue.id,
208+
relationship=GroupLink.Relationship.references,
209+
)
210+
211+
with self.feature({"organizations:integrations-issue-sync": True}):
212+
with self.tasks():
213+
# Assert that we don't perform an outbound assignment if
214+
# the source of the assignment is the same target integration
215+
GroupAssignee.objects.assign(
216+
self.group,
217+
self.user,
218+
assignment_source=AssignmentSource.from_integration(integration),
155219
)
156220

221+
mock_sync_assignee_outbound.assert_not_called()
222+
157223
assert GroupAssignee.objects.filter(
158224
project=self.group.project,
159225
group=self.group,
@@ -205,7 +271,9 @@ def test_assignee_sync_outbound_unassign(self, mock_sync_assignee_outbound):
205271
with self.feature({"organizations:integrations-issue-sync": True}):
206272
with self.tasks():
207273
GroupAssignee.objects.deassign(self.group)
208-
mock_sync_assignee_outbound.assert_called_with(external_issue, None, assign=False)
274+
mock_sync_assignee_outbound.assert_called_with(
275+
external_issue, None, assign=False, assignment_source=None
276+
)
209277

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

0 commit comments

Comments
 (0)