From e19355582ce985c5f235a8020504cf4f758bf55e Mon Sep 17 00:00:00 2001 From: Kyrylo Kireiev Date: Fri, 8 Sep 2023 12:15:24 +0000 Subject: [PATCH 01/24] feat: [AXIM-44] Adapt Delete Account to Bearer Authorization --- .../accounts/tests/test_retirement_views.py | 17 +++++++++++++++++ .../core/djangoapps/user_api/accounts/views.py | 3 ++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py index 8f34d4ba42d..6ec6e3694fe 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py @@ -74,6 +74,7 @@ from openedx.core.djangolib.testing.utils import skip_unless_lms from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order +from openedx.core.djangoapps.oauth_dispatch.tests.factories import ApplicationFactory, AccessTokenFactory from ...tests.factories import UserOrgTagFactory from ..views import USER_PROFILE_PII, AccountRetirementView @@ -263,6 +264,22 @@ def test_called_twice(self): response = self.client.post(self.url, self.build_post(self.test_password), **headers) assert response.status_code == status.HTTP_403_FORBIDDEN + def test_bearer_auth(self): + """ + Test the account deactivation/logout endpoint using Bearer auth + """ + # testing with broken token + headers = {'HTTP_AUTHORIZATION': 'Bearer broken_token'} + response = self.client.post(self.url, self.build_post(self.test_password), **headers) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + # testing with correct token + access_token = AccessTokenFactory(user=self.test_user, + application=ApplicationFactory(name="test_bearer", + user=self.test_user)).token + headers = {'HTTP_AUTHORIZATION': f'Bearer {access_token}'} + response = self.client.post(self.url, self.build_post(self.test_password), **headers) + assert response.status_code == status.HTTP_204_NO_CONTENT + @skip_unless_lms class TestPartnerReportingCleanup(ModuleStoreTestCase): diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 13b7ad39f61..2fb9fbde21c 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -21,6 +21,7 @@ from edx_ace import ace from edx_ace.recipient import Recipient from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from openedx.core.lib.api.authentication import BearerAuthentication from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser from enterprise.models import EnterpriseCourseEnrollment, EnterpriseCustomerUser, PendingEnterpriseCustomerUser from integrated_channels.degreed.models import DegreedLearnerDataTransmissionAudit @@ -567,7 +568,7 @@ class DeactivateLogoutView(APIView): - Log the user out - Create a row in the retirement table for that user """ - authentication_classes = (JwtAuthentication, SessionAuthentication,) + authentication_classes = (JwtAuthentication, SessionAuthentication, BearerAuthentication) permission_classes = (permissions.IsAuthenticated,) def post(self, request): From a677b6818ecfc01452ad55b539b2575a50bca338 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Mon, 25 Sep 2023 14:12:17 +0300 Subject: [PATCH 02/24] fix: (review) Add comment to Delete Account view --- openedx/core/djangoapps/user_api/accounts/views.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 2fb9fbde21c..48cb10fc7f4 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -568,6 +568,9 @@ class DeactivateLogoutView(APIView): - Log the user out - Create a row in the retirement table for that user """ + # BearerAuthentication is added here to support account deletion + # from the mobile app until it moves to JWT Auth. + # See mobile roadmap issue https://github.com/openedx/edx-platform/issues/33307. authentication_classes = (JwtAuthentication, SessionAuthentication, BearerAuthentication) permission_classes = (permissions.IsAuthenticated,) From 0a5110c515b4a1cc665efb1fd19800b7db2a38e8 Mon Sep 17 00:00:00 2001 From: Kyrylo Kireiev Date: Tue, 12 Sep 2023 07:54:08 +0000 Subject: [PATCH 03/24] fix: [AXIM-50] Fix count items in pagination --- lms/djangoapps/course_api/api.py | 9 +++----- lms/djangoapps/course_api/tests/test_views.py | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/course_api/api.py b/lms/djangoapps/course_api/api.py index 721c213992a..b4732394977 100644 --- a/lms/djangoapps/course_api/api.py +++ b/lms/djangoapps/course_api/api.py @@ -100,13 +100,10 @@ def _filter_by_search(course_queryset, search_term): ) search_courses_ids = {course['data']['id'] for course in search_courses['results']} - + courses = [course for course in course_queryset if str(course.id) in search_courses_ids] return LazySequence( - ( - course for course in course_queryset - if str(course.id) in search_courses_ids - ), - est_len=len(course_queryset) + iter(courses), + est_len=len(courses) ) diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py index 57b64d264ca..bd0c10aff6d 100644 --- a/lms/djangoapps/course_api/tests/test_views.py +++ b/lms/djangoapps/course_api/tests/test_views.py @@ -449,6 +449,29 @@ def test_too_many_courses(self): assert len(response.data['results']) == (30 if (page < 11) else 3) assert [c['id'] for c in response.data['results']] == ordered_course_ids[((page - 1) * 30):(page * 30)] + def test_count_item_pagination_with_search_term(self): + """ + Test count items in pagination for api courses list - class CourseListView + """ + # Create 15 new courses, courses have the word "new" in the title + _ = [self.create_and_index_course(f"numb_{number}", f"new_{number}") for number in range(15)] + response = self.verify_response(params={"search_term": "new"}) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["pagination"]["count"], 15) + + def test_count_item_pagination_with_search_term_and_filter(self): + """ + Test count items in pagination for api courses list + with search_term and filter by organisation - + class CourseListView + """ + # Create 25 new courses with two different organisations + _ = [self.create_and_index_course("Org_N", f"new_{number}") for number in range(10)] + _ = [self.create_and_index_course("Org_X", f"new_{number}") for number in range(15)] + response = self.verify_response(params={"org": "Org_X", "search_term": "new"}) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["pagination"]["count"], 15) + class CourseIdListViewTestCase(CourseApiTestViewMixin, ModuleStoreTestCase): """ From 9060663b476c4128ff147b4d5e3725b980b8d375 Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Mon, 26 Jun 2023 13:22:36 +0300 Subject: [PATCH 04/24] feat: list courses details by keys This adds the ability to get a list of detailed courses based on their keys provided in the newly added `keys` query param in the `GET /courses/v1/courses/` endpoint. --- lms/djangoapps/branding/__init__.py | 14 +++++-- lms/djangoapps/course_api/api.py | 10 ++++- lms/djangoapps/course_api/forms.py | 15 ++++++++ lms/djangoapps/course_api/tests/test_api.py | 37 ++++++++++++++++++- lms/djangoapps/course_api/tests/test_forms.py | 9 +++++ lms/djangoapps/course_api/views.py | 7 +++- lms/djangoapps/courseware/courses.py | 5 ++- .../content/course_overviews/models.py | 7 +++- 8 files changed, 93 insertions(+), 11 deletions(-) diff --git a/lms/djangoapps/branding/__init__.py b/lms/djangoapps/branding/__init__.py index 0847f631cb6..954e5da9b1b 100644 --- a/lms/djangoapps/branding/__init__.py +++ b/lms/djangoapps/branding/__init__.py @@ -14,7 +14,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers -def get_visible_courses(org=None, filter_=None, active_only=False): +def get_visible_courses(org=None, filter_=None, active_only=False, course_keys=None): """ Yield the CourseOverviews that should be visible in this branded instance. @@ -25,6 +25,8 @@ def get_visible_courses(org=None, filter_=None, active_only=False): filter_ (dict): Optional parameter that allows custom filtering by fields on the course. active_only (bool): Optional parameter that enables fetching active courses only. + course_keys (list[str]): Optional parameter that allows for selecting which + courses to fetch the `CourseOverviews` for """ # Import is placed here to avoid model import at project startup. from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -36,12 +38,16 @@ def get_visible_courses(org=None, filter_=None, active_only=False): if org: # Check the current site's orgs to make sure the org's courses should be displayed if not current_site_orgs or org in current_site_orgs: - courses = CourseOverview.get_all_courses(orgs=[org], filter_=filter_, active_only=active_only) + courses = CourseOverview.get_all_courses( + orgs=[org], filter_=filter_, active_only=active_only, course_keys=course_keys + ) elif current_site_orgs: # Only display courses that should be displayed on this site - courses = CourseOverview.get_all_courses(orgs=current_site_orgs, filter_=filter_, active_only=active_only) + courses = CourseOverview.get_all_courses( + orgs=current_site_orgs, filter_=filter_, active_only=active_only, course_keys=course_keys + ) else: - courses = CourseOverview.get_all_courses(filter_=filter_, active_only=active_only) + courses = CourseOverview.get_all_courses(filter_=filter_, active_only=active_only, course_keys=course_keys) courses = courses.order_by('id') diff --git a/lms/djangoapps/course_api/api.py b/lms/djangoapps/course_api/api.py index b4732394977..39c34ec6da9 100644 --- a/lms/djangoapps/course_api/api.py +++ b/lms/djangoapps/course_api/api.py @@ -113,7 +113,8 @@ def list_courses(request, filter_=None, search_term=None, permissions=None, - active_only=False): + active_only=False, + course_keys=None): """ Yield all available courses. @@ -143,12 +144,17 @@ def list_courses(request, If specified, it filters visible `CourseOverview` objects by checking if each permission specified is granted for the username. active_only (bool): Optional parameter that enables fetching active courses only. + course_keys (list[str]): + If specified, it filters visible `CourseOverview` objects by + the course keys (ids) provided Return value: Yield `CourseOverview` objects representing the collection of courses. """ user = get_effective_user(request.user, username) - course_qs = get_courses(user, org=org, filter_=filter_, permissions=permissions, active_only=active_only) + course_qs = get_courses( + user, org=org, filter_=filter_, permissions=permissions, active_only=active_only, course_keys=course_keys + ) course_qs = _filter_by_search(course_qs, search_term) return course_qs diff --git a/lms/djangoapps/course_api/forms.py b/lms/djangoapps/course_api/forms.py index 055692da712..ff6cee84aea 100644 --- a/lms/djangoapps/course_api/forms.py +++ b/lms/djangoapps/course_api/forms.py @@ -64,6 +64,7 @@ class CourseListGetForm(UsernameValidatorMixin, Form): mobile = ExtendedNullBooleanField(required=False) active_only = ExtendedNullBooleanField(required=False) permissions = MultiValueField(required=False) + course_keys = MultiValueField(required=False) def clean(self): """ @@ -80,6 +81,20 @@ def clean(self): return cleaned_data + def clean_course_keys(self): + """ + Ensure valid course_keys were provided. + """ + course_keys = self.cleaned_data['course_keys'] + if course_keys: + for course_key in course_keys: + try: + CourseKey.from_string(course_key) + except InvalidKeyError: + raise ValidationError(f"'{str(course_key)}' is not a valid course key.") # lint-amnesty, pylint: disable=raise-missing-from + + return course_keys + class CourseIdListGetForm(UsernameValidatorMixin, Form): """ diff --git a/lms/djangoapps/course_api/tests/test_api.py b/lms/djangoapps/course_api/tests/test_api.py index 8d91b1ee97e..8e4d1253f8f 100644 --- a/lms/djangoapps/course_api/tests/test_api.py +++ b/lms/djangoapps/course_api/tests/test_api.py @@ -107,7 +107,8 @@ def _make_api_call(self, specified_user, org=None, filter_=None, - permissions=None): + permissions=None, + course_keys=None): """ Call the list_courses api endpoint to get information about `specified_user` on behalf of `requesting_user`. @@ -121,6 +122,7 @@ def _make_api_call(self, org=org, filter_=filter_, permissions=permissions, + course_keys=course_keys, ) def verify_courses(self, courses): @@ -244,6 +246,39 @@ def test_permissions(self): self.assertEqual({c.id for c in filtered_courses}, {self.course.id}) + def test_filter_by_keys(self): + """ + Verify that courses are filtered by the provided course keys. + """ + + # Create alternative courses to be included in the `course_keys` filter. + alternative_course_1 = self.create_course(course='alternative-course-1') + alternative_course_2 = self.create_course(course='alternative-course-2') + + # No filtering. + unfiltered_expected_courses = [ + self.course, + alternative_course_1, + alternative_course_2, + ] + unfiltered_courses = self._make_api_call(self.honor_user, self.honor_user) + assert {course.id for course in unfiltered_courses} == {course.id for course in unfiltered_expected_courses} + + # With filtering. + filtered_expected_courses = [ + alternative_course_1, + alternative_course_2, + ] + filtered_courses = self._make_api_call( + self.honor_user, + self.honor_user, + course_keys={ + alternative_course_1.id, + alternative_course_2.id + } + ) + assert {course.id for course in filtered_courses} == {course.id for course in filtered_expected_courses} + class TestGetCourseListExtras(CourseListTestMixin, ModuleStoreTestCase): """ diff --git a/lms/djangoapps/course_api/tests/test_forms.py b/lms/djangoapps/course_api/tests/test_forms.py index 9d14e5a9989..3b5744402e7 100644 --- a/lms/djangoapps/course_api/tests/test_forms.py +++ b/lms/djangoapps/course_api/tests/test_forms.py @@ -71,6 +71,7 @@ def set_up_data(self, user): 'filter_': None, 'permissions': set(), 'active_only': None, + 'course_keys': set(), } def test_basic(self): @@ -100,6 +101,14 @@ def test_filter(self, param_field_name, param_field_value): self.assert_valid(self.cleaned_data) + def test_invalid_course_keys(self): + """ + Verify form checks for validity of course keys provided + """ + + self.form_data['course_keys'] = 'course-v1:edX+DemoX+Demo_Course,invalid_course_key' + self.assert_error('course_keys', "'invalid_course_key' is not a valid course key.") + class TestCourseIdListGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring FORM_CLASS = CourseIdListGetForm diff --git a/lms/djangoapps/course_api/views.py b/lms/djangoapps/course_api/views.py index fbb8517cf00..98b01232a1b 100644 --- a/lms/djangoapps/course_api/views.py +++ b/lms/djangoapps/course_api/views.py @@ -286,6 +286,10 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView): date are returned. This is different from search_term because this filtering is done on CourseOverview and not ElasticSearch. + course_keys (optional): + If specified, it fetches the `CourseOverview` objects for the + the specified course keys + **Returns** * 200 on success, with a list of course discovery objects as returned @@ -343,7 +347,8 @@ def get_queryset(self): filter_=form.cleaned_data['filter_'], search_term=form.cleaned_data['search_term'], permissions=form.cleaned_data['permissions'], - active_only=form.cleaned_data.get('active_only', False) + active_only=form.cleaned_data.get('active_only', False), + course_keys=form.cleaned_data['course_keys'], ) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 676a42bd70e..6ded860329f 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -753,7 +753,7 @@ def get_course_syllabus_section(course, section_key): @function_trace('get_courses') -def get_courses(user, org=None, filter_=None, permissions=None, active_only=False): +def get_courses(user, org=None, filter_=None, permissions=None, active_only=False, course_keys=None): """ Return a LazySequence of courses available, optionally filtered by org code (case-insensitive) or a set of permissions to be satisfied for the specified @@ -763,7 +763,8 @@ def get_courses(user, org=None, filter_=None, permissions=None, active_only=Fals courses = branding.get_visible_courses( org=org, filter_=filter_, - active_only=active_only + active_only=active_only, + course_keys=course_keys ).prefetch_related( 'modes', ).select_related( diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index 55d2b8d2bb0..149159826cc 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -657,7 +657,7 @@ def update_select_courses(cls, course_keys, force_update=False): log.info('Finished generating course overviews.') @classmethod - def get_all_courses(cls, orgs=None, filter_=None, active_only=False): + def get_all_courses(cls, orgs=None, filter_=None, active_only=False, course_keys=None): """ Return a queryset containing all CourseOverview objects in the database. @@ -666,12 +666,17 @@ def get_all_courses(cls, orgs=None, filter_=None, active_only=False): filtering by organization. filter_ (dict): Optional parameter that allows custom filtering. active_only (bool): If provided, only the courses that have not ended will be returned. + course_keys (list[string]): Optional parameter that allows case-insensitive + filter by course ids """ # Note: If a newly created course is not returned in this QueryList, # make sure the "publish" signal was emitted when the course was # created. For tests using CourseFactory, use emit_signals=True. course_overviews = CourseOverview.objects.all() + if course_keys: + course_overviews = course_overviews.filter(id__in=course_keys) + if orgs: # In rare cases, courses belonging to the same org may be accidentally assigned # an org code with a different casing (e.g., Harvardx as opposed to HarvardX). From b7f49239771a84e31e7c97f6c02a44757a5aa9dc Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Wed, 1 Nov 2023 15:20:00 +0200 Subject: [PATCH 05/24] fix: [FC-0031] Add limit the number of returned results for mobile_search --- lms/djangoapps/course_api/api.py | 26 ++++++++++++++----- lms/djangoapps/course_api/forms.py | 1 + lms/djangoapps/course_api/tests/test_forms.py | 1 + lms/djangoapps/course_api/tests/test_views.py | 25 +++++++++++++++--- lms/djangoapps/course_api/views.py | 5 ++++ lms/envs/common.py | 3 +++ 6 files changed, 51 insertions(+), 10 deletions(-) diff --git a/lms/djangoapps/course_api/api.py b/lms/djangoapps/course_api/api.py index 39c34ec6da9..8210396441c 100644 --- a/lms/djangoapps/course_api/api.py +++ b/lms/djangoapps/course_api/api.py @@ -82,7 +82,7 @@ def course_detail(request, username, course_key): return overview -def _filter_by_search(course_queryset, search_term): +def _filter_by_search(course_queryset, search_term, mobile_search=False): """ Filters a course queryset by the specified search term. """ @@ -100,10 +100,20 @@ def _filter_by_search(course_queryset, search_term): ) search_courses_ids = {course['data']['id'] for course in search_courses['results']} - courses = [course for course in course_queryset if str(course.id) in search_courses_ids] + + if mobile_search is True: + course_limit = getattr(settings, 'MOBILE_SEARCH_COURSE_LIMIT', 100) + courses = [course for course in course_queryset[:course_limit] if str(course.id) in search_courses_ids] + return LazySequence( + iter(courses), + est_len=len(courses) + ) return LazySequence( - iter(courses), - est_len=len(courses) + ( + course for course in course_queryset + if str(course.id) in search_courses_ids + ), + est_len=len(course_queryset) ) @@ -114,7 +124,8 @@ def list_courses(request, search_term=None, permissions=None, active_only=False, - course_keys=None): + course_keys=None, + mobile_search=False): """ Yield all available courses. @@ -147,6 +158,9 @@ def list_courses(request, course_keys (list[str]): If specified, it filters visible `CourseOverview` objects by the course keys (ids) provided + mobile_search (bool): + Optional parameter that limits the number of returned courses + to MOBILE_SEARCH_COURSE_LIMIT. Return value: Yield `CourseOverview` objects representing the collection of courses. @@ -155,7 +169,7 @@ def list_courses(request, course_qs = get_courses( user, org=org, filter_=filter_, permissions=permissions, active_only=active_only, course_keys=course_keys ) - course_qs = _filter_by_search(course_qs, search_term) + course_qs = _filter_by_search(course_qs, search_term, mobile_search) return course_qs diff --git a/lms/djangoapps/course_api/forms.py b/lms/djangoapps/course_api/forms.py index ff6cee84aea..79dba03f45e 100644 --- a/lms/djangoapps/course_api/forms.py +++ b/lms/djangoapps/course_api/forms.py @@ -65,6 +65,7 @@ class CourseListGetForm(UsernameValidatorMixin, Form): active_only = ExtendedNullBooleanField(required=False) permissions = MultiValueField(required=False) course_keys = MultiValueField(required=False) + mobile_search = ExtendedNullBooleanField(required=False) def clean(self): """ diff --git a/lms/djangoapps/course_api/tests/test_forms.py b/lms/djangoapps/course_api/tests/test_forms.py index 3b5744402e7..43a4aa104bd 100644 --- a/lms/djangoapps/course_api/tests/test_forms.py +++ b/lms/djangoapps/course_api/tests/test_forms.py @@ -72,6 +72,7 @@ def set_up_data(self, user): 'permissions': set(), 'active_only': None, 'course_keys': set(), + 'mobile_search': None, } def test_basic(self): diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py index bd0c10aff6d..c39f808478e 100644 --- a/lms/djangoapps/course_api/tests/test_views.py +++ b/lms/djangoapps/course_api/tests/test_views.py @@ -454,10 +454,12 @@ def test_count_item_pagination_with_search_term(self): Test count items in pagination for api courses list - class CourseListView """ # Create 15 new courses, courses have the word "new" in the title - _ = [self.create_and_index_course(f"numb_{number}", f"new_{number}") for number in range(15)] + [self.create_and_index_course(f"numb_{number}", f"new_{number}") for number in range(15)] # pylint: disable=expression-not-assigned response = self.verify_response(params={"search_term": "new"}) self.assertEqual(response.status_code, 200) - self.assertEqual(response.json()["pagination"]["count"], 15) + # We don't have 'count' 15 because 'mobile_search' param is None + # And LazySequence contains all courses + self.assertEqual(response.json()["pagination"]["count"], 18) def test_count_item_pagination_with_search_term_and_filter(self): """ @@ -466,12 +468,27 @@ def test_count_item_pagination_with_search_term_and_filter(self): class CourseListView """ # Create 25 new courses with two different organisations - _ = [self.create_and_index_course("Org_N", f"new_{number}") for number in range(10)] - _ = [self.create_and_index_course("Org_X", f"new_{number}") for number in range(15)] + [self.create_and_index_course("Org_N", f"new_{number}") for number in range(10)] # pylint: disable=expression-not-assigned + [self.create_and_index_course("Org_X", f"new_{number}") for number in range(15)] # pylint: disable=expression-not-assigned response = self.verify_response(params={"org": "Org_X", "search_term": "new"}) self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["pagination"]["count"], 15) + def test_count_item_pagination_with_search_term_and_mobile_search(self): + """ + Test count items in pagination for api courses list + with search_term and 'mobile_search' is True + """ + # Create 25 new courses with two different words in titles + [self.create_and_index_course("Org_N", f"old_{number}") for number in range(10)] # pylint: disable=expression-not-assigned + [self.create_and_index_course("Org_N", f"new_{number}") for number in range(15)] # pylint: disable=expression-not-assigned + response = self.verify_response( + params={"search_term": "new", "mobile_search": True} + ) + self.assertEqual(response.status_code, 200) + # We have 'count' 15 because 'mobile_search' param is true + self.assertEqual(response.json()["pagination"]["count"], 15) + class CourseIdListViewTestCase(CourseApiTestViewMixin, ModuleStoreTestCase): """ diff --git a/lms/djangoapps/course_api/views.py b/lms/djangoapps/course_api/views.py index 98b01232a1b..b2019c793fe 100644 --- a/lms/djangoapps/course_api/views.py +++ b/lms/djangoapps/course_api/views.py @@ -290,6 +290,10 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView): If specified, it fetches the `CourseOverview` objects for the the specified course keys + mobile_search (bool): + Optional parameter that limits the number of returned courses + to MOBILE_SEARCH_COURSE_LIMIT. + **Returns** * 200 on success, with a list of course discovery objects as returned @@ -349,6 +353,7 @@ def get_queryset(self): permissions=form.cleaned_data['permissions'], active_only=form.cleaned_data.get('active_only', False), course_keys=form.cleaned_data['course_keys'], + mobile_search=form.cleaned_data.get('mobile_search', False), ) diff --git a/lms/envs/common.py b/lms/envs/common.py index f0e8bc9383b..57ce369542d 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -4446,6 +4446,9 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring r'edX/org.edx.mobile', ] +# set course limit for mobile search +MOBILE_SEARCH_COURSE_LIMIT = 100 + # cache timeout in seconds for Mobile App Version Upgrade APP_UPGRADE_CACHE_TIMEOUT = 3600 From 1100983c2d5963d0be76343d1676c50676170e3b Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Tue, 12 Sep 2023 15:39:36 +0300 Subject: [PATCH 06/24] feat: [AXIM-6] Add DefaultPagination for UserCourseEnrollmentsList v3 --- lms/djangoapps/mobile_api/users/tests.py | 25 +++++++++++++++++++++++- lms/djangoapps/mobile_api/users/views.py | 16 +++++++++++++-- lms/djangoapps/mobile_api/utils.py | 1 + lms/urls.py | 2 +- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 9d4752ea7c2..f3ec1d055dc 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -34,7 +34,7 @@ MobileAuthUserTestMixin, MobileCourseAccessTestMixin ) -from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2 +from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2, API_V3 from openedx.core.lib.courses import course_image_url from openedx.features.course_duration_limits.models import CourseDurationLimitConfig from openedx.features.course_experience.tests.views.helpers import add_course_mode @@ -376,6 +376,29 @@ def test_enrollment_with_configs(self): self.assertDictEqual(response.data['configs'], expected_result) assert 'enrollments' in response.data + def test_pagination_enrollment(self): + """ + Test pagination for UserCourseEnrollmentsList view v3 + for 3rd version of this view we use DefaultPagination + + Test for /api/mobile/{api_version}/users//course_enrollments/ + api_version = v3 + """ + self.login() + # Create and enroll to 15 courses + courses = [CourseFactory.create(org="my_org", mobile_available=True) for _ in range(15)] + for course in courses: + self.enroll(course.id) + + response = self.api_response(api_version=API_V3) + assert response.status_code == 200 + assert response.data["enrollments"]["count"] == 15 + assert response.data["enrollments"]["num_pages"] == 2 + assert response.data["enrollments"]["current_page"] == 1 + assert len(response.data["enrollments"]["results"]) == 10 + assert "next" in response.data["enrollments"] + assert "previous" in response.data["enrollments"] + @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) class TestUserEnrollmentCertificates(UrlResetMixin, MobileAPITestCase, MilestonesTestCaseMixin): diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 0e2cb61d311..5041ea33b19 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -19,6 +19,7 @@ from rest_framework.response import Response from xblock.fields import Scope from xblock.runtime import KeyValueStore +from edx_rest_framework_extensions.paginators import DefaultPagination from common.djangoapps.student.models import CourseEnrollment, User # lint-amnesty, pylint: disable=reimported from lms.djangoapps.courseware.access import is_mobile_available_for_user @@ -28,7 +29,7 @@ from lms.djangoapps.courseware.block_render import get_block_for_descriptor from lms.djangoapps.courseware.views.index import save_positions_recursively_up from lms.djangoapps.mobile_api.models import MobileConfig -from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2 +from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2, API_V3 from openedx.features.course_duration_limits.access import check_course_expired from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -365,7 +366,7 @@ def list(self, request, *args, **kwargs): response = super().list(request, *args, **kwargs) api_version = self.kwargs.get('api_version') - if api_version == API_V2: + if api_version in (API_V2, API_V3): enrollment_data = { 'configs': MobileConfig.get_structured_configs(), 'enrollments': response.data @@ -374,6 +375,17 @@ def list(self, request, *args, **kwargs): return response + # pylint: disable=attribute-defined-outside-init + @property + def paginator(self): + super().paginator # pylint: disable=expression-not-assigned + api_version = self.kwargs.get('api_version') + + if self._paginator is None and api_version == API_V3: + self._paginator = DefaultPagination() + + return self._paginator + @api_view(["GET"]) @mobile_view() diff --git a/lms/djangoapps/mobile_api/utils.py b/lms/djangoapps/mobile_api/utils.py index 5dcdf9e04c6..73a0cfea082 100644 --- a/lms/djangoapps/mobile_api/utils.py +++ b/lms/djangoapps/mobile_api/utils.py @@ -5,6 +5,7 @@ API_V05 = 'v0.5' API_V1 = 'v1' API_V2 = 'v2' +API_V3 = 'v3' def parsed_version(version): diff --git a/lms/urls.py b/lms/urls.py index 26f05a40007..b03c084d021 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -222,7 +222,7 @@ if settings.FEATURES.get('ENABLE_MOBILE_REST_API'): urlpatterns += [ - re_path(r'^api/mobile/(?Pv(2|1|0.5))/', include('lms.djangoapps.mobile_api.urls')), + re_path(r'^api/mobile/(?Pv(3|2|1|0.5))/', include('lms.djangoapps.mobile_api.urls')), ] if settings.FEATURES.get('ENABLE_OPENBADGES'): From 51f0ebe39d44c2547519e6f9a1eb9a0400f4641e Mon Sep 17 00:00:00 2001 From: Glib Glugovskiy Date: Mon, 20 Nov 2023 22:39:48 +0200 Subject: [PATCH 07/24] docs: add docstring for the paginator property override --- lms/djangoapps/mobile_api/users/views.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 5041ea33b19..86da7e0bfee 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -378,6 +378,11 @@ def list(self, request, *args, **kwargs): # pylint: disable=attribute-defined-outside-init @property def paginator(self): + """ + Overrides API View paginator property to dynamically determine pagination class + based on the provided api_version. Implements solutions from the discussion at + https://www.github.com/encode/django-rest-framework/issues/6397. + """ super().paginator # pylint: disable=expression-not-assigned api_version = self.kwargs.get('api_version') From b739c009256d30b977a34a6fc52763ded1950912 Mon Sep 17 00:00:00 2001 From: Glib Glugovskiy Date: Mon, 20 Nov 2023 23:08:56 +0200 Subject: [PATCH 08/24] fix: remove trailing whitespace failing quality check --- lms/djangoapps/mobile_api/users/views.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 86da7e0bfee..ae0733d5cd9 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -379,8 +379,9 @@ def list(self, request, *args, **kwargs): @property def paginator(self): """ - Overrides API View paginator property to dynamically determine pagination class - based on the provided api_version. Implements solutions from the discussion at + Override API View paginator property to dynamically determine pagination class + + Implements solutions from the discussion at https://www.github.com/encode/django-rest-framework/issues/6397. """ super().paginator # pylint: disable=expression-not-assigned From 25577e2da2cb70375a099ecdcd1926676ab6c98d Mon Sep 17 00:00:00 2001 From: SaadYousaf Date: Wed, 4 Oct 2023 13:23:52 +0500 Subject: [PATCH 09/24] feat: add tracking event for following post --- .../django_comment_client/base/tests.py | 28 +++++++++++++++++++ .../django_comment_client/base/views.py | 20 +++++++++++++ lms/djangoapps/discussion/rest_api/api.py | 9 ++++-- .../discussion/rest_api/tests/test_api.py | 10 ++++++- 4 files changed, 63 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/discussion/django_comment_client/base/tests.py b/lms/djangoapps/discussion/django_comment_client/base/tests.py index bf346405ebf..ad6d021cd18 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/tests.py +++ b/lms/djangoapps/discussion/django_comment_client/base/tests.py @@ -1860,6 +1860,34 @@ def test_thread_voted_event(self, view_name, obj_id_name, obj_type, mock_request assert event['undo_vote'] == undo assert event['vote_value'] == 'up' + @ddt.data('follow_thread', 'unfollow_thread',) + @patch('eventtracking.tracker.emit') + @patch('openedx.core.djangoapps.django_comment_common.comment_client.utils.requests.request', autospec=True) + def test_thread_followed_event(self, view_name, mock_request, mock_emit): + self._set_mock_request_data(mock_request, { + 'closed': False, + 'commentable_id': 'test_commentable_id', + 'username': 'test_user', + }) + request = RequestFactory().post('dummy_url', {}) + request.user = self.student + request.view_name = view_name + view_function = getattr(views, view_name) + kwargs = dict(course_id=str(self.course.id)) + kwargs['thread_id'] = 'thread_id' + view_function(request, **kwargs) + + assert mock_emit.called + event_name, event_data = mock_emit.call_args[0] + action_name = 'followed' if view_name == 'follow_thread' else 'unfollowed' + expected_action_value = True if view_name == 'follow_thread' else False + assert event_name == f'edx.forum.thread.{action_name}' + assert event_data['commentable_id'] == 'test_commentable_id' + assert event_data['id'] == 'thread_id' + assert event_data['followed'] == expected_action_value + assert event_data['user_forums_roles'] == ['Student'] + assert event_data['user_course_roles'] == ['Wizard'] + class UsersEndpointTestCase(ForumsEnableMixin, SharedModuleStoreTestCase, MockRequestSetupMixin): diff --git a/lms/djangoapps/discussion/django_comment_client/base/views.py b/lms/djangoapps/discussion/django_comment_client/base/views.py index 54f8e493b52..4677e318f38 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/views.py +++ b/lms/djangoapps/discussion/django_comment_client/base/views.py @@ -134,6 +134,20 @@ def track_thread_created_event(request, course, thread, followed, from_mfe_sideb track_created_event(request, event_name, course, thread, event_data) +def track_thread_followed_event(request, course, thread, followed): + """ + Send analytics event for a newly followed/unfollowed thread. + """ + action_name = 'followed' if followed else 'unfollowed' + event_name = _EVENT_NAME_TEMPLATE.format(obj_type='thread', action_name=action_name) + event_data = { + 'commentable_id': thread.commentable_id, + 'id': thread.id, + 'followed': followed, + } + track_forum_event(request, event_name, course, thread, event_data) + + def track_comment_created_event(request, course, comment, commentable_id, followed, from_mfe_sidebar=False): """ Send analytics event for a newly created response or comment. @@ -938,9 +952,12 @@ def un_pin_thread(request, course_id, thread_id): @permitted def follow_thread(request, course_id, thread_id): # lint-amnesty, pylint: disable=missing-function-docstring, unused-argument user = cc.User.from_django_user(request.user) + course_key = CourseKey.from_string(course_id) + course = get_course_by_id(course_key) thread = cc.Thread.find(thread_id) user.follow(thread) thread_followed.send(sender=None, user=request.user, post=thread) + track_thread_followed_event(request, course, thread, True) return JsonResponse({}) @@ -966,10 +983,13 @@ def unfollow_thread(request, course_id, thread_id): # lint-amnesty, pylint: dis given a course id and thread id, stop following this thread ajax only """ + course_key = CourseKey.from_string(course_id) + course = get_course_by_id(course_key) user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) user.unfollow(thread) thread_unfollowed.send(sender=None, user=request.user, post=thread) + track_thread_followed_event(request, course, thread, False) return JsonResponse({}) diff --git a/lms/djangoapps/discussion/rest_api/api.py b/lms/djangoapps/discussion/rest_api/api.py index f480fb31fe2..fdf4e7c674b 100644 --- a/lms/djangoapps/discussion/rest_api/api.py +++ b/lms/djangoapps/discussion/rest_api/api.py @@ -89,7 +89,7 @@ track_voted_event, track_discussion_reported_event, track_discussion_unreported_event, - track_forum_search_event + track_forum_search_event, track_thread_followed_event ) from ..django_comment_client.utils import ( get_group_id_for_user, @@ -1323,7 +1323,7 @@ def _do_extra_actions(api_content, cc_content, request_fields, actions_form, con if field in request_fields and field in api_content and form_value != api_content[field]: api_content[field] = form_value if field == "following": - _handle_following_field(form_value, context["cc_requester"], cc_content) + _handle_following_field(form_value, context["cc_requester"], cc_content, request) elif field == "abuse_flagged": _handle_abuse_flagged_field(form_value, context["cc_requester"], cc_content, request) elif field == "voted": @@ -1336,12 +1336,15 @@ def _do_extra_actions(api_content, cc_content, request_fields, actions_form, con raise ValidationError({field: ["Invalid Key"]}) -def _handle_following_field(form_value, user, cc_content): +def _handle_following_field(form_value, user, cc_content, request): """follow/unfollow thread for the user""" + course_key = CourseKey.from_string(cc_content.course_id) + course = get_course_with_access(request.user, 'load', course_key) if form_value: user.follow(cc_content) else: user.unfollow(cc_content) + track_thread_followed_event(request, course, cc_content, form_value) def _handle_abuse_flagged_field(form_value, user, cc_content, request): diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index d9188463fef..f0daa202e55 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -2684,7 +2684,8 @@ def test_author_only_fields(self, role_name): @ddt.data(*itertools.product([True, False], [True, False])) @ddt.unpack - def test_following(self, old_following, new_following): + @mock.patch("eventtracking.tracker.emit") + def test_following(self, old_following, new_following, mock_emit): """ Test attempts to edit the "following" field. @@ -2714,6 +2715,13 @@ def test_following(self, old_following, new_following): ) request_data.pop("request_id", None) assert request_data == {'source_type': ['thread'], 'source_id': ['test_thread']} + event_name, event_data = mock_emit.call_args[0] + expected_event_action = 'followed' if new_following else 'unfollowed' + assert event_name == f'edx.forum.thread.{expected_event_action}' + assert event_data['commentable_id'] == 'original_topic' + assert event_data['id'] == 'test_thread' + assert event_data['followed'] == new_following + assert event_data['user_forums_roles'] == ['Student'] @ddt.data(*itertools.product([True, False], [True, False])) @ddt.unpack From 8b94c2ceb6a940c7a0daf0b650b5d81f3306ca5e Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul <77053848+muhammadadeeltajamul@users.noreply.github.com> Date: Wed, 19 Apr 2023 19:55:10 +0500 Subject: [PATCH 10/24] feat: added edit_by_label and closed_by_label in threads response (#32070) --- .../discussion/rest_api/serializers.py | 38 +++++- .../discussion/rest_api/tests/test_api.py | 5 + .../rest_api/tests/test_serializers.py | 122 ++++++++++++++++-- .../discussion/rest_api/tests/test_views.py | 6 + .../discussion/rest_api/tests/utils.py | 2 + 5 files changed, 160 insertions(+), 13 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/serializers.py b/lms/djangoapps/discussion/rest_api/serializers.py index 6e17f201d0f..5c2115674c3 100644 --- a/lms/djangoapps/discussion/rest_api/serializers.py +++ b/lms/djangoapps/discussion/rest_api/serializers.py @@ -6,7 +6,7 @@ from django.conf import settings from django.contrib.auth import get_user_model -from django.core.exceptions import ValidationError +from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.db.models import TextChoices from django.urls import reverse from django.utils.html import strip_tags @@ -83,6 +83,7 @@ def get_context(course, request, thread=None): "ta_user_ids": ta_user_ids, "cc_requester": cc_requester, "has_moderation_privilege": has_moderation_privilege, + "is_global_staff": is_global_staff, } @@ -155,6 +156,7 @@ class _ContentSerializer(serializers.Serializer): anonymous_to_peers = serializers.BooleanField(default=False) last_edit = serializers.SerializerMethodField(required=False) edit_reason_code = serializers.CharField(required=False, validators=[validate_edit_reason_code]) + edit_by_label = serializers.SerializerMethodField(required=False) non_updatable_fields = set() @@ -204,13 +206,25 @@ def _get_user_label(self, user_id): """ is_staff = user_id in self.context["course_staff_user_ids"] or user_id in self.context["moderator_user_ids"] is_ta = user_id in self.context["ta_user_ids"] + is_global_staff = self.context["is_global_staff"] return ( - "Staff" if is_staff else + "Staff" if (is_staff or is_global_staff) else "Community TA" if is_ta else None ) + def _get_user_label_from_username(self, username): + """ + Returns role label of user from username + Possible Role Labels: Staff, Community TA or None + """ + try: + user = User.objects.get(username=username) + return self._get_user_label(user.id) + except ObjectDoesNotExist: + return None + def get_author_label(self, obj): """ Returns the role label for the content author. @@ -282,6 +296,17 @@ def get_last_edit(self, obj): last_edit["reason"] = EDIT_REASON_CODES.get(reason_code) return last_edit + def get_edit_by_label(self, obj): + """ + Returns the role label for the last edit user. + """ + is_user_author = str(obj['user_id']) == str(self.context['request'].user.id) + is_user_privileged = _validate_privileged_access(self.context) + edit_history = obj.get("edit_history") + if (is_user_author or is_user_privileged) and edit_history: + last_edit = edit_history[-1] + return self._get_user_label_from_username(last_edit.get('editor_username')) + class ThreadSerializer(_ContentSerializer): """ @@ -316,6 +341,7 @@ class ThreadSerializer(_ContentSerializer): close_reason_code = serializers.CharField(required=False, validators=[validate_close_reason_code]) close_reason = serializers.SerializerMethodField() closed_by = serializers.SerializerMethodField() + closed_by_label = serializers.SerializerMethodField(required=False) non_updatable_fields = NON_UPDATABLE_THREAD_FIELDS @@ -425,6 +451,14 @@ def get_closed_by(self, obj): if _validate_privileged_access(self.context) or is_user_author: return obj.get("closed_by") + def get_closed_by_label(self, obj): + """ + Returns the role label for the user who closed the post. + """ + is_user_author = str(obj['user_id']) == str(self.context['request'].user.id) + if is_user_author or _validate_privileged_access(self.context): + return self._get_user_label_from_username(obj.get("closed_by")) + def create(self, validated_data): thread = Thread(user_id=self.context["cc_requester"]["id"], **validated_data) thread.save() diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index f0daa202e55..09cf2a805c2 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -1472,6 +1472,7 @@ def test_discussion_content(self): "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, }, { "id": "test_comment_2", @@ -1498,6 +1499,7 @@ def test_discussion_content(self): "anonymous": True, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, }, ] actual_comments = self.get_comment_list( @@ -2232,6 +2234,7 @@ def test_success(self, parent_id, mock_emit): "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, } assert actual == expected expected_url = ( @@ -2328,6 +2331,7 @@ def test_success_in_black_out_with_user_access(self, parent_id, mock_emit): "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, } assert actual == expected expected_url = ( @@ -3162,6 +3166,7 @@ def test_basic(self, parent_id): "child_count": 0, "can_delete": True, "last_edit": None, + "edit_by_label": None, } assert actual == expected assert parsed_body(httpretty.last_request()) == { diff --git a/lms/djangoapps/discussion/rest_api/tests/test_serializers.py b/lms/djangoapps/discussion/rest_api/tests/test_serializers.py index bd9cb0f1eac..570c7852e36 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_serializers.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_serializers.py @@ -188,6 +188,8 @@ def test_basic(self): "pinned": True, "editable_fields": ["abuse_flagged", "copy_link", "following", "read", "voted"], "abuse_flagged_count": None, + "edit_by_label": None, + "closed_by_label": None, }) assert self.serialize(thread) == expected @@ -242,20 +244,117 @@ def test_response_count_missing(self): serialized = self.serialize(thread_data) assert 'response_count' not in serialized - def test_get_preview_body(self): + @ddt.data( + (FORUM_ROLE_MODERATOR, True), + (FORUM_ROLE_STUDENT, False), + ("author", True), + ) + @ddt.unpack + def test_closed_by_label_field(self, role, visible): + """ + Tests if closed by field is visible to author and priviledged users """ - Test for the 'get_preview_body' method. + moderator = UserFactory() + request_role = FORUM_ROLE_STUDENT if role == "author" else role + author = self.user if role == "author" else self.author + self.create_role(FORUM_ROLE_MODERATOR, [moderator]) + self.create_role(request_role, [self.user]) + + thread = make_minimal_cs_thread({ + "id": "test_thread", + "course_id": str(self.course.id), + "commentable_id": "test_topic", + "user_id": str(author.id), + "username": author.username, + "title": "Test Title", + "body": "Test body", + "pinned": True, + "votes": {"up_count": 4}, + "comments_count": 5, + "unread_comments_count": 3, + "closed_by": moderator + }) + closed_by_label = "Staff" if visible else None + closed_by = moderator if visible else None + can_delete = role != FORUM_ROLE_STUDENT + editable_fields = ["abuse_flagged", "copy_link", "following", "read", "voted"] + if role == "author": + editable_fields.extend(['anonymous', 'raw_body', 'title', 'topic_id', 'type']) + elif role == FORUM_ROLE_MODERATOR: + editable_fields.extend(['close_reason_code', 'closed', 'edit_reason_code', 'pinned', + 'raw_body', 'title', 'topic_id', 'type']) + expected = self.expected_thread_data({ + "author": author.username, + "can_delete": can_delete, + "vote_count": 4, + "comment_count": 6, + "unread_comment_count": 3, + "pinned": True, + "editable_fields": sorted(editable_fields), + "abuse_flagged_count": None, + "edit_by_label": None, + "closed_by_label": closed_by_label, + "closed_by": closed_by, + }) + assert self.serialize(thread) == expected - This test verifies that the 'get_preview_body' method returns a cleaned - version of the thread's body that is suitable for display as a preview. - The test specifically focuses on handling the presence of multiple - spaces within the body. + @ddt.data( + (FORUM_ROLE_MODERATOR, True), + (FORUM_ROLE_STUDENT, False), + ("author", True), + ) + @ddt.unpack + def test_edit_by_label_field(self, role, visible): """ - thread_data = self.make_cs_content( - {"body": "

This is a test thread body with some text.

"} - ) - serialized = self.serialize(thread_data) - assert serialized['preview_body'] == "This is a test thread body with some text." + Tests if closed by field is visible to author and priviledged users + """ + moderator = UserFactory() + request_role = FORUM_ROLE_STUDENT if role == "author" else role + author = self.user if role == "author" else self.author + self.create_role(FORUM_ROLE_MODERATOR, [moderator]) + self.create_role(request_role, [self.user]) + + thread = make_minimal_cs_thread({ + "id": "test_thread", + "course_id": str(self.course.id), + "commentable_id": "test_topic", + "user_id": str(author.id), + "username": author.username, + "title": "Test Title", + "body": "Test body", + "pinned": True, + "votes": {"up_count": 4}, + "edit_history": [{"editor_username": moderator}], + "comments_count": 5, + "unread_comments_count": 3, + "closed_by": None + }) + edit_by_label = "Staff" if visible else None + can_delete = role != FORUM_ROLE_STUDENT + last_edit = None if role == FORUM_ROLE_STUDENT else {"editor_username": moderator} + editable_fields = ["abuse_flagged", "copy_link", "following", "read", "voted"] + + if role == "author": + editable_fields.extend(['anonymous', 'raw_body', 'title', 'topic_id', 'type']) + elif role == FORUM_ROLE_MODERATOR: + editable_fields.extend(['close_reason_code', 'closed', 'edit_reason_code', 'pinned', + 'raw_body', 'title', 'topic_id', 'type']) + + expected = self.expected_thread_data({ + "author": author.username, + "can_delete": can_delete, + "vote_count": 4, + "comment_count": 6, + "unread_comment_count": 3, + "pinned": True, + "editable_fields": sorted(editable_fields), + "abuse_flagged_count": None, + "last_edit": last_edit, + "edit_by_label": edit_by_label, + "closed_by_label": None, + "closed_by": None, + }) + assert self.serialize(thread) == expected @ddt.ddt @@ -333,6 +432,7 @@ def test_basic(self): "child_count": 0, "can_delete": False, "last_edit": None, + "edit_by_label": None, } assert self.serialize(comment) == expected diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index 559b4ff8f6c..56e690761da 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -1709,6 +1709,8 @@ def test_basic(self): "votes": {"up_count": 4}, "comments_count": 5, "unread_comments_count": 3, + "closed_by_label": None, + "edit_by_label": None, })], "page": 1, "num_pages": 1, @@ -1968,6 +1970,7 @@ def expected_response_comment(self, overrides=None): "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, } response_data.update(overrides or {}) return response_data @@ -2392,6 +2395,7 @@ def test_basic(self): "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, } response = self.client.post( self.url, @@ -2483,6 +2487,7 @@ def expected_response_data(self, overrides=None): "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, } response_data.update(overrides or {}) return response_data @@ -2672,6 +2677,7 @@ def test_basic(self): "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, } response = self.client.get(self.url) diff --git a/lms/djangoapps/discussion/rest_api/tests/utils.py b/lms/djangoapps/discussion/rest_api/tests/utils.py index e4b52853244..fd456674c3a 100644 --- a/lms/djangoapps/discussion/rest_api/tests/utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/utils.py @@ -499,7 +499,9 @@ def expected_thread_data(self, overrides=None): "type": "discussion", "response_count": 0, "last_edit": None, + "edit_by_label": None, "closed_by": None, + "closed_by_label": None, "close_reason": None, "close_reason_code": None, } From da9354245112b7bc0329773e987264914a0a55ea Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Wed, 13 Sep 2023 20:42:39 +0300 Subject: [PATCH 11/24] feat: [AXIM-20] Add profile_image to API CommentViewSet --- .../discussion/rest_api/serializers.py | 6 ++++ .../discussion/rest_api/tests/test_api.py | 35 +++++++++++++++++++ .../rest_api/tests/test_serializers.py | 7 ++++ .../discussion/rest_api/tests/test_views.py | 28 +++++++++++++++ 4 files changed, 76 insertions(+) diff --git a/lms/djangoapps/discussion/rest_api/serializers.py b/lms/djangoapps/discussion/rest_api/serializers.py index 5c2115674c3..a7c0bd47cd1 100644 --- a/lms/djangoapps/discussion/rest_api/serializers.py +++ b/lms/djangoapps/discussion/rest_api/serializers.py @@ -42,6 +42,7 @@ from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientRequestError from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings from openedx.core.lib.api.serializers import CourseKeyField +from openedx.core.djangoapps.user_api.accounts.serializers import AccountLegacyProfileSerializer User = get_user_model() @@ -503,6 +504,7 @@ class CommentSerializer(_ContentSerializer): child_count = serializers.IntegerField(read_only=True) children = serializers.SerializerMethodField(required=False) abuse_flagged_any_user = serializers.SerializerMethodField(required=False) + profile_image = serializers.SerializerMethodField(read_only=True) non_updatable_fields = NON_UPDATABLE_COMMENT_FIELDS @@ -585,6 +587,10 @@ def get_abuse_flagged_any_user(self, obj): if _validate_privileged_access(self.context): return len(obj.get("abuse_flaggers", [])) > 0 + def get_profile_image(self, obj): + request = self.context["request"] + return AccountLegacyProfileSerializer.get_profile_image(request.user.profile, request.user, request) + def validate(self, attrs): """ Ensure that parent_id identifies a comment that is actually in the diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index 09cf2a805c2..fd3024899a0 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -1473,6 +1473,13 @@ def test_discussion_content(self): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, }, { "id": "test_comment_2", @@ -1500,6 +1507,13 @@ def test_discussion_content(self): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, }, ] actual_comments = self.get_comment_list( @@ -2235,6 +2249,13 @@ def test_success(self, parent_id, mock_emit): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } assert actual == expected expected_url = ( @@ -2332,6 +2353,13 @@ def test_success_in_black_out_with_user_access(self, parent_id, mock_emit): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } assert actual == expected expected_url = ( @@ -3167,6 +3195,13 @@ def test_basic(self, parent_id): "can_delete": True, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } assert actual == expected assert parsed_body(httpretty.last_request()) == { diff --git a/lms/djangoapps/discussion/rest_api/tests/test_serializers.py b/lms/djangoapps/discussion/rest_api/tests/test_serializers.py index 570c7852e36..240f78aa367 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_serializers.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_serializers.py @@ -433,6 +433,13 @@ def test_basic(self): "can_delete": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } assert self.serialize(comment) == expected diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index 56e690761da..dccabe1facd 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -1971,6 +1971,13 @@ def expected_response_comment(self, overrides=None): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } response_data.update(overrides or {}) return response_data @@ -2396,6 +2403,13 @@ def test_basic(self): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } response = self.client.post( self.url, @@ -2488,6 +2502,13 @@ def expected_response_data(self, overrides=None): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } response_data.update(overrides or {}) return response_data @@ -2678,6 +2699,13 @@ def test_basic(self): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } response = self.client.get(self.url) From 77987462af3491263800c10b20dd140f3f73bc5b Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Wed, 8 Nov 2023 17:43:57 +0200 Subject: [PATCH 12/24] refactor: [FC-0031] Move get_profile_image method to api --- lms/djangoapps/discussion/rest_api/serializers.py | 4 ++-- openedx/core/djangoapps/user_api/accounts/api.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/serializers.py b/lms/djangoapps/discussion/rest_api/serializers.py index a7c0bd47cd1..2d9056c87c7 100644 --- a/lms/djangoapps/discussion/rest_api/serializers.py +++ b/lms/djangoapps/discussion/rest_api/serializers.py @@ -41,8 +41,8 @@ from openedx.core.djangoapps.django_comment_common.comment_client.user import User as CommentClientUser from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientRequestError from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings +from openedx.core.djangoapps.user_api.accounts.api import get_profile_images from openedx.core.lib.api.serializers import CourseKeyField -from openedx.core.djangoapps.user_api.accounts.serializers import AccountLegacyProfileSerializer User = get_user_model() @@ -589,7 +589,7 @@ def get_abuse_flagged_any_user(self, obj): def get_profile_image(self, obj): request = self.context["request"] - return AccountLegacyProfileSerializer.get_profile_image(request.user.profile, request.user, request) + return get_profile_images(request.user.profile, request.user, request) def validate(self, attrs): """ diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 4e1c6b426db..26836812198 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -33,6 +33,8 @@ AccountValidationError, PreferenceValidationError ) +from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_urls_for_user +from openedx.core.djangoapps.user_api.accounts.serializers import PROFILE_IMAGE_KEY_PREFIX from openedx.core.djangoapps.user_api.preferences.api import update_user_preferences from openedx.core.djangoapps.user_authn.utils import check_pwned_password from openedx.core.djangoapps.user_authn.views.registration_form import validate_name, validate_username @@ -510,6 +512,19 @@ def get_email_existence_validation_error(email): return _validate(_validate_email_doesnt_exist, errors.AccountEmailAlreadyExists, email) +def get_profile_images(user_profile, user, request=None): + """ + Returns metadata about a user's profile image. + """ + data = {'has_image': user_profile.has_profile_image} + urls = get_profile_image_urls_for_user(user, request) + data.update({ + f'{PROFILE_IMAGE_KEY_PREFIX}_{size_display_name}': url + for size_display_name, url in urls.items() + }) + return data + + def _get_user_and_profile(username): """ Helper method to return the legacy user and profile objects based on username. From e0b4d50ce692229ac07d7e311d8c12ae8936e459 Mon Sep 17 00:00:00 2001 From: Glib Glugovskiy Date: Tue, 14 Nov 2023 00:43:10 +0200 Subject: [PATCH 13/24] fix: remove duplicate implementation and correct the docstring --- .../core/djangoapps/user_api/accounts/api.py | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 26836812198..9cc54fa970a 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -33,8 +33,6 @@ AccountValidationError, PreferenceValidationError ) -from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_urls_for_user -from openedx.core.djangoapps.user_api.accounts.serializers import PROFILE_IMAGE_KEY_PREFIX from openedx.core.djangoapps.user_api.preferences.api import update_user_preferences from openedx.core.djangoapps.user_authn.utils import check_pwned_password from openedx.core.djangoapps.user_authn.views.registration_form import validate_name, validate_username @@ -515,14 +513,18 @@ def get_email_existence_validation_error(email): def get_profile_images(user_profile, user, request=None): """ Returns metadata about a user's profile image. + + The output is a dict that looks like: + + { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + } """ - data = {'has_image': user_profile.has_profile_image} - urls = get_profile_image_urls_for_user(user, request) - data.update({ - f'{PROFILE_IMAGE_KEY_PREFIX}_{size_display_name}': url - for size_display_name, url in urls.items() - }) - return data + return AccountLegacyProfileSerializer.get_profile_image(user_profile, user, request) def _get_user_and_profile(username): From f37ddd3215a01da430b09b60362e34226236addf Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Fri, 15 Sep 2023 15:20:18 +0300 Subject: [PATCH 14/24] feat: [AXIM-26] Extended BlocksInCourseView API --- .../mobile_api/course_info/tests.py | 47 +++++++ lms/djangoapps/mobile_api/course_info/urls.py | 3 +- .../mobile_api/course_info/views.py | 120 ++++++++++++++++++ 3 files changed, 169 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/mobile_api/course_info/tests.py b/lms/djangoapps/mobile_api/course_info/tests.py index 086359cafb8..82358862428 100644 --- a/lms/djangoapps/mobile_api/course_info/tests.py +++ b/lms/djangoapps/mobile_api/course_info/tests.py @@ -15,6 +15,7 @@ from common.djangoapps.student.tests.factories import UserFactory # pylint: disable=unused-import from lms.djangoapps.mobile_api.testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin from lms.djangoapps.mobile_api.utils import API_V1, API_V05 +from lms.djangoapps.course_api.blocks.tests.test_views import TestBlocksInCourseView from openedx.features.course_experience import ENABLE_COURSE_GOALS from xmodule.html_block import CourseInfoBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order @@ -255,3 +256,49 @@ def test_flag_disabled(self, mock_logger): 'For this mobile request, user activity is not enabled for this user {} and course {}'.format( str(self.user.id), str(self.course.id)) ) + + +@ddt.ddt +class TestBlocksInfoInCourseView(TestBlocksInCourseView): # lint-amnesty, pylint: disable=test-inherits-tests + """ + Test class for BlocksInfoInCourseView + """ + + def setUp(self): + super().setUp() + self.url = reverse('blocks_info_in_course', kwargs={ + 'api_version': 'v3', + }) + + @patch('lms.djangoapps.mobile_api.course_info.views.certificate_downloadable_status') + def test_additional_info_response(self, mock_certificate_downloadable_status): + certificate_url = 'https://test_certificate_url' + mock_certificate_downloadable_status.return_value = { + 'is_downloadable': True, + 'download_url': certificate_url, + } + + expected_image_urls = { + 'image': + { + 'large': '/asset-v1:edX+toy+2012_Fall+type@asset+block@just_a_test.jpg', + 'raw': '/asset-v1:edX+toy+2012_Fall+type@asset+block@just_a_test.jpg', + 'small': '/asset-v1:edX+toy+2012_Fall+type@asset+block@just_a_test.jpg' + } + } + + response = self.verify_response(url=self.url) + + assert response.status_code == 200 + assert response.data['id'] == str(self.course.id) + assert response.data['name'] == self.course.display_name + assert response.data['number'] == self.course.display_number_with_default + assert response.data['org'] == self.course.display_org_with_default + assert response.data['start'] == self.course.start + assert response.data['start_display'] == 'July 17, 2015' + assert response.data['start_type'] == 'timestamp' + assert response.data['end'] == self.course.end + assert response.data['media'] == expected_image_urls + assert response.data['certificate'] == {'url': certificate_url} + assert response.data['is_self_paced'] is False + mock_certificate_downloadable_status.assert_called_once() diff --git a/lms/djangoapps/mobile_api/course_info/urls.py b/lms/djangoapps/mobile_api/course_info/urls.py index 5314b43bc5b..e369930e255 100644 --- a/lms/djangoapps/mobile_api/course_info/urls.py +++ b/lms/djangoapps/mobile_api/course_info/urls.py @@ -6,7 +6,7 @@ from django.conf import settings from django.urls import path, re_path -from .views import CourseHandoutsList, CourseUpdatesList, CourseGoalsRecordUserActivity +from .views import CourseHandoutsList, CourseUpdatesList, CourseGoalsRecordUserActivity, BlocksInfoInCourseView urlpatterns = [ re_path( @@ -20,4 +20,5 @@ name='course-updates-list' ), path('record_user_activity', CourseGoalsRecordUserActivity.as_view(), name='record_user_activity'), + path('blocks/', BlocksInfoInCourseView.as_view(), name="blocks_info_in_course"), ] diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index cb474a5fd75..65a703f9813 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -12,8 +12,12 @@ from rest_framework.views import APIView from common.djangoapps.static_replace import make_static_urls_absolute +from lms.djangoapps.certificates.api import certificate_downloadable_status from lms.djangoapps.courseware.courses import get_course_info_section_block from lms.djangoapps.course_goals.models import UserActivity +from lms.djangoapps.course_api.blocks.views import BlocksInCourseView +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.lib.api.view_utils import view_auth_classes from openedx.core.lib.xblock_utils import get_course_update_items from openedx.features.course_experience import ENABLE_COURSE_GOALS from ..decorators import mobile_course_access, mobile_view @@ -163,3 +167,119 @@ def post(self, request, *args, **kwargs): # Populate user activity for tracking progress towards a user's course goals UserActivity.record_user_activity(user, course_key) return Response(status=(200)) + + +@view_auth_classes(is_authenticated=False) +class BlocksInfoInCourseView(BlocksInCourseView): + """ + **Use Case** + + Returns the blocks in the course according to the requesting user's access level. + Add to response info fields with information about course + + **Example requests**: + + This api works with all versions {api_version}, you can use: v0.5, v1, v2 or v3 + + GET /api/mobile/{api_version}/course_info/blocks/?course_id= + GET /api/mobile/{api_version}/course_info/blocks/?course_id= + &username=anjali + &depth=all + &requested_fields=graded,format,student_view_multi_device,lti_url + &block_counts=video + &student_view_data=video + &block_types_filter=problem,html + + **Response example** + + Body consists of the following fields: + + root: (str) The ID of the root node of the requested course block structure.\ + blocks: (dict) A dictionary or list, based on the value of the + "return_type" parameter. Maps block usage IDs to a collection of + information about each block. Each block contains the following + fields. + + id: (str) The Course's id (Course Run key) + name: (str) The course's name + number: (str) The course's number + org: (str) The course's organisation + start: (str) Date the course begins, in ISO 8601 notation + start_display: (str) Readably formatted start of the course + start_type: (str) Hint describing how `start_display` is set. One of: + * `"string"`: manually set by the course author + * `"timestamp"`: generated from the `start` timestamp + * `"empty"`: no start date is specified + end: (str) Date the course ends, in ISO 8601 notation + media: (dict) An object that contains named media items. Included here: + * course_image: An image to show for the course. Represented + as an object with the following fields: + * uri: The location of the image + certificate: (dict) Information about the user's earned certificate in the course. + Included here: + * uri: The location of the user's certificate + is_self_paced: (bool) Indicates if the course is self paced + + **Returns** + + * 200 on success with above fields. + * 400 if an invalid parameter was sent or the username was not provided + * 401 unauthorized, the provided access token has expired and is no longer valid + for an authenticated request. + * 403 if a user who does not have permission to masquerade as + another user specifies a username other than their own. + * 404 if the course is not available or cannot be seen. + """ + + def get_certificate(self, request, course_id): + """Returns the information about the user's certificate in the course.""" + if request.user.is_authenticated: + certificate_info = certificate_downloadable_status(request.user, course_id) + if certificate_info['is_downloadable']: + return { + 'url': request.build_absolute_uri( + certificate_info['download_url'] + ), + } + return {} + + # pylint: disable=arguments-differ + def list(self, request, **kwargs): + """ + REST API endpoint for listing all the blocks information in the course and + information about the course while regarding user access and roles. + + Arguments: + request - Django request object + """ + + response = super().list(request, kwargs) + + if request.GET.get('return_type', 'dict') == 'dict': + course_id = request.query_params.get('course_id', None) + course_key = CourseKey.from_string(course_id) + course_overview = CourseOverview.get_from_id(course_key) + + course_data = { + # identifiers + 'id': course_id, + 'name': course_overview.display_name, + 'number': course_overview.display_number_with_default, + 'org': course_overview.display_org_with_default, + + # dates + 'start': course_overview.start, + 'start_display': course_overview.start_display, + 'start_type': course_overview.start_type, + 'end': course_overview.end, + + # various URLs + 'media': { + 'image': course_overview.image_urls, + }, + 'certificate': self.get_certificate(request, course_key), + 'is_self_paced': course_overview.self_paced + } + + response.data.update(course_data) + return response From 3bb8ebcf6ac16937c0f8cc88dfdb91eba7a39f69 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Mon, 30 Oct 2023 16:50:25 +0200 Subject: [PATCH 15/24] fix: [FC-0031] Add parameters description, refactor list method --- .../mobile_api/course_info/views.py | 67 +++++++++++++------ 1 file changed, 48 insertions(+), 19 deletions(-) diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index 65a703f9813..404cc5420a7 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -190,6 +190,23 @@ class BlocksInfoInCourseView(BlocksInCourseView): &student_view_data=video &block_types_filter=problem,html + **Parameters:** + + username (str): The username of the specified user for whom the course data + is being accessed. + depth (integer, str, None): Optional number of blocks you receive in response + course nesting depth, you can get only sections, sections and subsections, + or provide string 'all' to receive all blocks of the course. + requested_field (list): Optional list of names of additional fields to return for each block. + Supported fields can be found in transformers.SUPPORTED_FIELDS. + block_counts (list): Optional list of names of block types for which an aggregated count + of blocks is returned. + student_view_data (list): Optional list of names of block types for + which student_view_data is returned. + block_types_filter (list): Filter by block types: + 'video', 'discussion', 'html', 'chapter', 'sequential', 'vertical'. + return_type (list, dict): Optional list or dictionary of block's fields based on 'return_type'. + **Response example** Body consists of the following fields: @@ -243,8 +260,36 @@ def get_certificate(self, request, course_id): } return {} - # pylint: disable=arguments-differ - def list(self, request, **kwargs): + @staticmethod + def compose_course_info(course_overview): + """ + Method for obtaining additional information about the course. + + Arguments: + request - Django request object + """ + + course_data = { + # identifiers + 'name': course_overview.display_name, + 'number': course_overview.display_number_with_default, + 'org': course_overview.display_org_with_default, + + # dates + 'start': course_overview.start, + 'start_display': course_overview.start_display, + 'start_type': course_overview.start_type, + 'end': course_overview.end, + + # various URLs + 'media': { + 'image': course_overview.image_urls, + }, + 'is_self_paced': course_overview.self_paced + } + return course_data + + def list(self, request, **kwargs): # pylint: disable=W0221 """ REST API endpoint for listing all the blocks information in the course and information about the course while regarding user access and roles. @@ -259,27 +304,11 @@ def list(self, request, **kwargs): course_id = request.query_params.get('course_id', None) course_key = CourseKey.from_string(course_id) course_overview = CourseOverview.get_from_id(course_key) - course_data = { - # identifiers 'id': course_id, - 'name': course_overview.display_name, - 'number': course_overview.display_number_with_default, - 'org': course_overview.display_org_with_default, - - # dates - 'start': course_overview.start, - 'start_display': course_overview.start_display, - 'start_type': course_overview.start_type, - 'end': course_overview.end, - - # various URLs - 'media': { - 'image': course_overview.image_urls, - }, 'certificate': self.get_certificate(request, course_key), - 'is_self_paced': course_overview.self_paced } + course_data.update(BlocksInfoInCourseView.compose_course_info(course_overview)) response.data.update(course_data) return response From 462c7807d2f6b06832cfd4ceb066a477d823483a Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Fri, 15 Dec 2023 14:10:58 +0200 Subject: [PATCH 16/24] refactor: [FC-0031] Use serializer instead of custom function --- .../mobile_api/course_info/serializers.py | 36 +++++++++++ .../mobile_api/course_info/tests.py | 2 +- .../mobile_api/course_info/views.py | 63 +++++++++---------- 3 files changed, 67 insertions(+), 34 deletions(-) create mode 100644 lms/djangoapps/mobile_api/course_info/serializers.py diff --git a/lms/djangoapps/mobile_api/course_info/serializers.py b/lms/djangoapps/mobile_api/course_info/serializers.py new file mode 100644 index 00000000000..c996d24945b --- /dev/null +++ b/lms/djangoapps/mobile_api/course_info/serializers.py @@ -0,0 +1,36 @@ +""" +Course Info serializers +""" +from rest_framework import serializers + +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + + +class CourseInfoOverviewSerializer(serializers.ModelSerializer): + """ + Serializer for serialize additional fields in BlocksInfoInCourseView. + """ + + name = serializers.CharField(source='display_name') + number = serializers.CharField(source='display_number_with_default') + org = serializers.CharField(source='display_org_with_default') + is_self_paced = serializers.BooleanField(source='self_paced') + media = serializers.SerializerMethodField() + + class Meta: + model = CourseOverview + fields = ( + 'name', + 'number', + 'org', + 'start', + 'start_display', + 'start_type', + 'end', + 'is_self_paced', + 'media', + ) + + @staticmethod + def get_media(obj): + return {'image': obj.image_urls} diff --git a/lms/djangoapps/mobile_api/course_info/tests.py b/lms/djangoapps/mobile_api/course_info/tests.py index 82358862428..e7552382cb3 100644 --- a/lms/djangoapps/mobile_api/course_info/tests.py +++ b/lms/djangoapps/mobile_api/course_info/tests.py @@ -294,7 +294,7 @@ def test_additional_info_response(self, mock_certificate_downloadable_status): assert response.data['name'] == self.course.display_name assert response.data['number'] == self.course.display_number_with_default assert response.data['org'] == self.course.display_org_with_default - assert response.data['start'] == self.course.start + assert response.data['start'] == self.course.start.strftime('%Y-%m-%dT%H:%M:%SZ') assert response.data['start_display'] == 'July 17, 2015' assert response.data['start_type'] == 'timestamp' assert response.data['end'] == self.course.end diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index 404cc5420a7..587da7cab4d 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -16,6 +16,7 @@ from lms.djangoapps.courseware.courses import get_course_info_section_block from lms.djangoapps.course_goals.models import UserActivity from lms.djangoapps.course_api.blocks.views import BlocksInCourseView +from lms.djangoapps.mobile_api.course_info.serializers import CourseInfoOverviewSerializer from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.api.view_utils import view_auth_classes from openedx.core.lib.xblock_utils import get_course_update_items @@ -209,7 +210,8 @@ class BlocksInfoInCourseView(BlocksInCourseView): **Response example** - Body consists of the following fields: + Body consists of the following fields, you received this response if you use + 'return_type=dict' in query params: root: (str) The ID of the root node of the requested course block structure.\ blocks: (dict) A dictionary or list, based on the value of the @@ -237,6 +239,22 @@ class BlocksInfoInCourseView(BlocksInCourseView): * uri: The location of the user's certificate is_self_paced: (bool) Indicates if the course is self paced + Body consists of the following fields, you received this response if you use + 'return_type=list' in query params: + + id: (str) The Course's id (Course Run key) + block_id: (str) The unique identifier for the block_id + lms_web_url: (str) The URL to the navigational container of the xBlock on the web. + legacy_web_url: (str) Like `lms_web_url`, but always directs to + the "Legacy" frontend experience. + student_view_url: (str) The URL to retrieve the HTML rendering + of this block's student view + type: (str): The type of block. Possible values the names of any + XBlock type in the system, including custom blocks. Examples are + course, chapter, sequential, vertical, html, problem, video, and + discussion. + display_name: (str) The display name of the block. + **Returns** * 200 on success with above fields. @@ -249,7 +267,16 @@ class BlocksInfoInCourseView(BlocksInCourseView): """ def get_certificate(self, request, course_id): - """Returns the information about the user's certificate in the course.""" + """ + Returns the information about the user's certificate in the course. + + Arguments: + request (Request): The request object. + course_id (str): The identifier of the course. + Returns: + (dict): A dict containing information about location of the user's certificate + or an empty dictionary, if there is no certificate. + """ if request.user.is_authenticated: certificate_info = certificate_downloadable_status(request.user, course_id) if certificate_info['is_downloadable']: @@ -260,35 +287,6 @@ def get_certificate(self, request, course_id): } return {} - @staticmethod - def compose_course_info(course_overview): - """ - Method for obtaining additional information about the course. - - Arguments: - request - Django request object - """ - - course_data = { - # identifiers - 'name': course_overview.display_name, - 'number': course_overview.display_number_with_default, - 'org': course_overview.display_org_with_default, - - # dates - 'start': course_overview.start, - 'start_display': course_overview.start_display, - 'start_type': course_overview.start_type, - 'end': course_overview.end, - - # various URLs - 'media': { - 'image': course_overview.image_urls, - }, - 'is_self_paced': course_overview.self_paced - } - return course_data - def list(self, request, **kwargs): # pylint: disable=W0221 """ REST API endpoint for listing all the blocks information in the course and @@ -308,7 +306,6 @@ def list(self, request, **kwargs): # pylint: disable=W0221 'id': course_id, 'certificate': self.get_certificate(request, course_key), } - - course_data.update(BlocksInfoInCourseView.compose_course_info(course_overview)) + course_data.update(CourseInfoOverviewSerializer(course_overview).data) response.data.update(course_data) return response From fd3d355bba5c8d3175c7339fbe9de2e402459538 Mon Sep 17 00:00:00 2001 From: Glib Glugovskiy Date: Tue, 19 Dec 2023 19:14:29 +0200 Subject: [PATCH 17/24] docs: [FC-0031] Update docstring --- lms/djangoapps/mobile_api/course_info/views.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index 587da7cab4d..7198cfc818a 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -175,8 +175,10 @@ class BlocksInfoInCourseView(BlocksInCourseView): """ **Use Case** - Returns the blocks in the course according to the requesting user's access level. - Add to response info fields with information about course + This API endpoint is specifically optimized for the course homepage on Mobile Apps. + The endpoint returns the blocks in the course according to the requesting user's access level. + Additionally, response encompasses info fields with information about the course, + including certificate URL, media dictionary with course image URLs, start and end dates for the course. **Example requests**: From 4d179d620ba7ceff55b84712d1b5964c9646be4a Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Tue, 31 Oct 2023 16:02:36 +0200 Subject: [PATCH 18/24] feat: [FC-0031] Add optional field 'is_enrolled' to course detail view --- lms/djangoapps/course_api/serializers.py | 11 +++++ .../course_api/tests/test_serializers.py | 43 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/lms/djangoapps/course_api/serializers.py b/lms/djangoapps/course_api/serializers.py index dbf4736d4e8..e4855e05cb1 100644 --- a/lms/djangoapps/course_api/serializers.py +++ b/lms/djangoapps/course_api/serializers.py @@ -5,6 +5,8 @@ import urllib +from common.djangoapps.student.models import CourseEnrollment +from django.contrib.auth import get_user_model from django.urls import reverse from edx_django_utils import monitoring as monitoring_utils from rest_framework import serializers @@ -164,10 +166,19 @@ def to_representation(self, instance): """ Get the `certificate_available_date` in response if the `certificates.auto_certificate_generation` waffle switch is enabled + + Get the 'is_enrolled' in response + if user is authenticated and 'username' is in query params. """ response = super().to_representation(instance) if can_show_certificate_available_date_field(instance): response['certificate_available_date'] = instance.certificate_available_date + + requested_user = self.context['request'].query_params.get('username', None) + if self.context['request'].user.is_authenticated and requested_user: + User = get_user_model() + requested_user = User.objects.get(username=requested_user) + response['is_enrolled'] = CourseEnrollment.is_enrolled(requested_user, instance.id) return response diff --git a/lms/djangoapps/course_api/tests/test_serializers.py b/lms/djangoapps/course_api/tests/test_serializers.py index 4ee5e10265e..21be6b793c1 100644 --- a/lms/djangoapps/course_api/tests/test_serializers.py +++ b/lms/djangoapps/course_api/tests/test_serializers.py @@ -193,6 +193,49 @@ def test_basic(self): ) self.assertDictEqual(result, self.expected_data) + @mock.patch('lms.djangoapps.course_api.serializers.CourseEnrollment.is_enrolled', return_value=True) + def test_is_enrolled_field_true(self, mock_is_enrolled): + course = self.create_course() + result = self._get_result_with_query_param(course) + assert result['is_enrolled'] is True + mock_is_enrolled.assert_called_once() + + @mock.patch('lms.djangoapps.course_api.serializers.CourseEnrollment.is_enrolled', return_value=False) + def test_is_enrolled_field_false(self, mock_is_enrolled): + course = self.create_course() + result = self._get_result_with_query_param(course) + assert result['is_enrolled'] is False + mock_is_enrolled.assert_called_once() + + def test_is_enrolled_field_anonymous_user(self): + course = self.create_course() + result = self._get_anonymous_result(course) + self.assertNotIn('is_enrolled', result) + + def _get_anonymous_request(self): + return Request(self.request_factory.get('/')) + + def _get_anonymous_result(self, course): + course_overview = CourseOverview.get_from_id(course.id) + return self.serializer_class(course_overview, context={'request': self._get_anonymous_request()}).data + + def _get_result_with_query_param(self, course): + """ + Return the CourseSerializer for the specified course with 'username' in query params. + """ + course_overview = CourseOverview.get_from_id(course.id) + return self.serializer_class(course_overview, context={'request': self._get_request_with_query_param()}).data + + def _get_request_with_query_param(self, user=None): + """ + Build a Request object for the specified user with 'username' in query params. + """ + if user is None: + user = self.honor_user + request = Request(self.request_factory.get('/', {'username': user.username})) + request.user = user + return request + class TestCourseKeySerializer(TestCase): # lint-amnesty, pylint: disable=missing-class-docstring From 5c1fff6c1b5050e65231622b9f34128309a596b3 Mon Sep 17 00:00:00 2001 From: Glib Glugovskiy Date: Fri, 15 Dec 2023 20:28:31 +0200 Subject: [PATCH 19/24] fix: [FC-0031] Restrict access to is_enrolled field --- lms/djangoapps/course_api/serializers.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/course_api/serializers.py b/lms/djangoapps/course_api/serializers.py index e4855e05cb1..5fc71137678 100644 --- a/lms/djangoapps/course_api/serializers.py +++ b/lms/djangoapps/course_api/serializers.py @@ -167,18 +167,22 @@ def to_representation(self, instance): Get the `certificate_available_date` in response if the `certificates.auto_certificate_generation` waffle switch is enabled - Get the 'is_enrolled' in response - if user is authenticated and 'username' is in query params. + Get the 'is_enrolled' in response if 'username' is in query params, + user is staff, superuser, or user is authenticated and + the has the same 'username' as the 'username' in the query params. """ response = super().to_representation(instance) if can_show_certificate_available_date_field(instance): response['certificate_available_date'] = instance.certificate_available_date - requested_user = self.context['request'].query_params.get('username', None) - if self.context['request'].user.is_authenticated and requested_user: - User = get_user_model() - requested_user = User.objects.get(username=requested_user) - response['is_enrolled'] = CourseEnrollment.is_enrolled(requested_user, instance.id) + requested_username = self.context['request'].query_params.get('username', None) + if requested_username: + user = self.context['request'].user + if ((user.is_authenticated and user.username == requested_username) + or user.is_staff or user.is_superuser): + User = get_user_model() + requested_user = User.objects.get(username=requested_username) + response['is_enrolled'] = CourseEnrollment.is_enrolled(requested_user, instance.id) return response From 8a43facaaa00b1881a8caa74c19b0fca75270a2a Mon Sep 17 00:00:00 2001 From: Glib Glugovskiy Date: Thu, 25 Apr 2024 11:13:43 +0300 Subject: [PATCH 20/24] feat(mobile_api): Add course access object to mobile course info API (#34273) * feat: include access serializer into mobile info api view * test: add tests for serializer and view methods * test: move tests to common directory and update test case * fix: cr fixes and use snake case for functions * test: fix additional get call assertion * feat: add required course access messages to mobile endpoint * test: [AXM-229] Improve test coverage * style: [AXM-229] Try to fix linters * fix: remove redundant comment * refactor: change names for the test files --------- Co-authored-by: KyryloKireiev --- .../mobile_api/course_info/serializers.py | 95 +++++++++- .../mobile_api/course_info/views.py | 94 ++++++++-- .../tests/test_course_info_serializers.py | 171 ++++++++++++++++++ .../test_course_info_views.py} | 130 ++++++++++++- 4 files changed, 470 insertions(+), 20 deletions(-) create mode 100644 lms/djangoapps/mobile_api/tests/test_course_info_serializers.py rename lms/djangoapps/mobile_api/{course_info/tests.py => tests/test_course_info_views.py} (69%) diff --git a/lms/djangoapps/mobile_api/course_info/serializers.py b/lms/djangoapps/mobile_api/course_info/serializers.py index c996d24945b..b2bb0ce2470 100644 --- a/lms/djangoapps/mobile_api/course_info/serializers.py +++ b/lms/djangoapps/mobile_api/course_info/serializers.py @@ -2,13 +2,25 @@ Course Info serializers """ from rest_framework import serializers +from typing import Union +from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.util.course import get_encoded_course_sharing_utm_params, get_link_for_about_page +from common.djangoapps.util.milestones_helpers import ( + get_pre_requisite_courses_not_completed, +) +from lms.djangoapps.courseware.access import has_access +from lms.djangoapps.courseware.access import administrative_accesses_to_course_for_user +from lms.djangoapps.courseware.access_utils import check_course_open_for_learner +from lms.djangoapps.mobile_api.users.serializers import ModeSerializer from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.features.course_duration_limits.access import get_user_course_expiration_date class CourseInfoOverviewSerializer(serializers.ModelSerializer): """ - Serializer for serialize additional fields in BlocksInfoInCourseView. + Serializer for additional course fields that should be returned in BlocksInfoInCourseView. """ name = serializers.CharField(source='display_name') @@ -16,6 +28,9 @@ class CourseInfoOverviewSerializer(serializers.ModelSerializer): org = serializers.CharField(source='display_org_with_default') is_self_paced = serializers.BooleanField(source='self_paced') media = serializers.SerializerMethodField() + course_sharing_utm_parameters = serializers.SerializerMethodField() + course_about = serializers.SerializerMethodField('get_course_about_url') + course_modes = serializers.SerializerMethodField() class Meta: model = CourseOverview @@ -29,8 +44,86 @@ class Meta: 'end', 'is_self_paced', 'media', + 'course_sharing_utm_parameters', + 'course_about', + 'course_modes', ) @staticmethod def get_media(obj): + """ + Return course images in the correct format. + """ return {'image': obj.image_urls} + + def get_course_sharing_utm_parameters(self, obj): + return get_encoded_course_sharing_utm_params() + + def get_course_about_url(self, course_overview): + return get_link_for_about_page(course_overview) + + def get_course_modes(self, course_overview): + """ + Retrieve course modes associated with the course. + """ + course_modes = CourseMode.modes_for_course( + course_overview.id, + only_selectable=False + ) + return [ + ModeSerializer(mode).data + for mode in course_modes + ] + + +class MobileCourseEnrollmentSerializer(serializers.ModelSerializer): + """ + Serializer for the CourseEnrollment object used in the BlocksInfoInCourseView. + """ + + class Meta: + fields = ('created', 'mode', 'is_active') + model = CourseEnrollment + lookup_field = 'username' + + +class CourseAccessSerializer(serializers.Serializer): + """ + Get info whether a user should be able to view course material. + """ + + has_unmet_prerequisites = serializers.SerializerMethodField(method_name='get_has_unmet_prerequisites') + is_too_early = serializers.SerializerMethodField(method_name='get_is_too_early') + is_staff = serializers.SerializerMethodField(method_name='get_is_staff') + audit_access_expires = serializers.SerializerMethodField() + courseware_access = serializers.SerializerMethodField() + + def get_has_unmet_prerequisites(self, data: dict) -> bool: + """ + Check whether or not a course has unmet prerequisites. + """ + return any(get_pre_requisite_courses_not_completed(data.get('user'), [data.get('course_id')])) + + def get_is_too_early(self, data: dict) -> bool: + """ + Determine if the course is open to a learner (course has started or user has early beta access). + """ + return not check_course_open_for_learner(data.get('user'), data.get('course')) + + def get_is_staff(self, data: dict) -> bool: + """ + Determine whether a user has staff access to this course. + """ + return any(administrative_accesses_to_course_for_user(data.get('user'), data.get('course_id'))) + + def get_audit_access_expires(self, data: dict) -> Union[str, None]: + """ + Returns expiration date for a course audit expiration, if any or null + """ + return get_user_course_expiration_date(data.get('user'), data.get('course')) + + def get_courseware_access(self, data: dict) -> dict: + """ + Determine if the learner has access to the course, otherwise show error message. + """ + return has_access(data.get('user'), 'load_mobile', data.get('course')).to_json() diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index 7198cfc818a..fc48a7c4f93 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -3,20 +3,28 @@ """ import logging +from typing import Optional, Union +import django from django.contrib.auth import get_user_model from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from rest_framework import generics, status from rest_framework.response import Response +from rest_framework.reverse import reverse from rest_framework.views import APIView +from common.djangoapps.student.models import CourseEnrollment, User as StudentUser from common.djangoapps.static_replace import make_static_urls_absolute from lms.djangoapps.certificates.api import certificate_downloadable_status from lms.djangoapps.courseware.courses import get_course_info_section_block from lms.djangoapps.course_goals.models import UserActivity from lms.djangoapps.course_api.blocks.views import BlocksInCourseView -from lms.djangoapps.mobile_api.course_info.serializers import CourseInfoOverviewSerializer +from lms.djangoapps.mobile_api.course_info.serializers import ( + CourseInfoOverviewSerializer, + CourseAccessSerializer, + MobileCourseEnrollmentSerializer +) from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.api.view_utils import view_auth_classes from openedx.core.lib.xblock_utils import get_course_update_items @@ -26,6 +34,8 @@ User = get_user_model() log = logging.getLogger(__name__) +UserType = Union[django.contrib.auth.models.User, django.contrib.auth.models.AnonymousUser, StudentUser] + @mobile_view() class CourseUpdatesList(generics.ListAPIView): @@ -268,31 +278,52 @@ class BlocksInfoInCourseView(BlocksInCourseView): * 404 if the course is not available or cannot be seen. """ - def get_certificate(self, request, course_id): + def get_requested_user(self, user: UserType, username: Optional[str] = None) -> Union[UserType, None]: + """ + Return a user for whom the course blocks are fetched. + + Arguments: + user: current user from request. + username: string with username. + Returns: A user object or None. + """ + if user.is_anonymous: + return None + + if not username or (username and user.username == username): + return user + if username and (user.is_staff or user.is_superuser): + try: + return User.objects.get(username=username) + except User.DoesNotExist: + log.warning('Provided username does not correspond to an existing user %s', username) + return None + + def get_certificate(self, request, user, course_id): """ - Returns the information about the user's certificate in the course. + Return the information about the user's certificate in the course. Arguments: request (Request): The request object. + user (User): The user object. course_id (str): The identifier of the course. Returns: (dict): A dict containing information about location of the user's certificate or an empty dictionary, if there is no certificate. """ - if request.user.is_authenticated: - certificate_info = certificate_downloadable_status(request.user, course_id) - if certificate_info['is_downloadable']: - return { - 'url': request.build_absolute_uri( - certificate_info['download_url'] - ), - } + certificate_info = certificate_downloadable_status(user, course_id) + if certificate_info['is_downloadable']: + return { + 'url': request.build_absolute_uri( + certificate_info['download_url'] + ), + } return {} def list(self, request, **kwargs): # pylint: disable=W0221 """ REST API endpoint for listing all the blocks information in the course and - information about the course while regarding user access and roles. + information about the course considering user access and roles. Arguments: request - Django request object @@ -301,13 +332,48 @@ def list(self, request, **kwargs): # pylint: disable=W0221 response = super().list(request, kwargs) if request.GET.get('return_type', 'dict') == 'dict': + api_version = self.kwargs.get('api_version') course_id = request.query_params.get('course_id', None) course_key = CourseKey.from_string(course_id) course_overview = CourseOverview.get_from_id(course_key) + requested_username = request.query_params.get('username', None) + course_data = { 'id': course_id, - 'certificate': self.get_certificate(request, course_key), + 'course_updates': reverse( + 'course-updates-list', + kwargs={'api_version': api_version, 'course_id': course_id}, + request=request, + ), + 'course_handouts': reverse( + 'course-handouts-list', + kwargs={'api_version': api_version, 'course_id': course_id}, + request=request, + ), } - course_data.update(CourseInfoOverviewSerializer(course_overview).data) + + course_info_context = {} + if requested_user := self.get_requested_user(request.user, requested_username): + course_info_context = { + 'user': requested_user + } + user_enrollment = CourseEnrollment.get_enrollment(user=requested_user, course_key=course_key) + course_data.update({ + 'discussion_url': reverse( + 'discussion_course', + kwargs={'course_id': course_id}, + request=request, + ) if course_overview.is_discussion_tab_enabled(requested_user) else None, + 'course_access_details': CourseAccessSerializer({ + 'user': requested_user, + 'course': course_overview, + 'course_id': course_key + }).data, + 'certificate': self.get_certificate(request, requested_user, course_key), + 'enrollment_details': MobileCourseEnrollmentSerializer(user_enrollment).data, + }) + + course_data.update(CourseInfoOverviewSerializer(course_overview, context=course_info_context).data) + response.data.update(course_data) return response diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py b/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py new file mode 100644 index 00000000000..6c50f68d681 --- /dev/null +++ b/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py @@ -0,0 +1,171 @@ +""" +Tests for serializers for the Mobile Course Info +""" + +import ddt +from django.test import TestCase +from mock import MagicMock, Mock, patch +from typing import Dict, List, Tuple, Union + +from common.djangoapps.student.tests.factories import UserFactory +from lms.djangoapps.mobile_api.course_info.serializers import ( + CourseAccessSerializer, + CourseInfoOverviewSerializer, +) +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory + + +@ddt.ddt +class TestCourseAccessSerializer(TestCase): + """ + Tests for the CourseAccessSerializer. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory() + self.course = CourseOverviewFactory() + + @ddt.data( + ([{'course_id': {}}], True), + ([], False), + ) + @ddt.unpack + @patch('lms.djangoapps.mobile_api.course_info.serializers.get_pre_requisite_courses_not_completed') + def test_has_unmet_prerequisites( + self, + mock_return_value: List[Dict], + has_unmet_prerequisites: bool, + mock_get_prerequisites: MagicMock, + ) -> None: + mock_get_prerequisites.return_value = mock_return_value + + output_data = CourseAccessSerializer({ + 'user': self.user, + 'course': self.course, + 'course_id': self.course.id, + }).data + + self.assertEqual(output_data['has_unmet_prerequisites'], has_unmet_prerequisites) + mock_get_prerequisites.assert_called_once_with(self.user, [self.course.id]) + + @ddt.data( + (True, False), + (False, True), + ) + @ddt.unpack + @patch('lms.djangoapps.mobile_api.course_info.serializers.check_course_open_for_learner') + def test_is_too_early( + self, + mock_return_value: bool, + is_too_early: bool, + mock_check_course_open: MagicMock, + ) -> None: + mock_check_course_open.return_value = mock_return_value + + output_data = CourseAccessSerializer({ + 'user': self.user, + 'course': self.course, + 'course_id': self.course.id + }).data + + self.assertEqual(output_data['is_too_early'], is_too_early) + mock_check_course_open.assert_called_once_with(self.user, self.course) + + @ddt.data( + ((False, False, False), False), + ((True, True, True), True), + ((True, False, False), True), + ) + @ddt.unpack + @patch('lms.djangoapps.mobile_api.course_info.serializers.administrative_accesses_to_course_for_user') + def test_is_staff( + self, + mock_return_value: Tuple[bool], + is_staff: bool, + mock_administrative_access: MagicMock, + ) -> None: + mock_administrative_access.return_value = mock_return_value + + output_data = CourseAccessSerializer({ + 'user': self.user, + 'course': self.course, + 'course_id': self.course.id + }).data + + self.assertEqual(output_data['is_staff'], is_staff) + mock_administrative_access.assert_called_once_with(self.user, self.course.id) + + @ddt.data(None, 'mocked_user_course_expiration_date') + @patch('lms.djangoapps.mobile_api.course_info.serializers.get_user_course_expiration_date') + def test_get_audit_access_expires( + self, + mock_return_value: Union[str, None], + mock_get_user_course_expiration_date: MagicMock, + ) -> None: + mock_get_user_course_expiration_date.return_value = mock_return_value + + output_data = CourseAccessSerializer({ + 'user': self.user, + 'course': self.course, + 'course_id': self.course.id + }).data + + self.assertEqual(output_data['audit_access_expires'], mock_return_value) + mock_get_user_course_expiration_date.assert_called_once_with(self.user, self.course) + + @patch('lms.djangoapps.mobile_api.course_info.serializers.has_access') + def test_get_courseware_access(self, mock_has_access: MagicMock) -> None: + mocked_access = { + 'has_access': True, + 'error_code': None, + 'developer_message': None, + 'user_message': None, + 'additional_context_user_message': None, + 'user_fragment': None + } + mock_has_access.return_value = Mock(to_json=Mock(return_value=mocked_access)) + + output_data = CourseAccessSerializer({ + 'user': self.user, + 'course': self.course, + 'course_id': self.course.id + }).data + + self.assertDictEqual(output_data['courseware_access'], mocked_access) + mock_has_access.assert_called_once_with(self.user, 'load_mobile', self.course) + mock_has_access.return_value.to_json.assert_called_once_with() + + +class TestCourseInfoOverviewSerializer(TestCase): + """ + Tests for the CourseInfoOverviewSerializer. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory() + self.course_overview = CourseOverviewFactory() + + def test_get_media(self): + output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data + + self.assertIn('media', output_data) + self.assertIn('image', output_data['media']) + self.assertIn('raw', output_data['media']['image']) + self.assertIn('small', output_data['media']['image']) + self.assertIn('large', output_data['media']['image']) + + @patch('lms.djangoapps.mobile_api.course_info.serializers.get_link_for_about_page', return_value='mock_about_link') + def test_get_course_sharing_utm_parameters(self, mock_get_link_for_about_page: MagicMock) -> None: + output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data + + self.assertEqual(output_data['course_about'], mock_get_link_for_about_page.return_value) + mock_get_link_for_about_page.assert_called_once_with(self.course_overview) + + def test_get_course_modes(self): + expected_course_modes = [{'slug': 'audit', 'sku': None, 'android_sku': None, 'ios_sku': None, 'min_price': 0}] + + output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data + + self.assertListEqual(output_data['course_modes'], expected_course_modes) diff --git a/lms/djangoapps/mobile_api/course_info/tests.py b/lms/djangoapps/mobile_api/tests/test_course_info_views.py similarity index 69% rename from lms/djangoapps/mobile_api/course_info/tests.py rename to lms/djangoapps/mobile_api/tests/test_course_info_views.py index e7552382cb3..67d2c79f901 100644 --- a/lms/djangoapps/mobile_api/course_info/tests.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_views.py @@ -5,26 +5,33 @@ import ddt from django.conf import settings +from django.contrib.auth import get_user_model +from django.contrib.auth.models import AnonymousUser +from django.test import RequestFactory from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag from milestones.tests.utils import MilestonesTestCaseMixin from mock import patch -from rest_framework.test import APIClient # pylint: disable=unused-import +from rest_framework import status -from common.djangoapps.student.models import CourseEnrollment # pylint: disable=unused-import from common.djangoapps.student.tests.factories import UserFactory # pylint: disable=unused-import +from common.djangoapps.util.course import get_link_for_about_page from lms.djangoapps.mobile_api.testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin from lms.djangoapps.mobile_api.utils import API_V1, API_V05 +from lms.djangoapps.mobile_api.course_info.views import BlocksInfoInCourseView from lms.djangoapps.course_api.blocks.tests.test_views import TestBlocksInCourseView +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.features.course_experience import ENABLE_COURSE_GOALS from xmodule.html_block import CourseInfoBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=unused-import, wrong-import-order from xmodule.modulestore.xml_importer import import_course_from_xml # lint-amnesty, pylint: disable=wrong-import-order +User = get_user_model() + + @ddt.ddt class TestUpdates(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin, MilestonesTestCaseMixin): """ @@ -259,9 +266,9 @@ def test_flag_disabled(self, mock_logger): @ddt.ddt -class TestBlocksInfoInCourseView(TestBlocksInCourseView): # lint-amnesty, pylint: disable=test-inherits-tests +class TestBlocksInfoInCourseView(TestBlocksInCourseView, MilestonesTestCaseMixin): # lint-amnesty, pylint: disable=test-inherits-tests """ - Test class for BlocksInfoInCourseView + Test class for BlocksInfoInCourseView """ def setUp(self): @@ -269,6 +276,70 @@ def setUp(self): self.url = reverse('blocks_info_in_course', kwargs={ 'api_version': 'v3', }) + self.request = RequestFactory().get(self.url) + self.student_user = UserFactory.create(username="student_user") + + @ddt.data( + ('anonymous', None, None), + ('staff', 'student_user', 'student_user'), + ('student', 'student_user', 'student_user'), + ('student', None, 'student_user'), + ('student', 'other_student', None), + ) + @ddt.unpack + @patch('lms.djangoapps.mobile_api.course_info.views.User.objects.get') + def test_get_requested_user(self, user_role, username, expected_username, mock_get): + """ + Test get_requested_user utility from the BlocksInfoInCourseView. + + Parameters: + user_role: type of the user that making a request. + username: username query parameter from the request. + expected_username: username of the returned user. + """ + if user_role == 'anonymous': + request_user = AnonymousUser() + elif user_role == 'staff': + request_user = self.admin_user + elif user_role == 'student': + request_user = self.student_user + + self.request.user = request_user + + if expected_username == 'student_user': + mock_user = self.student_user + mock_get.return_value = mock_user + + result_user = BlocksInfoInCourseView().get_requested_user(self.request.user, username) + if expected_username: + self.assertEqual(result_user.username, expected_username) + if username and request_user.username != username: + mock_get.assert_called_with(username=username) + else: + self.assertIsNone(result_user) + + @ddt.data( + ({'is_downloadable': True, 'download_url': 'https://test_certificate_url'}, + {'url': 'https://test_certificate_url'}), + ({'is_downloadable': False}, {}), + ) + @ddt.unpack + @patch('lms.djangoapps.mobile_api.course_info.views.certificate_downloadable_status') + def test_get_certificate(self, certificate_status_return, expected_output, mock_certificate_status): + """ + Test get_certificate utility from the BlocksInfoInCourseView. + + Parameters: + certificate_status_return: returned value of the mocked certificate_downloadable_status function. + expected_output: return_value of the get_certificate function with specified mock return_value. + """ + mock_certificate_status.return_value = certificate_status_return + self.request.user = self.user + + certificate_info = BlocksInfoInCourseView().get_certificate( + self.request, self.user, 'course-v1:Test+T101+2021_T1' + ) + self.assertEqual(certificate_info, expected_output) @patch('lms.djangoapps.mobile_api.course_info.views.certificate_downloadable_status') def test_additional_info_response(self, mock_certificate_downloadable_status): @@ -302,3 +373,52 @@ def test_additional_info_response(self, mock_certificate_downloadable_status): assert response.data['certificate'] == {'url': certificate_url} assert response.data['is_self_paced'] is False mock_certificate_downloadable_status.assert_called_once() + + def test_course_access_details(self): + response = self.verify_response(url=self.url) + + expected_course_access_details = { + 'has_unmet_prerequisites': False, + 'is_too_early': False, + 'is_staff': False, + 'audit_access_expires': None, + 'courseware_access': { + 'has_access': True, + 'error_code': None, + 'developer_message': None, + 'user_message': None, + 'additional_context_user_message': None, + 'user_fragment': None + } + } + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(response.data['course_access_details'], expected_course_access_details) + + def test_course_sharing_utm_parameters(self): + response = self.verify_response(url=self.url) + + expected_course_sharing_utm_parameters = { + 'facebook': 'utm_medium=social&utm_campaign=social-sharing-db&utm_source=facebook', + 'twitter': 'utm_medium=social&utm_campaign=social-sharing-db&utm_source=twitter' + } + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(response.data['course_sharing_utm_parameters'], expected_course_sharing_utm_parameters) + + def test_course_about_url(self): + response = self.verify_response(url=self.url) + + course_overview = CourseOverview.objects.get(id=self.course.course_id) + expected_course_about_link = get_link_for_about_page(course_overview) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['course_about'], expected_course_about_link) + + def test_course_modes(self): + response = self.verify_response(url=self.url) + + expected_course_modes = [{'slug': 'audit', 'sku': None, 'android_sku': None, 'ios_sku': None, 'min_price': 0}] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertListEqual(response.data['course_modes'], expected_course_modes) From fb0f16a1336b6d3a9ec7b4f76afa70cd34677ec4 Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul <77053848+muhammadadeeltajamul@users.noreply.github.com> Date: Mon, 11 Dec 2023 15:06:37 +0500 Subject: [PATCH 21/24] fix: discussion tab should be None if discussion tab is disabled (#33861) --- .../mobile_api/users/serializers.py | 3 +- lms/djangoapps/mobile_api/users/tests.py | 49 +++++++++++++++++++ .../content/course_overviews/models.py | 9 ++-- 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 446bfc3dc21..77c4b8c668c 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -23,7 +23,6 @@ def to_representation(self, course_overview): # lint-amnesty, pylint: disable=a request = self.context.get('request') api_version = self.context.get('api_version') enrollment = CourseEnrollment.get_enrollment(user=self.context.get('request').user, course_key=course_id) - return { # identifiers 'id': course_id, @@ -74,7 +73,7 @@ def to_representation(self, course_overview): # lint-amnesty, pylint: disable=a 'discussion_course', kwargs={'course_id': course_id}, request=request, - ) if course_overview.is_discussion_tab_enabled() else None, + ) if course_overview.is_discussion_tab_enabled(request.user) else None, # This is an old API that was removed as part of DEPR-4. We keep the # field present in case API parsers expect it, but this API is now diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index f3ec1d055dc..7cdd0e76f4e 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -36,6 +36,7 @@ ) from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2, API_V3 from openedx.core.lib.courses import course_image_url +from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration from openedx.features.course_duration_limits.models import CourseDurationLimitConfig from openedx.features.course_experience.tests.views.helpers import add_course_mode from xmodule.course_block import DEFAULT_START_DATE # lint-amnesty, pylint: disable=wrong-import-order @@ -713,3 +714,51 @@ def test_with_display_overrides(self, api_version): assert serialized['course']['number'] == self.course.display_coursenumber assert serialized['course']['org'] == self.course.display_organization self._expiration_in_response(serialized, api_version) + + +@ddt.ddt +class TestDiscussionCourseEnrollmentSerializer(UrlResetMixin, MobileAPITestCase, MilestonesTestCaseMixin): + """ + Tests discussion data in course enrollment serializer + """ + + def setUp(self): + """ + Setup data for test + """ + with patch.dict('django.conf.settings.FEATURES', {'ENABLE_DISCUSSION_SERVICE': True}): + super().setUp() + self.login_and_enroll() + self.request = RequestFactory().get('/') + self.request.user = self.user + + def get_serialized_data(self, api_version): + """ + Return data from CourseEnrollmentSerializer + """ + if api_version == API_V05: + serializer = CourseEnrollmentSerializerv05 + else: + serializer = CourseEnrollmentSerializer + + return serializer( + CourseEnrollment.enrollments_for_user(self.user)[0], + context={'request': self.request, 'api_version': api_version}, + ).data + + @ddt.data(True, False) + def test_discussion_tab_url(self, discussion_tab_enabled): + """ + Tests discussion tab url is None if tab is disabled + """ + config, _ = DiscussionsConfiguration.objects.get_or_create(context_key=self.course.id) + config.enabled = discussion_tab_enabled + config.save() + with patch.dict('django.conf.settings.FEATURES', {'ENABLE_DISCUSSION_SERVICE': True}): + serialized = self.get_serialized_data(API_V2) + discussion_url = serialized["course"]["discussion_url"] + if discussion_tab_enabled: + assert discussion_url is not None + assert isinstance(discussion_url, str) + else: + assert discussion_url is None diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index 149159826cc..8dffb14fa86 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -23,7 +23,6 @@ from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField from simple_history.models import HistoricalRecords -from lms.djangoapps.discussion import django_comment_client from openedx.core.djangoapps.catalog.models import CatalogIntegration from openedx.core.djangoapps.lang_pref.api import get_closest_released_language from openedx.core.djangoapps.models.course_details import CourseDetails @@ -702,15 +701,17 @@ def get_all_course_keys(cls): """ return CourseOverview.objects.values_list('id', flat=True) - def is_discussion_tab_enabled(self): + def is_discussion_tab_enabled(self, user=None): """ Returns True if course has discussion tab and is enabled """ + # Importing here to avoid circular import + from lms.djangoapps.discussion.plugins import DiscussionTab tabs = self.tab_set.all() # creates circular import; hence explicitly referenced is_discussion_enabled for tab in tabs: - if tab.tab_id == "discussion" and django_comment_client.utils.is_discussion_enabled(self.id): - return True + if tab.tab_id == "discussion": + return DiscussionTab.is_enabled(self, user) return False @property From 79756e87dfb3bc37c9154b2068e523f71c8345de Mon Sep 17 00:00:00 2001 From: jawad khan Date: Thu, 9 May 2024 12:19:50 +0500 Subject: [PATCH 22/24] feat: Added upgrade deadline in blocks api (#34750) --- lms/djangoapps/mobile_api/course_info/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/mobile_api/course_info/serializers.py b/lms/djangoapps/mobile_api/course_info/serializers.py index b2bb0ce2470..d7a9471088a 100644 --- a/lms/djangoapps/mobile_api/course_info/serializers.py +++ b/lms/djangoapps/mobile_api/course_info/serializers.py @@ -82,7 +82,7 @@ class MobileCourseEnrollmentSerializer(serializers.ModelSerializer): """ class Meta: - fields = ('created', 'mode', 'is_active') + fields = ('created', 'mode', 'is_active', 'upgrade_deadline') model = CourseEnrollment lookup_field = 'username' From 37b7549efaa2ed858ec3d14bd7979f41f07e5511 Mon Sep 17 00:00:00 2001 From: jawad khan Date: Mon, 19 Feb 2024 20:49:33 +0500 Subject: [PATCH 23/24] feat: Add course price in mobile enrollment api (#34255) * feat: Add course price in mobile enrollment api --- lms/djangoapps/mobile_api/users/serializers.py | 1 + lms/djangoapps/mobile_api/users/tests.py | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 77c4b8c668c..d7005e5f68e 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -179,3 +179,4 @@ class ModeSerializer(serializers.Serializer): # pylint: disable=abstract-method sku = serializers.CharField() android_sku = serializers.CharField() ios_sku = serializers.CharField() + min_price = serializers.IntegerField() diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 7cdd0e76f4e..65b1fba65ce 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -319,6 +319,12 @@ def _assert_enrollment_results(self, api_version, courses, num_courses_returned, assert 'audit_access_expires' not in courses[0] else: assert 'audit_access_expires' in courses[0] + + for course_mode in courses[0]['course_modes']: + assert 'android_sku' in course_mode + assert 'ios_sku' in course_mode + assert 'min_price' in course_mode + if gating_enabled: assert courses[0].get('audit_access_expires') is not None From 83ead1337bb3b0536c06573ae5118f2f3c50cd9f Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Tue, 4 Jun 2024 14:57:29 +0300 Subject: [PATCH 24/24] temp: fix TestDiscussionCourseEnrollmentSerializer.test_discussion_tab_url failed tests for Palm Tests will fail again after Palm upgrade --- lms/djangoapps/mobile_api/users/tests.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 65b1fba65ce..94ae9b64a1f 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -4,6 +4,7 @@ import datetime +import unittest from unittest.mock import patch from urllib.parse import parse_qs @@ -36,6 +37,7 @@ ) from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2, API_V3 from openedx.core.lib.courses import course_image_url +from openedx.core.release import RELEASE_LINE from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration from openedx.features.course_duration_limits.models import CourseDurationLimitConfig from openedx.features.course_experience.tests.views.helpers import add_course_mode @@ -752,6 +754,10 @@ def get_serialized_data(self, api_version): context={'request': self.request, 'api_version': api_version}, ).data + @unittest.skipIf( + condition=RELEASE_LINE == 'palm', + reason='Temporarily disable in NELC Palm because of Redwood mobile api cherry-picks', + ) @ddt.data(True, False) def test_discussion_tab_url(self, discussion_tab_enabled): """