Skip to content

Commit

Permalink
Merge pull request #34971 from raccoongang/NiedielnitsevIvan/FC-0047/…
Browse files Browse the repository at this point in the history
…feature/implement-push-notifications-chanel

feat: [FC-0047] add mobile push notifications functionality
  • Loading branch information
bmtcril authored Sep 20, 2024
2 parents 0f975ad + 5fe7a2e commit 471bdd2
Show file tree
Hide file tree
Showing 36 changed files with 441 additions and 73 deletions.
4 changes: 2 additions & 2 deletions lms/djangoapps/ccx/api/v0/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -730,8 +730,8 @@ def make_ccx(self, max_students_allowed=200):
course_id=ccx_course_key,
student_email=self.coach.email,
auto_enroll=True,
email_students=False,
email_params=email_params,
message_students=False,
message_params=email_params,
)
return ccx

Expand Down
8 changes: 4 additions & 4 deletions lms/djangoapps/ccx/api/v0/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,8 @@ def post(self, request):
course_id=ccx_course_key,
student_email=coach.email,
auto_enroll=True,
email_students=True,
email_params=email_params,
message_students=True,
message_params=email_params,
)
# assign staff role for the coach to the newly created ccx
assign_staff_role_to_ccx(ccx_course_key, coach, master_course_object.id)
Expand Down Expand Up @@ -768,8 +768,8 @@ def patch(self, request, ccx_course_id=None):
course_id=ccx_course_key,
student_email=coach.email,
auto_enroll=True,
email_students=True,
email_params=email_params,
message_students=True,
message_params=email_params,
)
# make the new coach staff on the CCX
assign_staff_role_to_ccx(ccx_course_key, coach, master_course_object.id)
Expand Down
26 changes: 16 additions & 10 deletions lms/djangoapps/ccx/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,13 @@ def ccx_students_enrolling_center(action, identifiers, email_students, course_ke
log.info("%s", error)
errors.append(error)
break
enroll_email(course_key, email, auto_enroll=True, email_students=email_students, email_params=email_params)
enroll_email(
course_key,
email,
auto_enroll=True,
message_students=email_students,
message_params=email_params
)
elif action == 'Unenroll' or action == 'revoke': # lint-amnesty, pylint: disable=consider-using-in
for identifier in identifiers:
try:
Expand All @@ -278,7 +284,7 @@ def ccx_students_enrolling_center(action, identifiers, email_students, course_ke
log.info("%s", exp)
errors.append(f"{exp}")
continue
unenroll_email(course_key, email, email_students=email_students, email_params=email_params)
unenroll_email(course_key, email, message_students=email_students, message_params=email_params)
return errors


Expand Down Expand Up @@ -348,8 +354,8 @@ def add_master_course_staff_to_ccx(master_course, ccx_key, display_name, send_em
course_id=ccx_key,
student_email=staff.email,
auto_enroll=True,
email_students=send_email,
email_params=email_params,
message_students=send_email,
message_params=email_params,
)

# allow 'staff' access on ccx to staff of master course
Expand All @@ -373,8 +379,8 @@ def add_master_course_staff_to_ccx(master_course, ccx_key, display_name, send_em
course_id=ccx_key,
student_email=instructor.email,
auto_enroll=True,
email_students=send_email,
email_params=email_params,
message_students=send_email,
message_params=email_params,
)

# allow 'instructor' access on ccx to instructor of master course
Expand Down Expand Up @@ -417,8 +423,8 @@ def remove_master_course_staff_from_ccx(master_course, ccx_key, display_name, se
unenroll_email(
course_id=ccx_key,
student_email=staff.email,
email_students=send_email,
email_params=email_params,
message_students=send_email,
message_params=email_params,
)

for instructor in list_instructor:
Expand All @@ -430,6 +436,6 @@ def remove_master_course_staff_from_ccx(master_course, ccx_key, display_name, se
unenroll_email(
course_id=ccx_key,
student_email=instructor.email,
email_students=send_email,
email_params=email_params,
message_students=send_email,
message_params=email_params,
)
4 changes: 2 additions & 2 deletions lms/djangoapps/ccx/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,8 @@ def create_ccx(request, course, ccx=None):
course_id=ccx_id,
student_email=request.user.email,
auto_enroll=True,
email_students=True,
email_params=email_params,
message_students=True,
message_params=email_params,
)

assign_staff_role_to_ccx(ccx_id, request.user, course.id)
Expand Down
2 changes: 2 additions & 0 deletions lms/djangoapps/discussion/signals/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,10 @@ def create_message_context(comment, site):
'course_id': str(thread.course_id),
'comment_id': comment.id,
'comment_body': comment.body,
'comment_body_text': comment.body_text,
'comment_author_id': comment.user_id,
'comment_created_at': comment.created_at, # comment_client models dates are already serialized
'comment_parent_id': comment.parent_id,
'thread_id': thread.id,
'thread_title': thread.title,
'thread_author_id': thread.user_id,
Expand Down
71 changes: 63 additions & 8 deletions lms/djangoapps/discussion/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -74,6 +75,12 @@ def __init__(self, *args, **kwargs):
self.options['transactional'] = True


class CommentNotification(BaseMessageType):
"""
Notify discussion participants of new comments.
"""


@shared_task(base=LoggedTask)
@set_code_owner_attribute
def send_ace_message(context): # lint-amnesty, pylint: disable=missing-function-docstring
Expand All @@ -82,16 +89,39 @@ 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)
Expand Down Expand Up @@ -154,19 +184,36 @@ def _should_send_message(context):
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'])
not _comment_author_is_thread_author(context)
)


def _should_send_subcomment_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_subcomment(context['comment_id']) and
not _comment_author_is_thread_author(context)
)


def _comment_author_is_thread_author(context):
return context.get('comment_author_id', '') == context['thread_author_id']


def _is_content_still_reported(context):
if context.get('comment_id') is not None:
return len(cc.Comment.find(context['comment_id']).abuse_flaggers) > 0
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
Expand Down Expand Up @@ -204,7 +251,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'])
Expand All @@ -218,6 +265,14 @@ def _build_message_context(context): # lint-amnesty, pylint: disable=missing-fu
'thread_username': thread_author.username,
'comment_username': comment_author.username,
'post_link': post_link,
'push_notification_extra_context': {
'course_id': str(context['course_id']),
'parent_id': str(context['comment_parent_id']),
'notification_type': notification_type,
'topic_id': str(context['thread_commentable_id']),
'thread_id': context['thread_id'],
'comment_id': context['comment_id'],
},
'comment_created_at': date.deserialize(context['comment_created_at']),
'thread_created_at': date.deserialize(context['thread_created_at'])
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{% load i18n %}
{% blocktrans trimmed %}{{ comment_username }} commented to {{ thread_title }}:{% endblocktrans %}
{{ comment_body_text }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{% load i18n %}
{% blocktrans %}Comment to {{ thread_title }}{% endblocktrans %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{% load i18n %}
{% blocktrans trimmed %}{{ comment_username }} replied to {{ thread_title }}: {{ comment_body|truncatechars:200 }}{% endblocktrans %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{% load i18n %}
{% blocktrans %}Response to {{ thread_title }}{% endblocktrans %}
26 changes: 23 additions & 3 deletions lms/djangoapps/discussion/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@
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
Expand Down Expand Up @@ -222,6 +226,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)
Expand All @@ -231,10 +237,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
Expand Down Expand Up @@ -271,8 +279,10 @@ def test_send_discussion_email_notification(self, user_subscribed):
expected_message_context.update({
'comment_author_id': self.comment_author.id,
'comment_body': comment['body'],
'comment_body_text': comment.body_text,
'comment_created_at': ONE_HOUR_AGO,
'comment_id': comment['id'],
'comment_parent_id': comment['parent_id'],
'comment_username': self.comment_author.username,
'course_id': self.course.id,
'thread_author_id': self.thread_author.id,
Expand All @@ -283,7 +293,15 @@ def test_send_discussion_email_notification(self, user_subscribed):
'thread_commentable_id': thread['commentable_id'],
'post_link': f'https://{site.domain}{self.mock_permalink.return_value}',
'site': site,
'site_id': site.id
'site_id': site.id,
'push_notification_extra_context': {
'notification_type': 'forum_response',
'topic_id': thread['commentable_id'],
'course_id': comment['course_id'],
'parent_id': str(comment['parent_id']),
'thread_id': thread['id'],
'comment_id': comment['id'],
},
})
expected_recipient = Recipient(self.thread_author.id, self.thread_author.email)
actual_message = self.mock_ace_send.call_args_list[0][0][0]
Expand Down Expand Up @@ -326,7 +344,9 @@ 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

should_email_send = _is_first_comment(comment_dict['id'], thread['id'])
assert not should_email_send
assert not self.mock_ace_send.called

def test_subcomment_should_not_send_email(self):
Expand Down
4 changes: 2 additions & 2 deletions lms/djangoapps/instructor/access.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ def _change_access(course, user, level, action, send_email=True):
course_id=course.id,
student_email=user.email,
auto_enroll=True,
email_students=send_email,
email_params=email_params,
message_students=send_email,
message_params=email_params,
)
role.add_users(user)
elif action == 'revoke':
Expand Down
Loading

0 comments on commit 471bdd2

Please sign in to comment.