Skip to content

Commit

Permalink
Added notifications for discussions events (openedx#32432)
Browse files Browse the repository at this point in the history
* feat: added notifications for discussions app

* feat: added unit tests for handler

* feat: updated openedx-events package

* fix: updated notification creation logic and tests

* refactor: updated openedx-event version and event name

* refactor: moved logic to separate methods
  • Loading branch information
AhtishamShahid authored Jul 6, 2023
1 parent 67b9770 commit b477a20
Show file tree
Hide file tree
Showing 19 changed files with 337 additions and 49 deletions.
4 changes: 2 additions & 2 deletions common/djangoapps/student/models/course_enrollment.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class EnrollStatusChange:
# complete a paid course purchase
paid_complete = 'paid_complete'


UNENROLLED_TO_ALLOWEDTOENROLL = 'from unenrolled to allowed to enroll'
ALLOWEDTOENROLL_TO_ENROLLED = 'from allowed to enroll to enrolled'
ENROLLED_TO_ENROLLED = 'from enrolled to enrolled'
Expand All @@ -95,7 +96,6 @@ class EnrollStatusChange:
(DEFAULT_TRANSITION_STATE, DEFAULT_TRANSITION_STATE)
)


EVENT_NAME_ENROLLMENT_ACTIVATED = 'edx.course.enrollment.activated'
EVENT_NAME_ENROLLMENT_DEACTIVATED = 'edx.course.enrollment.deactivated'
EVENT_NAME_ENROLLMENT_MODE_CHANGED = 'edx.course.enrollment.mode_changed'
Expand Down Expand Up @@ -292,7 +292,7 @@ def course_price(self):
MODE_CACHE_NAMESPACE = 'CourseEnrollment.mode_and_active'

class Meta:
unique_together = (('user', 'course'), )
unique_together = (('user', 'course'),)
indexes = [Index(fields=['user', '-created'])]
ordering = ('user', 'course')

Expand Down
5 changes: 3 additions & 2 deletions lms/djangoapps/discussion/rest_api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@
discussion_open_for_user,
get_usernames_for_course,
get_usernames_from_search_string,
set_attribute
set_attribute, send_response_notifications
)


Expand Down Expand Up @@ -1532,7 +1532,8 @@ def create_comment(request, comment_data):

track_comment_created_event(request, course, cc_comment, cc_thread["commentable_id"], followed=False,
from_mfe_sidebar=from_mfe_sidebar)

send_response_notifications(thread=cc_thread, course=course, creator=request.user,
parent_id=comment_data.get("parent_id"))
return api_comment


Expand Down
8 changes: 6 additions & 2 deletions lms/djangoapps/discussion/rest_api/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2243,13 +2243,16 @@ def test_success(self, parent_id, mock_emit):
"/api/v1/threads/test_thread/comments"
)
assert urlparse(httpretty.last_request().path).path == expected_url # lint-amnesty, pylint: disable=no-member
assert parsed_body(httpretty.last_request()) == {

data = httpretty.latest_requests()
assert parsed_body(data[len(data) - 2]) == {
'course_id': [str(self.course.id)],
'body': ['Test body'],
'user_id': [str(self.user.id)],
'anonymous': ['False'],
'anonymous_to_peers': ['False'],
}

expected_event_name = (
"edx.forum.comment.created" if parent_id else
"edx.forum.response.created"
Expand Down Expand Up @@ -2340,7 +2343,8 @@ def test_success_in_black_out_with_user_access(self, parent_id, mock_emit):
"/api/v1/threads/test_thread/comments"
)
assert urlparse(httpretty.last_request().path).path == expected_url # pylint: disable=no-member
assert parsed_body(httpretty.last_request()) == {
data = httpretty.latest_requests()
assert parsed_body(data[len(data) - 2]) == {
"course_id": [str(self.course.id)],
"body": ["Test body"],
"user_id": [str(self.user.id)],
Expand Down
111 changes: 107 additions & 4 deletions lms/djangoapps/discussion/rest_api/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@
"""

from datetime import datetime, timedelta
from unittest.mock import Mock

from httpretty import httpretty
from pytz import UTC
import unittest
from common.djangoapps.student.roles import CourseStaffRole, CourseInstructorRole
from lms.djangoapps.discussion.django_comment_client.tests.utils import ForumsEnableMixin
from lms.djangoapps.discussion.rest_api.tests.utils import CommentsServiceMockMixin, ThreadMock
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory

Expand All @@ -17,8 +21,11 @@
get_course_ta_users_list,
get_course_staff_users_list,
get_moderator_users_list,
get_archived_topics, remove_empty_sequentials
get_archived_topics,
remove_empty_sequentials,
send_response_notifications
)
from openedx_events.learning.signals import USER_NOTIFICATION_REQUESTED


class DiscussionAPIUtilsTestCase(ModuleStoreTestCase):
Expand All @@ -28,7 +35,7 @@ class DiscussionAPIUtilsTestCase(ModuleStoreTestCase):
CREATE_USER = False

def setUp(self):
super().setUp() # lint-amnesty, pylint: disable=super-with-arguments
super().setUp() # lint-amnesty, pylint: disable=super-with-arguments

self.course = CourseFactory.create()
self.course.discussion_blackouts = [datetime.now(UTC) - timedelta(days=3),
Expand Down Expand Up @@ -100,6 +107,7 @@ class TestRemoveEmptySequentials(unittest.TestCase):
"""
Test for the remove_empty_sequentials function
"""

def test_empty_data(self):
# Test that the function can handle an empty list
data = []
Expand Down Expand Up @@ -135,8 +143,7 @@ def test_remove_empty_sequentials(self):
{"type": "chapter", "children": [
{"type": "sequential", "children": []},
{"type": "sequential", "children": []},
]
}
]}
]
expected_output = [
{"type": "chapter", "children": [
Expand All @@ -150,3 +157,99 @@ def test_remove_empty_sequentials(self):
]
result = remove_empty_sequentials(data)
self.assertEqual(result, expected_output)


class TestSendResponseNotifications(ForumsEnableMixin, CommentsServiceMockMixin, ModuleStoreTestCase):
"""
Test for the send_response_notifications function
"""
def setUp(self):
super().setUp()
httpretty.reset()
httpretty.enable()

self.user_1 = UserFactory.create()
self.user_2 = UserFactory.create()
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.course = CourseFactory.create()

def test_send_notification_to_thread_creator(self):
"""
Test that the notification is sent to the thread creator
"""
handler = Mock()
USER_NOTIFICATION_REQUESTED.connect(handler)

# Post the form or do what it takes to send the signal

send_response_notifications(self.thread, self.course, self.user_2, parent_id=None)
self.assertEqual(handler.call_count, 1)
args = handler.call_args[1]['notification_data']
self.assertEqual(args.user_ids, [self.user_1.id])
self.assertEqual(args.notification_type, 'new_response')
expected_context = {
'replier_name': self.user_2.username,
'post_title': 'test thread',
'course_name': self.course.display_name,
}
self.assertDictEqual(args.context, expected_context)
self.assertEqual(args.content_url, 'http://example.com/1')
self.assertEqual(args.app_name, 'discussion')

def test_send_notification_to_parent_threads(self):
"""
Test that the notification signal is sent to the parent response creator and
parent thread creator, it checks signal is sent with correct arguments for both
types of notifications.
"""
handler = Mock()
USER_NOTIFICATION_REQUESTED.connect(handler)

self.register_get_comment_response({
'id': self.thread_2.id,
'thread_id': self.thread.id,
'user_id': self.thread_2.user_id
})

send_response_notifications(self.thread, self.course, self.user_3, parent_id=self.thread_2.id)
# check if 2 call are made to the handler i.e. one for the response creator and one for the thread creator
self.assertEqual(handler.call_count, 2)

# check if the notification is sent to the thread creator
args_comment = handler.call_args_list[0][1]['notification_data']
args_comment_on_response = handler.call_args_list[1][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': 'dummy',
'course_name': self.course.display_name,
}
self.assertDictEqual(args_comment.context, expected_context)
self.assertEqual(args_comment.content_url, 'http://example.com/1')
self.assertEqual(args_comment.app_name, 'discussion')

# check if the notification is sent to the parent response creator
self.assertEqual(args_comment_on_response.user_ids, [self.user_2.id])
self.assertEqual(args_comment_on_response.notification_type, 'new_comment_on_response')
expected_context = {
'replier_name': self.user_3.username,
'post_title': self.thread.title,
'course_name': self.course.display_name,
}
self.assertDictEqual(args_comment_on_response.context, expected_context)
self.assertEqual(args_comment_on_response.content_url, 'http://example.com/1')
self.assertEqual(args_comment_on_response.app_name, 'discussion')

def test_no_signal_on_creators_own_thread(self):
"""
Makes sure that no signal is emitted if user creates response on
their own thread.
"""
handler = Mock()
USER_NOTIFICATION_REQUESTED.connect(handler)
send_response_notifications(self.thread, self.course, self.user_1, parent_id=None)
self.assertEqual(handler.call_count, 0)
16 changes: 16 additions & 0 deletions lms/djangoapps/discussion/rest_api/tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -656,3 +656,19 @@ def querystring(request):
# This could just be HTTPrettyRequest.querystring, but that method double-decodes '%2B' -> '+' -> ' '.
# You can just remove this method when this issue is fixed: https://github.com/gabrielfalcao/HTTPretty/issues/240
return parse_qs(request.path.split('?', 1)[-1])


class ThreadMock(object):
"""
A mock thread object
"""

def __init__(self, thread_id, creator, title, parent_id=None):
self.id = thread_id
self.user_id = creator.id
self.username = creator.username
self.title = title
self.parent_id = parent_id

def url_with_id(self, params):
return f"http://example.com/{params['id']}"
86 changes: 86 additions & 0 deletions lms/djangoapps/discussion/rest_api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
FORUM_ROLE_GROUP_MODERATOR,
FORUM_ROLE_COMMUNITY_TA,
)
from openedx_events.learning.signals import USER_NOTIFICATION_REQUESTED
from openedx_events.learning.data import UserNotificationData
from openedx.core.djangoapps.django_comment_common.comment_client.comment import Comment


class AttributeDict(dict):
Expand Down Expand Up @@ -351,3 +354,86 @@ def get_archived_topics(filtered_topic_ids: List[str], topics: List[Dict[str, st
if topic['id'] == topic_id and topic['usage_key'] is not None:
archived_topics.append(topic)
return archived_topics


def send_response_notifications(thread, course, creator, parent_id=None):
"""
Send notifications to users who are subscribed to the thread.
"""
notification_sender = DiscussionNotificationSender(thread, course, creator, parent_id)
notification_sender.send_new_comment_notification()
notification_sender.send_new_response_notification()
notification_sender.send_new_comment_on_response_notification()


class DiscussionNotificationSender:
"""
Class to send notifications to users who are subscribed to the thread.
"""

def __init__(self, thread, course, creator, parent_id=None):
self.thread = thread
self.course = course
self.creator = creator
self.parent_id = parent_id
self.parent_response = None
self._get_parent_response()

def _send_notification(self, user_ids, notification_type, extra_context=None):
"""
Send notification to users
"""
if not user_ids:
return

if extra_context is None:
extra_context = {}

notification_data = UserNotificationData(
user_ids=user_ids,
context={
"replier_name": self.creator.username,
"post_title": self.thread.title,
"course_name": self.course.display_name,
**extra_context,
},
notification_type=notification_type,
content_url=self.thread.url_with_id(params={'id': self.thread.id}),
app_name="discussion",
course_key=self.course.id,
)
USER_NOTIFICATION_REQUESTED.send_event(notification_data=notification_data)

def _get_parent_response(self):
"""
Get parent response object
"""
if self.parent_id and not self.parent_response:
self.parent_response = Comment(id=self.parent_id).retrieve()

return self.parent_response

def send_new_response_notification(self):
"""
Send notification to users who are subscribed to the main thread/post i.e.
there is a response to the main thread.
"""
if not self.parent_id and self.creator.id != self.thread.user_id:
self._send_notification([self.thread.user_id], "new_response")

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):
context = {
"author_name": self.parent_response.username,
}
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.
"""
if self.parent_response and self.creator.id != int(self.parent_response.user_id):
self._send_notification([self.parent_response.user_id], "new_comment_on_response")
2 changes: 1 addition & 1 deletion openedx/core/djangoapps/notifications/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from django.contrib import admin

from .models import Notification, CourseNotificationPreference
from .models import CourseNotificationPreference, Notification


class NotificationAdmin(admin.ModelAdmin):
Expand Down
1 change: 0 additions & 1 deletion openedx/core/djangoapps/notifications/base_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
find_pref_in_normalized_prefs,
)


COURSE_NOTIFICATION_TYPES = {
'new_comment_on_response': {
'notification_app': 'discussion',
Expand Down
Loading

0 comments on commit b477a20

Please sign in to comment.