From 3af1ce0441c18417b5f17cc2bef8071379a7a287 Mon Sep 17 00:00:00 2001 From: Ahtisham Shahid Date: Wed, 19 Jul 2023 15:26:03 +0500 Subject: [PATCH] allow app level enable/disable in notifications app (#32781) * fix: allow app level enable/disable in notifications app --- .../discussion/rest_api/tests/test_utils.py | 6 ++--- .../discussion/rest_api/tests/utils.py | 2 +- lms/djangoapps/discussion/rest_api/utils.py | 2 +- .../core/djangoapps/notifications/tasks.py | 6 ++++- .../notifications/tests/test_tasks.py | 26 +++++++++++++++++++ 5 files changed, 36 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/tests/test_utils.py b/lms/djangoapps/discussion/rest_api/tests/test_utils.py index 5f5ac7bf97..d5d5275ea5 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_utils.py @@ -188,7 +188,7 @@ class TestSendResponseNotifications(ForumsEnableMixin, CommentsServiceMockMixin, send_response_notifications(self.thread, self.course, self.user_2, parent_id=None) self.assertEqual(handler.call_count, 1) args = handler.call_args[1]['notification_data'] - self.assertEqual(args.user_ids, [self.user_1.id]) + self.assertEqual([int(user_id) for user_id in args.user_ids], [self.user_1.id]) self.assertEqual(args.notification_type, 'new_response') expected_context = { 'replier_name': self.user_2.username, @@ -224,7 +224,7 @@ class TestSendResponseNotifications(ForumsEnableMixin, CommentsServiceMockMixin, # check if the notification is sent to the thread creator args_comment = handler.call_args_list[0][1]['notification_data'] args_comment_on_response = handler.call_args_list[1][1]['notification_data'] - self.assertEqual(args_comment.user_ids, [self.user_1.id]) + self.assertEqual([int(user_id) for user_id in args_comment.user_ids], [self.user_1.id]) self.assertEqual(args_comment.notification_type, 'new_comment') expected_context = { 'replier_name': self.user_3.username, @@ -240,7 +240,7 @@ class TestSendResponseNotifications(ForumsEnableMixin, CommentsServiceMockMixin, self.assertEqual(args_comment.app_name, 'discussion') # check if the notification is sent to the parent response creator - self.assertEqual(args_comment_on_response.user_ids, [self.user_2.id]) + self.assertEqual([int(user_id) for user_id in args_comment_on_response.user_ids], [self.user_2.id]) self.assertEqual(args_comment_on_response.notification_type, 'new_comment_on_response') expected_context = { 'replier_name': self.user_3.username, diff --git a/lms/djangoapps/discussion/rest_api/tests/utils.py b/lms/djangoapps/discussion/rest_api/tests/utils.py index ee90087668..5db131bec8 100644 --- a/lms/djangoapps/discussion/rest_api/tests/utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/utils.py @@ -665,7 +665,7 @@ class ThreadMock(object): def __init__(self, thread_id, creator, title, parent_id=None): self.id = thread_id - self.user_id = creator.id + self.user_id = str(creator.id) self.username = creator.username self.title = title self.parent_id = parent_id diff --git a/lms/djangoapps/discussion/rest_api/utils.py b/lms/djangoapps/discussion/rest_api/utils.py index 66d4a8c974..efb6c57dd9 100644 --- a/lms/djangoapps/discussion/rest_api/utils.py +++ b/lms/djangoapps/discussion/rest_api/utils.py @@ -419,7 +419,7 @@ class DiscussionNotificationSender: Send notification to users who are subscribed to the main thread/post i.e. there is a response to the main thread. """ - if not self.parent_id and self.creator.id != self.thread.user_id: + if not self.parent_id and self.creator.id != int(self.thread.user_id): self._send_notification([self.thread.user_id], "new_response") def send_new_comment_notification(self): diff --git a/openedx/core/djangoapps/notifications/tasks.py b/openedx/core/djangoapps/notifications/tasks.py index f7611f0998..ca5bdabbdd 100644 --- a/openedx/core/djangoapps/notifications/tasks.py +++ b/openedx/core/djangoapps/notifications/tasks.py @@ -100,7 +100,11 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c notifications = [] for preference in preferences: preference = update_user_preference(preference, preference.user, course_key) - if preference and preference.get_web_config(app_name, notification_type): + if ( + preference and + preference.get_web_config(app_name, notification_type) and + preference.get_app_config(app_name).get('enabled', False) + ): notification = Notification( user_id=preference.user_id, app_name=app_name, diff --git a/openedx/core/djangoapps/notifications/tests/test_tasks.py b/openedx/core/djangoapps/notifications/tests/test_tasks.py index e4c3a1eaaa..51cdc741d6 100644 --- a/openedx/core/djangoapps/notifications/tests/test_tasks.py +++ b/openedx/core/djangoapps/notifications/tests/test_tasks.py @@ -133,3 +133,29 @@ class SendNotificationsTest(ModuleStoreTestCase): self.assertEqual(notification.content_context, context) self.assertEqual(notification.content_url, content_url) self.assertEqual(notification.course_id, self.course_1.id) + + @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True) + @ddt.data( + ('discussion', 'new_comment_on_response'), # core notification + ('discussion', 'new_response'), # non core notification + ) + @ddt.unpack + def test_send_with_app_disabled_notifications(self, app_name, notification_type): + """ + Test send_notifications does not create a new notification if the app is disabled. + """ + self.preference_v1.notification_preference_config['discussion']['enabled'] = False + self.preference_v1.save() + + context = { + 'post_title': 'Post title', + 'replier_name': 'replier name', + } + content_url = 'https://example.com/' + + # Call the `send_notifications` function. + send_notifications([self.user.id], str(self.course_1.id), app_name, notification_type, context, content_url) + + # Assert that `Notification` objects are not created for the users. + notification = Notification.objects.filter(user_id=self.user.id).first() + self.assertIsNone(notification)