feat: [AXM-556] refactor discussion push notifications sending

lms/djangoapps/discussion/tasks.py

@@ -12,6 +12,7 @@
 from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
 from django.contrib.sites.models import Site
 from edx_ace import ace
+from edx_ace.channel import ChannelType
 from edx_ace.recipient import Recipient
 from edx_ace.utils import date
 from edx_django_utils.monitoring import set_code_owner_attribute
@@ -74,6 +75,12 @@ def __init__(self, *args, **kwargs):
         self.options['transactional'] = True
 
 
+class CommentNotification(BaseMessageType):
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.options['transactional'] = True
+
+
 @shared_task(base=LoggedTask)
 @set_code_owner_attribute
 def send_ace_message(context):  # lint-amnesty, pylint: disable=missing-function-docstring
@@ -82,17 +89,40 @@ def send_ace_message(context):  # lint-amnesty, pylint: disable=missing-function
     if _should_send_message(context):
         context['site'] = Site.objects.get(id=context['site_id'])
         thread_author = User.objects.get(id=context['thread_author_id'])
-        with emulate_http_request(site=context['site'], user=thread_author):
-            message_context = _build_message_context(context)
+        comment_author = User.objects.get(id=context['comment_author_id'])
+        with emulate_http_request(site=context['site'], user=comment_author):
+            message_context = _build_message_context(context, notification_type='forum_response')
             message = ResponseNotification().personalize(
                 Recipient(thread_author.id, thread_author.email),
                 _get_course_language(context['course_id']),
                 message_context
             )
-            log.info('Sending forum comment email notification with context %s', message_context)
-            ace.send(message)
+            log.info('Sending forum comment notification with context %s', message_context)
+            if _is_first_comment(context['comment_id'], context['thread_id']):
+                limit_to_channels = None
+            else:
+                limit_to_channels = [ChannelType.PUSH]
+            ace.send(message, limit_to_channels=limit_to_channels)
             _track_notification_sent(message, context)
 
+    elif _should_send_subcomment_message(context):
+        context['site'] = Site.objects.get(id=context['site_id'])
+        comment_author = User.objects.get(id=context['comment_author_id'])
+        thread_author = User.objects.get(id=context['thread_author_id'])
+
+        with emulate_http_request(site=context['site'], user=comment_author):
+            message_context = _build_message_context(context)
+            message = CommentNotification().personalize(
+                Recipient(thread_author.id, thread_author.email),
+                _get_course_language(context['course_id']),
+                message_context
+            )
+            log.info('Sending forum comment notification with context %s', message_context)
+            ace.send(message, limit_to_channels=[ChannelType.PUSH])
+            _track_notification_sent(message, context)
+    else:
+        return
+
 
 @shared_task(base=LoggedTask)
 @set_code_owner_attribute
@@ -153,8 +183,17 @@ def _should_send_message(context):
     cc_thread_author = cc.User(id=context['thread_author_id'], course_id=context['course_id'])
     return (
         _is_user_subscribed_to_thread(cc_thread_author, context['thread_id']) and
-        _is_not_subcomment(context['comment_id']) and
-        _is_first_comment(context['comment_id'], context['thread_id'])
+        _is_not_subcomment(context['comment_id'])
+    )
+
+
+def _should_send_subcomment_message(context):
+    cc_thread_author = cc.User(id=context['thread_author_id'], course_id=context['course_id'])
+    comment_author_is_thread_author = context['comment_author_id'] == context['thread_author_id']
+    return (
+        _is_user_subscribed_to_thread(cc_thread_author, context['thread_id']) and
+        _is_subcomment(context['comment_id']) and
+        not comment_author_is_thread_author
     )
 
 
@@ -164,9 +203,13 @@ def _is_content_still_reported(context):
     return len(cc.Thread.find(context['thread_id']).abuse_flaggers) > 0
 
 
-def _is_not_subcomment(comment_id):
+def _is_subcomment(comment_id):
     comment = cc.Comment.find(id=comment_id).retrieve()
-    return not getattr(comment, 'parent_id', None)
+    return getattr(comment, 'parent_id', None)
+
+
+def _is_not_subcomment(comment_id):
+    return not _is_subcomment(comment_id)
 
 
 def _is_first_comment(comment_id, thread_id):  # lint-amnesty, pylint: disable=missing-function-docstring
@@ -204,7 +247,7 @@ def _get_course_language(course_id):
     return language
 
 
-def _build_message_context(context):  # lint-amnesty, pylint: disable=missing-function-docstring
+def _build_message_context(context, notification_type='forum_comment'):  # lint-amnesty, pylint: disable=missing-function-docstring
     message_context = get_base_template_context(context['site'])
     message_context.update(context)
     thread_author = User.objects.get(id=context['thread_author_id'])
@@ -219,7 +262,8 @@ def _build_message_context(context):  # lint-amnesty, pylint: disable=missing-fu
         'comment_username': comment_author.username,
         'post_link': post_link,
         'push_notification_extra_context': {
-            'notification_type': 'forum_comment',
+            'notification_type': notification_type,
+            'topic_id': context.get('thread_commentable_id', ''),
             'thread_id': context['thread_id'],
             'comment_id': context['comment_id'],
         },

lms/djangoapps/discussion/templates/discussion/edx_ace/commentnotification/push/body.txt

@@ -0,0 +1,3 @@
+{% load i18n %}
+{% blocktrans trimmed %}{{ comment_username }} commented to {{ thread_title }}:{% endblocktrans %}
+{{ comment_body_text }}

lms/djangoapps/discussion/templates/discussion/edx_ace/commentnotification/push/subject.txt

@@ -0,0 +1,3 @@
+{% load i18n %}
+
+{% blocktrans %}Comment to {{ thread_title }}{% endblocktrans %}

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):

lms/templates/instructor/edx_ace/addbetatester/push/subject.txt

@@ -1,4 +1,4 @@
 {% load i18n %}
 {% autoescape off %}
-{% blocktrans %}You have been invited to a betca test for {{ course_name }}{% endblocktrans %}
+{% blocktrans %}You have been invited to a beta test for {{ course_name }}{% endblocktrans %}
 {% endautoescape %}

openedx/core/djangoapps/ace_common/settings/common.py

@@ -36,7 +36,7 @@ def plugin_settings(settings):  # lint-amnesty, pylint: disable=missing-function
 
     if getattr(settings, 'FIREBASE_APP', None):
         settings.ACE_ENABLED_CHANNELS.append(settings.ACE_CHANNEL_DEFAULT_PUSH)
-        settings.ACE_ENABLED_POLICIES.append(settings.ACE_CHANNEL_DEFAULT_PUSH)
+        settings.ACE_ENABLED_POLICIES.append('bulk_push_notification_optout')
 
         settings.PUSH_NOTIFICATIONS_SETTINGS = {
             'CONFIG': 'push_notifications.conf.AppConfig',

openedx/core/djangoapps/ace_common/settings/production.py

@@ -33,7 +33,7 @@ def plugin_settings(settings):
     settings.FIREBASE_APP = setup_firebase_app(settings.FIREBASE_CREDENTIALS, settings.FCM_APP_NAME)
     if settings.FIREBASE_APP:
         settings.ACE_ENABLED_CHANNELS.append(settings.ACE_CHANNEL_DEFAULT_PUSH)
-        settings.ACE_ENABLED_POLICIES.append(settings.ACE_CHANNEL_DEFAULT_PUSH)
+        settings.ACE_ENABLED_POLICIES.append('bulk_push_notification_optout')
 
         settings.PUSH_NOTIFICATIONS_SETTINGS = {
             'CONFIG': 'push_notifications.conf.AppConfig',