diff --git a/lms/djangoapps/discussion/tasks.py b/lms/djangoapps/discussion/tasks.py index ce6baaa5873d..23af89888134 100644 --- a/lms/djangoapps/discussion/tasks.py +++ b/lms/djangoapps/discussion/tasks.py @@ -98,7 +98,7 @@ def send_ace_message(context): # lint-amnesty, pylint: disable=missing-function message_context ) log.info('Sending forum comment notification with context %s', message_context) - if context['comment_author_id'] != context['thread_author_id']: + if _is_first_comment(context['comment_id'], context['thread_id']): limit_to_channels = None else: limit_to_channels = [ChannelType.PUSH] @@ -263,7 +263,7 @@ def _build_message_context(context, notification_type='forum_comment'): # lint- 'post_link': post_link, 'push_notification_extra_context': { 'notification_type': notification_type, - 'topic_id': context['thread_commentable_id'], + 'topic_id': context.get('thread_commentable_id', ''), 'thread_id': context['thread_id'], 'comment_id': context['comment_id'], }, diff --git a/lms/djangoapps/discussion/tests/test_tasks.py b/lms/djangoapps/discussion/tests/test_tasks.py index d3d3edb1f9e2..be9d2e994b70 100644 --- a/lms/djangoapps/discussion/tests/test_tasks.py +++ b/lms/djangoapps/discussion/tests/test_tasks.py @@ -19,7 +19,7 @@ import openedx.core.djangoapps.django_comment_common.comment_client as cc from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from lms.djangoapps.discussion.signals.handlers import ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY -from lms.djangoapps.discussion.tasks import _should_send_message, _track_notification_sent +from lms.djangoapps.discussion.tasks import _is_first_comment, _should_send_message, _track_notification_sent from openedx.core.djangoapps.ace_common.template_context import get_base_template_context from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.django_comment_common.models import ForumsConfig @@ -222,6 +222,8 @@ def setUp(self): self.ace_send_patcher = mock.patch('edx_ace.ace.send') self.mock_ace_send = self.ace_send_patcher.start() + self.mock_message_patcher = mock.patch('lms.djangoapps.discussion.tasks.ResponseNotification') + self.mock_message = self.mock_message_patcher.start() thread_permalink = '/courses/discussion/dummy_discussion_id' self.permalink_patcher = mock.patch('lms.djangoapps.discussion.tasks.permalink', return_value=thread_permalink) @@ -231,10 +233,12 @@ def tearDown(self): super().tearDown() self.request_patcher.stop() self.ace_send_patcher.stop() + self.mock_message_patcher.stop() self.permalink_patcher.stop() @ddt.data(True, False) def test_send_discussion_email_notification(self, user_subscribed): + self.mock_message_patcher.stop() if user_subscribed: non_matching_id = 'not-a-match' # with per_page left with a default value of 1, this ensures @@ -286,7 +290,8 @@ def test_send_discussion_email_notification(self, user_subscribed): 'site': site, 'site_id': site.id, 'push_notification_extra_context': { - 'notification_type': 'forum_comment', + 'notification_type': 'forum_response', + 'topic_id': thread['commentable_id'], 'thread_id': thread['id'], 'comment_id': comment['id'], }, @@ -332,7 +337,12 @@ def run_should_not_send_email_test(self, thread, comment_dict): 'comment_id': comment_dict['id'], 'thread_id': thread['id'], }) - assert actual_result is False + # from edx_ace.channel import ChannelType + # import pdb; pdb.set_trace() + should_email_send = _is_first_comment(comment_dict['id'], thread['id']) + assert should_email_send is False + + # self.mock_ace_send.assert_called_once_with(self.mock_message, [ChannelType.PUSH]) assert not self.mock_ace_send.called def test_subcomment_should_not_send_email(self):