diff --git a/src/sentry/integrations/gitlab/webhooks.py b/src/sentry/integrations/gitlab/webhooks.py index 0422eb1f3fdd9f..2a0e94d4c5b418 100644 --- a/src/sentry/integrations/gitlab/webhooks.py +++ b/src/sentry/integrations/gitlab/webhooks.py @@ -33,7 +33,7 @@ from sentry.integrations.utils.webhook_viewer_context import webhook_viewer_context from sentry.models.commit import Commit from sentry.models.commitauthor import CommitAuthor -from sentry.models.pullrequest import PullRequest +from sentry.models.pullrequest import PullRequest, PullRequestLifecycleState from sentry.models.repository import Repository from sentry.organizations.services.organization import organization_service from sentry.organizations.services.organization.model import RpcOrganization @@ -381,6 +381,15 @@ def _extract_issue_key( return f"{integration.metadata['domain_name']}:{path_with_namespace}#{issue_iid}" +def _map_gitlab_state_to_pullrequest_lifecycle(gitlab_state: str | None) -> str | None: + return { + "opened": PullRequestLifecycleState.OPEN, + "closed": PullRequestLifecycleState.CLOSED, + "merged": PullRequestLifecycleState.MERGED, + "locked": PullRequestLifecycleState.LOCKED, + }.get(gitlab_state or "") + + class MergeEventWebhook(GitlabWebhook): """ Handle Merge Request Hook @@ -445,9 +454,19 @@ def __call__(self, event: Mapping[str, Any], **kwargs): last_commit = event["object_attributes"]["last_commit"] author_email = None author_name = None + head_commit_sha = None if last_commit: author_email = last_commit["author"]["email"] author_name = last_commit["author"]["name"] + head_commit_sha = last_commit.get("id") + + updated_at = event["object_attributes"].get("updated_at") + merged_at = event["object_attributes"].get("merged_at") + state = _map_gitlab_state_to_pullrequest_lifecycle( + event["object_attributes"].get("state") + ) + action = event["object_attributes"].get("action") + draft = event["object_attributes"].get("work_in_progress") except KeyError as e: logger.warning( "gitlab.webhook.invalid-merge-data", @@ -475,19 +494,40 @@ def __call__(self, event: Mapping[str, Any], **kwargs): organization_id=organization.id, email=author_email, defaults={"name": author_name} )[0] + opened_at = parse_date(created_at).astimezone(timezone.utc) + state_changed_at = parse_date(updated_at).astimezone(timezone.utc) if updated_at else None + merged_at_dt = parse_date(merged_at).astimezone(timezone.utc) if merged_at else None + + defaults = { + "title": title, + "author": author, + "message": body, + "merge_commit_sha": merge_commit_sha, + "head_commit_sha": head_commit_sha, + "date_added": opened_at, + "opened_at": opened_at, + "merged_at": merged_at_dt, + "state": state, + "draft": draft, + } + + # GitLab has no closed_at, so derive it from the lifecycle action. A + # merged merge request is also closed. Actions that don't change + # lifecycle state (e.g. "update") leave the stored closed_at untouched. + if action == "merge": + defaults["closed_at"] = merged_at_dt or state_changed_at + elif action == "close": + defaults["closed_at"] = state_changed_at + elif action in ("reopen", "open"): + defaults["closed_at"] = None + author.preload_users() try: PullRequest.objects.update_or_create( organization_id=organization.id, repository_id=repo.id, key=number, - defaults={ - "title": title, - "author": author, - "message": body, - "merge_commit_sha": merge_commit_sha, - "date_added": parse_date(created_at).astimezone(timezone.utc), - }, + defaults=defaults, ) except IntegrityError: pass diff --git a/tests/sentry/integrations/gitlab/test_webhook.py b/tests/sentry/integrations/gitlab/test_webhook.py index d07a5766fe330f..91b3eb3e14d25d 100644 --- a/tests/sentry/integrations/gitlab/test_webhook.py +++ b/tests/sentry/integrations/gitlab/test_webhook.py @@ -1,3 +1,4 @@ +from datetime import datetime, timezone from unittest.mock import MagicMock, patch import orjson @@ -20,7 +21,7 @@ from sentry.models.commit import Commit from sentry.models.commitauthor import CommitAuthor from sentry.models.grouplink import GroupLink -from sentry.models.pullrequest import PullRequest +from sentry.models.pullrequest import PullRequest, PullRequestLifecycleState from sentry.seer.code_review.webhooks.merge_request import handle_merge_request_event from sentry.silo.base import SiloMode from sentry.testutils.asserts import assert_failure_metric, assert_success_metric @@ -42,6 +43,12 @@ def assert_pull_request(self, pull: PullRequest, author: CommitAuthor) -> None: assert pull.author == author assert pull.merge_commit_sha is None assert pull.organization_id == self.organization.id + assert pull.state == PullRequestLifecycleState.OPEN + assert pull.opened_at is not None + assert pull.closed_at is None + assert pull.merged_at is None + assert pull.head_commit_sha == "ba3e0d8ff79c80d5b0bbb4f3e2e343e0aaa662b7" + assert pull.draft is False def assert_group_link(self, group, pull): link = GroupLink.objects.get() @@ -453,6 +460,81 @@ def test_merge_event_update_pull_request(self) -> None: self.assert_pull_request(pull, author) self.assert_group_link(group, pull) + def test_merge_event_merged_pull_request_state(self) -> None: + self.create_gitlab_repo("getsentry/sentry") + self.create_group(project=self.project, short_id=9) + + payload = orjson.loads(MERGE_REQUEST_OPENED_EVENT) + payload["object_attributes"]["state"] = "merged" + payload["object_attributes"]["action"] = "merge" + payload["object_attributes"]["merge_commit_sha"] = "abc123" + payload["object_attributes"]["merged_at"] = "2017-09-28T12:23:42.365Z" + + response = self.client.post( + self.url, + data=orjson.dumps(payload), + content_type="application/json", + HTTP_X_GITLAB_TOKEN=WEBHOOK_TOKEN, + HTTP_X_GITLAB_EVENT="Merge Request Hook", + ) + assert response.status_code == 204 + + pull = PullRequest.objects.get() + assert pull.state == PullRequestLifecycleState.MERGED + assert pull.merge_commit_sha == "abc123" + # merged_at is taken directly from the webhook's object_attributes. + assert pull.merged_at == datetime(2017, 9, 28, 12, 23, 42, 365000, tzinfo=timezone.utc) + # A merged merge request is also closed. + assert pull.closed_at == pull.merged_at + + def test_merge_event_edit_after_merge_preserves_terminal_timestamps(self) -> None: + self.create_gitlab_repo("getsentry/sentry") + + # Merge the merge request, stamping merged_at from the reported + # merged_at and closed_at from the same value. + merge_payload = orjson.loads(MERGE_REQUEST_OPENED_EVENT) + merge_payload["object_attributes"]["state"] = "merged" + merge_payload["object_attributes"]["action"] = "merge" + merge_payload["object_attributes"]["merged_at"] = "2017-09-28T12:23:42.365Z" + self.client.post( + self.url, + data=orjson.dumps(merge_payload), + content_type="application/json", + HTTP_X_GITLAB_TOKEN=WEBHOOK_TOKEN, + HTTP_X_GITLAB_EVENT="Merge Request Hook", + ) + + pull = PullRequest.objects.get() + merged_at = pull.merged_at + closed_at = pull.closed_at + assert merged_at is not None + assert closed_at is not None + + # GitLab fires the hook again for an edit (e.g. label change) with a + # later updated_at while the state is still "merged". The edit carries + # the same absolute merged_at, and an "update" action never touches + # closed_at, so the terminal timestamps must not drift to the edit time. + edit_payload = orjson.loads(MERGE_REQUEST_OPENED_EVENT) + edit_payload["object_attributes"]["state"] = "merged" + edit_payload["object_attributes"]["action"] = "update" + edit_payload["object_attributes"]["updated_at"] = "2018-01-01T00:00:00.000Z" + edit_payload["object_attributes"]["merged_at"] = "2017-09-28T12:23:42.365Z" + edit_payload["object_attributes"]["title"] = "Edited after merge" + response = self.client.post( + self.url, + data=orjson.dumps(edit_payload), + content_type="application/json", + HTTP_X_GITLAB_TOKEN=WEBHOOK_TOKEN, + HTTP_X_GITLAB_EVENT="Merge Request Hook", + ) + assert response.status_code == 204 + + pull.refresh_from_db() + assert pull.title == "Edited after merge" + assert pull.state == PullRequestLifecycleState.MERGED + assert pull.merged_at == merged_at + assert pull.closed_at == closed_at + def test_update_repo_path(self) -> None: repo_out_of_date_path = self.create_gitlab_repo( name="Cool Group / Sentry", url="http://example.com/cool-group/sentry"