diff --git a/fixtures/github.py b/fixtures/github.py index 0563606eca939a..c070ea6cb1826a 100644 --- a/fixtures/github.py +++ b/fixtures/github.py @@ -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", diff --git a/src/sentry/integrations/github/webhook.py b/src/sentry/integrations/github/webhook.py index fb62a9d8e8f646..ced99c0efb1272 100644 --- a/src/sentry/integrations/github/webhook.py +++ b/src/sentry/integrations/github/webhook.py @@ -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 + 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( @@ -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( @@ -584,6 +613,7 @@ def _handle_assignment( "external_issue_key": external_issue_key, "assignee_name": assignee_name, "action": action, + "total_assignees": len(assignees), }, ) diff --git a/tests/sentry/integrations/github/test_webhooks.py b/tests/sentry/integrations/github/test_webhooks.py index 46a775c4aac799..ab7d600da8be27 100644 --- a/tests/sentry/integrations/github/test_webhooks.py +++ b/tests/sentry/integrations/github/test_webhooks.py @@ -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()), ) @@ -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, ) @@ -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()), )