From a84ad87f4bc09992555eee820c732f34249ff2ea Mon Sep 17 00:00:00 2001 From: jawad khan Date: Mon, 3 Mar 2025 10:44:47 +0500 Subject: [PATCH] fix: Adjusted discussion notification context for mobile (#36304) * fix: Adjusted discussion notification context for mobile --- .../rest_api/discussions_notifications.py | 52 ++++++++++++------- .../discussion/rest_api/tests/test_tasks.py | 18 +++---- 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/discussions_notifications.py b/lms/djangoapps/discussion/rest_api/discussions_notifications.py index f6572c829c7a..b1bc6aa95473 100644 --- a/lms/djangoapps/discussion/rest_api/discussions_notifications.py +++ b/lms/djangoapps/discussion/rest_api/discussions_notifications.py @@ -113,12 +113,13 @@ 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. """ + notification_type = "new_response" if not self.parent_id and self.creator.id != int(self.thread.user_id): context = { 'email_content': clean_thread_html_body(self.comment.body), } - self._populate_context_with_ids_for_mobile(context) - self._send_notification([self.thread.user_id], "new_response", extra_context=context) + self._populate_context_with_ids_for_mobile(context, notification_type) + self._send_notification([self.thread.user_id], notification_type, extra_context=context) def _response_and_thread_has_same_creator(self) -> bool: """ @@ -133,6 +134,7 @@ def send_new_comment_notification(self): """ Send notification to parent thread creator i.e. comment on the response. """ + notification_type = "new_comment" if ( self.parent_response and self.creator.id != int(self.thread.user_id) @@ -157,14 +159,15 @@ def send_new_comment_notification(self): "author_pronoun": str(author_pronoun), "email_content": clean_thread_html_body(self.comment.body), } - self._populate_context_with_ids_for_mobile(context) - self._send_notification([self.thread.user_id], "new_comment", extra_context=context) + self._populate_context_with_ids_for_mobile(context, notification_type) + self._send_notification([self.thread.user_id], notification_type, extra_context=context) def send_new_comment_on_response_notification(self): """ Send notification to parent response creator i.e. comment on the response. Do not send notification if author of response is same as author of post. """ + notification_type = "new_comment_on_response" if ( self.parent_response and self.creator.id != int(self.parent_response.user_id) and not @@ -173,10 +176,10 @@ def send_new_comment_on_response_notification(self): context = { "email_content": clean_thread_html_body(self.comment.body), } - self._populate_context_with_ids_for_mobile(context) + self._populate_context_with_ids_for_mobile(context, notification_type) self._send_notification( [self.parent_response.user_id], - "new_comment_on_response", + notification_type, extra_context=context ) @@ -222,10 +225,11 @@ def send_response_on_followed_post_notification(self): context = { "email_content": clean_thread_html_body(self.comment.body), } - self._populate_context_with_ids_for_mobile(context) + notification_type = "response_on_followed_post" + self._populate_context_with_ids_for_mobile(context, notification_type) self._send_notification( users, - "response_on_followed_post", + notification_type, extra_context=context ) else: @@ -242,10 +246,11 @@ def send_response_on_followed_post_notification(self): "author_pronoun": str(author_pronoun), "email_content": clean_thread_html_body(self.comment.body), } - self._populate_context_with_ids_for_mobile(context) + notification_type = "comment_on_followed_post" + self._populate_context_with_ids_for_mobile(context, notification_type) self._send_notification( users, - "comment_on_followed_post", + notification_type, extra_context=context ) @@ -298,8 +303,9 @@ def send_response_endorsed_on_thread_notification(self): context = { "email_content": clean_thread_html_body(self.comment.body) } - self._populate_context_with_ids_for_mobile(context) - self._send_notification([self.thread.user_id], "response_endorsed_on_thread", extra_context=context) + notification_type = "response_endorsed_on_thread" + self._populate_context_with_ids_for_mobile(context, notification_type) + self._send_notification([self.thread.user_id], notification_type, extra_context=context) def send_response_endorsed_notification(self): """ @@ -308,8 +314,9 @@ def send_response_endorsed_notification(self): context = { "email_content": clean_thread_html_body(self.comment.body) } - self._populate_context_with_ids_for_mobile(context) - self._send_notification([self.creator.id], "response_endorsed", extra_context=context) + notification_type = "response_endorsed" + self._populate_context_with_ids_for_mobile(context, notification_type) + self._send_notification([self.creator.id], notification_type, extra_context=context) def send_new_thread_created_notification(self): """ @@ -340,7 +347,7 @@ def send_new_thread_created_notification(self): 'post_title': self.thread.title, "email_content": clean_thread_html_body(self.thread.body), } - self._populate_context_with_ids_for_mobile(context) + self._populate_context_with_ids_for_mobile(context, notification_type) self._send_course_wide_notification(notification_type, audience_filters, context) def send_reported_content_notification(self): @@ -373,11 +380,20 @@ def send_reported_content_notification(self): ]} self._send_course_wide_notification("content_reported", audience_filters, context) - def _populate_context_with_ids_for_mobile(self, context): + def _populate_context_with_ids_for_mobile(self, context, notification_type): + """ + Populate notification context with attributes required by mobile apps. + """ + context['thread_id'] = self.thread.id context['topic_id'] = self.thread.commentable_id - context['comment_id'] = self.comment_id - context['parent_id'] = self.parent_id + + if notification_type in ("response_on_followed_post", 'new_response'): + context['response_id'] = self.comment_id + context['comment_id'] = None + else: + context['response_id'] = self.parent_id + context['comment_id'] = self.comment_id def is_discussion_cohorted(course_key_str): diff --git a/lms/djangoapps/discussion/rest_api/tests/test_tasks.py b/lms/djangoapps/discussion/rest_api/tests/test_tasks.py index 8171ca7e6a71..a6eb4948ce03 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_tasks.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_tasks.py @@ -357,10 +357,10 @@ def test_send_notification_to_thread_creator(self): 'email_content': self.comment.body, 'course_name': self.course.display_name, 'sender_id': self.user_2.id, - 'parent_id': None, + 'response_id': 4, 'topic_id': None, 'thread_id': 1, - 'comment_id': 4, + 'comment_id': None, } self.assertDictEqual(args.context, expected_context) self.assertEqual( @@ -407,7 +407,7 @@ def test_send_notification_to_parent_threads(self): 'author_pronoun': 'dummy\'s', 'course_name': self.course.display_name, 'sender_id': self.user_3.id, - 'parent_id': 2, + 'response_id': 2, 'topic_id': None, 'thread_id': 1, 'comment_id': 4, @@ -428,7 +428,7 @@ def test_send_notification_to_parent_threads(self): 'email_content': self.comment.body, 'course_name': self.course.display_name, 'sender_id': self.user_3.id, - 'parent_id': 2, + 'response_id': 2, 'topic_id': None, 'thread_id': 1, 'comment_id': 4, @@ -492,7 +492,7 @@ def test_comment_creators_own_response(self): 'course_name': self.course.display_name, 'sender_id': self.user_3.id, 'email_content': self.comment.body, - 'parent_id': 2, + 'response_id': 2, 'topic_id': None, 'thread_id': 1, 'comment_id': 4, @@ -540,10 +540,10 @@ def test_send_notification_to_followers(self, parent_id, notification_type): 'email_content': self.comment.body, 'course_name': self.course.display_name, 'sender_id': self.user_2.id, - 'parent_id': parent_id, + 'response_id': 4 if notification_type == 'response_on_followed_post' else parent_id, 'topic_id': None, 'thread_id': 1, - 'comment_id': 4, + 'comment_id': 4 if not notification_type == 'response_on_followed_post' else None, } if parent_id: expected_context['author_name'] = 'dummy\'s' @@ -760,7 +760,7 @@ def test_response_endorsed_notifications(self): 'course_name': self.course.display_name, 'sender_id': int(self.user_2.id), 'email_content': 'dummy', - 'parent_id': None, + 'response_id': None, 'topic_id': None, 'thread_id': 1, 'comment_id': 2, @@ -782,7 +782,7 @@ def test_response_endorsed_notifications(self): 'course_name': self.course.display_name, 'sender_id': int(response.user_id), 'email_content': 'dummy', - 'parent_id': None, + 'response_id': None, 'topic_id': None, 'thread_id': 1, 'comment_id': 2,