Refine PR Builder Code

readthedocs/api/v2/views/integrations.py

@@ -200,7 +200,7 @@ def sync_versions(self, project):
 
     def get_external_version_response(self, project):
         """
-        Build External version on pull/merge request events and return API response.
+        Trigger builds for External versions on pull/merge request events and return API response.
 
         Return a JSON response with the following::
 
@@ -211,7 +211,7 @@ def get_external_version_response(self, project):
             }
 
         :param project: Project instance
-        :type project: Project
+        :type project: readthedocs.projects.models.Project
         """
         identifier, verbose_name = self.get_external_version_data()
         # create or get external version object using `verbose_name`.
@@ -369,7 +369,7 @@ def handle_webhook(self):
             return self.sync_versions(self.project)
 
         if (
-            self.project.has_feature(Feature.ENABLE_EXTERNAL_VERSION_BUILD) and
+            self.project.has_feature(Feature.EXTERNAL_VERSION_BUILD) and
             event == GITHUB_PULL_REQUEST and action
         ):
             if (

readthedocs/builds/constants.py

@@ -90,3 +90,5 @@
 
 GITHUB_EXTERNAL_VERSION_NAME = 'Pull Request'
 GENERIC_EXTERNAL_VERSION_NAME = 'External Version'
+
+RTD_BUILD_STATUS_API_NAME = 'continuous-documentation/read-the-docs'

readthedocs/builds/models.py

@@ -212,7 +212,7 @@ def config(self):
         """
         last_build = (
             self.builds(manager=INTERNAL).filter(
-                state='finished',
+                state=BUILD_STATE_FINISHED,
                 success=True,
             ).order_by('-date')
             .only('_config')
@@ -742,7 +742,11 @@ def get_absolute_url(self):
         return reverse('builds_detail', args=[self.project.slug, self.pk])
 
     def get_full_url(self):
-        """Get full url including domain."""
+        """
+        Get full url of the build including domain.
+
+        Example: https://readthedocs.org/projects/pip/builds/99999999/
+        """
         scheme = 'http' if settings.DEBUG else 'https'
         full_url = '{scheme}://{domain}{absolute_url}'.format(
             scheme=scheme,
@@ -803,7 +807,7 @@ def get_commit_url(self):
                     commit=self.commit
                 )
 
-        return ''
+        return None
 
     @property
     def finished(self):

readthedocs/builds/views.py

@@ -35,7 +35,7 @@ class BuildBase:
     def get_queryset(self):
         self.project_slug = self.kwargs.get('project_slug', None)
         self.project = get_object_or_404(
-            Project.objects.protected(self.request.user),
+            Project.objects.public(self.request.user),
             slug=self.project_slug,
         )
         queryset = Build.objects.public(

readthedocs/core/views/hooks.py

@@ -105,30 +105,27 @@ def get_or_create_external_version(project, identifier, verbose_name):
     :returns:  External version.
     :rtype: Version
     """
-    external_version = project.versions(
-        manager=EXTERNAL
-    ).filter(verbose_name=verbose_name).first()
+    external_version, created = project.versions.get_or_create(
+        verbose_name=verbose_name,
+        type=EXTERNAL,
+        defaults={'identifier': identifier, 'active': True},
+    )
 
-    if external_version:
+    if created:
+        log.info(
+            '(Create External Version) Added Version: [%s] ',
+            external_version.slug
+        )
+    else:
         # identifier will change if there is a new commit to the Pull/Merge Request
         if external_version.identifier != identifier:
             external_version.identifier = identifier
             external_version.save()
-    else:
-        # create an external version if the version does not exist.
-        created_external_version = Version.objects.create(
-            project=project,
-            type=EXTERNAL,
-            identifier=identifier,
-            verbose_name=verbose_name,
-            active=True
-        )
-        log.info(
-            '(Create External Version) Added Version: [%s] ', ' '.join(
-                created_external_version.slug
+
+            log.info(
+                '(Update External Version) Updated Version: [%s] ',
+                external_version.slug
             )
-        )
-        return created_external_version
     return external_version
 
 
@@ -152,9 +149,8 @@ def delete_external_version(project, identifier, verbose_name):
         # Delete External Version
         external_version.delete()
         log.info(
-            '(Delete External Version) Deleted Version: [%s]', ' '.join(
-                external_version.slug
-            )
+            '(Delete External Version) Deleted Version: [%s]',
+            external_version.slug
         )
 
         return external_version.verbose_name

readthedocs/oauth/services/github.py

@@ -12,7 +12,10 @@
 
 from readthedocs.api.v2.client import api
 from readthedocs.builds import utils as build_utils
-from readthedocs.builds.constants import SELECT_BUILD_STATUS
+from readthedocs.builds.constants import (
+    SELECT_BUILD_STATUS,
+    RTD_BUILD_STATUS_API_NAME
+)
 from readthedocs.integrations.models import Integration
 
 from ..models import RemoteOrganization, RemoteRepository
@@ -336,7 +339,7 @@ def send_build_status(self, build, state):
             'state': github_build_state,
             'target_url': build.get_full_url(),
             'description': description,
-            'context': 'continuous-documentation/read-the-docs'
+            'context': RTD_BUILD_STATUS_API_NAME
         }
 
         resp = None
@@ -349,8 +352,9 @@ def send_build_status(self, build, state):
             )
             if resp.status_code == 201:
                 log.info(
-                    'GitHub commit status for project: %s',
+                    "GitHub commit status created for project: %s, commit status: %s",
                     project,
+                    github_build_state,
                 )
                 return True
 

readthedocs/projects/constants.py

@@ -354,4 +354,4 @@
     'commit/{commit}'
 )
 
-GITHUB_GIT_PATTERN = 'pull/{id}/head:external-{id}'
+GITHUB_PR_PULL_PATTERN = 'pull/{id}/head:external-{id}'

readthedocs/projects/models.py

@@ -1407,7 +1407,7 @@ def add_features(sender, **kwargs):
     SHARE_SPHINX_DOCTREE = 'share_sphinx_doctree'
     DEFAULT_TO_MKDOCS_0_17_3 = 'default_to_mkdocs_0_17_3'
     CLEAN_AFTER_BUILD = 'clean_after_build'
-    ENABLE_EXTERNAL_VERSION_BUILD = 'enable_external_version_build'
+    EXTERNAL_VERSION_BUILD = 'enable_external_version_build'
     UPDATE_CONDA_STARTUP = 'update_conda_startup'
 
     FEATURES = (
@@ -1450,7 +1450,7 @@ def add_features(sender, **kwargs):
             _('Clean all files used in the build process'),
         ),
         (
-            ENABLE_EXTERNAL_VERSION_BUILD,
+            EXTERNAL_VERSION_BUILD,
             _('Enable project to build on pull/merge requests'),
         ),
         (

readthedocs/projects/tasks.py

@@ -578,17 +578,18 @@ def run_build(self, docker, record):
 
         if self.build_env.failed:
             self.send_notifications(self.version.pk, self.build['id'])
-            # send build failure status to git Status API
             send_external_build_status(
                 version=self.version, build_pk=self.build['id'], status=BUILD_STATUS_FAILURE
             )
         elif self.build_env.successful:
-            # send build successful status to git Status API
             send_external_build_status(
                 version=self.version, build_pk=self.build['id'], status=BUILD_STATUS_SUCCESS
             )
         else:
             msg = 'Unhandled Build Status'
+            send_external_build_status(
+                version=self.version, build_pk=self.build['id'], status=BUILD_STATUS_FAILURE
+            )
             log.warning(
                 LOG_TEMPLATE,
                 {

readthedocs/rtd_tests/tests/test_api.py

@@ -771,7 +771,7 @@ def setUp(self):
         self.feature_flag = get(
             Feature,
             projects=[self.project],
-            feature_id=Feature.ENABLE_EXTERNAL_VERSION_BUILD,
+            feature_id=Feature.EXTERNAL_VERSION_BUILD,
         )
         self.version = get(
             Version, slug='master', verbose_name='master',

readthedocs/rtd_tests/tests/test_oauth.py

@@ -161,7 +161,8 @@ def test_send_build_status_successful(self, session, mock_logger):
 
         self.assertTrue(success)
         mock_logger.info.assert_called_with(
-            'GitHub commit status for project: %s',
+            "GitHub '%s' commit status created for project: %s",
+            BUILD_STATUS_SUCCESS,
             self.project
         )
 

readthedocs/templates/builds/build_detail.html

@@ -91,7 +91,11 @@
         {% endif %}
       </span>
       <span class="build-commit" data-bind="visible: commit">
+        {% if build.get_commit_url %}
           (<a data-bind="attr: {href: commit_url}"><span data-bind="text: commit">{{ build.commit }}</span></a>)
+        {% else %}
+            (<span data-bind="text: commit">{{ build.commit }}</span>)
+        {% endif %}
       </span>
     </div>
 

readthedocs/vcs_support/backends/git.py

@@ -12,7 +12,7 @@
 
 from readthedocs.builds.constants import EXTERNAL
 from readthedocs.config import ALL
-from readthedocs.projects.constants import GITHUB_GIT_PATTERN
+from readthedocs.projects.constants import GITHUB_PR_PULL_PATTERN
 from readthedocs.projects.exceptions import RepositoryError
 from readthedocs.projects.validators import validate_submodule_url
 from readthedocs.vcs_support.base import BaseVCS, VCSVersion
@@ -57,7 +57,6 @@ def update(self):
         super().update()
         if self.repo_exists():
             self.set_remote_url(self.repo_url)
-            # A fetch is always required to get external versions properly
             return self.fetch()
         self.make_clean_working_dir()
         # A fetch is always required to get external versions properly
@@ -154,18 +153,18 @@ def fetch(self):
         cmd = ['git', 'fetch', 'origin',
                '--tags', '--prune', '--prune-tags']
 
+        if self.use_shallow_clone():
+            cmd.extend(['--depth', str(self.repo_depth)])
+
         if (
             self.verbose_name and
             self.version_type == EXTERNAL and
             'github.com' in self.repo_url
         ):
             cmd.append(
-                GITHUB_GIT_PATTERN.format(id=self.verbose_name)
+                GITHUB_PR_PULL_PATTERN.format(id=self.verbose_name)
             )
 
-        if self.use_shallow_clone():
-            cmd.extend(['--depth', str(self.repo_depth)])
-
         code, stdout, stderr = self.run(*cmd)
         if code != 0:
             raise RepositoryError