From c8e6a760f969fdb26d5f7d98a04569feec52f0ac Mon Sep 17 00:00:00 2001 From: Ahtisham Shahid Date: Tue, 2 Dec 2025 15:21:36 +0500 Subject: [PATCH] feat: add grouping for new response notification (#37674) --- .../rest_api/discussions_notifications.py | 2 ++ .../rest_api/tests/test_tasks_v2.py | 3 ++ .../notifications/base_notification.py | 4 +++ .../notifications/grouping_notifications.py | 32 +++++++++++++++++++ .../tests/test_notification_grouping.py | 9 +++--- 5 files changed, 46 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/discussions_notifications.py b/lms/djangoapps/discussion/rest_api/discussions_notifications.py index 8efb2e8acb..428f3d5457 100644 --- a/lms/djangoapps/discussion/rest_api/discussions_notifications.py +++ b/lms/djangoapps/discussion/rest_api/discussions_notifications.py @@ -122,6 +122,7 @@ class DiscussionNotificationSender: if not self.parent_id and self.creator.id != int(self.thread.user_id): context = { 'email_content': clean_thread_html_body(self.comment.body), + 'group_by_id': str(self.thread.id), } self._populate_context_with_ids_for_mobile(context, notification_type) self._send_notification([self.thread.user_id], notification_type, extra_context=context) @@ -229,6 +230,7 @@ class DiscussionNotificationSender: if not self.parent_id: context = { "email_content": clean_thread_html_body(self.comment.body), + "group_by_id": str(self.thread.id), } notification_type = "response_on_followed_post" self._populate_context_with_ids_for_mobile(context, notification_type) diff --git a/lms/djangoapps/discussion/rest_api/tests/test_tasks_v2.py b/lms/djangoapps/discussion/rest_api/tests/test_tasks_v2.py index 09339558c4..97ab3f68ba 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_tasks_v2.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_tasks_v2.py @@ -148,6 +148,7 @@ class TestSendResponseNotifications(DiscussionAPIViewTestMixin, ModuleStoreTestC 'topic_id': None, 'thread_id': 1, 'comment_id': None, + 'group_by_id': '1', } self.assertDictEqual(args.context, expected_context) self.assertEqual( @@ -214,6 +215,8 @@ class TestSendResponseNotifications(DiscussionAPIViewTestMixin, ModuleStoreTestC 'thread_id': 1, 'comment_id': 4 if not notification_type == 'response_on_followed_post' else None, } + if notification_type == 'response_on_followed_post': + expected_context['group_by_id'] = '1' if parent_id: expected_context['author_name'] = 'dummy\'s' expected_context['author_pronoun'] = 'dummy\'s' diff --git a/openedx/core/djangoapps/notifications/base_notification.py b/openedx/core/djangoapps/notifications/base_notification.py index 1fec0d5930..95690fd4b1 100644 --- a/openedx/core/djangoapps/notifications/base_notification.py +++ b/openedx/core/djangoapps/notifications/base_notification.py @@ -95,6 +95,8 @@ COURSE_NOTIFICATION_TYPES = { 'is_core': True, 'content_template': _('<{p}><{strong}>{replier_name} responded to your ' 'post <{strong}>{post_title}'), + 'grouped_content_template': _('<{p}><{strong}>{replier_name} and others have responded to your post ' + '<{strong}>{post_title}'), 'content_context': { 'post_title': 'Post title', 'replier_name': 'replier name', @@ -148,6 +150,8 @@ COURSE_NOTIFICATION_TYPES = { 'non_editable': [], 'content_template': _('<{p}><{strong}>{replier_name} responded to a post you’re following: ' '<{strong}>{post_title}'), + 'grouped_content_template': _('<{p}><{strong}>{replier_name} and others responded to a post you’re ' + 'following: <{strong}>{post_title}'), 'content_context': { 'post_title': 'Post title', 'replier_name': 'replier name', diff --git a/openedx/core/djangoapps/notifications/grouping_notifications.py b/openedx/core/djangoapps/notifications/grouping_notifications.py index c855ca3d23..19abed761c 100644 --- a/openedx/core/djangoapps/notifications/grouping_notifications.py +++ b/openedx/core/djangoapps/notifications/grouping_notifications.py @@ -106,6 +106,38 @@ class OraStaffGrouper(BaseNotificationGrouper): return content_context +@NotificationRegistry.register('new_response') +class NewResponseGrouper(BaseNotificationGrouper): + """ + Grouper for new response on post. + """ + + def group(self, new_notification, old_notification): + """ + Groups new ora staff notifications based on the xblock ID. + """ + content_context = old_notification.content_context + content_context.setdefault("grouped", True) + content_context["replier_name"] = new_notification.content_context["replier_name"] + return content_context + + +@NotificationRegistry.register('response_on_followed_post') +class NewResponseOnFollowedPostGrouper(BaseNotificationGrouper): + """ + Grouper for new response on post. + """ + + def group(self, new_notification, old_notification): + """ + Groups new ora staff notifications based on the xblock ID. + """ + content_context = old_notification.content_context + content_context.setdefault("grouped", True) + content_context["replier_name"] = new_notification.content_context["replier_name"] + return content_context + + def group_user_notifications(new_notification: Notification, old_notification: Notification): """ Groups user notification based on notification type and group_id diff --git a/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py b/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py index fd28bb999e..7c0def059d 100644 --- a/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py +++ b/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py @@ -13,7 +13,7 @@ from openedx.core.djangoapps.notifications.grouping_notifications import ( BaseNotificationGrouper, NotificationRegistry, group_user_notifications, - get_user_existing_notifications, NewPostGrouper + get_user_existing_notifications, NewPostGrouper, NewResponseGrouper, NewResponseOnFollowedPostGrouper ) from openedx.core.djangoapps.notifications.models import Notification from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -98,12 +98,13 @@ class TestGroupUserNotifications(ModuleStoreTestCase): """ @patch('openedx.core.djangoapps.notifications.grouping_notifications.NotificationRegistry.get_grouper') - def test_group_user_notifications(self, mock_get_grouper): + @ddt.data(NewPostGrouper, NewResponseGrouper, NewResponseOnFollowedPostGrouper) + def test_group_user_notifications(self, grouper_class, mock_get_grouper): """ - Test that the function groups notifications using the appropriate grou + Test that the function groups notifications using the appropriate grouping class """ # Mock the grouper - mock_grouper = MagicMock(spec=NewPostGrouper) + mock_grouper = MagicMock(spec=grouper_class) mock_get_grouper.return_value = mock_grouper new_notification = MagicMock(spec=Notification)