Skip to content

Build overview: don't create comment for projects with empty diffs #12291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 15, 2025
Merged
Show file tree
Hide file tree
Changes from 5 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
17 changes: 15 additions & 2 deletions readthedocs/builds/reporting.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
from dataclasses import dataclass

from django.conf import settings
from django.template.loader import render_to_string

from readthedocs.builds.models import Build
from readthedocs.filetreediff import get_diff
from readthedocs.filetreediff.dataclasses import FileTreeDiff


@dataclass
class BuildOverview:
content: str
diff: FileTreeDiff

def get_build_overview(build: Build) -> str | None:

def get_build_overview(build: Build) -> BuildOverview | None:
"""
Generate a build overview for the given build.

Expand All @@ -27,7 +36,7 @@ def get_build_overview(build: Build) -> str | None:
if not diff:
return None

return render_to_string(
content = render_to_string(
"core/build-overview.md",
{
"PRODUCTION_DOMAIN": settings.PRODUCTION_DOMAIN,
Expand All @@ -39,3 +48,7 @@ def get_build_overview(build: Build) -> str | None:
"diff": diff,
},
)
return BuildOverview(
content=content,
diff=diff,
)
18 changes: 15 additions & 3 deletions readthedocs/builds/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,17 @@ def send_build_status(build_pk, commit, status):

@app.task(max_retries=3, default_retry_delay=60, queue="web")
def post_build_overview(build_pk):
"""
Post an overview about the build to the project's Git service.

The overview contains information about the build,
and the list of files that were changed in the build.

If no files changed in the build,
we only post the build overview if there is an existng comment.

Only GitHub is supported at the moment.
"""
build = Build.objects.filter(pk=build_pk).first()
if not build:
return
Expand All @@ -455,15 +466,16 @@ def post_build_overview(build_pk):
log.debug("Git service doesn't support creating comments.")
return

comment = get_build_overview(build)
if not comment:
build_overview = get_build_overview(build)
if not build_overview:
log.debug("No build overview available, skipping posting comment.")
return

for service in service_class.for_project(build.project):
service.post_comment(
build=build,
comment=comment,
comment=build_overview.content,
update_only=not build_overview.diff.files,
)
log.debug("PR comment posted successfully.")
return
Expand Down
3 changes: 3 additions & 0 deletions readthedocs/builds/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ def test_post_build_overview(self, get_diff, post_comment):
post_comment.assert_called_once_with(
build=self.current_version_build,
comment=expected_comment,
update_only=False,
)

@mock.patch.object(GitHubAppService, "post_comment")
Expand Down Expand Up @@ -280,6 +281,7 @@ def test_post_build_overview_more_than_5_files(self, get_diff, post_comment):
post_comment.assert_called_once_with(
build=self.current_version_build,
comment=expected_comment,
update_only=False,
)

@mock.patch.object(GitHubAppService, "post_comment")
Expand Down Expand Up @@ -312,6 +314,7 @@ def test_post_build_overview_no_files_changed(self, get_diff, post_comment):
post_comment.assert_called_once_with(
build=self.current_version_build,
comment=expected_comment,
update_only=True,
)

@mock.patch.object(GitHubAppService, "post_comment")
Expand Down
9 changes: 7 additions & 2 deletions readthedocs/oauth/services/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,13 @@ def get_clone_token(self, project):
"""Get a token used for cloning the repository."""
raise NotImplementedError

def post_comment(self, build, comment: str):
"""Post a comment on the pull request attached to the build."""
def post_comment(self, build, comment: str, update_only: bool = False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this didn't get updated?

"""
Post a comment on the pull request attached to the build.

:param update_only: If True, only update an existing comment,
don't create a new one.
"""
raise NotImplementedError

@classmethod
Expand Down
10 changes: 8 additions & 2 deletions readthedocs/oauth/services/githubapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ def update_webhook(self, project, integration=None) -> bool:
"""When using a GitHub App, we don't need to set up a webhook."""
return True

def post_comment(self, build, comment: str):
def post_comment(self, build, comment: str, update_only: bool = False):
"""
Post a comment on the pull request attached to the build.

Expand Down Expand Up @@ -545,5 +545,11 @@ def post_comment(self, build, comment: str):
comment = f"{comment_marker}\n{comment}"
if existing_gh_comment:
existing_gh_comment.edit(body=comment)
else:
elif not update_only:
gh_issue.create_comment(body=comment)
else:
log.debug(
"No comment to update, skipping commenting",
project=project.slug,
build=build.pk,
)
67 changes: 67 additions & 0 deletions readthedocs/rtd_tests/tests/test_oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,73 @@ def test_post_comment(self, request):
"body": f"<!-- readthedocs-{another_project.id} -->\nComment from another project.",
}

@requests_mock.Mocker(kw="request")
def test_post_comment_update_only(self, request):
version = get(
Version,
verbose_name="1234",
project=self.project,
type=EXTERNAL,
)
build = get(
Build,
project=self.project,
version=version,
)

request.post(
f"{self.api_url}/app/installations/1111/access_tokens",
json=self._get_access_token_json(),
)
request.get(
f"{self.api_url}/repositories/{self.remote_repository.remote_id}/issues/{version.verbose_name}",
json=self._get_pull_request_json(
number=int(version.verbose_name),
repo_full_name=self.remote_repository.full_name,
),
)
request.get(
f"{self.api_url}/repos/{self.remote_repository.full_name}/issues/{version.verbose_name}/comments",
json=[],
)
request_post_comment = request.post(
f"{self.api_url}/repos/{self.remote_repository.full_name}/issues/{version.verbose_name}/comments",
)

service = self.installation.service

# No comments exist, so it will not create a new one.
service.post_comment(build, "Comment!", update_only=True)
assert not request_post_comment.called

request.get(
f"{self.api_url}/repos/{self.remote_repository.full_name}/issues/{version.verbose_name}/comments",
json=[
self._get_comment_json(
id=1,
issue_number=int(version.verbose_name),
repo_full_name=self.remote_repository.full_name,
user={"login": f"{settings.GITHUB_APP_NAME}[bot]"},
body=f"<!-- readthedocs-{self.project.id} -->\nComment!",
),
],
)
request_patch_comment = request.patch(
f"{self.api_url}/repos/{self.remote_repository.full_name}/issues/comments/1",
json={},
)

request_post_comment.reset()

# A comment exists from the bot, so it will update it.
service.post_comment(build, "Comment!", update_only=True)
assert not request_post_comment.called

assert request_patch_comment.called
assert request_patch_comment.last_request.json() == {
"body": f"<!-- readthedocs-{self.project.id} -->\nComment!",
}

def test_integration_attributes(self):
assert self.integration.is_active
assert self.integration.get_absolute_url() == "https://github.com/apps/readthedocs/installations/1111"
Expand Down