From 44b62cafe760640a3044bb2efe5316fac430b9ce Mon Sep 17 00:00:00 2001
From: Muhammad Adeel Tajamul
 <77053848+muhammadadeeltajamul@users.noreply.github.com>
Date: Wed, 1 Nov 2023 09:32:35 +0500
Subject: [PATCH] fix: notification audience was not filtering users with no
 preference (#33625)

---
 .../core/djangoapps/notifications/tasks.py    | 42 ++++++++++---------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/openedx/core/djangoapps/notifications/tasks.py b/openedx/core/djangoapps/notifications/tasks.py
index f38c6cebc0..2a3766d41c 100644
--- a/openedx/core/djangoapps/notifications/tasks.py
+++ b/openedx/core/djangoapps/notifications/tasks.py
@@ -101,7 +101,13 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c
     sender_id = context.pop('sender_id', None)
     default_web_config = get_default_values_of_preference(app_name, notification_type).get('web', False)
     generated_notification_audience = []
+
     for batch_user_ids in get_list_in_batches(user_ids, batch_size):
+        if ENABLE_NOTIFICATIONS_FILTERS.is_enabled(course_key):
+            logger.info(f'Sending notifications to {len(batch_user_ids)} users in {course_key}')
+            batch_user_ids = NotificationFilter().apply_filters(batch_user_ids, course_key, notification_type)
+            logger.info(f'After applying filters, sending notifications to {len(batch_user_ids)} users in {course_key}')
+
         # 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,
@@ -115,32 +121,26 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c
         if not preferences:
             continue
 
+        notifications = []
         for preference in preferences:
-            preference = update_user_preference(preference, preference.user_id, course_key)
-            if not (
+            user_id = preference.user_id
+            preference = update_user_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)
             ):
-                batch_user_ids.remove(preference.user_id)
-        if ENABLE_NOTIFICATIONS_FILTERS.is_enabled(course_key):
-            logger.info(f'Sending notifications to {len(batch_user_ids)} users.')
-            batch_user_ids = NotificationFilter().apply_filters(batch_user_ids, course_key, notification_type)
-            logger.info(f'After applying filters, sending notifications to {len(batch_user_ids)} users.')
-
-        notifications = []
-        for user_id in batch_user_ids:
-            notifications.append(
-                Notification(
-                    user_id=user_id,
-                    app_name=app_name,
-                    notification_type=notification_type,
-                    content_context=context,
-                    content_url=content_url,
-                    course_id=course_key,
+                notifications.append(
+                    Notification(
+                        user_id=user_id,
+                        app_name=app_name,
+                        notification_type=notification_type,
+                        content_context=context,
+                        content_url=content_url,
+                        course_id=course_key,
+                    )
                 )
-            )
-            generated_notification_audience.append(user_id)
+                generated_notification_audience.append(user_id)
 
         # send notification to users but use bulk_create
         notification_objects = Notification.objects.bulk_create(notifications)
@@ -149,6 +149,8 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c
             notification_content = notification_objects[0].content
 
     if notifications_generated:
+        logger.info(f'Temp: Notifications generated for {len(generated_notification_audience)} out of '
+                    f'{len(user_ids)} users - {app_name} - {notification_type} - {course_key}.')
         notification_generated_event(
             generated_notification_audience, app_name, notification_type, course_key, content_url,
             notification_content, sender_id=sender_id