Skip to content

Commit b83b092

Browse files
authored
feat(github-comments): Don't comment info-level issues on merged PRs (#66110)
1 parent 0daa386 commit b83b092

File tree

4 files changed

+102
-1
lines changed

4 files changed

+102
-1
lines changed

src/sentry/tasks/commit_context.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from sentry.locks import locks
2222
from sentry.models.commit import Commit
2323
from sentry.models.commitauthor import CommitAuthor
24+
from sentry.models.group import Group
2425
from sentry.models.groupowner import GroupOwner, GroupOwnerType
2526
from sentry.models.options.organization_option import OrganizationOption
2627
from sentry.models.project import Project
@@ -280,8 +281,10 @@ def process_commit_context(
280281
extra={"organization_id": project.organization_id},
281282
)
282283
repo = Repository.objects.filter(id=commit.repository_id)
284+
group = Group.objects.get_from_cache(id=group_id)
283285
if (
284-
installation is not None
286+
group.level is not logging.INFO # Don't comment on info level issues
287+
and installation is not None
285288
and repo.exists()
286289
and repo.get().provider == "integrations:github"
287290
):

src/sentry/tasks/integrations/github/pr_comment.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ def get_top_5_issues_by_count(issue_list: list[int], project: Project) -> list[d
113113
Condition(Column("group_id"), Op.IN, issue_list),
114114
Condition(Column("timestamp"), Op.GTE, datetime.now() - timedelta(days=30)),
115115
Condition(Column("timestamp"), Op.LT, datetime.now()),
116+
Condition(Column("level"), Op.NEQ, "info"),
116117
]
117118
)
118119
.set_orderby([OrderBy(Column("event_count"), Direction.DESC)])

tests/sentry/tasks/integrations/github/test_pr_comment.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import logging
12
from datetime import datetime, timedelta, timezone
23
from unittest.mock import patch
34

@@ -267,6 +268,76 @@ def test_over_5_issues(self):
267268
res = get_top_5_issues_by_count(issue_ids, self.project)
268269
assert len(res) == 5
269270

271+
def test_ignore_info_level_issues(self):
272+
group1 = [
273+
self.store_event(
274+
{
275+
"fingerprint": ["group-1"],
276+
"timestamp": iso_format(before_now(days=1)),
277+
"level": logging.INFO,
278+
},
279+
project_id=self.project.id,
280+
)
281+
for _ in range(3)
282+
][0].group.id
283+
group2 = [
284+
self.store_event(
285+
{"fingerprint": ["group-2"], "timestamp": iso_format(before_now(days=1))},
286+
project_id=self.project.id,
287+
)
288+
for _ in range(6)
289+
][0].group.id
290+
group3 = [
291+
self.store_event(
292+
{
293+
"fingerprint": ["group-3"],
294+
"timestamp": iso_format(before_now(days=1)),
295+
"level": logging.INFO,
296+
},
297+
project_id=self.project.id,
298+
)
299+
for _ in range(4)
300+
][0].group.id
301+
res = get_top_5_issues_by_count([group1, group2, group3], self.project)
302+
assert [issue["group_id"] for issue in res] == [group2]
303+
304+
def test_do_not_ignore_other_issues(self):
305+
group1 = [
306+
self.store_event(
307+
{
308+
"fingerprint": ["group-1"],
309+
"timestamp": iso_format(before_now(days=1)),
310+
"level": logging.ERROR,
311+
},
312+
project_id=self.project.id,
313+
)
314+
for _ in range(3)
315+
][0].group.id
316+
group2 = [
317+
self.store_event(
318+
{
319+
"fingerprint": ["group-2"],
320+
"timestamp": iso_format(before_now(days=1)),
321+
"level": logging.INFO,
322+
},
323+
project_id=self.project.id,
324+
)
325+
for _ in range(6)
326+
][0].group.id
327+
group3 = [
328+
self.store_event(
329+
{
330+
"fingerprint": ["group-3"],
331+
"timestamp": iso_format(before_now(days=1)),
332+
"level": logging.DEBUG,
333+
},
334+
project_id=self.project.id,
335+
)
336+
for _ in range(4)
337+
][0].group.id
338+
res = get_top_5_issues_by_count([group1, group2, group3], self.project)
339+
assert [issue["group_id"] for issue in res] == [group3, group1]
340+
270341

271342
@region_silo_test
272343
class TestCommentBuilderQueries(GithubCommentTestCase):

tests/sentry/tasks/test_commit_context.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import logging
12
from datetime import datetime, timedelta
23
from datetime import timezone as datetime_timezone
34
from unittest.mock import patch
@@ -996,6 +997,31 @@ def test_gh_comment_pr_too_old(self, get_jwt, mock_comment_workflow, mock_get_co
996997
assert not mock_comment_workflow.called
997998
assert len(PullRequestCommit.objects.all()) == 0
998999

1000+
@patch("sentry.integrations.github.client.get_jwt", return_value=b"jwt_token_1")
1001+
@responses.activate
1002+
def test_gh_comment_pr_info_level_issue(
1003+
self, get_jwt, mock_comment_workflow, mock_get_commit_context
1004+
):
1005+
"""No comment on pr that's has info level issue"""
1006+
mock_get_commit_context.return_value = [self.blame]
1007+
self.pull_request.date_added = before_now(days=1)
1008+
self.pull_request.save()
1009+
1010+
self.add_responses()
1011+
self.event.group.update(level=logging.INFO)
1012+
1013+
with self.tasks():
1014+
event_frames = get_frame_paths(self.event)
1015+
process_commit_context(
1016+
event_id=self.event.event_id,
1017+
event_platform=self.event.platform,
1018+
event_frames=event_frames,
1019+
group_id=self.event.group_id,
1020+
project_id=self.event.project_id,
1021+
)
1022+
assert not mock_comment_workflow.called
1023+
assert len(PullRequestCommit.objects.all()) == 0
1024+
9991025
@patch("sentry.integrations.github.client.get_jwt", return_value=b"jwt_token_1")
10001026
@responses.activate
10011027
def test_gh_comment_repeat_issue(self, get_jwt, mock_comment_workflow, mock_get_commit_context):

0 commit comments

Comments
 (0)