Merge pull request #3331 from rtfd/humitos/versions/tags-over-branches

Take preferece of tags over branches when selecting the stable version

.flake8

@@ -1,3 +1,3 @@
 [flake8]
-ignore = E125,D100,D101,D102,D105,D107,D200,D211,P101,FI15,FI16,FI12,FI11,FI17,FI50,FI53,FI54
+ignore = E125,D100,D101,D102,D105,D107,D200,D211,P101,FI15,FI16,FI12,FI11,FI17,FI50,FI53,FI54,T000,MQ101
 max-line-length = 80

.isort.cfg

@@ -2,8 +2,8 @@
 line_length=80
 indent='    '
 multi_line_output=4
-default_section=FIRSTPARTY
-known_firstparty=readthedocs,readthedocsinc
-known_third_party=celery,stripe,requests,pytz,builtins,django,annoying,readthedocs_build
+default_section=THIRDPARTY
+known_first_party=readthedocs,readthedocsinc
+known_third_party=mock
 sections=FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,LOCALFOLDER
 add_imports=from __future__ import division, from __future__ import print_function, from __future__ import unicode_literals

readthedocs/projects/version_handling.py

@@ -1,24 +1,37 @@
-"""Project version handling"""
-from __future__ import absolute_import
+# -*- coding: utf-8 -*-
+"""Project version handling."""
+from __future__ import (
+    absolute_import, division, print_function, unicode_literals)
 
 import unicodedata
+from builtins import object, range
 from collections import defaultdict
 
-from builtins import (object, range)
-from packaging.version import Version
-from packaging.version import InvalidVersion
 import six
+from packaging.version import InvalidVersion, Version
 
-from readthedocs.builds.constants import LATEST_VERBOSE_NAME
-from readthedocs.builds.constants import STABLE_VERBOSE_NAME
+from readthedocs.builds.constants import (
+    LATEST_VERBOSE_NAME, STABLE_VERBOSE_NAME, TAG)
 
 
 def get_major(version):
+    """
+    Return the major version.
+
+    :param version: version to get the major
+    :type version: packaging.version.Version
+    """
     # pylint: disable=protected-access
     return version._version.release[0]
 
 
 def get_minor(version):
+    """
+    Return the minor version.
+
+    :param version: version to get the minor
+    :type version: packaging.version.Version
+    """
     # pylint: disable=protected-access
     try:
         return version._version.release[1]
@@ -28,7 +41,7 @@ def get_minor(version):
 
 class VersionManager(object):
 
-    """Prune list of versions based on version windows"""
+    """Prune list of versions based on version windows."""
 
     def __init__(self):
         self._state = defaultdict(lambda: defaultdict(list))
@@ -72,13 +85,13 @@ def get_version_list(self):
                 versions.extend(version_list)
         versions = sorted(versions)
         return [
-            version.public
-            for version in versions
-            if not version.is_prerelease]
+            version.public for version in versions if not version.is_prerelease
+        ]
 
 
 def version_windows(versions, major=1, minor=1, point=1):
-    """Return list of versions that have been pruned to version windows
+    """
+    Return list of versions that have been pruned to version windows.
 
     Uses :py:class:`VersionManager` to prune the list of versions
 
@@ -111,6 +124,18 @@ def version_windows(versions, major=1, minor=1, point=1):
 
 
 def parse_version_failsafe(version_string):
+    """
+    Parse a version in string form and return Version object.
+
+    If there is an error parsing the string, ``None`` is returned.
+
+    :param version_string: version as string object (e.g. '3.10.1')
+    :type version_string: str or unicode
+
+    :returns: version object created from a string object
+
+    :rtype: packaging.version.Version
+    """
     if not isinstance(version_string, six.text_type):
         uni_version = version_string.decode('utf-8')
     else:
@@ -126,11 +151,19 @@ def parse_version_failsafe(version_string):
 
 
 def comparable_version(version_string):
-    """This can be used as ``key`` argument to ``sorted``.
+    """
+    Can be used as ``key`` argument to ``sorted``.
 
     The ``LATEST`` version shall always beat other versions in comparison.
     ``STABLE`` should be listed second. If we cannot figure out the version
     number then we sort it to the bottom of the list.
+
+    :param version_string: version as string object (e.g. '3.10.1' or 'latest')
+    :type version_string: str or unicode
+
+    :returns: a comparable version object (e.g. 'latest' -> Version('99999.0'))
+
+    :rtype: packaging.version.Version
     """
     comparable = parse_version_failsafe(version_string)
     if not comparable:
@@ -144,12 +177,16 @@ def comparable_version(version_string):
 
 
 def sort_versions(version_list):
-    """Takes a list of ``Version`` models and return a sorted list,
+    """
+    Take a list of Version models and return a sorted list.
 
-    The returned value is a list of two-tuples. The first is the actual
-    ``Version`` model instance, the second is an instance of
-    ``packaging.version.Version``. They are ordered in descending order (latest
-    version first).
+    :param version_list: list of Version models
+    :type version_list: list(readthedocs.builds.models.Version)
+
+    :returns: sorted list in descending order (latest version first) of versions
+
+    :rtype: list(tupe(readthedocs.builds.models.Version,
+            packaging.version.Version))
     """
     versions = []
     for version_obj in version_list:
@@ -158,32 +195,50 @@ def sort_versions(version_list):
         if comparable_version:
             versions.append((version_obj, comparable_version))
 
-    return list(sorted(
-        versions,
-        key=lambda version_info: version_info[1],
-        reverse=True))
+    return list(
+        sorted(
+            versions,
+            key=lambda version_info: version_info[1],
+            reverse=True,
+        ))
 
 
 def highest_version(version_list):
+    """
+    Return the highest version for a given ``version_list``.
+
+    :rtype: tupe(readthedocs.builds.models.Version, packaging.version.Version)
+    """
     versions = sort_versions(version_list)
     if versions:
         return versions[0]
     return (None, None)
 
 
 def determine_stable_version(version_list):
-    """Determine a stable version for version list
+    """
+    Determine a stable version for version list.
+
+    :param version_list: list of versions
+    :type version_list: list(readthedocs.builds.models.Version)
 
-    Takes a list of ``Version`` model instances and returns the version
-    instance which can be considered the most recent stable one. It will return
-    ``None`` if there is no stable version in the list.
+    :returns: version considered the most recent stable one or ``None`` if there
+              is no stable version in the list
+
+    :rtype: readthedocs.builds.models.Version
     """
     versions = sort_versions(version_list)
-    versions = [
-        (version_obj, comparable)
-        for version_obj, comparable in versions
-        if not comparable.is_prerelease]
+    versions = [(version_obj, comparable)
+                for version_obj, comparable in versions
+                if not comparable.is_prerelease]
+
     if versions:
+        # We take preference for tags over branches. If we don't find any tag,
+        # we just return the first branch found.
+        for version_obj, comparable in versions:
+            if version_obj.type == TAG:
+                return version_obj
+
         version_obj, comparable = versions[0]
         return version_obj
     return None

readthedocs/restapi/utils.py

@@ -13,12 +13,9 @@
 
 def sync_versions(project, versions, type):  # pylint: disable=redefined-builtin
     """Update the database with the current versions from the repository."""
-    # Bookkeeping for keeping tag/branch identifies correct
-    verbose_names = [v['verbose_name'] for v in versions]
-    project.versions.filter(verbose_name__in=verbose_names).update(type=type)
-
     old_versions = {}
-    old_version_values = project.versions.values('identifier', 'verbose_name')
+    old_version_values = project.versions.filter(type=type).values(
+        'identifier', 'verbose_name')
     for version in old_version_values:
         old_versions[version['verbose_name']] = version['identifier']
 

readthedocs/restapi/views/footer_views.py

@@ -1,35 +1,41 @@
+# -*- coding: utf-8 -*-
 """Endpoint to generate footer HTML."""
 
-from __future__ import absolute_import
+from __future__ import (
+    absolute_import, division, print_function, unicode_literals)
 
-from django.shortcuts import get_object_or_404
-from django.template import RequestContext, loader as template_loader
+import six
 from django.conf import settings
-
-
+from django.shortcuts import get_object_or_404
+from django.template import loader as template_loader
 from rest_framework import decorators, permissions
 from rest_framework.renderers import JSONRenderer
 from rest_framework.response import Response
 from rest_framework_jsonp.renderers import JSONPRenderer
 
-from readthedocs.builds.constants import LATEST
-from readthedocs.builds.constants import TAG
+from readthedocs.builds.constants import LATEST, TAG
 from readthedocs.builds.models import Version
 from readthedocs.projects.models import Project
-from readthedocs.projects.version_handling import highest_version
-from readthedocs.projects.version_handling import parse_version_failsafe
+from readthedocs.projects.version_handling import (
+    highest_version, parse_version_failsafe)
 from readthedocs.restapi.signals import footer_response
-import six
 
 
 def get_version_compare_data(project, base_version=None):
-    """Retrieve metadata about the highest version available for this project.
+    """
+    Retrieve metadata about the highest version available for this project.
 
     :param base_version: We assert whether or not the base_version is also the
                          highest version in the resulting "is_highest" value.
     """
+    versions_qs = project.versions.public().filter(active=True)
+
+    # Take preferences over tags only if the project has at least one tag
+    if versions_qs.filter(type=TAG).exists():
+        versions_qs = versions_qs.filter(type=TAG)
+
     highest_version_obj, highest_version_comparable = highest_version(
-        project.versions.public().filter(active=True))
+        versions_qs)
     ret_val = {
         'project': six.text_type(highest_version_obj),
         'version': six.text_type(highest_version_comparable),
@@ -69,32 +75,30 @@ def footer_html(request):
     subproject = request.GET.get('subproject', False)
     source_suffix = request.GET.get('source_suffix', '.rst')
 
-    new_theme = (theme == "sphinx_rtd_theme")
-    using_theme = (theme == "default")
+    new_theme = (theme == 'sphinx_rtd_theme')
+    using_theme = (theme == 'default')
     project = get_object_or_404(Project, slug=project_slug)
     version = get_object_or_404(
-        Version.objects.public(request.user, project=project, only_active=False),
+        Version.objects.public(
+            request.user, project=project, only_active=False),
         slug=version_slug)
     main_project = project.main_language_project or project
 
-    if page_slug and page_slug != "index":
-        if (
-                main_project.documentation_type == "sphinx_htmldir" or
-                main_project.documentation_type == "mkdocs"):
-            path = page_slug + "/"
-        elif main_project.documentation_type == "sphinx_singlehtml":
-            path = "index.html#document-" + page_slug
+    if page_slug and page_slug != 'index':
+        if (main_project.documentation_type == 'sphinx_htmldir' or
+                main_project.documentation_type == 'mkdocs'):
+            path = page_slug + '/'
+        elif main_project.documentation_type == 'sphinx_singlehtml':
+            path = 'index.html#document-' + page_slug
         else:
-            path = page_slug + ".html"
+            path = page_slug + '.html'
     else:
-        path = ""
+        path = ''
 
     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(
-                project=project.slug,
-                version=version.slug))
+            'https://keminglabs.com/print-the-docs/quote?project={project}&version={version}'  # noqa
+            .format(project=project.slug, version=version.slug))
     else:
         print_url = None
 
@@ -115,23 +119,38 @@ def footer_html(request):
         'settings': settings,
         'subproject': subproject,
         'print_url': print_url,
-        'github_edit_url': version.get_github_url(docroot, page_slug, source_suffix, 'edit'),
-        'github_view_url': version.get_github_url(docroot, page_slug, source_suffix, 'view'),
-        'bitbucket_url': version.get_bitbucket_url(docroot, page_slug, source_suffix),
+        'github_edit_url': version.get_github_url(
+            docroot,
+            page_slug,
+            source_suffix,
+            'edit',
+        ),
+        'github_view_url': version.get_github_url(
+            docroot,
+            page_slug,
+            source_suffix,
+            'view',
+        ),
+        'bitbucket_url': version.get_bitbucket_url(
+            docroot,
+            page_slug,
+            source_suffix,
+        ),
         'theme': theme,
     }
 
-    html = template_loader.get_template('restapi/footer.html').render(context,
-                                                                      request)
+    html = template_loader.get_template('restapi/footer.html').render(
+        context, request)
     resp_data = {
         'html': html,
         'version_active': version.active,
         'version_compare': version_compare_data,
         'version_supported': version.supported,
     }
 
-    # Allow folks to hook onto the footer response for various information collection,
-    # or to modify the resp_data.
-    footer_response.send(sender=None, request=request, context=context, resp_data=resp_data)
+    # Allow folks to hook onto the footer response for various information
+    # collection, or to modify the resp_data.
+    footer_response.send(
+        sender=None, request=request, context=context, resp_data=resp_data)
 
     return Response(resp_data)