Skip to content

Commit 0d9a78b

Browse files
authored
🐛 fix(github): Use issues.assignees for inbound assignment sync (#103008)
1 parent 3dcf2c7 commit 0d9a78b

File tree

3 files changed

+61
-9
lines changed

3 files changed

+61
-9
lines changed

fixtures/github.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3012,7 +3012,27 @@
30123012
"type": "User",
30133013
"site_admin": false
30143014
},
3015-
"assignees": [],
3015+
"assignees": [
3016+
{
3017+
"login": "octocat",
3018+
"id": 1,
3019+
"avatar_url": "https://avatars.githubusercontent.com/u/1?v=3",
3020+
"gravatar_id": "",
3021+
"url": "https://api.github.com/users/octocat",
3022+
"html_url": "https://github.com/octocat",
3023+
"followers_url": "https://api.github.com/users/octocat/followers",
3024+
"following_url": "https://api.github.com/users/octocat/following{/other_user}",
3025+
"gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
3026+
"starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
3027+
"subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
3028+
"organizations_url": "https://api.github.com/users/octocat/orgs",
3029+
"repos_url": "https://api.github.com/users/octocat/repos",
3030+
"events_url": "https://api.github.com/users/octocat/events{/privacy}",
3031+
"received_events_url": "https://api.github.com/users/octocat/received_events",
3032+
"type": "User",
3033+
"site_admin": false
3034+
}
3035+
],
30163036
"milestone": null,
30173037
"comments": 0,
30183038
"created_at": "2015-05-05T23:40:28Z",

src/sentry/integrations/github/webhook.py

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -547,14 +547,43 @@ def _handle_assignment(
547547
"""
548548
Handle issue assignment and unassignment events.
549549
550+
When switching assignees, GitHub sends two webhooks (assigned and unassigned) in
551+
non-deterministic order. To avoid race conditions, we sync based on the current
552+
state in issue.assignees rather than the delta in the assignee field.
553+
550554
Args:
551555
integration: The GitHub integration
552556
event: The webhook event payload
553557
external_issue_key: The formatted issue key
554558
action: The action type ('assigned' or 'unassigned')
555559
"""
556-
assignee = event.get("assignee", {})
557-
assignee_gh_name = assignee.get("login")
560+
# Use issue.assignees (current state) instead of assignee (delta) to avoid race conditions
561+
issue = event.get("issue", {})
562+
assignees = issue.get("assignees", [])
563+
564+
# If there are no assignees, deassign
565+
if not assignees:
566+
sync_group_assignee_inbound_by_external_actor(
567+
integration=integration,
568+
external_user_name="", # Not used for deassignment
569+
external_issue_key=external_issue_key,
570+
assign=False,
571+
)
572+
logger.info(
573+
"github.webhook.assignment.synced",
574+
extra={
575+
"integration_id": integration.id,
576+
"external_issue_key": external_issue_key,
577+
"assignee_name": None,
578+
"action": "deassigned",
579+
},
580+
)
581+
return
582+
583+
# GitHub supports multiple assignees, but Sentry currently only supports one
584+
# Take the first assignee from the current state
585+
first_assignee = assignees[0]
586+
assignee_gh_name = first_assignee.get("login")
558587

559588
if not assignee_gh_name:
560589
logger.warning(
@@ -574,7 +603,7 @@ def _handle_assignment(
574603
integration=integration,
575604
external_user_name=assignee_name,
576605
external_issue_key=external_issue_key,
577-
assign=(action == IssueEvenntWebhookActionType.ASSIGNED.value),
606+
assign=True,
578607
)
579608

580609
logger.info(
@@ -584,6 +613,7 @@ def _handle_assignment(
584613
"external_issue_key": external_issue_key,
585614
"assignee_name": assignee_name,
586615
"action": action,
616+
"total_assignees": len(assignees),
587617
},
588618
)
589619

tests/sentry/integrations/github/test_webhooks.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,8 +1038,8 @@ def test_assigned_issue(self, mock_record: MagicMock, mock_sync: MagicMock) -> N
10381038
data=ISSUES_ASSIGNED_EVENT_EXAMPLE,
10391039
content_type="application/json",
10401040
HTTP_X_GITHUB_EVENT="issues",
1041-
HTTP_X_HUB_SIGNATURE="sha1=e19d80f5a6e09e20d126e923464778bb4b601a7e",
1042-
HTTP_X_HUB_SIGNATURE_256="sha256=e91927e8d8e0db9cb1f5ead889bba2deb24aa2f8925a8eae85ba732424604489",
1041+
HTTP_X_HUB_SIGNATURE="sha1=75deab06ede0068fe16b5f1f6ee1a9509738e006",
1042+
HTTP_X_HUB_SIGNATURE_256="sha256=1703af48011c6709662f776163fce1e86772eff189f94e1ebff5ad66a81b711e",
10431043
HTTP_X_GITHUB_DELIVERY=str(uuid4()),
10441044
)
10451045

@@ -1081,9 +1081,11 @@ def test_unassigned_issue(self, mock_record: MagicMock, mock_sync: MagicMock) ->
10811081

10821082
rpc_integration = integration_service.get_integration(integration_id=self.integration.id)
10831083

1084+
# With the fix, we now use issue.assignees (current state) instead of assignee (delta)
1085+
# ISSUES_UNASSIGNED_EVENT_EXAMPLE has assignees=[], so we deassign
10841086
mock_sync.assert_called_once_with(
10851087
integration=rpc_integration,
1086-
external_user_name="@octocat",
1088+
external_user_name="",
10871089
external_issue_key="baxterthehacker/public-repo#2",
10881090
assign=False,
10891091
)
@@ -1123,8 +1125,8 @@ def test_creates_missing_repo_for_issues(self, mock_metrics: MagicMock) -> None:
11231125
data=ISSUES_ASSIGNED_EVENT_EXAMPLE,
11241126
content_type="application/json",
11251127
HTTP_X_GITHUB_EVENT="issues",
1126-
HTTP_X_HUB_SIGNATURE="sha1=e19d80f5a6e09e20d126e923464778bb4b601a7e",
1127-
HTTP_X_HUB_SIGNATURE_256="sha256=e91927e8d8e0db9cb1f5ead889bba2deb24aa2f8925a8eae85ba732424604489",
1128+
HTTP_X_HUB_SIGNATURE="sha1=75deab06ede0068fe16b5f1f6ee1a9509738e006",
1129+
HTTP_X_HUB_SIGNATURE_256="sha256=1703af48011c6709662f776163fce1e86772eff189f94e1ebff5ad66a81b711e",
11281130
HTTP_X_GITHUB_DELIVERY=str(uuid4()),
11291131
)
11301132

0 commit comments

Comments
 (0)