From 111f944262128618d4bf92075368cfc12e45b1d2 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Tue, 31 Mar 2026 22:37:01 +0800 Subject: [PATCH 1/2] Build: skip invalid single-version builds --- readthedocs/api/v3/tests/test_builds.py | 38 ++++++++++++++++ readthedocs/api/v3/views.py | 23 +++++----- readthedocs/core/tests/test_hooks.py | 45 +++++++++++++++++++ readthedocs/core/utils/__init__.py | 12 +++++ readthedocs/core/views/hooks.py | 8 +++- .../rtd_tests/tests/test_core_utils.py | 17 +++++++ 6 files changed, 131 insertions(+), 12 deletions(-) create mode 100644 readthedocs/core/tests/test_hooks.py diff --git a/readthedocs/api/v3/tests/test_builds.py b/readthedocs/api/v3/tests/test_builds.py index 37d451133fb..bcb5054cf03 100644 --- a/readthedocs/api/v3/tests/test_builds.py +++ b/readthedocs/api/v3/tests/test_builds.py @@ -1,9 +1,12 @@ from unittest import mock +import django_dynamic_fixture as fixture from django.test import override_settings from django.urls import reverse from readthedocs.builds.constants import EXTERNAL +from readthedocs.builds.models import Version +from readthedocs.projects.constants import SINGLE_VERSION_WITHOUT_TRANSLATIONS from readthedocs.projects.constants import PRIVATE, PUBLIC from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS from readthedocs.subscriptions.products import RTDProductFeature @@ -341,6 +344,41 @@ def test_external_version_projects_versions_builds_list_post(self): expected["version"]["urls"]["vcs"] = "https://github.com/rtfd/project/pull/v1.0" self.assertDictEqual(response_json, expected) + def test_single_version_project_rejects_non_default_version_build(self): + self.project.versioning_scheme = SINGLE_VERSION_WITHOUT_TRANSLATIONS + self.project.default_version = "latest" + self.project.save() + version = fixture.get( + Version, + project=self.project, + slug="v2.0", + verbose_name="v2.0", + identifier="d4e5f6", + active=True, + ) + url = reverse( + "projects-versions-builds-list", + kwargs={ + "parent_lookup_project__slug": self.project.slug, + "parent_lookup_version__slug": version.slug, + }, + ) + + self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}") + builds_count = self.project.builds.count() + response = self.client.post(url) + + self.assertEqual(response.status_code, 400) + self.assertEqual(self.project.builds.count(), builds_count) + self.assertDictEqual( + response.json(), + { + "project": mock.ANY, + "triggered": False, + "version": mock.ANY, + }, + ) + def test_projects_builds_notifications_list_anonymous_user(self): url = reverse( "projects-builds-notifications-list", diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 2c73717f088..de6549c25b9 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -428,19 +428,22 @@ def create(self, request, **kwargs): # pylint: disable=arguments-differ commit=commit, ) - # TODO: refactor this to be a serializer - # BuildTriggeredSerializer(build, project, version).data - data = { - "build": BuildSerializer(build).data, - "project": ProjectSerializer(project).data, - "version": VersionSerializer(version).data, - } - if build: - data.update({"triggered": True}) + # TODO: refactor this to be a serializer + # BuildTriggeredSerializer(build, project, version).data + data = { + "build": BuildSerializer(build).data, + "project": ProjectSerializer(project).data, + "version": VersionSerializer(version).data, + "triggered": True, + } code = status.HTTP_202_ACCEPTED else: - data.update({"triggered": False}) + data = { + "project": ProjectSerializer(project).data, + "version": VersionSerializer(version).data, + "triggered": False, + } code = status.HTTP_400_BAD_REQUEST return Response(data=data, status=code) diff --git a/readthedocs/core/tests/test_hooks.py b/readthedocs/core/tests/test_hooks.py new file mode 100644 index 00000000000..aade7cc316d --- /dev/null +++ b/readthedocs/core/tests/test_hooks.py @@ -0,0 +1,45 @@ +from unittest import mock + +import django_dynamic_fixture as fixture +from django.test import TestCase + +from readthedocs.builds.constants import BRANCH +from readthedocs.builds.models import Version +from readthedocs.core.views.hooks import VersionInfo +from readthedocs.core.views.hooks import build_versions_from_names +from readthedocs.projects.constants import SINGLE_VERSION_WITHOUT_TRANSLATIONS +from readthedocs.projects.models import Project + + +class BuildVersionsFromNamesTests(TestCase): + @mock.patch("readthedocs.core.views.hooks.trigger_build") + def test_skips_non_default_version_on_single_version_project(self, trigger_build): + project = fixture.get( + Project, + versioning_scheme=SINGLE_VERSION_WITHOUT_TRANSLATIONS, + default_version="latest", + main_language_project=None, + ) + version = fixture.get( + Version, + project=project, + slug="feature-branch", + verbose_name="feature-branch", + type=BRANCH, + active=True, + machine=False, + ) + trigger_build.return_value = (None, None) + + to_build, not_building = build_versions_from_names( + project, + [VersionInfo(name=version.verbose_name, type=version.type)], + ) + + self.assertEqual(to_build, set()) + self.assertEqual(not_building, {version.slug}) + trigger_build.assert_called_once_with( + project=project, + version=version, + from_webhook=True, + ) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index 2faf183107d..7b6401d0a28 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -67,6 +67,18 @@ def prepare_build( default_version = project.get_default_version() version = project.versions.get(slug=default_version) + if ( + project.is_single_version + and not version.is_external + and version.slug != project.get_default_version() + ): + log.warning( + "Build not triggered for non-default version on single-version project.", + default_version_slug=project.get_default_version(), + version_slug=version.slug, + ) + return (None, None) + build = Build.objects.create( project=project, version=version, diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py index 8830328c08a..246036f1352 100644 --- a/readthedocs/core/views/hooks.py +++ b/readthedocs/core/views/hooks.py @@ -50,8 +50,12 @@ def build_versions_from_names(project, versions_info: list[VersionInfo]): continue if version.active: - trigger_build(project=project, version=version, from_webhook=True) - to_build.add(version.slug) + _, build = trigger_build(project=project, version=version, from_webhook=True) + if build: + to_build.add(version.slug) + else: + log.info("Not building.", version_slug=version.slug) + not_building.add(version.slug) else: log.info("Not building.", version_slug=version.slug) not_building.add(version.slug) diff --git a/readthedocs/rtd_tests/tests/test_core_utils.py b/readthedocs/rtd_tests/tests/test_core_utils.py index b8d80335f80..8b053a4c288 100644 --- a/readthedocs/rtd_tests/tests/test_core_utils.py +++ b/readthedocs/rtd_tests/tests/test_core_utils.py @@ -10,6 +10,7 @@ from readthedocs.builds.models import Build, Version from readthedocs.core.utils import slugify, trigger_build from readthedocs.doc_builder.exceptions import BuildMaxConcurrencyError +from readthedocs.projects.constants import SINGLE_VERSION_WITHOUT_TRANSLATIONS from readthedocs.projects.models import Project from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS from readthedocs.subscriptions.products import RTDProductFeature @@ -172,6 +173,22 @@ def test_trigger_build_rounded_time_limit(self, update_docs): immutable=True, ) + @mock.patch("readthedocs.projects.tasks.builds.update_docs_task") + def test_trigger_build_skips_non_default_version_on_single_version_project( + self, update_docs + ): + self.project.versioning_scheme = SINGLE_VERSION_WITHOUT_TRANSLATIONS + self.project.default_version = "latest" + self.project.save() + + result = trigger_build(project=self.project, version=self.version) + + self.assertEqual(result, (None, None)) + self.assertFalse(update_docs.signature.called) + self.assertFalse( + Build.objects.filter(project=self.project, version=self.version).exists() + ) + @mock.patch("readthedocs.core.utils.app") @mock.patch("readthedocs.projects.tasks.builds.update_docs_task") def test_trigger_max_concurrency_reached(self, update_docs, app): From bf283c64910f7b5fd89af5f3ecd063697a19084e Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Tue, 7 Apr 2026 10:06:24 +0800 Subject: [PATCH 2/2] Fix trigger_build mock return values to support tuple unpacking The build_versions_from_names function now unpacks trigger_build return value as (_, build), but existing test mocks did not set a return_value, causing unpacking errors in 12 tests. --- readthedocs/oauth/tests/test_githubapp_webhook.py | 6 +++--- readthedocs/rtd_tests/tests/test_api.py | 5 ++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/readthedocs/oauth/tests/test_githubapp_webhook.py b/readthedocs/oauth/tests/test_githubapp_webhook.py index 2b1a3e9d5c8..643aa189eb5 100644 --- a/readthedocs/oauth/tests/test_githubapp_webhook.py +++ b/readthedocs/oauth/tests/test_githubapp_webhook.py @@ -447,7 +447,7 @@ def test_push_tag_deleted(self, sync_repository_task): kwargs={"build_api_key": mock.ANY}, ) - @mock.patch("readthedocs.core.views.hooks.trigger_build") + @mock.patch("readthedocs.core.views.hooks.trigger_build", return_value=(mock.MagicMock(), mock.MagicMock())) def test_push_branch(self, trigger_build): payload = { "installation": { @@ -472,7 +472,7 @@ def test_push_branch(self, trigger_build): ] ) - @mock.patch("readthedocs.core.views.hooks.trigger_build") + @mock.patch("readthedocs.core.views.hooks.trigger_build", return_value=(mock.MagicMock(), mock.MagicMock())) def test_push_tag(self, trigger_build): payload = { "installation": { @@ -1172,7 +1172,7 @@ def test_push_branch_with_non_matching_webhook_rule(self, trigger_build): # Should NOT trigger build because src/code.py doesn't match docs/*.rst trigger_build.assert_not_called() - @mock.patch("readthedocs.core.views.hooks.trigger_build") + @mock.patch("readthedocs.core.views.hooks.trigger_build", return_value=(mock.MagicMock(), mock.MagicMock())) def test_push_branch_without_webhook_rules(self, trigger_build): """Test that push triggers build normally when no WebhookAutomationRules exist.""" # No automation rules - should trigger build as usual diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 83eff726317..2f2ce2aa0fb 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -1905,7 +1905,10 @@ def test_permissions(self): self.assertEqual(len(repos), 0) -@mock.patch("readthedocs.core.views.hooks.trigger_build") +@mock.patch( + "readthedocs.core.views.hooks.trigger_build", + return_value=(mock.MagicMock(), mock.MagicMock()), +) class IntegrationsTests(TestCase): """Integration for webhooks, etc."""