feat: added unit tests and refactored

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)

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