From eebd7d236b3c3092eb1098481eee9bad6ef1c0c3 Mon Sep 17 00:00:00 2001 From: Ahtisham Shahid Date: Fri, 14 Jul 2023 10:44:14 +0500 Subject: [PATCH] fix: added correct url in notification content_url (#32725) * fix: added correct URL in notification content_url * fix: updated logic to compare user ids --- .../discussion/rest_api/tests/test_utils.py | 19 ++++++++++++++++--- lms/djangoapps/discussion/rest_api/utils.py | 3 ++- .../core/djangoapps/notifications/tasks.py | 2 +- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/tests/test_utils.py b/lms/djangoapps/discussion/rest_api/tests/test_utils.py index f551147022..5f5ac7bf97 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_utils.py @@ -5,6 +5,7 @@ Tests for Discussion REST API utils. from datetime import datetime, timedelta from unittest.mock import Mock +from django.conf import settings from httpretty import httpretty from pytz import UTC import unittest @@ -195,7 +196,10 @@ class TestSendResponseNotifications(ForumsEnableMixin, CommentsServiceMockMixin, 'course_name': self.course.display_name, } self.assertDictEqual(args.context, expected_context) - self.assertEqual(args.content_url, 'http://example.com/1') + self.assertEqual( + args.content_url, + self._get_mfe_url(self.course.id, self.thread.id) + ) self.assertEqual(args.app_name, 'discussion') def test_send_notification_to_parent_threads(self): @@ -229,7 +233,10 @@ class TestSendResponseNotifications(ForumsEnableMixin, CommentsServiceMockMixin, '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.content_url, + self._get_mfe_url(self.course.id, self.thread.id) + ) self.assertEqual(args_comment.app_name, 'discussion') # check if the notification is sent to the parent response creator @@ -241,7 +248,10 @@ class TestSendResponseNotifications(ForumsEnableMixin, CommentsServiceMockMixin, '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.content_url, + self._get_mfe_url(self.course.id, self.thread.id) + ) self.assertEqual(args_comment_on_response.app_name, 'discussion') def test_no_signal_on_creators_own_thread(self): @@ -253,3 +263,6 @@ class TestSendResponseNotifications(ForumsEnableMixin, CommentsServiceMockMixin, USER_NOTIFICATION_REQUESTED.connect(handler) 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}" diff --git a/lms/djangoapps/discussion/rest_api/utils.py b/lms/djangoapps/discussion/rest_api/utils.py index c869f1ef39..66d4a8c974 100644 --- a/lms/djangoapps/discussion/rest_api/utils.py +++ b/lms/djangoapps/discussion/rest_api/utils.py @@ -3,6 +3,7 @@ Utils for discussion API. """ from typing import List, Dict +from django.conf import settings from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.core.paginator import Paginator from django.db.models.functions import Length @@ -398,7 +399,7 @@ class DiscussionNotificationSender: **extra_context, }, notification_type=notification_type, - content_url=self.thread.url_with_id(params={'id': self.thread.id}), + content_url=f"{settings.DISCUSSIONS_MICROFRONTEND_URL}/{str(self.course.id)}/posts/{self.thread.id}", app_name="discussion", course_key=self.course.id, ) diff --git a/openedx/core/djangoapps/notifications/tasks.py b/openedx/core/djangoapps/notifications/tasks.py index 477cbbc0a0..f7611f0998 100644 --- a/openedx/core/djangoapps/notifications/tasks.py +++ b/openedx/core/djangoapps/notifications/tasks.py @@ -132,7 +132,7 @@ def create_notification_pref_if_not_exists(user_ids: List, preferences: List, co new_preferences = [] for user_id in user_ids: - if not any(preference.user_id == user_id for preference in preferences): + if not any(preference.user_id == int(user_id) for preference in preferences): new_preferences.append(CourseNotificationPreference( user_id=user_id, course_id=course_id,