Merge pull request #5916 from saadmk11/fix-build-ux

Update build list and detail page UX

Author: ericholscher

readthedocs/api/v2/serializers.py

@@ -118,6 +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')
+    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/constants.py

@@ -87,3 +87,6 @@
         'description': 'The build succeeded!',
     },
 }
+
+GITHUB_EXTERNAL_VERSION_NAME = 'Pull Request'
+GENERIC_EXTERNAL_VERSION_NAME = 'External Version'

readthedocs/builds/models.py

@@ -20,8 +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,
@@ -36,6 +41,8 @@
     BUILD_STATE_FINISHED,
     BUILD_STATE_TRIGGERED,
     BUILD_TYPES,
+    GENERIC_EXTERNAL_VERSION_NAME,
+    GITHUB_EXTERNAL_VERSION_NAME,
     INTERNAL,
     LATEST,
     NON_REPOSITORY_VERSIONS,
@@ -63,6 +70,7 @@
     get_gitlab_username_repo,
 )
 from readthedocs.builds.version_slug import VersionSlugField
+from readthedocs.oauth.models import RemoteRepository
 
 
 log = logging.getLogger(__name__)
@@ -158,9 +166,20 @@ def vcs_url(self):
         """
         Generate VCS (github, gitlab, bitbucket) URL for this version.
 
-        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/.
+        Example: https://github.com/rtfd/readthedocs.org/tree/3.4.2/.
+        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)
+                return GITHUB_PULL_REQUEST_URL.format(
+                    user=user,
+                    repo=repo,
+                    number=self.verbose_name,
+                )
+            # TODO: Add VCS URL for other Git Providers
+            return ''
+
         url = ''
         if self.slug == STABLE:
             slug_url = self.ref
@@ -169,25 +188,12 @@ def vcs_url(self):
         else:
             slug_url = self.slug
 
-        if self.type == EXTERNAL:
-            if 'github' in self.project.repo:
-                slug_url = self.verbose_name
-                url = f'/pull/{slug_url}/'
+        if ('github' in self.project.repo) or ('gitlab' in self.project.repo):
+            url = f'/tree/{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}'
+        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
@@ -745,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."""
@@ -756,6 +816,28 @@ def is_stale(self):
         mins_ago = timezone.now() - datetime.timedelta(minutes=5)
         return self.state == BUILD_STATE_TRIGGERED and self.date < mins_ago
 
+    @property
+    def is_external(self):
+        return self.version.type == EXTERNAL
+
+    @property
+    def external_version_name(self):
+        if self.is_external:
+            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
+
     def using_latest_config(self):
         return int(self.config.get('version', '1')) == LATEST_CONFIGURATION_VERSION
 

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

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

readthedocs/builds/static/builds/js/detail.js

@@ -84,21 +84,6 @@ def post(self, request, project_slug):
 
 class BuildList(BuildBase, BuildTriggerMixin, ListView):
 
-    def get_queryset(self):
-        # this is used to include only internal version
-        # builds in the build list page
-        self.project_slug = self.kwargs.get('project_slug', None)
-        self.project = get_object_or_404(
-            Project.objects.protected(self.request.user),
-            slug=self.project_slug,
-        )
-        queryset = Build.objects.public(
-            user=self.request.user,
-            project=self.project,
-        ).select_related('project', 'version')
-
-        return queryset
-
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
 

readthedocs/builds/views.py

@@ -325,13 +325,33 @@
     '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}'
+)
+GITHUB_PULL_REQUEST_COMMIT_URL = (
+    'https://github.com/{user}/{repo}/'
+    'pull/{number}/commits/{commit}'
+)
 BITBUCKET_URL = (
     '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/projects/constants.py

@@ -98,7 +98,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,
         )
@@ -274,7 +274,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,