From 694bf77993bf0d209301d907be0ece89bb71282a Mon Sep 17 00:00:00 2001 From: Hassan Raza Date: Tue, 15 Apr 2025 12:21:39 +0500 Subject: [PATCH] fix: Remove code for group new comment notification (#36501) --- .../notifications/base_notification.py | 2 - .../notifications/grouping_notifications.py | 21 ------ .../notifications/notification_content.py | 11 +--- .../tests/test_notification_grouping.py | 64 +------------------ .../notifications/tests/test_tasks.py | 16 +++-- 5 files changed, 13 insertions(+), 101 deletions(-) diff --git a/openedx/core/djangoapps/notifications/base_notification.py b/openedx/core/djangoapps/notifications/base_notification.py index 899046e440..507a5996b7 100644 --- a/openedx/core/djangoapps/notifications/base_notification.py +++ b/openedx/core/djangoapps/notifications/base_notification.py @@ -31,8 +31,6 @@ COURSE_NOTIFICATION_TYPES = { 'is_core': True, 'content_template': _('<{p}><{strong}>{replier_name} commented on <{strong}>{author_name}' ' response to your post <{strong}>{post_title}'), - 'grouped_content_template': _('<{p}><{strong}>{replier_name} commented on <{strong}>{author_name}' - ' response to your post <{strong}>{post_title}'), 'content_context': { 'post_title': 'Post title', 'author_name': 'author name', diff --git a/openedx/core/djangoapps/notifications/grouping_notifications.py b/openedx/core/djangoapps/notifications/grouping_notifications.py index eeb4400462..5aa22d64ef 100644 --- a/openedx/core/djangoapps/notifications/grouping_notifications.py +++ b/openedx/core/djangoapps/notifications/grouping_notifications.py @@ -69,27 +69,6 @@ class NotificationRegistry: return grouper_class() -@NotificationRegistry.register('new_comment') -class NewCommentGrouper(BaseNotificationGrouper): - """ - Groups new comment notifications based on the replier name. - """ - - def group(self, new_notification, old_notification): - """ - Groups new comment notifications based on the replier name. - """ - context = old_notification.content_context.copy() - if not context.get('grouped'): - context['replier_name_list'] = [context['replier_name']] - context['grouped_count'] = 1 - 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): """ diff --git a/openedx/core/djangoapps/notifications/notification_content.py b/openedx/core/djangoapps/notifications/notification_content.py index c2103cb835..aa1e87d002 100644 --- a/openedx/core/djangoapps/notifications/notification_content.py +++ b/openedx/core/djangoapps/notifications/notification_content.py @@ -31,16 +31,7 @@ def get_notification_context_with_author_pronoun(context: Dict) -> Dict: # Returns notification content for the new_comment notification. -def get_new_comment_notification_context(context): - """ - Returns the context for the new_comment notification - """ - if not context.get('grouped'): - return get_notification_context_with_author_pronoun(context) - num_repliers = context['grouped_count'] - repliers_string = f"{num_repliers - 1} other{'s' if num_repliers > 2 else ''}" - context['replier_name'] = f"{context['replier_name_list'][0]} and {repliers_string}" - return context +get_new_comment_notification_context = get_notification_context_with_author_pronoun # Returns notification content for the comment_on_followed_post notification. diff --git a/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py b/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py index 23419e46cb..dbb1afa0c4 100644 --- a/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py +++ b/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py @@ -12,7 +12,6 @@ from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.notifications.grouping_notifications import ( BaseNotificationGrouper, NotificationRegistry, - NewCommentGrouper, group_user_notifications, get_user_existing_notifications, NewPostGrouper ) @@ -51,65 +50,6 @@ class TestNotificationRegistry(unittest.TestCase): self.assertIsNone(grouper) -class TestNewCommentGrouper(unittest.TestCase): - """ - Tests for the NewCommentGrouper class - """ - - def setUp(self): - """ - Set up the test - """ - self.new_notification = MagicMock(spec=Notification) - self.old_notification = MagicMock(spec=Notification) - self.old_notification.content_context = { - 'replier_name': 'User1' - } - - def test_group_creates_grouping_keys(self): - """ - Test that the function creates the grouping keys - """ - updated_context = NewCommentGrouper().group(self.new_notification, self.old_notification) - - self.assertIn('replier_name_list', updated_context) - self.assertIn('grouped_count', updated_context) - self.assertEqual(updated_context['grouped_count'], 2) - self.assertTrue(updated_context['grouped']) - - def test_group_appends_to_existing_grouping(self): - """ - Test that the function appends to the existing grouping - """ - # Mock a pre-grouped notification - self.old_notification.content_context = { - 'replier_name': 'User1', - 'replier_name_list': ['User1', 'User2'], - 'grouped': True, - 'grouped_count': 2 - } - self.new_notification.content_context = {'replier_name': 'User3'} - - updated_context = NewCommentGrouper().group(self.new_notification, self.old_notification) - - self.assertIn('replier_name_list', updated_context) - self.assertEqual(len(updated_context['replier_name_list']), 3) - self.assertEqual(updated_context['grouped_count'], 3) - - def test_group_email_content(self): - """ - Tests email_content in content_context when grouping notification - """ - self.old_notification.content_context['email_content'] = 'old content' - self.new_notification.content_context = { - 'email_content': 'new content', - 'replier_name': 'user_2', - } - content_context = NewCommentGrouper().group(self.new_notification, self.old_notification) - self.assertIn('email_content', content_context) - self.assertEqual(content_context['email_content'], 'new content') - - class TestNewPostGrouper(unittest.TestCase): """ Tests for the NewPostGrouper class @@ -163,7 +103,7 @@ class TestGroupUserNotifications(ModuleStoreTestCase): Test that the function groups notifications using the appropriate grou """ # Mock the grouper - mock_grouper = MagicMock(spec=NewCommentGrouper) + mock_grouper = MagicMock(spec=NewPostGrouper) mock_get_grouper.return_value = mock_grouper new_notification = MagicMock(spec=Notification) @@ -241,7 +181,7 @@ class TestGetUserExistingNotifications(unittest.TestCase): mock_filter.return_value = [mock_notification1, mock_notification2] user_ids = [1, 2] - notification_type = 'new_comment' + notification_type = 'new_discussion_post' group_by_id = 'group_id_1' course_id = 'course_1' diff --git a/openedx/core/djangoapps/notifications/tests/test_tasks.py b/openedx/core/djangoapps/notifications/tests/test_tasks.py index 6dca9fd1f9..14608a2135 100644 --- a/openedx/core/djangoapps/notifications/tests/test_tasks.py +++ b/openedx/core/djangoapps/notifications/tests/test_tasks.py @@ -196,19 +196,23 @@ class SendNotificationsTest(ModuleStoreTestCase): """ Test send_notifications with grouping enabled. """ + ( + self.preference_v1.notification_preference_config['discussion'] + ['notification_types']['new_discussion_post']['web'] + ) = True + self.preference_v1.save() with patch('openedx.core.djangoapps.notifications.tasks.group_user_notifications') as user_notifications_mock: context = { - 'post_title': 'Post title', - 'author_name': 'author name', - 'replier_name': 'replier name', - 'group_by_id': 'group_by_id', + 'post_title': 'Test Post', + 'username': 'Test Author', + 'group_by_id': 'group_by_id' } content_url = 'https://example.com/' send_notifications( [self.user.id], str(self.course_1.id), 'discussion', - 'new_comment', + 'new_discussion_post', {**context}, content_url ) @@ -216,7 +220,7 @@ class SendNotificationsTest(ModuleStoreTestCase): [self.user.id], str(self.course_1.id), 'discussion', - 'new_comment', + 'new_discussion_post', {**context}, content_url )