diff --git a/common b/common index d8aa79b2b56..a6cb6bbafb3 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit d8aa79b2b560b8f8d691298fec1912c6c679c8c1 +Subproject commit a6cb6bbafb3cf93bfad7bab98c6636021719cc48 diff --git a/readthedocs/builds/views.py b/readthedocs/builds/views.py index b1e8146ea67..7cbe310fbb5 100644 --- a/readthedocs/builds/views.py +++ b/readthedocs/builds/views.py @@ -12,14 +12,15 @@ import logging from builtins import object +from django.contrib import messages from django.contrib.auth.decorators import login_required -from django.urls import reverse from django.http import ( HttpResponseForbidden, HttpResponsePermanentRedirect, HttpResponseRedirect, ) from django.shortcuts import get_object_or_404 +from django.urls import reverse from django.utils.decorators import method_decorator from django.views.generic import DetailView, ListView @@ -28,6 +29,7 @@ from readthedocs.core.utils import trigger_build from readthedocs.projects.models import Project + log = logging.getLogger(__name__) @@ -63,7 +65,18 @@ def post(self, request, project_slug): slug=version_slug, ) - _, build = trigger_build(project=project, version=version) + update_docs_task, build = trigger_build(project=project, version=version) + if (update_docs_task, build) == (None, None): + # Build was skipped + messages.add_message( + request, + messages.WARNING, + "This project is currently disabled and can't trigger new builds.", + ) + return HttpResponseRedirect( + reverse('builds_project_list', args=[project.slug]), + ) + return HttpResponseRedirect( reverse('builds_detail', args=[project.slug, build.pk]), ) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index ea221c4149a..1927c98b74f 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -20,6 +20,7 @@ from readthedocs.builds.constants import LATEST, BUILD_STATE_TRIGGERED from readthedocs.doc_builder.constants import DOCKER_LIMITS + log = logging.getLogger(__name__) SYNC_USER = getattr(settings, 'SYNC_USER', getpass.getuser()) @@ -87,18 +88,22 @@ def prepare_build( :param record: whether or not record the build in a new Build object :param force: build the HTML documentation even if the files haven't changed :param immutable: whether or not create an immutable Celery signature - :returns: Celery signature of update_docs_task to be executed + :returns: Celery signature of update_docs_task and Build instance + :rtype: tuple """ # Avoid circular import - from readthedocs.projects.tasks import update_docs_task from readthedocs.builds.models import Build + from readthedocs.projects.models import Project + from readthedocs.projects.tasks import update_docs_task + + build = None - if project.skip: - log.info( - 'Build not triggered because Project.skip=True: project=%s', + if not Project.objects.is_active(project): + log.warning( + 'Build not triggered because Project is not active: project=%s', project.slug, ) - return None + return (None, None) if not version: version = project.versions.get(slug=LATEST) @@ -158,7 +163,8 @@ def trigger_build(project, version=None, record=True, force=False): :param version: version of the project to be built. Default: ``latest`` :param record: whether or not record the build in a new Build object :param force: build the HTML documentation even if the files haven't changed - :returns: A tuple (Celery AsyncResult promise, Task Signature from ``prepare_build``) + :returns: Celery AsyncResult promise and Build instance + :rtype: tuple """ update_docs_task, build = prepare_build( project, @@ -168,9 +174,9 @@ def trigger_build(project, version=None, record=True, force=False): immutable=True, ) - if update_docs_task is None: - # Current project is skipped - return None + if (update_docs_task, build) == (None, None): + # Build was skipped + return (None, None) return (update_docs_task.apply_async(), build) diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index ababa7641aa..3884eb26db6 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -1,4 +1,5 @@ -"""Project model QuerySet classes""" +# -*- coding: utf-8 -*- +"""Project model QuerySet classes.""" from __future__ import absolute_import @@ -6,9 +7,10 @@ from django.db.models import Q from guardian.shortcuts import get_objects_for_user -from . import constants from readthedocs.core.utils.extend import SettingsOverrideObject +from . import constants + class ProjectQuerySetBase(models.QuerySet): @@ -54,6 +56,24 @@ def private(self, user=None): return self._add_user_repos(queryset, user) return queryset + def is_active(self, project): + """ + Check if the project is active. + + The check consists on, + * the Project shouldn't be marked as skipped. + + :param project: project to be checked + :type project: readthedocs.projects.models.Project + + :returns: whether or not the project is active + :rtype: bool + """ + if project.skip: + return False + + return True + # Aliases def dashboard(self, user=None): diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 06791847dc0..330c22b5b7d 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -16,7 +16,6 @@ from django.contrib import messages from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User -from django.urls import reverse from django.http import ( Http404, HttpResponseBadRequest, @@ -25,6 +24,7 @@ ) from django.middleware.csrf import get_token from django.shortcuts import get_object_or_404, render +from django.urls import reverse from django.utils.safestring import mark_safe from django.utils.translation import ugettext_lazy as _ from django.views.generic import ListView, TemplateView, View @@ -287,6 +287,9 @@ def done(self, form_list, **kwargs): def trigger_initial_build(self, project): """Trigger initial build.""" update_docs, build = prepare_build(project) + if (update_docs, build) == (None, None): + return None + task_promise = chain( attach_webhook.si(project.pk, self.request.user.pk), update_docs, diff --git a/readthedocs/restapi/views/integrations.py b/readthedocs/restapi/views/integrations.py index 3e44530d367..9a27e2ef9af 100644 --- a/readthedocs/restapi/views/integrations.py +++ b/readthedocs/restapi/views/integrations.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- """Endpoints integrating with Github, Bitbucket, and other webhooks.""" from __future__ import ( @@ -13,7 +14,7 @@ import six from django.shortcuts import get_object_or_404 -from rest_framework import permissions +from rest_framework import permissions, status from rest_framework.exceptions import NotFound, ParseError from rest_framework.renderers import JSONRenderer from rest_framework.response import Response @@ -55,11 +56,14 @@ def post(self, request, project_slug): """Set up webhook post view with request and project objects.""" self.request = request self.project = None + self.data = self.get_data() try: self.project = self.get_project(slug=project_slug) + if not Project.objects.is_active(self.project): + resp = {'detail': 'This project is currently disabled'} + return Response(resp, status=status.HTTP_406_NOT_ACCEPTABLE) except Project.DoesNotExist: raise NotFound('Project not found') - self.data = self.get_data() resp = self.handle_webhook() if resp is None: log.info('Unhandled webhook event') diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index a477922f58e..707a4da79c3 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -862,6 +862,22 @@ def setUp(self): }, } + def test_webhook_skipped_project(self, trigger_build): + client = APIClient() + self.project.skip = True + self.project.save() + + response = client.post( + '/api/v2/webhook/github/{0}/'.format( + self.project.slug, + ), + self.github_payload, + format='json', + ) + self.assertDictEqual(response.data, {'detail': 'This project is currently disabled'}) + self.assertEqual(response.status_code, status.HTTP_406_NOT_ACCEPTABLE) + self.assertFalse(trigger_build.called) + def test_github_webhook_for_branches(self, trigger_build): """GitHub webhook API.""" client = APIClient() diff --git a/readthedocs/rtd_tests/tests/test_core_utils.py b/readthedocs/rtd_tests/tests/test_core_utils.py index 78fb68cb1fd..85e7d735507 100644 --- a/readthedocs/rtd_tests/tests/test_core_utils.py +++ b/readthedocs/rtd_tests/tests/test_core_utils.py @@ -18,6 +18,18 @@ def setUp(self): self.project = get(Project, container_time_limit=None) self.version = get(Version, project=self.project) + @mock.patch('readthedocs.projects.tasks.update_docs_task') + def test_trigger_skipped_project(self, update_docs_task): + self.project.skip = True + self.project.save() + result = trigger_build( + project=self.project, + version=self.version, + ) + self.assertEqual(result, (None, None)) + self.assertFalse(update_docs_task.signature.called) + self.assertFalse(update_docs_task.signature().apply_async.called) + @mock.patch('readthedocs.projects.tasks.update_docs_task') def test_trigger_custom_queue(self, update_docs): """Use a custom queue when routing the task""" diff --git a/readthedocs/rtd_tests/tests/test_project_querysets.py b/readthedocs/rtd_tests/tests/test_project_querysets.py index 3772c6688ee..682e54cd9ea 100644 --- a/readthedocs/rtd_tests/tests/test_project_querysets.py +++ b/readthedocs/rtd_tests/tests/test_project_querysets.py @@ -30,6 +30,13 @@ def test_subproject_queryset_as_manager_gets_correct_class(self): 'ManagerFromParentRelatedProjectQuerySetBase' ) + def test_is_active(self): + project = fixture.get(Project, skip=False) + self.assertTrue(Project.objects.is_active(project)) + + project = fixture.get(Project, skip=True) + self.assertFalse(Project.objects.is_active(project)) + class FeatureQuerySetTests(TestCase):