From f3053de6b7e4458a37f88ffc89fce0eef4554e9b Mon Sep 17 00:00:00 2001 From: pganesh-apphelix Date: Mon, 23 Jun 2025 11:31:34 +0000 Subject: [PATCH 1/9] chore: remove ensure csrf cookie decorator from discussion api --- .../tests/test_bulk_enabledisable_discussions.py | 2 +- cms/djangoapps/contentstore/views/course.py | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_bulk_enabledisable_discussions.py b/cms/djangoapps/contentstore/tests/test_bulk_enabledisable_discussions.py index e9ed344f9d..149a7a5318 100644 --- a/cms/djangoapps/contentstore/tests/test_bulk_enabledisable_discussions.py +++ b/cms/djangoapps/contentstore/tests/test_bulk_enabledisable_discussions.py @@ -85,7 +85,7 @@ class BulkEnableDisableDiscussionsTestCase(ModuleStoreTestCase): self.assertEqual(response.status_code, 200) response_data = response.json() print(response_data) - self.assertEqual(response_data['updated_and_republished'], 0 if is_enabled else 2) + self.assertEqual(response_data['units_updated_and_republished'], 0 if is_enabled else 2) # Check that all verticals now have discussion_enabled set to the expected value with self.store.bulk_operations(self.course_key): diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 2a75710be7..3de22d7f88 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -29,6 +29,8 @@ from opaque_keys.edx.locator import BlockUsageLocator from organizations.api import add_organization_course, ensure_organization from organizations.exceptions import InvalidOrganizationException from rest_framework.exceptions import ValidationError +from rest_framework.decorators import api_view +from openedx.core.lib.api.view_utils import view_auth_classes from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import create_xblock_info from cms.djangoapps.course_creators.views import add_user_with_status_unrequested, get_course_creator_status @@ -1710,10 +1712,9 @@ def group_configurations_detail_handler(request, course_key_string, group_config ) -@login_required +@api_view(['PUT']) +@view_auth_classes() @expect_json -@ensure_csrf_cookie -@require_http_methods(["PUT"]) def bulk_enable_disable_discussions(request, course_key_string): """ API endpoint to enable/disable discussions for all verticals in the course and republish them. @@ -1733,9 +1734,6 @@ def bulk_enable_disable_discussions(request, course_key_string): if not has_studio_write_access(user, course_key): raise PermissionDenied() - if 'application/json' not in request.META.get('HTTP_ACCEPT', 'application/json'): - return JsonResponseBadRequest({"error": "Only supports json requests"}) - if 'discussion_enabled' not in request.json: return JsonResponseBadRequest({"error": "Missing 'discussion_enabled' field in request body"}) discussion_enabled = request.json['discussion_enabled'] @@ -1760,7 +1758,7 @@ def bulk_enable_disable_discussions(request, course_key_string): if store.has_published_version(vertical): store.publish(vertical.location, user.id) changed += 1 - return JsonResponse({"updated_and_republished": changed}) + return JsonResponse({"units_updated_and_republished": changed}) except Exception as e: # lint-amnesty, pylint: disable=broad-except log.exception("Exception occurred while enabling/disabling discussion: %s", str(e)) return JsonResponseBadRequest({"error": str(e)}) From 3806f9f4f0580f03586ef44c94df45830309c8bc Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul <77053848+muhammadadeeltajamul@users.noreply.github.com> Date: Thu, 26 Jun 2025 13:50:47 +0500 Subject: [PATCH 2/9] chore: added immediate email event (#36950) --- .../djangoapps/notifications/email/events.py | 27 +++++++++++++++++++ .../djangoapps/notifications/email/tasks.py | 3 ++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/notifications/email/events.py b/openedx/core/djangoapps/notifications/email/events.py index a575d78eae..d05c829df2 100644 --- a/openedx/core/djangoapps/notifications/email/events.py +++ b/openedx/core/djangoapps/notifications/email/events.py @@ -48,3 +48,30 @@ def send_user_email_digest_sent_event(user, cadence_type, notifications, message EMAIL_DIGEST_SENT, event_data, ) + + +def send_immediate_email_digest_sent_event(user, cadence_type, notification): + """ + Sends tracker and segment event for immediate notification email + """ + event_data = { + "username": user.username, + "email": user.email, + "cadence_type": cadence_type, + "course_id": str(notification.course_id), + "app_name": notification.app_name, + "notification_type": notification.notification_type, + "content_url": notification.content_url, + "content": notification.content, + "send_at": str(datetime.datetime.now()) + } + with tracker.get_tracker().context(EMAIL_DIGEST_SENT, event_data): + tracker.emit( + EMAIL_DIGEST_SENT, + event_data, + ) + segment.track( + user.id, + EMAIL_DIGEST_SENT, + event_data, + ) diff --git a/openedx/core/djangoapps/notifications/email/tasks.py b/openedx/core/djangoapps/notifications/email/tasks.py index c907a6f09b..17aeca7ab2 100644 --- a/openedx/core/djangoapps/notifications/email/tasks.py +++ b/openedx/core/djangoapps/notifications/email/tasks.py @@ -16,7 +16,7 @@ from openedx.core.djangoapps.notifications.models import ( Notification, get_course_notification_preference_config_version ) -from .events import send_user_email_digest_sent_event +from .events import send_immediate_email_digest_sent_event, send_user_email_digest_sent_event from .message_type import EmailNotificationMessageType from .utils import ( add_headers_to_email_message, @@ -183,3 +183,4 @@ def send_immediate_cadence_email(email_notification_mapping, course_key): ).personalize(Recipient(user.id, user.email), language, message_context) message = add_headers_to_email_message(message, message_context) ace.send(message) + send_immediate_email_digest_sent_event(user, EmailCadence.IMMEDIATELY, notification) From e0fbb96ee7ad390d0cfbd3235e69fbbee3d5ec9e Mon Sep 17 00:00:00 2001 From: Ali-Salman29 Date: Tue, 24 Jun 2025 15:48:12 +0200 Subject: [PATCH 3/9] feat!: remove cs_comments_service support for forum's search APIs This will force the use of the new v2 forum's APIs for searching. --- .../django_comment_client/tests/group_id.py | 216 +++ .../discussion/rest_api/tests/test_api.py | 539 ------- .../discussion/rest_api/tests/test_api_v2.py | 637 +++++++++ .../discussion/rest_api/tests/test_views.py | 348 ----- .../rest_api/tests/test_views_v2.py | 749 ++++++++-- lms/djangoapps/discussion/tests/test_views.py | 977 ------------- .../discussion/tests/test_views_v2.py | 1264 +++++++++++++++++ .../comment_client/thread.py | 55 +- .../comment_client/user.py | 22 +- .../comment_client/utils.py | 17 + 10 files changed, 2778 insertions(+), 2046 deletions(-) create mode 100644 lms/djangoapps/discussion/tests/test_views_v2.py diff --git a/lms/djangoapps/discussion/django_comment_client/tests/group_id.py b/lms/djangoapps/discussion/django_comment_client/tests/group_id.py index 0a5fbe4919..9907db95bb 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/group_id.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/group_id.py @@ -241,3 +241,219 @@ class NonCohortedTopicGroupIdTestMixin(GroupIdAssertionMixin): self.call_view(mock_is_forum_v2_enabled, mock_request, team.discussion_topic_id, self.student, '') self._assert_comments_service_called_without_group_id(mock_request) + + +class GroupIdAssertionMixinV2: + """ + Provides assertion methods for testing group_id functionality in forum v2. + + This mixin contains helper methods to verify that the comments service is called + with the correct group_id parameters and that responses contain the expected + group information. + """ + def _get_params_last_call(self, function_name): + """ + Returns the data or params dict that `mock_request` was called with. + """ + return self.get_mock_func_calls(function_name)[-1][1] + + def _assert_comments_service_called_with_group_id(self, group_id): + assert self.check_mock_called('get_user_threads') + assert self._get_params_last_call('get_user_threads')['group_id'] == group_id + + def _assert_comments_service_called_without_group_id(self): + assert self.check_mock_called('get_user_threads') + assert 'group_id' not in self._get_params_last_call('get_user_threads') + + def _assert_html_response_contains_group_info(self, response): + group_info = {"group_id": None, "group_name": None} + match = re.search(r'"group_id": (\d*),', response.content.decode('utf-8')) + if match and match.group(1) != '': + group_info["group_id"] = int(match.group(1)) + match = re.search(r'"group_name": "(\w*)"', response.content.decode('utf-8')) + if match: + group_info["group_name"] = match.group(1) + self._assert_thread_contains_group_info(group_info) + + def _assert_json_response_contains_group_info(self, response, extract_thread=None): + """ + :param extract_thread: a function which accepts a dictionary (complete + json response payload) and returns another dictionary (first + occurrence of a thread model within that payload). if None is + passed, the identity function is assumed. + """ + payload = json.loads(response.content.decode('utf-8')) + thread = extract_thread(payload) if extract_thread else payload + self._assert_thread_contains_group_info(thread) + + def _assert_thread_contains_group_info(self, thread): + assert thread['group_id'] == self.student_cohort.id + assert thread['group_name'] == self.student_cohort.name + + +class CohortedTopicGroupIdTestMixinV2(GroupIdAssertionMixinV2): + """ + Provides test cases to verify that views pass the correct `group_id` to + the comments service when requesting content in cohorted discussions for forum v2. + """ + def call_view(self, commentable_id, user, group_id, pass_group_id=True): + """ + Call the view for the implementing test class, constructing a request + from the parameters. + """ + pass # lint-amnesty, pylint: disable=unnecessary-pass + + def test_cohorted_topic_student_without_group_id(self): + self.call_view("cohorted_topic", self.student, '', pass_group_id=False) + self._assert_comments_service_called_with_group_id(self.student_cohort.id) + + def test_cohorted_topic_student_none_group_id(self): + self.call_view("cohorted_topic", self.student, "") + self._assert_comments_service_called_with_group_id(self.student_cohort.id) + + def test_cohorted_topic_student_with_own_group_id(self): + self.call_view("cohorted_topic", self.student, self.student_cohort.id) + self._assert_comments_service_called_with_group_id(self.student_cohort.id) + + def test_cohorted_topic_student_with_other_group_id(self): + self.call_view( + "cohorted_topic", + self.student, + self.moderator_cohort.id + ) + self._assert_comments_service_called_with_group_id(self.student_cohort.id) + + def test_cohorted_topic_moderator_without_group_id(self): + self.call_view( + "cohorted_topic", + self.moderator, + '', + pass_group_id=False + ) + self._assert_comments_service_called_without_group_id() + + def test_cohorted_topic_moderator_none_group_id(self): + self.call_view("cohorted_topic", self.moderator, "") + self._assert_comments_service_called_without_group_id() + + def test_cohorted_topic_moderator_with_own_group_id(self): + self.call_view( + "cohorted_topic", + self.moderator, + self.moderator_cohort.id + ) + self._assert_comments_service_called_with_group_id(self.moderator_cohort.id) + + def test_cohorted_topic_moderator_with_other_group_id(self): + self.call_view( + "cohorted_topic", + self.moderator, + self.student_cohort.id + ) + self._assert_comments_service_called_with_group_id(self.student_cohort.id) + + def test_cohorted_topic_moderator_with_invalid_group_id(self): + invalid_id = self.student_cohort.id + self.moderator_cohort.id + response = self.call_view("cohorted_topic", self.moderator, invalid_id) # lint-amnesty, pylint: disable=assignment-from-no-return + assert response.status_code == 500 + + def test_cohorted_topic_enrollment_track_invalid_group_id(self): + CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.AUDIT) + CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.VERIFIED) + discussion_settings = CourseDiscussionSettings.get(self.course.id) + discussion_settings.update({ + 'divided_discussions': ['cohorted_topic'], + 'division_scheme': CourseDiscussionSettings.ENROLLMENT_TRACK, + 'always_divide_inline_discussions': True, + }) + + invalid_id = -1000 + response = self.call_view("cohorted_topic", self.moderator, invalid_id) # lint-amnesty, pylint: disable=assignment-from-no-return + assert response.status_code == 500 + + +class NonCohortedTopicGroupIdTestMixinV2(GroupIdAssertionMixinV2): + """ + Provides test cases to verify that views pass the correct `group_id` to + the comments service when requesting content in non-cohorted discussions for forum v2. + """ + def call_view(self, commentable_id, user, group_id, pass_group_id=True): + """ + Call the view for the implementing test class, constructing a request + from the parameters. + """ + pass # lint-amnesty, pylint: disable=unnecessary-pass + + def test_non_cohorted_topic_student_without_group_id(self): + self.call_view( + "non_cohorted_topic", + self.student, + '', + pass_group_id=False + ) + self._assert_comments_service_called_without_group_id() + + def test_non_cohorted_topic_student_none_group_id(self): + self.call_view("non_cohorted_topic", self.student, '') + self._assert_comments_service_called_without_group_id() + + def test_non_cohorted_topic_student_with_own_group_id(self): + self.call_view( + "non_cohorted_topic", + self.student, + self.student_cohort.id + ) + self._assert_comments_service_called_without_group_id() + + def test_non_cohorted_topic_student_with_other_group_id(self): + self.call_view( + "non_cohorted_topic", + self.student, + self.moderator_cohort.id + ) + self._assert_comments_service_called_without_group_id() + + def test_non_cohorted_topic_moderator_without_group_id(self): + self.call_view( + "non_cohorted_topic", + self.moderator, + "", + pass_group_id=False, + ) + self._assert_comments_service_called_without_group_id() + + def test_non_cohorted_topic_moderator_none_group_id(self): + self.call_view("non_cohorted_topic", self.moderator, '') + self._assert_comments_service_called_without_group_id() + + def test_non_cohorted_topic_moderator_with_own_group_id(self): + self.call_view( + "non_cohorted_topic", + self.moderator, + self.moderator_cohort.id, + ) + self._assert_comments_service_called_without_group_id() + + def test_non_cohorted_topic_moderator_with_other_group_id(self): + self.call_view( + "non_cohorted_topic", + self.moderator, + self.student_cohort.id, + ) + self._assert_comments_service_called_without_group_id() + + def test_non_cohorted_topic_moderator_with_invalid_group_id(self): + invalid_id = self.student_cohort.id + self.moderator_cohort.id + self.call_view("non_cohorted_topic", self.moderator, invalid_id) + self._assert_comments_service_called_without_group_id() + + def test_team_discussion_id_not_cohorted(self): + team = CourseTeamFactory( + course_id=self.course.id, + topic_id='topic-id' + ) + + team.add_user(self.student) + self.call_view(team.discussion_topic_id, self.student, '') + + self._assert_comments_service_called_without_group_id() diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index aec308bd62..48b943543c 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -21,7 +21,6 @@ from opaque_keys.edx.locator import CourseLocator from pytz import UTC from rest_framework.exceptions import PermissionDenied -from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory @@ -48,7 +47,6 @@ from lms.djangoapps.discussion.rest_api.api import ( get_course_topics, get_course_topics_v2, get_thread, - get_thread_list, get_user_comments, update_comment, update_thread @@ -77,7 +75,6 @@ from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_MODERATOR, FORUM_ROLE_STUDENT, - FORUM_ROLE_GROUP_MODERATOR, Role ) from openedx.core.lib.exceptions import CourseNotFoundError, PageNotFoundError @@ -698,542 +695,6 @@ class GetCourseTopicsTest(CommentsServiceMockMixin, ForumsEnableMixin, UrlResetM } -@ddt.ddt -@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) -class GetThreadListTest(ForumsEnableMixin, CommentsServiceMockMixin, UrlResetMixin, SharedModuleStoreTestCase): - """Test for get_thread_list""" - @classmethod - def setUpClass(cls): - super().setUpClass() - cls.course = CourseFactory.create() - - @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) - def setUp(self): - super().setUp() - httpretty.reset() - httpretty.enable() - self.addCleanup(httpretty.reset) - self.addCleanup(httpretty.disable) - self.maxDiff = None # pylint: disable=invalid-name - self.user = UserFactory.create() - self.register_get_user_response(self.user) - self.request = RequestFactory().get("/test_path") - self.request.user = self.user - CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id) - self.author = UserFactory.create() - self.course.cohort_config = {"cohorted": False} - modulestore().update_item(self.course, ModuleStoreEnum.UserID.test) - self.cohort = CohortFactory.create(course_id=self.course.id) - - def get_thread_list( - self, - threads, - page=1, - page_size=1, - num_pages=1, - course=None, - topic_id_list=None, - ): - """ - Register the appropriate comments service response, then call - get_thread_list and return the result. - """ - course = course or self.course - self.register_get_threads_response(threads, page, num_pages) - ret = get_thread_list(self.request, course.id, page, page_size, topic_id_list) - return ret - - def test_nonexistent_course(self): - with pytest.raises(CourseNotFoundError): - get_thread_list(self.request, CourseLocator.from_string("course-v1:non+existent+course"), 1, 1) - - def test_not_enrolled(self): - self.request.user = UserFactory.create() - with pytest.raises(CourseNotFoundError): - self.get_thread_list([]) - - def test_discussions_disabled(self): - with pytest.raises(DiscussionDisabledError): - self.get_thread_list([], course=_discussion_disabled_course_for(self.user)) - - def test_empty(self): - assert self.get_thread_list( - [], num_pages=0 - ).data == { - 'pagination': { - 'next': None, - 'previous': None, - 'num_pages': 0, - 'count': 0 - }, - 'results': [], - 'text_search_rewrite': None - } - - def test_get_threads_by_topic_id(self): - self.get_thread_list([], topic_id_list=["topic_x", "topic_meow"]) - assert urlparse(httpretty.last_request().path).path == '/api/v1/threads' # lint-amnesty, pylint: disable=no-member - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["1"], - "per_page": ["1"], - "commentable_ids": ["topic_x,topic_meow"] - }) - - def test_basic_query_params(self): - self.get_thread_list([], page=6, page_size=14) - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["6"], - "per_page": ["14"], - }) - - def test_thread_content(self): - self.course.cohort_config = {"cohorted": True} - modulestore().update_item(self.course, ModuleStoreEnum.UserID.test) - source_threads = [ - make_minimal_cs_thread({ - "id": "test_thread_id_0", - "course_id": str(self.course.id), - "commentable_id": "topic_x", - "username": self.author.username, - "user_id": str(self.author.id), - "title": "Test Title", - "body": "Test body", - "votes": {"up_count": 4}, - "comments_count": 5, - "unread_comments_count": 3, - "endorsed": True, - "read": True, - "created_at": "2015-04-28T00:00:00Z", - "updated_at": "2015-04-28T11:11:11Z", - }), - make_minimal_cs_thread({ - "id": "test_thread_id_1", - "course_id": str(self.course.id), - "commentable_id": "topic_y", - "group_id": self.cohort.id, - "username": self.author.username, - "user_id": str(self.author.id), - "thread_type": "question", - "title": "Another Test Title", - "body": "More content", - "votes": {"up_count": 9}, - "comments_count": 18, - "created_at": "2015-04-28T22:22:22Z", - "updated_at": "2015-04-28T00:33:33Z", - }) - ] - expected_threads = [ - self.expected_thread_data({ - "id": "test_thread_id_0", - "author": self.author.username, - "topic_id": "topic_x", - "vote_count": 4, - "comment_count": 6, - "unread_comment_count": 3, - "comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_thread_id_0", - "editable_fields": ["abuse_flagged", "copy_link", "following", "read", "voted"], - "has_endorsed": True, - "read": True, - "created_at": "2015-04-28T00:00:00Z", - "updated_at": "2015-04-28T11:11:11Z", - "abuse_flagged_count": None, - "can_delete": False, - }), - self.expected_thread_data({ - "id": "test_thread_id_1", - "author": self.author.username, - "topic_id": "topic_y", - "group_id": self.cohort.id, - "group_name": self.cohort.name, - "type": "question", - "title": "Another Test Title", - "raw_body": "More content", - "preview_body": "More content", - "rendered_body": "

More content

", - "vote_count": 9, - "comment_count": 19, - "created_at": "2015-04-28T22:22:22Z", - "updated_at": "2015-04-28T00:33:33Z", - "comment_list_url": None, - "endorsed_comment_list_url": ( - "http://testserver/api/discussion/v1/comments/?thread_id=test_thread_id_1&endorsed=True" - ), - "non_endorsed_comment_list_url": ( - "http://testserver/api/discussion/v1/comments/?thread_id=test_thread_id_1&endorsed=False" - ), - "editable_fields": ["abuse_flagged", "copy_link", "following", "read", "voted"], - "abuse_flagged_count": None, - "can_delete": False, - }), - ] - - expected_result = make_paginated_api_response( - results=expected_threads, count=2, num_pages=1, next_link=None, previous_link=None - ) - expected_result.update({"text_search_rewrite": None}) - assert self.get_thread_list(source_threads).data == expected_result - - @ddt.data( - *itertools.product( - [ - FORUM_ROLE_ADMINISTRATOR, - FORUM_ROLE_MODERATOR, - FORUM_ROLE_COMMUNITY_TA, - FORUM_ROLE_STUDENT, - FORUM_ROLE_GROUP_MODERATOR, - ], - [True, False] - ) - ) - @ddt.unpack - def test_request_group(self, role_name, course_is_cohorted): - cohort_course = CourseFactory.create(cohort_config={"cohorted": course_is_cohorted}) - CourseEnrollmentFactory.create(user=self.user, course_id=cohort_course.id) - CohortFactory.create(course_id=cohort_course.id, users=[self.user]) - _assign_role_to_user(user=self.user, course_id=cohort_course.id, role=role_name) - self.get_thread_list([], course=cohort_course) - actual_has_group = "group_id" in httpretty.last_request().querystring # lint-amnesty, pylint: disable=no-member - expected_has_group = (course_is_cohorted and - role_name in (FORUM_ROLE_STUDENT, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_GROUP_MODERATOR)) - assert actual_has_group == expected_has_group - - def test_pagination(self): - # N.B. Empty thread list is not realistic but convenient for this test - expected_result = make_paginated_api_response( - results=[], count=0, num_pages=3, next_link="http://testserver/test_path?page=2", previous_link=None - ) - expected_result.update({"text_search_rewrite": None}) - assert self.get_thread_list([], page=1, num_pages=3).data == expected_result - - expected_result = make_paginated_api_response( - results=[], - count=0, - num_pages=3, - next_link="http://testserver/test_path?page=3", - previous_link="http://testserver/test_path?page=1" - ) - expected_result.update({"text_search_rewrite": None}) - assert self.get_thread_list([], page=2, num_pages=3).data == expected_result - - expected_result = make_paginated_api_response( - results=[], count=0, num_pages=3, next_link=None, previous_link="http://testserver/test_path?page=2" - ) - expected_result.update({"text_search_rewrite": None}) - assert self.get_thread_list([], page=3, num_pages=3).data == expected_result - - # Test page past the last one - self.register_get_threads_response([], page=3, num_pages=3) - with pytest.raises(PageNotFoundError): - get_thread_list(self.request, self.course.id, page=4, page_size=10) - - @ddt.data(None, "rewritten search string") - def test_text_search(self, text_search_rewrite): - expected_result = make_paginated_api_response( - results=[], count=0, num_pages=0, next_link=None, previous_link=None - ) - expected_result.update({"text_search_rewrite": text_search_rewrite}) - self.register_get_threads_search_response([], text_search_rewrite, num_pages=0) - assert get_thread_list( - self.request, - self.course.id, - page=1, - page_size=10, - text_search='test search string' - ).data == expected_result - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["1"], - "per_page": ["10"], - "text": ["test search string"], - }) - - def test_filter_threads_by_author(self): - thread = make_minimal_cs_thread() - self.register_get_threads_response([thread], page=1, num_pages=10) - thread_results = get_thread_list( - self.request, - self.course.id, - page=1, - page_size=10, - author=self.user.username, - ).data.get('results') - assert len(thread_results) == 1 - - expected_last_query_params = { - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["1"], - "per_page": ["10"], - "author_id": [str(self.user.id)], - } - - self.assert_last_query_params(expected_last_query_params) - - def test_filter_threads_by_missing_author(self): - self.register_get_threads_response([make_minimal_cs_thread()], page=1, num_pages=10) - results = get_thread_list( - self.request, - self.course.id, - page=1, - page_size=10, - author="a fake and missing username", - ).data.get('results') - assert len(results) == 0 - - @ddt.data('question', 'discussion', None) - def test_thread_type(self, thread_type): - expected_result = make_paginated_api_response( - results=[], count=0, num_pages=0, next_link=None, previous_link=None - ) - expected_result.update({"text_search_rewrite": None}) - - self.register_get_threads_response([], page=1, num_pages=0) - assert get_thread_list( - self.request, - self.course.id, - page=1, - page_size=10, - thread_type=thread_type, - ).data == expected_result - - expected_last_query_params = { - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["1"], - "per_page": ["10"], - "thread_type": [thread_type], - } - - if thread_type is None: - del expected_last_query_params["thread_type"] - - self.assert_last_query_params(expected_last_query_params) - - @ddt.data(True, False, None) - def test_flagged(self, flagged_boolean): - expected_result = make_paginated_api_response( - results=[], count=0, num_pages=0, next_link=None, previous_link=None - ) - expected_result.update({"text_search_rewrite": None}) - - self.register_get_threads_response([], page=1, num_pages=0) - assert get_thread_list( - self.request, - self.course.id, - page=1, - page_size=10, - flagged=flagged_boolean, - ).data == expected_result - - expected_last_query_params = { - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["1"], - "per_page": ["10"], - "flagged": [str(flagged_boolean)], - } - - if flagged_boolean is None: - del expected_last_query_params["flagged"] - - self.assert_last_query_params(expected_last_query_params) - - @ddt.data( - FORUM_ROLE_ADMINISTRATOR, - FORUM_ROLE_MODERATOR, - FORUM_ROLE_COMMUNITY_TA, - ) - def test_flagged_count(self, role): - expected_result = make_paginated_api_response( - results=[], count=0, num_pages=0, next_link=None, previous_link=None - ) - expected_result.update({"text_search_rewrite": None}) - - _assign_role_to_user(self.user, self.course.id, role=role) - - self.register_get_threads_response([], page=1, num_pages=0) - get_thread_list( - self.request, - self.course.id, - page=1, - page_size=10, - count_flagged=True, - ) - - expected_last_query_params = { - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "count_flagged": ["True"], - "page": ["1"], - "per_page": ["10"], - } - - self.assert_last_query_params(expected_last_query_params) - - def test_flagged_count_denied(self): - expected_result = make_paginated_api_response( - results=[], count=0, num_pages=0, next_link=None, previous_link=None - ) - expected_result.update({"text_search_rewrite": None}) - - _assign_role_to_user(self.user, self.course.id, role=FORUM_ROLE_STUDENT) - - self.register_get_threads_response([], page=1, num_pages=0) - - with pytest.raises(PermissionDenied): - get_thread_list( - self.request, - self.course.id, - page=1, - page_size=10, - count_flagged=True, - ) - - def test_following(self): - self.register_subscribed_threads_response(self.user, [], page=1, num_pages=0) - result = get_thread_list( - self.request, - self.course.id, - page=1, - page_size=11, - following=True, - ).data - - expected_result = make_paginated_api_response( - results=[], count=0, num_pages=0, next_link=None, previous_link=None - ) - expected_result.update({"text_search_rewrite": None}) - assert result == expected_result - assert urlparse( - httpretty.last_request().path # lint-amnesty, pylint: disable=no-member - ).path == f"/api/v1/users/{self.user.id}/subscribed_threads" - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["1"], - "per_page": ["11"], - }) - - @ddt.data("unanswered", "unread") - def test_view_query(self, query): - self.register_get_threads_response([], page=1, num_pages=0) - result = get_thread_list( - self.request, - self.course.id, - page=1, - page_size=11, - view=query, - ).data - - expected_result = make_paginated_api_response( - results=[], count=0, num_pages=0, next_link=None, previous_link=None - ) - expected_result.update({"text_search_rewrite": None}) - assert result == expected_result - assert urlparse(httpretty.last_request().path).path == '/api/v1/threads' # lint-amnesty, pylint: disable=no-member - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["1"], - "per_page": ["11"], - query: ["true"], - }) - - @ddt.data( - ("last_activity_at", "activity"), - ("comment_count", "comments"), - ("vote_count", "votes") - ) - @ddt.unpack - def test_order_by_query(self, http_query, cc_query): - """ - Tests the order_by parameter - - Arguments: - http_query (str): Query string sent in the http request - cc_query (str): Query string used for the comments client service - """ - self.register_get_threads_response([], page=1, num_pages=0) - result = get_thread_list( - self.request, - self.course.id, - page=1, - page_size=11, - order_by=http_query, - ).data - - expected_result = make_paginated_api_response( - results=[], count=0, num_pages=0, next_link=None, previous_link=None - ) - expected_result.update({"text_search_rewrite": None}) - assert result == expected_result - assert urlparse(httpretty.last_request().path).path == '/api/v1/threads' # lint-amnesty, pylint: disable=no-member - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": [cc_query], - "page": ["1"], - "per_page": ["11"], - }) - - def test_order_direction(self): - """ - Only "desc" is supported for order. Also, since it is simply swallowed, - it isn't included in the params. - """ - self.register_get_threads_response([], page=1, num_pages=0) - result = get_thread_list( - self.request, - self.course.id, - page=1, - page_size=11, - order_direction="desc", - ).data - - expected_result = make_paginated_api_response( - results=[], count=0, num_pages=0, next_link=None, previous_link=None - ) - expected_result.update({"text_search_rewrite": None}) - assert result == expected_result - assert urlparse(httpretty.last_request().path).path == '/api/v1/threads' # lint-amnesty, pylint: disable=no-member - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["1"], - "per_page": ["11"], - }) - - def test_invalid_order_direction(self): - """ - Test with invalid order_direction (e.g. "asc") - """ - with pytest.raises(ValidationError) as assertion: - self.register_get_threads_response([], page=1, num_pages=0) - get_thread_list( # pylint: disable=expression-not-assigned - self.request, - self.course.id, - page=1, - page_size=11, - order_direction="asc", - ).data - assert 'order_direction' in assertion.value.message_dict - - @ddt.ddt @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModuleStoreTestCase): diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py b/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py index 4804c73a06..4efadd6385 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py @@ -92,6 +92,7 @@ from openedx.core.djangoapps.discussions.tasks import ( from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_GROUP_MODERATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_STUDENT, Role, @@ -1054,3 +1055,639 @@ class UpdateCommentTest( vote_count -= 1 assert result["vote_count"] == vote_count self.register_get_user_response(self.user, upvoted_ids=[]) + + +@ddt.ddt +@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) +class GetThreadListTest( + ForumsEnableMixin, ForumMockUtilsMixin, UrlResetMixin, SharedModuleStoreTestCase +): + """Test for get_thread_list""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + super().setUpClassAndForumMock() + cls.course = CourseFactory.create() + + @classmethod + def tearDownClass(cls): + """Stop patches after tests complete.""" + super().tearDownClass() + super().disposeForumMocks() + + @mock.patch.dict( + "django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True} + ) + def setUp(self): + super().setUp() + httpretty.reset() + httpretty.enable() + self.addCleanup(httpretty.reset) + self.addCleanup(httpretty.disable) + self.maxDiff = None # pylint: disable=invalid-name + self.user = UserFactory.create() + self.register_get_user_response(self.user) + self.request = RequestFactory().get("/test_path") + self.request.user = self.user + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id) + self.author = UserFactory.create() + self.course.cohort_config = {"cohorted": False} + modulestore().update_item(self.course, ModuleStoreEnum.UserID.test) + self.cohort = CohortFactory.create(course_id=self.course.id) + + def get_thread_list( + self, + threads, + page=1, + page_size=1, + num_pages=1, + course=None, + topic_id_list=None, + ): + """ + Register the appropriate comments service response, then call + get_thread_list and return the result. + """ + course = course or self.course + self.register_get_threads_response(threads, page, num_pages) + ret = get_thread_list(self.request, course.id, page, page_size, topic_id_list) + return ret + + def test_nonexistent_course(self): + with pytest.raises(CourseNotFoundError): + get_thread_list( + self.request, + CourseLocator.from_string("course-v1:non+existent+course"), + 1, + 1, + ) + + def test_not_enrolled(self): + self.request.user = UserFactory.create() + with pytest.raises(CourseNotFoundError): + self.get_thread_list([]) + + def test_discussions_disabled(self): + with pytest.raises(DiscussionDisabledError): + self.get_thread_list([], course=_discussion_disabled_course_for(self.user)) + + def test_empty(self): + assert self.get_thread_list([], num_pages=0).data == { + "pagination": {"next": None, "previous": None, "num_pages": 0, "count": 0}, + "results": [], + "text_search_rewrite": None, + } + + def test_get_threads_by_topic_id(self): + self.get_thread_list([], topic_id_list=["topic_x", "topic_meow"]) + self.check_mock_called("get_user_threads") + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 1, + "per_page": 1, + "commentable_ids": ["topic_x", "topic_meow"], + } + self.check_mock_called_with( + "get_user_threads", + -1, + **params, + ) + + def test_basic_query_params(self): + self.get_thread_list([], page=6, page_size=14) + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 6, + "per_page": 14, + } + self.check_mock_called_with( + "get_user_threads", + -1, + **params, + ) + + def test_thread_content(self): + self.course.cohort_config = {"cohorted": True} + modulestore().update_item(self.course, ModuleStoreEnum.UserID.test) + source_threads = [ + make_minimal_cs_thread( + { + "id": "test_thread_id_0", + "course_id": str(self.course.id), + "commentable_id": "topic_x", + "username": self.author.username, + "user_id": str(self.author.id), + "title": "Test Title", + "body": "Test body", + "votes": {"up_count": 4}, + "comments_count": 5, + "unread_comments_count": 3, + "endorsed": True, + "read": True, + "created_at": "2015-04-28T00:00:00Z", + "updated_at": "2015-04-28T11:11:11Z", + } + ), + make_minimal_cs_thread( + { + "id": "test_thread_id_1", + "course_id": str(self.course.id), + "commentable_id": "topic_y", + "group_id": self.cohort.id, + "username": self.author.username, + "user_id": str(self.author.id), + "thread_type": "question", + "title": "Another Test Title", + "body": "More content", + "votes": {"up_count": 9}, + "comments_count": 18, + "created_at": "2015-04-28T22:22:22Z", + "updated_at": "2015-04-28T00:33:33Z", + } + ), + ] + expected_threads = [ + self.expected_thread_data( + { + "id": "test_thread_id_0", + "author": self.author.username, + "topic_id": "topic_x", + "vote_count": 4, + "comment_count": 6, + "unread_comment_count": 3, + "comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_thread_id_0", + "editable_fields": [ + "abuse_flagged", + "copy_link", + "following", + "read", + "voted", + ], + "has_endorsed": True, + "read": True, + "created_at": "2015-04-28T00:00:00Z", + "updated_at": "2015-04-28T11:11:11Z", + "abuse_flagged_count": None, + "can_delete": False, + } + ), + self.expected_thread_data( + { + "id": "test_thread_id_1", + "author": self.author.username, + "topic_id": "topic_y", + "group_id": self.cohort.id, + "group_name": self.cohort.name, + "type": "question", + "title": "Another Test Title", + "raw_body": "More content", + "preview_body": "More content", + "rendered_body": "

More content

", + "vote_count": 9, + "comment_count": 19, + "created_at": "2015-04-28T22:22:22Z", + "updated_at": "2015-04-28T00:33:33Z", + "comment_list_url": None, + "endorsed_comment_list_url": ( + "http://testserver/api/discussion/v1/comments/?thread_id=test_thread_id_1&endorsed=True" + ), + "non_endorsed_comment_list_url": ( + "http://testserver/api/discussion/v1/comments/?thread_id=test_thread_id_1&endorsed=False" + ), + "editable_fields": [ + "abuse_flagged", + "copy_link", + "following", + "read", + "voted", + ], + "abuse_flagged_count": None, + "can_delete": False, + } + ), + ] + + expected_result = make_paginated_api_response( + results=expected_threads, + count=2, + num_pages=1, + next_link=None, + previous_link=None, + ) + expected_result.update({"text_search_rewrite": None}) + assert self.get_thread_list(source_threads).data == expected_result + + @ddt.data( + *itertools.product( + [ + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_MODERATOR, + FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_STUDENT, + ], + [True, False], + ) + ) + @ddt.unpack + def test_request_group(self, role_name, course_is_cohorted): + cohort_course = CourseFactory.create( + cohort_config={"cohorted": course_is_cohorted} + ) + CourseEnrollmentFactory.create(user=self.user, course_id=cohort_course.id) + CohortFactory.create(course_id=cohort_course.id, users=[self.user]) + _assign_role_to_user(user=self.user, course_id=cohort_course.id, role=role_name) + self.get_thread_list([], course=cohort_course) + thread_func_params = self.get_mock_func_calls("get_user_threads")[-1][1] + actual_has_group = "group_id" in thread_func_params + expected_has_group = ( + course_is_cohorted and role_name in ( + FORUM_ROLE_STUDENT, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_GROUP_MODERATOR + ) + ) + assert actual_has_group == expected_has_group + + def test_pagination(self): + # N.B. Empty thread list is not realistic but convenient for this test + expected_result = make_paginated_api_response( + results=[], + count=0, + num_pages=3, + next_link="http://testserver/test_path?page=2", + previous_link=None, + ) + expected_result.update({"text_search_rewrite": None}) + assert self.get_thread_list([], page=1, num_pages=3).data == expected_result + + expected_result = make_paginated_api_response( + results=[], + count=0, + num_pages=3, + next_link="http://testserver/test_path?page=3", + previous_link="http://testserver/test_path?page=1", + ) + expected_result.update({"text_search_rewrite": None}) + assert self.get_thread_list([], page=2, num_pages=3).data == expected_result + + expected_result = make_paginated_api_response( + results=[], + count=0, + num_pages=3, + next_link=None, + previous_link="http://testserver/test_path?page=2", + ) + expected_result.update({"text_search_rewrite": None}) + assert self.get_thread_list([], page=3, num_pages=3).data == expected_result + + # Test page past the last one + self.register_get_threads_response([], page=3, num_pages=3) + with pytest.raises(PageNotFoundError): + get_thread_list(self.request, self.course.id, page=4, page_size=10) + + @ddt.data(None, "rewritten search string") + def test_text_search(self, text_search_rewrite): + expected_result = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_result.update({"text_search_rewrite": text_search_rewrite}) + self.register_get_threads_search_response([], text_search_rewrite, num_pages=0) + assert ( + get_thread_list( + self.request, + self.course.id, + page=1, + page_size=10, + text_search="test search string", + ).data + == expected_result + ) + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 1, + "per_page": 10, + "text": "test search string", + } + self.check_mock_called_with( + "search_threads", + -1, + **params, + ) + + def test_filter_threads_by_author(self): + thread = make_minimal_cs_thread() + self.register_get_threads_response([thread], page=1, num_pages=10) + thread_results = get_thread_list( + self.request, + self.course.id, + page=1, + page_size=10, + author=self.user.username, + ).data.get("results") + assert len(thread_results) == 1 + + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 1, + "per_page": 10, + "author_id": str(self.user.id), + } + self.check_mock_called_with( + "get_user_threads", + -1, + **params, + ) + + def test_filter_threads_by_missing_author(self): + self.register_get_threads_response( + [make_minimal_cs_thread()], page=1, num_pages=10 + ) + results = get_thread_list( + self.request, + self.course.id, + page=1, + page_size=10, + author="a fake and missing username", + ).data.get("results") + assert len(results) == 0 + + @ddt.data("question", "discussion", None) + def test_thread_type(self, thread_type): + expected_result = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_result.update({"text_search_rewrite": None}) + + self.register_get_threads_response([], page=1, num_pages=0) + assert ( + get_thread_list( + self.request, + self.course.id, + page=1, + page_size=10, + thread_type=thread_type, + ).data + == expected_result + ) + + expected_last_query_params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 1, + "per_page": 10, + "thread_type": thread_type, + } + + if thread_type is None: + del expected_last_query_params["thread_type"] + + self.check_mock_called_with( + "get_user_threads", + -1, + **expected_last_query_params, + ) + + @ddt.data(True, False, None) + def test_flagged(self, flagged_boolean): + expected_result = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_result.update({"text_search_rewrite": None}) + + self.register_get_threads_response([], page=1, num_pages=0) + assert ( + get_thread_list( + self.request, + self.course.id, + page=1, + page_size=10, + flagged=flagged_boolean, + ).data + == expected_result + ) + + expected_last_query_params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 1, + "per_page": 10, + "flagged": flagged_boolean, + } + + if flagged_boolean is None: + del expected_last_query_params["flagged"] + + self.check_mock_called_with( + "get_user_threads", + -1, + **expected_last_query_params, + ) + + @ddt.data( + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_MODERATOR, + FORUM_ROLE_COMMUNITY_TA, + ) + def test_flagged_count(self, role): + expected_result = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_result.update({"text_search_rewrite": None}) + + _assign_role_to_user(self.user, self.course.id, role=role) + + self.register_get_threads_response([], page=1, num_pages=0) + get_thread_list( + self.request, + self.course.id, + page=1, + page_size=10, + count_flagged=True, + ) + + expected_last_query_params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "count_flagged": True, + "page": 1, + "per_page": 10, + } + + self.check_mock_called_with( + "get_user_threads", -1, **expected_last_query_params + ) + + def test_flagged_count_denied(self): + expected_result = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_result.update({"text_search_rewrite": None}) + + _assign_role_to_user(self.user, self.course.id, role=FORUM_ROLE_STUDENT) + + self.register_get_threads_response([], page=1, num_pages=0) + + with pytest.raises(PermissionDenied): + get_thread_list( + self.request, + self.course.id, + page=1, + page_size=10, + count_flagged=True, + ) + + def test_following(self): + self.register_subscribed_threads_response(self.user, [], page=1, num_pages=0) + result = get_thread_list( + self.request, + self.course.id, + page=1, + page_size=11, + following=True, + ).data + + expected_result = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_result.update({"text_search_rewrite": None}) + assert result == expected_result + self.check_mock_called("get_user_subscriptions") + + params = { + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 1, + "per_page": 11, + } + self.check_mock_called_with( + "get_user_subscriptions", -1, str(self.user.id), str(self.course.id), params + ) + + @ddt.data("unanswered", "unread") + def test_view_query(self, query): + self.register_get_threads_response([], page=1, num_pages=0) + result = get_thread_list( + self.request, + self.course.id, + page=1, + page_size=11, + view=query, + ).data + + expected_result = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_result.update({"text_search_rewrite": None}) + assert result == expected_result + self.check_mock_called("get_user_threads") + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 1, + "per_page": 11, + query: True, + } + self.check_mock_called_with( + "get_user_threads", + -1, + **params, + ) + + @ddt.data( + ("last_activity_at", "activity"), + ("comment_count", "comments"), + ("vote_count", "votes"), + ) + @ddt.unpack + def test_order_by_query(self, http_query, cc_query): + """ + Tests the order_by parameter + + Arguments: + http_query (str): Query string sent in the http request + cc_query (str): Query string used for the comments client service + """ + self.register_get_threads_response([], page=1, num_pages=0) + result = get_thread_list( + self.request, + self.course.id, + page=1, + page_size=11, + order_by=http_query, + ).data + + expected_result = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_result.update({"text_search_rewrite": None}) + assert result == expected_result + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": cc_query, + "page": 1, + "per_page": 11, + } + self.check_mock_called_with( + "get_user_threads", + -1, + **params, + ) + + def test_order_direction(self): + """ + Only "desc" is supported for order. Also, since it is simply swallowed, + it isn't included in the params. + """ + self.register_get_threads_response([], page=1, num_pages=0) + result = get_thread_list( + self.request, + self.course.id, + page=1, + page_size=11, + order_direction="desc", + ).data + + expected_result = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_result.update({"text_search_rewrite": None}) + assert result == expected_result + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 1, + "per_page": 11, + } + self.check_mock_called_with( + "get_user_threads", + -1, + **params, + ) + + def test_invalid_order_direction(self): + """ + Test with invalid order_direction (e.g. "asc") + """ + with pytest.raises(ValidationError) as assertion: + self.register_get_threads_response([], page=1, num_pages=0) + get_thread_list( # pylint: disable=expression-not-assigned + self.request, + self.course.id, + page=1, + page_size=11, + order_direction="asc", + ).data + assert "order_direction" in assertion.value.message_dict diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index 84efa95378..1df43ee4f9 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -1056,354 +1056,6 @@ class CourseTopicsViewV3Test(DiscussionAPIViewTestMixin, CommentsServiceMockMixi assert vertical_keys == expected_non_courseware_keys -@ddt.ddt -@httpretty.activate -@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) -class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, ProfileImageTestMixin): - """Tests for ThreadViewSet list""" - - def setUp(self): - super().setUp() - self.author = UserFactory.create() - self.url = reverse("thread-list") - patcher = mock.patch( - 'openedx.core.djangoapps.discussions.config.waffle.ENABLE_FORUM_V2.is_enabled', - return_value=False - ) - patcher.start() - self.addCleanup(patcher.stop) - - def create_source_thread(self, overrides=None): - """ - Create a sample source cs_thread - """ - thread = make_minimal_cs_thread({ - "id": "test_thread", - "course_id": str(self.course.id), - "commentable_id": "test_topic", - "user_id": str(self.user.id), - "username": self.user.username, - "created_at": "2015-04-28T00:00:00Z", - "updated_at": "2015-04-28T11:11:11Z", - "title": "Test Title", - "body": "Test body", - "votes": {"up_count": 4}, - "comments_count": 5, - "unread_comments_count": 3, - }) - - thread.update(overrides or {}) - return thread - - def test_course_id_missing(self): - response = self.client.get(self.url) - self.assert_response_correct( - response, - 400, - {"field_errors": {"course_id": {"developer_message": "This field is required."}}} - ) - - def test_404(self): - response = self.client.get(self.url, {"course_id": "non/existent/course"}) - self.assert_response_correct( - response, - 404, - {"developer_message": "Course not found."} - ) - - def test_basic(self): - self.register_get_user_response(self.user, upvoted_ids=["test_thread"]) - source_threads = [ - self.create_source_thread({"user_id": str(self.author.id), "username": self.author.username}) - ] - expected_threads = [self.expected_thread_data({ - "created_at": "2015-04-28T00:00:00Z", - "updated_at": "2015-04-28T11:11:11Z", - "vote_count": 4, - "comment_count": 6, - "can_delete": False, - "unread_comment_count": 3, - "voted": True, - "author": self.author.username, - "editable_fields": ["abuse_flagged", "copy_link", "following", "read", "voted"], - "abuse_flagged_count": None, - })] - self.register_get_threads_response(source_threads, page=1, num_pages=2) - response = self.client.get(self.url, {"course_id": str(self.course.id), "following": ""}) - expected_response = make_paginated_api_response( - results=expected_threads, - count=1, - num_pages=2, - next_link="http://testserver/api/discussion/v1/threads/?course_id=course-v1%3Ax%2By%2Bz&following=&page=2", - previous_link=None - ) - expected_response.update({"text_search_rewrite": None}) - self.assert_response_correct( - response, - 200, - expected_response - ) - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["1"], - "per_page": ["10"], - }) - - @ddt.data("unread", "unanswered", "unresponded") - def test_view_query(self, query): - threads = [make_minimal_cs_thread()] - self.register_get_user_response(self.user) - self.register_get_threads_response(threads, page=1, num_pages=1) - self.client.get( - self.url, - { - "course_id": str(self.course.id), - "view": query, - } - ) - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["1"], - "per_page": ["10"], - query: ["true"], - }) - - def test_pagination(self): - self.register_get_user_response(self.user) - self.register_get_threads_response([], page=1, num_pages=1) - response = self.client.get( - self.url, - {"course_id": str(self.course.id), "page": "18", "page_size": "4"} - ) - self.assert_response_correct( - response, - 404, - {"developer_message": "Page not found (No results on this page)."} - ) - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["18"], - "per_page": ["4"], - }) - - def test_text_search(self): - self.register_get_user_response(self.user) - self.register_get_threads_search_response([], None, num_pages=0) - response = self.client.get( - self.url, - {"course_id": str(self.course.id), "text_search": "test search string"} - ) - - expected_response = make_paginated_api_response( - results=[], count=0, num_pages=0, next_link=None, previous_link=None - ) - expected_response.update({"text_search_rewrite": None}) - self.assert_response_correct( - response, - 200, - expected_response - ) - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["1"], - "per_page": ["10"], - "text": ["test search string"], - }) - - @ddt.data(True, "true", "1") - def test_following_true(self, following): - self.register_get_user_response(self.user) - self.register_subscribed_threads_response(self.user, [], page=1, num_pages=0) - response = self.client.get( - self.url, - { - "course_id": str(self.course.id), - "following": following, - } - ) - - expected_response = make_paginated_api_response( - results=[], count=0, num_pages=0, next_link=None, previous_link=None - ) - expected_response.update({"text_search_rewrite": None}) - self.assert_response_correct( - response, - 200, - expected_response - ) - assert urlparse( - httpretty.last_request().path # lint-amnesty, pylint: disable=no-member - ).path == f"/api/v1/users/{self.user.id}/subscribed_threads" - - @ddt.data(False, "false", "0") - def test_following_false(self, following): - response = self.client.get( - self.url, - { - "course_id": str(self.course.id), - "following": following, - } - ) - self.assert_response_correct( - response, - 400, - {"field_errors": { - "following": {"developer_message": "The value of the 'following' parameter must be true."} - }} - ) - - def test_following_error(self): - response = self.client.get( - self.url, - { - "course_id": str(self.course.id), - "following": "invalid-boolean", - } - ) - self.assert_response_correct( - response, - 400, - {"field_errors": { - "following": {"developer_message": "Invalid Boolean Value."} - }} - ) - - @ddt.data( - ("last_activity_at", "activity"), - ("comment_count", "comments"), - ("vote_count", "votes") - ) - @ddt.unpack - def test_order_by(self, http_query, cc_query): - """ - Tests the order_by parameter - - Arguments: - http_query (str): Query string sent in the http request - cc_query (str): Query string used for the comments client service - """ - threads = [make_minimal_cs_thread()] - self.register_get_user_response(self.user) - self.register_get_threads_response(threads, page=1, num_pages=1) - self.client.get( - self.url, - { - "course_id": str(self.course.id), - "order_by": http_query, - } - ) - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "page": ["1"], - "per_page": ["10"], - "sort_key": [cc_query], - }) - - def test_order_direction(self): - """ - Test order direction, of which "desc" is the only valid option. The - option actually just gets swallowed, so it doesn't affect the params. - """ - threads = [make_minimal_cs_thread()] - self.register_get_user_response(self.user) - self.register_get_threads_response(threads, page=1, num_pages=1) - self.client.get( - self.url, - { - "course_id": str(self.course.id), - "order_direction": "desc", - } - ) - self.assert_last_query_params({ - "user_id": [str(self.user.id)], - "course_id": [str(self.course.id)], - "sort_key": ["activity"], - "page": ["1"], - "per_page": ["10"], - }) - - def test_mutually_exclusive(self): - """ - Tests GET thread_list api does not allow filtering on mutually exclusive parameters - """ - self.register_get_user_response(self.user) - self.register_get_threads_search_response([], None, num_pages=0) - response = self.client.get(self.url, { - "course_id": str(self.course.id), - "text_search": "test search string", - "topic_id": "topic1, topic2", - }) - self.assert_response_correct( - response, - 400, - { - "developer_message": "The following query parameters are mutually exclusive: topic_id, " - "text_search, following" - } - ) - - def test_profile_image_requested_field(self): - """ - Tests thread has user profile image details if called in requested_fields - """ - user_2 = UserFactory.create(password=self.password) - # Ensure that parental controls don't apply to this user - user_2.profile.year_of_birth = 1970 - user_2.profile.save() - source_threads = [ - self.create_source_thread(), - self.create_source_thread({"user_id": str(user_2.id), "username": user_2.username}), - ] - - self.register_get_user_response(self.user, upvoted_ids=["test_thread"]) - self.register_get_threads_response(source_threads, page=1, num_pages=1) - self.create_profile_image(self.user, get_profile_image_storage()) - self.create_profile_image(user_2, get_profile_image_storage()) - - response = self.client.get( - self.url, - {"course_id": str(self.course.id), "requested_fields": "profile_image"}, - ) - assert response.status_code == 200 - response_threads = json.loads(response.content.decode('utf-8'))['results'] - - for response_thread in response_threads: - expected_profile_data = self.get_expected_user_profile(response_thread['author']) - response_users = response_thread['users'] - assert expected_profile_data == response_users[response_thread['author']] - - def test_profile_image_requested_field_anonymous_user(self): - """ - Tests profile_image in requested_fields for thread created with anonymous user - """ - source_threads = [ - self.create_source_thread( - {"user_id": None, "username": None, "anonymous": True, "anonymous_to_peers": True} - ), - ] - - self.register_get_user_response(self.user, upvoted_ids=["test_thread"]) - self.register_get_threads_response(source_threads, page=1, num_pages=1) - - response = self.client.get( - self.url, - {"course_id": str(self.course.id), "requested_fields": "profile_image"}, - ) - assert response.status_code == 200 - response_thread = json.loads(response.content.decode('utf-8'))['results'][0] - assert response_thread['author'] is None - assert {} == response_thread['users'] - - @httpretty.activate @disable_signal(api, 'thread_created') @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views_v2.py b/lms/djangoapps/discussion/rest_api/tests/test_views_v2.py index 29e469c9a9..9c41c11b24 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views_v2.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views_v2.py @@ -21,7 +21,9 @@ from django.core.files.uploadedfile import SimpleUploadedFile from django.test import override_settings from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag -from lms.djangoapps.discussion.django_comment_client.tests.mixins import MockForumApiMixin +from lms.djangoapps.discussion.django_comment_client.tests.mixins import ( + MockForumApiMixin, +) from opaque_keys.edx.keys import CourseKey from pytz import UTC from rest_framework import status @@ -32,18 +34,32 @@ from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE from lms.djangoapps.discussion.rest_api.utils import get_usernames_from_search_string from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory, check_mongo_calls +from xmodule.modulestore.tests.django_utils import ( + ModuleStoreTestCase, + SharedModuleStoreTestCase, +) +from xmodule.modulestore.tests.factories import ( + CourseFactory, + BlockFactory, + check_mongo_calls, +) from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.course_modes.tests.factories import CourseModeFactory -from common.djangoapps.student.models import get_retired_username_by_username, CourseEnrollment -from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff +from common.djangoapps.student.models import ( + get_retired_username_by_username, + CourseEnrollment, +) +from common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseStaffRole, + GlobalStaff, +) from common.djangoapps.student.tests.factories import ( AdminFactory, CourseEnrollmentFactory, SuperuserFactory, - UserFactory + UserFactory, ) from common.djangoapps.util.testing import PatchMediaTypeMixin, UrlResetMixin from common.test.utils import disable_signal @@ -65,15 +81,34 @@ from lms.djangoapps.discussion.rest_api.tests.utils import ( parsed_body, ) from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts -from openedx.core.djangoapps.discussions.config.waffle import ENABLE_NEW_STRUCTURE_DISCUSSIONS -from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, DiscussionTopicLink, Provider -from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course_task -from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings, Role +from openedx.core.djangoapps.discussions.config.waffle import ( + ENABLE_NEW_STRUCTURE_DISCUSSIONS, +) +from openedx.core.djangoapps.discussions.models import ( + DiscussionsConfiguration, + DiscussionTopicLink, + Provider, +) +from openedx.core.djangoapps.discussions.tasks import ( + update_discussions_settings_from_course_task, +) +from openedx.core.djangoapps.django_comment_common.models import ( + CourseDiscussionSettings, + Role, +) from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user -from openedx.core.djangoapps.oauth_dispatch.tests.factories import AccessTokenFactory, ApplicationFactory -from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_storage -from openedx.core.djangoapps.user_api.models import RetirementState, UserRetirementStatus +from openedx.core.djangoapps.oauth_dispatch.tests.factories import ( + AccessTokenFactory, + ApplicationFactory, +) +from openedx.core.djangoapps.user_api.accounts.image_helpers import ( + get_profile_image_storage, +) +from openedx.core.djangoapps.user_api.models import ( + RetirementState, + UserRetirementStatus, +) class DiscussionAPIViewTestMixin(ForumsEnableMixin, ForumMockUtilsMixin, UrlResetMixin): @@ -86,7 +121,9 @@ class DiscussionAPIViewTestMixin(ForumsEnableMixin, ForumMockUtilsMixin, UrlRese client_class = APIClient - @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + @mock.patch.dict( + "django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True} + ) def setUp(self): super().setUp() self.maxDiff = None # pylint: disable=invalid-name @@ -95,7 +132,7 @@ class DiscussionAPIViewTestMixin(ForumsEnableMixin, ForumMockUtilsMixin, UrlRese course="y", run="z", start=datetime.now(UTC), - discussion_topics={"Test Topic": {"id": "test_topic"}} + discussion_topics={"Test Topic": {"id": "test_topic"}}, ) self.password = "Password1234" self.user = UserFactory.create(password=self.password) @@ -120,23 +157,25 @@ class DiscussionAPIViewTestMixin(ForumsEnableMixin, ForumMockUtilsMixin, UrlRese Assert that the response has the given status code and parsed content """ assert response.status_code == expected_status - parsed_content = json.loads(response.content.decode('utf-8')) + parsed_content = json.loads(response.content.decode("utf-8")) assert parsed_content == expected_content def register_thread(self, overrides=None): """ Create cs_thread with minimal fields and register response """ - cs_thread = make_minimal_cs_thread({ - "id": "test_thread", - "course_id": str(self.course.id), - "commentable_id": "test_topic", - "username": self.user.username, - "user_id": str(self.user.id), - "thread_type": "discussion", - "title": "Test Title", - "body": "Test body", - }) + cs_thread = make_minimal_cs_thread( + { + "id": "test_thread", + "course_id": str(self.course.id), + "commentable_id": "test_topic", + "username": self.user.username, + "user_id": str(self.user.id), + "thread_type": "discussion", + "title": "Test Title", + "body": "Test body", + } + ) cs_thread.update(overrides or {}) self.register_get_thread_response(cs_thread) self.register_put_thread_response(cs_thread) @@ -145,14 +184,16 @@ class DiscussionAPIViewTestMixin(ForumsEnableMixin, ForumMockUtilsMixin, UrlRese """ Create cs_comment with minimal fields and register response """ - cs_comment = make_minimal_cs_comment({ - "id": "test_comment", - "course_id": str(self.course.id), - "thread_id": "test_thread", - "username": self.user.username, - "user_id": str(self.user.id), - "body": "Original body", - }) + cs_comment = make_minimal_cs_comment( + { + "id": "test_comment", + "course_id": str(self.course.id), + "thread_id": "test_thread", + "username": self.user.username, + "user_id": str(self.user.id), + "body": "Original body", + } + ) cs_comment.update(overrides or {}) self.register_get_comment_response(cs_comment) self.register_put_comment_response(cs_comment) @@ -164,7 +205,7 @@ class DiscussionAPIViewTestMixin(ForumsEnableMixin, ForumMockUtilsMixin, UrlRese self.assert_response_correct( response, 401, - {"developer_message": "Authentication credentials were not provided."} + {"developer_message": "Authentication credentials were not provided."}, ) def test_inactive(self): @@ -174,9 +215,11 @@ class DiscussionAPIViewTestMixin(ForumsEnableMixin, ForumMockUtilsMixin, UrlRese @ddt.ddt @httpretty.activate -@disable_signal(api, 'thread_edited') +@disable_signal(api, "thread_edited") @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) -class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, PatchMediaTypeMixin): +class ThreadViewSetPartialUpdateTest( + DiscussionAPIViewTestMixin, ModuleStoreTestCase, PatchMediaTypeMixin +): """Tests for ThreadViewSet partial_update""" def setUp(self): @@ -186,47 +229,58 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest def test_basic(self): self.register_get_user_response(self.user) - self.register_thread({ - "created_at": "Test Created Date", - "updated_at": "Test Updated Date", - "read": True, - "resp_total": 2, - }) + self.register_thread( + { + "created_at": "Test Created Date", + "updated_at": "Test Updated Date", + "read": True, + "resp_total": 2, + } + ) request_data = {"raw_body": "Edited body"} response = self.request_patch(request_data) assert response.status_code == 200 - response_data = json.loads(response.content.decode('utf-8')) - assert response_data == self.expected_thread_data({ - 'raw_body': 'Edited body', - 'rendered_body': '

Edited body

', - 'preview_body': 'Edited body', - 'editable_fields': [ - 'abuse_flagged', 'anonymous', 'copy_link', 'following', 'raw_body', 'read', - 'title', 'topic_id', 'type' - ], - 'created_at': 'Test Created Date', - 'updated_at': 'Test Updated Date', - 'comment_count': 1, - 'read': True, - 'response_count': 2, - }) + response_data = json.loads(response.content.decode("utf-8")) + assert response_data == self.expected_thread_data( + { + "raw_body": "Edited body", + "rendered_body": "

Edited body

", + "preview_body": "Edited body", + "editable_fields": [ + "abuse_flagged", + "anonymous", + "copy_link", + "following", + "raw_body", + "read", + "title", + "topic_id", + "type", + ], + "created_at": "Test Created Date", + "updated_at": "Test Updated Date", + "comment_count": 1, + "read": True, + "response_count": 2, + } + ) params = { - 'thread_id': 'test_thread', - 'course_id': str(self.course.id), - 'commentable_id': 'test_topic', - 'thread_type': 'discussion', - 'title': 'Test Title', - 'body': 'Edited body', - 'user_id': str(self.user.id), - 'anonymous': False, - 'anonymous_to_peers': False, - 'closed': False, - 'pinned': False, - 'read': True, - 'editing_user_id': str(self.user.id), + "thread_id": "test_thread", + "course_id": str(self.course.id), + "commentable_id": "test_topic", + "thread_type": "discussion", + "title": "Test Title", + "body": "Edited body", + "user_id": str(self.user.id), + "anonymous": False, + "anonymous_to_peers": False, + "closed": False, + "pinned": False, + "read": True, + "editing_user_id": str(self.user.id), } - self.check_mock_called_with('update_thread', -1, **params) + self.check_mock_called_with("update_thread", -1, **params) def test_error(self): self.register_get_user_response(self.user) @@ -234,10 +288,12 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest request_data = {"title": ""} response = self.request_patch(request_data) expected_response_data = { - "field_errors": {"title": {"developer_message": "This field may not be blank."}} + "field_errors": { + "title": {"developer_message": "This field may not be blank."} + } } assert response.status_code == 400 - response_data = json.loads(response.content.decode('utf-8')) + response_data = json.loads(response.content.decode("utf-8")) assert response_data == expected_response_data @ddt.data( @@ -252,14 +308,17 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest request_data = {field: value} response = self.request_patch(request_data) assert response.status_code == 200 - response_data = json.loads(response.content.decode('utf-8')) - assert response_data == self.expected_thread_data({ - 'read': True, - 'closed': True, - 'abuse_flagged': value, - 'editable_fields': ['abuse_flagged', 'copy_link', 'read'], - 'comment_count': 1, 'unread_comment_count': 0 - }) + response_data = json.loads(response.content.decode("utf-8")) + assert response_data == self.expected_thread_data( + { + "read": True, + "closed": True, + "abuse_flagged": value, + "editable_fields": ["abuse_flagged", "copy_link", "read"], + "comment_count": 1, + "unread_comment_count": 0, + } + ) @ddt.data( ("raw_body", "Edited body"), @@ -283,47 +342,68 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest response = self.request_patch(request_data) assert response.status_code == 200 - response_data = json.loads(response.content.decode('utf-8')) - assert response_data == self.expected_thread_data({ - 'comment_count': 1, - 'read': True, - 'editable_fields': [ - 'abuse_flagged', 'anonymous', 'copy_link', 'following', 'raw_body', 'read', - 'title', 'topic_id', 'type' - ], - 'response_count': 2 - }) + response_data = json.loads(response.content.decode("utf-8")) + assert response_data == self.expected_thread_data( + { + "comment_count": 1, + "read": True, + "editable_fields": [ + "abuse_flagged", + "anonymous", + "copy_link", + "following", + "raw_body", + "read", + "title", + "topic_id", + "type", + ], + "response_count": 2, + } + ) def test_patch_read_non_owner_user(self): self.register_get_user_response(self.user) thread_owner_user = UserFactory.create(password=self.password) CourseEnrollmentFactory.create(user=thread_owner_user, course_id=self.course.id) - self.register_thread({ - "username": thread_owner_user.username, - "user_id": str(thread_owner_user.id), - "resp_total": 2, - }) + self.register_thread( + { + "username": thread_owner_user.username, + "user_id": str(thread_owner_user.id), + "resp_total": 2, + } + ) self.register_read_response(self.user, "thread", "test_thread") request_data = {"read": True} response = self.request_patch(request_data) assert response.status_code == 200 - response_data = json.loads(response.content.decode('utf-8')) - expected_data = self.expected_thread_data({ - 'author': str(thread_owner_user.username), - 'comment_count': 1, - 'can_delete': False, - 'read': True, - 'editable_fields': ['abuse_flagged', 'copy_link', 'following', 'read', 'voted'], - 'response_count': 2 - }) + response_data = json.loads(response.content.decode("utf-8")) + expected_data = self.expected_thread_data( + { + "author": str(thread_owner_user.username), + "comment_count": 1, + "can_delete": False, + "read": True, + "editable_fields": [ + "abuse_flagged", + "copy_link", + "following", + "read", + "voted", + ], + "response_count": 2, + } + ) assert response_data == expected_data @ddt.ddt -@disable_signal(api, 'comment_edited') +@disable_signal(api, "comment_edited") @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) -class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, PatchMediaTypeMixin): +class CommentViewSetPartialUpdateTest( + DiscussionAPIViewTestMixin, ModuleStoreTestCase, PatchMediaTypeMixin +): """Tests for CommentViewSet partial_update""" def setUp(self): @@ -375,29 +455,33 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes def test_basic(self): self.register_thread() - self.register_comment({"created_at": "Test Created Date", "updated_at": "Test Updated Date"}) + self.register_comment( + {"created_at": "Test Created Date", "updated_at": "Test Updated Date"} + ) request_data = {"raw_body": "Edited body"} response = self.request_patch(request_data) assert response.status_code == 200 - response_data = json.loads(response.content.decode('utf-8')) - assert response_data == self.expected_response_data({ - 'raw_body': 'Edited body', - 'rendered_body': '

Edited body

', - 'editable_fields': ['abuse_flagged', 'anonymous', 'raw_body'], - 'created_at': 'Test Created Date', - 'updated_at': 'Test Updated Date' - }) + response_data = json.loads(response.content.decode("utf-8")) + assert response_data == self.expected_response_data( + { + "raw_body": "Edited body", + "rendered_body": "

Edited body

", + "editable_fields": ["abuse_flagged", "anonymous", "raw_body"], + "created_at": "Test Created Date", + "updated_at": "Test Updated Date", + } + ) params = { - 'comment_id': 'test_comment', - 'body': 'Edited body', - 'course_id': str(self.course.id), - 'user_id': str(self.user.id), - 'anonymous': False, - 'anonymous_to_peers': False, - 'endorsed': False, - 'editing_user_id': str(self.user.id), + "comment_id": "test_comment", + "body": "Edited body", + "course_id": str(self.course.id), + "user_id": str(self.user.id), + "anonymous": False, + "anonymous_to_peers": False, + "endorsed": False, + "editing_user_id": str(self.user.id), } - self.check_mock_called_with('update_comment', -1, **params) + self.check_mock_called_with("update_comment", -1, **params) def test_error(self): self.register_thread() @@ -405,10 +489,12 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes request_data = {"raw_body": ""} response = self.request_patch(request_data) expected_response_data = { - "field_errors": {"raw_body": {"developer_message": "This field may not be blank."}} + "field_errors": { + "raw_body": {"developer_message": "This field may not be blank."} + } } assert response.status_code == 400 - response_data = json.loads(response.content.decode('utf-8')) + response_data = json.loads(response.content.decode("utf-8")) assert response_data == expected_response_data @ddt.data( @@ -423,12 +509,14 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes request_data = {field: value} response = self.request_patch(request_data) assert response.status_code == 200 - response_data = json.loads(response.content.decode('utf-8')) - assert response_data == self.expected_response_data({ - 'abuse_flagged': value, - "abuse_flagged_any_user": None, - 'editable_fields': ['abuse_flagged'] - }) + response_data = json.loads(response.content.decode("utf-8")) + assert response_data == self.expected_response_data( + { + "abuse_flagged": value, + "abuse_flagged_any_user": None, + "editable_fields": ["abuse_flagged"], + } + ) @ddt.data( ("raw_body", "Edited body"), @@ -442,3 +530,396 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes request_data = {field: value} response = self.request_patch(request_data) assert response.status_code == 400 + + +@ddt.ddt +@httpretty.activate +@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) +class ThreadViewSetListTest( + DiscussionAPIViewTestMixin, ModuleStoreTestCase, ProfileImageTestMixin +): + """Tests for ThreadViewSet list""" + + def setUp(self): + super().setUp() + self.author = UserFactory.create() + self.url = reverse("thread-list") + + def create_source_thread(self, overrides=None): + """ + Create a sample source cs_thread + """ + thread = make_minimal_cs_thread( + { + "id": "test_thread", + "course_id": str(self.course.id), + "commentable_id": "test_topic", + "user_id": str(self.user.id), + "username": self.user.username, + "created_at": "2015-04-28T00:00:00Z", + "updated_at": "2015-04-28T11:11:11Z", + "title": "Test Title", + "body": "Test body", + "votes": {"up_count": 4}, + "comments_count": 5, + "unread_comments_count": 3, + } + ) + + thread.update(overrides or {}) + return thread + + def test_course_id_missing(self): + response = self.client.get(self.url) + self.assert_response_correct( + response, + 400, + {"field_errors": {"course_id": {"developer_message": "This field is required."}}} + ) + + def test_404(self): + response = self.client.get(self.url, {"course_id": "non/existent/course"}) + self.assert_response_correct( + response, + 404, + {"developer_message": "Course not found."} + ) + + def test_basic(self): + self.register_get_user_response(self.user, upvoted_ids=["test_thread"]) + source_threads = [ + self.create_source_thread( + {"user_id": str(self.author.id), "username": self.author.username} + ) + ] + expected_threads = [ + self.expected_thread_data( + { + "created_at": "2015-04-28T00:00:00Z", + "updated_at": "2015-04-28T11:11:11Z", + "vote_count": 4, + "comment_count": 6, + "can_delete": False, + "unread_comment_count": 3, + "voted": True, + "author": self.author.username, + "editable_fields": [ + "abuse_flagged", + "copy_link", + "following", + "read", + "voted", + ], + "abuse_flagged_count": None, + } + ) + ] + self.register_get_threads_response(source_threads, page=1, num_pages=2) + response = self.client.get( + self.url, {"course_id": str(self.course.id), "following": ""} + ) + expected_response = make_paginated_api_response( + results=expected_threads, + count=1, + num_pages=2, + next_link="http://testserver/api/discussion/v1/threads/?course_id=course-v1%3Ax%2By%2Bz&following=&page=2", + previous_link=None, + ) + expected_response.update({"text_search_rewrite": None}) + self.assert_response_correct(response, 200, expected_response) + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 1, + "per_page": 10, + } + self.check_mock_called_with( + "get_user_threads", + -1, + **params, + ) + + @ddt.data("unread", "unanswered", "unresponded") + def test_view_query(self, query): + threads = [make_minimal_cs_thread()] + self.register_get_user_response(self.user) + self.register_get_threads_response(threads, page=1, num_pages=1) + self.client.get( + self.url, + { + "course_id": str(self.course.id), + "view": query, + }, + ) + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 1, + "per_page": 10, + query: True, + } + self.check_mock_called_with( + "get_user_threads", + -1, + **params, + ) + + def test_pagination(self): + self.register_get_user_response(self.user) + self.register_get_threads_response([], page=1, num_pages=1) + response = self.client.get( + self.url, {"course_id": str(self.course.id), "page": "18", "page_size": "4"} + ) + self.assert_response_correct( + response, + 404, + {"developer_message": "Page not found (No results on this page)."}, + ) + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 18, + "per_page": 4, + } + self.check_mock_called_with( + "get_user_threads", + -1, + **params, + ) + + def test_text_search(self): + self.register_get_user_response(self.user) + self.register_get_threads_search_response([], None, num_pages=0) + response = self.client.get( + self.url, + {"course_id": str(self.course.id), "text_search": "test search string"}, + ) + + expected_response = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_response.update({"text_search_rewrite": None}) + self.assert_response_correct(response, 200, expected_response) + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 1, + "per_page": 10, + "text": "test search string", + } + self.check_mock_called_with( + "search_threads", + -1, + **params, + ) + + @ddt.data(True, "true", "1") + def test_following_true(self, following): + self.register_get_user_response(self.user) + self.register_subscribed_threads_response(self.user, [], page=1, num_pages=0) + response = self.client.get( + self.url, + { + "course_id": str(self.course.id), + "following": following, + }, + ) + + expected_response = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_response.update({"text_search_rewrite": None}) + self.assert_response_correct(response, 200, expected_response) + self.check_mock_called("get_user_subscriptions") + + @ddt.data(False, "false", "0") + def test_following_false(self, following): + response = self.client.get( + self.url, + { + "course_id": str(self.course.id), + "following": following, + }, + ) + self.assert_response_correct( + response, + 400, + { + "field_errors": { + "following": { + "developer_message": "The value of the 'following' parameter must be true." + } + } + }, + ) + + def test_following_error(self): + response = self.client.get( + self.url, + { + "course_id": str(self.course.id), + "following": "invalid-boolean", + }, + ) + self.assert_response_correct( + response, + 400, + { + "field_errors": { + "following": {"developer_message": "Invalid Boolean Value."} + } + }, + ) + + @ddt.data( + ("last_activity_at", "activity"), + ("comment_count", "comments"), + ("vote_count", "votes"), + ) + @ddt.unpack + def test_order_by(self, http_query, cc_query): + """ + Tests the order_by parameter + + Arguments: + http_query (str): Query string sent in the http request + cc_query (str): Query string used for the comments client service + """ + threads = [make_minimal_cs_thread()] + self.register_get_user_response(self.user) + self.register_get_threads_response(threads, page=1, num_pages=1) + self.client.get( + self.url, + { + "course_id": str(self.course.id), + "order_by": http_query, + }, + ) + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "page": 1, + "per_page": 10, + "sort_key": cc_query, + } + self.check_mock_called_with( + "get_user_threads", + -1, + **params, + ) + + def test_order_direction(self): + """ + Test order direction, of which "desc" is the only valid option. The + option actually just gets swallowed, so it doesn't affect the params. + """ + threads = [make_minimal_cs_thread()] + self.register_get_user_response(self.user) + self.register_get_threads_response(threads, page=1, num_pages=1) + self.client.get( + self.url, + { + "course_id": str(self.course.id), + "order_direction": "desc", + }, + ) + params = { + "user_id": str(self.user.id), + "course_id": str(self.course.id), + "sort_key": "activity", + "page": 1, + "per_page": 10, + } + self.check_mock_called_with( + "get_user_threads", + -1, + **params, + ) + + def test_mutually_exclusive(self): + """ + Tests GET thread_list api does not allow filtering on mutually exclusive parameters + """ + self.register_get_user_response(self.user) + self.register_get_threads_search_response([], None, num_pages=0) + response = self.client.get( + self.url, + { + "course_id": str(self.course.id), + "text_search": "test search string", + "topic_id": "topic1, topic2", + }, + ) + self.assert_response_correct( + response, + 400, + { + "developer_message": "The following query parameters are mutually exclusive: topic_id, " + "text_search, following" + }, + ) + + def test_profile_image_requested_field(self): + """ + Tests thread has user profile image details if called in requested_fields + """ + user_2 = UserFactory.create(password=self.password) + # Ensure that parental controls don't apply to this user + user_2.profile.year_of_birth = 1970 + user_2.profile.save() + source_threads = [ + self.create_source_thread(), + self.create_source_thread( + {"user_id": str(user_2.id), "username": user_2.username} + ), + ] + + self.register_get_user_response(self.user, upvoted_ids=["test_thread"]) + self.register_get_threads_response(source_threads, page=1, num_pages=1) + self.create_profile_image(self.user, get_profile_image_storage()) + self.create_profile_image(user_2, get_profile_image_storage()) + + response = self.client.get( + self.url, + {"course_id": str(self.course.id), "requested_fields": "profile_image"}, + ) + assert response.status_code == 200 + response_threads = json.loads(response.content.decode("utf-8"))["results"] + + for response_thread in response_threads: + expected_profile_data = self.get_expected_user_profile( + response_thread["author"] + ) + response_users = response_thread["users"] + assert expected_profile_data == response_users[response_thread["author"]] + + def test_profile_image_requested_field_anonymous_user(self): + """ + Tests profile_image in requested_fields for thread created with anonymous user + """ + source_threads = [ + self.create_source_thread( + { + "user_id": None, + "username": None, + "anonymous": True, + "anonymous_to_peers": True, + } + ), + ] + + self.register_get_user_response(self.user, upvoted_ids=["test_thread"]) + self.register_get_threads_response(source_threads, page=1, num_pages=1) + + response = self.client.get( + self.url, + {"course_id": str(self.course.id), "requested_fields": "profile_image"}, + ) + assert response.status_code == 200 + response_thread = json.loads(response.content.decode("utf-8"))["results"][0] + assert response_thread["author"] is None + assert {} == response_thread["users"] diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index facdb368f1..8bb0b45500 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -17,8 +17,6 @@ from django.urls import reverse from django.utils import translation from edx_django_utils.cache import RequestCache from edx_toggles.toggles.testutils import override_waffle_flag -from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ( TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase, @@ -27,7 +25,6 @@ from xmodule.modulestore.tests.django_utils import ( from xmodule.modulestore.tests.factories import ( CourseFactory, BlockFactory, - check_mongo_calls ) from common.djangoapps.course_modes.models import CourseMode @@ -42,7 +39,6 @@ from lms.djangoapps.discussion.django_comment_client.permissions import get_team from lms.djangoapps.discussion.django_comment_client.tests.group_id import ( CohortedTopicGroupIdTestMixin, GroupIdAssertionMixin, - NonCohortedTopicGroupIdTestMixin ) from lms.djangoapps.discussion.django_comment_client.tests.unicode import UnicodeTestMixin from lms.djangoapps.discussion.django_comment_client.tests.utils import ( @@ -55,7 +51,6 @@ from lms.djangoapps.discussion.django_comment_client.utils import strip_none from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE from lms.djangoapps.discussion.views import _get_discussion_default_topic_id, course_discussions_settings_handler from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory -from openedx.core.djangoapps.course_groups.models import CourseUserGroup from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts from openedx.core.djangoapps.course_groups.tests.test_views import CohortViewsTestCase from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientPaginatedResult @@ -68,8 +63,6 @@ from openedx.core.djangoapps.django_comment_common.utils import ThreadContext, s from openedx.core.djangoapps.util.testing import ContentGroupTestCase from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from openedx.core.lib.teams_config import TeamsConfig -from openedx.features.content_type_gating.models import ContentTypeGatingConfig -from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseTestConsentRequired log = logging.getLogger(__name__) @@ -529,93 +522,6 @@ class AllowPlusOrMinusOneInt(int): return f"({self.value} +/- 1)" -@ddt.ddt -@patch('requests.request', autospec=True) -class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase): - """ - Ensures the number of modulestore queries and number of sql queries are - independent of the number of responses retrieved for a given discussion thread. - """ - def setUp(self): - super().setUp() - patcher = mock.patch( - 'openedx.core.djangoapps.discussions.config.waffle.ENABLE_FORUM_V2.is_enabled', - return_value=False - ) - patcher.start() - self.addCleanup(patcher.stop) - patcher = mock.patch( - "openedx.core.djangoapps.django_comment_common.comment_client.thread.forum_api.get_course_id_by_thread" - ) - self.mock_get_course_id_by_thread = patcher.start() - self.addCleanup(patcher.stop) - - @ddt.data( - # split mongo: 3 queries, regardless of thread response size. - (False, 1, 2, 2, 21, 8), - (False, 50, 2, 2, 21, 8), - - # Enabling Enterprise integration should have no effect on the number of mongo queries made. - # split mongo: 3 queries, regardless of thread response size. - (True, 1, 2, 2, 21, 8), - (True, 50, 2, 2, 21, 8), - ) - @ddt.unpack - def test_number_of_mongo_queries( - self, - enterprise_enabled, - num_thread_responses, - num_uncached_mongo_calls, - num_cached_mongo_calls, - num_uncached_sql_queries, - num_cached_sql_queries, - mock_request - ): - ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1)) - with modulestore().default_store(ModuleStoreEnum.Type.split): - course = CourseFactory.create(discussion_topics={'dummy discussion': {'id': 'dummy_discussion_id'}}) - - student = UserFactory.create() - CourseEnrollmentFactory.create(user=student, course_id=course.id) - - test_thread_id = "test_thread_id" - mock_request.side_effect = make_mock_request_impl( - course=course, text="dummy content", thread_id=test_thread_id, num_thread_responses=num_thread_responses - ) - request = RequestFactory().get( - "dummy_url", - HTTP_X_REQUESTED_WITH="XMLHttpRequest" - ) - request.user = student - - def call_single_thread(): - """ - Call single_thread and assert that it returns what we expect. - """ - with patch.dict("django.conf.settings.FEATURES", dict(ENABLE_ENTERPRISE_INTEGRATION=enterprise_enabled)): - response = views.single_thread( - request, - str(course.id), - "dummy_discussion_id", - test_thread_id - ) - assert response.status_code == 200 - assert len(json.loads(response.content.decode('utf-8'))['content']['children']) == num_thread_responses - - # Test uncached first, then cached now that the cache is warm. - cached_calls = [ - [num_uncached_mongo_calls, num_uncached_sql_queries], - # Sometimes there will be one more or fewer sql call than expected, because the call to - # CourseMode.modes_for_course sometimes does / doesn't get cached and does / doesn't hit the DB. - # EDUCATOR-5167 - [num_cached_mongo_calls, AllowPlusOrMinusOneInt(num_cached_sql_queries)], - ] - for expected_mongo_calls, expected_sql_queries in cached_calls: - with self.assertNumQueries(expected_sql_queries, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): - with check_mongo_calls(expected_mongo_calls): - call_single_thread() - - @patch('requests.request', autospec=True) class SingleCohortedThreadTestCase(CohortedTestCase): # lint-amnesty, pylint: disable=missing-class-docstring @@ -868,92 +774,6 @@ class SingleThreadGroupIdTestCase(CohortedTestCase, GroupIdAssertionMixin): # l ) -@patch('requests.request', autospec=True) -class ForumFormDiscussionContentGroupTestCase(ForumsEnableMixin, ContentGroupTestCase): - """ - Tests `forum_form_discussion api` works with different content groups. - Discussion blocks are setup in ContentGroupTestCase class i.e - alpha_block => alpha_group_discussion => alpha_cohort => alpha_user/community_ta - beta_block => beta_group_discussion => beta_cohort => beta_user - """ - - @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) - def setUp(self): - super().setUp() - self.thread_list = [ - {"thread_id": "test_general_thread_id"}, - {"thread_id": "test_global_group_thread_id", "commentable_id": self.global_block.discussion_id}, - {"thread_id": "test_alpha_group_thread_id", "group_id": self.alpha_block.group_access[0][0], - "commentable_id": self.alpha_block.discussion_id}, - {"thread_id": "test_beta_group_thread_id", "group_id": self.beta_block.group_access[0][0], - "commentable_id": self.beta_block.discussion_id} - ] - - def assert_has_access(self, response, expected_discussion_threads): - """ - Verify that a users have access to the threads in their assigned - cohorts and non-cohorted blocks. - """ - discussion_data = json.loads(response.content.decode('utf-8'))['discussion_data'] - assert len(discussion_data) == expected_discussion_threads - - def call_view(self, mock_request, user): # lint-amnesty, pylint: disable=missing-function-docstring - mock_request.side_effect = make_mock_request_impl( - course=self.course, - text="dummy content", - thread_list=self.thread_list - ) - self.client.login(username=user.username, password=self.TEST_PASSWORD) - return self.client.get( - reverse("forum_form_discussion", args=[str(self.course.id)]), - HTTP_X_REQUESTED_WITH="XMLHttpRequest" - ) - - def test_community_ta_user(self, mock_request): - """ - Verify that community_ta user has access to all threads regardless - of cohort. - """ - response = self.call_view( - mock_request, - self.community_ta - ) - self.assert_has_access(response, 4) - - def test_alpha_cohort_user(self, mock_request): - """ - Verify that alpha_user has access to alpha_cohort and non-cohorted - threads. - """ - response = self.call_view( - mock_request, - self.alpha_user - ) - self.assert_has_access(response, 3) - - def test_beta_cohort_user(self, mock_request): - """ - Verify that beta_user has access to beta_cohort and non-cohorted - threads. - """ - response = self.call_view( - mock_request, - self.beta_user - ) - self.assert_has_access(response, 3) - - def test_global_staff_user(self, mock_request): - """ - Verify that global staff user has access to all threads regardless - of cohort. - """ - response = self.call_view( - mock_request, - self.staff_user - ) - self.assert_has_access(response, 4) - - @patch('requests.request', autospec=True) class SingleThreadContentGroupTestCase(ForumsEnableMixin, UrlResetMixin, ContentGroupTestCase): # lint-amnesty, pylint: disable=missing-class-docstring @@ -1080,417 +900,6 @@ class SingleThreadContentGroupTestCase(ForumsEnableMixin, UrlResetMixin, Content self.assert_can_access(self.beta_user, self.alpha_block.discussion_id, thread_id, True) -@patch('openedx.core.djangoapps.django_comment_common.comment_client.utils.requests.request', autospec=True) -class InlineDiscussionContextTestCase(ForumsEnableMixin, ModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring - - def setUp(self): - super().setUp() - self.course = CourseFactory.create() - CourseEnrollmentFactory(user=self.user, course_id=self.course.id) - self.discussion_topic_id = "dummy_topic" - self.team = CourseTeamFactory( - name="A team", - course_id=self.course.id, - topic_id='topic_id', - discussion_topic_id=self.discussion_topic_id - ) - - self.team.add_user(self.user) - self.user_not_in_team = UserFactory.create() - - def test_context_can_be_standalone(self, mock_request): - mock_request.side_effect = make_mock_request_impl( - course=self.course, - text="dummy text", - commentable_id=self.discussion_topic_id - ) - - request = RequestFactory().get("dummy_url") - request.user = self.user - - response = views.inline_discussion( - request, - str(self.course.id), - self.discussion_topic_id, - ) - - json_response = json.loads(response.content.decode('utf-8')) - assert json_response['discussion_data'][0]['context'] == ThreadContext.STANDALONE - - def test_private_team_discussion(self, mock_request): - # First set the team discussion to be private - CourseEnrollmentFactory(user=self.user_not_in_team, course_id=self.course.id) - request = RequestFactory().get("dummy_url") - request.user = self.user_not_in_team - - mock_request.side_effect = make_mock_request_impl( - course=self.course, - text="dummy text", - commentable_id=self.discussion_topic_id - ) - - with patch('lms.djangoapps.teams.api.is_team_discussion_private', autospec=True) as mocked: - mocked.return_value = True - response = views.inline_discussion( - request, - str(self.course.id), - self.discussion_topic_id, - ) - assert response.status_code == 403 - assert response.content.decode('utf-8') == views.TEAM_PERMISSION_MESSAGE - - -@patch('openedx.core.djangoapps.django_comment_common.comment_client.utils.requests.request', autospec=True) -@patch('openedx.core.djangoapps.discussions.config.waffle.ENABLE_FORUM_V2.is_enabled', autospec=True) -class InlineDiscussionGroupIdTestCase( # lint-amnesty, pylint: disable=missing-class-docstring - CohortedTestCase, - CohortedTopicGroupIdTestMixin, - NonCohortedTopicGroupIdTestMixin -): - cs_endpoint = "/threads" - - def setUp(self): - super().setUp() - self.cohorted_commentable_id = 'cohorted_topic' - patcher = mock.patch( - "openedx.core.djangoapps.django_comment_common.comment_client.thread.forum_api.get_course_id_by_thread" - ) - self.mock_get_course_id_by_thread = patcher.start() - self.addCleanup(patcher.stop) - - def call_view( - self, - mock_is_forum_v2_enabled, - mock_request, - commentable_id, - user, - group_id, - pass_group_id=True - ): # pylint: disable=arguments-differ - mock_is_forum_v2_enabled.return_value = False - kwargs = {'commentable_id': self.cohorted_commentable_id} - if group_id: - # avoid causing a server error when the LMS chokes attempting - # to find a group name for the group_id, when we're testing with - # an invalid one. - try: - CourseUserGroup.objects.get(id=group_id) - kwargs['group_id'] = group_id - except CourseUserGroup.DoesNotExist: - pass - mock_request.side_effect = make_mock_request_impl(self.course, "dummy content", **kwargs) - - request_data = {} - if pass_group_id: - request_data["group_id"] = group_id - request = RequestFactory().get( - "dummy_url", - data=request_data - ) - request.user = user - return views.inline_discussion( - request, - str(self.course.id), - commentable_id - ) - - def test_group_info_in_ajax_response(self, mock_is_forum_v2_enabled, mock_request): - response = self.call_view( - mock_is_forum_v2_enabled, - mock_request, - self.cohorted_commentable_id, - self.student, - self.student_cohort.id - ) - self._assert_json_response_contains_group_info( - response, lambda d: d['discussion_data'][0] - ) - - -@patch('openedx.core.djangoapps.django_comment_common.comment_client.utils.requests.request', autospec=True) -@patch('openedx.core.djangoapps.discussions.config.waffle.ENABLE_FORUM_V2.is_enabled', autospec=True) -class ForumFormDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixin): # lint-amnesty, pylint: disable=missing-class-docstring - cs_endpoint = "/threads" - - def setUp(self): - super().setUp() - patcher = mock.patch( - "openedx.core.djangoapps.django_comment_common.comment_client.thread.forum_api.get_course_id_by_thread" - ) - self.mock_get_course_id_by_thread = patcher.start() - self.addCleanup(patcher.stop) - - def call_view( - self, - mock_is_forum_v2_enabled, - mock_request, - commentable_id, - user, - group_id, - pass_group_id=True, - is_ajax=False - ): # pylint: disable=arguments-differ - mock_is_forum_v2_enabled.return_value = False - kwargs = {} - if group_id: - kwargs['group_id'] = group_id - mock_request.side_effect = make_mock_request_impl(self.course, "dummy content", **kwargs) - - request_data = {} - if pass_group_id: - request_data["group_id"] = group_id - headers = {} - if is_ajax: - headers['HTTP_X_REQUESTED_WITH'] = "XMLHttpRequest" - - self.client.login(username=user.username, password=self.TEST_PASSWORD) - return self.client.get( - reverse("forum_form_discussion", args=[str(self.course.id)]), - data=request_data, - **headers - ) - - def test_group_info_in_html_response(self, mock_is_forum_v2_enabled, mock_request): - response = self.call_view( - mock_is_forum_v2_enabled, - mock_request, - "cohorted_topic", - self.student, - self.student_cohort.id - ) - self._assert_html_response_contains_group_info(response) - - def test_group_info_in_ajax_response(self, mock_is_forum_v2_enabled, mock_request): - response = self.call_view( - mock_is_forum_v2_enabled, - mock_request, - "cohorted_topic", - self.student, - self.student_cohort.id, - is_ajax=True - ) - self._assert_json_response_contains_group_info( - response, lambda d: d['discussion_data'][0] - ) - - -@patch('openedx.core.djangoapps.django_comment_common.comment_client.utils.requests.request', autospec=True) -@patch('openedx.core.djangoapps.discussions.config.waffle.ENABLE_FORUM_V2.is_enabled', autospec=True) -class UserProfileDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixin): # lint-amnesty, pylint: disable=missing-class-docstring - cs_endpoint = "/active_threads" - - def setUp(self): - super().setUp() - patcher = mock.patch( - "openedx.core.djangoapps.django_comment_common.comment_client.models.forum_api.get_course_id_by_comment" - ) - self.mock_get_course_id_by_comment = patcher.start() - self.addCleanup(patcher.stop) - patcher = mock.patch( - "openedx.core.djangoapps.django_comment_common.comment_client.thread.forum_api.get_course_id_by_thread" - ) - self.mock_get_course_id_by_thread = patcher.start() - self.addCleanup(patcher.stop) - - def call_view_for_profiled_user( - self, - mock_is_forum_v2_enabled, - mock_request, - requesting_user, - profiled_user, - group_id, - pass_group_id, - is_ajax=False - ): - """ - Calls "user_profile" view method on behalf of "requesting_user" to get information about - the user "profiled_user". - """ - mock_is_forum_v2_enabled.return_value = False - kwargs = {} - if group_id: - kwargs['group_id'] = group_id - mock_request.side_effect = make_mock_request_impl(self.course, "dummy content", **kwargs) - - request_data = {} - if pass_group_id: - request_data["group_id"] = group_id - headers = {} - if is_ajax: - headers['HTTP_X_REQUESTED_WITH'] = "XMLHttpRequest" - - self.client.login(username=requesting_user.username, password=self.TEST_PASSWORD) - return self.client.get( - reverse('user_profile', args=[str(self.course.id), profiled_user.id]), - data=request_data, - **headers - ) - - def call_view( - self, - mock_is_forum_v2_enabled, - mock_request, - _commentable_id, - user, - group_id, - pass_group_id=True, - is_ajax=False - ): # pylint: disable=arguments-differ - return self.call_view_for_profiled_user( - mock_is_forum_v2_enabled, mock_request, user, user, group_id, pass_group_id=pass_group_id, is_ajax=is_ajax - ) - - def test_group_info_in_html_response(self, mock_is_forum_v2_enabled, mock_request): - response = self.call_view( - mock_is_forum_v2_enabled, - mock_request, - "cohorted_topic", - self.student, - self.student_cohort.id, - is_ajax=False - ) - self._assert_html_response_contains_group_info(response) - - def test_group_info_in_ajax_response(self, mock_is_forum_v2_enabled, mock_request): - response = self.call_view( - mock_is_forum_v2_enabled, - mock_request, - "cohorted_topic", - self.student, - self.student_cohort.id, - is_ajax=True - ) - self._assert_json_response_contains_group_info( - response, lambda d: d['discussion_data'][0] - ) - - def _test_group_id_passed_to_user_profile( - self, - mock_is_forum_v2_enabled, - mock_request, - expect_group_id_in_request, - requesting_user, - profiled_user, - group_id, - pass_group_id - ): - """ - Helper method for testing whether or not group_id was passed to the user_profile request. - """ - - def get_params_from_user_info_call(for_specific_course): - """ - Returns the request parameters for the user info call with either course_id specified or not, - depending on value of 'for_specific_course'. - """ - # There will be 3 calls from user_profile. One has the cs_endpoint "active_threads", and it is already - # tested. The other 2 calls are for user info; one of those calls is for general information about the user, - # and it does not specify a course_id. The other call does specify a course_id, and if the caller did not - # have discussion moderator privileges, it should also contain a group_id. - for r_call in mock_request.call_args_list: - if not r_call[0][1].endswith(self.cs_endpoint): - params = r_call[1]["params"] - has_course_id = "course_id" in params - if (for_specific_course and has_course_id) or (not for_specific_course and not has_course_id): - return params - pytest.fail(f"Did not find appropriate user_profile call for 'for_specific_course'={for_specific_course}") - - mock_request.reset_mock() - self.call_view_for_profiled_user( - mock_is_forum_v2_enabled, - mock_request, - requesting_user, - profiled_user, - group_id, - pass_group_id=pass_group_id, - is_ajax=False - ) - # Should never have a group_id if course_id was not included in the request. - params_without_course_id = get_params_from_user_info_call(False) - assert 'group_id' not in params_without_course_id - - params_with_course_id = get_params_from_user_info_call(True) - if expect_group_id_in_request: - assert 'group_id' in params_with_course_id - assert group_id == params_with_course_id['group_id'] - else: - assert 'group_id' not in params_with_course_id - - def test_group_id_passed_to_user_profile_student(self, mock_is_forum_v2_enabled, mock_request): - """ - Test that the group id is always included when requesting user profile information for a particular - course if the requester does not have discussion moderation privileges. - """ - def verify_group_id_always_present(profiled_user, pass_group_id): - """ - Helper method to verify that group_id is always present for student in course - (non-privileged user). - """ - self._test_group_id_passed_to_user_profile( - mock_is_forum_v2_enabled, - mock_request, - True, - self.student, - profiled_user, - self.student_cohort.id, - pass_group_id - ) - - # In all these test cases, the requesting_user is the student (non-privileged user). - # The profile returned on behalf of the student is for the profiled_user. - verify_group_id_always_present(profiled_user=self.student, pass_group_id=True) - verify_group_id_always_present(profiled_user=self.student, pass_group_id=False) - verify_group_id_always_present(profiled_user=self.moderator, pass_group_id=True) - verify_group_id_always_present(profiled_user=self.moderator, pass_group_id=False) - - def test_group_id_user_profile_moderator(self, mock_is_forum_v2_enabled, mock_request): - """ - Test that the group id is only included when a privileged user requests user profile information for a - particular course and user if the group_id is explicitly passed in. - """ - def verify_group_id_present(profiled_user, pass_group_id, requested_cohort=self.moderator_cohort): - """ - Helper method to verify that group_id is present. - """ - self._test_group_id_passed_to_user_profile( - mock_is_forum_v2_enabled, - mock_request, - True, - self.moderator, - profiled_user, - requested_cohort.id, - pass_group_id - ) - - def verify_group_id_not_present(profiled_user, pass_group_id, requested_cohort=self.moderator_cohort): - """ - Helper method to verify that group_id is not present. - """ - self._test_group_id_passed_to_user_profile( - mock_is_forum_v2_enabled, - mock_request, - False, - self.moderator, - profiled_user, - requested_cohort.id, - pass_group_id - ) - - # In all these test cases, the requesting_user is the moderator (privileged user). - - # If the group_id is explicitly passed, it will be present in the request. - verify_group_id_present(profiled_user=self.student, pass_group_id=True) - verify_group_id_present(profiled_user=self.moderator, pass_group_id=True) - verify_group_id_present( - profiled_user=self.student, pass_group_id=True, requested_cohort=self.student_cohort - ) - - # If the group_id is not explicitly passed, it will not be present because the requesting_user - # has discussion moderator privileges. - verify_group_id_not_present(profiled_user=self.student, pass_group_id=False) - verify_group_id_not_present(profiled_user=self.moderator, pass_group_id=False) - - @patch('openedx.core.djangoapps.django_comment_common.comment_client.utils.requests.request', autospec=True) @patch('openedx.core.djangoapps.discussions.config.waffle.ENABLE_FORUM_V2.is_enabled', autospec=True) class FollowedThreadsDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixin): # lint-amnesty, pylint: disable=missing-class-docstring @@ -1547,189 +956,6 @@ class FollowedThreadsDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGr ) -@patch('openedx.core.djangoapps.django_comment_common.comment_client.utils.requests.request', autospec=True) -class InlineDiscussionTestCase(ForumsEnableMixin, ModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring - - def setUp(self): - super().setUp() - - self.course = CourseFactory.create( - org="TestX", - number="101", - display_name="Test Course", - teams_configuration=TeamsConfig({ - 'topics': [{ - 'id': 'topic_id', - 'name': 'A topic', - 'description': 'A topic', - }] - }) - ) - self.student = UserFactory.create() - CourseEnrollmentFactory(user=self.student, course_id=self.course.id) - self.discussion1 = BlockFactory.create( - parent_location=self.course.location, - category="discussion", - discussion_id="discussion1", - display_name='Discussion1', - discussion_category="Chapter", - discussion_target="Discussion1" - ) - - def send_request(self, mock_request, params=None): - """ - Creates and returns a request with params set, and configures - mock_request to return appropriate values. - """ - request = RequestFactory().get("dummy_url", params if params else {}) - request.user = self.student - mock_request.side_effect = make_mock_request_impl( - course=self.course, text="dummy content", commentable_id=self.discussion1.discussion_id - ) - return views.inline_discussion( - request, str(self.course.id), self.discussion1.discussion_id - ) - - def test_context(self, mock_request): - team = CourseTeamFactory( - name='Team Name', - topic_id='topic_id', - course_id=self.course.id, - discussion_topic_id=self.discussion1.discussion_id - ) - - team.add_user(self.student) - - self.send_request(mock_request) - assert mock_request.call_args[1]['params']['context'] == ThreadContext.STANDALONE - - -@patch('requests.request', autospec=True) -class UserProfileTestCase(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring - - TEST_THREAD_TEXT = 'userprofile-test-text' - TEST_THREAD_ID = 'userprofile-test-thread-id' - - @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) - def setUp(self): - super().setUp() - - self.course = CourseFactory.create() - self.student = UserFactory.create() - self.profiled_user = UserFactory.create() - CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id) - CourseEnrollmentFactory.create(user=self.profiled_user, course_id=self.course.id) - - def get_response(self, mock_request, params, **headers): # lint-amnesty, pylint: disable=missing-function-docstring - mock_request.side_effect = make_mock_request_impl( - course=self.course, text=self.TEST_THREAD_TEXT, thread_id=self.TEST_THREAD_ID - ) - self.client.login(username=self.student.username, password=self.TEST_PASSWORD) - - response = self.client.get( - reverse('user_profile', kwargs={ - 'course_id': str(self.course.id), - 'user_id': self.profiled_user.id, - }), - data=params, - **headers - ) - mock_request.assert_any_call( - "get", - StringEndsWithMatcher(f'/users/{self.profiled_user.id}/active_threads'), - data=None, - params=PartialDictMatcher({ - "course_id": str(self.course.id), - "page": params.get("page", 1), - "per_page": views.THREADS_PER_PAGE - }), - headers=ANY, - timeout=ANY - ) - return response - - def check_html(self, mock_request, **params): # lint-amnesty, pylint: disable=missing-function-docstring - response = self.get_response(mock_request, params) - assert response.status_code == 200 - assert response['Content-Type'] == 'text/html; charset=utf-8' - html = response.content.decode('utf-8') - self.assertRegex(html, r'data-page="1"') - self.assertRegex(html, r'data-num-pages="1"') - self.assertRegex(html, r'1 discussion started') - self.assertRegex(html, r'2 comments') - self.assertRegex(html, f''id': '{self.TEST_THREAD_ID}'') - self.assertRegex(html, f''title': '{self.TEST_THREAD_TEXT}'') - self.assertRegex(html, f''body': '{self.TEST_THREAD_TEXT}'') - self.assertRegex(html, f''username': '{self.student.username}'') - - def check_ajax(self, mock_request, **params): # lint-amnesty, pylint: disable=missing-function-docstring - response = self.get_response(mock_request, params, HTTP_X_REQUESTED_WITH="XMLHttpRequest") - assert response.status_code == 200 - assert response['Content-Type'] == 'application/json; charset=utf-8' - response_data = json.loads(response.content.decode('utf-8')) - assert sorted(response_data.keys()) == ['annotated_content_info', 'discussion_data', 'num_pages', 'page'] - assert len(response_data['discussion_data']) == 1 - assert response_data['page'] == 1 - assert response_data['num_pages'] == 1 - assert response_data['discussion_data'][0]['id'] == self.TEST_THREAD_ID - assert response_data['discussion_data'][0]['title'] == self.TEST_THREAD_TEXT - assert response_data['discussion_data'][0]['body'] == self.TEST_THREAD_TEXT - - def test_html(self, mock_request): - self.check_html(mock_request) - - def test_ajax(self, mock_request): - self.check_ajax(mock_request) - - def test_404_non_enrolled_user(self, __): - """ - Test that when student try to visit un-enrolled students' discussion profile, - the system raises Http404. - """ - unenrolled_user = UserFactory.create() - request = RequestFactory().get("dummy_url") - request.user = self.student - with pytest.raises(Http404): - views.user_profile( - request, - str(self.course.id), - unenrolled_user.id - ) - - def test_404_profiled_user(self, _mock_request): - request = RequestFactory().get("dummy_url") - request.user = self.student - with pytest.raises(Http404): - views.user_profile( - request, - str(self.course.id), - -999 - ) - - def test_404_course(self, _mock_request): - request = RequestFactory().get("dummy_url") - request.user = self.student - with pytest.raises(Http404): - views.user_profile( - request, - "non/existent/course", - self.profiled_user.id - ) - - def test_post(self, mock_request): - mock_request.side_effect = make_mock_request_impl( - course=self.course, text=self.TEST_THREAD_TEXT, thread_id=self.TEST_THREAD_ID - ) - request = RequestFactory().post("dummy_url") - request.user = self.student - response = views.user_profile( - request, - str(self.course.id), - self.profiled_user.id - ) - assert response.status_code == 405 - - @patch('requests.request', autospec=True) class CommentsServiceRequestHeadersTestCase(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring @@ -1811,155 +1037,6 @@ class CommentsServiceRequestHeadersTestCase(ForumsEnableMixin, UrlResetMixin, Mo self.assert_all_calls_have_header(mock_request, "X-Edx-Api-Key", "test_api_key") -class InlineDiscussionUnicodeTestCase(ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin): # lint-amnesty, pylint: disable=missing-class-docstring - - @classmethod - def setUpClass(cls): - # pylint: disable=super-method-not-called - with super().setUpClassAndTestData(): - cls.course = CourseFactory.create() - - @classmethod - def setUpTestData(cls): - super().setUpTestData() - - cls.student = UserFactory.create() - CourseEnrollmentFactory(user=cls.student, course_id=cls.course.id) - - @patch('openedx.core.djangoapps.django_comment_common.comment_client.utils.requests.request', autospec=True) - def _test_unicode_data(self, text, mock_request): # lint-amnesty, pylint: disable=missing-function-docstring - mock_request.side_effect = make_mock_request_impl(course=self.course, text=text) - request = RequestFactory().get("dummy_url") - request.user = self.student - - response = views.inline_discussion( - request, str(self.course.id), self.course.discussion_topics['General']['id'] - ) - assert response.status_code == 200 - response_data = json.loads(response.content.decode('utf-8')) - assert response_data['discussion_data'][0]['title'] == text - assert response_data['discussion_data'][0]['body'] == text - - -class ForumFormDiscussionUnicodeTestCase(ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin): # lint-amnesty, pylint: disable=missing-class-docstring - - @classmethod - def setUpClass(cls): - # pylint: disable=super-method-not-called - with super().setUpClassAndTestData(): - cls.course = CourseFactory.create() - - @classmethod - def setUpTestData(cls): - super().setUpTestData() - - cls.student = UserFactory.create() - CourseEnrollmentFactory(user=cls.student, course_id=cls.course.id) - - @patch('openedx.core.djangoapps.django_comment_common.comment_client.utils.requests.request', autospec=True) - def _test_unicode_data(self, text, mock_request): # lint-amnesty, pylint: disable=missing-function-docstring - mock_request.side_effect = make_mock_request_impl(course=self.course, text=text) - request = RequestFactory().get("dummy_url") - request.user = self.student - # so (request.headers.get('x-requested-with') == 'XMLHttpRequest') == True - request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" - - response = views.forum_form_discussion(request, str(self.course.id)) - assert response.status_code == 200 - response_data = json.loads(response.content.decode('utf-8')) - assert response_data['discussion_data'][0]['title'] == text - assert response_data['discussion_data'][0]['body'] == text - - -@ddt.ddt -@patch('openedx.core.djangoapps.django_comment_common.comment_client.utils.requests.request', autospec=True) -class ForumDiscussionXSSTestCase(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring - - @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) - def setUp(self): - super().setUp() - - username = "foo" - password = "bar" - - self.course = CourseFactory.create() - self.student = UserFactory.create(username=username, password=password) - CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id) - assert self.client.login(username=username, password=password) - - @ddt.data('">', '', '') - @patch('common.djangoapps.student.models.user.cc.User.from_django_user') - def test_forum_discussion_xss_prevent(self, malicious_code, mock_user, mock_req): - """ - Test that XSS attack is prevented - """ - mock_user.return_value.to_dict.return_value = {} - mock_req.return_value.status_code = 200 - reverse_url = "{}{}".format(reverse( - "forum_form_discussion", - kwargs={"course_id": str(self.course.id)}), '/forum_form_discussion') - # Test that malicious code does not appear in html - url = "{}?{}={}".format(reverse_url, 'sort_key', malicious_code) - resp = self.client.get(url) - self.assertNotContains(resp, malicious_code) - - @ddt.data('">', '', '') - @patch('common.djangoapps.student.models.user.cc.User.from_django_user') - @patch('common.djangoapps.student.models.user.cc.User.active_threads') - def test_forum_user_profile_xss_prevent(self, malicious_code, mock_threads, mock_from_django_user, mock_request): - """ - Test that XSS attack is prevented - """ - mock_threads.return_value = [], 1, 1 - mock_from_django_user.return_value.to_dict.return_value = { - 'upvoted_ids': [], - 'downvoted_ids': [], - 'subscribed_thread_ids': [] - } - mock_request.side_effect = make_mock_request_impl(course=self.course, text='dummy') - - url = reverse('user_profile', - kwargs={'course_id': str(self.course.id), 'user_id': str(self.student.id)}) - # Test that malicious code does not appear in html - url_string = "{}?{}={}".format(url, 'page', malicious_code) - resp = self.client.get(url_string) - self.assertNotContains(resp, malicious_code) - - -class ForumDiscussionSearchUnicodeTestCase(ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin): # lint-amnesty, pylint: disable=missing-class-docstring - - @classmethod - def setUpClass(cls): - # pylint: disable=super-method-not-called - with super().setUpClassAndTestData(): - cls.course = CourseFactory.create() - - @classmethod - def setUpTestData(cls): - super().setUpTestData() - - cls.student = UserFactory.create() - CourseEnrollmentFactory(user=cls.student, course_id=cls.course.id) - - @patch('openedx.core.djangoapps.django_comment_common.comment_client.utils.requests.request', autospec=True) - def _test_unicode_data(self, text, mock_request): # lint-amnesty, pylint: disable=missing-function-docstring - mock_request.side_effect = make_mock_request_impl(course=self.course, text=text) - data = { - "ajax": 1, - "text": text, - } - request = RequestFactory().get("dummy_url", data) - request.user = self.student - # so (request.headers.get('x-requested-with') == 'XMLHttpRequest') == True - request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" - - response = views.forum_form_discussion(request, str(self.course.id)) - assert response.status_code == 200 - response_data = json.loads(response.content.decode('utf-8')) - assert response_data['discussion_data'][0]['title'] == text - assert response_data['discussion_data'][0]['body'] == text - - class SingleThreadUnicodeTestCase(ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin): # lint-amnesty, pylint: disable=missing-class-docstring @classmethod @@ -2087,60 +1164,6 @@ class EnrollmentTestCase(ForumsEnableMixin, ModuleStoreTestCase): views.forum_form_discussion(request, course_id=str(self.course.id)) # pylint: disable=no-value-for-parameter, unexpected-keyword-arg -@patch('requests.request', autospec=True) -class EnterpriseConsentTestCase(EnterpriseTestConsentRequired, ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase): - """ - Ensure that the Enterprise Data Consent redirects are in place only when consent is required. - """ - CREATE_USER = False - - @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) - def setUp(self): - # Invoke UrlResetMixin setUp - super().setUp() - patcher = mock.patch( - 'openedx.core.djangoapps.discussions.config.waffle.ENABLE_FORUM_V2.is_enabled', - return_value=False - ) - patcher.start() - self.addCleanup(patcher.stop) - patcher = mock.patch( - "openedx.core.djangoapps.django_comment_common.comment_client.thread.forum_api.get_course_id_by_thread" - ) - self.mock_get_course_id_by_thread = patcher.start() - self.addCleanup(patcher.stop) - username = "foo" - password = "bar" - - self.discussion_id = 'dummy_discussion_id' - self.course = CourseFactory.create(discussion_topics={'dummy discussion': {'id': self.discussion_id}}) - self.student = UserFactory.create(username=username, password=password) - CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id) - assert self.client.login(username=username, password=password) - - self.addCleanup(translation.deactivate) - - @patch('openedx.features.enterprise_support.api.enterprise_customer_for_request') - def test_consent_required(self, mock_enterprise_customer_for_request, mock_request): - """ - Test that enterprise data sharing consent is required when enabled for the various discussion views. - """ - # ENT-924: Temporary solution to replace sensitive SSO usernames. - mock_enterprise_customer_for_request.return_value = None - - thread_id = 'dummy' - course_id = str(self.course.id) - mock_request.side_effect = make_mock_request_impl(course=self.course, text='dummy', thread_id=thread_id) - - for url in ( - reverse('forum_form_discussion', - kwargs=dict(course_id=course_id)), - reverse('single_thread', - kwargs=dict(course_id=course_id, discussion_id=self.discussion_id, thread_id=thread_id)), - ): - self.verify_consent_required(self.client, url) # pylint: disable=no-value-for-parameter - - class DividedDiscussionsTestCase(CohortViewsTestCase): # lint-amnesty, pylint: disable=missing-class-docstring def create_divided_discussions(self): diff --git a/lms/djangoapps/discussion/tests/test_views_v2.py b/lms/djangoapps/discussion/tests/test_views_v2.py new file mode 100644 index 0000000000..3ac375ed03 --- /dev/null +++ b/lms/djangoapps/discussion/tests/test_views_v2.py @@ -0,0 +1,1264 @@ +# pylint: disable=unused-import +""" +Tests the forum notification views. +""" + +import json +import logging +from datetime import datetime +from unittest import mock +from unittest.mock import ANY, Mock, call, patch + +import ddt +import pytest +from django.conf import settings +from django.http import Http404 +from django.test.client import Client, RequestFactory +from django.test.utils import override_settings +from django.urls import reverse +from django.utils import translation +from edx_django_utils.cache import RequestCache +from edx_toggles.toggles.testutils import override_waffle_flag +from lms.djangoapps.discussion.django_comment_client.tests.mixins import ( + MockForumApiMixin, +) +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ( + TEST_DATA_SPLIT_MODULESTORE, + ModuleStoreTestCase, + SharedModuleStoreTestCase, +) +from xmodule.modulestore.tests.factories import ( + CourseFactory, + BlockFactory, + check_mongo_calls, +) + +from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.course_modes.tests.factories import CourseModeFactory +from common.djangoapps.student.roles import CourseStaffRole, UserBasedRole +from common.djangoapps.student.tests.factories import ( + AdminFactory, + CourseEnrollmentFactory, + UserFactory, +) +from common.djangoapps.util.testing import EventTestMixin, UrlResetMixin +from lms.djangoapps.courseware.exceptions import CourseAccessRedirect +from lms.djangoapps.discussion import views +from lms.djangoapps.discussion.django_comment_client.constants import ( + TYPE_ENTRY, + TYPE_SUBCATEGORY, +) +from lms.djangoapps.discussion.django_comment_client.permissions import get_team +from lms.djangoapps.discussion.django_comment_client.tests.group_id import ( + CohortedTopicGroupIdTestMixinV2, + GroupIdAssertionMixinV2, + NonCohortedTopicGroupIdTestMixinV2, +) +from lms.djangoapps.discussion.django_comment_client.tests.unicode import ( + UnicodeTestMixin, +) +from lms.djangoapps.discussion.django_comment_client.tests.utils import ( + CohortedTestCase, + ForumsEnableMixin, + config_course_discussions, + topic_name_to_id, +) +from lms.djangoapps.discussion.django_comment_client.utils import strip_none +from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE +from lms.djangoapps.discussion.views import ( + _get_discussion_default_topic_id, + course_discussions_settings_handler, +) +from lms.djangoapps.teams.tests.factories import ( + CourseTeamFactory, + CourseTeamMembershipFactory, +) +from openedx.core.djangoapps.course_groups.models import CourseUserGroup +from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts +from openedx.core.djangoapps.course_groups.tests.test_views import CohortViewsTestCase +from openedx.core.djangoapps.django_comment_common.comment_client.utils import ( + CommentClientPaginatedResult, +) +from openedx.core.djangoapps.django_comment_common.models import ( + FORUM_ROLE_STUDENT, + CourseDiscussionSettings, + ForumsConfig, +) +from openedx.core.djangoapps.django_comment_common.utils import ( + ThreadContext, + seed_permissions_roles, +) +from openedx.core.djangoapps.util.testing import ContentGroupTestCase +from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES +from openedx.core.lib.teams_config import TeamsConfig +from openedx.features.content_type_gating.models import ContentTypeGatingConfig +from openedx.features.enterprise_support.tests.mixins.enterprise import ( + EnterpriseTestConsentRequired, +) + +log = logging.getLogger(__name__) + +QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + + +def make_mock_thread_data( + course, + text, + thread_id, + num_children, + group_id=None, + group_name=None, + commentable_id=None, + is_commentable_divided=None, + anonymous=False, + anonymous_to_peers=False, +): + """ + Creates mock thread data for testing purposes. + """ + data_commentable_id = ( + commentable_id + or course.discussion_topics.get("General", {}).get("id") + or "dummy_commentable_id" + ) + thread_data = { + "id": thread_id, + "type": "thread", + "title": text, + "body": text, + "commentable_id": data_commentable_id, + "resp_total": 42, + "resp_skip": 25, + "resp_limit": 5, + "group_id": group_id, + "anonymous": anonymous, + "anonymous_to_peers": anonymous_to_peers, + "context": ( + ThreadContext.COURSE + if get_team(data_commentable_id) is None + else ThreadContext.STANDALONE + ), + } + if group_id is not None: + thread_data["group_name"] = group_name + if is_commentable_divided is not None: + thread_data["is_commentable_divided"] = is_commentable_divided + if num_children is not None: + thread_data["children"] = [ + { + "id": f"dummy_comment_id_{i}", + "type": "comment", + "body": text, + } + for i in range(num_children) + ] + return thread_data + + +def make_mock_collection_data( + course, + text, + thread_id, + num_children=None, + group_id=None, + commentable_id=None, + thread_list=None, +): + """ + Creates mock collection data for testing purposes. + """ + if thread_list: + return [ + make_mock_thread_data( + course=course, text=text, num_children=num_children, **thread + ) + for thread in thread_list + ] + else: + return [ + make_mock_thread_data( + course=course, + text=text, + thread_id=thread_id, + num_children=num_children, + group_id=group_id, + commentable_id=commentable_id, + ) + ] + + +def make_collection_callback( + course, + text, + thread_id="dummy_thread_id", + group_id=None, + commentable_id=None, + thread_list=None, +): + """ + Creates a callback function for simulating collection data. + """ + + def callback(*args, **kwargs): + # Simulate default user thread response + return { + "collection": make_mock_collection_data( + course, text, thread_id, None, group_id, commentable_id, thread_list + ) + } + + return callback + + +def make_thread_callback( + course, + text, + thread_id="dummy_thread_id", + group_id=None, + commentable_id=None, + num_thread_responses=1, + anonymous=False, + anonymous_to_peers=False, +): + """ + Creates a callback function for simulating thread data. + """ + + def callback(*args, **kwargs): + # Simulate default user thread response + return make_mock_thread_data( + course=course, + text=text, + thread_id=thread_id, + num_children=num_thread_responses, + group_id=group_id, + commentable_id=commentable_id, + anonymous=anonymous, + anonymous_to_peers=anonymous_to_peers, + ) + + return callback + + +def make_user_callback(): + """ + Creates a callback function for simulating user data. + """ + + def callback(*args, **kwargs): + res = { + "default_sort_key": "date", + "upvoted_ids": [], + "downvoted_ids": [], + "subscribed_thread_ids": [], + } + # comments service adds these attributes when course_id param is present + if kwargs.get("course_id"): + res.update({"threads_count": 1, "comments_count": 2}) + return res + + return callback + + +class ForumViewsUtilsMixin(MockForumApiMixin): + """ + Utils for the Forum Views. + """ + + def _configure_mock_responses( + self, + course, + text, + thread_id="dummy_thread_id", + group_id=None, + commentable_id=None, + num_thread_responses=1, + thread_list=None, + anonymous=False, + anonymous_to_peers=False, + ): + """ + Configure mock responses for the Forum Views. + """ + for func_name in [ + "search_threads", + "get_user_active_threads", + "get_user_threads", + ]: + self.set_mock_side_effect( + func_name, + make_collection_callback( + course, + text, + thread_id, + group_id, + commentable_id, + thread_list, + ), + ) + + self.set_mock_side_effect( + "get_thread", + make_thread_callback( + course, + text, + thread_id, + group_id, + commentable_id, + num_thread_responses, + anonymous, + anonymous_to_peers, + ), + ) + + self.set_mock_side_effect("get_user", make_user_callback()) + + +class ForumFormDiscussionContentGroupTestCase( + ForumsEnableMixin, ContentGroupTestCase, ForumViewsUtilsMixin +): + """ + Tests `forum_form_discussion api` works with different content groups. + Discussion blocks are setup in ContentGroupTestCase class i.e + alpha_block => alpha_group_discussion => alpha_cohort => alpha_user/community_ta + beta_block => beta_group_discussion => beta_cohort => beta_user + """ + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def setUp(self): + super().setUp() + self.thread_list = [ + {"thread_id": "test_general_thread_id"}, + { + "thread_id": "test_global_group_thread_id", + "commentable_id": self.global_block.discussion_id, + }, + { + "thread_id": "test_alpha_group_thread_id", + "group_id": self.alpha_block.group_access[0][0], + "commentable_id": self.alpha_block.discussion_id, + }, + { + "thread_id": "test_beta_group_thread_id", + "group_id": self.beta_block.group_access[0][0], + "commentable_id": self.beta_block.discussion_id, + }, + ] + + @classmethod + def setUpClass(cls): + super().setUpClass() + super().setUpClassAndForumMock() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + super().disposeForumMocks() + + def assert_has_access(self, response, expected_discussion_threads): + """ + Verify that a users have access to the threads in their assigned + cohorts and non-cohorted blocks. + """ + discussion_data = json.loads(response.content.decode("utf-8"))[ + "discussion_data" + ] + assert len(discussion_data) == expected_discussion_threads + + def call_view( + self, user + ): # lint-amnesty, pylint: disable=missing-function-docstring + self._configure_mock_responses( + course=self.course, text="dummy content", thread_list=self.thread_list + ) + self.client.login(username=user.username, password=self.TEST_PASSWORD) + return self.client.get( + reverse("forum_form_discussion", args=[str(self.course.id)]), + HTTP_X_REQUESTED_WITH="XMLHttpRequest", + ) + + def test_community_ta_user(self): + """ + Verify that community_ta user has access to all threads regardless + of cohort. + """ + response = self.call_view(self.community_ta) + self.assert_has_access(response, 4) + + def test_alpha_cohort_user(self): + """ + Verify that alpha_user has access to alpha_cohort and non-cohorted + threads. + """ + response = self.call_view(self.alpha_user) + self.assert_has_access(response, 3) + + def test_beta_cohort_user(self): + """ + Verify that beta_user has access to beta_cohort and non-cohorted + threads. + """ + response = self.call_view(self.beta_user) + self.assert_has_access(response, 3) + + def test_global_staff_user(self): + """ + Verify that global staff user has access to all threads regardless + of cohort. + """ + response = self.call_view(self.staff_user) + self.assert_has_access(response, 4) + + +class ForumFormDiscussionUnicodeTestCase( + ForumsEnableMixin, + SharedModuleStoreTestCase, + UnicodeTestMixin, + ForumViewsUtilsMixin, +): + """ + Discussiin Unicode Tests. + """ + + @classmethod + def setUpClass(cls): # pylint: disable=super-method-not-called + super().setUpClassAndForumMock() + + with super().setUpClassAndTestData(): + cls.course = CourseFactory.create() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + super().disposeForumMocks() + + @classmethod + def setUpTestData(cls): + super().setUpTestData() + + cls.student = UserFactory.create() + CourseEnrollmentFactory(user=cls.student, course_id=cls.course.id) + + def _test_unicode_data( + self, text + ): # lint-amnesty, pylint: disable=missing-function-docstring + self._configure_mock_responses(course=self.course, text=text) + request = RequestFactory().get("dummy_url") + request.user = self.student + # so (request.headers.get('x-requested-with') == 'XMLHttpRequest') == True + request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" + + response = views.forum_form_discussion(request, str(self.course.id)) + assert response.status_code == 200 + response_data = json.loads(response.content.decode("utf-8")) + assert response_data["discussion_data"][0]["title"] == text + assert response_data["discussion_data"][0]["body"] == text + + +class EnterpriseConsentTestCase( + EnterpriseTestConsentRequired, + ForumsEnableMixin, + UrlResetMixin, + ModuleStoreTestCase, + ForumViewsUtilsMixin, +): + """ + Ensure that the Enterprise Data Consent redirects are in place only when consent is required. + """ + + CREATE_USER = False + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def setUp(self): + # Invoke UrlResetMixin setUp + super().setUp() + username = "foo" + password = "bar" + + self.discussion_id = "dummy_discussion_id" + self.course = CourseFactory.create( + discussion_topics={"dummy discussion": {"id": self.discussion_id}} + ) + self.student = UserFactory.create(username=username, password=password) + CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id) + assert self.client.login(username=username, password=password) + + self.addCleanup(translation.deactivate) + + @classmethod + def setUpClass(cls): + super().setUpClass() + super().setUpClassAndForumMock() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + super().disposeForumMocks() + + @patch("openedx.features.enterprise_support.api.enterprise_customer_for_request") + def test_consent_required(self, mock_enterprise_customer_for_request): + """ + Test that enterprise data sharing consent is required when enabled for the various discussion views. + """ + # ENT-924: Temporary solution to replace sensitive SSO usernames. + mock_enterprise_customer_for_request.return_value = None + + thread_id = "dummy" + course_id = str(self.course.id) + self._configure_mock_responses( + course=self.course, text="dummy", thread_id=thread_id + ) + + for url in ( + reverse("forum_form_discussion", kwargs=dict(course_id=course_id)), + reverse( + "single_thread", + kwargs=dict( + course_id=course_id, + discussion_id=self.discussion_id, + thread_id=thread_id, + ), + ), + ): + self.verify_consent_required( # pylint: disable=no-value-for-parameter + self.client, url + ) + + +class InlineDiscussionGroupIdTestCase( # lint-amnesty, pylint: disable=missing-class-docstring + CohortedTestCase, + CohortedTopicGroupIdTestMixinV2, + NonCohortedTopicGroupIdTestMixinV2, + ForumViewsUtilsMixin, +): + function_name = "get_user_threads" + + def setUp(self): + super().setUp() + self.cohorted_commentable_id = "cohorted_topic" + + @classmethod + def setUpClass(cls): + super().setUpClass() + super().setUpClassAndForumMock() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + super().disposeForumMocks() + + def call_view( + self, commentable_id, user, group_id, pass_group_id=True + ): # pylint: disable=arguments-differ + kwargs = {"commentable_id": self.cohorted_commentable_id} + if group_id: + # avoid causing a server error when the LMS chokes attempting + # to find a group name for the group_id, when we're testing with + # an invalid one. + try: + CourseUserGroup.objects.get(id=group_id) + kwargs["group_id"] = group_id + except CourseUserGroup.DoesNotExist: + pass + self._configure_mock_responses(self.course, "dummy content", **kwargs) + + request_data = {} + if pass_group_id: + request_data["group_id"] = group_id + request = RequestFactory().get("dummy_url", data=request_data) + request.user = user + return views.inline_discussion(request, str(self.course.id), commentable_id) + + def test_group_info_in_ajax_response(self): + response = self.call_view( + self.cohorted_commentable_id, self.student, self.student_cohort.id + ) + self._assert_json_response_contains_group_info( + response, lambda d: d["discussion_data"][0] + ) + + +class InlineDiscussionContextTestCase( + ForumsEnableMixin, ModuleStoreTestCase, ForumViewsUtilsMixin +): # lint-amnesty, pylint: disable=missing-class-docstring + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + CourseEnrollmentFactory(user=self.user, course_id=self.course.id) + self.discussion_topic_id = "dummy_topic" + self.team = CourseTeamFactory( + name="A team", + course_id=self.course.id, + topic_id="topic_id", + discussion_topic_id=self.discussion_topic_id, + ) + + self.team.add_user(self.user) + self.user_not_in_team = UserFactory.create() + + @classmethod + def setUpClass(cls): + super().setUpClass() + super().setUpClassAndForumMock() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + super().disposeForumMocks() + + def test_context_can_be_standalone(self): + self._configure_mock_responses( + course=self.course, + text="dummy text", + commentable_id=self.discussion_topic_id, + ) + + request = RequestFactory().get("dummy_url") + request.user = self.user + + response = views.inline_discussion( + request, + str(self.course.id), + self.discussion_topic_id, + ) + + json_response = json.loads(response.content.decode("utf-8")) + assert ( + json_response["discussion_data"][0]["context"] == ThreadContext.STANDALONE + ) + + def test_private_team_discussion(self): + # First set the team discussion to be private + CourseEnrollmentFactory(user=self.user_not_in_team, course_id=self.course.id) + request = RequestFactory().get("dummy_url") + request.user = self.user_not_in_team + + self._configure_mock_responses( + course=self.course, + text="dummy text", + commentable_id=self.discussion_topic_id, + ) + + with patch( + "lms.djangoapps.teams.api.is_team_discussion_private", autospec=True + ) as mocked: + mocked.return_value = True + response = views.inline_discussion( + request, + str(self.course.id), + self.discussion_topic_id, + ) + assert response.status_code == 403 + assert response.content.decode("utf-8") == views.TEAM_PERMISSION_MESSAGE + + +class UserProfileDiscussionGroupIdTestCase( + CohortedTestCase, CohortedTopicGroupIdTestMixinV2, ForumViewsUtilsMixin +): # lint-amnesty, pylint: disable=missing-class-docstring + function_name = "get_user_active_threads" + + @classmethod + def setUpClass(cls): + super().setUpClass() + super().setUpClassAndForumMock() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + super().disposeForumMocks() + + def call_view_for_profiled_user( + self, requesting_user, profiled_user, group_id, pass_group_id, is_ajax=False + ): + """ + Calls "user_profile" view method on behalf of "requesting_user" to get information about + the user "profiled_user". + """ + kwargs = {} + if group_id: + kwargs["group_id"] = group_id + self._configure_mock_responses(self.course, "dummy content", **kwargs) + + request_data = {} + if pass_group_id: + request_data["group_id"] = group_id + headers = {} + if is_ajax: + headers["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" + + self.client.login( + username=requesting_user.username, password=self.TEST_PASSWORD + ) + return self.client.get( + reverse("user_profile", args=[str(self.course.id), profiled_user.id]), + data=request_data, + **headers, + ) + + def call_view( + self, _commentable_id, user, group_id, pass_group_id=True, is_ajax=False + ): # pylint: disable=arguments-differ + return self.call_view_for_profiled_user( + user, user, group_id, pass_group_id=pass_group_id, is_ajax=is_ajax + ) + + def test_group_info_in_html_response(self): + response = self.call_view( + "cohorted_topic", self.student, self.student_cohort.id, is_ajax=False + ) + self._assert_html_response_contains_group_info(response) + + def test_group_info_in_ajax_response(self): + response = self.call_view( + "cohorted_topic", self.student, self.student_cohort.id, is_ajax=True + ) + self._assert_json_response_contains_group_info( + response, lambda d: d["discussion_data"][0] + ) + + def _test_group_id_passed_to_user_profile( + self, + expect_group_id_in_request, + requesting_user, + profiled_user, + group_id, + pass_group_id, + ): + """ + Helper method for testing whether or not group_id was passed to the user_profile request. + """ + + def get_params_from_user_info_call(for_specific_course): + """ + Returns the request parameters for the user info call with either course_id specified or not, + depending on value of 'for_specific_course'. + """ + # There will be 3 calls from user_profile. One has the cs_endpoint "active_threads", and it is already + # tested. The other 2 calls are for user info; one of those calls is for general information about the user, + # and it does not specify a course_id. The other call does specify a course_id, and if the caller did not + # have discussion moderator privileges, it should also contain a group_id. + user_func_calls = self.get_mock_func_calls("get_user") + for r_call in user_func_calls: + has_course_id = "course_id" in r_call[1] + if (for_specific_course and has_course_id) or ( + not for_specific_course and not has_course_id + ): + return r_call[1] + pytest.fail( + f"Did not find appropriate user_profile call for 'for_specific_course'={for_specific_course}" + ) + + self.call_view_for_profiled_user( + requesting_user, + profiled_user, + group_id, + pass_group_id=pass_group_id, + is_ajax=False, + ) + # Should never have a group_id if course_id was not included in the request. + params_without_course_id = get_params_from_user_info_call(False) + assert "group_ids" not in params_without_course_id + + params_with_course_id = get_params_from_user_info_call(True) + if expect_group_id_in_request: + assert "group_ids" in params_with_course_id + assert [group_id] == params_with_course_id["group_ids"] + else: + assert "group_ids" not in params_with_course_id + + def test_group_id_passed_to_user_profile_student(self): + """ + Test that the group id is always included when requesting user profile information for a particular + course if the requester does not have discussion moderation privileges. + """ + + def verify_group_id_always_present(profiled_user, pass_group_id): + """ + Helper method to verify that group_id is always present for student in course + (non-privileged user). + """ + self._test_group_id_passed_to_user_profile( + True, self.student, profiled_user, self.student_cohort.id, pass_group_id + ) + + # In all these test cases, the requesting_user is the student (non-privileged user). + # The profile returned on behalf of the student is for the profiled_user. + verify_group_id_always_present(profiled_user=self.student, pass_group_id=True) + verify_group_id_always_present(profiled_user=self.student, pass_group_id=False) + verify_group_id_always_present(profiled_user=self.moderator, pass_group_id=True) + verify_group_id_always_present( + profiled_user=self.moderator, pass_group_id=False + ) + + def test_group_id_user_profile_moderator(self): + """ + Test that the group id is only included when a privileged user requests user profile information for a + particular course and user if the group_id is explicitly passed in. + """ + + def verify_group_id_present( + profiled_user, pass_group_id, requested_cohort=self.moderator_cohort + ): + """ + Helper method to verify that group_id is present. + """ + self._test_group_id_passed_to_user_profile( + True, self.moderator, profiled_user, requested_cohort.id, pass_group_id + ) + + def verify_group_id_not_present( + profiled_user, pass_group_id, requested_cohort=self.moderator_cohort + ): + """ + Helper method to verify that group_id is not present. + """ + self._test_group_id_passed_to_user_profile( + False, self.moderator, profiled_user, requested_cohort.id, pass_group_id + ) + + # In all these test cases, the requesting_user is the moderator (privileged user). + + # If the group_id is explicitly passed, it will be present in the request. + verify_group_id_present(profiled_user=self.student, pass_group_id=True) + verify_group_id_present(profiled_user=self.moderator, pass_group_id=True) + verify_group_id_present( + profiled_user=self.student, + pass_group_id=True, + requested_cohort=self.student_cohort, + ) + + # If the group_id is not explicitly passed, it will not be present because the requesting_user + # has discussion moderator privileges. + verify_group_id_not_present(profiled_user=self.student, pass_group_id=False) + verify_group_id_not_present(profiled_user=self.moderator, pass_group_id=False) + + +@ddt.ddt +class ForumDiscussionXSSTestCase( + ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase, ForumViewsUtilsMixin +): # lint-amnesty, pylint: disable=missing-class-docstring + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def setUp(self): + super().setUp() + + username = "foo" + password = "bar" + + self.course = CourseFactory.create() + self.student = UserFactory.create(username=username, password=password) + CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id) + assert self.client.login(username=username, password=password) + + @classmethod + def setUpClass(cls): + super().setUpClass() + super().setUpClassAndForumMock() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + super().disposeForumMocks() + + @ddt.data( + '">', + "", + "", + ) + @patch("common.djangoapps.student.models.user.cc.User.from_django_user") + def test_forum_discussion_xss_prevent(self, malicious_code, mock_user): + """ + Test that XSS attack is prevented + """ + self.set_mock_return_value("get_user", {}) + self.set_mock_return_value("get_user_threads", {}) + self.set_mock_return_value("get_user_active_threads", {}) + mock_user.return_value.to_dict.return_value = {} + reverse_url = "{}{}".format( + reverse("forum_form_discussion", kwargs={"course_id": str(self.course.id)}), + "/forum_form_discussion", + ) + # Test that malicious code does not appear in html + url = "{}?{}={}".format(reverse_url, "sort_key", malicious_code) + resp = self.client.get(url) + self.assertNotContains(resp, malicious_code) + + @ddt.data( + '">', + "", + "", + ) + @patch("common.djangoapps.student.models.user.cc.User.from_django_user") + @patch("common.djangoapps.student.models.user.cc.User.active_threads") + def test_forum_user_profile_xss_prevent( + self, malicious_code, mock_threads, mock_from_django_user + ): + """ + Test that XSS attack is prevented + """ + mock_threads.return_value = [], 1, 1 + mock_from_django_user.return_value.to_dict.return_value = { + "upvoted_ids": [], + "downvoted_ids": [], + "subscribed_thread_ids": [], + } + self._configure_mock_responses(course=self.course, text="dummy") + + url = reverse( + "user_profile", + kwargs={"course_id": str(self.course.id), "user_id": str(self.student.id)}, + ) + # Test that malicious code does not appear in html + url_string = "{}?{}={}".format(url, "page", malicious_code) + resp = self.client.get(url_string) + self.assertNotContains(resp, malicious_code) + + +class InlineDiscussionTestCase( + ForumsEnableMixin, ModuleStoreTestCase, ForumViewsUtilsMixin +): # lint-amnesty, pylint: disable=missing-class-docstring + + def setUp(self): + super().setUp() + + self.course = CourseFactory.create( + org="TestX", + number="101", + display_name="Test Course", + teams_configuration=TeamsConfig( + { + "topics": [ + { + "id": "topic_id", + "name": "A topic", + "description": "A topic", + } + ] + } + ), + ) + self.student = UserFactory.create() + CourseEnrollmentFactory(user=self.student, course_id=self.course.id) + self.discussion1 = BlockFactory.create( + parent_location=self.course.location, + category="discussion", + discussion_id="discussion1", + display_name="Discussion1", + discussion_category="Chapter", + discussion_target="Discussion1", + ) + + @classmethod + def setUpClass(cls): + super().setUpClass() + super().setUpClassAndForumMock() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + super().disposeForumMocks() + + def send_request(self, params=None): + """ + Creates and returns a request with params set, and configures + mock_request to return appropriate values. + """ + request = RequestFactory().get("dummy_url", params if params else {}) + request.user = self.student + self._configure_mock_responses( + course=self.course, + text="dummy content", + commentable_id=self.discussion1.discussion_id, + ) + return views.inline_discussion( + request, str(self.course.id), self.discussion1.discussion_id + ) + + def test_context(self): + team = CourseTeamFactory( + name="Team Name", + topic_id="topic_id", + course_id=self.course.id, + discussion_topic_id=self.discussion1.discussion_id, + ) + + team.add_user(self.student) + + self.send_request() + last_call = self.get_mock_func_calls("get_user_threads")[-1][1] + assert last_call["context"] == ThreadContext.STANDALONE + + +class ForumDiscussionSearchUnicodeTestCase( + ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, ForumViewsUtilsMixin +): # lint-amnesty, pylint: disable=missing-class-docstring + + @classmethod + def setUpClass(cls): # pylint: disable=super-method-not-called + super().setUpClassAndForumMock() + with super().setUpClassAndTestData(): + cls.course = CourseFactory.create() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + super().disposeForumMocks() + + @classmethod + def setUpTestData(cls): + super().setUpTestData() + + cls.student = UserFactory.create() + CourseEnrollmentFactory(user=cls.student, course_id=cls.course.id) + + def _test_unicode_data( + self, text + ): # lint-amnesty, pylint: disable=missing-function-docstring + self._configure_mock_responses(course=self.course, text=text) + data = { + "ajax": 1, + "text": text, + } + request = RequestFactory().get("dummy_url", data) + request.user = self.student + # so (request.headers.get('x-requested-with') == 'XMLHttpRequest') == True + request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" + + response = views.forum_form_discussion(request, str(self.course.id)) + assert response.status_code == 200 + response_data = json.loads(response.content.decode("utf-8")) + assert response_data["discussion_data"][0]["title"] == text + assert response_data["discussion_data"][0]["body"] == text + + +class InlineDiscussionUnicodeTestCase( + ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, ForumViewsUtilsMixin +): # lint-amnesty, pylint: disable=missing-class-docstring + + @classmethod + def setUpClass(cls): # pylint: disable=super-method-not-called + super().setUpClassAndForumMock() + + with super().setUpClassAndTestData(): + cls.course = CourseFactory.create() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + super().disposeForumMocks() + + @classmethod + def setUpTestData(cls): + super().setUpTestData() + + cls.student = UserFactory.create() + CourseEnrollmentFactory(user=cls.student, course_id=cls.course.id) + + def _test_unicode_data( + self, text + ): # lint-amnesty, pylint: disable=missing-function-docstring + self._configure_mock_responses(course=self.course, text=text) + request = RequestFactory().get("dummy_url") + request.user = self.student + + response = views.inline_discussion( + request, str(self.course.id), self.course.discussion_topics["General"]["id"] + ) + assert response.status_code == 200 + response_data = json.loads(response.content.decode("utf-8")) + assert response_data["discussion_data"][0]["title"] == text + assert response_data["discussion_data"][0]["body"] == text + + +class ForumFormDiscussionGroupIdTestCase( + CohortedTestCase, CohortedTopicGroupIdTestMixinV2, ForumViewsUtilsMixin +): # lint-amnesty, pylint: disable=missing-class-docstring + function_name = "get_user_threads" + + def call_view( + self, commentable_id, user, group_id, pass_group_id=True, is_ajax=False + ): # pylint: disable=arguments-differ + kwargs = {} + if group_id: + kwargs["group_id"] = group_id + self._configure_mock_responses(self.course, "dummy content", **kwargs) + + request_data = {} + if pass_group_id: + request_data["group_id"] = group_id + headers = {} + if is_ajax: + headers["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" + + self.client.login(username=user.username, password=self.TEST_PASSWORD) + return self.client.get( + reverse("forum_form_discussion", args=[str(self.course.id)]), + data=request_data, + **headers, + ) + + @classmethod + def setUpClass(cls): + super().setUpClass() + super().setUpClassAndForumMock() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + super().disposeForumMocks() + + def test_group_info_in_html_response(self): + response = self.call_view( + "cohorted_topic", self.student, self.student_cohort.id + ) + self._assert_html_response_contains_group_info(response) + + def test_group_info_in_ajax_response(self): + response = self.call_view( + "cohorted_topic", self.student, self.student_cohort.id, is_ajax=True + ) + self._assert_json_response_contains_group_info( + response, lambda d: d["discussion_data"][0] + ) + + +class UserProfileTestCase( + ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase, ForumViewsUtilsMixin +): # lint-amnesty, pylint: disable=missing-class-docstring + + TEST_THREAD_TEXT = "userprofile-test-text" + TEST_THREAD_ID = "userprofile-test-thread-id" + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def setUp(self): + super().setUp() + + self.course = CourseFactory.create() + self.student = UserFactory.create() + self.profiled_user = UserFactory.create() + CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id) + CourseEnrollmentFactory.create( + user=self.profiled_user, course_id=self.course.id + ) + + @classmethod + def setUpClass(cls): + super().setUpClass() + super().setUpClassAndForumMock() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + super().disposeForumMocks() + + def get_response( + self, params, **headers + ): # lint-amnesty, pylint: disable=missing-function-docstring + self._configure_mock_responses( + course=self.course, + text=self.TEST_THREAD_TEXT, + thread_id=self.TEST_THREAD_ID, + ) + self.client.login(username=self.student.username, password=self.TEST_PASSWORD) + + response = self.client.get( + reverse( + "user_profile", + kwargs={ + "course_id": str(self.course.id), + "user_id": self.profiled_user.id, + }, + ), + data=params, + **headers, + ) + params = { + "course_id": str(self.course.id), + "page": params.get("page", 1), + "per_page": views.THREADS_PER_PAGE, + } + self.check_mock_called_with("get_user_active_threads", -1, **params) + return response + + def check_html( + self, **params + ): # lint-amnesty, pylint: disable=missing-function-docstring + response = self.get_response(params) + assert response.status_code == 200 + assert response["Content-Type"] == "text/html; charset=utf-8" + html = response.content.decode("utf-8") + self.assertRegex(html, r'data-page="1"') + self.assertRegex(html, r'data-num-pages="1"') + self.assertRegex( + html, r'1 discussion started' + ) + self.assertRegex(html, r'2 comments') + self.assertRegex(html, f"'id': '{self.TEST_THREAD_ID}'") + self.assertRegex(html, f"'title': '{self.TEST_THREAD_TEXT}'") + self.assertRegex(html, f"'body': '{self.TEST_THREAD_TEXT}'") + self.assertRegex(html, f"'username': '{self.student.username}'") + + def check_ajax( + self, **params + ): # lint-amnesty, pylint: disable=missing-function-docstring + response = self.get_response(params, HTTP_X_REQUESTED_WITH="XMLHttpRequest") + assert response.status_code == 200 + assert response["Content-Type"] == "application/json; charset=utf-8" + response_data = json.loads(response.content.decode("utf-8")) + assert sorted(response_data.keys()) == [ + "annotated_content_info", + "discussion_data", + "num_pages", + "page", + ] + assert len(response_data["discussion_data"]) == 1 + assert response_data["page"] == 1 + assert response_data["num_pages"] == 1 + assert response_data["discussion_data"][0]["id"] == self.TEST_THREAD_ID + assert response_data["discussion_data"][0]["title"] == self.TEST_THREAD_TEXT + assert response_data["discussion_data"][0]["body"] == self.TEST_THREAD_TEXT + + def test_html(self): + self.check_html() + + def test_ajax(self): + self.check_ajax() + + def test_404_non_enrolled_user(self): + """ + Test that when student try to visit un-enrolled students' discussion profile, + the system raises Http404. + """ + unenrolled_user = UserFactory.create() + request = RequestFactory().get("dummy_url") + request.user = self.student + with pytest.raises(Http404): + views.user_profile(request, str(self.course.id), unenrolled_user.id) + + def test_404_profiled_user(self): + request = RequestFactory().get("dummy_url") + request.user = self.student + with pytest.raises(Http404): + views.user_profile(request, str(self.course.id), -999) + + def test_404_course(self): + request = RequestFactory().get("dummy_url") + request.user = self.student + with pytest.raises(Http404): + views.user_profile(request, "non/existent/course", self.profiled_user.id) + + def test_post(self): + self._configure_mock_responses( + course=self.course, + text=self.TEST_THREAD_TEXT, + thread_id=self.TEST_THREAD_ID, + ) + request = RequestFactory().post("dummy_url") + request.user = self.student + response = views.user_profile( + request, str(self.course.id), self.profiled_user.id + ) + assert response.status_code == 405 diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/thread.py b/openedx/core/djangoapps/django_comment_common/comment_client/thread.py index 49aa8f9bc1..de994bb13a 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/thread.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/thread.py @@ -56,42 +56,28 @@ class Thread(models.Model): utils.strip_blank(utils.strip_none(query_params)) ) - if query_params.get('text'): - url = cls.url(action='search') - else: - url = cls.url(action='get_all', params=utils.extract(params, 'commentable_id')) - if params.get('commentable_id'): - del params['commentable_id'] + # Convert user_id and author_id to strings if present + for field in ['user_id', 'author_id']: + if value := params.get(field): + params[field] = str(value) - if is_forum_v2_enabled(utils.get_course_key(query_params['course_id'])): - if query_params.get('text'): - search_params = utils.strip_none(params) - if user_id := search_params.get('user_id'): - search_params['user_id'] = str(user_id) - if group_ids := search_params.get('group_ids'): - search_params['group_ids'] = [int(group_id) for group_id in group_ids.split(',')] - elif group_id := search_params.get('group_id'): - search_params['group_ids'] = [int(group_id)] - search_params.pop('group_id', None) - if commentable_ids := search_params.get('commentable_ids'): - search_params['commentable_ids'] = commentable_ids.split(',') - elif commentable_id := search_params.get('commentable_id'): - search_params['commentable_ids'] = [commentable_id] - search_params.pop('commentable_id', None) - response = forum_api.search_threads(**search_params) - else: - if user_id := params.get('user_id'): - params['user_id'] = str(user_id) - response = forum_api.get_user_threads(**params) + # Handle commentable_ids/commentable_id conversion + if commentable_ids := params.get('commentable_ids'): + params['commentable_ids'] = commentable_ids.split(',') + elif commentable_id := params.get('commentable_id'): + params['commentable_ids'] = [commentable_id] + params.pop('commentable_id', None) + + params = utils.clean_forum_params(params) + if query_params.get('text'): # Handle group_ids/group_id conversion + if group_ids := params.get('group_ids'): + params['group_ids'] = [int(group_id) for group_id in group_ids.split(',')] + elif group_id := params.get('group_id'): + params['group_ids'] = [int(group_id)] + params.pop('group_id', None) + response = forum_api.search_threads(**params) else: - response = utils.perform_request( - 'get', - url, - params, - metric_tags=['course_id:{}'.format(query_params['course_id'])], - metric_action='thread.search', - paged_results=True - ) + response = forum_api.get_user_threads(**params) if query_params.get('text'): search_query = query_params['text'] @@ -124,7 +110,6 @@ class Thread(models.Model): total_results=total_results ) ) - return utils.CommentClientPaginatedResult( collection=response.get('collection', []), page=response.get('page', 1), diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/user.py b/openedx/core/djangoapps/django_comment_common/comment_client/user.py index 731825aa71..ee9591e51d 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/user.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/user.py @@ -181,7 +181,7 @@ class User(models.Model): user_id = params.pop("user_id", None) if "text" in params: params.pop("text") - response = forum_api.get_user_subscriptions(user_id, str(course_key), params) + response = forum_api.get_user_subscriptions(user_id, str(course_key), utils.clean_forum_params(params)) else: response = utils.perform_request( 'get', @@ -218,21 +218,17 @@ class User(models.Model): if is_forum_v2_enabled(course_key): group_ids = [retrieve_params['group_id']] if 'group_id' in retrieve_params else [] is_complete = retrieve_params['complete'] + params = utils.clean_forum_params({ + "user_id": self.attributes["id"], + "group_ids": group_ids, + "course_id": course_id, + "complete": is_complete + }) try: - response = forum_api.get_user( - self.attributes["id"], - group_ids=group_ids, - course_id=course_id, - complete=is_complete - ) + response = forum_api.get_user(**params) except ForumV2RequestError as e: self.save({"course_id": course_id}) - response = forum_api.get_user( - self.attributes["id"], - group_ids=group_ids, - course_id=course_id, - complete=is_complete - ) + response = forum_api.get_user(**params) else: try: response = utils.perform_request( diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/utils.py b/openedx/core/djangoapps/django_comment_common/comment_client/utils.py index e77f39e627..26625ed3a7 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/utils.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/utils.py @@ -103,6 +103,23 @@ def perform_request(method, url, data_or_params=None, raw=False, return data +def clean_forum_params(params): + """Convert string booleans to actual booleans and remove None values and empty lists from forum parameters.""" + result = {} + for k, v in params.items(): + if v is not None and v != []: + if isinstance(v, str): + if v.lower() == 'true': + result[k] = True + elif v.lower() == 'false': + result[k] = False + else: + result[k] = v + else: + result[k] = v + return result + + class CommentClientError(Exception): pass From fc84ff39d6f48271b4cc772db1226cac2f551957 Mon Sep 17 00:00:00 2001 From: Maxwell Frank Date: Thu, 26 Jun 2025 18:03:25 +0000 Subject: [PATCH 4/9] fix: add oel-publishing migration dependency to contentstore 0010 --- .../contentstore/migrations/0010_container_link_models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cms/djangoapps/contentstore/migrations/0010_container_link_models.py b/cms/djangoapps/contentstore/migrations/0010_container_link_models.py index 7a02a1c74d..8d42ad9614 100644 --- a/cms/djangoapps/contentstore/migrations/0010_container_link_models.py +++ b/cms/djangoapps/contentstore/migrations/0010_container_link_models.py @@ -10,6 +10,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ + ('oel_publishing', '0003_containers'), ('oel_components', '0003_remove_componentversioncontent_learner_downloadable'), ('contentstore', '0009_learningcontextlinksstatus_publishableentitylink'), ] From 8fac3bc060ca7c79dce33117903b715841d4385f Mon Sep 17 00:00:00 2001 From: Hassan Raza Date: Fri, 27 Jun 2025 15:25:22 +0500 Subject: [PATCH 5/9] feat: Add notify all learners option for discussion post (#36922) * feat: Add notify all learners option for discussion post * fix: Remove waffle flag from default notification dict --- lms/djangoapps/discussion/rest_api/api.py | 16 ++++-- .../rest_api/discussions_notifications.py | 7 +-- lms/djangoapps/discussion/rest_api/tasks.py | 15 +++++- .../discussion/rest_api/tests/test_api.py | 13 +++-- .../discussion/rest_api/tests/test_tasks.py | 49 +++++++++++++------ .../discussion/rest_api/tests/test_views.py | 1 + lms/djangoapps/discussion/rest_api/utils.py | 23 +++++++++ lms/djangoapps/discussion/signals/handlers.py | 5 +- .../notifications/base_notification.py | 18 +++++++ .../djangoapps/notifications/config/waffle.py | 10 ++++ .../core/djangoapps/notifications/models.py | 2 +- .../notifications/tests/test_utils.py | 5 +- .../core/djangoapps/notifications/utils.py | 15 +++++- .../core/djangoapps/notifications/views.py | 14 +++++- 14 files changed, 158 insertions(+), 35 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/api.py b/lms/djangoapps/discussion/rest_api/api.py index 100482ea56..e1f83cd7e6 100644 --- a/lms/djangoapps/discussion/rest_api/api.py +++ b/lms/djangoapps/discussion/rest_api/api.py @@ -127,7 +127,8 @@ from .utils import ( get_usernames_for_course, get_usernames_from_search_string, set_attribute, - is_posting_allowed + is_posting_allowed, + can_user_notify_all_learners ) User = get_user_model() @@ -333,6 +334,8 @@ def get_course(request, course_key, check_tab=True): course.get_discussion_blackout_datetimes() ) discussion_tab = CourseTabList.get_tab_by_type(course.tabs, 'discussion') + is_course_staff = CourseStaffRole(course_key).has_user(request.user) + is_course_admin = CourseInstructorRole(course_key).has_user(request.user) return { "id": str(course_key), "is_posting_enabled": is_posting_enabled, @@ -358,8 +361,8 @@ def get_course(request, course_key, check_tab=True): }), "is_group_ta": bool(user_roles & {FORUM_ROLE_GROUP_MODERATOR}), "is_user_admin": request.user.is_staff, - "is_course_staff": CourseStaffRole(course_key).has_user(request.user), - "is_course_admin": CourseInstructorRole(course_key).has_user(request.user), + "is_course_staff": is_course_staff, + "is_course_admin": is_course_admin, "provider": course_config.provider_type, "enable_in_context": course_config.enable_in_context, "group_at_subsection": course_config.plugin_configuration.get("group_at_subsection", False), @@ -372,6 +375,9 @@ def get_course(request, course_key, check_tab=True): for (reason_code, label) in CLOSE_REASON_CODES.items() ], 'show_discussions': bool(discussion_tab and discussion_tab.is_enabled(course, request.user)), + 'is_notify_all_learners_enabled': can_user_notify_all_learners( + course_key, user_roles, is_course_staff, is_course_admin + ), } @@ -1469,6 +1475,8 @@ def create_thread(request, thread_data): if not discussion_open_for_user(course, user): raise DiscussionBlackOutException + notify_all_learners = thread_data.pop("notify_all_learners", False) + context = get_context(course, request) _check_initializable_thread_fields(thread_data, context) discussion_settings = CourseDiscussionSettings.get(course_key) @@ -1484,7 +1492,7 @@ def create_thread(request, thread_data): raise ValidationError(dict(list(serializer.errors.items()) + list(actions_form.errors.items()))) serializer.save() cc_thread = serializer.instance - thread_created.send(sender=None, user=user, post=cc_thread) + thread_created.send(sender=None, user=user, post=cc_thread, notify_all_learners=notify_all_learners) api_thread = serializer.data _do_extra_actions(api_thread, cc_thread, list(thread_data.keys()), actions_form, context, request) diff --git a/lms/djangoapps/discussion/rest_api/discussions_notifications.py b/lms/djangoapps/discussion/rest_api/discussions_notifications.py index 57e231b966..648259249d 100644 --- a/lms/djangoapps/discussion/rest_api/discussions_notifications.py +++ b/lms/djangoapps/discussion/rest_api/discussions_notifications.py @@ -318,17 +318,18 @@ class DiscussionNotificationSender: self._populate_context_with_ids_for_mobile(context, notification_type) self._send_notification([self.creator.id], notification_type, extra_context=context) - def send_new_thread_created_notification(self): + def send_new_thread_created_notification(self, notify_all_learners=False): """ Send notification based on notification_type """ thread_type = self.thread.attributes['thread_type'] - notification_type = ( + + notification_type = "new_instructor_all_learners_post" if notify_all_learners else ( "new_question_post" if thread_type == "question" else ("new_discussion_post" if thread_type == "discussion" else "") ) - if notification_type not in ['new_discussion_post', 'new_question_post']: + if notification_type not in ['new_discussion_post', 'new_question_post', 'new_instructor_all_learners_post']: raise ValueError(f'Invalid notification type {notification_type}') audience_filters = self._create_cohort_course_audience() diff --git a/lms/djangoapps/discussion/rest_api/tasks.py b/lms/djangoapps/discussion/rest_api/tasks.py index cbf4389889..e87804b1ca 100644 --- a/lms/djangoapps/discussion/rest_api/tasks.py +++ b/lms/djangoapps/discussion/rest_api/tasks.py @@ -6,8 +6,11 @@ from django.contrib.auth import get_user_model from edx_django_utils.monitoring import set_code_owner_attribute from opaque_keys.edx.locator import CourseKey +from common.djangoapps.student.roles import CourseStaffRole, CourseInstructorRole from lms.djangoapps.courseware.courses import get_course_with_access +from lms.djangoapps.discussion.django_comment_client.utils import get_user_role_names from lms.djangoapps.discussion.rest_api.discussions_notifications import DiscussionNotificationSender +from lms.djangoapps.discussion.rest_api.utils import can_user_notify_all_learners from openedx.core.djangoapps.django_comment_common.comment_client import Comment from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS @@ -17,7 +20,7 @@ User = get_user_model() @shared_task @set_code_owner_attribute -def send_thread_created_notification(thread_id, course_key_str, user_id): +def send_thread_created_notification(thread_id, course_key_str, user_id, notify_all_learners=False): """ Send notification when a new thread is created """ @@ -26,9 +29,17 @@ def send_thread_created_notification(thread_id, course_key_str, user_id): return thread = Thread(id=thread_id).retrieve() user = User.objects.get(id=user_id) + + if notify_all_learners: + is_course_staff = CourseStaffRole(course_key).has_user(user) + is_course_admin = CourseInstructorRole(course_key).has_user(user) + user_roles = get_user_role_names(user, course_key) + if not can_user_notify_all_learners(course_key, user_roles, is_course_staff, is_course_admin): + return + course = get_course_with_access(user, 'load', course_key, check_if_enrolled=True) notification_sender = DiscussionNotificationSender(thread, course, user) - notification_sender.send_new_thread_created_notification() + notification_sender.send_new_thread_created_notification(notify_all_learners) @shared_task diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index 48b943543c..d0eae92316 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -214,6 +214,7 @@ class GetCourseTest(ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase) 'edit_reasons': [{'code': 'test-edit-reason', 'label': 'Test Edit Reason'}], 'post_close_reasons': [{'code': 'test-close-reason', 'label': 'Test Close Reason'}], 'show_discussions': True, + 'is_notify_all_learners_enabled': False } @ddt.data( @@ -1379,7 +1380,9 @@ class CreateThreadTest( "read": True, }) self.register_post_thread_response(cs_thread) - with self.assert_signal_sent(api, 'thread_created', sender=None, user=self.user, exclude_args=('post',)): + with self.assert_signal_sent( + api, 'thread_created', sender=None, user=self.user, exclude_args=('post', 'notify_all_learners') + ): actual = create_thread(self.request, self.minimal_data) expected = self.expected_thread_data({ "id": "test_id", @@ -1445,7 +1448,9 @@ class CreateThreadTest( _assign_role_to_user(user=self.user, course_id=self.course.id, role=FORUM_ROLE_MODERATOR) - with self.assert_signal_sent(api, 'thread_created', sender=None, user=self.user, exclude_args=('post',)): + with self.assert_signal_sent( + api, 'thread_created', sender=None, user=self.user, exclude_args=('post', 'notify_all_learners') + ): actual = create_thread(self.request, self.minimal_data) expected = self.expected_thread_data({ "author_label": "Moderator", @@ -1517,7 +1522,9 @@ class CreateThreadTest( "read": True, }) self.register_post_thread_response(cs_thread) - with self.assert_signal_sent(api, 'thread_created', sender=None, user=self.user, exclude_args=('post',)): + with self.assert_signal_sent( + api, 'thread_created', sender=None, user=self.user, exclude_args=('post', 'notify_all_learners') + ): create_thread(self.request, data) event_name, event_data = mock_emit.call_args[0] assert event_name == 'edx.forum.thread.created' diff --git a/lms/djangoapps/discussion/rest_api/tests/test_tasks.py b/lms/djangoapps/discussion/rest_api/tests/test_tasks.py index a6eb4948ce..70aeb8dd4a 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_tasks.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_tasks.py @@ -29,7 +29,7 @@ from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_STUDENT, CourseDiscussionSettings ) -from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS +from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS, ENABLE_NOTIFY_ALL_LEARNERS from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -190,29 +190,46 @@ class TestNewThreadCreatedNotification(DiscussionAPIViewTestMixin, ModuleStoreTe """ @ddt.data( - ('new_question_post',), - ('new_discussion_post',), + ('new_question_post', False, False), + ('new_discussion_post', False, False), + ('new_discussion_post', True, True), + ('new_discussion_post', True, False), ) @ddt.unpack - def test_notification_is_send_to_all_enrollments(self, notification_type): + def test_notification_is_send_to_all_enrollments( + self, notification_type, notify_all_learners, waffle_flag_enabled + ): """ Tests notification is sent to all users if course is not cohorted """ self._assign_enrollments() thread_type = ( - "discussion" - if notification_type == "new_discussion_post" - else ("question" if notification_type == "new_question_post" else "") + "discussion" if notification_type == "new_discussion_post" else "question" ) - thread = self._create_thread(thread_type=thread_type) - handler = mock.Mock() - COURSE_NOTIFICATION_REQUESTED.connect(handler) - send_thread_created_notification(thread['id'], str(self.course.id), self.author.id) - self.assertEqual(handler.call_count, 1) - course_notification_data = handler.call_args[1]['course_notification_data'] - assert notification_type == course_notification_data.notification_type - notification_audience_filters = {} - assert notification_audience_filters == course_notification_data.audience_filters + + with override_waffle_flag(ENABLE_NOTIFY_ALL_LEARNERS, active=waffle_flag_enabled): + thread = self._create_thread(thread_type=thread_type) + handler = mock.Mock() + COURSE_NOTIFICATION_REQUESTED.connect(handler) + + send_thread_created_notification( + thread['id'], + str(self.course.id), + self.author.id, + notify_all_learners + ) + expected_handler_calls = 0 if notify_all_learners and not waffle_flag_enabled else 1 + self.assertEqual(handler.call_count, expected_handler_calls) + + if handler.call_count: + course_notification_data = handler.call_args[1]['course_notification_data'] + expected_type = ( + 'new_instructor_all_learners_post' + if notify_all_learners and waffle_flag_enabled + else notification_type + ) + self.assertEqual(course_notification_data.notification_type, expected_type) + self.assertEqual(course_notification_data.audience_filters, {}) @ddt.data( ('cohort_1', 'new_question_post'), diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index 1df43ee4f9..3e11b1ba5f 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -557,6 +557,7 @@ class CourseViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "edit_reasons": [{"code": "test-edit-reason", "label": "Test Edit Reason"}], "post_close_reasons": [{"code": "test-close-reason", "label": "Test Close Reason"}], 'show_discussions': True, + 'is_notify_all_learners_enabled': False } ) diff --git a/lms/djangoapps/discussion/rest_api/utils.py b/lms/djangoapps/discussion/rest_api/utils.py index e7dca49910..bddd81c3f0 100644 --- a/lms/djangoapps/discussion/rest_api/utils.py +++ b/lms/djangoapps/discussion/rest_api/utils.py @@ -11,6 +11,7 @@ from pytz import UTC from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole from lms.djangoapps.discussion.django_comment_client.utils import has_discussion_privileges +from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFY_ALL_LEARNERS from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, PostingRestriction from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_ADMINISTRATOR, @@ -379,3 +380,25 @@ def is_posting_allowed(posting_restrictions: str, blackout_schedules: List): return not any(schedule["start"] <= now <= schedule["end"] for schedule in blackout_schedules) else: return False + + +def can_user_notify_all_learners(course_key, user_roles, is_course_staff, is_course_admin): + """ + Check if user posting is allowed to notify all learners based on the given restrictions + + Args: + course_key (CourseKey): CourseKey for which user creating any discussion post. + user_roles (Dict): Roles of the posting user + is_course_staff (Boolean): Whether the user has a course staff access. + is_course_admin (Boolean): Whether the user has a course admin access. + + Returns: + bool: True if posting for all learner is allowed to this user, False otherwise. + """ + is_staff_or_instructor = any([ + user_roles.intersection({FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR}), + is_course_staff, + is_course_admin, + ]) + + return is_staff_or_instructor and ENABLE_NOTIFY_ALL_LEARNERS.is_enabled(course_key) diff --git a/lms/djangoapps/discussion/signals/handlers.py b/lms/djangoapps/discussion/signals/handlers.py index 73c19d2785..ec14cd8281 100644 --- a/lms/djangoapps/discussion/signals/handlers.py +++ b/lms/djangoapps/discussion/signals/handlers.py @@ -166,7 +166,10 @@ def create_thread_created_notification(*args, **kwargs): """ user = kwargs['user'] post = kwargs['post'] - send_thread_created_notification.apply_async(args=[post.id, post.attributes['course_id'], user.id]) + notify_all_learners = kwargs.get('notify_all_learners', False) + send_thread_created_notification.apply_async( + args=[post.id, post.attributes['course_id'], user.id, notify_all_learners] + ) @receiver(signals.comment_created) diff --git a/openedx/core/djangoapps/notifications/base_notification.py b/openedx/core/djangoapps/notifications/base_notification.py index e47486df32..d345eb1263 100644 --- a/openedx/core/djangoapps/notifications/base_notification.py +++ b/openedx/core/djangoapps/notifications/base_notification.py @@ -230,6 +230,24 @@ COURSE_NOTIFICATION_TYPES = { 'email_template': '', 'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE], }, + 'new_instructor_all_learners_post': { + 'notification_app': 'discussion', + 'name': 'new_instructor_all_learners_post', + 'is_core': False, + 'info': '', + 'web': True, + 'email': False, + 'email_cadence': EmailCadence.DAILY, + 'push': False, + 'non_editable': [], + 'content_template': _('<{p}>Your instructor posted <{strong}>{post_title}'), + 'grouped_content_template': '', + 'content_context': { + 'post_title': 'Post title', + }, + 'email_template': '', + 'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE] + }, } COURSE_NOTIFICATION_APPS = { diff --git a/openedx/core/djangoapps/notifications/config/waffle.py b/openedx/core/djangoapps/notifications/config/waffle.py index 4a1481f6f5..cfc7e1039f 100644 --- a/openedx/core/djangoapps/notifications/config/waffle.py +++ b/openedx/core/djangoapps/notifications/config/waffle.py @@ -49,3 +49,13 @@ ENABLE_ORA_GRADE_NOTIFICATION = CourseWaffleFlag(f"{WAFFLE_NAMESPACE}.enable_ora # .. toggle_warning: When the flag is ON, Notifications Grouping feature is enabled. # .. toggle_tickets: INF-1472 ENABLE_NOTIFICATION_GROUPING = CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.enable_notification_grouping', __name__) + +# .. toggle_name: notifications.post_enable_notify_all_learners +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: Waffle flag to enable the notify all learners on discussion post +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2025-06-11 +# .. toggle_warning: When the flag is ON, notification to all learners feature is enabled on discussion post. +# .. toggle_tickets: INF-1917 +ENABLE_NOTIFY_ALL_LEARNERS = CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.enable_post_notify_all_learners', __name__) diff --git a/openedx/core/djangoapps/notifications/models.py b/openedx/core/djangoapps/notifications/models.py index 2864fe2408..fc994b46ea 100644 --- a/openedx/core/djangoapps/notifications/models.py +++ b/openedx/core/djangoapps/notifications/models.py @@ -26,7 +26,7 @@ NOTIFICATION_CHANNELS = ['web', 'push', 'email'] ADDITIONAL_NOTIFICATION_CHANNEL_SETTINGS = ['email_cadence'] # Update this version when there is a change to any course specific notification type or app. -COURSE_NOTIFICATION_CONFIG_VERSION = 13 +COURSE_NOTIFICATION_CONFIG_VERSION = 14 def get_course_notification_preference_config(): diff --git a/openedx/core/djangoapps/notifications/tests/test_utils.py b/openedx/core/djangoapps/notifications/tests/test_utils.py index 066c7b7f59..c00f2ba237 100644 --- a/openedx/core/djangoapps/notifications/tests/test_utils.py +++ b/openedx/core/djangoapps/notifications/tests/test_utils.py @@ -311,7 +311,10 @@ class TestVisibilityFilter(unittest.TestCase): 'core': {'web': True, 'push': True, 'email': True, 'email_cadence': 'Daily'}, 'content_reported': {'web': True, 'push': True, 'email': True, 'email_cadence': 'Daily'}, 'new_question_post': {'web': False, 'push': False, 'email': False, 'email_cadence': 'Daily'}, - 'new_discussion_post': {'web': False, 'push': False, 'email': False, 'email_cadence': 'Daily'} + 'new_discussion_post': {'web': False, 'push': False, 'email': False, 'email_cadence': 'Daily'}, + 'new_instructor_all_learners_post': { + 'web': True, 'push': False, 'email': False, 'email_cadence': 'Daily' + } }, 'core_notification_types': [ 'new_response', 'comment_on_followed_post', diff --git a/openedx/core/djangoapps/notifications/utils.py b/openedx/core/djangoapps/notifications/utils.py index 62e51b7ab9..762777e35b 100644 --- a/openedx/core/djangoapps/notifications/utils.py +++ b/openedx/core/djangoapps/notifications/utils.py @@ -4,9 +4,11 @@ Utils function for notifications app import copy from typing import Dict, List, Set +from opaque_keys.edx.keys import CourseKey + from common.djangoapps.student.models import CourseAccessRole, CourseEnrollment from openedx.core.djangoapps.django_comment_common.models import Role -from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS +from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS, ENABLE_NOTIFY_ALL_LEARNERS from openedx.core.lib.cache_utils import request_cached @@ -132,12 +134,21 @@ def remove_preferences_with_no_access(preferences: dict, user) -> dict: user=user, course_id=preferences['course_id'] ).values_list('role', flat=True) - preferences['notification_preference_config'] = filter_out_visible_notifications( + + user_preferences = filter_out_visible_notifications( user_preferences, notifications_with_visibility_settings, user_forum_roles, user_course_roles ) + + course_key = CourseKey.from_string(preferences['course_id']) + discussion_config = user_preferences.get('discussion', {}) + notification_types = discussion_config.get('notification_types', {}) + + if notification_types and not ENABLE_NOTIFY_ALL_LEARNERS.is_enabled(course_key): + notification_types.pop('new_instructor_all_learners_post', None) + return preferences diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index 49e368ed1a..bece28da1f 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -26,7 +26,7 @@ from openedx.core.djangoapps.notifications.serializers import add_info_to_notifi from openedx.core.djangoapps.user_api.models import UserPreference from .base_notification import COURSE_NOTIFICATION_APPS -from .config.waffle import ENABLE_NOTIFICATIONS +from .config.waffle import ENABLE_NOTIFICATIONS, ENABLE_NOTIFY_ALL_LEARNERS from .events import ( notification_preference_update_event, notification_preferences_viewed_event, @@ -603,13 +603,23 @@ class AggregatedNotificationPreferences(APIView): notification_configs = aggregate_notification_configs( notification_configs ) + course_ids = notification_preferences.values_list('course_id', flat=True) + filter_out_visible_preferences_by_course_ids( request.user, notification_configs, - notification_preferences.values_list('course_id', flat=True), + course_ids, ) + notification_preferences_viewed_event(request) notification_configs = add_info_to_notification_config(notification_configs) + + discussion_config = notification_configs.get('discussion', {}) + notification_types = discussion_config.get('notification_types', {}) + + if not any(ENABLE_NOTIFY_ALL_LEARNERS.is_enabled(course_key) for course_key in course_ids): + notification_types.pop('new_instructor_all_learners_post', None) + return Response({ 'status': 'success', 'message': 'Notification preferences retrieved', From e101298fed1edcad67124cc2f5396e422a9e6f42 Mon Sep 17 00:00:00 2001 From: jawad khan Date: Mon, 30 Jun 2025 10:48:48 +0500 Subject: [PATCH 6/9] feat: send mobile push notifications (#36272) * feat: send mobile braze notifications * fix: fixed pylint issues * feat: Added push_notification flag and preferences * fix: fixed pylint issues * fix: Moved braze logic to edx-ace * fix: Un delete admin file * fix: Added review suggestions * fix: Added review suggestions * fix: updated migration file * fix: Removed all braze references from changes * fix: fixed test cases * fix: removed braze metnion in code * fix: fixed migration file issue * fix: Added review suggestions * fix: bumped edx-ace version --- .../djangoapps/notifications/config/waffle.py | 10 +++ .../migrations/0009_notification_push.py | 18 +++++ .../core/djangoapps/notifications/models.py | 1 + .../djangoapps/notifications/push/__init__.py | 0 .../notifications/push/message_type.py | 10 +++ .../djangoapps/notifications/push/tasks.py | 45 ++++++++++++ .../notifications/push/tests/__init__.py | 0 .../notifications/push/tests/test_tasks.py | 73 +++++++++++++++++++ .../core/djangoapps/notifications/tasks.py | 36 ++++++--- .../notifications/edx_ace/push/push/body.txt | 0 .../notifications/edx_ace/push/push/title.txt | 0 .../notifications/tests/test_tasks.py | 22 ++++-- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 16 files changed, 199 insertions(+), 24 deletions(-) create mode 100644 openedx/core/djangoapps/notifications/migrations/0009_notification_push.py create mode 100644 openedx/core/djangoapps/notifications/push/__init__.py create mode 100644 openedx/core/djangoapps/notifications/push/message_type.py create mode 100644 openedx/core/djangoapps/notifications/push/tasks.py create mode 100644 openedx/core/djangoapps/notifications/push/tests/__init__.py create mode 100644 openedx/core/djangoapps/notifications/push/tests/test_tasks.py create mode 100644 openedx/core/djangoapps/notifications/templates/notifications/edx_ace/push/push/body.txt create mode 100644 openedx/core/djangoapps/notifications/templates/notifications/edx_ace/push/push/title.txt diff --git a/openedx/core/djangoapps/notifications/config/waffle.py b/openedx/core/djangoapps/notifications/config/waffle.py index cfc7e1039f..12b0ef5ee1 100644 --- a/openedx/core/djangoapps/notifications/config/waffle.py +++ b/openedx/core/djangoapps/notifications/config/waffle.py @@ -59,3 +59,13 @@ ENABLE_NOTIFICATION_GROUPING = CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.enable_noti # .. toggle_warning: When the flag is ON, notification to all learners feature is enabled on discussion post. # .. toggle_tickets: INF-1917 ENABLE_NOTIFY_ALL_LEARNERS = CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.enable_post_notify_all_learners', __name__) + +# .. toggle_name: notifications.enable_push_notifications +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: Waffle flag to enable push Notifications feature on mobile devices +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2025-05-27 +# .. toggle_target_removal_date: 2026-05-27 +# .. toggle_warning: When the flag is ON, Notifications will go through ace push channels. +ENABLE_PUSH_NOTIFICATIONS = CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.enable_push_notifications', __name__) diff --git a/openedx/core/djangoapps/notifications/migrations/0009_notification_push.py b/openedx/core/djangoapps/notifications/migrations/0009_notification_push.py new file mode 100644 index 0000000000..8314f8fe59 --- /dev/null +++ b/openedx/core/djangoapps/notifications/migrations/0009_notification_push.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.18 on 2025-03-12 22:30 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('notifications', '0008_notificationpreference'), + ] + + operations = [ + migrations.AddField( + model_name='notification', + name='push', + field=models.BooleanField(default=False), + ), + ] diff --git a/openedx/core/djangoapps/notifications/models.py b/openedx/core/djangoapps/notifications/models.py index fc994b46ea..0f8539ccbf 100644 --- a/openedx/core/djangoapps/notifications/models.py +++ b/openedx/core/djangoapps/notifications/models.py @@ -110,6 +110,7 @@ class Notification(TimeStampedModel): content_url = models.URLField(null=True, blank=True) web = models.BooleanField(default=True, null=False, blank=False) email = models.BooleanField(default=False, null=False, blank=False) + push = models.BooleanField(default=False, null=False, blank=False) last_read = models.DateTimeField(null=True, blank=True) last_seen = models.DateTimeField(null=True, blank=True) group_by_id = models.CharField(max_length=255, db_index=True, null=False, default="") diff --git a/openedx/core/djangoapps/notifications/push/__init__.py b/openedx/core/djangoapps/notifications/push/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/notifications/push/message_type.py b/openedx/core/djangoapps/notifications/push/message_type.py new file mode 100644 index 0000000000..dd061e092d --- /dev/null +++ b/openedx/core/djangoapps/notifications/push/message_type.py @@ -0,0 +1,10 @@ +""" +Push notifications MessageType +""" +from openedx.core.djangoapps.ace_common.message import BaseMessageType + + +class PushNotificationMessageType(BaseMessageType): + """ + Edx-ace MessageType for Push Notifications + """ diff --git a/openedx/core/djangoapps/notifications/push/tasks.py b/openedx/core/djangoapps/notifications/push/tasks.py new file mode 100644 index 0000000000..fb24dfc19d --- /dev/null +++ b/openedx/core/djangoapps/notifications/push/tasks.py @@ -0,0 +1,45 @@ +""" Tasks for sending notification to ace push channel """ +from celery.utils.log import get_task_logger +from django.conf import settings +from django.contrib.auth import get_user_model +from edx_ace import ace + +from .message_type import PushNotificationMessageType + +User = get_user_model() +logger = get_task_logger(__name__) + + +def send_ace_msg_to_push_channel(audience_ids, notification_object, sender_id): + """ + Send mobile notifications using ace to push channels. + """ + if not audience_ids: + return + + # We are releasing this feature gradually. For now, it is only tested with the discussion app. + # We might have a list here in the future. + if notification_object.app_name != 'discussion': + return + + notification_type = notification_object.notification_type + + post_data = { + 'notification_type': notification_type, + 'course_id': str(notification_object.course_id), + 'content_url': notification_object.content_url, + **notification_object.content_context + } + emails = list(User.objects.filter(id__in=audience_ids).values_list('email', flat=True)) + context = {'post_data': post_data} + + message = PushNotificationMessageType( + app_label="notifications", name="push" + ).personalize(None, 'en', context) + message.options['emails'] = emails + message.options['notification_type'] = notification_type + message.options['skip_disable_user_policy'] = True + + ace.send(message, limit_to_channels=getattr(settings, 'ACE_PUSH_CHANNELS', [])) + log_msg = 'Sent mobile notification for %s to ace push channel. Audience IDs: %s' + logger.info(log_msg, notification_type, audience_ids) diff --git a/openedx/core/djangoapps/notifications/push/tests/__init__.py b/openedx/core/djangoapps/notifications/push/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/notifications/push/tests/test_tasks.py b/openedx/core/djangoapps/notifications/push/tests/test_tasks.py new file mode 100644 index 0000000000..5f3458ac5a --- /dev/null +++ b/openedx/core/djangoapps/notifications/push/tests/test_tasks.py @@ -0,0 +1,73 @@ +""" +Tests for push notifications tasks. +""" +from unittest import mock + +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.notifications.push.tasks import send_ace_msg_to_push_channel +from openedx.core.djangoapps.notifications.tests.utils import create_notification +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +class SendNotificationsTest(ModuleStoreTestCase): + """ + Tests for send_notifications. + """ + + def setUp(self): + """ + Create a course and users for the course. + """ + + super().setUp() + self.user_1 = UserFactory() + self.user_2 = UserFactory() + self.course_1 = CourseFactory.create( + org='testorg', + number='testcourse', + run='testrun' + ) + + self.notification = create_notification( + self.user, self.course_1.id, app_name='discussion', notification_type='new_comment' + ) + + @mock.patch('openedx.core.djangoapps.notifications.push.tasks.ace.send') + def test_send_ace_msg_success(self, mock_ace_send): + """ Test send_ace_msg_success """ + send_ace_msg_to_push_channel( + [self.user_1.id, self.user_2.id], + self.notification, + sender_id=self.user_1.id + ) + + mock_ace_send.assert_called_once() + message_sent = mock_ace_send.call_args[0][0] + assert message_sent.options['emails'] == [self.user_1.email, self.user_2.email] + assert message_sent.options['notification_type'] == 'new_comment' + + @mock.patch('openedx.core.djangoapps.notifications.push.tasks.ace.send') + def test_send_ace_msg_no_sender(self, mock_ace_send): + """ Test when sender is not valid """ + send_ace_msg_to_push_channel( + [self.user_1.id, self.user_2.id], + self.notification, + sender_id=999 + ) + + mock_ace_send.assert_called_once() + + @mock.patch('openedx.core.djangoapps.notifications.push.tasks.ace.send') + def test_send_ace_msg_empty_audience(self, mock_ace_send): + """ Test send_ace_msg_success with empty audience """ + send_ace_msg_to_push_channel([], self.notification, sender_id=self.user_1.id) + mock_ace_send.assert_not_called() + + @mock.patch('openedx.core.djangoapps.notifications.push.tasks.ace.send') + def test_send_ace_msg_non_discussion_app(self, mock_ace_send): + """ Test send_ace_msg_success with non-discussion app """ + self.notification.app_name = 'ecommerce' + self.notification.save() + send_ace_msg_to_push_channel([1], self.notification, sender_id=self.user_1.id) + mock_ace_send.assert_not_called() diff --git a/openedx/core/djangoapps/notifications/tasks.py b/openedx/core/djangoapps/notifications/tasks.py index fbb6a871a1..d08614b821 100644 --- a/openedx/core/djangoapps/notifications/tasks.py +++ b/openedx/core/djangoapps/notifications/tasks.py @@ -19,19 +19,25 @@ from openedx.core.djangoapps.notifications.base_notification import ( get_default_values_of_preference, get_notification_content ) -from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATION_GROUPING, ENABLE_NOTIFICATIONS from openedx.core.djangoapps.notifications.email.tasks import send_immediate_cadence_email from openedx.core.djangoapps.notifications.email_notifications import EmailCadence +from openedx.core.djangoapps.notifications.config.waffle import ( + ENABLE_NOTIFICATION_GROUPING, + ENABLE_NOTIFICATIONS, + ENABLE_PUSH_NOTIFICATIONS +) from openedx.core.djangoapps.notifications.events import notification_generated_event from openedx.core.djangoapps.notifications.grouping_notifications import ( + NotificationRegistry, get_user_existing_notifications, - group_user_notifications, NotificationRegistry, + group_user_notifications ) from openedx.core.djangoapps.notifications.models import ( CourseNotificationPreference, Notification, get_course_notification_preference_config_version ) +from openedx.core.djangoapps.notifications.push.tasks import send_ace_msg_to_push_channel from openedx.core.djangoapps.notifications.utils import clean_arguments, get_list_in_batches @@ -123,6 +129,7 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c """ Send notifications to the users. """ + # pylint: disable=too-many-statements course_key = CourseKey.from_string(course_key) if not ENABLE_NOTIFICATIONS.is_enabled(course_key): return @@ -136,12 +143,13 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c grouping_function = NotificationRegistry.get_grouper(notification_type) waffle_flag_enabled = ENABLE_NOTIFICATION_GROUPING.is_enabled(course_key) grouping_enabled = waffle_flag_enabled and group_by_id and grouping_function is not None - notifications_generated = False - notification_content = '' + generated_notification = None sender_id = context.pop('sender_id', None) default_web_config = get_default_values_of_preference(app_name, notification_type).get('web', False) generated_notification_audience = [] email_notification_mapping = {} + push_notification_audience = [] + is_push_notification_enabled = ENABLE_PUSH_NOTIFICATIONS.is_enabled(course_key) if group_by_id and not grouping_enabled: logger.info( @@ -185,6 +193,7 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c notification_preferences = preference.get_channels_for_notification_type(app_name, notification_type) email_enabled = 'email' in preference.get_channels_for_notification_type(app_name, notification_type) email_cadence = preference.get_email_cadence_for_notification_type(app_name, notification_type) + push_notification = is_push_notification_enabled and 'push' in notification_preferences new_notification = Notification( user_id=user_id, app_name=app_name, @@ -194,34 +203,37 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c course_id=course_key, web='web' in notification_preferences, email=email_enabled, + push=push_notification, group_by_id=group_by_id, ) if email_enabled and (email_cadence == EmailCadence.IMMEDIATELY): email_notification_mapping[user_id] = new_notification + if push_notification: + push_notification_audience.append(user_id) + if grouping_enabled and existing_notifications.get(user_id, None): group_user_notifications(new_notification, existing_notifications[user_id]) - if not notifications_generated: - notifications_generated = True - notification_content = new_notification.content + if not generated_notification: + generated_notification = new_notification else: notifications.append(new_notification) generated_notification_audience.append(user_id) # send notification to users but use bulk_create notification_objects = Notification.objects.bulk_create(notifications) - if notification_objects and not notifications_generated: - notifications_generated = True - notification_content = notification_objects[0].content + if notification_objects and not generated_notification: + generated_notification = notification_objects[0] if email_notification_mapping: send_immediate_cadence_email(email_notification_mapping, course_key) - if notifications_generated: + if generated_notification: notification_generated_event( generated_notification_audience, app_name, notification_type, course_key, content_url, - notification_content, sender_id=sender_id + generated_notification.content, sender_id=sender_id ) + send_ace_msg_to_push_channel(push_notification_audience, generated_notification, sender_id) def is_notification_valid(notification_type, context): diff --git a/openedx/core/djangoapps/notifications/templates/notifications/edx_ace/push/push/body.txt b/openedx/core/djangoapps/notifications/templates/notifications/edx_ace/push/push/body.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/notifications/templates/notifications/edx_ace/push/push/title.txt b/openedx/core/djangoapps/notifications/templates/notifications/edx_ace/push/push/title.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/notifications/tests/test_tasks.py b/openedx/core/djangoapps/notifications/tests/test_tasks.py index 14608a2135..04e5ae052c 100644 --- a/openedx/core/djangoapps/notifications/tests/test_tasks.py +++ b/openedx/core/djangoapps/notifications/tests/test_tasks.py @@ -15,7 +15,7 @@ from common.djangoapps.student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from ..config.waffle import ENABLE_NOTIFICATIONS, ENABLE_NOTIFICATION_GROUPING +from ..config.waffle import ENABLE_NOTIFICATION_GROUPING, ENABLE_NOTIFICATIONS, ENABLE_PUSH_NOTIFICATIONS from ..models import CourseNotificationPreference, Notification from ..tasks import ( create_notification_pref_if_not_exists, @@ -116,6 +116,7 @@ class SendNotificationsTest(ModuleStoreTestCase): ) @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True) + @override_waffle_flag(ENABLE_PUSH_NOTIFICATIONS, active=True) @ddt.data( ('discussion', 'new_comment_on_response'), # core notification ('discussion', 'new_response'), # non core notification @@ -168,6 +169,7 @@ class SendNotificationsTest(ModuleStoreTestCase): self.assertEqual(len(Notification.objects.all()), created_notifications_count) @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True) + @override_waffle_flag(ENABLE_PUSH_NOTIFICATIONS, active=True) def test_notification_not_send_with_preference_disabled(self): """ Tests notification not send if preference is disabled @@ -192,6 +194,7 @@ class SendNotificationsTest(ModuleStoreTestCase): @override_waffle_flag(ENABLE_NOTIFICATION_GROUPING, True) @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True) + @override_waffle_flag(ENABLE_PUSH_NOTIFICATIONS, active=True) def test_send_notification_with_grouping_enabled(self): """ Test send_notifications with grouping enabled. @@ -292,9 +295,9 @@ class SendBatchNotificationsTest(ModuleStoreTestCase): @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True) @ddt.data( - (settings.NOTIFICATION_CREATION_BATCH_SIZE, 10, 4), - (settings.NOTIFICATION_CREATION_BATCH_SIZE + 10, 12, 7), - (settings.NOTIFICATION_CREATION_BATCH_SIZE - 10, 10, 4), + (settings.NOTIFICATION_CREATION_BATCH_SIZE, 13, 6), + (settings.NOTIFICATION_CREATION_BATCH_SIZE + 10, 15, 9), + (settings.NOTIFICATION_CREATION_BATCH_SIZE - 10, 13, 5), ) @ddt.unpack def test_notification_is_send_in_batch(self, creation_size, prefs_query_count, notifications_query_count): @@ -323,6 +326,7 @@ class SendBatchNotificationsTest(ModuleStoreTestCase): for preference in preferences: discussion_config = preference.notification_preference_config['discussion'] discussion_config['notification_types'][notification_type]['web'] = True + discussion_config['notification_types'][notification_type]['push'] = True preference.save() # Creating notifications and asserting query count @@ -344,7 +348,7 @@ class SendBatchNotificationsTest(ModuleStoreTestCase): "username": "Test Author" } with override_waffle_flag(ENABLE_NOTIFICATIONS, active=True): - with self.assertNumQueries(10): + with self.assertNumQueries(13): send_notifications(user_ids, str(self.course.id), notification_app, notification_type, context, "http://test.url") @@ -363,9 +367,10 @@ class SendBatchNotificationsTest(ModuleStoreTestCase): "replier_name": "Replier Name" } with override_waffle_flag(ENABLE_NOTIFICATIONS, active=True): - with self.assertNumQueries(12): - send_notifications(user_ids, str(self.course.id), notification_app, notification_type, - context, "http://test.url") + with override_waffle_flag(ENABLE_PUSH_NOTIFICATIONS, active=True): + with self.assertNumQueries(15): + send_notifications(user_ids, str(self.course.id), notification_app, notification_type, + context, "http://test.url") def _update_user_preference(self, user_id, pref_exists): """ @@ -377,6 +382,7 @@ class SendBatchNotificationsTest(ModuleStoreTestCase): CourseNotificationPreference.objects.filter(user_id=user_id, course_id=self.course.id).delete() @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True) + @override_waffle_flag(ENABLE_PUSH_NOTIFICATIONS, active=True) @ddt.data( ("new_response", True, True, 2), ("new_response", False, False, 2), diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index ce3567fceb..075aedce9f 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -409,7 +409,7 @@ drf-yasg==1.21.10 # via # django-user-tasks # edx-api-doc-tools -edx-ace==1.14.0 +edx-ace==1.15.0 # via -r requirements/edx/kernel.in edx-api-doc-tools==2.1.0 # via diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 66201eb877..e35e36089e 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -669,7 +669,7 @@ drf-yasg==1.21.10 # -r requirements/edx/testing.txt # django-user-tasks # edx-api-doc-tools -edx-ace==1.14.0 +edx-ace==1.15.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 675592a536..9c8ee794ca 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -491,7 +491,7 @@ drf-yasg==1.21.10 # -r requirements/edx/base.txt # django-user-tasks # edx-api-doc-tools -edx-ace==1.14.0 +edx-ace==1.15.0 # via -r requirements/edx/base.txt edx-api-doc-tools==2.1.0 # via diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 0bbbe3f301..8f7f29df17 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -516,7 +516,7 @@ drf-yasg==1.21.10 # -r requirements/edx/base.txt # django-user-tasks # edx-api-doc-tools -edx-ace==1.14.0 +edx-ace==1.15.0 # via -r requirements/edx/base.txt edx-api-doc-tools==2.1.0 # via From a60b8f8d2c0c1d54c367b00b4dfe2f9140b4c04c Mon Sep 17 00:00:00 2001 From: Hassan Raza Date: Mon, 30 Jun 2025 14:54:11 +0500 Subject: [PATCH 7/9] chore: Track 'notify all learners' option in event data (#36962) * chore: Track 'notify all learners' option in event data * fix: unit test --- .../django_comment_client/base/views.py | 7 +++++-- lms/djangoapps/discussion/rest_api/api.py | 2 +- .../discussion/rest_api/tests/test_api.py | 15 ++++++++++++--- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/discussion/django_comment_client/base/views.py b/lms/djangoapps/discussion/django_comment_client/base/views.py index 458b0a0285..b24c04a3a1 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/views.py +++ b/lms/djangoapps/discussion/django_comment_client/base/views.py @@ -161,7 +161,7 @@ def add_truncated_title_to_event_data(event_data, full_title): event_data['title'] = full_title[:TRACKING_MAX_FORUM_TITLE] -def track_thread_created_event(request, course, thread, followed, from_mfe_sidebar=False): +def track_thread_created_event(request, course, thread, followed, from_mfe_sidebar=False, notify_all_learners=False): """ Send analytics event for a newly created thread. """ @@ -172,7 +172,10 @@ def track_thread_created_event(request, course, thread, followed, from_mfe_sideb 'thread_type': thread.thread_type, 'anonymous': thread.anonymous, 'anonymous_to_peers': thread.anonymous_to_peers, - 'options': {'followed': followed}, + 'options': { + 'followed': followed, + 'notify_all_learners': notify_all_learners + }, 'from_mfe_sidebar': from_mfe_sidebar, # There is a stated desire for an 'origin' property that will state # whether this thread was created via courseware or the forum. diff --git a/lms/djangoapps/discussion/rest_api/api.py b/lms/djangoapps/discussion/rest_api/api.py index e1f83cd7e6..8d4a2dd112 100644 --- a/lms/djangoapps/discussion/rest_api/api.py +++ b/lms/djangoapps/discussion/rest_api/api.py @@ -1497,7 +1497,7 @@ def create_thread(request, thread_data): _do_extra_actions(api_thread, cc_thread, list(thread_data.keys()), actions_form, context, request) track_thread_created_event(request, course, cc_thread, actions_form.cleaned_data["following"], - from_mfe_sidebar) + from_mfe_sidebar, notify_all_learners) return api_thread diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index d0eae92316..72e9426a65 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -1411,7 +1411,10 @@ class CreateThreadTest( 'title_truncated': False, 'anonymous': False, 'anonymous_to_peers': False, - 'options': {'followed': False}, + 'options': { + 'followed': False, + 'notify_all_learners': False + }, 'id': 'test_id', 'truncated': False, 'body': 'Test body', @@ -1500,7 +1503,10 @@ class CreateThreadTest( "title_truncated": False, "anonymous": False, "anonymous_to_peers": False, - "options": {"followed": False}, + "options": { + "followed": False, + "notify_all_learners": False + }, "id": "test_id", "truncated": False, "body": "Test body", @@ -1536,7 +1542,10 @@ class CreateThreadTest( 'title_truncated': True, 'anonymous': False, 'anonymous_to_peers': False, - 'options': {'followed': False}, + 'options': { + 'followed': False, + 'notify_all_learners': False + }, 'id': 'test_id', 'truncated': False, 'body': 'Test body', From 631352fe058cdf2c945cf7752752d16eca6c2901 Mon Sep 17 00:00:00 2001 From: sameeramin <35958006+sameeramin@users.noreply.github.com> Date: Mon, 30 Jun 2025 11:57:31 +0000 Subject: [PATCH 8/9] feat: Upgrade Python dependency enterprise-integrated-channels Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 075aedce9f..1629f6830d 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -566,7 +566,7 @@ enmerkar==0.7.1 # via enmerkar-underscore enmerkar-underscore==2.4.0 # via -r requirements/edx/kernel.in -enterprise-integrated-channels==0.1.7 +enterprise-integrated-channels==0.1.8 # via -r requirements/edx/bundled.in event-tracking==3.3.0 # via diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index e35e36089e..978554cea5 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -879,7 +879,7 @@ enmerkar-underscore==2.4.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -enterprise-integrated-channels==0.1.7 +enterprise-integrated-channels==0.1.8 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 9c8ee794ca..c5ed792c6c 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -653,7 +653,7 @@ enmerkar==0.7.1 # enmerkar-underscore enmerkar-underscore==2.4.0 # via -r requirements/edx/base.txt -enterprise-integrated-channels==0.1.7 +enterprise-integrated-channels==0.1.8 # via -r requirements/edx/base.txt event-tracking==3.3.0 # via diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 8f7f29df17..a1199f5549 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -680,7 +680,7 @@ enmerkar==0.7.1 # enmerkar-underscore enmerkar-underscore==2.4.0 # via -r requirements/edx/base.txt -enterprise-integrated-channels==0.1.7 +enterprise-integrated-channels==0.1.8 # via -r requirements/edx/base.txt event-tracking==3.3.0 # via From 97b91bdfc825445edc8c960a2fd51f8fd9ee123a Mon Sep 17 00:00:00 2001 From: Talha Rizwan Date: Mon, 30 Jun 2025 18:32:32 +0500 Subject: [PATCH 9/9] feat: export ora2 submission files to DRF (#36557) * feat: export ora2 submission files to DRF --- lms/djangoapps/instructor/views/api.py | 34 ++++++++++++--------- lms/djangoapps/instructor/views/api_urls.py | 2 +- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 9b2cfda1fa..c813c400a0 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -2773,26 +2773,30 @@ class ExportOra2SummaryView(DeveloperErrorViewMixin, APIView): return JsonResponse({"error": str(err)}, status=400) -@transaction.non_atomic_requests -@require_POST -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_course_permission(permissions.CAN_RESEARCH) -@common_exceptions_400 -def export_ora2_submission_files(request, course_id): +@method_decorator(transaction.non_atomic_requests, name='dispatch') +class ExportOra2SubmissionFilesView(DeveloperErrorViewMixin, APIView): """ Pushes a Celery task which will download and compress all submission files (texts, attachments) into a zip archive. """ - course_key = CourseKey.from_string(course_id) + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.CAN_RESEARCH - task_api.submit_export_ora2_submission_files(request, course_key) - - return JsonResponse({ - "status": _( - "Attachments archive is being created." - ) - }) + @method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True)) + @method_decorator(ensure_csrf_cookie) + def post(self, request, course_id): + """ + Initiates a task to export all ORA2 submission files for a course. + Returns a JSON response indicating the export task has been started. + """ + course_key = CourseKey.from_string(course_id) + try: + task_api.submit_export_ora2_submission_files(request, course_key) + return Response({ + "status": _("Attachments archive is being created.") + }) + except (AlreadyRunningError, QueueConnectionError, AttributeError) as err: + return JsonResponse({"error": str(err)}, status=400) @method_decorator(transaction.non_atomic_requests, name='dispatch') diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 1415541893..9ffdd0e65f 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -69,7 +69,7 @@ urlpatterns = [ path('export_ora2_data', api.ExportOra2DataView.as_view(), name='export_ora2_data'), path('export_ora2_summary', api.ExportOra2SummaryView.as_view(), name='export_ora2_summary'), - path('export_ora2_submission_files', api.export_ora2_submission_files, + path('export_ora2_submission_files', api.ExportOra2SubmissionFilesView.as_view(), name='export_ora2_submission_files'), # spoc gradebook