diff --git a/docs/webhooks.rst b/docs/webhooks.rst index 584548d88ed..7e6e109bf3e 100644 --- a/docs/webhooks.rst +++ b/docs/webhooks.rst @@ -20,6 +20,8 @@ details and a list of HTTP exchanges that have taken place for the integration. You need this information for the URL, webhook, or Payload URL needed by the repository provider such as GitHub, GitLab, or Bitbucket. +.. _webhook-creation: + Webhook Creation ---------------- @@ -181,3 +183,32 @@ on your Read the Docs project, or you can use a account. .. [1] https://developer.github.com/changes/2018-04-25-github-services-deprecation/ + +.. _webhook-deprecated-endpoints: + +I was warned that my project won't automatically build after April 1st +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +In addition to :ref:`no longer supporting GitHub Services `, +we have decided to no longer support several other legacy incoming webhook +endpoints that were used before we introduced project webhook integrations. When +we introduced our webhook integrations, we added several features and improved +security for incoming webhooks and these features were not added to our leagcy +incoming webhooks. New projects have not been able to use our legacy incoming +webhooks since, however if you have a project that has been established for a +while, you may still be using these endpoints. + +After March 1st, 2019, we will stop accepting incoming webhook notifications for +these legacy incoming webhooks. Your project will need to be reconfigured and +have a webhook integration configured, pointing to a new webhook with your VCS +provider. + +In particular, the incoming webhook URLs that will be removed are: + +* ``https://readthedocs.org/build`` +* ``https://readthedocs.org/bitbucket`` +* ``https://readthedocs.org/github`` (as noted :ref:`above `) +* ``https://readthedocs.org/gitlab`` + +In order to establish a new project webhook integration, :ref:`follow +the directions for your VCS provider ` diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py index bd3ca775e03..c6d4bc91188 100644 --- a/readthedocs/core/views/hooks.py +++ b/readthedocs/core/views/hooks.py @@ -17,7 +17,6 @@ from readthedocs.builds.constants import LATEST from readthedocs.core.utils import trigger_build -from readthedocs.notifications.decorators import notify_deprecated_endpoint from readthedocs.projects import constants from readthedocs.projects.models import Feature, Project from readthedocs.projects.tasks import sync_repository_task @@ -127,7 +126,6 @@ def log_info(project, msg): msg=msg)) -@notify_deprecated_endpoint def _build_url(url, projects, branches): """ Map a URL onto specific projects to build that are linked to that URL. @@ -343,7 +341,6 @@ def bitbucket_build(request): @csrf_exempt -@notify_deprecated_endpoint def generic_build(request, project_id_or_slug=None): """ Generic webhook build endpoint. @@ -373,7 +370,10 @@ def generic_build(request, project_id_or_slug=None): if request.method == 'POST': slug = request.POST.get('version_slug', project.default_version) log.info( - "(Incoming Generic Build) %s [%s]", project.slug, slug) + "(Incoming Generic Build) %s [%s]", + project.slug, + slug, + ) _build_version(project, slug) else: return HttpResponse("You must POST to this resource.") diff --git a/readthedocs/notifications/backends.py b/readthedocs/notifications/backends.py index 28529f1f1a6..6cc794ddbbf 100644 --- a/readthedocs/notifications/backends.py +++ b/readthedocs/notifications/backends.py @@ -32,11 +32,7 @@ def send_notification(request, notification): backends = getattr(settings, 'NOTIFICATION_BACKENDS', []) for cls_name in backends: backend = import_string(cls_name)(request) - # Do not send email notification if defined explicitly - if backend.name == EmailBackend.name and not notification.send_email: - pass - else: - backend.send(notification) + backend.send(notification) class Backend(object): @@ -55,11 +51,16 @@ class EmailBackend(Backend): The content body is first rendered from an on-disk template, then passed into the standard email templates as a string. + + If the notification is set to ``send_email=False``, this backend will exit + early from :py:meth:`send`. """ name = 'email' def send(self, notification): + if not notification.send_email: + return # FIXME: if the level is an ERROR an email is received and sometimes # it's not necessary. This behavior should be clearly documented in the # code @@ -114,6 +115,6 @@ def send(self, notification): backend_name=self.name, source_format=HTML, ), - extra_tags='', + extra_tags=notification.extra_tags, user=notification.user, ) diff --git a/readthedocs/notifications/decorators.py b/readthedocs/notifications/decorators.py deleted file mode 100644 index 7817c50fea6..00000000000 --- a/readthedocs/notifications/decorators.py +++ /dev/null @@ -1,51 +0,0 @@ -# -*- coding: utf-8 -*- - -import logging - -from django.db.models import Q -from django.http import HttpRequest - -from readthedocs.projects.models import Project -from readthedocs.projects.notifications import DeprecatedWebhookEndpointNotification - -log = logging.getLogger(__name__) - - -def notify_deprecated_endpoint(function): - """ - Decorator to notify owners that the endpoint is DEPRECATED. - - .. note:: - - See the class ``DeprecatedWebhookEndpointNotification`` which contains - all the logic to send notification for this messages properly without - spamming. - """ - def wrap(*args, **kwargs): - # Called from ``generic_build`` - project_id_or_slug = kwargs.get('project_id_or_slug') - - if project_id_or_slug: - projects = Project.objects.filter( - Q(pk=project_id_or_slug) | Q(slug=project_id_or_slug), - ) - else: - # Called from ``_build_url`` - projects = args[1] # ``projects`` argument - - if projects: - for project in projects: - # Send one notification to each owner of the project - for user in project.users.all(): - notification = DeprecatedWebhookEndpointNotification( - project, - HttpRequest(), - user, - ) - notification.send() - else: - log.info('Projects not found when hitting deprecated webhook') - - return function(*args, **kwargs) - - return wrap diff --git a/readthedocs/notifications/notification.py b/readthedocs/notifications/notification.py index d2300e1f4f0..c4d1ca4de77 100644 --- a/readthedocs/notifications/notification.py +++ b/readthedocs/notifications/notification.py @@ -35,6 +35,7 @@ class Notification(object): subject = None user = None send_email = True + extra_tags = '' def __init__(self, context_object, request, user=None): self.object = context_object diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index d55954fdbcf..4640a7abc73 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -30,12 +30,20 @@ ProjectRelationship, WebHook, ) -from .notifications import ResourceUsageNotification +from .notifications import ( + ResourceUsageNotification, + DeprecatedBuildWebhookNotification, + DeprecatedGitHubWebhookNotification, +) from .tasks import remove_dir class ProjectSendNotificationView(SendNotificationView): - notification_classes = [ResourceUsageNotification] + notification_classes = [ + ResourceUsageNotification, + DeprecatedBuildWebhookNotification, + DeprecatedGitHubWebhookNotification, + ] def get_object_recipients(self, obj): for owner in obj.users.all(): @@ -119,7 +127,7 @@ class ProjectAdmin(GuardedModelAdmin): list_display = ('name', 'slug', 'repo', 'repo_type', 'featured') list_filter = ('repo_type', 'featured', 'privacy_level', 'documentation_type', 'programming_language', - ProjectOwnerBannedFilter) + 'feature__feature_id', ProjectOwnerBannedFilter) list_editable = ('featured',) search_fields = ('slug', 'repo') inlines = [ProjectRelationshipInline, RedirectInline, diff --git a/readthedocs/projects/notifications.py b/readthedocs/projects/notifications.py index d3b4af2e272..db7838bc80e 100644 --- a/readthedocs/projects/notifications.py +++ b/readthedocs/projects/notifications.py @@ -4,6 +4,7 @@ from __future__ import absolute_import from datetime import timedelta from django.utils import timezone +from django.http import HttpRequest from messages_extends.models import Message from readthedocs.notifications import Notification from readthedocs.notifications.constants import REQUIREMENT @@ -17,67 +18,38 @@ class ResourceUsageNotification(Notification): level = REQUIREMENT -class DeprecatedWebhookEndpointNotification(Notification): +class DeprecatedViewNotification(Notification): - """ - Notification for the usage of deprecated webhook endpoints. + """Notification to alert user of a view that is going away.""" - Each time that a view decorated with ``notify_deprecated_endpoint`` is hit, - a new instance of this class is created. Then, ``.send`` is called and the - ``SiteBackend`` will create (avoiding duplication) a site notification and - the ``EmailBackend`` will do nothing (because of ``send_email=False``). - - Besides, a ``message_extends.models.Message`` object is created to track - sending an email if this endpoint is hit again after ``email_period``. When, - ``.send`` is call and the ``email_period`` was reach from the - ``Message.created`` time we mark ``send_email=True`` in this instance and - call the super ``.send`` method that will effectively send the email and - mark the message as ``extra_tags='email_sent'``. - """ - - name = 'deprecated_webhook_endpoint' context_object_name = 'project' subject = '{{ project.name }} project webhook needs to be updated' - send_email = False - email_period = timedelta(days=7) level = REQUIREMENT - def __init__(self, context_object, request, user=None): - super(DeprecatedWebhookEndpointNotification, self).__init__( - context_object, - request, - user, - ) - self.message, created = self._create_message() - - # Mark this notification to be sent as email the first time that it's - # created (user hits this endpoint for the first time) - if created: - self.send_email = True - - def _create_message(self): - # Each time this class is instantiated we create a new Message (it's - # de-duplicated by using the ``message``, ``user`` and ``extra_tags`` - # status) - return Message.objects.get_or_create( - message='{}: {}'.format(self.name, self.get_subject()), - level=self.level, - user=self.user, - extra_tags='email_delayed', - ) - - def send(self, *args, **kwargs): # noqa - if self.message.created + self.email_period < timezone.now(): - # Mark this instance to really send the email and rely on the - # EmailBackend to effectively send the email - self.send_email = True - - # Mark the message as sent and send the email - self.message.extra_tags = 'email_sent' - self.message.save() - - # Create a new Message with ``email_delayed`` so we are prepared to - # de-duplicate the following one - self._create_message() - - super(DeprecatedWebhookEndpointNotification, self).send(*args, **kwargs) + @classmethod + def notify_project_users(cls, projects): + """ + Notify project users of deprecated view. + + :param projects: List of project instances + :type projects: [:py:class:`Project`] + """ + for project in projects: + # Send one notification to each owner of the project + for user in project.users.all(): + notification = cls( + context_object=project, + request=HttpRequest(), + user=user, + ) + notification.send() + + +class DeprecatedGitHubWebhookNotification(DeprecatedViewNotification): + + name = 'deprecated_github_webhook' + + +class DeprecatedBuildWebhookNotification(DeprecatedViewNotification): + + name = 'deprecated_build_webhook' diff --git a/readthedocs/rtd_tests/tests/test_notifications.py b/readthedocs/rtd_tests/tests/test_notifications.py index 2c36023f90c..1fa16323df4 100644 --- a/readthedocs/rtd_tests/tests/test_notifications.py +++ b/readthedocs/rtd_tests/tests/test_notifications.py @@ -10,14 +10,16 @@ from django.test.utils import override_settings from django.contrib.auth.models import User, AnonymousUser from django.utils import timezone -from freezegun import freeze_time from messages_extends.models import Message as PersistentMessage from readthedocs.notifications import Notification, SiteNotification from readthedocs.notifications.backends import EmailBackend, SiteBackend from readthedocs.notifications.constants import ERROR, INFO_NON_PERSISTENT, WARNING_NON_PERSISTENT from readthedocs.projects.models import Project -from readthedocs.projects.notifications import DeprecatedWebhookEndpointNotification +from readthedocs.projects.notifications import ( + DeprecatedGitHubWebhookNotification, + DeprecatedBuildWebhookNotification, +) from readthedocs.builds.models import Build @@ -235,85 +237,36 @@ class DeprecatedWebhookEndpointNotificationTests(TestCase): def setUp(self): PersistentMessage.objects.all().delete() - self.project = fixture.get(Project) self.user = fixture.get(User) + self.project = fixture.get(Project, users=[self.user]) self.request = HttpRequest() - self.notification = DeprecatedWebhookEndpointNotification( - self.project, - self.request, - self.user, - ) - - def test_deduplication(self): - self.assertEqual(PersistentMessage.objects.filter(user=self.user).count(), 1) - for x in range(5): - DeprecatedWebhookEndpointNotification( - self.project, - self.request, - self.user, - ) - self.assertEqual(PersistentMessage.objects.filter(user=self.user).count(), 1) - DeprecatedWebhookEndpointNotification( - self.project, - self.request, - fixture.get(User), - ) - self.assertEqual(PersistentMessage.objects.count(), 2) - self.notification.message.extra_tags = 'email_sent' - self.notification.message.save() - DeprecatedWebhookEndpointNotification( + self.notification = DeprecatedBuildWebhookNotification( self.project, self.request, self.user, ) - self.assertEqual(PersistentMessage.objects.filter(user=self.user).count(), 2) - self.assertEqual(PersistentMessage.objects.count(), 3) @mock.patch('readthedocs.notifications.backends.send_email') - def test_send_email(self, send_email): - # After creating the notification object, ``send_email=True`` - self.assertTrue(self.notification.send_email) - - # Calling ``.send`` on this instance will send the email and the - # ``Message`` will stay in ``email_delayed`` status. - self.notification.send() - self.notification.message.refresh_from_db() - self.assertEqual(self.notification.message.extra_tags, 'email_delayed') + def test_dedupliation(self, send_email): + user = fixture.get(User) + project = fixture.get(Project, main_language_project=None) + project.users.add(user) + project.refresh_from_db() + self.assertEqual(project.users.count(), 1) + + self.assertEqual(PersistentMessage.objects.filter(user=user).count(), 0) + DeprecatedGitHubWebhookNotification.notify_project_users([project]) + + # Site and email notification will go out, site message doesn't have + # any reason to deduplicate yet + self.assertEqual(PersistentMessage.objects.filter(user=user).count(), 1) self.assertTrue(send_email.called) send_email.reset_mock() - - # Hit the endpoint twice - for x in range(2): - notification = DeprecatedWebhookEndpointNotification( - self.project, - self.request, - self.user, - ) - # A SiteNotification is created after calling ``.send`` so we filter for - # the message also to be sure that no new notification was created - self.assertEqual(PersistentMessage.objects.filter( - user=self.user, - message__startswith=DeprecatedWebhookEndpointNotification.name).count(), - 1, - ) - self.assertFalse(notification.send_email) # second notification - notification.send() self.assertFalse(send_email.called) - self.assertEqual(notification.message.extra_tags, 'email_delayed') - - # In 7 days, when the ``.send`` method is called from a new created - # instance, this method will send the email and mark this ``Message`` as - # ``email_sent`` - with freeze_time(timezone.now() + timedelta(days=7)): - notification.send() + # Expect the site message to deduplicate, the email won't + DeprecatedGitHubWebhookNotification.notify_project_users([project]) + self.assertEqual(PersistentMessage.objects.filter(user=user).count(), 1) self.assertTrue(send_email.called) - self.assertEqual(notification.message.extra_tags, 'email_sent') - # A new Message object is created - self.assertEqual(PersistentMessage.objects.filter( - user=self.user, - message__startswith=DeprecatedWebhookEndpointNotification.name).count(), - 2, - ) - self.assertEqual(PersistentMessage.objects.last().extra_tags, 'email_delayed') + send_email.reset_mock() diff --git a/readthedocs/templates/projects/notifications/deprecated_build_webhook_email.html b/readthedocs/templates/projects/notifications/deprecated_build_webhook_email.html new file mode 100644 index 00000000000..fb19f176532 --- /dev/null +++ b/readthedocs/templates/projects/notifications/deprecated_build_webhook_email.html @@ -0,0 +1,6 @@ +

Your project, {{ project.name }}, is currently using a legacy incoming webhook to trigger builds on Read the Docs. Effective April 1st, 2019, Read the Docs will no longer accept incoming webhooks through these endpoints.

+ +

To continue building your Read the Docs project on changes to your repository, you will need to configure a new webhook with your VCS provider. You can find more information on how to configure a new webhook in our documentation, at:

+ +{% comment %}Plain text link because of text version of email{% endcomment %} +

https://docs.readthedocs.io/en/latest/webhooks.html#webhook-deprecated-endpoints

diff --git a/readthedocs/templates/projects/notifications/deprecated_build_webhook_site.html b/readthedocs/templates/projects/notifications/deprecated_build_webhook_site.html new file mode 100644 index 00000000000..33c5e27e616 --- /dev/null +++ b/readthedocs/templates/projects/notifications/deprecated_build_webhook_site.html @@ -0,0 +1 @@ +Your project, {{ project.name }}, needs to be reconfigured in order to continue building automatically after April 1st, 2019. For more information, see our documentation on webhook integrations. diff --git a/readthedocs/templates/projects/notifications/deprecated_webhook_endpoint_email.html b/readthedocs/templates/projects/notifications/deprecated_github_webhook_email.html similarity index 81% rename from readthedocs/templates/projects/notifications/deprecated_webhook_endpoint_email.html rename to readthedocs/templates/projects/notifications/deprecated_github_webhook_email.html index 6ab3247fc21..7d352390d42 100644 --- a/readthedocs/templates/projects/notifications/deprecated_webhook_endpoint_email.html +++ b/readthedocs/templates/projects/notifications/deprecated_github_webhook_email.html @@ -5,4 +5,4 @@

You can find more information on our webhook intergrations in our documentation, at:

{% comment %}Plain text link because of text version of email{% endcomment %} -

https://docs.readthedocs.io/en/latest/webhooks.html

+

https://docs.readthedocs.io/en/latest/webhooks.html#webhook-github-services

diff --git a/readthedocs/templates/projects/notifications/deprecated_github_webhook_site.html b/readthedocs/templates/projects/notifications/deprecated_github_webhook_site.html new file mode 100644 index 00000000000..0832efaf793 --- /dev/null +++ b/readthedocs/templates/projects/notifications/deprecated_github_webhook_site.html @@ -0,0 +1 @@ +Your project, {{ project.name }}, needs to be reconfigured in order to continue building automatically after January 31st, 2019. For more information, see our documentation on webhook integrations. diff --git a/readthedocs/templates/projects/notifications/deprecated_webhook_endpoint_site.html b/readthedocs/templates/projects/notifications/deprecated_webhook_endpoint_site.html deleted file mode 100644 index 5f1ada22295..00000000000 --- a/readthedocs/templates/projects/notifications/deprecated_webhook_endpoint_site.html +++ /dev/null @@ -1 +0,0 @@ -Your project, {{ project.name }}, needs to be reconfigured in order to continue building automatically after Jan 31st. For more information, see our documentation on webhook integrations. diff --git a/requirements/testing.txt b/requirements/testing.txt index 01edf9a67d2..dae26eb67af 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -19,8 +19,6 @@ Mercurial==4.4.2 yamale==1.8.0 pytest-mock==1.10.0 -freezegun==0.3.11 - # local debugging tools datadiff ipdb