Skip to content
Open
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
106 changes: 55 additions & 51 deletions src/sentry/integrations/gitlab/issue_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
logger = logging.getLogger("sentry.integrations.gitlab.issue_sync")


def _add_lifecycle_extras(lifecycle: Any, extras: Mapping[str, Any]) -> None:
if lifecycle is not None and hasattr(lifecycle, "add_extras"):
lifecycle.add_extras(extras)


class GitlabIssueSyncSpec(IssueSyncIntegration):
comment_key = "sync_comments"
outbound_assignee_key = "sync_forward_assignment"
Expand Down Expand Up @@ -70,19 +75,23 @@ def sync_assignee_outbound(
If assign=True, we're assigning the issue. Otherwise, deassign.
"""
client = self.get_client()
lifecycle = kwargs.get("lifecycle")

project_id, issue_id = self.split_external_issue_key(external_issue.key)
log_context: dict[str, Any] = {
"provider": self.model.provider,
"integration_id": external_issue.integration_id,
"external_issue_key": external_issue.key,
"external_issue_id": external_issue.id,
"project_path": project_id,
"issue_iid": issue_id,
"assign": assign,
"user_id": user.id if user else None,
}
_add_lifecycle_extras(lifecycle, log_context)

if not project_id or not issue_id:
logger.error(
"assignee-outbound.invalid-key",
extra={
"provider": self.model.provider,
"integration_id": external_issue.integration_id,
"external_issue_key": external_issue.key,
"external_issue_id": external_issue.id,
},
)
logger.warning("assignee-outbound.invalid-key", extra=log_context)
return

gitlab_user_id = None
Expand All @@ -98,58 +107,53 @@ def sync_assignee_outbound(
organization=external_issue.organization,
).first()
if not external_actor:
logger.info(
"assignee-outbound.external-actor-not-found",
extra={
"provider": self.model.provider,
"integration_id": external_issue.integration_id,
"user_id": user.id,
},
)
logger.info("assignee-outbound.external-actor-not-found", extra=log_context)
return

log_context.update(
{
"external_actor_id": external_actor.id,
"external_actor_name": external_actor.external_name,
"external_actor_external_id": external_actor.external_id,
}
)
_add_lifecycle_extras(lifecycle, log_context)

# Strip the @ from the username stored in external_name
gitlab_username = external_actor.external_name.lstrip("@")

# Search for the GitLab user by username to get their user ID
try:
users = client.search_users(gitlab_username)
if not users or len(users) == 0:
logger.warning(
"assignee-outbound.gitlab-user-not-found",
extra={
"provider": self.model.provider,
"integration_id": external_issue.integration_id,
"gitlab_username": gitlab_username,
},
)
return

# Take the first matching user (exact username match)
gitlab_user = users[0]
gitlab_user_id = gitlab_user["id"]

logger.info(
"assignee-outbound.gitlab-user-found",
extra={
"provider": self.model.provider,
"integration_id": external_issue.integration_id,
"gitlab_username": gitlab_username,
"gitlab_user_id": gitlab_user_id,
},
)
users = client.search_users(gitlab_username) or []
except ApiError as e:
logger.warning(
"assignee-outbound.gitlab-user-search-failed",
extra={
"provider": self.model.provider,
"integration_id": external_issue.integration_id,
"gitlab_username": gitlab_username,
"error": str(e),
},
)
log_context["error"] = str(e)
logger.warning("assignee-outbound.gitlab-user-search-failed", extra=log_context)
_add_lifecycle_extras(lifecycle, log_context)
return

usernames = [
found_user.get("username") for found_user in users[:5] if found_user.get("username")
]
log_context.update(
{
"assignee_resolution_method": "username_search",
"gitlab_search_result_count": len(users),
"gitlab_search_usernames": usernames,
}
)

if not users:
logger.warning("assignee-outbound.gitlab-user-not-found", extra=log_context)
_add_lifecycle_extras(lifecycle, log_context)
return

gitlab_user = users[0]
gitlab_user_id = gitlab_user["id"]
log_context["resolved_gitlab_user_id"] = gitlab_user_id

logger.info("assignee-outbound.gitlab-user-found", extra=log_context)
_add_lifecycle_extras(lifecycle, log_context)

# Update GitLab issue assignees
try:
# URL-encode project_id since it's a path_with_namespace (e.g., "owner/repo")
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/integrations/gitlab/webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ def _handle_assignment(
# GitLab supports multiple assignees, but Sentry currently only supports one
# Take the first assignee from the current state
first_assignee = assignees[0]
assignee_id = first_assignee.get("id")
assignee_username = first_assignee.get("username")

if not assignee_username:
Expand All @@ -264,13 +265,15 @@ def _handle_assignment(
external_user_name=assignee_name,
external_issue_key=external_issue_key,
assign=True,
external_user_id=assignee_id,
)

logger.info(
"gitlab.webhook.assignment.synced",
extra={
"integration_id": integration.id,
"external_issue_key": external_issue_key,
"assignee_id": assignee_id,
"assignee_name": assignee_name,
"total_assignees": len(assignees),
},
Expand Down
6 changes: 5 additions & 1 deletion src/sentry/integrations/tasks/sync_assignee_outbound.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,11 @@ def sync_assignee_outbound(
# 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, assignment_source=parsed_assignment_source
external_issue,
user,
assign=assign,
assignment_source=parsed_assignment_source,
lifecycle=lifecycle,
)
analytics.record(
IntegrationIssueAssigneeSyncedEvent(
Expand Down
42 changes: 32 additions & 10 deletions src/sentry/integrations/utils/sync.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import logging
from collections.abc import Iterable
from enum import StrEnum
from typing import TYPE_CHECKING

Expand Down Expand Up @@ -74,8 +75,9 @@ def _get_affected_groups(


def _handle_deassign(
groups: QuerySet[Group], integration: RpcIntegration | Integration
) -> QuerySet[Group]:
groups: Iterable[Group], integration: RpcIntegration | Integration
) -> list[Group]:
groups_deassigned: list[Group] = []
for group in groups:
if not should_sync_assignee_inbound(group.organization, integration.provider):
continue
Expand All @@ -84,11 +86,12 @@ def _handle_deassign(
group,
assignment_source=AssignmentSource.from_integration(integration),
)
return groups
groups_deassigned.append(group)
return groups_deassigned


def _handle_assign(
affected_groups: QuerySet[Group],
affected_groups: Iterable[Group],
integration: RpcIntegration | Integration,
users: list[RpcUser],
) -> list[Group]:
Expand Down Expand Up @@ -137,45 +140,64 @@ def sync_group_assignee_inbound_by_external_actor(
external_user_name: str,
external_issue_key: str | None,
assign: bool = True,
external_user_id: str | int | None = None,
) -> QuerySet[Group] | list[Group]:
logger = logging.getLogger(f"sentry.integrations.{integration.provider}")

with ProjectManagementEvent(
action_type=ProjectManagementActionType.INBOUND_ASSIGNMENT_SYNC, integration=integration
).capture() as lifecycle:
affected_groups = _get_affected_groups(integration, external_issue_key)
affected_groups = list(_get_affected_groups(integration, external_issue_key))
external_user_id_str = str(external_user_id) if external_user_id is not None else None
log_context = {
"integration_id": integration.id,
"external_user_name": external_user_name,
"external_user_id": external_user_id_str,
"issue_key": external_issue_key,
"method": AssigneeInboundSyncMethod.EXTERNAL_ACTOR.value,
"assign": assign,
"affected_group_ids": [group.id for group in affected_groups],
}
lifecycle.add_extras(log_context)

if not affected_groups:
logger.info("no-affected-groups", extra=log_context)
return []

if not assign:
return _handle_deassign(affected_groups, integration)
groups_deassigned = _handle_deassign(affected_groups, integration)
log_context["unassigned_group_ids"] = [group.id for group in groups_deassigned]
lifecycle.add_extras(log_context)
return groups_deassigned

external_actors = ExternalActor.objects.filter(
base_external_actors = ExternalActor.objects.filter(
provider=EXTERNAL_PROVIDERS_REVERSE[ExternalProviderEnum(integration.provider)].value,
external_name__iexact=external_user_name,
integration_id=integration.id,
user_id__isnull=False,
).values_list("user_id", flat=True)
)

match_method = "external_name"
external_actors = base_external_actors.filter(external_name__iexact=external_user_name)

external_actor_user_ids = list(external_actors.values_list("user_id", flat=True))
user_ids = [
external_actor for external_actor in external_actors if external_actor is not None
external_actor_user_id
for external_actor_user_id in external_actor_user_ids
if external_actor_user_id is not None
]

log_context["match_method"] = match_method
log_context["external_actor_count"] = len(user_ids)
log_context["matched_user_ids"] = user_ids
log_context["user_ids"] = user_ids
logger.info("sync_group_assignee_inbound_by_external_actor.user_ids", extra=log_context)
lifecycle.add_extras(log_context)

users = user_service.get_many_by_id(ids=user_ids)

groups_assigned = _handle_assign(affected_groups, integration, users)
log_context["assigned_group_ids"] = [group.id for group in groups_assigned]
lifecycle.add_extras(log_context)

if len(groups_assigned) != len(affected_groups):
log_context["groups_assigned_count"] = len(groups_assigned)
Expand Down
24 changes: 22 additions & 2 deletions tests/sentry/integrations/gitlab/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def _setup_assignee_sync_test(
self,
user_email: str = "foo@example.com",
external_name: str = "@gitlab_user",
external_id: str = "123",
external_id: str | None = None,
issue_key: str = "example.gitlab.com/group-x:cool-group/sentry#45",
create_external_user: bool = True,
) -> tuple:
Expand Down Expand Up @@ -402,7 +402,7 @@ def test_sync_assignee_outbound_unassign(self) -> None:
@responses.activate
@with_feature("organizations:integrations-gitlab-project-management")
def test_sync_assignee_outbound_no_external_actor(self) -> None:
"""Test that sync fails gracefully when user has no GitLab account linked"""
"""Test that sync no-ops when user has no GitLab account linked."""
user, installation, external_issue, _, _ = self._setup_assignee_sync_test(
create_external_user=False
)
Expand Down Expand Up @@ -493,6 +493,26 @@ def test_sync_assignee_outbound_user_not_found(self) -> None:
assert len(responses.calls) == 1
assert "users?username=gitlab_user" in responses.calls[0].request.url

@responses.activate
@with_feature("organizations:integrations-gitlab-project-management")
def test_sync_assignee_outbound_user_search_failed(self) -> None:
user, installation, external_issue, _, _ = self._setup_assignee_sync_test()

responses.add(
responses.GET,
"https://example.gitlab.com/api/v4/users?username=gitlab_user",
json={"message": "Internal server error"},
status=500,
)

responses.calls.reset()

with assume_test_silo_mode(SiloMode.CELL):
installation.sync_assignee_outbound(external_issue, user, assign=True)

assert len(responses.calls) == 1
assert "users?username=gitlab_user" in responses.calls[0].request.url

def test_create_comment(self) -> None:
"""Test creating a comment on a GitLab issue"""
self.user.name = "Sentry Admin"
Expand Down
25 changes: 25 additions & 0 deletions tests/sentry/integrations/gitlab/test_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,31 @@ def test_issue_assigned(self, mock_sync: MagicMock) -> None:
call_args[1]["external_issue_key"] == "example.gitlab.com/group-x:cool-group/sentry#23"
)
assert call_args[1]["assign"] is True
assert call_args[1]["external_user_id"] == 1

@patch("sentry.integrations.gitlab.webhooks.sync_group_assignee_inbound_by_external_actor")
def test_issue_assigned_with_dotted_username(self, mock_sync: MagicMock) -> None:
event = orjson.loads(ISSUE_ASSIGNED_EVENT)
event["assignees"][0]["id"] = 123
event["assignees"][0]["username"] = "first.last"

response = self.client.post(
self.url,
data=orjson.dumps(event),
content_type="application/json",
HTTP_X_GITLAB_TOKEN=WEBHOOK_TOKEN,
HTTP_X_GITLAB_EVENT="Issue Hook",
)
assert response.status_code == 204

assert mock_sync.called
call_args = mock_sync.call_args
assert call_args[1]["external_user_name"] == "@first.last"
assert call_args[1]["external_user_id"] == 123
assert (
call_args[1]["external_issue_key"] == "example.gitlab.com/group-x:cool-group/sentry#23"
)
assert call_args[1]["assign"] is True

@patch("sentry.integrations.gitlab.webhooks.sync_group_assignee_inbound_by_external_actor")
def test_issue_unassigned(self, mock_sync: MagicMock) -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ def test_syncs_outbound_assignee(
sync_assignee_outbound(self.external_issue.id, self.user.id, True, None)
mock_sync_assignee.assert_called_once()
mock_sync_assignee.assert_called_with(
self.external_issue, mock.ANY, assign=True, assignment_source=None
self.external_issue,
mock.ANY,
assign=True,
assignment_source=None,
lifecycle=mock.ANY,
)

user_arg = mock_sync_assignee.call_args_list[0][0][1]
Expand Down
Loading
Loading