diff --git a/ietf/community/apps.py b/ietf/community/apps.py new file mode 100644 index 0000000000..ab0a6d6054 --- /dev/null +++ b/ietf/community/apps.py @@ -0,0 +1,12 @@ +# Copyright The IETF Trust 2024, All Rights Reserved + +from django.apps import AppConfig + + +class CommunityConfig(AppConfig): + name = "ietf.community" + + def ready(self): + """Initialize the app after the registry is populated""" + # implicitly connects @receiver-decorated signals + from . import signals # pyflakes: ignore diff --git a/ietf/community/models.py b/ietf/community/models.py index 0407730107..6945918f9a 100644 --- a/ietf/community/models.py +++ b/ietf/community/models.py @@ -1,19 +1,14 @@ # Copyright The IETF Trust 2012-2020, All Rights Reserved # -*- coding: utf-8 -*- - -from django.conf import settings -from django.db import models, transaction -from django.db.models import signals +from django.db import models from django.urls import reverse as urlreverse -from ietf.doc.models import Document, DocEvent, State +from ietf.doc.models import Document, State from ietf.group.models import Group from ietf.person.models import Person, Email from ietf.utils.models import ForeignKey -from .tasks import notify_event_to_subscribers_task - class CommunityList(models.Model): person = ForeignKey(Person, blank=True, null=True) @@ -98,29 +93,3 @@ class EmailSubscription(models.Model): def __str__(self): return "%s to %s (%s changes)" % (self.email, self.community_list, self.notify_on) - - -def notify_events(sender, instance, **kwargs): - if not isinstance(instance, DocEvent): - return - - if not kwargs.get("created", False): - return # only notify on creation - - if instance.doc.type_id != 'draft': - return - - if getattr(instance, "skip_community_list_notification", False): - return - - # kludge alert: queuing a celery task in response to a signal can cause unexpected attempts to - # start a Celery task during tests. To prevent this, don't queue a celery task if we're running - # tests. - if settings.SERVER_MODE != "test": - # Wrap in on_commit in case a transaction is open - transaction.on_commit( - lambda: notify_event_to_subscribers_task.delay(event_id=instance.pk) - ) - - -signals.post_save.connect(notify_events) diff --git a/ietf/community/signals.py b/ietf/community/signals.py new file mode 100644 index 0000000000..20ee761129 --- /dev/null +++ b/ietf/community/signals.py @@ -0,0 +1,44 @@ +# Copyright The IETF Trust 2024, All Rights Reserved + +from django.conf import settings +from django.db import transaction +from django.db.models.signals import post_save +from django.dispatch import receiver + +from ietf.doc.models import DocEvent +from .tasks import notify_event_to_subscribers_task + + +def notify_of_event(event: DocEvent): + """Send subscriber notification emails for a 'draft'-related DocEvent + + If the event is attached to a draft of type 'doc', queues a task to send notification emails to + community list subscribers. No emails will be sent when SERVER_MODE is 'test'. + """ + if event.doc.type_id != "draft": + return + + if getattr(event, "skip_community_list_notification", False): + return + + # kludge alert: queuing a celery task in response to a signal can cause unexpected attempts to + # start a Celery task during tests. To prevent this, don't queue a celery task if we're running + # tests. + if settings.SERVER_MODE != "test": + # Wrap in on_commit in case a transaction is open + transaction.on_commit( + lambda: notify_event_to_subscribers_task.delay(event_id=event.pk) + ) + + +# dispatch_uid ensures only a single signal receiver binding is made +@receiver(post_save, dispatch_uid="notify_of_events_receiver_uid") +def notify_of_events_receiver(sender, instance, **kwargs): + """Call notify_of_event after saving a new DocEvent""" + if not isinstance(instance, DocEvent): + return + + if not kwargs.get("created", False): + return # only notify on creation + + notify_of_event(instance) diff --git a/ietf/community/tests.py b/ietf/community/tests.py index 743242f11b..9bd7789958 100644 --- a/ietf/community/tests.py +++ b/ietf/community/tests.py @@ -1,7 +1,6 @@ # Copyright The IETF Trust 2016-2023, All Rights Reserved # -*- coding: utf-8 -*- - import mock from pyquery import PyQuery @@ -11,6 +10,7 @@ import debug # pyflakes:ignore from ietf.community.models import CommunityList, SearchRule, EmailSubscription +from ietf.community.signals import notify_of_event from ietf.community.utils import docs_matching_community_list_rule, community_list_rules_matching_doc from ietf.community.utils import reset_name_contains_index_for_rule, notify_event_to_subscribers from ietf.community.tasks import notify_event_to_subscribers_task @@ -431,53 +431,58 @@ def test_subscription_for_group(self): r = self.client.get(url) self.assertEqual(r.status_code, 200) - # Mock out the on_commit call so we can tell whether the task was actually queued - @mock.patch("ietf.submit.views.transaction.on_commit", side_effect=lambda x: x()) - @mock.patch("ietf.community.models.notify_event_to_subscribers_task") - def test_notification_signal_receiver(self, mock_notify_task, mock_on_commit): - """Saving a DocEvent should notify subscribers + @mock.patch("ietf.community.signals.notify_of_event") + def test_notification_signal_receiver(self, mock_notify_of_event): + """Saving a newly created DocEvent should notify subscribers - This implicitly tests that notify_events is hooked up to the post_save signal. + This implicitly tests that notify_of_event_receiver is hooked up to the post_save signal. """ # Arbitrary model that's not a DocEvent - person = PersonFactory() - mock_notify_task.reset_mock() # clear any calls that resulted from the factories - # be careful overriding SERVER_MODE - we do it here because the method - # under test does not make this call when in "test" mode - with override_settings(SERVER_MODE="not-test"): - person.save() - self.assertFalse(mock_notify_task.delay.called) - + person = PersonFactory.build() # builds but does not save... + mock_notify_of_event.reset_mock() # clear any calls that resulted from the factories + person.save() + self.assertFalse(mock_notify_of_event.called) + # build a DocEvent that is not yet persisted doc = DocumentFactory() - d = DocEventFactory.build(by=person, doc=doc) - # mock_notify_task.reset_mock() # clear any calls that resulted from the factories + event = DocEventFactory.build(by=person, doc=doc) # builds but does not save... + mock_notify_of_event.reset_mock() # clear any calls that resulted from the factories + event.save() + self.assertEqual(mock_notify_of_event.call_count, 1, "notify_task should be run on creation of DocEvent") + self.assertEqual(mock_notify_of_event.call_args, mock.call(event)) + + # save the existing DocEvent and see that no notification is sent + mock_notify_of_event.reset_mock() + event.save() + self.assertFalse(mock_notify_of_event.called, "notify_task should not be run save of on existing DocEvent") + + # Mock out the on_commit call so we can tell whether the task was actually queued + @mock.patch("ietf.submit.views.transaction.on_commit", side_effect=lambda x: x()) + @mock.patch("ietf.community.signals.notify_event_to_subscribers_task") + def test_notify_of_event(self, mock_notify_task, mock_on_commit): + """The community notification task should be called as intended""" + person = PersonFactory() # builds but does not save... + doc = DocumentFactory() + event = DocEventFactory(by=person, doc=doc) # be careful overriding SERVER_MODE - we do it here because the method # under test does not make this call when in "test" mode with override_settings(SERVER_MODE="not-test"): - d.save() - self.assertEqual(mock_notify_task.delay.call_count, 1, "notify_task should be run on creation of DocEvent") - self.assertEqual(mock_notify_task.delay.call_args, mock.call(event_id = d.pk)) - - mock_notify_task.reset_mock() - with override_settings(SERVER_MODE="not-test"): - d.save() - self.assertFalse(mock_notify_task.delay.called, "notify_task should not be run save of on existing DocEvent") - + notify_of_event(event) + self.assertTrue(mock_notify_task.delay.called, "notify_task should run for a DocEvent on a draft") mock_notify_task.reset_mock() - d = DocEventFactory.build(by=person, doc=doc) - d.skip_community_list_notification = True + + event.skip_community_list_notification = True # be careful overriding SERVER_MODE - we do it here because the method # under test does not make this call when in "test" mode with override_settings(SERVER_MODE="not-test"): - d.save() + notify_of_event(event) self.assertFalse(mock_notify_task.delay.called, "notify_task should not run when skip_community_list_notification is set") - d = DocEventFactory.build(by=person, doc=DocumentFactory(type_id="rfc")) + event = DocEventFactory.build(by=person, doc=DocumentFactory(type_id="rfc")) # be careful overriding SERVER_MODE - we do it here because the method # under test does not make this call when in "test" mode with override_settings(SERVER_MODE="not-test"): - d.save() + notify_of_event(event) self.assertFalse(mock_notify_task.delay.called, "notify_task should not run on a document with type 'rfc'") @mock.patch("ietf.utils.mail.send_mail_text") diff --git a/ietf/utils/management/commands/loadrelated.py b/ietf/utils/management/commands/loadrelated.py index da9d00d5dc..d8ae19dc77 100644 --- a/ietf/utils/management/commands/loadrelated.py +++ b/ietf/utils/management/commands/loadrelated.py @@ -23,7 +23,7 @@ import debug # pyflakes:ignore -from ietf.community.models import notify_events +from ietf.community.signals import notify_of_events_receiver class Command(loaddata.Command): help = (""" @@ -62,7 +62,7 @@ def handle(self, *args, **options): # self.serialization_formats = serializers.get_public_serializer_formats() # - post_save.disconnect(notify_events) + post_save.disconnect(notify_of_events_receiver()) # connection = connections[self.using] self.fixture_count = 0