Skip to content

Commit 33af326

Browse files
authored
GitHub App: post comments on PRs (#12230)
- The FileTreeDiff dataclass was changed to a normal class, to make it easier to interact with its contents in a dynamic way. - Since a new FileTreeDiffFile class was introduced, the old one was renamed to FileTreeDiffManifestFile, since it's a class only used in the manifest dataclass. - The comment is posted after the manifest from the PR is created, since it depends on having the manifest ready to get the diff. - The application creates one comment per-project linked to the remote repository. First I tried including the information of several projects in one single comment, but I think it's better for each project to have their own comment (so users get the notification for when each build is done). - There is a new option in the project model, so users can decide if this feature should be enabled. This option is hidden for projects not linked to a GH app. I tried using the pre-validation form thing, but it isn't per-field, and it blocks the whole form from saving. - Try/catch around the indexers, so if one fails, we can still get the build overview posted. - For the comment, I took some inspiration from https://docs.codecov.com/docs/pull-request-comments and from my initial ideas around the UI for FTD. My vision is to keep the comment minimal, but still have all the information available if needed. This means that all metadata is either hidden or one-liner, while the list of top files changed is a list. The list of all files changed is included in a table, so we can add more information if needed (I'm envisioning putting here a redirect suggestion, for example), and also should be search friendly, so users can easily search all files that are marked as changed/added/deleted. - Should the comment also respect the ignore patterns from the addon config? It's also kind of confusing that we have these settings in two places... in addons and in the PR form. - The list of the top 5 files changed is literally the 5 files from the diff (sorted). I was also playing around with maybe taking one file from each category (added/modifed/deleted), but will see how useful is just having the top first. You can see a live example at stsewd/rtd-test-builds#1 (comment). ![Screenshot 2025-06-24 at 20-08-57 Update index rst by stsewd · Pull Request #1 · stsewd_rtd-test-builds](https://github.com/user-attachments/assets/fe68a161-128b-4521-880d-de6b58e69365) Closes #11780
1 parent 3be8947 commit 33af326

File tree

18 files changed

+875
-113
lines changed

18 files changed

+875
-113
lines changed

docs/user/pull-requests.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@ Build status report
2424

2525
GitHub build status reporting
2626

27+
Build overview with changed files
28+
We create a comment on the pull request including a link to the preview of the documentation,
29+
and a list of the :doc:`files that changed </visual-diff>` between the current pull request and the latest version of the project's documentation.
30+
31+
.. note::
32+
33+
This feature is only available for projects connected to a :ref:`reference/git-integration:GitHub App`.
34+
2735
Pull request notifications
2836
A pull request notifications is shown at the top of preview pages,
2937
which let readers know they aren't viewing an active version of the project.

readthedocs/builds/reporting.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
from django.conf import settings
2+
from django.template.loader import render_to_string
3+
4+
from readthedocs.builds.models import Build
5+
from readthedocs.filetreediff import get_diff
6+
7+
8+
def get_build_overview(build: Build) -> str | None:
9+
"""
10+
Generate a build overview for the given build.
11+
12+
The overview includes a diff of the files changed between the current
13+
build and the base version of the project (latest by default).
14+
15+
The returned string is rendered using a Markdown template,
16+
which can be included in a comment on a pull request.
17+
"""
18+
project = build.project
19+
base_version = project.addons.options_base_version or project.get_latest_version()
20+
if not base_version:
21+
return None
22+
23+
diff = get_diff(
24+
current_version=build.version,
25+
base_version=base_version,
26+
)
27+
if not diff:
28+
return None
29+
30+
return render_to_string(
31+
"core/build-overview.md",
32+
{
33+
"PRODUCTION_DOMAIN": settings.PRODUCTION_DOMAIN,
34+
"project": project,
35+
"current_version": diff.current_version,
36+
"current_version_build": diff.current_version_build,
37+
"base_version": diff.base_version,
38+
"base_version_build": diff.base_version_build,
39+
"diff": diff,
40+
},
41+
)

readthedocs/builds/tasks.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
from readthedocs.builds.constants import TAG
2828
from readthedocs.builds.models import Build
2929
from readthedocs.builds.models import Version
30+
from readthedocs.builds.reporting import get_build_overview
3031
from readthedocs.builds.utils import memcache_lock
3132
from readthedocs.core.utils import send_email
3233
from readthedocs.core.utils import trigger_build
@@ -428,6 +429,48 @@ def send_build_status(build_pk, commit, status):
428429
return False
429430

430431

432+
@app.task(max_retries=3, default_retry_delay=60, queue="web")
433+
def post_build_overview(build_pk):
434+
build = Build.objects.filter(pk=build_pk).first()
435+
if not build:
436+
return
437+
438+
version = build.version
439+
log.bind(
440+
build_id=build.pk,
441+
project_slug=build.project.slug,
442+
version_slug=version.slug,
443+
)
444+
445+
if not version.is_external:
446+
log.debug("Build is not for an external version, skipping build overview.")
447+
return
448+
449+
service_class = build.project.get_git_service_class()
450+
if not service_class:
451+
log.debug("Project isn't connected to a Git service, skipping build overview.")
452+
return
453+
454+
if not service_class.supports_commenting:
455+
log.debug("Git service doesn't support creating comments.")
456+
return
457+
458+
comment = get_build_overview(build)
459+
if not comment:
460+
log.debug("No build overview available, skipping posting comment.")
461+
return
462+
463+
for service in service_class.for_project(build.project):
464+
service.post_comment(
465+
build=build,
466+
comment=comment,
467+
)
468+
log.debug("PR comment posted successfully.")
469+
return
470+
471+
log.debug("No service available, no build overview posted.")
472+
473+
431474
@app.task(queue="web")
432475
def send_build_notifications(version_pk, build_pk, event):
433476
version = Version.objects.get_object_or_log(pk=version_pk)

readthedocs/builds/tests/test_tasks.py

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,35 @@
11
from datetime import datetime, timedelta
2+
from textwrap import dedent
23
from unittest import mock
34

5+
from django.contrib.auth.models import User
46
from django.test import TestCase, override_settings
57
from django.utils import timezone
68
from django_dynamic_fixture import get
79

810
from readthedocs.builds.constants import (
911
BRANCH,
12+
BUILD_STATE_FINISHED,
1013
EXTERNAL,
1114
EXTERNAL_VERSION_STATE_CLOSED,
1215
EXTERNAL_VERSION_STATE_OPEN,
16+
LATEST,
1317
TAG,
1418
)
1519
from readthedocs.builds.models import Build, BuildCommandResult, Version
1620
from readthedocs.builds.tasks import (
1721
archive_builds_task,
1822
delete_closed_external_versions,
23+
post_build_overview,
1924
)
25+
from readthedocs.filetreediff.dataclasses import FileTreeDiff, FileTreeDiffFileStatus
26+
from readthedocs.oauth.constants import GITHUB_APP
27+
from readthedocs.oauth.models import (
28+
GitHubAccountType,
29+
GitHubAppInstallation,
30+
RemoteRepository,
31+
)
32+
from readthedocs.oauth.services import GitHubAppService
2033
from readthedocs.projects.models import Project
2134

2235

@@ -110,3 +123,217 @@ def test_archive_builds(self, build_commands_storage):
110123
self.assertEqual(Build.objects.count(), 10)
111124
self.assertEqual(Build.objects.filter(cold_storage=True).count(), 5)
112125
self.assertEqual(BuildCommandResult.objects.count(), 50)
126+
127+
128+
@override_settings(
129+
PRODUCTION_DOMAIN="readthedocs.org",
130+
PUBLIC_DOMAIN="readthedocs.io",
131+
RTD_EXTERNAL_VERSION_DOMAIN="readthedocs.build",
132+
)
133+
class TestPostBuildOverview(TestCase):
134+
135+
def setUp(self):
136+
self.user = get(User)
137+
self.github_app_installation = get(
138+
GitHubAppInstallation,
139+
installation_id=1111,
140+
target_id=1111,
141+
target_type=GitHubAccountType.USER,
142+
)
143+
self.remote_repository = get(
144+
RemoteRepository,
145+
name="repo",
146+
full_name="user/repo",
147+
vcs_provider=GITHUB_APP,
148+
github_app_installation=self.github_app_installation,
149+
)
150+
151+
self.project = get(
152+
Project,
153+
name="My project",
154+
slug="my-project",
155+
users=[self.user],
156+
remote_repository=self.remote_repository,
157+
)
158+
self.base_version = self.project.versions.get(slug=LATEST)
159+
self.base_version.built = True
160+
self.base_version.save()
161+
self.base_version_build = get(
162+
Build,
163+
project=self.project,
164+
version=self.base_version,
165+
commit="1234abcd",
166+
state=BUILD_STATE_FINISHED,
167+
success=True,
168+
)
169+
self.current_version = get(
170+
Version,
171+
project=self.project,
172+
verbose_name="1",
173+
slug="1",
174+
type=EXTERNAL,
175+
active=True,
176+
built=True,
177+
)
178+
self.current_version_build = get(
179+
Build,
180+
project=self.project,
181+
version=self.current_version,
182+
commit="5678abcd",
183+
state=BUILD_STATE_FINISHED,
184+
success=True,
185+
)
186+
187+
@mock.patch.object(GitHubAppService, "post_comment")
188+
@mock.patch("readthedocs.builds.reporting.get_diff")
189+
def test_post_build_overview(self, get_diff, post_comment):
190+
get_diff.return_value = FileTreeDiff(
191+
current_version=self.current_version,
192+
current_version_build=self.current_version_build,
193+
base_version=self.base_version,
194+
base_version_build=self.base_version_build,
195+
files=[
196+
("index.html", FileTreeDiffFileStatus.modified),
197+
("changes.html", FileTreeDiffFileStatus.added),
198+
("deleteme.html", FileTreeDiffFileStatus.deleted),
199+
],
200+
outdated=False,
201+
)
202+
post_build_overview(build_pk=self.current_version_build.pk)
203+
expected_comment = dedent(
204+
f"""
205+
## Documentation build overview
206+
207+
> 📚 [My project](https://readthedocs.org/projects/my-project/) | 🛠️ build [#{self.current_version_build.id}](https://readthedocs.org/projects/my-project/builds/{self.current_version_build.id}/) (5678abcd) | 🔍 [preview](http://my-project--1.readthedocs.build/en/1/)
208+
209+
### Files changed
210+
211+
> Comparing with [latest](http://my-project.readthedocs.io/en/latest/) (1234abcd)
212+
213+
214+
<details>
215+
<summary>Show files (3) | 1 modified | 1 added | 1 deleted</summary>
216+
217+
| File | Status |
218+
| --- | --- |
219+
| [changes.html](http://my-project--1.readthedocs.build/en/1/changes.html) | ➕ added |
220+
| [deleteme.html](http://my-project--1.readthedocs.build/en/1/deleteme.html) | ❌ deleted |
221+
| [index.html](http://my-project--1.readthedocs.build/en/1/index.html) | 📝 modified |
222+
223+
224+
</details>
225+
226+
"""
227+
)
228+
post_comment.assert_called_once_with(
229+
build=self.current_version_build,
230+
comment=expected_comment,
231+
)
232+
233+
@mock.patch.object(GitHubAppService, "post_comment")
234+
@mock.patch("readthedocs.builds.reporting.get_diff")
235+
def test_post_build_overview_more_than_5_files(self, get_diff, post_comment):
236+
get_diff.return_value = FileTreeDiff(
237+
current_version=self.current_version,
238+
current_version_build=self.current_version_build,
239+
base_version=self.base_version,
240+
base_version_build=self.base_version_build,
241+
files=[
242+
("index.html", FileTreeDiffFileStatus.modified),
243+
("changes.html", FileTreeDiffFileStatus.added),
244+
("deleteme.html", FileTreeDiffFileStatus.deleted),
245+
("one.html", FileTreeDiffFileStatus.modified),
246+
("three.html", FileTreeDiffFileStatus.modified),
247+
("two.html", FileTreeDiffFileStatus.modified),
248+
],
249+
outdated=False,
250+
)
251+
post_build_overview(build_pk=self.current_version_build.pk)
252+
expected_comment = dedent(
253+
f"""
254+
## Documentation build overview
255+
256+
> 📚 [My project](https://readthedocs.org/projects/my-project/) | 🛠️ build [#{self.current_version_build.id}](https://readthedocs.org/projects/my-project/builds/{self.current_version_build.id}/) (5678abcd) | 🔍 [preview](http://my-project--1.readthedocs.build/en/1/)
257+
258+
### Files changed
259+
260+
> Comparing with [latest](http://my-project.readthedocs.io/en/latest/) (1234abcd)
261+
262+
263+
<details>
264+
<summary>Show files (6) | 4 modified | 1 added | 1 deleted</summary>
265+
266+
| File | Status |
267+
| --- | --- |
268+
| [changes.html](http://my-project--1.readthedocs.build/en/1/changes.html) | ➕ added |
269+
| [deleteme.html](http://my-project--1.readthedocs.build/en/1/deleteme.html) | ❌ deleted |
270+
| [index.html](http://my-project--1.readthedocs.build/en/1/index.html) | 📝 modified |
271+
| [one.html](http://my-project--1.readthedocs.build/en/1/one.html) | 📝 modified |
272+
| [three.html](http://my-project--1.readthedocs.build/en/1/three.html) | 📝 modified |
273+
| [two.html](http://my-project--1.readthedocs.build/en/1/two.html) | 📝 modified |
274+
275+
276+
</details>
277+
278+
"""
279+
)
280+
post_comment.assert_called_once_with(
281+
build=self.current_version_build,
282+
comment=expected_comment,
283+
)
284+
285+
@mock.patch.object(GitHubAppService, "post_comment")
286+
@mock.patch("readthedocs.builds.reporting.get_diff")
287+
def test_post_build_overview_no_files_changed(self, get_diff, post_comment):
288+
get_diff.return_value = FileTreeDiff(
289+
current_version=self.current_version,
290+
current_version_build=self.current_version_build,
291+
base_version=self.base_version,
292+
base_version_build=self.base_version_build,
293+
files=[],
294+
outdated=False,
295+
)
296+
post_build_overview(build_pk=self.current_version_build.pk)
297+
expected_comment = dedent(
298+
f"""
299+
## Documentation build overview
300+
301+
> 📚 [My project](https://readthedocs.org/projects/my-project/) | 🛠️ build [#{self.current_version_build.id}](https://readthedocs.org/projects/my-project/builds/{self.current_version_build.id}/) (5678abcd) | 🔍 [preview](http://my-project--1.readthedocs.build/en/1/)
302+
303+
### Files changed
304+
305+
> Comparing with [latest](http://my-project.readthedocs.io/en/latest/) (1234abcd)
306+
307+
308+
No files changed.
309+
310+
"""
311+
)
312+
post_comment.assert_called_once_with(
313+
build=self.current_version_build,
314+
comment=expected_comment,
315+
)
316+
317+
@mock.patch.object(GitHubAppService, "post_comment")
318+
def test_post_build_overview_no_external_version(self, post_comment):
319+
assert not self.base_version.is_external
320+
post_build_overview(build_pk=self.base_version_build.pk)
321+
post_comment.assert_not_called()
322+
323+
@mock.patch.object(GitHubAppService, "post_comment")
324+
def test_post_build_overview_no_github_app_project(self, post_comment):
325+
self.project.remote_repository = None
326+
self.project.save()
327+
328+
assert not self.project.is_github_app_project
329+
assert self.current_version.is_external
330+
post_build_overview(build_pk=self.current_version_build.pk)
331+
post_comment.assert_not_called()
332+
333+
@mock.patch.object(GitHubAppService, "post_comment")
334+
@mock.patch("readthedocs.builds.reporting.get_diff")
335+
def test_post_build_overview_no_diff_available(self, get_diff, post_comment):
336+
get_diff.return_value = None
337+
assert self.current_version.is_external
338+
post_build_overview(build_pk=self.current_version_build.pk)
339+
post_comment.assert_not_called()

0 commit comments

Comments
 (0)