Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Populate notification context with post, comment and response ids #36008

Merged
merged 4 commits into from
Dec 20, 2024
Merged

feat: Populate notification context with post, comment and response ids #36008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b174203
feat: Populate notification context with post, comment and response ids
jawad-khan Dec 11, 2024
1391010
fix: Added topic id in notification context
jawad-khan Dec 17, 2024
2405a82
fix: fixed failing test cases
jawad-khan Dec 18, 2024
4d1ce47
fix: removed extra param
jawad-khan Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions lms/djangoapps/discussion/rest_api/discussions_notifications.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,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:
Expand Down Expand Up @@ -156,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):
Expand All @@ -171,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",
Expand Down Expand Up @@ -216,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.
Expand All @@ -231,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):
Expand Down Expand Up @@ -290,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):
Expand All @@ -299,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):
Expand Down Expand Up @@ -330,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):
Expand Down Expand Up @@ -362,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):
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):
"""
Expand Down
45 changes: 39 additions & 6 deletions lms/djangoapps/discussion/rest_api/tests/test_tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down
3 changes: 2 additions & 1 deletion lms/djangoapps/discussion/rest_api/tests/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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']}"
Loading