Skip to content

Commit

Permalink
fix: Update notification sending logic for discussions (openedx#32879)
Browse files Browse the repository at this point in the history
  • Loading branch information
AhtishamShahid authored Aug 4, 2023
1 parent 69cab96 commit b37286e
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 10 deletions.
51 changes: 46 additions & 5 deletions lms/djangoapps/discussion/rest_api/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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):
Expand All @@ -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')

Expand Down Expand Up @@ -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')

Expand All @@ -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')

Expand All @@ -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')
25 changes: 21 additions & 4 deletions lms/djangoapps/discussion/rest_api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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")
2 changes: 1 addition & 1 deletion openedx/core/djangoapps/notifications/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit b37286e

Please sign in to comment.