From ad513cbd235f6bc60f519be0bb0064fbedd567af Mon Sep 17 00:00:00 2001 From: Ahtisham Shahid Date: Wed, 3 Dec 2025 16:38:17 +0500 Subject: [PATCH] fix: removed course_id from notification flag implementation (#37716) --- lms/djangoapps/discussion/rest_api/tasks.py | 6 +++--- .../djangoapps/notifications/config/waffle.py | 4 ++-- .../core/djangoapps/notifications/tasks.py | 2 +- .../test_tasks_with_account_level_pref.py | 10 +++++----- .../core/djangoapps/notifications/utils.py | 19 ++++--------------- .../core/djangoapps/notifications/views.py | 6 +++--- 6 files changed, 18 insertions(+), 29 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/tasks.py b/lms/djangoapps/discussion/rest_api/tasks.py index cd725a3513..c8d983f90c 100644 --- a/lms/djangoapps/discussion/rest_api/tasks.py +++ b/lms/djangoapps/discussion/rest_api/tasks.py @@ -31,7 +31,7 @@ def send_thread_created_notification(thread_id, course_key_str, user_id, notify_ Send notification when a new thread is created """ course_key = CourseKey.from_string(course_key_str) - if not ENABLE_NOTIFICATIONS.is_enabled(course_key): + if not ENABLE_NOTIFICATIONS.is_enabled(): return thread = Thread(id=thread_id).retrieve() user = User.objects.get(id=user_id) @@ -55,7 +55,7 @@ def send_response_notifications(thread_id, course_key_str, user_id, comment_id, Send notifications to users who are subscribed to the thread. """ course_key = CourseKey.from_string(course_key_str) - if not ENABLE_NOTIFICATIONS.is_enabled(course_key): + if not ENABLE_NOTIFICATIONS.is_enabled(): return thread = Thread(id=thread_id).retrieve() user = User.objects.get(id=user_id) @@ -74,7 +74,7 @@ def send_response_endorsed_notifications(thread_id, response_id, course_key_str, Send notifications when a response is marked answered/ endorsed """ course_key = CourseKey.from_string(course_key_str) - if not ENABLE_NOTIFICATIONS.is_enabled(course_key): + if not ENABLE_NOTIFICATIONS.is_enabled(): return thread = Thread(id=thread_id).retrieve() response = Comment(id=response_id).retrieve() diff --git a/openedx/core/djangoapps/notifications/config/waffle.py b/openedx/core/djangoapps/notifications/config/waffle.py index e6e8f462d4..8ea010c171 100644 --- a/openedx/core/djangoapps/notifications/config/waffle.py +++ b/openedx/core/djangoapps/notifications/config/waffle.py @@ -13,10 +13,10 @@ WAFFLE_NAMESPACE = 'notifications' # .. toggle_description: Waffle flag to enable the Notifications feature # .. toggle_use_cases: temporary, open_edx # .. toggle_creation_date: 2023-05-05 -# .. toggle_target_removal_date: 2023-11-05 +# .. toggle_target_removal_date: None # .. toggle_warning: When the flag is ON, Notifications feature is enabled. # .. toggle_tickets: INF-866 -ENABLE_NOTIFICATIONS = CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.enable_notifications', __name__) +ENABLE_NOTIFICATIONS = WaffleFlag(f'{WAFFLE_NAMESPACE}.enable_notifications', __name__) # .. toggle_name: notifications.enable_email_notifications # .. toggle_implementation: WaffleFlag diff --git a/openedx/core/djangoapps/notifications/tasks.py b/openedx/core/djangoapps/notifications/tasks.py index 290a7c02b3..153f18f0ba 100644 --- a/openedx/core/djangoapps/notifications/tasks.py +++ b/openedx/core/djangoapps/notifications/tasks.py @@ -104,7 +104,7 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c """ # pylint: disable=too-many-statements course_key = CourseKey.from_string(course_key) - if not ENABLE_NOTIFICATIONS.is_enabled(course_key): + if not ENABLE_NOTIFICATIONS.is_enabled(): return if not is_notification_valid(notification_type, context): diff --git a/openedx/core/djangoapps/notifications/tests/test_tasks_with_account_level_pref.py b/openedx/core/djangoapps/notifications/tests/test_tasks_with_account_level_pref.py index ae65afc9aa..ccc0d0c483 100644 --- a/openedx/core/djangoapps/notifications/tests/test_tasks_with_account_level_pref.py +++ b/openedx/core/djangoapps/notifications/tests/test_tasks_with_account_level_pref.py @@ -216,9 +216,9 @@ class SendBatchNotificationsTest(ModuleStoreTestCase): @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True) @ddt.data( - (settings.NOTIFICATION_CREATION_BATCH_SIZE, 12, 3), - (settings.NOTIFICATION_CREATION_BATCH_SIZE + 10, 14, 5), - (settings.NOTIFICATION_CREATION_BATCH_SIZE - 10, 12, 3), + (settings.NOTIFICATION_CREATION_BATCH_SIZE, 10, 3), + (settings.NOTIFICATION_CREATION_BATCH_SIZE + 10, 12, 5), + (settings.NOTIFICATION_CREATION_BATCH_SIZE - 10, 10, 3), ) @ddt.unpack def test_notification_is_send_in_batch(self, creation_size, prefs_query_count, notifications_query_count): @@ -268,7 +268,7 @@ class SendBatchNotificationsTest(ModuleStoreTestCase): "username": "Test Author" } with override_waffle_flag(ENABLE_NOTIFICATIONS, active=True): - with self.assertNumQueries(12): + with self.assertNumQueries(10): send_notifications(user_ids, str(self.course.id), notification_app, notification_type, context, "http://test.url") @@ -288,7 +288,7 @@ class SendBatchNotificationsTest(ModuleStoreTestCase): "replier_name": "Replier Name" } with override_waffle_flag(ENABLE_NOTIFICATIONS, active=True): - with self.assertNumQueries(14): + with self.assertNumQueries(12): send_notifications(user_ids, str(self.course.id), notification_app, notification_type, context, "http://test.url") diff --git a/openedx/core/djangoapps/notifications/utils.py b/openedx/core/djangoapps/notifications/utils.py index 7b3f574813..8b31f5ee8a 100644 --- a/openedx/core/djangoapps/notifications/utils.py +++ b/openedx/core/djangoapps/notifications/utils.py @@ -3,7 +3,7 @@ Utils function for notifications app """ from typing import Dict, List, Set -from common.djangoapps.student.models import CourseAccessRole, CourseEnrollment +from common.djangoapps.student.models import CourseAccessRole from openedx.core.djangoapps.django_comment_common.models import Role from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS from openedx.core.lib.cache_utils import request_cached @@ -29,22 +29,11 @@ def find_pref_in_normalized_prefs(pref_name, app_name, prefs_list): return None -def get_show_notifications_tray(user): +def get_show_notifications_tray(): """ - Returns show_notifications_tray as boolean for the courses in which user is enrolled + Returns whether notifications tray is enabled via waffle flag """ - show_notifications_tray = False - learner_enrollments_course_ids = CourseEnrollment.objects.filter( - user=user, - is_active=True - ).values_list('course_id', flat=True) - - for course_id in learner_enrollments_course_ids: - if ENABLE_NOTIFICATIONS.is_enabled(course_id): - show_notifications_tray = True - break - - return show_notifications_tray + return ENABLE_NOTIFICATIONS.is_enabled() def get_list_in_batches(input_list, batch_size): diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index 041f68f956..3276c9b9db 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -131,7 +131,7 @@ class NotificationCountView(APIView): .annotate(count=Count('*')) ) count_total = 0 - show_notifications_tray = get_show_notifications_tray(self.request.user) + show_notifications_tray = get_show_notifications_tray() count_by_app_name_dict = { app_name: 0 for app_name in COURSE_NOTIFICATION_APPS @@ -330,7 +330,7 @@ class NotificationPreferencesView(APIView): return Response({ 'status': 'success', 'message': 'Notification preferences retrieved successfully.', - 'show_preferences': get_show_notifications_tray(self.request.user), + 'show_preferences': get_show_notifications_tray(), 'data': structured_preferences }, status=status.HTTP_200_OK) @@ -438,7 +438,7 @@ class NotificationPreferencesView(APIView): return { 'status': 'success', 'message': 'Notification preferences update completed', - 'show_preferences': get_show_notifications_tray(self.request.user), + 'show_preferences': get_show_notifications_tray(), 'data': { 'updated_value': updated_value, 'notification_type': validated_data['notification_type'],