Merge pull request #4733 from stsewd/fix-webhook-latest

Don't build latest on webhook if it is deactivated

readthedocs/core/views/hooks.py

@@ -33,35 +33,19 @@ def _build_version(project, slug, already_built=()):
 
     All webhook logic should route here to call ``trigger_build``.
     """
-    default = project.default_branch or (project.vcs_repo().fallback_branch)
     if not project.has_valid_webhook:
         project.has_valid_webhook = True
         project.save()
-    if slug == default and slug not in already_built:
-        # short circuit versions that are default
-        # these will build at "latest", and thus won't be
-        # active
-        latest_version = project.versions.get(slug=LATEST)
-        trigger_build(project=project, version=latest_version, force=True)
-        log.info("(Version build) Building %s:%s",
-                 project.slug, latest_version.slug)
-        if project.versions.exclude(active=False).filter(slug=slug).exists():
-            # Handle the case where we want to build the custom branch too
-            slug_version = project.versions.get(slug=slug)
-            trigger_build(project=project, version=slug_version, force=True)
-            log.info("(Version build) Building %s:%s",
-                     project.slug, slug_version.slug)
-        return LATEST
-
-    if project.versions.exclude(active=True).filter(slug=slug).exists():
-        log.info("(Version build) Not Building %s", slug)
-        return None
-
-    if slug not in already_built:
-        version = project.versions.get(slug=slug)
+    # Previously we were building the latest version (inactive or active)
+    # when building the default version,
+    # some users may have relied on this to update the version list #4450
+    version = project.versions.filter(active=True, slug=slug).first()
+    if version and slug not in already_built:
+        log.info(
+            "(Version build) Building %s:%s",
+            project.slug, version.slug,
+        )
         trigger_build(project=project, version=version, force=True)
-        log.info("(Version build) Building %s:%s",
-                 project.slug, version.slug)
         return slug
 
     log.info("(Version build) Not Building %s", slug)

readthedocs/rtd_tests/tests/test_api.py

@@ -1,14 +1,18 @@
 # -*- coding: utf-8 -*-
 from __future__ import (
-    absolute_import, division, print_function, unicode_literals)
+    absolute_import,
+    division,
+    print_function,
+    unicode_literals,
+)
 
 import base64
 import datetime
 import json
-from builtins import str
 
 import mock
 from allauth.socialaccount.models import SocialAccount
+from builtins import str
 from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
 from django.http import QueryDict
@@ -18,10 +22,11 @@
 from rest_framework import status
 from rest_framework.test import APIClient
 
+from readthedocs.builds.constants import LATEST
 from readthedocs.builds.models import Build, BuildCommandResult, Version
 from readthedocs.integrations.models import Integration
 from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
-from readthedocs.projects.models import Feature, Project, APIProject
+from readthedocs.projects.models import APIProject, Feature, Project
 from readthedocs.restapi.views.integrations import GitHubWebhookView
 from readthedocs.restapi.views.task_views import get_status_data
 
@@ -661,8 +666,14 @@ class IntegrationsTests(TestCase):
 
     def setUp(self):
         self.project = get(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)
+        self.version = get(
+            Version, slug='master', verbose_name='master',
+            active=True, project=self.project
+        )
+        self.version_tag = get(
+            Version, slug='v1.0', verbose_name='v1.0',
+            active=True, project=self.project
+        )
 
     def test_github_webhook_for_branches(self, trigger_build):
         """GitHub webhook API."""
@@ -903,6 +914,109 @@ def test_generic_api_falls_back_to_token_auth(self, trigger_build):
         self.assertEqual(resp.status_code, 200)
         self.assertTrue(resp.data['build_triggered'])
 
+    def test_webhook_doesnt_build_latest_if_is_deactivated(self, trigger_build):
+        client = APIClient()
+        integration = Integration.objects.create(
+            project=self.project,
+            integration_type=Integration.API_WEBHOOK,
+        )
+
+        latest_version = self.project.versions.get(slug=LATEST)
+        latest_version.active = False
+        latest_version.save()
+
+        default_branch = self.project.versions.get(slug='master')
+        default_branch.active = False
+        default_branch.save()
+
+        resp = client.post(
+            '/api/v2/webhook/{}/{}/'.format(
+                self.project.slug,
+                integration.pk,
+            ),
+            {'token': integration.token, 'branches': default_branch.slug},
+            format='json',
+        )
+        self.assertEqual(resp.status_code, 200)
+        self.assertFalse(resp.data['build_triggered'])
+        trigger_build.assert_not_called()
+
+    def test_webhook_builds_only_master(self, trigger_build):
+        client = APIClient()
+        integration = Integration.objects.create(
+            project=self.project,
+            integration_type=Integration.API_WEBHOOK,
+        )
+
+        latest_version = self.project.versions.get(slug=LATEST)
+        latest_version.active = False
+        latest_version.save()
+
+        default_branch = self.project.versions.get(slug='master')
+
+        self.assertFalse(latest_version.active)
+        self.assertTrue(default_branch.active)
+
+        resp = client.post(
+            '/api/v2/webhook/{}/{}/'.format(
+                self.project.slug,
+                integration.pk,
+            ),
+            {'token': integration.token, 'branches': default_branch.slug},
+            format='json',
+        )
+        self.assertEqual(resp.status_code, 200)
+        self.assertTrue(resp.data['build_triggered'])
+        self.assertEqual(resp.data['versions'], ['master'])
+
+    def test_webhook_build_latest_and_master(self, trigger_build):
+        client = APIClient()
+        integration = Integration.objects.create(
+            project=self.project,
+            integration_type=Integration.API_WEBHOOK,
+        )
+
+        latest_version = self.project.versions.get(slug=LATEST)
+        default_branch = self.project.versions.get(slug='master')
+
+        self.assertTrue(latest_version.active)
+        self.assertTrue(default_branch.active)
+
+        resp = client.post(
+            '/api/v2/webhook/{}/{}/'.format(
+                self.project.slug,
+                integration.pk,
+            ),
+            {'token': integration.token, 'branches': default_branch.slug},
+            format='json',
+        )
+        self.assertEqual(resp.status_code, 200)
+        self.assertTrue(resp.data['build_triggered'])
+        self.assertEqual(set(resp.data['versions']), {'latest', 'master'})
+
+    def test_webhook_build_another_branch(self, trigger_build):
+        client = APIClient()
+        integration = Integration.objects.create(
+            project=self.project,
+            integration_type=Integration.API_WEBHOOK,
+        )
+
+        version_v1 = self.project.versions.get(slug='v1.0')
+
+        self.assertTrue(version_v1.active)
+
+        resp = client.post(
+            '/api/v2/webhook/{}/{}/'.format(
+                self.project.slug,
+                integration.pk,
+            ),
+            {'token': integration.token, 'branches': version_v1.slug},
+            format='json',
+        )
+        self.assertEqual(resp.status_code, 200)
+        self.assertTrue(resp.data['build_triggered'])
+        self.assertEqual(resp.data['versions'], ['v1.0'])
+
 
 class APIVersionTests(TestCase):
     fixtures = ['eric', 'test_data']