feat: notifications and preferences will be created in batches (#33418)

lms/envs/common.py

@@ -5380,6 +5380,7 @@ def _make_locale_paths(settings):  # pylint: disable=missing-function-docstring
 ############## NOTIFICATIONS EXPIRY ##############
 NOTIFICATIONS_EXPIRY = 60
 EXPIRED_NOTIFICATIONS_DELETE_BATCH_SIZE = 10000
+NOTIFICATION_CREATION_BATCH_SIZE = 99
 
 #### django-simple-history##
 # disable indexing on date field its coming from django-simple-history.

openedx/core/djangoapps/notifications/base_notification.py

@@ -322,3 +322,16 @@ def get_notification_content(notification_type, context):
         if notification_type_content_template:
             return notification_type_content_template.format(**context, **html_tags_context)
     return ''
+
+
+def get_default_values_of_preference(notification_app, notification_type):
+    """
+    Returns default preference for notification_type
+    """
+    default_prefs = NotificationAppManager().get_notification_app_preferences()
+    app_prefs = default_prefs.get(notification_app, {})
+    core_notification_types = app_prefs.get('core_notification_types', [])
+    notification_types = app_prefs.get('notification_types', {})
+    if notification_type in core_notification_types:
+        return notification_types.get('core', {})
+    return notification_types.get(notification_type, {})

openedx/core/djangoapps/notifications/models.py

@@ -130,12 +130,12 @@ def __str__(self):
         return f'{self.user.username} - {self.course_id}'
 
     @staticmethod
-    def get_updated_user_course_preferences(user, course_id):
+    def get_user_course_preference(user_id, course_id):
         """
         Returns updated courses preferences for a user
         """
         preferences, _ = CourseNotificationPreference.objects.get_or_create(
-            user=user,
+            user_id=user_id,
             course_id=course_id,
             is_active=True,
         )
@@ -149,9 +149,13 @@ def get_updated_user_course_preferences(user, course_id):
                 preferences.save()
                 # pylint: disable-next=broad-except
             except Exception as e:
-                log.error(f'Unable to update notification preference for {user.username} to new config. {e}')
+                log.error(f'Unable to update notification preference to new config. {e}')
         return preferences
 
+    @staticmethod
+    def get_updated_user_course_preferences(user, course_id):
+        return CourseNotificationPreference.get_user_course_preference(user.id, course_id)
+
     def get_app_config(self, app_name) -> Dict:
         """
         Returns the app config for the given app name.

openedx/core/djangoapps/notifications/tasks.py

@@ -13,13 +13,15 @@
 from pytz import UTC
 
 from common.djangoapps.student.models import CourseEnrollment
+from openedx.core.djangoapps.notifications.base_notification import get_default_values_of_preference
 from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS
 from openedx.core.djangoapps.notifications.events import notification_generated_event
 from openedx.core.djangoapps.notifications.models import (
     CourseNotificationPreference,
     Notification,
     get_course_notification_preference_config_version
 )
+from openedx.core.djangoapps.notifications.utils import get_list_in_batches
 
 logger = get_task_logger(__name__)
 
@@ -90,49 +92,61 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c
     if not ENABLE_NOTIFICATIONS.is_enabled(course_key):
         return
     user_ids = list(set(user_ids))
+    batch_size = settings.NOTIFICATION_CREATION_BATCH_SIZE
 
-    # check if what is preferences of user and make decision to send notification or not
-    preferences = CourseNotificationPreference.objects.filter(
-        user_id__in=user_ids,
-        course_id=course_key,
-    )
-    preferences = create_notification_pref_if_not_exists(user_ids, list(preferences), course_key)
-    notifications = []
     audience = []
-    for preference in preferences:
-        preference = update_user_preference(preference, preference.user, course_key)
-        if (
-            preference and
-            preference.get_web_config(app_name, notification_type) and
-            preference.get_app_config(app_name).get('enabled', False)
-        ):
-            notifications.append(
-                Notification(
-                    user_id=preference.user_id,
-                    app_name=app_name,
-                    notification_type=notification_type,
-                    content_context=context,
-                    content_url=content_url,
-                    course_id=course_key,
+    notifications_generated = False
+    notification_content = ''
+    default_web_config = get_default_values_of_preference(app_name, notification_type).get('web', True)
+    for batch_user_ids in get_list_in_batches(user_ids, batch_size):
+        # check if what is preferences of user and make decision to send notification or not
+        preferences = CourseNotificationPreference.objects.filter(
+            user_id__in=batch_user_ids,
+            course_id=course_key,
+        )
+        preferences = list(preferences)
+
+        if default_web_config:
+            preferences = create_notification_pref_if_not_exists(batch_user_ids, preferences, course_key)
+
+        notifications = []
+        for preference in preferences:
+            preference = update_user_preference(preference, preference.user_id, course_key)
+            if (
+                preference and
+                preference.get_web_config(app_name, notification_type) and
+                preference.get_app_config(app_name).get('enabled', False)
+            ):
+                notifications.append(
+                    Notification(
+                        user_id=preference.user_id,
+                        app_name=app_name,
+                        notification_type=notification_type,
+                        content_context=context,
+                        content_url=content_url,
+                        course_id=course_key,
+                    )
                 )
-            )
-            audience.append(preference.user_id)
-    # send notification to users but use bulk_create
-    notifications_generated = Notification.objects.bulk_create(notifications)
+                audience.append(preference.user_id)
+        # send notification to users but use bulk_create
+        notification_objects = Notification.objects.bulk_create(notifications)
+        if notification_objects and not notifications_generated:
+            notifications_generated = True
+            notification_content = notification_objects[0].content
+
     if notifications_generated:
-        notification_content = notifications_generated[0].content
         notification_generated_event(
             audience, app_name, notification_type, course_key, content_url, notification_content,
         )
 
 
-def update_user_preference(preference: CourseNotificationPreference, user, course_id):
+def update_user_preference(preference: CourseNotificationPreference, user_id, course_id):
     """
     Update user preference if config version is changed.
     """
     current_version = get_course_notification_preference_config_version()
     if preference.config_version != current_version:
-        return preference.get_updated_user_course_preferences(user, course_id)
+        return preference.get_user_course_preference(user_id, course_id)
     return preference
 
 

openedx/core/djangoapps/notifications/tests/test_tasks.py

@@ -5,8 +5,10 @@
 from unittest.mock import patch
 
 import ddt
+from django.conf import settings
 from edx_toggles.toggles.testutils import override_waffle_flag
 
+from common.djangoapps.student.models import CourseEnrollment
 from common.djangoapps.student.tests.factories import UserFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -159,3 +161,109 @@ def test_send_with_app_disabled_notifications(self, app_name, notification_type)
         # Assert that `Notification` objects are not created for the users.
         notification = Notification.objects.filter(user_id=self.user.id).first()
         self.assertIsNone(notification)
+
+
+@ddt.ddt
+class SendBatchNotificationsTest(ModuleStoreTestCase):
+    """
+    Test that notification and notification preferences are created in batches
+    """
+    def setUp(self):
+        """
+        Setups test case
+        """
+        super().setUp()
+        self.course = CourseFactory.create(
+            org='test_org',
+            number='test_course',
+            run='test_run'
+        )
+
+    def _create_users(self, num_of_users):
+        """
+        Create users and enroll them in course
+        """
+        users = [
+            UserFactory.create(username=f'user{i}', email=f'user{i}@example.com')
+            for i in range(num_of_users)
+        ]
+        for user in users:
+            CourseEnrollment.enroll(user=user, course_key=self.course.id)
+        return users
+
+    @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True)
+    @ddt.data(
+        (settings.NOTIFICATION_CREATION_BATCH_SIZE, 1, 2),
+        (settings.NOTIFICATION_CREATION_BATCH_SIZE + 10, 2, 4),
+        (settings.NOTIFICATION_CREATION_BATCH_SIZE - 10, 1, 2),
+    )
+    @ddt.unpack
+    def test_notification_is_send_in_batch(self, creation_size, prefs_query_count, notifications_query_count):
+        """
+        Tests notifications and notification preferences are created in batches
+        """
+        notification_app = "discussion"
+        notification_type = "new_discussion_post"
+        users = self._create_users(creation_size)
+        user_ids = [user.id for user in users]
+        context = {
+            "post_title": "Test Post",
+            "username": "Test Author"
+        }
+
+        # Creating preferences and asserting query count
+        with self.assertNumQueries(prefs_query_count):
+            send_notifications(user_ids, str(self.course.id), notification_app, notification_type,
+                               context, "http://test.url")
+
+        # Updating preferences for notification creation
+        preferences = CourseNotificationPreference.objects.filter(
+            user_id__in=user_ids,
+            course_id=self.course.id
+        )
+        for preference in preferences:
+            discussion_config = preference.notification_preference_config['discussion']
+            discussion_config['notification_types'][notification_type]['web'] = True
+            preference.save()
+
+        # Creating notifications and asserting query count
+        with self.assertNumQueries(notifications_query_count):
+            send_notifications(user_ids, str(self.course.id), notification_app, notification_type,
+                               context, "http://test.url")
+
+    def test_preference_not_created_for_default_off_preference(self):
+        """
+        Tests if new preferences are NOT created when default preference for
+        notification type is False
+        """
+        notification_app = "discussion"
+        notification_type = "new_discussion_post"
+        users = self._create_users(20)
+        user_ids = [user.id for user in users]
+        context = {
+            "post_title": "Test Post",
+            "username": "Test Author"
+        }
+        with override_waffle_flag(ENABLE_NOTIFICATIONS, active=True):
+            with self.assertNumQueries(1):
+                send_notifications(user_ids, str(self.course.id), notification_app, notification_type,
+                                   context, "http://test.url")
+
+    def test_preference_created_for_default_on_preference(self):
+        """
+        Tests if new preferences are created when default preference for
+        notification type is True
+        """
+        notification_app = "discussion"
+        notification_type = "new_comment"
+        users = self._create_users(20)
+        user_ids = [user.id for user in users]
+        context = {
+            "post_title": "Test Post",
+            "author_name": "Test Author",
+            "replier_name": "Replier Name"
+        }
+        with override_waffle_flag(ENABLE_NOTIFICATIONS, active=True):
+            with self.assertNumQueries(3):
+                send_notifications(user_ids, str(self.course.id), notification_app, notification_type,
+                                   context, "http://test.url")

openedx/core/djangoapps/notifications/utils.py

@@ -42,3 +42,12 @@ def get_show_notifications_tray(user):
             break
 
     return show_notifications_tray
+
+
+def get_list_in_batches(input_list, batch_size):
+    """
+    Divides the list of objects into list of list of objects each of length batch_size.
+    """
+    list_length = len(input_list)
+    for index in range(0, list_length, batch_size):
+        yield input_list[index: index + batch_size]