Split up deprecated view notification to GitHub and other webhook endpoints

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 March 1st
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+In addition to :ref:`no longer supporting GitHub Services <webhook-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 <webhook-github-services>`)
+* ``https://readthedocs.org/gitlab``
+
+In order to establish a new project webhook integration, :ref:`follow
+the directions for your VCS provider <webhook-creation>`

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.")

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):
@@ -60,6 +56,8 @@ class EmailBackend(Backend):
     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 +112,6 @@ def send(self, notification):
                 backend_name=self.name,
                 source_format=HTML,
             ),
-            extra_tags='',
+            extra_tags=notification.extra_tags,
             user=notification.user,
         )

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

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,44 +18,83 @@ class ResourceUsageNotification(Notification):
     level = REQUIREMENT
 
 
-class DeprecatedWebhookEndpointNotification(Notification):
+class DeprecatedViewNotification(Notification):
 
     """
-    Notification for the usage of deprecated webhook endpoints.
-
-    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'``.
+    Notification to alert user of a view that is going away.
+
+    This notification is used for cases where we want to alert the project
+    users that a view that they are using is going to be going away.
+
+    .. warning::
+        This is currently used primarily for deprecated webhook endpoints, which
+        aren't even hit by a user. There are likely some mechanics to this
+        class that expect a webhook endpoint and not a generic view.
+
+    The first time that a notification is sent to a user, ``SiteBackend`` will
+    create (avoiding duplication) a site notification and the ``EmailBackend``
+    will send an email notification. The :py:cls:`Message` object will now have
+    ``extra_tags`` of ``email_delayed``. This means that the first email was
+    sent and that we won't send the second email for
+    ``DeprecatedViewNotification.email_period``.
+
+    The second time that a notification is sent to a user,
+    :py:cls:`message_extends.models.Message` will deduplicate the site message
+    and we rely on message meta data, stored in ``extra_tags`` on the model, to
+    determine if an email was already sent. We will send a second email to a
+    :py:cls:`Message` that has ``email_delayed`` in extra_tags, at which point
+    we'll set ``extra_tags`` to have ``email_sent``. Any further attempts to
+    send a message won't work as the ``extra_tags`` is no longer
+    ``email_delayed``.
     """
 
-    name = 'deprecated_webhook_endpoint'
+    # This is an abstract class, we won't set a name yet.
+    name = None
     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__(
+        super(DeprecatedViewNotification, self).__init__(
             context_object,
             request,
             user,
         )
         self.message, created = self._create_message()
 
+        if self.name is None:
+            raise ValueError('{} is an abstract class.'.format(
+                self.__class__.__name__,
+            ))
+
         # 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
 
+    @classmethod
+    def notify_project_users(cls, projects):
+        """
+        Notify project users of deprecated view.
+
+        This is primarily used for deprecated webhook endpoints, though is not
+        particular to this usage.
+
+        :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()
+
     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``
@@ -80,4 +120,14 @@ def send(self, *args, **kwargs):  # noqa
             # de-duplicate the following one
             self._create_message()
 
-        super(DeprecatedWebhookEndpointNotification, self).send(*args, **kwargs)
+        super(DeprecatedViewNotification, self).send(*args, **kwargs)
+
+
+class DeprecatedGitHubWebhookNotification(DeprecatedViewNotification):
+
+    name = 'deprecated_github_webhook'
+
+
+class DeprecatedBuildWebhookNotification(DeprecatedViewNotification):
+
+    name = 'deprecated_build_webhook'

readthedocs/rtd_tests/tests/test_notifications.py

@@ -17,7 +17,10 @@
 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,34 +238,46 @@ 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.notification = DeprecatedBuildWebhookNotification(
             self.project,
             self.request,
             self.user,
         )
 
+    def test_classmethod_for_project_users(self):
+        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])
+        self.assertEqual(PersistentMessage.objects.filter(user=user).count(), 1)
+        DeprecatedGitHubWebhookNotification.notify_project_users([project])
+        self.assertEqual(PersistentMessage.objects.filter(user=user).count(), 1)
+
     def test_deduplication(self):
         self.assertEqual(PersistentMessage.objects.filter(user=self.user).count(), 1)
         for x in range(5):
-            DeprecatedWebhookEndpointNotification(
+            DeprecatedBuildWebhookNotification(
                 self.project,
                 self.request,
                 self.user,
             )
         self.assertEqual(PersistentMessage.objects.filter(user=self.user).count(), 1)
-        DeprecatedWebhookEndpointNotification(
+        DeprecatedBuildWebhookNotification(
             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(
+        DeprecatedBuildWebhookNotification(
             self.project,
             self.request,
             self.user,
@@ -285,7 +300,7 @@ def test_send_email(self, send_email):
 
         # Hit the endpoint twice
         for x in range(2):
-            notification = DeprecatedWebhookEndpointNotification(
+            notification = DeprecatedBuildWebhookNotification(
                 self.project,
                 self.request,
                 self.user,
@@ -294,7 +309,7 @@ def test_send_email(self, send_email):
         # 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(),
+            message__startswith=DeprecatedBuildWebhookNotification.name).count(),
             1,
         )
         self.assertFalse(notification.send_email)  # second notification
@@ -313,7 +328,7 @@ def test_send_email(self, send_email):
         # A new Message object is created
         self.assertEqual(PersistentMessage.objects.filter(
             user=self.user,
-            message__startswith=DeprecatedWebhookEndpointNotification.name).count(),
+            message__startswith=DeprecatedBuildWebhookNotification.name).count(),
             2,
         )
         self.assertEqual(PersistentMessage.objects.last().extra_tags, 'email_delayed')

readthedocs/templates/projects/notifications/deprecated_build_webhook_email.html

@@ -0,0 +1,6 @@
+<p>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.</p>
+
+<p>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:</p>
+
+{% comment %}Plain text link because of text version of email{% endcomment %}
+<p><a href="https://docs.readthedocs.io/en/latest/webhooks.html#webhook-deprecated-endpoints">https://docs.readthedocs.io/en/latest/webhooks.html#webhook-deprecated-endpoints</a></p>

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, <a href="https://docs.readthedocs.io/en/latest/webhooks.html#webhook-deprecated-endpoints">see our documentation on webhook integrations</a>.

readthedocs/templates/projects/notifications/deprecated_webhook_endpoint_email.html → readthedocs/templates/projects/notifications/deprecated_github_webhook_email.html

@@ -5,4 +5,4 @@
 <p>You can find more information on our webhook intergrations in our documentation, at:</p>
 
 {% comment %}Plain text link because of text version of email{% endcomment %}
-<p><a href="https://docs.readthedocs.io/en/latest/webhooks.html">https://docs.readthedocs.io/en/latest/webhooks.html</a></p>
+<p><a href="https://docs.readthedocs.io/en/latest/webhooks.html#webhook-github-services">https://docs.readthedocs.io/en/latest/webhooks.html#webhook-github-services</a></p>

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, <a href="https://docs.readthedocs.io/en/latest/webhooks.html#webhook-github-services">see our documentation on webhook integrations</a>.