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

Author: stsewd

readthedocs/builds/reporting.py

@@ -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.
 
@@ -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,
@@ -39,3 +48,7 @@ def get_build_overview(build: Build) -> str | None:
             "diff": diff,
         },
     )
+    return BuildOverview(
+        content=content,
+        diff=diff,
+    )

readthedocs/builds/tasks.py

@@ -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 update the build overview if there is an existing comment.
+
+    Only GitHub is supported at the moment.
+    """
     build = Build.objects.filter(pk=build_pk).first()
     if not build:
         return
@@ -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,
+            create_new=bool(build_overview.diff.files),
         )
         log.debug("PR comment posted successfully.")
         return

readthedocs/builds/tests/test_tasks.py

@@ -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,
+            create_new=True,
         )
 
     @mock.patch.object(GitHubAppService, "post_comment")
@@ -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,
+            create_new=True,
         )
 
     @mock.patch.object(GitHubAppService, "post_comment")
@@ -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,
+            create_new=False,
         )
 
     @mock.patch.object(GitHubAppService, "post_comment")

readthedocs/oauth/services/base.py

@@ -111,8 +111,12 @@ 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, create_new: bool = True):
+        """
+        Post a comment on the pull request attached to the build.
+
+        :param create_new: Create a new comment if one doesn't exist.
+        """
         raise NotImplementedError
 
     @classmethod

readthedocs/oauth/services/githubapp.py

@@ -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, create_new: bool = True):
         """
         Post a comment on the pull request attached to the build.
 
@@ -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 create_new:
             gh_issue.create_comment(body=comment)
+        else:
+            log.debug(
+                "No comment to update, skipping commenting",
+                project=project.slug,
+                build=build.pk,
+            )

readthedocs/rtd_tests/tests/test_oauth.py

@@ -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!", create_new=False)
+        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!", create_new=False)
+        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"