Allow tags from GitHub webhooks (#3546)

* Filter by verbose_name

* Allow tags from GitHub webhook

* Add tests

* Remove useless instance

* Add real tests for parsing ref

* Fix versions

* Check for versions

* Refactor tests

* Split tests

Author: stsewd

readthedocs/projects/models.py

@@ -784,7 +784,8 @@ def versions_from_branch_name(self, branch):
         return (
             self.versions.filter(identifier=branch) |
             self.versions.filter(identifier='remotes/origin/%s' % branch) |
-            self.versions.filter(identifier='origin/%s' % branch)
+            self.versions.filter(identifier='origin/%s' % branch) |
+            self.versions.filter(verbose_name=branch)
         )
 
     def get_default_version(self):

readthedocs/restapi/views/integrations.py

@@ -3,6 +3,7 @@
 from __future__ import absolute_import
 import json
 import logging
+import re
 
 from builtins import object
 from rest_framework import permissions
@@ -159,11 +160,15 @@ def handle_webhook(self):
         # Handle push events and trigger builds
         if event == GITHUB_PUSH:
             try:
-                branches = [self.data['ref'].replace('refs/heads/', '')]
+                branches = [self._normalize_ref(self.data['ref'])]
                 return self.get_response_push(self.project, branches)
             except KeyError:
                 raise ParseError('Parameter "ref" is required')
 
+    def _normalize_ref(self, ref):
+        pattern = re.compile(r'^refs/(heads|tags)/')
+        return pattern.sub('', ref)
+
 
 class GitLabWebhookView(WebhookMixin, APIView):
 

readthedocs/rtd_tests/tests/test_api.py

@@ -21,6 +21,7 @@
 from readthedocs.integrations.models import Integration
 from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
 from readthedocs.projects.models import Feature, Project
+from readthedocs.restapi.views.integrations import GitHubWebhookView
 from readthedocs.restapi.views.task_views import get_status_data
 
 super_auth = base64.b64encode(b'super:test').decode('utf-8')
@@ -632,18 +633,21 @@ class IntegrationsTests(TestCase):
 
     def setUp(self):
         self.project = get(Project)
-        self.version = get(Version, verbose_name='master', project=self.project)
+        self.version = get(Version, verbose_name='master', active=True, project=self.project)
+        self.version_tag = get(Version, verbose_name='v1.0', active=True, project=self.project)
 
-    def test_github_webhook(self, trigger_build):
+    def test_github_webhook_for_branches(self, trigger_build):
         """GitHub webhook API."""
         client = APIClient()
+
         client.post(
             '/api/v2/webhook/github/{0}/'.format(self.project.slug),
             {'ref': 'master'},
             format='json',
         )
         trigger_build.assert_has_calls(
-            [mock.call(force=True, version=mock.ANY, project=self.project)])
+            [mock.call(force=True, version=self.version, project=self.project)])
+
         client.post(
             '/api/v2/webhook/github/{0}/'.format(self.project.slug),
             {'ref': 'non-existent'},
@@ -652,6 +656,52 @@ def test_github_webhook(self, trigger_build):
         trigger_build.assert_has_calls(
             [mock.call(force=True, version=mock.ANY, project=self.project)])
 
+        client.post(
+            '/api/v2/webhook/github/{0}/'.format(self.project.slug),
+            {'ref': 'refs/heads/master'},
+            format='json',
+        )
+        trigger_build.assert_has_calls(
+            [mock.call(force=True, version=self.version, project=self.project)])
+
+    def test_github_webhook_for_tags(self, trigger_build):
+        """GitHub webhook API."""
+        client = APIClient()
+
+        client.post(
+            '/api/v2/webhook/github/{0}/'.format(self.project.slug),
+            {'ref': 'v1.0'},
+            format='json',
+        )
+        trigger_build.assert_has_calls(
+            [mock.call(force=True, version=self.version_tag, project=self.project)])
+
+        client.post(
+            '/api/v2/webhook/github/{0}/'.format(self.project.slug),
+            {'ref': 'refs/heads/non-existent'},
+            format='json',
+        )
+        trigger_build.assert_has_calls(
+            [mock.call(force=True, version=mock.ANY, project=self.project)])
+
+        client.post(
+            '/api/v2/webhook/github/{0}/'.format(self.project.slug),
+            {'ref': 'refs/tags/v1.0'},
+            format='json',
+        )
+        trigger_build.assert_has_calls(
+            [mock.call(force=True, version=self.version_tag, project=self.project)])
+
+    def test_github_parse_ref(self, trigger_build):
+        wh = GitHubWebhookView()
+
+        self.assertEqual(wh._normalize_ref('refs/heads/master'), 'master')
+        self.assertEqual(wh._normalize_ref('refs/heads/v0.1'), 'v0.1')
+        self.assertEqual(wh._normalize_ref('refs/tags/v0.1'), 'v0.1')
+        self.assertEqual(wh._normalize_ref('refs/tags/tag'), 'tag')
+        self.assertEqual(wh._normalize_ref('refs/heads/stable/2018'), 'stable/2018')
+        self.assertEqual(wh._normalize_ref('refs/tags/tag/v0.1'), 'tag/v0.1')
+
     def test_github_invalid_webhook(self, trigger_build):
         """GitHub webhook unhandled event."""
         client = APIClient()