Update build list and detail page UX

readthedocs/api/v2/serializers.py

@@ -118,7 +118,7 @@ class BuildSerializer(serializers.ModelSerializer):
     version_slug = serializers.ReadOnlyField(source='version.slug')
     docs_url = serializers.ReadOnlyField(source='version.get_absolute_url')
     state_display = serializers.ReadOnlyField(source='get_state_display')
-    vcs_url = serializers.ReadOnlyField(source='version.vcs_url')
+    commit_url = serializers.ReadOnlyField(source='get_commit_url')
     # Jsonfield needs an explicit serializer
     # https://github.com/dmkoch/django-jsonfield/issues/188#issuecomment-300439829
     config = serializers.JSONField(required=False)

readthedocs/builds/models.py

@@ -20,10 +20,13 @@
 from readthedocs.config import LATEST_CONFIGURATION_VERSION
 from readthedocs.core.utils import broadcast
 from readthedocs.projects.constants import (
+    BITBUCKET_COMMIT_URL,
     BITBUCKET_URL,
+    GITHUB_COMMIT_URL,
     GITHUB_URL,
     GITHUB_PULL_REQUEST_URL,
     GITHUB_PULL_REQUEST_COMMIT_URL,
+    GITLAB_COMMIT_URL,
     GITLAB_URL,
     PRIVACY_CHOICES,
     PRIVATE,
@@ -67,6 +70,7 @@
     get_gitlab_username_repo,
 )
 from readthedocs.builds.version_slug import VersionSlugField
+from readthedocs.oauth.models import RemoteRepository
 
 
 log = logging.getLogger(__name__)
@@ -163,19 +167,17 @@ def vcs_url(self):
         Generate VCS (github, gitlab, bitbucket) URL for this version.
 
         Example: https://github.com/rtfd/readthedocs.org/tree/3.4.2/.
-        External Version Example: https://github.com/rtfd/readthedocs.org/pull/99/commits/b630b630/.
+        External Version Example: https://github.com/rtfd/readthedocs.org/pull/99/.
         """
         if self.type == EXTERNAL:
             if 'github' in self.project.repo:
                 user, repo = get_github_username_repo(self.project.repo)
-                # Get Github Pull Request Commit URL.
-                return GITHUB_PULL_REQUEST_COMMIT_URL.format(
+                return GITHUB_PULL_REQUEST_URL.format(
                     user=user,
                     repo=repo,
                     number=self.verbose_name,
-                    commit=self.commit_name
                 )
-            # TODO: Add VCS ULR for other Git Providers
+            # TODO: Add VCS URL for other Git Providers
             return ''
 
         url = ''
@@ -543,24 +545,6 @@ def get_bitbucket_url(self, docroot, filename, source_suffix='.rst'):
             source_suffix=source_suffix,
         )
 
-    def get_external_version_url(self):
-        """Return a Pull/Merge Request URL."""
-        repo_url = self.project.repo
-
-        if 'github' in repo_url:
-            user, repo = get_github_username_repo(repo_url)
-            if not user and not repo:
-                return ''
-
-            repo = repo.rstrip('/')
-            return GITHUB_PULL_REQUEST_URL.format(
-                user=user,
-                repo=repo,
-                number=self.verbose_name
-            )
-        # TODO: Add External Version ULR for other Git Providers
-        return ''
-
 
 class APIVersion(Version):
 
@@ -767,6 +751,60 @@ def get_full_url(self):
         )
         return full_url
 
+    def get_commit_url(self):
+        """Return the commit URL."""
+        repo_url = self.project.repo
+        if self.is_external:
+            if 'github' in repo_url:
+                user, repo = get_github_username_repo(repo_url)
+                if not user and not repo:
+                    return ''
+
+                repo = repo.rstrip('/')
+                return GITHUB_PULL_REQUEST_COMMIT_URL.format(
+                    user=user,
+                    repo=repo,
+                    number=self.version.verbose_name,
+                    commit=self.commit
+                )
+            # TODO: Add External Version Commit URL for other Git Providers
+        else:
+            if 'github' in repo_url:
+                user, repo = get_github_username_repo(repo_url)
+                if not user and not repo:
+                    return ''
+
+                repo = repo.rstrip('/')
+                return GITHUB_COMMIT_URL.format(
+                    user=user,
+                    repo=repo,
+                    commit=self.commit
+                )
+            if 'gitlab' in repo_url:
+                user, repo = get_gitlab_username_repo(repo_url)
+                if not user and not repo:
+                    return ''
+
+                repo = repo.rstrip('/')
+                return GITLAB_COMMIT_URL.format(
+                    user=user,
+                    repo=repo,
+                    commit=self.commit
+                )
+            if 'bitbucket' in repo_url:
+                user, repo = get_bitbucket_username_repo(repo_url)
+                if not user and not repo:
+                    return ''
+
+                repo = repo.rstrip('/')
+                return BITBUCKET_COMMIT_URL.format(
+                    user=user,
+                    repo=repo,
+                    commit=self.commit
+                )
+
+        return ''
+
     @property
     def finished(self):
         """Return if build has a finished state."""
@@ -788,7 +826,15 @@ def external_version_name(self):
             try:
                 if self.project.remote_repository.account.provider == 'github':
                     return GITHUB_EXTERNAL_VERSION_NAME
+                # TODO: Add External Version Name for other Git Providers
+            except RemoteRepository.DoesNotExist:
+                log.info('Remote repository does not exist for %s', self.project)
+                return GENERIC_EXTERNAL_VERSION_NAME
             except Exception:
+                log.exception(
+                    'Unhandled exception raised for %s while getting external_version_name',
+                    self.project
+                )
                 return GENERIC_EXTERNAL_VERSION_NAME
         return None
 

readthedocs/builds/static-src/builds/js/detail.js

@@ -52,7 +52,7 @@ function BuildDetailView(instance) {
     });
     self.commit = ko.observable(instance.commit);
     self.docs_url = ko.observable(instance.docs_url);
-    self.vcs_url = ko.observable(instance.vcs_url);
+    self.commit_url = ko.observable(instance.commit_url);
 
     /* Others */
     self.legacy_output = ko.observable(false);
@@ -73,7 +73,7 @@ function BuildDetailView(instance) {
             self.length(data.length);
             self.commit(data.commit);
             self.docs_url(data.docs_url);
-            self.vcs_url(data.vcs_url);
+            self.commit_url(data.commit_url);
             var n;
             for (n in data.commands) {
                 var command = data.commands[n];

readthedocs/projects/constants.py

@@ -325,6 +325,10 @@
     'https://github.com/{user}/{repo}/'
     '{action}/{version}{docroot}{path}{source_suffix}'
 )
+GITHUB_COMMIT_URL = (
+    'https://github.com/{user}/{repo}/'
+    'commit/{commit}'
+)
 GITHUB_PULL_REQUEST_URL = (
     'https://github.com/{user}/{repo}/'
     'pull/{number}'
@@ -337,9 +341,17 @@
     'https://bitbucket.org/{user}/{repo}/'
     'src/{version}{docroot}{path}{source_suffix}'
 )
+BITBUCKET_COMMIT_URL = (
+    'https://bitbucket.org/{user}/{repo}/'
+    'commits/{commit}'
+)
 GITLAB_URL = (
     'https://gitlab.com/{user}/{repo}/'
     '{action}/{version}{docroot}{path}{source_suffix}'
 )
+GITLAB_COMMIT_URL = (
+    'https://gitlab.com/{user}/{repo}/'
+    'commit/{commit}'
+)
 
 GITHUB_GIT_PATTERN = 'pull/{id}/head:external-{id}'

readthedocs/rtd_tests/tests/test_builds.py

@@ -11,6 +11,7 @@
 from allauth.socialaccount.models import SocialAccount
 
 from readthedocs.builds.constants import (
+    BRANCH,
     EXTERNAL,
     GITHUB_EXTERNAL_VERSION_NAME,
     GENERIC_EXTERNAL_VERSION_NAME
@@ -377,16 +378,37 @@ def test_get_env_vars(self):
 
 class BuildModelTests(TestCase):
 
+    fixtures = ['test_data']
+
     def setUp(self):
         self.eric = User(username='eric')
         self.eric.set_password('test')
         self.eric.save()
 
         self.project = get(Project)
         self.project.users.add(self.eric)
-
         self.version = get(Version, project=self.project)
 
+        self.pip = Project.objects.get(slug='pip')
+        self.external_version = get(
+            Version,
+            identifier='9F86D081884C7D659A2FEAA0C55AD015A',
+            verbose_name='9999',
+            slug='pr-9999',
+            project=self.pip,
+            active=True,
+            type=EXTERNAL
+        )
+        self.pip_version = get(
+            Version,
+            identifier='origin/stable',
+            verbose_name='stable',
+            slug='stable',
+            project=self.pip,
+            active=True,
+            type=BRANCH
+        )
+
     def test_get_previous_build(self):
         build_one = get(
             Build,
@@ -667,3 +689,29 @@ def test_external_version_name_generic(self):
             external_build.external_version_name,
             GENERIC_EXTERNAL_VERSION_NAME
         )
+
+    def test_get_commit_url_external_version_github(self):
+
+        external_build = get(
+            Build,
+            project=self.pip,
+            version=self.external_version,
+            config={'version': 1},
+        )
+        expected_url = 'https://github.com/pypa/pip/pull/{number}/commits/{sha}'.format(
+            number=self.external_version.verbose_name,
+            sha=external_build.commit
+        )
+        self.assertEqual(external_build.get_commit_url(), expected_url)
+
+    def test_get_commit_url_internal_version(self):
+        build = get(
+            Build,
+            project=self.pip,
+            version=self.pip_version,
+            config={'version': 1},
+        )
+        expected_url = 'https://github.com/pypa/pip/commit/{sha}'.format(
+            sha=build.commit
+        )
+        self.assertEqual(build.get_commit_url(), expected_url)

readthedocs/rtd_tests/tests/test_version.py

@@ -46,10 +46,7 @@ def setUp(self):
 class TestVersionModel(VersionMixin, TestCase):
 
     def test_vcs_url_for_external_version(self):
-        expected_url = 'https://github.com/pypa/pip/pull/{number}/commits/{sha}'.format(
-            number=self.external_version.verbose_name,
-            sha=self.external_version.identifier
-        )
+        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):
@@ -69,9 +66,3 @@ def test_commit_name_for_latest_version(self):
 
     def test_commit_name_for_external_version(self):
         self.assertEqual(self.external_version.commit_name, self.external_version.identifier)
-
-    def test_get_external_version_url(self):
-        expected_url = 'https://github.com/pypa/pip/pull/{number}'.format(
-            number=self.external_version.verbose_name,
-        )
-        self.assertEqual(self.external_version.get_external_version_url(), expected_url)

readthedocs/templates/builds/build_detail.html

@@ -85,13 +85,13 @@
           {% blocktrans with build.external_version_name as external_version_name %}
             <b>{{ external_version_name }}</b>
           {% endblocktrans %}
-            #<a href="{{ build.version.get_external_version_url }}">{{ build.version.verbose_name }}</a>
+            #<a href="{{ build.version.vcs_url }}">{{ build.version.verbose_name }}</a>
         {% else %}
           {{ build.version.slug }}
         {% endif %}
       </span>
       <span class="build-commit" data-bind="visible: commit">
-          (<a data-bind="attr: {href: vcs_url}"><span data-bind="text: commit">{{ build.commit }}</span></a>)
+          (<a data-bind="attr: {href: commit_url}"><span data-bind="text: commit">{{ build.commit }}</span></a>)
       </span>
     </div>