From b37286e0354272475814ff563c0125ed26fae0fa Mon Sep 17 00:00:00 2001 From: Ahtisham Shahid Date: Fri, 4 Aug 2023 14:12:12 +0500 Subject: [PATCH] fix: Update notification sending logic for discussions (#32879) --- .../discussion/rest_api/tests/test_utils.py | 51 +++++++++++++++++-- lms/djangoapps/discussion/rest_api/utils.py | 25 +++++++-- .../core/djangoapps/notifications/handlers.py | 2 +- 3 files changed, 68 insertions(+), 10 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/tests/test_utils.py b/lms/djangoapps/discussion/rest_api/tests/test_utils.py index d5d5275ea5b..400a35a4291 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_utils.py @@ -160,10 +160,18 @@ def test_remove_empty_sequentials(self): self.assertEqual(result, expected_output) +def _get_mfe_url(course_id, post_id): + """ + get discussions mfe url to specific post. + """ + return f"{settings.DISCUSSIONS_MICROFRONTEND_URL}/{str(course_id)}/posts/{post_id}" + + class TestSendResponseNotifications(ForumsEnableMixin, CommentsServiceMockMixin, ModuleStoreTestCase): """ Test for the send_response_notifications function """ + def setUp(self): super().setUp() httpretty.reset() @@ -174,6 +182,7 @@ def setUp(self): self.user_3 = UserFactory.create() self.thread = ThreadMock(thread_id=1, creator=self.user_1, title='test thread') self.thread_2 = ThreadMock(thread_id=2, creator=self.user_2, title='test thread 2') + self.thread_3 = ThreadMock(thread_id=2, creator=self.user_1, title='test thread 3') self.course = CourseFactory.create() def test_send_notification_to_thread_creator(self): @@ -198,7 +207,7 @@ def test_send_notification_to_thread_creator(self): self.assertDictEqual(args.context, expected_context) self.assertEqual( args.content_url, - self._get_mfe_url(self.course.id, self.thread.id) + _get_mfe_url(self.course.id, self.thread.id) ) self.assertEqual(args.app_name, 'discussion') @@ -235,7 +244,7 @@ def test_send_notification_to_parent_threads(self): self.assertDictEqual(args_comment.context, expected_context) self.assertEqual( args_comment.content_url, - self._get_mfe_url(self.course.id, self.thread.id) + _get_mfe_url(self.course.id, self.thread.id) ) self.assertEqual(args_comment.app_name, 'discussion') @@ -250,7 +259,7 @@ def test_send_notification_to_parent_threads(self): self.assertDictEqual(args_comment_on_response.context, expected_context) self.assertEqual( args_comment_on_response.content_url, - self._get_mfe_url(self.course.id, self.thread.id) + _get_mfe_url(self.course.id, self.thread.id) ) self.assertEqual(args_comment_on_response.app_name, 'discussion') @@ -264,5 +273,37 @@ def test_no_signal_on_creators_own_thread(self): send_response_notifications(self.thread, self.course, self.user_1, parent_id=None) self.assertEqual(handler.call_count, 0) - def _get_mfe_url(self, course_id, post_id): - return f"{settings.DISCUSSIONS_MICROFRONTEND_URL}/{str(course_id)}/posts/{post_id}" + def test_comment_creators_own_response(self): + """ + Check incase post author and response auther is same only send + new comment signal , with your as author_name. + """ + handler = Mock() + USER_NOTIFICATION_REQUESTED.connect(handler) + + self.register_get_comment_response({ + 'id': self.thread_3.id, + 'thread_id': self.thread.id, + 'user_id': self.thread_3.user_id + }) + + send_response_notifications(self.thread, self.course, self.user_3, parent_id=self.thread_2.id) + # check if 1 call is made to the handler i.e. for the thread creator + self.assertEqual(handler.call_count, 1) + + # check if the notification is sent to the thread creator + args_comment = handler.call_args_list[0][1]['notification_data'] + self.assertEqual(args_comment.user_ids, [self.user_1.id]) + self.assertEqual(args_comment.notification_type, 'new_comment') + expected_context = { + 'replier_name': self.user_3.username, + 'post_title': self.thread.title, + 'author_name': 'your', + 'course_name': self.course.display_name, + } + self.assertDictEqual(args_comment.context, expected_context) + self.assertEqual( + args_comment.content_url, + _get_mfe_url(self.course.id, self.thread.id) + ) + self.assertEqual(args_comment.app_name, 'discussion') diff --git a/lms/djangoapps/discussion/rest_api/utils.py b/lms/djangoapps/discussion/rest_api/utils.py index efb6c57dd97..23a6b79746c 100644 --- a/lms/djangoapps/discussion/rest_api/utils.py +++ b/lms/djangoapps/discussion/rest_api/utils.py @@ -391,7 +391,7 @@ def _send_notification(self, user_ids, notification_type, extra_context=None): extra_context = {} notification_data = UserNotificationData( - user_ids=user_ids, + user_ids=[int(user_id) for user_id in user_ids], context={ "replier_name": self.creator.username, "post_title": self.thread.title, @@ -422,19 +422,36 @@ def send_new_response_notification(self): if not self.parent_id and self.creator.id != int(self.thread.user_id): self._send_notification([self.thread.user_id], "new_response") + def _response_and_thread_has_same_creator(self) -> bool: + """ + Check if response and main thread have same author. + """ + return int(self.parent_response.user_id) == int(self.thread.user_id) + def send_new_comment_notification(self): """ Send notification to parent thread creator i.e. comment on the response. """ - if self.parent_response and self.creator.id != int(self.thread.user_id): + # + if ( + self.parent_response and + self.creator.id != int(self.thread.user_id) + ): + # use your if author of response is same as author of post. + author_name = "your" if self._response_and_thread_has_same_creator() else self.parent_response.username context = { - "author_name": self.parent_response.username, + "author_name": author_name, } self._send_notification([self.thread.user_id], "new_comment", extra_context=context) def send_new_comment_on_response_notification(self): """ Send notification to parent response creator i.e. comment on the response. + Do not send notification if author of response is same as author of post. """ - if self.parent_response and self.creator.id != int(self.parent_response.user_id): + if ( + self.parent_response and + self.creator.id != int(self.parent_response.user_id) and not + self._response_and_thread_has_same_creator() + ): self._send_notification([self.parent_response.user_id], "new_comment_on_response") diff --git a/openedx/core/djangoapps/notifications/handlers.py b/openedx/core/djangoapps/notifications/handlers.py index 36dc91ca9e7..8d39c63025c 100644 --- a/openedx/core/djangoapps/notifications/handlers.py +++ b/openedx/core/djangoapps/notifications/handlers.py @@ -32,7 +32,7 @@ def course_enrollment_post_save(signal, sender, enrollment, metadata, **kwargs): ) except IntegrityError: log.info(f'CourseNotificationPreference already exists for user {enrollment.user} ' - f'and course {enrollment.course_id}') + f'and course {enrollment.course.course_key}') @receiver(COURSE_UNENROLLMENT_COMPLETED)