Merge pull request #5779 from saadmk11/build-manager-update

Add Build managers and Update Build Querysets.

Author: ericholscher

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',
     ]

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

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__)
@@ -201,7 +208,7 @@ def config(self):
         :rtype: dict
         """
         last_build = (
-            self.builds.filter(
+            self.builds(manager=INTERNAL).filter(
                 state='finished',
                 success=True,
             ).order_by('-date').first()
@@ -628,9 +635,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'
 

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):

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)
 

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 ''

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):

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)

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'],

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())

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):
 

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'])