From e201f5b4a2578700e6920a21fb3a842ea21a63c3 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 29 Jan 2020 14:16:34 -0500 Subject: [PATCH 1/2] Protection against None when sending notifications --- readthedocs/projects/tasks.py | 6 +++-- .../tests/test_build_notifications.py | 24 +++++++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index fdb0c5c60da..94c352c033f 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -1118,7 +1118,9 @@ def build_docs_class(self, builder_class): def send_notifications(self, version_pk, build_pk, email=False): """Send notifications on build failure.""" - if self.version.type != EXTERNAL: + # Try to infer the version type if we can + # before creating a task. + if not self.version or self.version.type != EXTERNAL: send_notifications.delay(version_pk, build_pk=build_pk, email=email) def is_type_sphinx(self): @@ -1694,7 +1696,7 @@ def _sync_imported_files(version, build, changed_files): def send_notifications(version_pk, build_pk, email=False): version = Version.objects.get_object_or_log(pk=version_pk) - if not version: + if not version or version.type == EXTERNAL: return build = Build.objects.get(pk=build_pk) diff --git a/readthedocs/rtd_tests/tests/test_build_notifications.py b/readthedocs/rtd_tests/tests/test_build_notifications.py index e01e9a6ab42..7629de2d498 100644 --- a/readthedocs/rtd_tests/tests/test_build_notifications.py +++ b/readthedocs/rtd_tests/tests/test_build_notifications.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- """Notifications sent after build is completed.""" import django_dynamic_fixture as fixture @@ -6,6 +5,7 @@ from django.test import TestCase from mock import patch +from readthedocs.builds.constants import EXTERNAL from readthedocs.builds.models import Build, Version from readthedocs.projects.forms import WebHookForm from readthedocs.projects.models import EmailHook, Project, WebHook @@ -17,7 +17,7 @@ def setUp(self): self.project = fixture.get(Project) self.version = fixture.get(Version, project=self.project) self.build = fixture.get(Build, version=self.version) - + @patch('readthedocs.builds.managers.log') def test_send_notification_none_if_wrong_version_pk(self, mock_logger): self.assertFalse(Version.objects.filter(pk=345343).exists()) @@ -38,11 +38,31 @@ def test_send_webhook_notification(self): self.assertEqual(len(mail.outbox), 0) + def test_dont_send_webhook_notifications_for_external_versions(self): + fixture.get(WebHook, project=self.project) + self.version.type = EXTERNAL + self.version.save() + + with patch('readthedocs.projects.tasks.requests.post') as mock: + mock.return_value = None + send_notifications(self.version.pk, self.build.pk) + mock.assert_not_called() + + self.assertEqual(len(mail.outbox), 0) + def test_send_email_notification(self): fixture.get(EmailHook, project=self.project) send_notifications(self.version.pk, self.build.pk, email=True) self.assertEqual(len(mail.outbox), 1) + def test_dont_send_email_notifications_for_external_versions(self): + fixture.get(EmailHook, project=self.project) + self.version.type = EXTERNAL + self.version.save() + + send_notifications(self.version.pk, self.build.pk, email=True) + self.assertEqual(len(mail.outbox), 0) + def test_send_email_and_webhook__notification(self): fixture.get(EmailHook, project=self.project) fixture.get(WebHook, project=self.project) From adeeb4a1ed58d64fbf011270585501252a875820 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 20 Apr 2020 12:09:59 -0500 Subject: [PATCH 2/2] Update docs --- docs/guides/build-notifications.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/guides/build-notifications.rst b/docs/guides/build-notifications.rst index 3e8fcfe394f..bf2d194ccc7 100644 --- a/docs/guides/build-notifications.rst +++ b/docs/guides/build-notifications.rst @@ -1,6 +1,11 @@ Enabling Build Notifications ============================ +.. note:: + + Currently we don't send notifications when + a :doc:`build from a pull/merge request fails `. + Using email -----------