Merge pull request #8237 from abravalheri/improve-pr-version-name

Improve displayed version name when building from PR

docs/user/builds.rst

@@ -80,10 +80,17 @@ The *Sphinx* and *Mkdocs* builders set the following RTD-specific environment va
    :widths: 15, 10, 30
 
    ``READTHEDOCS``, Whether the build is running inside RTD, ``True``
-   ``READTHEDOCS_VERSION``, The RTD name of the version which is being built, ``latest``
+   ``READTHEDOCS_VERSION``, The RTD slug of the version which is being built, ``latest``
+   ``READTHEDOCS_VERSION_NAME``, Corresponding version name as displayed in RTD's version switch menu, ``stable``
+   ``READTHEDOCS_VERSION_TYPE``, Type of the event triggering the build, ``branch`` | ``tag`` | ``external`` (for :doc:`pull request builds </pull-requests>`) | ``unknown``
    ``READTHEDOCS_PROJECT``, The RTD slug of the project which is being built, ``my-example-project``
    ``READTHEDOCS_LANGUAGE``, The RTD language slug of the project which is being built, ``en``
 
+.. note::
+
+   The term slug is used to refer to a unique string across projects/versions containing ASCII characters only.
+   This value is used in the URLs of your documentation.
+
 
 .. tip::
 

readthedocs/api/v2/templates/restapi/footer.html

@@ -6,7 +6,7 @@
   <div class="rst-versions rst-badge" data-toggle="rst-versions">
     <span class="rst-current-version" data-toggle="rst-current-version">
       <span class="fa fa-book">&nbsp;</span>
-      v: {{ current_version }}
+      v: {{ current_version.explicit_name }}
       <span class="fa fa-caret-down"></span>
     </span>
     <div class="rst-other-versions">
@@ -18,7 +18,7 @@
         <dt>{% trans "Languages" %}</dt>
 
         {# Output the main project language since it isn't included in translations list #}
-        
+
         <dd {% if main_project.language == current_language %} class="rtd-current-item" {% endif %}>
           <a href="{{ main_project.get_docs_url }}{{ path }}">{{ main_project.language }}</a>
         </dd>
@@ -41,8 +41,8 @@
       <dl>
         <dt>{% trans "Versions" %}</dt>
         {% for version in versions %}
-        <dd {% if version.verbose_name == current_version %} class="rtd-current-item" {% endif %}>
-          <a href="{{ version.get_subdomain_url }}{{ path|default_if_none:"" }}">{{ version.slug }}</a>
+        <dd {% if version == current_version %} class="rtd-current-item" {% endif %}>
+          <a href="{{ version.get_subdomain_url }}{{ path|default_if_none:"" }}">{{ version.verbose_name }}</a>
         </dd>
         {% endfor %}
       </dl>

readthedocs/api/v2/views/footer_views.py

@@ -1,9 +1,9 @@
 """Endpoint to generate footer HTML."""
 
-import structlog
 import re
 from functools import lru_cache
 
+import structlog
 from django.conf import settings
 from django.shortcuts import get_object_or_404
 from django.template import loader as template_loader
@@ -162,18 +162,18 @@ def _get_context(self):
                 path = page_slug + '.html'
 
         context = {
-            'project': project,
-            'version': version,
-            'path': path,
-            'downloads': version.get_downloads(pretty=True),
-            'current_version': version.verbose_name,
-            'versions': self._get_active_versions_sorted(),
-            'main_project': main_project,
-            'translations': main_project.translations.all(),
-            'current_language': project.language,
-            'new_theme': new_theme,
-            'settings': settings,
-            'github_edit_url': version.get_github_url(
+            "project": project,
+            "version": version,
+            "path": path,
+            "downloads": version.get_downloads(pretty=True),
+            "current_version": version,
+            "versions": self._get_active_versions_sorted(),
+            "main_project": main_project,
+            "translations": main_project.translations.all(),
+            "current_language": project.language,
+            "new_theme": new_theme,
+            "settings": settings,
+            "github_edit_url": version.get_github_url(
                 docroot,
                 page_slug,
                 source_suffix,

readthedocs/builds/models.py

@@ -14,10 +14,7 @@
 from django.utils import timezone
 from django.utils.translation import gettext
 from django.utils.translation import gettext_lazy as _
-from django_extensions.db.fields import (
-    CreationDateTimeField,
-    ModificationDateTimeField,
-)
+from django_extensions.db.fields import CreationDateTimeField, ModificationDateTimeField
 from django_extensions.db.models import TimeStampedModel
 from polymorphic.models import PolymorphicModel
 
@@ -30,9 +27,6 @@
     BUILD_STATUS_CHOICES,
     BUILD_TYPES,
     EXTERNAL,
-    GENERIC_EXTERNAL_VERSION_NAME,
-    GITHUB_EXTERNAL_VERSION_NAME,
-    GITLAB_EXTERNAL_VERSION_NAME,
     INTERNAL,
     LATEST,
     NON_REPOSITORY_VERSIONS,
@@ -57,6 +51,7 @@
     VersionQuerySet,
 )
 from readthedocs.builds.utils import (
+    external_version_name,
     get_bitbucket_username_repo,
     get_github_username_repo,
     get_gitlab_username_repo,
@@ -211,6 +206,27 @@ def is_private(self):
     def is_external(self):
         return self.type == EXTERNAL
 
+    @property
+    def explicit_name(self):
+        """
+        Version name that is explicit about external origins.
+
+        For example, if a version originates from GitHub pull request #4, then
+        ``version.explicit_name == "#4 (PR)"``.
+
+        On the other hand, Versions associated with regular RTD builds
+        (e.g. new tags or branches), simply return :obj:`~.verbose_name`.
+        This means that a regular git tag named **v4** will correspond to
+        ``version.explicit_name == "v4"``.
+        """
+        if not self.is_external:
+            return self.verbose_name
+
+        template = "#{name} ({abbrev})"
+        external_origin = external_version_name(self)
+        abbrev = "".join(word[0].upper() for word in external_origin.split())
+        return template.format(name=self.verbose_name, abbrev=abbrev)
+
     @property
     def ref(self):
         if self.slug == STABLE:
@@ -946,16 +962,7 @@ def can_rebuild(self):
 
     @property
     def external_version_name(self):
-        if self.is_external:
-            if self.project.git_provider_name == GITHUB_BRAND:
-                return GITHUB_EXTERNAL_VERSION_NAME
-
-            if self.project.git_provider_name == GITLAB_BRAND:
-                return GITLAB_EXTERNAL_VERSION_NAME
-
-            # TODO: Add External Version Name for BitBucket.
-            return GENERIC_EXTERNAL_VERSION_NAME
-        return None
+        return external_version_name(self)
 
     def using_latest_config(self):
         if self.config:
@@ -1324,7 +1331,7 @@ def match(self, version, match_arg):
                 pattern=match_arg,
                 version_slug=version.slug,
             )
-        except Exception as e:
+        except Exception:
             log.exception('Error parsing regex.', exc_info=True)
         return False, None
 

readthedocs/builds/utils.py

@@ -1,15 +1,21 @@
 """Utilities for the builds app."""
-from time import monotonic
-
 from contextlib import contextmanager
+from time import monotonic
 
 from django.core.cache import cache
 
-from readthedocs.builds.constants import EXTERNAL
+from readthedocs.builds.constants import (
+    EXTERNAL,
+    GENERIC_EXTERNAL_VERSION_NAME,
+    GITHUB_EXTERNAL_VERSION_NAME,
+    GITLAB_EXTERNAL_VERSION_NAME,
+)
 from readthedocs.projects.constants import (
     BITBUCKET_REGEXS,
+    GITHUB_BRAND,
     GITHUB_PULL_REQUEST_URL,
     GITHUB_REGEXS,
+    GITLAB_BRAND,
     GITLAB_MERGE_REQUEST_URL,
     GITLAB_REGEXS,
 )
@@ -79,6 +85,23 @@ def get_vcs_url(*, project, version_type, version_name):
     return project.repo.replace('git://', 'https://').replace('.git', '') + url
 
 
+def external_version_name(build_or_version):
+    """Returns a string identifying the external build/version's nature."""
+    if not build_or_version.is_external:
+        return None
+
+    project = build_or_version.project
+
+    if project.git_provider_name == GITHUB_BRAND:
+        return GITHUB_EXTERNAL_VERSION_NAME
+
+    if project.git_provider_name == GITLAB_BRAND:
+        return GITLAB_EXTERNAL_VERSION_NAME
+
+    # TODO: Add External Version Name for BitBucket.
+    return GENERIC_EXTERNAL_VERSION_NAME
+
+
 @contextmanager
 def memcache_lock(lock_id, oid):
     """

readthedocs/projects/tasks/mixins.py

@@ -1,21 +1,8 @@
-import datetime
-import json
 import os
-import signal
-import socket
-import tarfile
-import tempfile
-from collections import Counter, defaultdict
-
-from celery.exceptions import SoftTimeLimitExceeded
-from django.conf import settings
-from django.db.models import Q
-from django.utils import timezone
-from django.utils.translation import gettext_lazy as _
-from slumber.exceptions import HttpClientError
-from sphinx.ext import intersphinx
+from collections import Counter
 
 import structlog
+from django.utils.translation import gettext_lazy as _
 
 from readthedocs.api.v2.client import api as api_v2
 from readthedocs.builds import tasks as build_tasks
@@ -30,30 +17,14 @@
     LATEST_VERBOSE_NAME,
     STABLE_VERBOSE_NAME,
 )
-from readthedocs.builds.models import APIVersion, Build, Version
-from readthedocs.builds.signals import build_complete
-from readthedocs.config import ConfigError
-from readthedocs.doc_builder.config import load_yaml_config
+from readthedocs.builds.models import APIVersion
 from readthedocs.doc_builder.environments import (
     DockerBuildEnvironment,
     LocalBuildEnvironment,
 )
-from readthedocs.doc_builder.loader import get_builder_class
-from readthedocs.doc_builder.python_environments import Conda, Virtualenv
-from readthedocs.search.utils import index_new_files, remove_indexed_files
-from readthedocs.sphinx_domains.models import SphinxDomain
-from readthedocs.storage import build_environment_storage, build_media_storage
-from readthedocs.worker import app
-
-from ..models import APIProject, Feature, WebHookEvent
-from ..models import HTMLFile, ImportedFile, Project
-from ..signals import (
-    after_build,
-    before_build,
-    before_vcs,
-    files_changed,
-)
+
 from ..exceptions import RepositoryError
+from ..models import Feature
 
 log = structlog.get_logger(__name__)
 
@@ -209,9 +180,11 @@ def get_vcs_env_vars(self):
     def get_rtd_env_vars(self):
         """Get bash environment variables specific to Read the Docs."""
         env = {
-            'READTHEDOCS': 'True',
-            'READTHEDOCS_VERSION': self.data.version.slug,
-            'READTHEDOCS_PROJECT': self.data.project.slug,
-            'READTHEDOCS_LANGUAGE': self.data.project.language,
+            "READTHEDOCS": "True",
+            "READTHEDOCS_VERSION": self.data.version.slug,
+            "READTHEDOCS_VERSION_TYPE": self.data.version.type,
+            "READTHEDOCS_VERSION_NAME": self.data.version.verbose_name,
+            "READTHEDOCS_PROJECT": self.data.project.slug,
+            "READTHEDOCS_LANGUAGE": self.data.project.language,
         }
         return env

readthedocs/projects/tests/test_build_tasks.py

@@ -15,15 +15,8 @@
 from readthedocs.config.config import BuildConfigV2
 from readthedocs.doc_builder.exceptions import BuildAppError
 from readthedocs.projects.exceptions import RepositoryError
-from readthedocs.projects.models import (
-    EnvironmentVariable,
-    Project,
-    WebHookEvent,
-)
-from readthedocs.projects.tasks.builds import (
-    sync_repository_task,
-    update_docs_task,
-)
+from readthedocs.projects.models import EnvironmentVariable, Project, WebHookEvent
+from readthedocs.projects.tasks.builds import sync_repository_task, update_docs_task
 
 from .mockers import BuildEnvironmentMocker
 
@@ -199,12 +192,14 @@ def test_get_env_vars_default(self, load_yaml_config):
         )
 
         env = {
-            'NO_COLOR': '1',
-            'READTHEDOCS': 'True',
-            'READTHEDOCS_VERSION': self.version.slug,
-            'READTHEDOCS_PROJECT': self.project.slug,
-            'READTHEDOCS_LANGUAGE': self.project.language,
-            'BIN_PATH': os.path.join(
+            "NO_COLOR": "1",
+            "READTHEDOCS": "True",
+            "READTHEDOCS_VERSION": self.version.slug,
+            "READTHEDOCS_VERSION_TYPE": self.version.type,
+            "READTHEDOCS_VERSION_NAME": self.version.verbose_name,
+            "READTHEDOCS_PROJECT": self.project.slug,
+            "READTHEDOCS_LANGUAGE": self.project.language,
+            "BIN_PATH": os.path.join(
                 self.project.doc_path,
                 'envs',
                 self.version.slug,

readthedocs/rtd_tests/tests/test_builds.py

@@ -491,7 +491,8 @@ def setUp(self):
         self.project = get(Project)
         self.version = get(
             Version,
-            project=self.project
+            project=self.project,
+            type=BRANCH,
         )
 
         get(

readthedocs/rtd_tests/tests/test_footer.py

@@ -11,7 +11,7 @@
 from readthedocs.builds.constants import BRANCH, EXTERNAL, LATEST, TAG
 from readthedocs.builds.models import Version
 from readthedocs.core.middleware import ReadTheDocsSessionMiddleware
-from readthedocs.projects.constants import PUBLIC
+from readthedocs.projects.constants import GITHUB_BRAND, GITLAB_BRAND, PUBLIC
 from readthedocs.projects.models import Project
 
 
@@ -72,6 +72,29 @@ def test_footer_dont_show_version_warning(self):
         self.assertFalse(r.data['show_version_warning'])
         self.assertEqual(r.context['main_project'], self.pip)
 
+    def test_footer_show_explicit_name_for_external_version(self):
+        project = Project.objects.get(slug="pip")
+        version = project.versions.get(slug=LATEST)
+        version.type = EXTERNAL
+        version.verbose_name = "4"
+        version.save()
+        self.url = (
+            reverse("footer_html")
+            + f"?project={project.slug}&version={version.slug}&page=index&docroot=/"
+        )
+
+        git_provider_name = "readthedocs.projects.models.Project.git_provider_name"
+        with mock.patch(git_provider_name, GITHUB_BRAND):
+            r = self.render()
+            self.assertIn("#4 (PR)", r.data["html"])
+            self.assertNotIn("#4 (MR)", r.data["html"])
+            self.assertNotIn("#4 (EV)", r.data["html"])
+        with mock.patch(git_provider_name, GITLAB_BRAND):
+            r = self.render()
+            self.assertIn("#4 (MR)", r.data["html"])
+            self.assertNotIn("#4 (PR)", r.data["html"])
+            self.assertNotIn("#4 (EV)", r.data["html"])
+
     def test_footer_dont_show_version_warning_for_external_versions(self):
         self.latest.type = EXTERNAL
         self.latest.save()