Skip to content

Commit

Permalink
feat: save author pronoun separately for notification to prevent info…
Browse files Browse the repository at this point in the history
… loss (#35234)

* feat: save author pronoun separately for notification to prevent info loss

* fix: missing pronoun in comment on followed post

* test: updated tests for new comment notifications
  • Loading branch information
eemaanamir authored Aug 12, 2024
1 parent 25d5d08 commit 152b678
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 7 deletions.
18 changes: 16 additions & 2 deletions lms/djangoapps/discussion/rest_api/discussions_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,10 @@ def send_new_comment_notification(self):
self.parent_response and
self.creator.id != int(self.thread.user_id)
):
author_name = f"{self.parent_response.username}'s"
# use your if author of response is same as author of post.
# use 'their' if comment author is also response author.
author_name = (
author_pronoun = (
# Translators: Replier commented on "your" response to your post
_("your")
if self._response_and_thread_has_same_creator()
Expand All @@ -129,10 +130,12 @@ def send_new_comment_notification(self):
_("their")
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),
}
self._send_notification([self.thread.user_id], "new_comment", extra_context=context)

Expand Down Expand Up @@ -189,10 +192,21 @@ def send_response_on_followed_post_notification(self):
if not self.parent_id:
self._send_notification(users, "response_on_followed_post")
else:
author_name = f"{self.parent_response.username}'s"
# use 'their' if comment author is also response author.
author_pronoun = (
# Translators: Replier commented on "their" response in a post you're following
_("their")
if self._response_and_comment_has_same_creator()
else f"{self.parent_response.username}'s"
)
self._send_notification(
users,
"comment_on_followed_post",
extra_context={"author_name": self.parent_response.username}
extra_context={
"author_name": str(author_name),
"author_pronoun": str(author_pronoun),
}
)

def _create_cohort_course_audience(self):
Expand Down
10 changes: 7 additions & 3 deletions lms/djangoapps/discussion/rest_api/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ def test_send_notification_to_parent_threads(self):
'replier_name': self.user_3.username,
'post_title': self.thread.title,
'author_name': 'dummy\'s',
'author_pronoun': 'dummy\'s',
'course_name': self.course.display_name,
'sender_id': self.user_3.id
}
Expand Down Expand Up @@ -399,7 +400,8 @@ def test_comment_creators_own_response(self):
expected_context = {
'replier_name': self.user_3.username,
'post_title': self.thread.title,
'author_name': 'your',
'author_name': 'dummy\'s',
'author_pronoun': 'your',
'course_name': self.course.display_name,
'sender_id': self.user_3.id,
}
Expand Down Expand Up @@ -441,7 +443,8 @@ def test_send_notification_to_followers(self, parent_id, notification_type):
'sender_id': self.user_2.id,
}
if parent_id:
expected_context['author_name'] = 'dummy'
expected_context['author_name'] = 'dummy\'s'
expected_context['author_pronoun'] = 'dummy\'s'
self.assertDictEqual(args.context, expected_context)
self.assertEqual(
args.content_url,
Expand Down Expand Up @@ -531,7 +534,8 @@ def test_new_comment_notification(self):
send_response_notifications(thread.id, str(self.course.id), self.user_2.id, parent_id=response.id)
handler.assert_called_once()
context = handler.call_args[1]['notification_data'].context
self.assertEqual(context['author_name'], 'their')
self.assertEqual(context['author_name'], 'dummy\'s')
self.assertEqual(context['author_pronoun'], 'their')


@override_waffle_flag(ENABLE_NOTIFICATIONS, active=True)
Expand Down
9 changes: 7 additions & 2 deletions openedx/core/djangoapps/notifications/base_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole
from .utils import find_app_in_normalized_apps, find_pref_in_normalized_prefs
from ..django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA
from .notification_content import get_notification_type_content_function

FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE = 'filter_audit_expired_users_with_no_role'

Expand Down Expand Up @@ -109,8 +110,9 @@
'is_core': True,
'info': '',
'non_editable': [],
'content_template': _('<{p}><{strong}>{replier_name}</{strong}> commented on {author_name}\'s response in '
'a post you’re following <{strong}>{post_title}</{strong}></{p}>'),
'content_template': _('<{p}><{strong}>{replier_name}</{strong}> commented on <{strong}>{author_name}'
'</{strong}> response in a post you’re following <{strong}>{post_title}'
'</{strong}></{p}>'),
'content_context': {
'post_title': 'Post title',
'author_name': 'author name',
Expand Down Expand Up @@ -465,10 +467,13 @@ def get_notification_content(notification_type, context):
'strong': 'strong',
'p': 'p',
}
content_function = get_notification_type_content_function(notification_type)
if notification_type == 'course_update':
notification_type = 'course_updates'
notification_type = NotificationTypeManager().notification_types.get(notification_type, None)
if notification_type:
if content_function:
return content_function(notification_type, context)
notification_type_content_template = notification_type.get('content_template', None)
if notification_type_content_template:
return notification_type_content_template.format(**context, **html_tags_context)
Expand Down
35 changes: 35 additions & 0 deletions openedx/core/djangoapps/notifications/notification_content.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
"""
Helper functions for overriding notification content for given notification type.
"""


def get_notification_type_content_function(notification_type):
"""
Returns the content function for the given notification if it exists.
"""
try:
return globals()[f"get_{notification_type}_notification_content"]
except KeyError:
return None


def get_notification_content_with_author_pronoun(notification_type, context):
"""
Helper function to get notification content with author's pronoun.
"""
html_tags_context = {
'strong': 'strong',
'p': 'p',
}
notification_type_content_template = notification_type.get('content_template', None)
if 'author_pronoun' in context:
context['author_name'] = context['author_pronoun']
if notification_type_content_template:
return notification_type_content_template.format(**context, **html_tags_context)
return ''


# Returns notification content for the new_comment notification.
get_new_comment_notification_content = get_notification_content_with_author_pronoun
# Returns notification content for the comment_on_followed_post notification.
get_comment_on_followed_post_notification_content = get_notification_content_with_author_pronoun

0 comments on commit 152b678

Please sign in to comment.