diff --git a/readthedocs/builds/constants.py b/readthedocs/builds/constants.py index 26c988121ae..7390cfc980b 100644 --- a/readthedocs/builds/constants.py +++ b/readthedocs/builds/constants.py @@ -25,10 +25,14 @@ ('dash', _('Dash')), ) +BRANCH = 'branch' +TAG = 'tag' +UNKNOWN = 'unknown' + VERSION_TYPES = ( - ('branch', _('Branch')), - ('tag', _('Tag')), - ('unknown', _('Unknown')), + (BRANCH, _('Branch')), + (TAG, _('Tag')), + (UNKNOWN, _('Unknown')), ) LATEST = 'latest' diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index b53a3e3616b..c5b5e3241da 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -21,7 +21,7 @@ from .constants import (BUILD_STATE, BUILD_TYPES, VERSION_TYPES, LATEST, NON_REPOSITORY_VERSIONS, STABLE, - BUILD_STATE_FINISHED) + BUILD_STATE_FINISHED, BRANCH, TAG) from .version_slug import VersionSlugField @@ -99,12 +99,46 @@ def __unicode__(self): @property def commit_name(self): - """Return the branch name, the tag name or the revision identifier.""" - if self.type == 'branch': - return self.identifier - if self.verbose_name in NON_REPOSITORY_VERSIONS: + """ + Return the branch name, the tag name or the revision identifier. + + The result could be used as ref in a git repo, e.g. for linking to + GitHub or Bitbucket. + """ + + # LATEST is special as it is usually a branch but does not contain the + # name in verbose_name. + if self.slug == LATEST: + if self.project.default_branch: + return self.project.default_branch + else: + return self.project.vcs_repo().fallback_branch + + if self.slug == STABLE: + if self.type == BRANCH: + # Special case, as we do not store the original branch name + # that the stable version works on. We can only interpolate the + # name from the commit identifier, but it's hacky. + # TODO: Refactor ``Version`` to store more actual info about + # the underlying commits. + if self.identifier.startswith('origin/'): + return self.identifier[len('origin/'):] return self.identifier - return self.verbose_name + + # By now we must have handled all special versions. + assert self.slug not in NON_REPOSITORY_VERSIONS + + if self.type in (BRANCH, TAG): + # If this version is a branch or a tag, the verbose_name will + # contain the actual name. We cannot use identifier as this might + # include the "origin/..." part in the case of a branch. A tag + # would contain the hash in identifier, which is not as pretty as + # the actual tag name. + return self.verbose_name + + # If we came that far it's not a special version nor a branch or tag. + # Therefore just return the identifier to make a safe guess. + return self.identifier def get_absolute_url(self): if not self.built and not self.uploaded: @@ -124,16 +158,6 @@ def save(self, *args, **kwargs): self.project.sync_supported_versions() return obj - @property - def remote_slug(self): - if self.slug == LATEST: - if self.project.default_branch: - return self.project.default_branch - else: - return self.project.vcs_repo().fallback_branch - else: - return self.slug - @property def identifier_friendly(self): '''Return display friendly identifier''' @@ -211,24 +235,6 @@ def clean_build_path(self): except OSError: log.error('Build path cleanup failed', exc_info=True) - def get_vcs_slug(self): - slug = None - if self.slug == LATEST: - if self.project.default_branch: - slug = self.project.default_branch - else: - slug = self.project.vcs_repo().fallback_branch - elif self.slug == STABLE: - return self.identifier - else: - slug = self.slug - # https://github.com/rtfd/readthedocs.org/issues/561 - # version identifiers with / characters in branch name need to un-slugify - # the branch name for remote links to work - if slug.replace('-', '/') in self.identifier: - slug = slug.replace('-', '/') - return slug - def get_github_url(self, docroot, filename, source_suffix='.rst', action='view'): repo_url = self.project.repo if 'github' not in repo_url: @@ -259,7 +265,7 @@ def get_github_url(self, docroot, filename, source_suffix='.rst', action='view') return GITHUB_URL.format( user=user, repo=repo, - version=self.remote_slug, + version=self.commit_name, docroot=docroot, path=filename, source_suffix=source_suffix, @@ -285,7 +291,7 @@ def get_bitbucket_url(self, docroot, filename, source_suffix='.rst'): return BITBUCKET_URL.format( user=user, repo=repo, - version=self.remote_slug, + version=self.commit_name, docroot=docroot, path=filename, source_suffix=source_suffix, diff --git a/readthedocs/doc_builder/backends/sphinx.py b/readthedocs/doc_builder/backends/sphinx.py index c7acb28ca68..e2d76108e4d 100644 --- a/readthedocs/doc_builder/backends/sphinx.py +++ b/readthedocs/doc_builder/backends/sphinx.py @@ -10,6 +10,7 @@ from django.conf import settings from readthedocs.builds import utils as version_utils +from readthedocs.builds.constants import BRANCH from readthedocs.projects.utils import safe_write from readthedocs.projects.exceptions import ProjectImportError from readthedocs.restapi.client import api @@ -72,16 +73,16 @@ def append_conf(self, **kwargs): raise ProjectImportError('Conf file not found'), None, trace outfile.write("\n") conf_py_path = self.version.get_conf_py_path() - remote_version = self.version.get_vcs_slug() + remote_version = self.version.commit_name github_user, github_repo = version_utils.get_github_username_repo( url=self.project.repo) - github_version_is_editable = (self.version.type == 'branch') + github_version_is_editable = (self.version.type == BRANCH) display_github = github_user is not None bitbucket_user, bitbucket_repo = version_utils.get_bitbucket_username_repo( url=self.project.repo) - bitbucket_version_is_editable = (self.version.type == 'branch') + bitbucket_version_is_editable = (self.version.type == BRANCH) display_bitbucket = bitbucket_user is not None rtd_ctx = { diff --git a/readthedocs/privacy/backend.py b/readthedocs/privacy/backend.py index fdd8806024e..f761ce4f0ec 100644 --- a/readthedocs/privacy/backend.py +++ b/readthedocs/privacy/backend.py @@ -3,6 +3,8 @@ from guardian.shortcuts import get_objects_for_user +from readthedocs.builds.constants import BRANCH +from readthedocs.builds.constants import TAG from readthedocs.builds.constants import LATEST from readthedocs.builds.constants import LATEST_VERBOSE_NAME from readthedocs.builds.constants import STABLE @@ -152,7 +154,7 @@ def create_stable(self, **kwargs): 'machine': True, 'active': True, 'identifier': STABLE, - 'type': 'tag', + 'type': TAG, } defaults.update(kwargs) return self.create(**defaults) @@ -164,7 +166,7 @@ def create_latest(self, **kwargs): 'machine': True, 'active': True, 'identifier': LATEST, - 'type': 'branch', + 'type': BRANCH, } defaults.update(kwargs) return self.create(**defaults) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index c6efccb4e99..5cb728b4471 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -12,6 +12,7 @@ from guardian.shortcuts import assign +from readthedocs.builds.constants import TAG from readthedocs.core.utils import trigger_build from readthedocs.redirects.models import Redirect from readthedocs.projects import constants @@ -272,7 +273,7 @@ def build_versions_form(project): for version in versions_qs: field_name = 'version-%s' % version.slug privacy_name = 'privacy-%s' % version.slug - if version.type == 'tag': + if version.type == TAG: label = "%s (%s)" % (version.verbose_name, version.identifier[:8]) else: label = version.verbose_name diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 088497783f8..ba670853ae6 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -312,7 +312,7 @@ def save(self, *args, **kwargs): if not self.versions.filter(slug=LATEST).exists(): self.versions.create_latest(identifier=branch) # if not self.versions.filter(slug=STABLE).exists(): - # self.versions.create_stable(type='branch', identifier=branch) + # self.versions.create_stable(type=BRANCH, identifier=branch) except Exception: log.error('Error creating default branches', exc_info=True) diff --git a/readthedocs/restapi/views/footer_views.py b/readthedocs/restapi/views/footer_views.py index 51e58f4f7b7..d4a4eef1dfb 100644 --- a/readthedocs/restapi/views/footer_views.py +++ b/readthedocs/restapi/views/footer_views.py @@ -8,6 +8,7 @@ from rest_framework.response import Response from readthedocs.builds.constants import LATEST +from readthedocs.builds.constants import TAG from readthedocs.builds.models import Version from readthedocs.donate.models import SupporterPromo from readthedocs.projects.models import Project @@ -74,7 +75,7 @@ def footer_html(request): else: path = "" - if version.type == 'tag' and version.project.has_pdf(version.slug): + if version.type == TAG and version.project.has_pdf(version.slug): print_url = ( 'https://keminglabs.com/print-the-docs/quote?project={project}&version={version}' .format( diff --git a/readthedocs/restapi/views/model_views.py b/readthedocs/restapi/views/model_views.py index 711ee386786..309d3eae6d7 100644 --- a/readthedocs/restapi/views/model_views.py +++ b/readthedocs/restapi/views/model_views.py @@ -7,6 +7,8 @@ from rest_framework.renderers import JSONPRenderer, JSONRenderer, BrowsableAPIRenderer from rest_framework.response import Response +from readthedocs.builds.constants import BRANCH +from readthedocs.builds.constants import TAG from readthedocs.builds.filters import VersionFilter from readthedocs.builds.models import Build, BuildCommandResult, Version from readthedocs.core.utils import trigger_build @@ -109,11 +111,11 @@ def sync_versions(self, request, **kwargs): added_versions = set() if 'tags' in data: ret_set = api_utils.sync_versions( - project=project, versions=data['tags'], type='tag') + project=project, versions=data['tags'], type=TAG) added_versions.update(ret_set) if 'branches' in data: ret_set = api_utils.sync_versions( - project=project, versions=data['branches'], type='branch') + project=project, versions=data['branches'], type=BRANCH) added_versions.update(ret_set) deleted_versions = api_utils.delete_versions(project, data) except Exception, e: diff --git a/readthedocs/rtd_tests/tests/test_supported.py b/readthedocs/rtd_tests/tests/test_supported.py index 3dc33d219f1..a6dfccc1664 100644 --- a/readthedocs/rtd_tests/tests/test_supported.py +++ b/readthedocs/rtd_tests/tests/test_supported.py @@ -2,6 +2,7 @@ from django.test import TestCase +from readthedocs.builds.constants import TAG from readthedocs.builds.models import Version from readthedocs.projects.models import Project @@ -13,11 +14,11 @@ def setUp(self): self.pip = Project.objects.create(name='Pip', slug='pip') Version.objects.create(project=self.pip, identifier='0.1', verbose_name='0.1', slug='0.1', - type='tag', + type=TAG, active=True) Version.objects.create(project=self.pip, identifier='0.2', verbose_name='0.2', slug='0.2', - type='tag', + type=TAG, active=True) def test_supported_versions(self): @@ -55,6 +56,6 @@ def test_adding_version_updates_supported(self): self.assertEqual(self.pip.versions.get(slug='0.1').supported, False) self.assertEqual(self.pip.versions.get(slug='0.2').supported, True) Version.objects.create(project=self.pip, identifier='0.1.1', - verbose_name='0.1.1', type='tag', active=True) + verbose_name='0.1.1', type=TAG, active=True) # This gets set to False on creation. self.assertEqual(self.pip.versions.get(slug='0.1.1').supported, False) diff --git a/readthedocs/rtd_tests/tests/test_version_commit_name.py b/readthedocs/rtd_tests/tests/test_version_commit_name.py new file mode 100644 index 00000000000..0be61131d24 --- /dev/null +++ b/readthedocs/rtd_tests/tests/test_version_commit_name.py @@ -0,0 +1,50 @@ +from django.test import TestCase +from django_dynamic_fixture import get +from django_dynamic_fixture import new + +from readthedocs.builds.constants import BRANCH +from readthedocs.builds.constants import LATEST +from readthedocs.builds.constants import STABLE +from readthedocs.builds.constants import TAG +from readthedocs.builds.models import Version +from readthedocs.projects.constants import REPO_TYPE_GIT +from readthedocs.projects.constants import REPO_TYPE_HG +from readthedocs.projects.models import Project + + +class VersionCommitNameTests(TestCase): + def test_branch_name(self): + version = new(Version, identifier='release-2.5.x', + slug='release-2.5.x', verbose_name='release-2.5.x', + type=BRANCH) + self.assertEqual(version.commit_name, 'release-2.5.x') + + def test_tag_name(self): + version = new(Version, identifier='10f1b29a2bd2', slug='release-2.5.0', + verbose_name='release-2.5.0', type=TAG) + self.assertEqual(version.commit_name, 'release-2.5.0') + + def test_branch_with_name_stable(self): + version = new(Version, identifier='origin/stable', slug=STABLE, + verbose_name='stable', type=BRANCH) + self.assertEqual(version.commit_name, 'stable') + + def test_stable_version_tag(self): + version = new(Version, + identifier='3d92b728b7d7b842259ac2020c2fa389f13aff0d', + slug=STABLE, verbose_name=STABLE, type=TAG) + self.assertEqual(version.commit_name, + '3d92b728b7d7b842259ac2020c2fa389f13aff0d') + + def test_hg_latest_branch(self): + hg_project = get(Project, repo_type=REPO_TYPE_HG) + version = new(Version, identifier='default', slug=LATEST, + verbose_name=LATEST, type=BRANCH, project=hg_project) + self.assertEqual(version.commit_name, 'default') + + def test_git_latest_branch(self): + git_project = get(Project, repo_type=REPO_TYPE_GIT) + version = new(Version, project=git_project, + identifier='origin/master', slug=LATEST, + verbose_name=LATEST, type=BRANCH) + self.assertEqual(version.commit_name, 'master')