Only build external version from webhook payload

readthedocs/api/v2/utils.py

@@ -13,9 +13,7 @@
     STABLE,
     STABLE_VERBOSE_NAME,
     TAG,
-    EXTERNAL,
 )
-from readthedocs.core.utils import trigger_build
 from readthedocs.builds.models import Version
 
 
@@ -79,15 +77,6 @@ def sync_versions(project, versions, type):  # pylint: disable=redefined-builtin
                     version_name,
                     version_id,
                 )
-        elif type == EXTERNAL:
-            created_version = Version.objects.create(
-                project=project,
-                type=type,
-                identifier=version_id,
-                verbose_name=version_name,
-            )
-            added.add(created_version.slug)
-
         else:
             # New Version
             created_version = Version.objects.create(
@@ -146,9 +135,6 @@ def delete_versions(project, version_data):
     versions_tags = [
         version['verbose_name'] for version in version_data.get('tags', [])
     ]
-    external_versions = [
-        version['verbose_name'] for version in version_data.get('external_branches', [])
-    ]
     versions_branches = [
         version['identifier'] for version in version_data.get('branches', [])
     ]
@@ -161,10 +147,6 @@ def delete_versions(project, version_data):
         type=BRANCH,
         identifier__in=versions_branches,
     )
-    to_delete_qs = to_delete_qs.exclude(
-        type=EXTERNAL,
-        verbose_name__in=external_versions,
-    )
     to_delete_qs = to_delete_qs.exclude(uploaded=True)
     to_delete_qs = to_delete_qs.exclude(active=True)
     to_delete_qs = to_delete_qs.exclude(slug__in=NON_REPOSITORY_VERSIONS)
@@ -194,21 +176,6 @@ def run_automation_rules(project, versions_slug):
         rule.run(version)
 
 
-def trigger_external_build(project, version_list):
-    """
-    Trigger Builds for all external versions provided.
-
-    The rules are sorted by priority.
-
-    """
-    for version_slug in version_list:
-        version = project.versions(manager=EXTERNAL).get(slug=version_slug)
-        version.active = True
-        version.save()
-
-        trigger_build(project=project, version=version)
-
-
 class RemoteOrganizationPagination(PageNumberPagination):
     page_size = 25
 

readthedocs/api/v2/views/integrations.py

@@ -19,7 +19,11 @@
     webhook_github,
     webhook_gitlab,
 )
-from readthedocs.core.views.hooks import build_branches, sync_versions
+from readthedocs.core.views.hooks import (
+    build_branches,
+    sync_versions,
+    get_or_create_external_version,
+)
 from readthedocs.integrations.models import HttpExchange, Integration
 from readthedocs.projects.models import Project
 
@@ -30,7 +34,7 @@
 GITHUB_SIGNATURE_HEADER = 'HTTP_X_HUB_SIGNATURE'
 GITHUB_PUSH = 'push'
 GITHUB_PULL_REQUEST = 'pull_request'
-GITHUB_PULL_REQUEST_OPEN = 'open'
+GITHUB_PULL_REQUEST_OPEN = 'opened'
 GITHUB_PULL_REQUEST_SYNC = 'synchronize'
 GITHUB_CREATE = 'create'
 GITHUB_DELETE = 'delete'
@@ -113,6 +117,10 @@ def handle_webhook(self):
         """Handle webhook payload."""
         raise NotImplementedError
 
+    def get_external_version_data(self):
+        """Get External Version data from payload."""
+        raise NotImplementedError
+
     def is_payload_valid(self):
         """Validates the webhook's payload using the integration's secret."""
         return False
@@ -221,6 +229,13 @@ def get_data(self):
                 pass
         return super().get_data()
 
+    def get_external_version_data(self):
+        """Get Commit Sha and pull request number from payload"""
+        identifier = self.data['pull_request']['head']['sha']
+        verbose_name = str(self.data['number'])
+
+        return identifier, verbose_name
+
     def is_payload_valid(self):
         """
         GitHub use a HMAC hexdigest hash to sign the payload.
@@ -275,8 +290,19 @@ def handle_webhook(self):
         if event in (GITHUB_CREATE, GITHUB_DELETE):
             return self.sync_versions(self.project)
 
-        if event == GITHUB_PULL_REQUEST:
-            return self.sync_versions(self.project)
+        if (
+            event == GITHUB_PULL_REQUEST and
+            self.data['action'] in [GITHUB_PULL_REQUEST_OPEN, GITHUB_PULL_REQUEST_SYNC]
+        ):
+            try:
+                identifier, verbose_name = self.get_external_version_data()
+                external_version = get_or_create_external_version(
+                    self.project, identifier, verbose_name
+                )
+                return self.get_response_push(self.project, external_version.verbose_name)
+
+            except KeyError:
+                raise ParseError('Parameters "sha" and "number" are required')
 
         return None
 

readthedocs/api/v2/views/model_views.py

@@ -10,7 +10,7 @@
 from rest_framework.renderers import BaseRenderer, JSONRenderer
 from rest_framework.response import Response
 
-from readthedocs.builds.constants import BRANCH, TAG, INTERNAL, EXTERNAL
+from readthedocs.builds.constants import BRANCH, TAG
 from readthedocs.builds.models import Build, BuildCommandResult, Version
 from readthedocs.core.utils import trigger_build
 from readthedocs.core.utils.extend import SettingsOverrideObject
@@ -130,7 +130,7 @@ def active_versions(self, request, **kwargs):
             Project.objects.api(request.user),
             pk=kwargs['pk'],
         )
-        versions = project.versions(manager=INTERNAL).filter(active=True)
+        versions = project.versions.filter(active=True)
         return Response({
             'versions': VersionSerializer(versions, many=True).data,
         })
@@ -189,7 +189,6 @@ def sync_versions(self, request, **kwargs):  # noqa: D205
             # Update All Versions
             data = request.data
             added_versions = set()
-            added_external_versions = set()
             if 'tags' in data:
                 ret_set = api_utils.sync_versions(
                     project=project,
@@ -204,15 +203,6 @@ def sync_versions(self, request, **kwargs):  # noqa: D205
                     type=BRANCH,
                 )
                 added_versions.update(ret_set)
-
-            if 'external_branches' in data:
-                ret_set = api_utils.sync_versions(
-                    project=project,
-                    versions=data['external_branches'],
-                    type=EXTERNAL,
-                )
-                added_external_versions.update(ret_set)
-
             deleted_versions = api_utils.delete_versions(project, data)
         except Exception as e:
             log.exception('Sync Versions Error')
@@ -223,13 +213,11 @@ def sync_versions(self, request, **kwargs):  # noqa: D205
                 status=status.HTTP_400_BAD_REQUEST,
             )
 
-        all_added_versions = added_versions | added_external_versions
-
         try:
             # The order of added_versions isn't deterministic.
             # We don't track the commit time or any other metadata.
             # We usually have one version added per webhook.
-            api_utils.run_automation_rules(project, all_added_versions)
+            api_utils.run_automation_rules(project, added_versions)
         except Exception:
             # Don't interrupt the request if something goes wrong
             # in the automation rules.
@@ -238,12 +226,6 @@ def sync_versions(self, request, **kwargs):  # noqa: D205
                 project.slug, added_versions
             )
 
-        if added_external_versions:
-            api_utils.trigger_external_build(
-                project=project,
-                version_list=added_external_versions
-            )
-
         # TODO: move this to an automation rule
         promoted_version = project.update_stable_version()
         new_stable = project.get_stable_version()
@@ -268,7 +250,7 @@ def sync_versions(self, request, **kwargs):  # noqa: D205
                 trigger_build(project=project, version=promoted_version)
 
         return Response({
-            'added_versions': all_added_versions,
+            'added_versions': added_versions,
             'deleted_versions': deleted_versions,
         })
 

readthedocs/core/views/hooks.py

@@ -2,6 +2,8 @@
 
 import logging
 
+from readthedocs.builds.constants import EXTERNAL
+from readthedocs.builds.models import Version
 from readthedocs.core.utils import trigger_build
 from readthedocs.projects.tasks import sync_repository_task
 
@@ -88,3 +90,21 @@ def sync_versions(project):
     except Exception:
         log.exception('Unknown sync versions exception')
     return None
+
+
+def get_or_create_external_version(project, identifier, verbose_name):
+    external_version = project.versions(manager=EXTERNAL).filter(verbose_name=verbose_name).first()
+    if external_version:
+        if external_version.identifier != identifier:
+            external_version.identifier = identifier
+            external_version.save()
+    else:
+        created_external_version = Version.objects.create(
+            project=project,
+            type=EXTERNAL,
+            identifier=identifier,
+            verbose_name=verbose_name,
+            active=True
+        )
+        return created_external_version
+    return external_version

readthedocs/projects/constants.py

@@ -334,6 +334,4 @@
     '{action}/{version}{docroot}{path}{source_suffix}'
 )
 
-DEFAULT_GIT_PATTERN = 'refs/heads/*:refs/remotes/origin/*'
-# https://help.github.com/en/articles/checking-out-pull-requests-locally#modifying-an-inactive-pull-request-locally
-GITHUB_GIT_PATTERN = 'refs/pull/*/head:refs/remotes/origin/external/*'
+GITHUB_GIT_PATTERN = 'pull/{id}/head:external-{id}'

readthedocs/projects/tasks.py

@@ -138,7 +138,7 @@ def sync_repo(self):
                 }
             )
             version_repo = self.get_vcs_repo()
-            version_repo.update()
+            version_repo.update(version=self.version)
             self.sync_versions(version_repo)
             version_repo.checkout(self.version.identifier)
         finally:
@@ -165,12 +165,6 @@ def sync_versions(self, version_repo):
                 'verbose_name': v.verbose_name,
             } for v in version_repo.branches]
 
-        if version_repo.supports_external_branches:
-            version_post_data['external_branches'] = [{
-                'identifier': v.identifier,
-                'verbose_name': v.verbose_name,
-            } for v in version_repo.external_branches]
-
         self.validate_duplicate_reserved_versions(version_post_data)
 
         try:

readthedocs/vcs_support/backends/bzr.py

@@ -17,7 +17,7 @@ class Backend(BaseVCS):
     supports_tags = True
     fallback_branch = ''
 
-    def update(self):
+    def update(self, version=None):  # pylint: disable=arguments-differ
         super().update()
         retcode = self.run('bzr', 'status', record=False)[0]
         if retcode == 0:

readthedocs/vcs_support/backends/git.py

@@ -10,8 +10,9 @@
 from django.core.exceptions import ValidationError
 from git.exc import BadName, InvalidGitRepositoryError
 
+from readthedocs.builds.constants import EXTERNAL
 from readthedocs.config import ALL
-from readthedocs.projects.constants import GITHUB_GIT_PATTERN, DEFAULT_GIT_PATTERN
+from readthedocs.projects.constants import GITHUB_GIT_PATTERN
 from readthedocs.projects.exceptions import RepositoryError
 from readthedocs.projects.validators import validate_submodule_url
 from readthedocs.vcs_support.base import BaseVCS, VCSVersion
@@ -52,16 +53,21 @@ def _get_clone_url(self):
     def set_remote_url(self, url):
         return self.run('git', 'remote', 'set-url', 'origin', url)
 
-    def update(self):
+    def update(self, version=None):  # pylint: disable=arguments-differ
         """Clone or update the repository."""
         super().update()
         if self.repo_exists():
             self.set_remote_url(self.repo_url)
-        else:
-            self.make_clean_working_dir()
-            self.clone()
+            # A fetch is always required to get external versions properly
+            if version and version.type == EXTERNAL:
+                return self.fetch(version.verbose_name)
+            return self.fetch()
+        self.make_clean_working_dir()
         # A fetch is always required to get external versions properly
-        self.fetch()
+        if version and version.type == EXTERNAL:
+            self.clone()
+            return self.fetch(version.verbose_name)
+        return self.clone()
 
     def repo_exists(self):
         try:
@@ -147,11 +153,15 @@ def use_shallow_clone(self):
         from readthedocs.projects.models import Feature
         return not self.project.has_feature(Feature.DONT_SHALLOW_CLONE)
 
-    def fetch(self):
+    def fetch(self, verbose_name=None):
         cmd = ['git', 'fetch', 'origin',
-               DEFAULT_GIT_PATTERN, GITHUB_GIT_PATTERN,
                '--tags', '--prune', '--prune-tags']
 
+        if verbose_name and 'github.com' in self.repo_url:
+            cmd.append(
+                GITHUB_GIT_PATTERN.format(id=verbose_name)
+            )
+
         if self.use_shallow_clone():
             cmd.extend(['--depth', str(self.repo_depth)])
 
@@ -222,24 +232,6 @@ def branches(self):
                 versions.append(VCSVersion(self, str(branch), verbose_name))
         return versions
 
-    @property
-    def external_branches(self):
-        repo = git.Repo(self.working_dir)
-        versions = []
-        branches = []
-
-        # ``repo.remotes.origin.refs`` returns remote branches
-        if repo.remotes:
-            branches += repo.remotes.origin.refs
-
-        for branch in branches:
-            verbose_name = branch.name
-            if verbose_name.startswith('origin/external/'):
-                verbose_name = verbose_name.replace('origin/', '')
-                versions.append(VCSVersion(self, str(branch.commit), verbose_name))
-                continue
-        return versions
-
     @property
     def commit(self):
         if self.repo_exists():

readthedocs/vcs_support/backends/hg.py

@@ -13,7 +13,7 @@ class Backend(BaseVCS):
     supports_branches = True
     fallback_branch = 'default'
 
-    def update(self):
+    def update(self, version=None):  # pylint: disable=arguments-differ
         super().update()
         retcode = self.run('hg', 'status', record=False)[0]
         if retcode == 0:

readthedocs/vcs_support/backends/svn.py

@@ -27,7 +27,7 @@ def __init__(self, *args, **kwargs):
         else:
             self.base_url = self.repo_url
 
-    def update(self):
+    def update(self, version=None):  # pylint: disable=arguments-differ
         super().update()
         # For some reason `svn status` gives me retcode 0 in non-svn
         # directories that's why I use `svn info` here.