From 1ca0278cee0e8ffa7af976e09aaa11a89b91829c Mon Sep 17 00:00:00 2001 From: Raj Joshi Date: Thu, 29 Feb 2024 14:16:38 -0800 Subject: [PATCH 1/3] prevent comment workflow trigger & wrote tests --- src/sentry/tasks/commit_context.py | 5 +++- tests/sentry/tasks/test_commit_context.py | 32 +++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/sentry/tasks/commit_context.py b/src/sentry/tasks/commit_context.py index 58c0830aff1b79..2af30223c97d76 100644 --- a/src/sentry/tasks/commit_context.py +++ b/src/sentry/tasks/commit_context.py @@ -21,6 +21,7 @@ from sentry.locks import locks from sentry.models.commit import Commit from sentry.models.commitauthor import CommitAuthor +from sentry.models.group import Group from sentry.models.groupowner import GroupOwner, GroupOwnerType from sentry.models.options.organization_option import OrganizationOption from sentry.models.project import Project @@ -280,8 +281,10 @@ def process_commit_context( extra={"organization_id": project.organization_id}, ) repo = Repository.objects.filter(id=commit.repository_id) + group = Group.objects.get_from_cache(id=group_id) if ( - installation is not None + group.level is not logging.INFO # Don't comment on info level issues + and installation is not None and repo.exists() and repo.get().provider == "integrations:github" ): diff --git a/tests/sentry/tasks/test_commit_context.py b/tests/sentry/tasks/test_commit_context.py index 2f6fe50f626161..0ffe4ff0f0bea7 100644 --- a/tests/sentry/tasks/test_commit_context.py +++ b/tests/sentry/tasks/test_commit_context.py @@ -1,3 +1,4 @@ +import logging from datetime import datetime, timedelta from datetime import timezone as datetime_timezone from unittest.mock import patch @@ -11,6 +12,7 @@ from sentry.integrations.mixins.commit_context import CommitInfo, FileBlameInfo, SourceLineInfo from sentry.models.commit import Commit from sentry.models.commitauthor import CommitAuthor +from sentry.models.group import Group from sentry.models.groupowner import GroupOwner, GroupOwnerType from sentry.models.options.organization_option import OrganizationOption from sentry.models.pullrequest import ( @@ -996,6 +998,36 @@ def test_gh_comment_pr_too_old(self, get_jwt, mock_comment_workflow, mock_get_co assert not mock_comment_workflow.called assert len(PullRequestCommit.objects.all()) == 0 + @patch("sentry.integrations.github.client.get_jwt", return_value=b"jwt_token_1") + @responses.activate + def test_gh_comment_pr_info_level_issue( + self, get_jwt, mock_comment_workflow, mock_get_commit_context + ): + """No comment on pr that's has info level issue""" + mock_get_commit_context.return_value = [self.blame] + self.pull_request.date_added = before_now(days=1) + self.pull_request.save() + + self.add_responses() + + group = Group.objects.get_from_cache(id=self.event.group_id) + Group.update(group, level=logging.INFO) + + with self.tasks(): + event_frames = get_frame_paths(self.event) + process_commit_context( + event_id=self.event.event_id, + event_platform=self.event.platform, + event_frames=event_frames, + group_id=self.event.group_id, + project_id=self.event.project_id, + ) + assert not mock_comment_workflow.called + assert len(PullRequestCommit.objects.all()) == 0 + + # Reset group level + Group.update(group, level=logging.ERROR) + @patch("sentry.integrations.github.client.get_jwt", return_value=b"jwt_token_1") @responses.activate def test_gh_comment_repeat_issue(self, get_jwt, mock_comment_workflow, mock_get_commit_context): From d68733833545c8dedd2b36f2616054c26b1d20c8 Mon Sep 17 00:00:00 2001 From: Raj Joshi Date: Thu, 29 Feb 2024 15:14:14 -0800 Subject: [PATCH 2/3] modifed snuba query & wrote test --- .../tasks/integrations/github/pr_comment.py | 1 + .../integrations/github/test_pr_comment.py | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/sentry/tasks/integrations/github/pr_comment.py b/src/sentry/tasks/integrations/github/pr_comment.py index 5b276719a21c08..f1967be6d5ed77 100644 --- a/src/sentry/tasks/integrations/github/pr_comment.py +++ b/src/sentry/tasks/integrations/github/pr_comment.py @@ -113,6 +113,7 @@ def get_top_5_issues_by_count(issue_list: list[int], project: Project) -> list[d Condition(Column("group_id"), Op.IN, issue_list), Condition(Column("timestamp"), Op.GTE, datetime.now() - timedelta(days=30)), Condition(Column("timestamp"), Op.LT, datetime.now()), + Condition(Column("level"), Op.NEQ, "info"), ] ) .set_orderby([OrderBy(Column("event_count"), Direction.DESC)]) diff --git a/tests/sentry/tasks/integrations/github/test_pr_comment.py b/tests/sentry/tasks/integrations/github/test_pr_comment.py index d26ccf7699d126..afa015a1f0ff85 100644 --- a/tests/sentry/tasks/integrations/github/test_pr_comment.py +++ b/tests/sentry/tasks/integrations/github/test_pr_comment.py @@ -1,3 +1,4 @@ +import logging from datetime import datetime, timedelta, timezone from unittest.mock import patch @@ -267,6 +268,39 @@ def test_over_5_issues(self): res = get_top_5_issues_by_count(issue_ids, self.project) assert len(res) == 5 + def test_ignore_info_level_issues(self): + group1 = [ + self.store_event( + { + "fingerprint": ["group-1"], + "timestamp": iso_format(before_now(days=1)), + "level": logging.INFO, + }, + project_id=self.project.id, + ) + for _ in range(3) + ][0].group.id + group2 = [ + self.store_event( + {"fingerprint": ["group-2"], "timestamp": iso_format(before_now(days=1))}, + project_id=self.project.id, + ) + for _ in range(6) + ][0].group.id + group3 = [ + self.store_event( + { + "fingerprint": ["group-3"], + "timestamp": iso_format(before_now(days=1)), + "level": logging.INFO, + }, + project_id=self.project.id, + ) + for _ in range(4) + ][0].group.id + res = get_top_5_issues_by_count([group1, group2, group3], self.project) + assert [issue["group_id"] for issue in res] == [group2] + @region_silo_test class TestCommentBuilderQueries(GithubCommentTestCase): From b09469e188b4fb6de28bb734369f836d06e878d2 Mon Sep 17 00:00:00 2001 From: Raj Joshi Date: Thu, 29 Feb 2024 15:34:06 -0800 Subject: [PATCH 3/3] addressed comments on pr --- .../integrations/github/test_pr_comment.py | 37 +++++++++++++++++++ tests/sentry/tasks/test_commit_context.py | 8 +--- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/tests/sentry/tasks/integrations/github/test_pr_comment.py b/tests/sentry/tasks/integrations/github/test_pr_comment.py index afa015a1f0ff85..c64dff50b09b12 100644 --- a/tests/sentry/tasks/integrations/github/test_pr_comment.py +++ b/tests/sentry/tasks/integrations/github/test_pr_comment.py @@ -301,6 +301,43 @@ def test_ignore_info_level_issues(self): res = get_top_5_issues_by_count([group1, group2, group3], self.project) assert [issue["group_id"] for issue in res] == [group2] + def test_do_not_ignore_other_issues(self): + group1 = [ + self.store_event( + { + "fingerprint": ["group-1"], + "timestamp": iso_format(before_now(days=1)), + "level": logging.ERROR, + }, + project_id=self.project.id, + ) + for _ in range(3) + ][0].group.id + group2 = [ + self.store_event( + { + "fingerprint": ["group-2"], + "timestamp": iso_format(before_now(days=1)), + "level": logging.INFO, + }, + project_id=self.project.id, + ) + for _ in range(6) + ][0].group.id + group3 = [ + self.store_event( + { + "fingerprint": ["group-3"], + "timestamp": iso_format(before_now(days=1)), + "level": logging.DEBUG, + }, + project_id=self.project.id, + ) + for _ in range(4) + ][0].group.id + res = get_top_5_issues_by_count([group1, group2, group3], self.project) + assert [issue["group_id"] for issue in res] == [group3, group1] + @region_silo_test class TestCommentBuilderQueries(GithubCommentTestCase): diff --git a/tests/sentry/tasks/test_commit_context.py b/tests/sentry/tasks/test_commit_context.py index 0ffe4ff0f0bea7..c9576fb87fa664 100644 --- a/tests/sentry/tasks/test_commit_context.py +++ b/tests/sentry/tasks/test_commit_context.py @@ -12,7 +12,6 @@ from sentry.integrations.mixins.commit_context import CommitInfo, FileBlameInfo, SourceLineInfo from sentry.models.commit import Commit from sentry.models.commitauthor import CommitAuthor -from sentry.models.group import Group from sentry.models.groupowner import GroupOwner, GroupOwnerType from sentry.models.options.organization_option import OrganizationOption from sentry.models.pullrequest import ( @@ -1009,9 +1008,7 @@ def test_gh_comment_pr_info_level_issue( self.pull_request.save() self.add_responses() - - group = Group.objects.get_from_cache(id=self.event.group_id) - Group.update(group, level=logging.INFO) + self.event.group.update(level=logging.INFO) with self.tasks(): event_frames = get_frame_paths(self.event) @@ -1025,9 +1022,6 @@ def test_gh_comment_pr_info_level_issue( assert not mock_comment_workflow.called assert len(PullRequestCommit.objects.all()) == 0 - # Reset group level - Group.update(group, level=logging.ERROR) - @patch("sentry.integrations.github.client.get_jwt", return_value=b"jwt_token_1") @responses.activate def test_gh_comment_repeat_issue(self, get_jwt, mock_comment_workflow, mock_get_commit_context):