From b3bbc159a0afa28382a8237606aa075c0ee21196 Mon Sep 17 00:00:00 2001 From: sandroroux Date: Wed, 6 Dec 2017 09:29:15 -0500 Subject: [PATCH 1/4] Removed discussions.forum_response_notifications and discussions.send_notifications_for_course --- lms/djangoapps/discussion/config/waffle.py | 24 ------------- lms/djangoapps/discussion/signals/handlers.py | 10 +----- .../discussion/tests/test_signals.py | 36 +++++++------------ lms/djangoapps/discussion/tests/test_tasks.py | 11 ++---- 4 files changed, 16 insertions(+), 65 deletions(-) delete mode 100644 lms/djangoapps/discussion/config/waffle.py diff --git a/lms/djangoapps/discussion/config/waffle.py b/lms/djangoapps/discussion/config/waffle.py deleted file mode 100644 index 2f542d1389..0000000000 --- a/lms/djangoapps/discussion/config/waffle.py +++ /dev/null @@ -1,24 +0,0 @@ -""" -This module contains various configuration settings via -waffle switches for the Discussions app. -""" -from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag, WaffleFlagNamespace, WaffleSwitchNamespace - -# Namespace -WAFFLE_NAMESPACE = u'discussions' - -# Switches -FORUM_RESPONSE_NOTIFICATIONS = u'forum_response_notifications' - -SEND_NOTIFICATIONS_FOR_COURSE = CourseWaffleFlag( - waffle_namespace=WaffleFlagNamespace(name=WAFFLE_NAMESPACE), - flag_name=u'send_notifications_for_course', - flag_undefined_default=False -) - - -def waffle(): - """ - Returns the namespaced, cached, audited Waffle class for Discussions. - """ - return WaffleSwitchNamespace(name=WAFFLE_NAMESPACE, log_prefix=u'Discussions: ') diff --git a/lms/djangoapps/discussion/signals/handlers.py b/lms/djangoapps/discussion/signals/handlers.py index 37ad3e36b2..545003b122 100644 --- a/lms/djangoapps/discussion/signals/handlers.py +++ b/lms/djangoapps/discussion/signals/handlers.py @@ -7,7 +7,7 @@ from django.dispatch import receiver from opaque_keys.edx.keys import CourseKey from django_comment_common import signals -from lms.djangoapps.discussion.config.waffle import waffle, FORUM_RESPONSE_NOTIFICATIONS, SEND_NOTIFICATIONS_FOR_COURSE +from lms.djangoapps.discussion.config.waffle import waffle, FORUM_RESPONSE_NOTIFICATIONS from lms.djangoapps.discussion import tasks from openedx.core.djangoapps.site_configuration.models import SiteConfiguration from openedx.core.djangoapps.theming.helpers import get_current_site @@ -21,14 +21,6 @@ ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY = 'enable_forum_notifications' @receiver(signals.comment_created) def send_discussion_email_notification(sender, user, post, **kwargs): - if not waffle().is_enabled(FORUM_RESPONSE_NOTIFICATIONS): - log.debug('Discussion: Response notifications waffle switch not enabled') - return - - if not SEND_NOTIFICATIONS_FOR_COURSE.is_enabled(CourseKey.from_string(post.thread.course_id)): - log.debug('Discussion: Response notifications not enabled for course: %s.', post.thread.course_id) - return - current_site = get_current_site() if current_site is None: log.info('Discussion: No current site, not sending notification about post: %s.', post.id) diff --git a/lms/djangoapps/discussion/tests/test_signals.py b/lms/djangoapps/discussion/tests/test_signals.py index 2af08a32a7..d937a894fc 100644 --- a/lms/djangoapps/discussion/tests/test_signals.py +++ b/lms/djangoapps/discussion/tests/test_signals.py @@ -2,7 +2,6 @@ from django.test import TestCase import mock from django_comment_common import signals -from lms.djangoapps.discussion.config.waffle import waffle, FORUM_RESPONSE_NOTIFICATIONS, SEND_NOTIFICATIONS_FOR_COURSE from lms.djangoapps.discussion.signals.handlers import ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory, SiteConfigurationFactory from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag @@ -19,54 +18,44 @@ class SendMessageHandlerTestCase(TestCase): @mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site') @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') - @override_waffle_flag(SEND_NOTIFICATIONS_FOR_COURSE, True) def test_comment_created_signal_sends_message(self, mock_send_message, mock_get_current_site): site_config = SiteConfigurationFactory.create(site=self.site) site_config.values[ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY] = True site_config.save() mock_get_current_site.return_value = self.site - with waffle().override(FORUM_RESPONSE_NOTIFICATIONS): - signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) + signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) - mock_send_message.assert_called_once_with(self.post, mock_get_current_site.return_value) + mock_send_message.assert_called_once_with(self.post, mock_get_current_site.return_value) @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') - @override_waffle_flag(SEND_NOTIFICATIONS_FOR_COURSE, True) def test_comment_created_signal_message_not_sent_without_waffle_switch(self, mock_send_message): - with waffle().override(FORUM_RESPONSE_NOTIFICATIONS, active=False): - signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) + signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) - self.assertFalse(mock_send_message.called) + self.assertFalse(mock_send_message.called) @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') def test_comment_created_signal_message_not_sent_without_course_waffle_flag(self, mock_send_message): - with waffle().override(FORUM_RESPONSE_NOTIFICATIONS, active=True): - signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) + signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) - self.assertFalse(mock_send_message.called) + self.assertFalse(mock_send_message.called) @mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site', return_value=None) @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') - @override_waffle_flag(SEND_NOTIFICATIONS_FOR_COURSE, True) def test_comment_created_signal_message_not_sent_without_site(self, mock_send_message, mock_get_current_site): - with waffle().override(FORUM_RESPONSE_NOTIFICATIONS, active=True): - signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) + signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) - self.assertFalse(mock_send_message.called) + self.assertFalse(mock_send_message.called) @mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site') @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') - @override_waffle_flag(SEND_NOTIFICATIONS_FOR_COURSE, True) def test_comment_created_signal_msg_not_sent_without_site_config(self, mock_send_message, mock_get_current_site): mock_get_current_site.return_value = self.site - with waffle().override(FORUM_RESPONSE_NOTIFICATIONS): - signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) + signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) - self.assertFalse(mock_send_message.called) + self.assertFalse(mock_send_message.called) @mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site') @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') - @override_waffle_flag(SEND_NOTIFICATIONS_FOR_COURSE, True) def test_comment_created_signal_msg_not_sent_with_site_config_disabled( self, mock_send_message, mock_get_current_site ): @@ -74,7 +63,6 @@ class SendMessageHandlerTestCase(TestCase): site_config.values[ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY] = False site_config.save() mock_get_current_site.return_value = self.site - with waffle().override(FORUM_RESPONSE_NOTIFICATIONS): - signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) + signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) - self.assertFalse(mock_send_message.called) + self.assertFalse(mock_send_message.called) diff --git a/lms/djangoapps/discussion/tests/test_tasks.py b/lms/djangoapps/discussion/tests/test_tasks.py index 985ff3f5c9..05cb4d583d 100644 --- a/lms/djangoapps/discussion/tests/test_tasks.py +++ b/lms/djangoapps/discussion/tests/test_tasks.py @@ -17,7 +17,6 @@ from django_comment_common.signals import comment_created from edx_ace.recipient import Recipient from edx_ace.renderers import EmailRenderer from edx_ace.utils import date -from lms.djangoapps.discussion.config.waffle import waffle, FORUM_RESPONSE_NOTIFICATIONS, SEND_NOTIFICATIONS_FOR_COURSE from lms.djangoapps.discussion.signals.handlers import ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY from lms.djangoapps.discussion.tasks import _should_send_message from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory @@ -166,7 +165,6 @@ class TaskTestCase(ModuleStoreTestCase): self.permalink_patcher.stop() @ddt.data(True, False) - @override_waffle_flag(SEND_NOTIFICATIONS_FOR_COURSE, True) def test_send_discussion_email_notification(self, user_subscribed): if user_subscribed: non_matching_id = 'not-a-match' @@ -188,9 +186,8 @@ class TaskTestCase(ModuleStoreTestCase): site_config = SiteConfigurationFactory.create(site=site) site_config.values[ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY] = True site_config.save() - with waffle().override(FORUM_RESPONSE_NOTIFICATIONS): - with mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site', return_value=site): - comment_created.send(sender=None, user=user, post=comment) + with mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site', return_value=site): + comment_created.send(sender=None, user=user, post=comment) if user_subscribed: expected_message_context = get_base_template_context(site) @@ -232,7 +229,6 @@ class TaskTestCase(ModuleStoreTestCase): self.assertTrue(self.mock_permalink in rendered_email.body_html) self.assertTrue(message.context['site'].domain in rendered_email.body_html) - @override_waffle_flag(SEND_NOTIFICATIONS_FOR_COURSE, True) def run_should_not_send_email_test(self, comment_dict): self.mock_request.side_effect = make_mock_responder( subscribed_thread_ids=[self.discussion_id], @@ -241,8 +237,7 @@ class TaskTestCase(ModuleStoreTestCase): ) user = mock.Mock() comment = cc.Comment.find(id=comment_dict['id']).retrieve() - with waffle().override(FORUM_RESPONSE_NOTIFICATIONS): - comment_created.send(sender=None, user=user, post=comment) + comment_created.send(sender=None, user=user, post=comment) actual_result = _should_send_message({ 'thread_author_id': self.thread_author.id, From f52f0914fa779289388cca90c6ee8176301e7895 Mon Sep 17 00:00:00 2001 From: sandroroux Date: Wed, 6 Dec 2017 10:12:29 -0500 Subject: [PATCH 2/4] Removed enable_forum_notifications --- lms/djangoapps/discussion/signals/handlers.py | 11 -------- .../discussion/tests/test_signals.py | 25 ------------------- lms/djangoapps/discussion/tests/test_tasks.py | 4 --- 3 files changed, 40 deletions(-) diff --git a/lms/djangoapps/discussion/signals/handlers.py b/lms/djangoapps/discussion/signals/handlers.py index 545003b122..71ae8fea83 100644 --- a/lms/djangoapps/discussion/signals/handlers.py +++ b/lms/djangoapps/discussion/signals/handlers.py @@ -7,7 +7,6 @@ from django.dispatch import receiver from opaque_keys.edx.keys import CourseKey from django_comment_common import signals -from lms.djangoapps.discussion.config.waffle import waffle, FORUM_RESPONSE_NOTIFICATIONS from lms.djangoapps.discussion import tasks from openedx.core.djangoapps.site_configuration.models import SiteConfiguration from openedx.core.djangoapps.theming.helpers import get_current_site @@ -26,16 +25,6 @@ def send_discussion_email_notification(sender, user, post, **kwargs): log.info('Discussion: No current site, not sending notification about post: %s.', post.id) return - try: - if not current_site.configuration.get_value(ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY, False): - log_message = 'Discussion: notifications not enabled for site: %s. Not sending message about post: %s.' - log.info(log_message, current_site, post.id) - return - except SiteConfiguration.DoesNotExist: - log_message = 'Discussion: No SiteConfiguration for site %s. Not sending message about post: %s.' - log.info(log_message, current_site, post.id) - return - send_message(post, current_site) diff --git a/lms/djangoapps/discussion/tests/test_signals.py b/lms/djangoapps/discussion/tests/test_signals.py index d937a894fc..f8142dee10 100644 --- a/lms/djangoapps/discussion/tests/test_signals.py +++ b/lms/djangoapps/discussion/tests/test_signals.py @@ -2,7 +2,6 @@ from django.test import TestCase import mock from django_comment_common import signals -from lms.djangoapps.discussion.signals.handlers import ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory, SiteConfigurationFactory from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag @@ -19,9 +18,6 @@ class SendMessageHandlerTestCase(TestCase): @mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site') @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') def test_comment_created_signal_sends_message(self, mock_send_message, mock_get_current_site): - site_config = SiteConfigurationFactory.create(site=self.site) - site_config.values[ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY] = True - site_config.save() mock_get_current_site.return_value = self.site signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) @@ -45,24 +41,3 @@ class SendMessageHandlerTestCase(TestCase): signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) self.assertFalse(mock_send_message.called) - - @mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site') - @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') - def test_comment_created_signal_msg_not_sent_without_site_config(self, mock_send_message, mock_get_current_site): - mock_get_current_site.return_value = self.site - signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) - - self.assertFalse(mock_send_message.called) - - @mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site') - @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') - def test_comment_created_signal_msg_not_sent_with_site_config_disabled( - self, mock_send_message, mock_get_current_site - ): - site_config = SiteConfigurationFactory.create(site=self.site) - site_config.values[ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY] = False - site_config.save() - mock_get_current_site.return_value = self.site - signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) - - self.assertFalse(mock_send_message.called) diff --git a/lms/djangoapps/discussion/tests/test_tasks.py b/lms/djangoapps/discussion/tests/test_tasks.py index 05cb4d583d..be1b3768ed 100644 --- a/lms/djangoapps/discussion/tests/test_tasks.py +++ b/lms/djangoapps/discussion/tests/test_tasks.py @@ -17,7 +17,6 @@ from django_comment_common.signals import comment_created from edx_ace.recipient import Recipient from edx_ace.renderers import EmailRenderer from edx_ace.utils import date -from lms.djangoapps.discussion.signals.handlers import ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY from lms.djangoapps.discussion.tasks import _should_send_message from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.ace_common.template_context import get_base_template_context @@ -183,9 +182,6 @@ class TaskTestCase(ModuleStoreTestCase): user = mock.Mock() comment = cc.Comment.find(id=self.comment['id']).retrieve() site = Site.objects.get_current() - site_config = SiteConfigurationFactory.create(site=site) - site_config.values[ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY] = True - site_config.save() with mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site', return_value=site): comment_created.send(sender=None, user=user, post=comment) From 4d948672cd741709ec29ee02313662b8af9eb0ae Mon Sep 17 00:00:00 2001 From: sandroroux Date: Wed, 6 Dec 2017 11:43:29 -0500 Subject: [PATCH 3/4] Brought SiteConfig checks back. --- lms/djangoapps/discussion/signals/handlers.py | 10 ++++++++ .../discussion/tests/test_signals.py | 25 +++++++++++++++++++ lms/djangoapps/discussion/tests/test_tasks.py | 4 +++ 3 files changed, 39 insertions(+) diff --git a/lms/djangoapps/discussion/signals/handlers.py b/lms/djangoapps/discussion/signals/handlers.py index 71ae8fea83..44941e91de 100644 --- a/lms/djangoapps/discussion/signals/handlers.py +++ b/lms/djangoapps/discussion/signals/handlers.py @@ -25,6 +25,16 @@ def send_discussion_email_notification(sender, user, post, **kwargs): log.info('Discussion: No current site, not sending notification about post: %s.', post.id) return + try: + if not current_site.configuration.get_value(ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY, False): + log_message = 'Discussion: notifications not enabled for site: %s. Not sending message about post: %s.' + log.info(log_message, current_site, post.id) + return + except SiteConfiguration.DoesNotExist: + log_message = 'Discussion: No SiteConfiguration for site %s. Not sending message about post: %s.' + log.info(log_message, current_site, post.id) + return + send_message(post, current_site) diff --git a/lms/djangoapps/discussion/tests/test_signals.py b/lms/djangoapps/discussion/tests/test_signals.py index f8142dee10..d937a894fc 100644 --- a/lms/djangoapps/discussion/tests/test_signals.py +++ b/lms/djangoapps/discussion/tests/test_signals.py @@ -2,6 +2,7 @@ from django.test import TestCase import mock from django_comment_common import signals +from lms.djangoapps.discussion.signals.handlers import ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory, SiteConfigurationFactory from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag @@ -18,6 +19,9 @@ class SendMessageHandlerTestCase(TestCase): @mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site') @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') def test_comment_created_signal_sends_message(self, mock_send_message, mock_get_current_site): + site_config = SiteConfigurationFactory.create(site=self.site) + site_config.values[ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY] = True + site_config.save() mock_get_current_site.return_value = self.site signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) @@ -41,3 +45,24 @@ class SendMessageHandlerTestCase(TestCase): signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) self.assertFalse(mock_send_message.called) + + @mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site') + @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') + def test_comment_created_signal_msg_not_sent_without_site_config(self, mock_send_message, mock_get_current_site): + mock_get_current_site.return_value = self.site + signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) + + self.assertFalse(mock_send_message.called) + + @mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site') + @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') + def test_comment_created_signal_msg_not_sent_with_site_config_disabled( + self, mock_send_message, mock_get_current_site + ): + site_config = SiteConfigurationFactory.create(site=self.site) + site_config.values[ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY] = False + site_config.save() + mock_get_current_site.return_value = self.site + signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) + + self.assertFalse(mock_send_message.called) diff --git a/lms/djangoapps/discussion/tests/test_tasks.py b/lms/djangoapps/discussion/tests/test_tasks.py index be1b3768ed..05cb4d583d 100644 --- a/lms/djangoapps/discussion/tests/test_tasks.py +++ b/lms/djangoapps/discussion/tests/test_tasks.py @@ -17,6 +17,7 @@ from django_comment_common.signals import comment_created from edx_ace.recipient import Recipient from edx_ace.renderers import EmailRenderer from edx_ace.utils import date +from lms.djangoapps.discussion.signals.handlers import ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY from lms.djangoapps.discussion.tasks import _should_send_message from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.ace_common.template_context import get_base_template_context @@ -182,6 +183,9 @@ class TaskTestCase(ModuleStoreTestCase): user = mock.Mock() comment = cc.Comment.find(id=self.comment['id']).retrieve() site = Site.objects.get_current() + site_config = SiteConfigurationFactory.create(site=site) + site_config.values[ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY] = True + site_config.save() with mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site', return_value=site): comment_created.send(sender=None, user=user, post=comment) From f365aa9b6a0322f35278fe050245874a12d8d66d Mon Sep 17 00:00:00 2001 From: sandroroux Date: Wed, 6 Dec 2017 16:20:47 -0500 Subject: [PATCH 4/4] Deleted waffle-based tests. --- lms/djangoapps/discussion/tests/test_signals.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lms/djangoapps/discussion/tests/test_signals.py b/lms/djangoapps/discussion/tests/test_signals.py index d937a894fc..7ef1e34411 100644 --- a/lms/djangoapps/discussion/tests/test_signals.py +++ b/lms/djangoapps/discussion/tests/test_signals.py @@ -27,18 +27,6 @@ class SendMessageHandlerTestCase(TestCase): mock_send_message.assert_called_once_with(self.post, mock_get_current_site.return_value) - @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') - def test_comment_created_signal_message_not_sent_without_waffle_switch(self, mock_send_message): - signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) - - self.assertFalse(mock_send_message.called) - - @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') - def test_comment_created_signal_message_not_sent_without_course_waffle_flag(self, mock_send_message): - signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) - - self.assertFalse(mock_send_message.called) - @mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site', return_value=None) @mock.patch('lms.djangoapps.discussion.signals.handlers.send_message') def test_comment_created_signal_message_not_sent_without_site(self, mock_send_message, mock_get_current_site):