From 4d23866ae416e4839f8813e7c813f5f68c950411 Mon Sep 17 00:00:00 2001 From: Ahtisham Shahid Date: Mon, 11 Nov 2024 16:37:32 +0500 Subject: [PATCH] feat: added grouping for new post notification (#35761) --- .../rest_api/discussions_notifications.py | 1 + .../notifications/base_notification.py | 2 + .../notifications/grouping_notifications.py | 26 ++++++++++- .../tests/test_notification_grouping.py | 43 ++++++++++++++++++- 4 files changed, 69 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/discussions_notifications.py b/lms/djangoapps/discussion/rest_api/discussions_notifications.py index b0eb7c89dc..929e9917a5 100644 --- a/lms/djangoapps/discussion/rest_api/discussions_notifications.py +++ b/lms/djangoapps/discussion/rest_api/discussions_notifications.py @@ -89,6 +89,7 @@ class DiscussionNotificationSender: "post_title": getattr(self.thread, 'title', ''), "course_name": self.course.display_name, "sender_id": self.creator.id, + "group_by_id": str(self.course.id), **extra_context, }, notification_type=notification_type, diff --git a/openedx/core/djangoapps/notifications/base_notification.py b/openedx/core/djangoapps/notifications/base_notification.py index 795aecef79..91a1635aec 100644 --- a/openedx/core/djangoapps/notifications/base_notification.py +++ b/openedx/core/djangoapps/notifications/base_notification.py @@ -65,6 +65,8 @@ COURSE_NOTIFICATION_TYPES = { 'push': False, 'non_editable': [], 'content_template': _('<{p}><{strong}>{username} posted <{strong}>{post_title}'), + 'grouped_content_template': _('<{p}><{strong}>{replier_name} and others started new discussions' + ''), 'content_context': { 'post_title': 'Post title', 'username': 'Post author name', diff --git a/openedx/core/djangoapps/notifications/grouping_notifications.py b/openedx/core/djangoapps/notifications/grouping_notifications.py index b24baeb331..3c4688b5ed 100644 --- a/openedx/core/djangoapps/notifications/grouping_notifications.py +++ b/openedx/core/djangoapps/notifications/grouping_notifications.py @@ -77,13 +77,35 @@ class NewCommentGrouper(BaseNotificationGrouper): if not context.get('grouped'): context['replier_name_list'] = [context['replier_name']] context['grouped_count'] = 1 - context['grouped'] = True + context['grouped'] = True context['replier_name_list'].append(new_notification.content_context['replier_name']) context['grouped_count'] += 1 context['email_content'] = new_notification.content_context.get('email_content', '') return context +@NotificationRegistry.register('new_discussion_post') +class NewPostGrouper(BaseNotificationGrouper): + """ + Groups new post notifications based on the author name. + """ + + def group(self, new_notification, old_notification): + """ + Groups new post notifications based on the author name. + """ + if ( + old_notification.content_context['username'] == new_notification.content_context['username'] + and not old_notification.content_context.get('grouped', False) + ): + return {**new_notification.content_context} + return { + **old_notification.content_context, + "grouped": True, + "replier_name": new_notification.content_context["replier_name"] + } + + def group_user_notifications(new_notification: Notification, old_notification: Notification): """ Groups user notification based on notification type and group_id @@ -93,9 +115,9 @@ def group_user_notifications(new_notification: Notification, old_notification: N if grouper_class: old_notification.content_context = grouper_class.group(new_notification, old_notification) - old_notification.content_context['grouped'] = True old_notification.web = old_notification.web or new_notification.web old_notification.email = old_notification.email or new_notification.email + old_notification.content_url = new_notification.content_url old_notification.last_read = None old_notification.last_seen = None old_notification.created = utc.localize(datetime.datetime.now()) diff --git a/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py b/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py index e46cde7e83..fea954a0ea 100644 --- a/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py +++ b/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py @@ -12,7 +12,7 @@ from openedx.core.djangoapps.notifications.grouping_notifications import ( NotificationRegistry, NewCommentGrouper, group_user_notifications, - get_user_existing_notifications + get_user_existing_notifications, NewPostGrouper ) from openedx.core.djangoapps.notifications.models import Notification @@ -102,6 +102,47 @@ class TestNewCommentGrouper(unittest.TestCase): self.assertEqual(content_context['email_content'], 'new content') +class TestNewPostGrouper(unittest.TestCase): + """ + Tests for the NewPostGrouper class + """ + + def test_group(self): + """ + Test that the function groups new post notifications based on the author name + """ + new_notification = MagicMock(spec=Notification) + old_notification = MagicMock(spec=Notification) + old_notification.content_context = { + 'author_name': 'User1', + 'username': 'User1' + } + + updated_context = NewPostGrouper().group(new_notification, old_notification) + + self.assertTrue(updated_context['grouped']) + self.assertEqual(updated_context['replier_name'], new_notification.content_context['replier_name']) + + def test_new_post_with_same_user(self): + """ + Test that the function does not group notifications with same authors if notification is not + already grouped + """ + new_notification = MagicMock(spec=Notification) + old_notification = MagicMock(spec=Notification) + old_notification.content_context = { + 'username': 'User1', + 'grouped': False + } + new_notification.content_context = { + 'username': 'User1', + } + + updated_context = NewPostGrouper().group(new_notification, old_notification) + + self.assertFalse(updated_context.get('grouped', False)) + + class TestGroupUserNotifications(unittest.TestCase): """ Tests for the group_user_notifications function