diff --git a/readthedocs/api/v2/views/footer_views.py b/readthedocs/api/v2/views/footer_views.py index 04d4fe8210b..04fedeadd3f 100644 --- a/readthedocs/api/v2/views/footer_views.py +++ b/readthedocs/api/v2/views/footer_views.py @@ -9,7 +9,7 @@ from rest_framework_jsonp.renderers import JSONPRenderer from readthedocs.api.v2.signals import footer_response -from readthedocs.builds.constants import LATEST, TAG +from readthedocs.builds.constants import LATEST, TAG, INTERNAL from readthedocs.builds.models import Version from readthedocs.projects.models import Project from readthedocs.projects.version_handling import ( @@ -25,7 +25,7 @@ def get_version_compare_data(project, base_version=None): :param base_version: We assert whether or not the base_version is also the highest version in the resulting "is_highest" value. """ - versions_qs = Version.objects.public(project=project) + versions_qs = Version.internal.public(project=project) # Take preferences over tags only if the project has at least one tag if versions_qs.filter(type=TAG).exists(): diff --git a/readthedocs/api/v2/views/model_views.py b/readthedocs/api/v2/views/model_views.py index af41f28ab84..fca91b32626 100644 --- a/readthedocs/api/v2/views/model_views.py +++ b/readthedocs/api/v2/views/model_views.py @@ -10,7 +10,7 @@ from rest_framework.renderers import BaseRenderer, JSONRenderer from rest_framework.response import Response -from readthedocs.builds.constants import BRANCH, TAG +from readthedocs.builds.constants import BRANCH, TAG, INTERNAL from readthedocs.builds.models import Build, BuildCommandResult, Version from readthedocs.core.utils import trigger_build from readthedocs.core.utils.extend import SettingsOverrideObject @@ -130,7 +130,7 @@ def active_versions(self, request, **kwargs): Project.objects.api(request.user), pk=kwargs['pk'], ) - versions = project.versions.filter(active=True) + versions = project.versions(manager=INTERNAL).filter(active=True) return Response({ 'versions': VersionSerializer(versions, many=True).data, }) diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 0e8b2b55a0e..57b86360e96 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -230,7 +230,7 @@ class VersionsViewSet(APIv3Settings, NestedViewSetMixin, ProjectQuerySetMixin, lookup_value_regex = r'[^/]+' filterset_class = VersionFilter - queryset = Version.objects.all() + queryset = Version.internal.all() permit_list_expands = [ 'last_build', 'last_build.config', diff --git a/readthedocs/builds/constants.py b/readthedocs/builds/constants.py index b0dc6cfba94..bdc9a70fe79 100644 --- a/readthedocs/builds/constants.py +++ b/readthedocs/builds/constants.py @@ -31,6 +31,13 @@ ('dash', _('Dash')), ) +# Manager name for Internal Versions or Builds. +# ie: Versions and Builds Excluding pull request/merge request Versions and Builds. +INTERNAL = 'internal' +# Manager name for External Versions or Builds. +# ie: Only pull request/merge request Versions and Builds. +EXTERNAL = 'external' + BRANCH = 'branch' TAG = 'tag' UNKNOWN = 'unknown' @@ -38,6 +45,7 @@ VERSION_TYPES = ( (BRANCH, _('Branch')), (TAG, _('Tag')), + (EXTERNAL, _('External')), (UNKNOWN, _('Unknown')), ) diff --git a/readthedocs/builds/managers.py b/readthedocs/builds/managers.py index 68cdec6efe9..06262ed3e24 100644 --- a/readthedocs/builds/managers.py +++ b/readthedocs/builds/managers.py @@ -19,6 +19,7 @@ STABLE, STABLE_VERBOSE_NAME, TAG, + EXTERNAL, ) from .querysets import VersionQuerySet @@ -85,6 +86,39 @@ def get_object_or_log(self, **kwargs): log.warning('Version not found for given kwargs. %s' % kwargs) +class InternalVersionManagerBase(VersionManagerBase): + + """ + Version manager that only includes internal version. + + It will exclude pull request/merge request versions from the queries + and only include BRANCH, TAG, UNKONWN type Versions. + """ + + def get_queryset(self): + return super().get_queryset().exclude(type=EXTERNAL) + + +class ExternalVersionManagerBase(VersionManagerBase): + + """ + Version manager that only includes external version. + + It will only include pull request/merge request Versions in the queries. + """ + + def get_queryset(self): + return super().get_queryset().filter(type=EXTERNAL) + + class VersionManager(SettingsOverrideObject): _default_class = VersionManagerBase _override_setting = 'VERSION_MANAGER' + + +class InternalVersionManager(SettingsOverrideObject): + _default_class = InternalVersionManagerBase + + +class ExternalVersionManager(SettingsOverrideObject): + _default_class = ExternalVersionManagerBase diff --git a/readthedocs/builds/migrations/0008_added_external_version_type.py b/readthedocs/builds/migrations/0008_added_external_version_type.py new file mode 100644 index 00000000000..8de9c4f504f --- /dev/null +++ b/readthedocs/builds/migrations/0008_added_external_version_type.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.21 on 2019-06-17 19:43 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('builds', '0007_add-automation-rules'), + ] + + operations = [ + migrations.AlterField( + model_name='version', + name='type', + field=models.CharField(choices=[('branch', 'Branch'), ('tag', 'Tag'), ('external', 'External'), ('unknown', 'Unknown')], default='unknown', max_length=20, verbose_name='Type'), + ), + migrations.AlterField( + model_name='versionautomationrule', + name='version_type', + field=models.CharField(choices=[('branch', 'Branch'), ('tag', 'Tag'), ('external', 'External'), ('unknown', 'Unknown')], max_length=32, verbose_name='Version type'), + ), + ] diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 1039b815326..6432283c293 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -32,19 +32,25 @@ from readthedocs.projects.models import APIProject, Project from readthedocs.projects.version_handling import determine_stable_version -from .constants import ( +from readthedocs.builds.constants import ( BRANCH, BUILD_STATE, BUILD_STATE_FINISHED, BUILD_STATE_TRIGGERED, BUILD_TYPES, + INTERNAL, LATEST, NON_REPOSITORY_VERSIONS, + EXTERNAL, STABLE, TAG, VERSION_TYPES, ) -from .managers import VersionManager +from .managers import ( + VersionManager, + InternalVersionManager, + ExternalVersionManager +) from .querysets import BuildQuerySet, RelatedBuildQuerySet, VersionQuerySet from .utils import ( get_bitbucket_username_repo, @@ -111,6 +117,10 @@ class Version(models.Model): machine = models.BooleanField(_('Machine Created'), default=False) objects = VersionManager.from_queryset(VersionQuerySet)() + # Only include BRANCH, TAG, UNKONWN type Versions. + internal = InternalVersionManager.from_queryset(VersionQuerySet)() + # Only include EXTERNAL type Versions. + external = ExternalVersionManager.from_queryset(VersionQuerySet)() class Meta: unique_together = [('project', 'slug')] @@ -133,7 +143,9 @@ def __str__(self): @property def ref(self): if self.slug == STABLE: - stable = determine_stable_version(self.project.versions.all()) + stable = determine_stable_version( + self.project.versions(manager=INTERNAL).all() + ) if stable: return stable.slug @@ -142,7 +154,8 @@ def vcs_url(self): """ Generate VCS (github, gitlab, bitbucket) URL for this version. - Example: https://github.com/rtfd/readthedocs.org/tree/3.4.2/. + Branch/Tag Example: https://github.com/rtfd/readthedocs.org/tree/3.4.2/. + Pull/merge Request Example: https://github.com/rtfd/readthedocs.org/pull/9999/. """ url = '' if self.slug == STABLE: @@ -152,12 +165,25 @@ def vcs_url(self): else: slug_url = self.slug - if ('github' in self.project.repo) or ('gitlab' in self.project.repo): - url = f'/tree/{slug_url}/' + if self.type == EXTERNAL: + if 'github' in self.project.repo: + slug_url = self.verbose_name + url = f'/pull/{slug_url}/' - if 'bitbucket' in self.project.repo: - slug_url = self.identifier - url = f'/src/{slug_url}' + if 'gitlab' in self.project.repo: + slug_url = self.identifier + url = f'/merge_requests/{slug_url}/' + + if 'bitbucket' in self.project.repo: + slug_url = self.identifier + url = f'/pull-requests/{slug_url}' + else: + if ('github' in self.project.repo) or ('gitlab' in self.project.repo): + url = f'/tree/{slug_url}/' + + if 'bitbucket' in self.project.repo: + slug_url = self.identifier + url = f'/src/{slug_url}' # TODO: improve this replacing return self.project.repo.replace('git://', 'https://').replace('.git', '') + url @@ -220,7 +246,14 @@ def commit_name(self): # the actual tag name. return self.verbose_name - # If we came that far it's not a special version nor a branch or tag. + if self.type == EXTERNAL: + # If this version is a EXTERNAL version, the identifier will + # contain the actual commit hash. which we can use to + # generate url for a given file name + return self.identifier + + # If we came that far it's not a special version + # nor a branch, tag or EXTERNAL version. # Therefore just return the identifier to make a safe guess. log.debug( 'TODO: Raise an exception here. Testing what cases it happens', diff --git a/readthedocs/builds/views.py b/readthedocs/builds/views.py index e8e3d458e2c..3333495f6c8 100644 --- a/readthedocs/builds/views.py +++ b/readthedocs/builds/views.py @@ -57,7 +57,7 @@ def post(self, request, project_slug): version_slug = request.POST.get('version_slug') version = get_object_or_404( - Version, + Version.internal.all(), project=project, slug=version_slug, ) @@ -93,7 +93,7 @@ def get_context_data(self, **kwargs): context['project'] = self.project context['active_builds'] = active_builds - context['versions'] = Version.objects.public( + context['versions'] = Version.internal.public( user=self.request.user, project=self.project, ) diff --git a/readthedocs/core/management/commands/update_repos.py b/readthedocs/core/management/commands/update_repos.py index 5852592d360..b15ea8c3c9c 100644 --- a/readthedocs/core/management/commands/update_repos.py +++ b/readthedocs/core/management/commands/update_repos.py @@ -10,6 +10,7 @@ from django.core.management.base import BaseCommand +from readthedocs.builds.constants import EXTERNAL, INTERNAL from readthedocs.builds.models import Build, Version from readthedocs.core.utils import trigger_build from readthedocs.projects import tasks @@ -76,6 +77,48 @@ def handle(self, *args, **options): version.pk, build_pk=build.pk, ) + elif version == INTERNAL: + log.info('Updating all internal versions for %s', slug) + for version in Version.internal.filter( + project__slug=slug, + active=True, + uploaded=False, + ): + + build = Build.objects.create( + project=version.project, + version=version, + type='html', + state='triggered', + ) + + # pylint: disable=no-value-for-parameter + tasks.update_docs_task( + version.project_id, + build_pk=build.pk, + version_pk=version.pk, + ) + elif version == EXTERNAL: + log.info('Updating all external versions for %s', slug) + for version in Version.external.filter( + project__slug=slug, + active=True, + uploaded=False, + ): + + build = Build.objects.create( + project=version.project, + version=version, + type='html', + state='triggered', + ) + + # pylint: disable=no-value-for-parameter + tasks.update_docs_task( + version.project_id, + build_pk=build.pk, + version_pk=version.pk, + ) else: p = Project.all_objects.get(slug=slug) log.info('Building %s', p) diff --git a/readthedocs/core/management/commands/update_versions.py b/readthedocs/core/management/commands/update_versions.py index b0aa1f877cb..6456accc430 100644 --- a/readthedocs/core/management/commands/update_versions.py +++ b/readthedocs/core/management/commands/update_versions.py @@ -13,7 +13,7 @@ class Command(BaseCommand): help = __doc__ def handle(self, *args, **options): - for version in Version.objects.filter(active=True, built=False): + for version in Version.internal.filter(active=True, built=False): # pylint: disable=no-value-for-parameter update_docs_task( version.pk, diff --git a/readthedocs/core/views/__init__.py b/readthedocs/core/views/__init__.py index 6b438866ae5..de519a13759 100644 --- a/readthedocs/core/views/__init__.py +++ b/readthedocs/core/views/__init__.py @@ -73,7 +73,7 @@ def random_page(request, project_slug=None): # pylint: disable=unused-argument def wipe_version(request, project_slug, version_slug): version = get_object_or_404( - Version, + Version.internal.all(), project__slug=project_slug, slug=version_slug, ) diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py index 6154685e5fe..2b8ff3cb417 100644 --- a/readthedocs/core/views/serve.py +++ b/readthedocs/core/views/serve.py @@ -404,7 +404,7 @@ def changefreqs_generator(): raise Http404 sorted_versions = sort_version_aware( - Version.objects.public( + Version.internal.public( project=project, only_active=True, ), @@ -444,7 +444,7 @@ def changefreqs_generator(): if project.translations.exists(): for translation in project.translations.all(): translation_versions = ( - Version.objects.public(project=translation) + Version.internal.public(project=translation) .values_list('slug', flat=True) ) if version.slug in translation_versions: diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index 8b00d7fc94d..1e0da7cde7d 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -239,7 +239,7 @@ def reindex_active_versions(self, request, queryset): """Reindex all active versions of the selected projects to ES.""" qs_iterator = queryset.iterator() for project in qs_iterator: - version_qs = Version.objects.filter(project=project) + version_qs = Version.internal.filter(project=project) active_versions = version_qs.filter(active=True) if not active_versions.exists(): @@ -271,7 +271,7 @@ def wipe_all_versions(self, request, queryset): """Wipe indexes of all versions of selected projects.""" qs_iterator = queryset.iterator() for project in qs_iterator: - version_qs = Version.objects.filter(project=project) + version_qs = Version.internal.filter(project=project) if not version_qs.exists(): self.message_user( request, diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index a701a49c020..3c86cc39e12 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -14,6 +14,7 @@ from guardian.shortcuts import assign from textclassifier.validators import ClassifierValidator +from readthedocs.builds.constants import INTERNAL from readthedocs.core.utils import slugify, trigger_build from readthedocs.core.utils.extend import SettingsOverrideObject from readthedocs.integrations.models import Integration @@ -240,7 +241,7 @@ def __init__(self, *args, **kwargs): self.helper.add_input(Submit('save', _('Save'))) default_choice = (None, '-' * 9) - versions_choices = self.instance.versions.filter( + versions_choices = self.instance.versions(manager=INTERNAL).filter( machine=False).values_list('verbose_name', flat=True) self.fields['default_branch'].widget = forms.Select( diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index b636b348aa8..a3f36feb5c8 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -20,7 +20,7 @@ from taggit.managers import TaggableManager from readthedocs.api.v2.client import api -from readthedocs.builds.constants import LATEST, STABLE +from readthedocs.builds.constants import LATEST, STABLE, INTERNAL from readthedocs.core.resolver import resolve, resolve_domain from readthedocs.core.utils import broadcast, slugify from readthedocs.projects import constants @@ -908,7 +908,7 @@ def api_versions(self): def active_versions(self): from readthedocs.builds.models import Version - versions = Version.objects.public(project=self, only_active=True) + versions = Version.internal.public(project=self, only_active=True) return ( versions.filter(built=True, active=True) | versions.filter(active=True, uploaded=True) @@ -922,7 +922,7 @@ def ordered_active_versions(self, user=None): } if user: kwargs['user'] = user - versions = Version.objects.public(**kwargs).select_related( + versions = Version.internal.public(**kwargs).select_related( 'project', 'project__main_language_project', ).prefetch_related( @@ -949,7 +949,7 @@ def all_active_versions(self): :returns: :py:class:`Version` queryset """ - return self.versions.filter(active=True) + return self.versions(manager=INTERNAL).filter(active=True) def get_stable_version(self): return self.versions.filter(slug=STABLE).first() @@ -961,7 +961,7 @@ def update_stable_version(self): Return ``None`` if no update was made or if there is no version on the project that can be considered stable. """ - versions = self.versions.all() + versions = self.versions(manager=INTERNAL).all() new_stable = determine_stable_version(versions) if new_stable: current_stable = self.get_stable_version() diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 46e83d96188..044a83641e6 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -157,7 +157,7 @@ def project_version_detail(request, project_slug, version_slug): slug=project_slug, ) version = get_object_or_404( - Version.objects.public( + Version.internal.public( user=request.user, project=project, only_active=False, @@ -682,7 +682,7 @@ def project_version_delete_html(request, project_slug, version_slug): slug=project_slug, ) version = get_object_or_404( - Version.objects.public( + Version.internal.public( user=request.user, project=project, only_active=False, diff --git a/readthedocs/projects/views/public.py b/readthedocs/projects/views/public.py index 64d82272738..ea15bfb168b 100644 --- a/readthedocs/projects/views/public.py +++ b/readthedocs/projects/views/public.py @@ -93,7 +93,7 @@ def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) project = self.get_object() - context['versions'] = Version.objects.public( + context['versions'] = Version.internal.public( user=self.request.user, project=project, ) @@ -179,7 +179,7 @@ def project_downloads(request, project_slug): Project.objects.protected(request.user), slug=project_slug, ) - versions = Version.objects.public(user=request.user, project=project) + versions = Version.internal.public(user=request.user, project=project) versions = sort_version_aware(versions) version_data = OrderedDict() for version in versions: @@ -268,7 +268,7 @@ def project_versions(request, project_slug): slug=project_slug, ) - versions = Version.objects.public( + versions = Version.internal.public( user=request.user, project=project, only_active=False, diff --git a/readthedocs/rtd_tests/tests/test_doc_serving.py b/readthedocs/rtd_tests/tests/test_doc_serving.py index fed119bbab5..ea9a198986f 100644 --- a/readthedocs/rtd_tests/tests/test_doc_serving.py +++ b/readthedocs/rtd_tests/tests/test_doc_serving.py @@ -10,7 +10,7 @@ from django.urls import reverse from mock import mock_open, patch -from readthedocs.builds.constants import LATEST +from readthedocs.builds.constants import LATEST, EXTERNAL, INTERNAL from readthedocs.builds.models import Version from readthedocs.core.middleware import SubdomainMiddleware from readthedocs.core.views import server_error_404_subdomain @@ -249,6 +249,16 @@ def test_sitemap_xml(self): project=self.public, active=True ) + # This is a EXTERNAL Version + external_version = fixture.get( + Version, + identifier='pr-version', + verbose_name='pr-version', + slug='pr-9999', + project=self.public, + active=True, + type=EXTERNAL + ) # This also creates a Version `latest` Automatically for this project translation = fixture.get( Project, @@ -268,7 +278,7 @@ def test_sitemap_xml(self): ) self.assertEqual(response.status_code, 200) self.assertEqual(response['Content-Type'], 'application/xml') - for version in self.public.versions.filter(privacy_level=constants.PUBLIC): + for version in self.public.versions(manager=INTERNAL).filter(privacy_level=constants.PUBLIC): self.assertContains( response, self.public.get_docs_url( @@ -303,6 +313,15 @@ 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. + self.assertNotContains( + response, + self.public.get_docs_url( + version_slug=external_version.slug, + lang_slug=self.public.language, + 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 new file mode 100644 index 00000000000..1d5cbd78645 --- /dev/null +++ b/readthedocs/rtd_tests/tests/test_managers.py @@ -0,0 +1,125 @@ +from django.contrib.auth import get_user_model +from django.test import TestCase +from django_dynamic_fixture import get + +from readthedocs.builds.constants import EXTERNAL, BRANCH, TAG +from readthedocs.builds.models import Version +from readthedocs.projects.constants import PUBLIC, PRIVATE, PROTECTED +from readthedocs.projects.models import Project + + +User = get_user_model() + + +class TestVersionManagerBase(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') + # Create a External Version. ie: pull/merge request Version. + self.public_external_version = get( + Version, + project=self.pip, + active=True, + type=EXTERNAL, + privacy_level=PUBLIC + ) + self.private_external_version = get( + Version, + project=self.pip, + active=True, + type=EXTERNAL, + privacy_level=PRIVATE + ) + self.protected_external_version = get( + Version, + project=self.pip, + active=True, + type=EXTERNAL, + privacy_level=PROTECTED + ) + self.internal_versions = Version.objects.exclude(type=EXTERNAL) + + +class TestInternalVersionManager(TestVersionManagerBase): + + """ + Queries using Internal Manager should only include Internal Versions. + + It will exclude EXTERNAL type Versions from the queries + and only include BRANCH, TAG, UNKONWN type Versions. + """ + + def test_internal_version_manager_with_all(self): + self.assertNotIn(self.public_external_version, Version.internal.all()) + + def test_internal_version_manager_with_public(self): + self.assertNotIn(self.public_external_version, Version.internal.public()) + + def test_internal_version_manager_with_public_with_user_and_project(self): + self.assertNotIn( + self.public_external_version, + Version.internal.public(self.user, self.pip) + ) + + def test_internal_version_manager_with_protected(self): + self.assertNotIn(self.protected_external_version, Version.internal.protected()) + + def test_internal_version_manager_with_private(self): + self.assertNotIn(self.private_external_version, Version.internal.private()) + + def test_internal_version_manager_with_api(self): + self.assertNotIn(self.public_external_version, Version.internal.api()) + + def test_internal_version_manager_with_for_project(self): + self.assertNotIn(self.public_external_version, Version.internal.for_project(self.pip)) + + +class TestExternalVersionManager(TestVersionManagerBase): + + """ + Queries using External Manager should only include External Versions. + + It will only include pull/merge request Version in the queries. + """ + + def test_external_version_manager_with_all(self): + self.assertNotIn(self.internal_versions, Version.external.all()) + self.assertIn(self.public_external_version, Version.external.all()) + + def test_external_version_manager_with_public(self): + self.assertNotIn(self.internal_versions, Version.external.public()) + self.assertIn(self.public_external_version, Version.external.public()) + + def test_external_version_manager_with_public_with_user_and_project(self): + self.assertNotIn( + self.internal_versions, + Version.external.public(self.user, self.pip) + ) + self.assertIn( + self.public_external_version, + Version.external.public(self.user, self.pip) + ) + + def test_external_version_manager_with_protected(self): + self.assertNotIn(self.internal_versions, Version.external.protected()) + self.assertIn(self.protected_external_version, Version.external.protected()) + + def test_external_version_manager_with_private(self): + self.assertNotIn(self.internal_versions, Version.external.private()) + self.assertIn(self.private_external_version, Version.external.private()) + + def test_external_version_manager_with_api(self): + self.assertNotIn(self.internal_versions, Version.external.api()) + self.assertIn(self.public_external_version, Version.external.api()) + + def test_external_version_manager_with_for_project(self): + self.assertNotIn( + self.internal_versions, Version.external.for_project(self.pip) + ) + self.assertIn( + self.public_external_version, Version.external.for_project(self.pip) + ) diff --git a/readthedocs/rtd_tests/tests/test_project.py b/readthedocs/rtd_tests/tests/test_project.py index 40f86350ed4..b7c0afa883a 100644 --- a/readthedocs/rtd_tests/tests/test_project.py +++ b/readthedocs/rtd_tests/tests/test_project.py @@ -14,8 +14,9 @@ BUILD_STATE_FINISHED, BUILD_STATE_TRIGGERED, LATEST, + EXTERNAL, ) -from readthedocs.builds.models import Build +from readthedocs.builds.models import Build, Version from readthedocs.projects.exceptions import ProjectConfigurationError from readthedocs.projects.models import Project from readthedocs.projects.tasks import finish_inactive_builds @@ -29,6 +30,16 @@ class ProjectMixin: def setUp(self): self.client.login(username='eric', password='test') self.pip = Project.objects.get(slug='pip') + # Create a External Version. ie: pull/merge request Version. + self.external_version = get( + Version, + identifier='pr-version', + verbose_name='pr-version', + slug='pr-9999', + project=self.pip, + active=True, + type=EXTERNAL + ) class TestProject(ProjectMixin, TestCase): @@ -134,6 +145,21 @@ def test_get_storage_path(self): 'htmlzip/pip/latest/pip.zip', ) + def test_ordered_active_versions_excludes_external_versions(self): + self.assertNotIn(self.external_version, self.pip.ordered_active_versions()) + + def test_active_versions_excludes_external_versions(self): + self.assertNotIn(self.external_version, self.pip.active_versions()) + + def test_all_active_versions_excludes_external_versions(self): + self.assertNotIn(self.external_version, self.pip.all_active_versions()) + + def test_update_stable_version_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 stable. + self.assertEqual(self.pip.update_stable_version(), None) + class TestProjectTranslations(ProjectMixin, TestCase): diff --git a/readthedocs/rtd_tests/tests/test_project_forms.py b/readthedocs/rtd_tests/tests/test_project_forms.py index 50d9815d051..9d2aaa01716 100644 --- a/readthedocs/rtd_tests/tests/test_project_forms.py +++ b/readthedocs/rtd_tests/tests/test_project_forms.py @@ -5,7 +5,7 @@ from django_dynamic_fixture import get from textclassifier.validators import ClassifierValidator -from readthedocs.builds.constants import LATEST, STABLE +from readthedocs.builds.constants import LATEST, STABLE, EXTERNAL from readthedocs.builds.models import Version from readthedocs.projects.constants import ( PRIVATE, @@ -314,7 +314,7 @@ def setUp(self): verbose_name='protected', ) - def test_list_only_non_auto_generated_versions_on_default_branch(self): + def test_list_only_non_auto_generated_versions_in_default_branch_choices(self): form = ProjectAdvancedForm(instance=self.project) # This version is created automatically by the project on save latest = self.project.versions.filter(slug=LATEST) @@ -336,7 +336,7 @@ def test_list_only_non_auto_generated_versions_on_default_branch(self): 'default_branch'].widget.choices], ) - def test_list_user_created_latest_and_stable_versions_on_default_branch(self): + def test_list_user_created_latest_and_stable_versions_in_default_branch_choices(self): self.project.versions.filter(slug=LATEST).first().delete() user_created_latest_version = get( Version, @@ -383,6 +383,25 @@ def test_commit_name_not_in_default_branch_choices(self): 'default_branch'].widget.choices], ) + def test_external_version_not_in_default_branch_choices(self): + external_version = get( + Version, + identifier='pr-version', + verbose_name='pr-version', + slug='pr-9999', + project=self.project, + active=True, + type=EXTERNAL, + privacy_level=PUBLIC, + ) + form = ProjectAdvancedForm(instance=self.project) + + self.assertNotIn( + external_version.verbose_name, + [identifier for identifier, _ in form.fields[ + 'default_branch'].widget.choices], + ) + class TestTranslationForms(TestCase): diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index 7c5b286bdd4..331f6982aa3 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -12,7 +12,7 @@ from django_dynamic_fixture import get, new from mock import patch -from readthedocs.builds.constants import LATEST +from readthedocs.builds.constants import LATEST, EXTERNAL from readthedocs.builds.models import Build, Version from readthedocs.oauth.models import RemoteRepository from readthedocs.projects import tasks @@ -370,12 +370,40 @@ def test_import_demo_imported_duplicate(self): class TestPublicViews(MockBuildTestCase): def setUp(self): self.pip = get(Project, slug='pip') + self.external_version = get( + Version, + identifier='pr-version', + verbose_name='pr-version', + slug='pr-9999', + project=self.pip, + active=True, + type=EXTERNAL + ) def test_project_download_media(self): url = reverse('project_download_media', args=[self.pip.slug, 'pdf', LATEST]) response = self.client.get(url) self.assertEqual(response.status_code, 302) + def test_project_detail_view_only_shows_internal_versons(self): + url = reverse('projects_detail', args=[self.pip.slug]) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertNotIn(self.external_version, response.context['versions']) + + def test_project_downloads_only_shows_internal_versons(self): + url = reverse('project_downloads', args=[self.pip.slug]) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertNotIn(self.external_version, response.context['versions']) + + def test_project_versions_only_shows_internal_versons(self): + url = reverse('project_version_list', args=[self.pip.slug]) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertNotIn(self.external_version, response.context['active_versions']) + self.assertNotIn(self.external_version, response.context['inactive_versions']) + class TestPrivateViews(MockBuildTestCase): def setUp(self): diff --git a/readthedocs/rtd_tests/tests/test_version.py b/readthedocs/rtd_tests/tests/test_version.py new file mode 100644 index 00000000000..6ae29c3e241 --- /dev/null +++ b/readthedocs/rtd_tests/tests/test_version.py @@ -0,0 +1,68 @@ +from django.test import TestCase +from django_dynamic_fixture import get + +from readthedocs.builds.constants import EXTERNAL, BRANCH, TAG +from readthedocs.builds.models import Version +from readthedocs.projects.models import Project + + +class VersionMixin: + + fixtures = ['eric', 'test_data'] + + def setUp(self): + self.client.login(username='eric', password='test') + self.pip = Project.objects.get(slug='pip') + # Create a External Version. ie: pull/merge request Version. + self.external_version = get( + Version, + identifier='9F86D081884C7D659A2FEAA0C55AD015A', + verbose_name='9999', + slug='pr-9999', + project=self.pip, + active=True, + type=EXTERNAL + ) + self.branch_version = get( + Version, + identifier='origin/stable', + verbose_name='stable', + slug='stable', + project=self.pip, + active=True, + type=BRANCH + ) + self.tag_version = get( + Version, + identifier='origin/master', + verbose_name='latest', + slug='latest', + project=self.pip, + active=True, + type=TAG + ) + + +class TestVersionModel(VersionMixin, TestCase): + + def test_vcs_url_for_external_version(self): + expected_url = f'https://github.com/pypa/pip/pull/{self.external_version.verbose_name}/' + self.assertEqual(self.external_version.vcs_url, expected_url) + + def test_vcs_url_for_latest_version(self): + slug = self.pip.default_branch or self.pip.vcs_repo().fallback_branch + expected_url = f'https://github.com/pypa/pip/tree/{slug}/' + self.assertEqual(self.tag_version.vcs_url, expected_url) + + def test_vcs_url_for_stable_version(self): + expected_url = f'https://github.com/pypa/pip/tree/{self.branch_version.ref}/' + self.assertEqual(self.branch_version.vcs_url, expected_url) + + def test_commit_name_for_stable_version(self): + self.assertEqual(self.branch_version.commit_name, 'stable') + + def test_commit_name_for_latest_version(self): + self.assertEqual(self.tag_version.commit_name, 'master') + + def test_commit_name_for_external_version(self): + self.assertEqual(self.external_version.commit_name, self.external_version.identifier) diff --git a/readthedocs/search/utils.py b/readthedocs/search/utils.py index 7eca59dbb45..6cbc0272722 100644 --- a/readthedocs/search/utils.py +++ b/readthedocs/search/utils.py @@ -88,7 +88,9 @@ def get_project_list_or_404(project_slug, user, version_slug=None): main_project = get_object_or_404(Project, slug=project_slug) subprojects = Project.objects.filter(superprojects__parent_id=main_project.id) for project in list(subprojects) + [main_project]: - version = Version.objects.public(user).filter(project__slug=project.slug, slug=version_slug) + version = Version.internal.public(user).filter( + project__slug=project.slug, slug=version_slug + ) if version.exists(): project_list.append(version.first().project) return project_list