Skip to content

Commit 4d23866

Browse files
feat: added grouping for new post notification (#35761)
1 parent d197077 commit 4d23866

File tree

4 files changed

+69
-3
lines changed

4 files changed

+69
-3
lines changed

lms/djangoapps/discussion/rest_api/discussions_notifications.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ def _send_course_wide_notification(self, notification_type, audience_filters=Non
8989
"post_title": getattr(self.thread, 'title', ''),
9090
"course_name": self.course.display_name,
9191
"sender_id": self.creator.id,
92+
"group_by_id": str(self.course.id),
9293
**extra_context,
9394
},
9495
notification_type=notification_type,

openedx/core/djangoapps/notifications/base_notification.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@
6565
'push': False,
6666
'non_editable': [],
6767
'content_template': _('<{p}><{strong}>{username}</{strong}> posted <{strong}>{post_title}</{strong}></{p}>'),
68+
'grouped_content_template': _('<{p}><{strong}>{replier_name}</{strong}> and others started new discussions'
69+
'</{p}>'),
6870
'content_context': {
6971
'post_title': 'Post title',
7072
'username': 'Post author name',

openedx/core/djangoapps/notifications/grouping_notifications.py

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,35 @@ def group(self, new_notification, old_notification):
7777
if not context.get('grouped'):
7878
context['replier_name_list'] = [context['replier_name']]
7979
context['grouped_count'] = 1
80-
context['grouped'] = True
80+
context['grouped'] = True
8181
context['replier_name_list'].append(new_notification.content_context['replier_name'])
8282
context['grouped_count'] += 1
8383
context['email_content'] = new_notification.content_context.get('email_content', '')
8484
return context
8585

8686

87+
@NotificationRegistry.register('new_discussion_post')
88+
class NewPostGrouper(BaseNotificationGrouper):
89+
"""
90+
Groups new post notifications based on the author name.
91+
"""
92+
93+
def group(self, new_notification, old_notification):
94+
"""
95+
Groups new post notifications based on the author name.
96+
"""
97+
if (
98+
old_notification.content_context['username'] == new_notification.content_context['username']
99+
and not old_notification.content_context.get('grouped', False)
100+
):
101+
return {**new_notification.content_context}
102+
return {
103+
**old_notification.content_context,
104+
"grouped": True,
105+
"replier_name": new_notification.content_context["replier_name"]
106+
}
107+
108+
87109
def group_user_notifications(new_notification: Notification, old_notification: Notification):
88110
"""
89111
Groups user notification based on notification type and group_id
@@ -93,9 +115,9 @@ def group_user_notifications(new_notification: Notification, old_notification: N
93115

94116
if grouper_class:
95117
old_notification.content_context = grouper_class.group(new_notification, old_notification)
96-
old_notification.content_context['grouped'] = True
97118
old_notification.web = old_notification.web or new_notification.web
98119
old_notification.email = old_notification.email or new_notification.email
120+
old_notification.content_url = new_notification.content_url
99121
old_notification.last_read = None
100122
old_notification.last_seen = None
101123
old_notification.created = utc.localize(datetime.datetime.now())

openedx/core/djangoapps/notifications/tests/test_notification_grouping.py

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
NotificationRegistry,
1313
NewCommentGrouper,
1414
group_user_notifications,
15-
get_user_existing_notifications
15+
get_user_existing_notifications, NewPostGrouper
1616
)
1717
from openedx.core.djangoapps.notifications.models import Notification
1818

@@ -102,6 +102,47 @@ def test_group_email_content(self):
102102
self.assertEqual(content_context['email_content'], 'new content')
103103

104104

105+
class TestNewPostGrouper(unittest.TestCase):
106+
"""
107+
Tests for the NewPostGrouper class
108+
"""
109+
110+
def test_group(self):
111+
"""
112+
Test that the function groups new post notifications based on the author name
113+
"""
114+
new_notification = MagicMock(spec=Notification)
115+
old_notification = MagicMock(spec=Notification)
116+
old_notification.content_context = {
117+
'author_name': 'User1',
118+
'username': 'User1'
119+
}
120+
121+
updated_context = NewPostGrouper().group(new_notification, old_notification)
122+
123+
self.assertTrue(updated_context['grouped'])
124+
self.assertEqual(updated_context['replier_name'], new_notification.content_context['replier_name'])
125+
126+
def test_new_post_with_same_user(self):
127+
"""
128+
Test that the function does not group notifications with same authors if notification is not
129+
already grouped
130+
"""
131+
new_notification = MagicMock(spec=Notification)
132+
old_notification = MagicMock(spec=Notification)
133+
old_notification.content_context = {
134+
'username': 'User1',
135+
'grouped': False
136+
}
137+
new_notification.content_context = {
138+
'username': 'User1',
139+
}
140+
141+
updated_context = NewPostGrouper().group(new_notification, old_notification)
142+
143+
self.assertFalse(updated_context.get('grouped', False))
144+
145+
105146
class TestGroupUserNotifications(unittest.TestCase):
106147
"""
107148
Tests for the group_user_notifications function

0 commit comments

Comments
 (0)