Skip to content
Merged
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
22 changes: 21 additions & 1 deletion fixtures/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -3012,7 +3012,27 @@
"type": "User",
"site_admin": false
},
"assignees": [],
"assignees": [
{
"login": "octocat",
"id": 1,
"avatar_url": "https://avatars.githubusercontent.com/u/1?v=3",
"gravatar_id": "",
"url": "https://api.github.com/users/octocat",
"html_url": "https://github.com/octocat",
"followers_url": "https://api.github.com/users/octocat/followers",
"following_url": "https://api.github.com/users/octocat/following{/other_user}",
"gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
"starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
"organizations_url": "https://api.github.com/users/octocat/orgs",
"repos_url": "https://api.github.com/users/octocat/repos",
"events_url": "https://api.github.com/users/octocat/events{/privacy}",
"received_events_url": "https://api.github.com/users/octocat/received_events",
"type": "User",
"site_admin": false
}
],
"milestone": null,
"comments": 0,
"created_at": "2015-05-05T23:40:28Z",
Expand Down
36 changes: 33 additions & 3 deletions src/sentry/integrations/github/webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -547,14 +547,43 @@ def _handle_assignment(
"""
Handle issue assignment and unassignment events.

When switching assignees, GitHub sends two webhooks (assigned and unassigned) in
non-deterministic order. To avoid race conditions, we sync based on the current
state in issue.assignees rather than the delta in the assignee field.

Args:
integration: The GitHub integration
event: The webhook event payload
external_issue_key: The formatted issue key
action: The action type ('assigned' or 'unassigned')
"""
assignee = event.get("assignee", {})
assignee_gh_name = assignee.get("login")
# Use issue.assignees (current state) instead of assignee (delta) to avoid race conditions
issue = event.get("issue", {})
assignees = issue.get("assignees", [])

# If there are no assignees, deassign
if not assignees:
sync_group_assignee_inbound_by_external_actor(
integration=integration,
external_user_name="", # Not used for deassignment
Copy link
Member

Choose a reason for hiding this comment

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

Weird that this isn't an optional param....

external_issue_key=external_issue_key,
assign=False,
)
logger.info(
"github.webhook.assignment.synced",
extra={
"integration_id": integration.id,
"external_issue_key": external_issue_key,
"assignee_name": None,
"action": "deassigned",
},
)
return

# GitHub supports multiple assignees, but Sentry currently only supports one
# Take the first assignee from the current state
first_assignee = assignees[0]
assignee_gh_name = first_assignee.get("login")

if not assignee_gh_name:
logger.warning(
Expand All @@ -574,7 +603,7 @@ def _handle_assignment(
integration=integration,
external_user_name=assignee_name,
external_issue_key=external_issue_key,
assign=(action == IssueEvenntWebhookActionType.ASSIGNED.value),
assign=True,
)

logger.info(
Expand All @@ -584,6 +613,7 @@ def _handle_assignment(
"external_issue_key": external_issue_key,
"assignee_name": assignee_name,
"action": action,
"total_assignees": len(assignees),
},
)

Expand Down
12 changes: 7 additions & 5 deletions tests/sentry/integrations/github/test_webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -1038,8 +1038,8 @@ def test_assigned_issue(self, mock_record: MagicMock, mock_sync: MagicMock) -> N
data=ISSUES_ASSIGNED_EVENT_EXAMPLE,
content_type="application/json",
HTTP_X_GITHUB_EVENT="issues",
HTTP_X_HUB_SIGNATURE="sha1=e19d80f5a6e09e20d126e923464778bb4b601a7e",
HTTP_X_HUB_SIGNATURE_256="sha256=e91927e8d8e0db9cb1f5ead889bba2deb24aa2f8925a8eae85ba732424604489",
HTTP_X_HUB_SIGNATURE="sha1=75deab06ede0068fe16b5f1f6ee1a9509738e006",
HTTP_X_HUB_SIGNATURE_256="sha256=1703af48011c6709662f776163fce1e86772eff189f94e1ebff5ad66a81b711e",
HTTP_X_GITHUB_DELIVERY=str(uuid4()),
)

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

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

# With the fix, we now use issue.assignees (current state) instead of assignee (delta)
# ISSUES_UNASSIGNED_EVENT_EXAMPLE has assignees=[], so we deassign
mock_sync.assert_called_once_with(
integration=rpc_integration,
external_user_name="@octocat",
external_user_name="",
external_issue_key="baxterthehacker/public-repo#2",
assign=False,
)
Expand Down Expand Up @@ -1123,8 +1125,8 @@ def test_creates_missing_repo_for_issues(self, mock_metrics: MagicMock) -> None:
data=ISSUES_ASSIGNED_EVENT_EXAMPLE,
content_type="application/json",
HTTP_X_GITHUB_EVENT="issues",
HTTP_X_HUB_SIGNATURE="sha1=e19d80f5a6e09e20d126e923464778bb4b601a7e",
HTTP_X_HUB_SIGNATURE_256="sha256=e91927e8d8e0db9cb1f5ead889bba2deb24aa2f8925a8eae85ba732424604489",
HTTP_X_HUB_SIGNATURE="sha1=75deab06ede0068fe16b5f1f6ee1a9509738e006",
HTTP_X_HUB_SIGNATURE_256="sha256=1703af48011c6709662f776163fce1e86772eff189f94e1ebff5ad66a81b711e",
HTTP_X_GITHUB_DELIVERY=str(uuid4()),
)

Expand Down
Loading