From b17420369df6799b3051bc119250ef3e8266a6cf Mon Sep 17 00:00:00 2001 From: Jawad Khan Date: Wed, 11 Dec 2024 15:48:34 +0500 Subject: [PATCH 1/4] feat: Populate notification context with post, comment and response ids --- .../discussion/rest_api/discussions_notifications.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lms/djangoapps/discussion/rest_api/discussions_notifications.py b/lms/djangoapps/discussion/rest_api/discussions_notifications.py index 88c7fea558c1..e3ed5e8f6987 100644 --- a/lms/djangoapps/discussion/rest_api/discussions_notifications.py +++ b/lms/djangoapps/discussion/rest_api/discussions_notifications.py @@ -66,6 +66,9 @@ def _send_notification(self, user_ids, notification_type, extra_context=None): "post_title": self.thread.title, "course_name": self.course.display_name, "sender_id": self.creator.id, + 'thread_id': self.thread.id, + 'parent_id': self.parent_id, + 'comment_id': self.comment_id **extra_context, }, notification_type=notification_type, From 1391010e4bc688f7fdc4600e14da5472f240dee8 Mon Sep 17 00:00:00 2001 From: Jawad Khan Date: Tue, 17 Dec 2024 06:09:20 +0500 Subject: [PATCH 2/4] fix: Added topic id in notification context --- .../discussion/rest_api/discussions_notifications.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/discussion/rest_api/discussions_notifications.py b/lms/djangoapps/discussion/rest_api/discussions_notifications.py index e3ed5e8f6987..556600e95899 100644 --- a/lms/djangoapps/discussion/rest_api/discussions_notifications.py +++ b/lms/djangoapps/discussion/rest_api/discussions_notifications.py @@ -68,7 +68,8 @@ def _send_notification(self, user_ids, notification_type, extra_context=None): "sender_id": self.creator.id, 'thread_id': self.thread.id, 'parent_id': self.parent_id, - 'comment_id': self.comment_id + 'comment_id': self.comment_id, + 'topic_id': self.thread.commentable_id, **extra_context, }, notification_type=notification_type, From 2405a829e55ebb772d6a2e8d7f532c2795f594b6 Mon Sep 17 00:00:00 2001 From: Jawad Khan Date: Wed, 18 Dec 2024 18:31:00 +0500 Subject: [PATCH 3/4] fix: fixed failing test cases --- .../rest_api/discussions_notifications.py | 37 ++++++++++----- .../discussion/rest_api/tests/test_tasks.py | 45 ++++++++++++++++--- .../discussion/rest_api/tests/utils.py | 3 +- 3 files changed, 66 insertions(+), 19 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/discussions_notifications.py b/lms/djangoapps/discussion/rest_api/discussions_notifications.py index 556600e95899..d20cbe39d55b 100644 --- a/lms/djangoapps/discussion/rest_api/discussions_notifications.py +++ b/lms/djangoapps/discussion/rest_api/discussions_notifications.py @@ -66,10 +66,6 @@ def _send_notification(self, user_ids, notification_type, extra_context=None): "post_title": self.thread.title, "course_name": self.course.display_name, "sender_id": self.creator.id, - 'thread_id': self.thread.id, - 'parent_id': self.parent_id, - 'comment_id': self.comment_id, - 'topic_id': self.thread.commentable_id, **extra_context, }, notification_type=notification_type, @@ -121,6 +117,7 @@ def send_new_response_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], "new_response", extra_context=context) def _response_and_thread_has_same_creator(self) -> bool: @@ -160,6 +157,7 @@ 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) def send_new_comment_on_response_notification(self): @@ -175,6 +173,7 @@ 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._send_notification( [self.parent_response.user_id], "new_comment_on_response", @@ -220,12 +219,15 @@ def send_response_on_followed_post_notification(self): # Remove duplicate users from the list of users to send notification users = list(set(users)) if not self.parent_id: + context = { + "email_content": clean_thread_html_body(self.comment.body), + } + self._populate_context_with_ids_for_mobile(context) self._send_notification( users, "response_on_followed_post", - extra_context={ - "email_content": clean_thread_html_body(self.comment.body), - }) + extra_context=context + ) else: author_name = f"{self.parent_response.username}'s" # use 'their' if comment author is also response author. @@ -235,14 +237,16 @@ def send_response_on_followed_post_notification(self): if self._response_and_comment_has_same_creator() else f"{self.parent_response.username}'s" ) + context = { + "author_name": str(author_name), + "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( users, "comment_on_followed_post", - extra_context={ - "author_name": str(author_name), - "author_pronoun": str(author_pronoun), - "email_content": clean_thread_html_body(self.comment.body), - } + extra_context=context ) def _create_cohort_course_audience(self): @@ -294,6 +298,7 @@ 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) def send_response_endorsed_notification(self): @@ -303,6 +308,7 @@ 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) def send_new_thread_created_notification(self): @@ -334,6 +340,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._send_course_wide_notification(notification_type, audience_filters, context) def send_reported_content_notification(self): @@ -366,6 +373,12 @@ 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, additional_context=False): + 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 + 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 ddfc120a8e4b..62c347fe647a 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_tasks.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_tasks.py @@ -270,6 +270,8 @@ def setUp(self): "username": thread.username, "thread_type": 'discussion', "title": thread.title, + "commentable_id": thread.commentable_id, + }) self._register_subscriptions_endpoint() @@ -319,7 +321,11 @@ def test_send_notification_to_thread_creator(self): 'post_title': 'test thread', 'email_content': self.comment.body, 'course_name': self.course.display_name, - 'sender_id': self.user_2.id + 'sender_id': self.user_2.id, + 'parent_id': None, + 'topic_id': None, + 'thread_id': 1, + 'comment_id': 4, } self.assertDictEqual(args.context, expected_context) self.assertEqual( @@ -365,7 +371,11 @@ def test_send_notification_to_parent_threads(self): 'author_name': 'dummy\'s', 'author_pronoun': 'dummy\'s', 'course_name': self.course.display_name, - 'sender_id': self.user_3.id + 'sender_id': self.user_3.id, + 'parent_id': 2, + 'topic_id': None, + 'thread_id': 1, + 'comment_id': 4, } self.assertDictEqual(args_comment.context, expected_context) self.assertEqual( @@ -382,7 +392,11 @@ def test_send_notification_to_parent_threads(self): 'post_title': self.thread.title, 'email_content': self.comment.body, 'course_name': self.course.display_name, - 'sender_id': self.user_3.id + 'sender_id': self.user_3.id, + 'parent_id': 2, + 'topic_id': None, + 'thread_id': 1, + 'comment_id': 4, } self.assertDictEqual(args_comment_on_response.context, expected_context) self.assertEqual( @@ -442,7 +456,11 @@ def test_comment_creators_own_response(self): 'author_pronoun': 'your', 'course_name': self.course.display_name, 'sender_id': self.user_3.id, - 'email_content': self.comment.body + 'email_content': self.comment.body, + 'parent_id': 2, + 'topic_id': None, + 'thread_id': 1, + 'comment_id': 4, } self.assertDictEqual(args_comment.context, expected_context) self.assertEqual( @@ -487,6 +505,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, + 'topic_id': None, + 'thread_id': 1, + 'comment_id': 4, } if parent_id: expected_context['author_name'] = 'dummy\'s' @@ -571,6 +593,8 @@ def test_new_comment_notification(self): "username": thread.username, "thread_type": 'discussion', "title": thread.title, + "commentable_id": thread.commentable_id, + }) self.register_get_comment_response({ 'id': response.id, @@ -636,6 +660,7 @@ def test_response_endorsed_notifications(self): "username": thread.username, "thread_type": 'discussion', "title": thread.title, + "commentable_id": thread.commentable_id, }) self.register_get_comment_response({ 'id': 1, @@ -663,7 +688,11 @@ def test_response_endorsed_notifications(self): 'post_title': 'test thread', 'course_name': self.course.display_name, 'sender_id': int(self.user_2.id), - 'email_content': 'dummy' + 'email_content': 'dummy', + 'parent_id': None, + 'topic_id': None, + 'thread_id': 1, + 'comment_id': 2, } self.assertDictEqual(notification_data.context, expected_context) self.assertEqual(notification_data.content_url, _get_mfe_url(self.course.id, thread.id)) @@ -681,7 +710,11 @@ def test_response_endorsed_notifications(self): 'post_title': 'test thread', 'course_name': self.course.display_name, 'sender_id': int(response.user_id), - 'email_content': 'dummy' + 'email_content': 'dummy', + 'parent_id': None, + 'topic_id': None, + 'thread_id': 1, + 'comment_id': 2, } self.assertDictEqual(notification_data.context, expected_context) self.assertEqual(notification_data.content_url, _get_mfe_url(self.course.id, thread.id)) diff --git a/lms/djangoapps/discussion/rest_api/tests/utils.py b/lms/djangoapps/discussion/rest_api/tests/utils.py index 27e34705f5df..496f8723acfb 100644 --- a/lms/djangoapps/discussion/rest_api/tests/utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/utils.py @@ -675,13 +675,14 @@ class ThreadMock(object): A mock thread object """ - def __init__(self, thread_id, creator, title, parent_id=None, body=''): + def __init__(self, thread_id, creator, title, parent_id=None, body='', commentable_id=None): self.id = thread_id self.user_id = str(creator.id) self.username = creator.username self.title = title self.parent_id = parent_id self.body = body + self.commentable_id = commentable_id def url_with_id(self, params): return f"http://example.com/{params['id']}" From 4d1ce47ed4c613c7a1a1f707c13333499b6761bb Mon Sep 17 00:00:00 2001 From: Jawad Khan Date: Thu, 19 Dec 2024 15:23:51 +0500 Subject: [PATCH 4/4] fix: removed extra param --- lms/djangoapps/discussion/rest_api/discussions_notifications.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/discussion/rest_api/discussions_notifications.py b/lms/djangoapps/discussion/rest_api/discussions_notifications.py index d20cbe39d55b..30d7fc6cb07a 100644 --- a/lms/djangoapps/discussion/rest_api/discussions_notifications.py +++ b/lms/djangoapps/discussion/rest_api/discussions_notifications.py @@ -373,7 +373,7 @@ 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, additional_context=False): + def _populate_context_with_ids_for_mobile(self, context): context['thread_id'] = self.thread.id context['topic_id'] = self.thread.commentable_id context['comment_id'] = self.comment_id