diff --git a/openedx/core/djangoapps/notifications/tasks.py b/openedx/core/djangoapps/notifications/tasks.py index 2281a482410..b5ed2a83eaf 100644 --- a/openedx/core/djangoapps/notifications/tasks.py +++ b/openedx/core/djangoapps/notifications/tasks.py @@ -129,7 +129,6 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c user_ids = list(set(user_ids)) batch_size = settings.NOTIFICATION_CREATION_BATCH_SIZE - group_by_id = context.pop('group_by_id', '') grouping_function = NotificationRegistry.get_grouper(notification_type) waffle_flag_enabled = ENABLE_NOTIFICATION_GROUPING.is_enabled(course_key) diff --git a/openedx/core/djangoapps/notifications/tests/test_tasks.py b/openedx/core/djangoapps/notifications/tests/test_tasks.py index 5058c0f492c..4baf0b8378a 100644 --- a/openedx/core/djangoapps/notifications/tests/test_tasks.py +++ b/openedx/core/djangoapps/notifications/tests/test_tasks.py @@ -15,7 +15,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from ..config.waffle import ENABLE_NOTIFICATIONS +from ..config.waffle import ENABLE_NOTIFICATIONS, ENABLE_NOTIFICATION_GROUPING from ..models import CourseNotificationPreference, Notification from ..tasks import ( create_notification_pref_if_not_exists, @@ -190,6 +190,40 @@ def test_notification_not_send_with_preference_disabled(self): send_notifications([self.user.id], str(self.course_1.id), app_name, notification_type, context, content_url) self.assertEqual(len(Notification.objects.all()), 0) + @override_waffle_flag(ENABLE_NOTIFICATION_GROUPING, True) + @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True) + def test_send_notification_with_grouping_enabled(self): + """ + Test send_notifications with grouping enabled. + """ + with patch( + 'openedx.core.djangoapps.notifications.tasks.group_user_notifications') as group_user_notifications_mock: + context = { + 'post_title': 'Post title', + 'author_name': 'author name', + 'replier_name': 'replier name', + 'group_by_id': 'group_by_id', + } + content_url = 'https://example.com/' + send_notifications( + [self.user.id], + str(self.course_1.id), + 'discussion', + 'new_comment', + {**context}, + content_url + ) + send_notifications( + [self.user.id], + str(self.course_1.id), + 'discussion', + 'new_comment', + {**context}, + content_url + ) + self.assertEqual(Notification.objects.filter(user_id=self.user.id).count(), 1) + group_user_notifications_mock.assert_called_once() + @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True) @ddt.data( ('discussion', 'new_comment_on_response'), # core notification @@ -220,7 +254,7 @@ def test_send_with_app_disabled_notifications(self, app_name, notification_type) def test_notification_not_created_when_context_is_incomplete(self): try: send_notifications([self.user.id], str(self.course_1.id), "discussion", "new_comment", {}, "") - except Exception as exc: # pylint: disable=broad-except + except Exception as exc: # pylint: disable=broad-except assert isinstance(exc, ValidationError) @@ -229,6 +263,7 @@ class SendBatchNotificationsTest(ModuleStoreTestCase): """ Test that notification and notification preferences are created in batches """ + def setUp(self): """ Setups test case @@ -254,9 +289,9 @@ def _create_users(self, num_of_users): @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True) @ddt.data( - (settings.NOTIFICATION_CREATION_BATCH_SIZE, 7, 3), - (settings.NOTIFICATION_CREATION_BATCH_SIZE + 10, 9, 6), - (settings.NOTIFICATION_CREATION_BATCH_SIZE - 10, 7, 3), + (settings.NOTIFICATION_CREATION_BATCH_SIZE, 10, 4), + (settings.NOTIFICATION_CREATION_BATCH_SIZE + 10, 12, 7), + (settings.NOTIFICATION_CREATION_BATCH_SIZE - 10, 10, 4), ) @ddt.unpack def test_notification_is_send_in_batch(self, creation_size, prefs_query_count, notifications_query_count): @@ -306,7 +341,7 @@ def test_preference_not_created_for_default_off_preference(self): "username": "Test Author" } with override_waffle_flag(ENABLE_NOTIFICATIONS, active=True): - with self.assertNumQueries(7): + with self.assertNumQueries(10): send_notifications(user_ids, str(self.course.id), notification_app, notification_type, context, "http://test.url") @@ -325,7 +360,7 @@ def test_preference_created_for_default_on_preference(self): "replier_name": "Replier Name" } with override_waffle_flag(ENABLE_NOTIFICATIONS, active=True): - with self.assertNumQueries(9): + with self.assertNumQueries(12): send_notifications(user_ids, str(self.course.id), notification_app, notification_type, context, "http://test.url") @@ -374,6 +409,7 @@ class TestDeleteNotificationTask(ModuleStoreTestCase): """ Tests delete_notification_function """ + def setUp(self): """ Setup