From 129178f57e3a02d3f66369751af28774bad2e9e1 Mon Sep 17 00:00:00 2001 From: Cathy Teng Date: Tue, 16 Jul 2024 10:39:24 -0700 Subject: [PATCH] pull request file issues API --- .../organization_pull_request_file_issues.py | 95 +++++++++++++++++ src/sentry/api/urls.py | 8 ++ .../integrations/github/open_pr_comment.py | 32 ++++-- ...t_organization_pull_request_file_issues.py | 100 ++++++++++++++++++ 4 files changed, 226 insertions(+), 9 deletions(-) create mode 100644 src/sentry/api/endpoints/organization_pull_request_file_issues.py create mode 100644 tests/sentry/api/endpoints/test_organization_pull_request_file_issues.py diff --git a/src/sentry/api/endpoints/organization_pull_request_file_issues.py b/src/sentry/api/endpoints/organization_pull_request_file_issues.py new file mode 100644 index 0000000000000..27225c64f646b --- /dev/null +++ b/src/sentry/api/endpoints/organization_pull_request_file_issues.py @@ -0,0 +1,95 @@ +from __future__ import annotations + +from rest_framework import serializers, status +from rest_framework.request import Request +from rest_framework.response import Response + +from sentry.api.api_owners import ApiOwner +from sentry.api.api_publish_status import ApiPublishStatus +from sentry.api.base import region_silo_endpoint +from sentry.api.bases.organization import OrganizationEndpoint +from sentry.models.group import Group +from sentry.models.organization import Organization +from sentry.snuba.referrer import Referrer +from sentry.tasks.integrations.github.language_parsers import PATCH_PARSERS +from sentry.tasks.integrations.github.open_pr_comment import ( + get_projects_and_filenames_from_source_file, + get_top_5_issues_by_count_for_file, +) + + +class PullRequestFileSerializer(serializers.Serializer): + filename = serializers.CharField(required=True) + repo = serializers.CharField(required=True) + patch = serializers.CharField(required=True) + + def validate_filename(self, value): + if not value: + raise serializers.ValidationError("Filename is required") + + file_extension = value.split(".")[-1] + language_parser = PATCH_PARSERS.get(file_extension, None) + if not language_parser: + raise serializers.ValidationError("Unsupported file type") + + return value + + +@region_silo_endpoint +class OrganizationPullRequestFilesIssuesEndpoint(OrganizationEndpoint): + owner = ApiOwner.ECOSYSTEM + publish_status = { + "GET": ApiPublishStatus.EXPERIMENTAL, + } + + def post(self, request: Request, organization: Organization) -> Response: + serializer = PullRequestFileSerializer(data=request.data) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + filename = serializer.validated_data["filename"] + repo_name = serializer.validated_data["repo"] + patch = serializer.validated_data["patch"] + + projects, sentry_filenames = get_projects_and_filenames_from_source_file( + org_id=organization.id, repo_name=repo_name, pr_filename=filename + ) + + if not len(projects) or not len(sentry_filenames): + return Response([]) + + file_extension = filename.split(".")[-1] + language_parser = PATCH_PARSERS[file_extension] + + function_names = language_parser.extract_functions_from_patch(patch) + + if not len(function_names): + return Response([]) + + top_issues = get_top_5_issues_by_count_for_file( + list(projects), list(sentry_filenames), list(function_names) + ) + + group_id_to_info = {} + for issue in top_issues: + group_id = issue["group_id"] + group_id_to_info[group_id] = dict(filter(lambda k: k[0] != "group_id", issue.items())) + + issues = Group.objects.filter(id__in=list(group_id_to_info.keys())).all() + + pr_file_issues = [ + { + "title": issue.title, + "culprit": issue.culprit, + "url": issue.get_absolute_url(), + "affected_users": issue.count_users_seen( + referrer=Referrer.TAGSTORE_GET_GROUPS_USER_COUNTS_OPEN_PR_COMMENT.value + ), + "event_count": group_id_to_info[issue.id]["event_count"], + "function_name": group_id_to_info[issue.id]["function_name"], + } + for issue in issues + ] + pr_file_issues.sort(key=lambda k: k.get("event_count", 0), reverse=True) + + return Response(pr_file_issues) diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index 518eb2e19d8cb..4bb36c4d96089 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -22,6 +22,9 @@ from sentry.api.endpoints.organization_projects_experiment import ( OrganizationProjectsExperimentEndpoint, ) +from sentry.api.endpoints.organization_pull_request_file_issues import ( + OrganizationPullRequestFilesIssuesEndpoint, +) from sentry.api.endpoints.organization_spans_aggregation import OrganizationSpansAggregationEndpoint from sentry.api.endpoints.organization_stats_summary import OrganizationStatsSummaryEndpoint from sentry.api.endpoints.organization_unsubscribe import ( @@ -1827,6 +1830,11 @@ def create_group_urls(name_prefix: str) -> list[URLPattern | URLResolver]: OrganizationPluginsConfigsEndpoint.as_view(), name="sentry-api-0-organization-plugins-configs", ), + re_path( + r"^(?P[^\/]+)/pr-file-issues/$", + OrganizationPullRequestFilesIssuesEndpoint.as_view(), + name="sentry-api-0-organization-pr-file-issues", + ), re_path( r"^(?P[^\/]+)/releases/$", OrganizationReleasesEndpoint.as_view(), diff --git a/src/sentry/tasks/integrations/github/open_pr_comment.py b/src/sentry/tasks/integrations/github/open_pr_comment.py index 12babe64eabf9..e4a363d841ae4 100644 --- a/src/sentry/tasks/integrations/github/open_pr_comment.py +++ b/src/sentry/tasks/integrations/github/open_pr_comment.py @@ -241,18 +241,32 @@ def get_pr_files(pr_files: list[dict[str, str]]) -> list[PullRequestFile]: def get_projects_and_filenames_from_source_file( org_id: int, - repo_id: int, pr_filename: str, + repo_id: int | None = None, + repo_name: str | None = None, ) -> tuple[set[Project], set[str]]: # fetch the code mappings in which the source_root is a substring at the start of pr_filename - code_mappings = ( - RepositoryProjectPathConfig.objects.filter( - organization_id=org_id, - repository_id=repo_id, + if not repo_id and not repo_name: + raise ValueError("repository ID or repository name are required") + + if repo_id: + code_mappings = ( + RepositoryProjectPathConfig.objects.filter( + organization_id=org_id, + repository_id=repo_id, + ) + .annotate(substring_match=StrIndex(Value(pr_filename), "source_root")) + .filter(substring_match=1) + ) + else: + code_mappings = ( + RepositoryProjectPathConfig.objects.filter( + organization_id=org_id, + repository__name=repo_name, + ) + .annotate(substring_match=StrIndex(Value(pr_filename), "source_root")) + .filter(substring_match=1) ) - .annotate(substring_match=StrIndex(Value(pr_filename), "source_root")) - .filter(substring_match=1) - ) project_list: set[Project] = set() sentry_filenames = set() @@ -476,7 +490,7 @@ def open_pr_comment_workflow(pr_id: int) -> None: # fetch issues related to the files for file in pullrequest_files: projects, sentry_filenames = get_projects_and_filenames_from_source_file( - org_id, repo.id, file.filename + org_id=org_id, repo_id=repo.id, pr_filename=file.filename ) if not len(projects) or not len(sentry_filenames): continue diff --git a/tests/sentry/api/endpoints/test_organization_pull_request_file_issues.py b/tests/sentry/api/endpoints/test_organization_pull_request_file_issues.py new file mode 100644 index 0000000000000..58874aae1c4ae --- /dev/null +++ b/tests/sentry/api/endpoints/test_organization_pull_request_file_issues.py @@ -0,0 +1,100 @@ +from unittest.mock import patch + +from sentry.models.group import Group +from sentry.testutils.cases import APITestCase +from tests.sentry.tasks.integrations.github.test_open_pr_comment import CreateEventTestCase + + +@patch( + "sentry.api.endpoints.organization_pull_request_file_issues.get_projects_and_filenames_from_source_file" +) +class OrganizationPullRequestFileIssuesTest(APITestCase, CreateEventTestCase): + endpoint = "sentry-api-0-organization-pr-file-issues" + method = "post" + + def setUp(self): + super().setUp() + self.user_id = "user_1" + self.app_id = "app_1" + + self.group_id_1 = [ + self._create_event( + culprit="issue1", + user_id=str(i), + filenames=["bar.py", "foo.py"], + function_names=["planet", "blue"], + ) + for i in range(5) + ][0].group.id + self.group_id_2 = [ + self._create_event( + culprit="issue2", + filenames=["foo.py", "bar.py"], + function_names=["blue", "planet"], + user_id=str(i), + ) + for i in range(6) + ][0].group.id + + self.gh_repo = self.create_repo( + name="getsentry/sentry", + provider="integrations:github", + integration_id=self.integration.id, + project=self.project, + url="https://github.com/getsentry/sentry", + ) + self.groups = [ + { + "group_id": g.id, + "event_count": 1000 * (i + 1), + "function_name": "function_" + str(i), + } + for i, g in enumerate(Group.objects.all()) + ] + self.groups.reverse() + + self.login_as(self.user) + + def test_simple(self, mock_reverse_codemappings): + mock_reverse_codemappings.return_value = ([self.project], ["foo.py"]) + + patch = """@@ -36,6 +36,7 @@\n def blue(self):""" + response = self.get_success_response( + self.organization.slug, + **{"filename": "foo.py", "repo": self.gh_repo.name, "patch": patch}, + ) + group_urls = [g["url"] for g in response.data] + assert group_urls == [ + f"http://testserver/organizations/{self.organization.slug}/issues/{self.group_id_2}/", + f"http://testserver/organizations/{self.organization.slug}/issues/{self.group_id_1}/", + ] + + def test_no_codemappings(self, mock_reverse_codemappings): + mock_reverse_codemappings.return_value = ([], []) + + patch = """@@ -36,6 +36,7 @@\n def blue(self):""" + response = self.get_success_response( + self.organization.slug, + **{"filename": "foo.py", "repo": self.gh_repo.name, "patch": patch}, + ) + assert response.data == [] + + def test_no_functions(self, mock_reverse_codemappings): + mock_reverse_codemappings.return_value = ([self.project], ["foo.py"]) + + patch = """@@ -36,6 +36,7 @@\n import pytest""" + response = self.get_success_response( + self.organization.slug, + **{"filename": "foo.py", "repo": self.gh_repo.name, "patch": patch}, + ) + assert response.data == [] + + def test_no_issues(self, mock_reverse_codemappings): + mock_reverse_codemappings.return_value = ([self.project], ["foo.py"]) + + patch = """@@ -36,6 +36,7 @@\n def purple(self):""" + response = self.get_success_response( + self.organization.slug, + **{"filename": "bar.py", "repo": self.gh_repo.name, "patch": patch}, + ) + assert response.data == []