diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 57b86360e96..e3ddb9daa93 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -269,7 +269,7 @@ class BuildsViewSet(APIv3Settings, NestedViewSetMixin, ProjectQuerySetMixin, lookup_url_kwarg = 'build_pk' serializer_class = BuildSerializer filterset_class = BuildFilter - queryset = Build.objects.all() + queryset = Build.internal.all() permit_list_expands = [ 'config', ] diff --git a/readthedocs/builds/managers.py b/readthedocs/builds/managers.py index 06262ed3e24..d80852478f2 100644 --- a/readthedocs/builds/managers.py +++ b/readthedocs/builds/managers.py @@ -21,7 +21,7 @@ TAG, EXTERNAL, ) -from .querysets import VersionQuerySet +from .querysets import VersionQuerySet, BuildQuerySet log = logging.getLogger(__name__) @@ -122,3 +122,61 @@ class InternalVersionManager(SettingsOverrideObject): class ExternalVersionManager(SettingsOverrideObject): _default_class = ExternalVersionManagerBase + + +class BuildManagerBase(models.Manager): + + """ + Build manager for manager only queries. + + For creating different Managers. + """ + + @classmethod + def from_queryset(cls, queryset_class, class_name=None): + # This is overridden because :py:meth:`models.Manager.from_queryset` + # uses `inspect` to retrieve the class methods, and the proxy class has + # no direct members. + queryset_class = get_override_class( + BuildQuerySet, + BuildQuerySet._default_class, # pylint: disable=protected-access + ) + return super().from_queryset(queryset_class, class_name) + + +class InternalBuildManagerBase(BuildManagerBase): + + """ + Build manager that only includes internal version builds. + + It will exclude pull request/merge request version builds from the queries + and only include BRANCH, TAG, UNKONWN type Version builds. + """ + + def get_queryset(self): + return super().get_queryset().exclude(version__type=EXTERNAL) + + +class ExternalBuildManagerBase(BuildManagerBase): + + """ + Build manager that only includes external version builds. + + It will only include pull request/merge request version builds in the queries. + """ + + def get_queryset(self): + return super().get_queryset().filter(version__type=EXTERNAL) + + +class BuildManager(SettingsOverrideObject): + _default_class = BuildManagerBase + _override_setting = 'BUILD_MANAGER' + + +class InternalBuildManager(SettingsOverrideObject): + _default_class = InternalBuildManagerBase + + +class ExternalBuildManager(SettingsOverrideObject): + _default_class = ExternalBuildManagerBase diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 34a3fd372a9..75abdcfda24 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -46,18 +46,25 @@ TAG, VERSION_TYPES, ) -from .managers import ( +from readthedocs.builds.managers import ( VersionManager, InternalVersionManager, - ExternalVersionManager + ExternalVersionManager, + BuildManager, + InternalBuildManager, + ExternalBuildManager, ) -from .querysets import BuildQuerySet, RelatedBuildQuerySet, VersionQuerySet -from .utils import ( +from readthedocs.builds.querysets import ( + BuildQuerySet, + RelatedBuildQuerySet, + VersionQuerySet, +) +from readthedocs.builds.utils import ( get_bitbucket_username_repo, get_github_username_repo, get_gitlab_username_repo, ) -from .version_slug import VersionSlugField +from readthedocs.builds.version_slug import VersionSlugField log = logging.getLogger(__name__) @@ -200,7 +207,7 @@ def config(self): :rtype: dict """ last_build = ( - self.builds.filter( + self.builds(manager=INTERNAL).filter( state='finished', success=True, ).order_by('-date').first() @@ -627,9 +634,12 @@ class Build(models.Model): help_text='Build steps stored outside the database.', ) - # Manager - - objects = BuildQuerySet.as_manager() + # Managers + objects = BuildManager.from_queryset(BuildQuerySet)() + # Only include BRANCH, TAG, UNKONWN type Version builds. + internal = InternalBuildManager.from_queryset(BuildQuerySet)() + # Only include EXTERNAL type Version builds. + external = ExternalBuildManager.from_queryset(BuildQuerySet)() CONFIG_KEY = '__config' diff --git a/readthedocs/builds/querysets.py b/readthedocs/builds/querysets.py index 6b4c9132b1a..d6e4c1bda2c 100644 --- a/readthedocs/builds/querysets.py +++ b/readthedocs/builds/querysets.py @@ -116,7 +116,6 @@ def api(self, user=None, detail=True): class BuildQuerySet(SettingsOverrideObject): _default_class = BuildQuerySetBase - _override_setting = 'BUILD_MANAGER' class RelatedBuildQuerySetBase(models.QuerySet): diff --git a/readthedocs/builds/views.py b/readthedocs/builds/views.py index 3333495f6c8..479d0797458 100644 --- a/readthedocs/builds/views.py +++ b/readthedocs/builds/views.py @@ -84,6 +84,21 @@ def post(self, request, project_slug): class BuildList(BuildBase, BuildTriggerMixin, ListView): + def get_queryset(self): + # this is used to include only internal version + # builds in the build list page + self.project_slug = self.kwargs.get('project_slug', None) + self.project = get_object_or_404( + Project.objects.protected(self.request.user), + slug=self.project_slug, + ) + queryset = Build.internal.public( + user=self.request.user, + project=self.project, + ).select_related('project', 'version') + + return queryset + def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) diff --git a/readthedocs/core/templatetags/privacy_tags.py b/readthedocs/core/templatetags/privacy_tags.py index 115bb9eadd2..e4ad684a8a0 100644 --- a/readthedocs/core/templatetags/privacy_tags.py +++ b/readthedocs/core/templatetags/privacy_tags.py @@ -26,7 +26,7 @@ def get_public_projects(context, user): viewer=context['request'].user, ).prefetch_latest_build().annotate( _good_build=Exists( - Build.objects.filter(success=True, project=OuterRef('pk'))) + Build.internal.filter(success=True, project=OuterRef('pk'))) ) context['public_projects'] = projects return '' diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index a3f36feb5c8..2e782c7296a 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -768,7 +768,7 @@ def has_good_build(self): # Used for Database optimization. if hasattr(self, '_good_build'): return self._good_build - return self.builds.filter(success=True).exists() + return self.builds(manager=INTERNAL).filter(success=True).exists() @property def has_versions(self): diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index 7ea7249ae0d..ff4221fd590 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -85,13 +85,13 @@ def prefetch_latest_build(self): # Prefetch the latest build for each project. subquery = Subquery( - Build.objects.filter( + Build.internal.filter( project=OuterRef('project_id') ).order_by('-date').values_list('id', flat=True)[:1] ) latest_build = Prefetch( 'builds', - Build.objects.filter(pk__in=subquery), + Build.internal.filter(pk__in=subquery), to_attr=self.model.LATEST_BUILD_CACHE, ) return self.prefetch_related(latest_build) diff --git a/readthedocs/rtd_tests/tests/test_doc_serving.py b/readthedocs/rtd_tests/tests/test_doc_serving.py index ea9a198986f..734ed3338b1 100644 --- a/readthedocs/rtd_tests/tests/test_doc_serving.py +++ b/readthedocs/rtd_tests/tests/test_doc_serving.py @@ -313,7 +313,7 @@ def test_sitemap_xml(self): # in language and country value. (zh_CN should be zh-CN) self.assertContains(response, 'zh-CN') - # PR Versions should not be in the sitemap_xml. + # External Versions should not be in the sitemap_xml. self.assertNotContains( response, self.public.get_docs_url( @@ -322,6 +322,7 @@ def test_sitemap_xml(self): private=True, ), ) + # Check if STABLE version has 'priority of 1 and changefreq of weekly. self.assertEqual( response.context['versions'][0]['loc'], diff --git a/readthedocs/rtd_tests/tests/test_managers.py b/readthedocs/rtd_tests/tests/test_managers.py index 1d5cbd78645..a17aee9c7a7 100644 --- a/readthedocs/rtd_tests/tests/test_managers.py +++ b/readthedocs/rtd_tests/tests/test_managers.py @@ -3,7 +3,7 @@ from django_dynamic_fixture import get from readthedocs.builds.constants import EXTERNAL, BRANCH, TAG -from readthedocs.builds.models import Version +from readthedocs.builds.models import Version, Build from readthedocs.projects.constants import PUBLIC, PRIVATE, PROTECTED from readthedocs.projects.models import Project @@ -123,3 +123,91 @@ def test_external_version_manager_with_for_project(self): self.assertIn( self.public_external_version, Version.external.for_project(self.pip) ) + + +class TestBuildManagerBase(TestCase): + + fixtures = ['test_data'] + + def setUp(self): + self.user = User.objects.create(username='test_user', password='test') + self.client.login(username='test_user', password='test') + self.pip = Project.objects.get(slug='pip') + print(self.pip.versions.all()) + # Create a External Version and build. ie: pull/merge request Version. + self.external_version = get( + Version, + project=self.pip, + active=True, + type=EXTERNAL, + privacy_level=PUBLIC + ) + self.external_version_build = get( + Build, + project=self.pip, + version=self.external_version + ) + # Create a Internal Version build. + self.internal_version_build = get( + Build, + project=self.pip, + version=self.pip.versions.get(slug='0.8') + ) + + self.internal_builds = Build.objects.exclude(version__type=EXTERNAL) + + +class TestInternalBuildManager(TestBuildManagerBase): + + """ + Queries using Internal Manager should only include Internal Version builds. + + It will exclude pull/merge request Version builds from the queries + and only include BRANCH, TAG, UNKONWN type Versions. + """ + + def test_internal_build_manager_with_all(self): + self.assertNotIn(self.external_version_build, Build.internal.all()) + + def test_internal_build_manager_with_public(self): + self.assertNotIn(self.external_version_build, Build.internal.public()) + + def test_internal_build_manager_with_public_with_user_and_project(self): + self.assertNotIn( + self.external_version_build, + Build.internal.public(self.user, self.pip) + ) + + def test_internal_build_manager_with_api(self): + self.assertNotIn(self.external_version_build, Build.internal.api()) + + +class TestExternalBuildManager(TestBuildManagerBase): + + """ + Queries using External Manager should only include External Version builds. + + It will only include pull/merge request Version builds in the queries. + """ + + def test_external_build_manager_with_all(self): + self.assertNotIn(self.internal_builds, Build.external.all()) + self.assertIn(self.external_version_build, Build.external.all()) + + def test_external_build_manager_with_public(self): + self.assertNotIn(self.internal_builds, Build.external.public()) + self.assertIn(self.external_version_build, Build.external.public()) + + def test_external_build_manager_with_public_with_user_and_project(self): + self.assertNotIn( + self.internal_builds, + Build.external.public(self.user, self.pip) + ) + self.assertIn( + self.external_version_build, + Build.external.public(self.user, self.pip) + ) + + def test_external_build_manager_with_api(self): + self.assertNotIn(self.internal_builds, Build.external.api()) + self.assertIn(self.external_version_build, Build.external.api()) diff --git a/readthedocs/rtd_tests/tests/test_project.py b/readthedocs/rtd_tests/tests/test_project.py index b7c0afa883a..ded11f52c53 100644 --- a/readthedocs/rtd_tests/tests/test_project.py +++ b/readthedocs/rtd_tests/tests/test_project.py @@ -160,6 +160,12 @@ def test_update_stable_version_excludes_external_versions(self): # Test that PR Version is not considered for stable. self.assertEqual(self.pip.update_stable_version(), None) + def test_has_good_build_excludes_external_versions(self): + # Delete all versions excluding PR Versions. + self.pip.versions.exclude(type=EXTERNAL).delete() + # Test that PR Version is not considered for has_good_build. + self.assertFalse(self.pip.has_good_build) + class TestProjectTranslations(ProjectMixin, TestCase): diff --git a/readthedocs/rtd_tests/tests/test_views.py b/readthedocs/rtd_tests/tests/test_views.py index 25e7b60a756..de77945b311 100644 --- a/readthedocs/rtd_tests/tests/test_views.py +++ b/readthedocs/rtd_tests/tests/test_views.py @@ -7,8 +7,8 @@ from django.urls import reverse from django_dynamic_fixture import get, new -from readthedocs.builds.constants import LATEST -from readthedocs.builds.models import Build +from readthedocs.builds.constants import LATEST, EXTERNAL +from readthedocs.builds.models import Build, Version from readthedocs.core.permissions import AdminPermission from readthedocs.projects.forms import UpdateProjectForm from readthedocs.projects.models import HTMLFile, Project @@ -266,6 +266,7 @@ class BuildViewTests(TestCase): def setUp(self): self.client.login(username='eric', password='test') + self.pip = Project.objects.get(slug='pip') @mock.patch('readthedocs.projects.tasks.update_docs_task') def test_build_redirect(self, mock): @@ -276,3 +277,23 @@ def test_build_redirect(self, mock): r._headers['location'][1], '/projects/pip/builds/%s/' % build.pk, ) + + def test_build_list_does_not_include_external_versions(self): + external_version = get( + Version, + project = self.pip, + active = True, + type = EXTERNAL, + ) + external_version_build = get( + Build, + project = self.pip, + version = external_version + ) + response = self.client.get( + reverse('builds_project_list', args=[self.pip.slug]), + ) + self.assertEqual(response.status_code, 200) + + self.assertNotIn(external_version_build, response.context['build_qs']) + self.assertNotIn(external_version_build, response.context['active_builds'])