Skip to content

Commit

Permalink
allow app level enable/disable in notifications app (#32781)
Browse files Browse the repository at this point in the history
* fix: allow app level enable/disable in notifications app
  • Loading branch information
AhtishamShahid authored Jul 19, 2023
1 parent 4cd7c3b commit 3af1ce0
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 6 deletions.
6 changes: 3 additions & 3 deletions lms/djangoapps/discussion/rest_api/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ def test_send_notification_to_thread_creator(self):
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,
Expand Down Expand Up @@ -224,7 +224,7 @@ def test_send_notification_to_parent_threads(self):
# 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,
Expand All @@ -240,7 +240,7 @@ def test_send_notification_to_parent_threads(self):
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,
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/discussion/rest_api/tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/discussion/rest_api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ def send_new_response_notification(self):
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):
Expand Down
6 changes: 5 additions & 1 deletion openedx/core/djangoapps/notifications/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
26 changes: 26 additions & 0 deletions openedx/core/djangoapps/notifications/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,29 @@ def test_send_notifications(self, app_name, notification_type):
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)

0 comments on commit 3af1ce0

Please sign in to comment.