Merge pull request #29 from nelc/mobile_api

feat: cherry pick mobile apis

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')
 

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.
     """
@@ -101,6 +101,13 @@ def _filter_by_search(course_queryset, search_term):
 
     search_courses_ids = {course['data']['id'] for course in search_courses['results']}
 
+    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(
         (
             course for course in course_queryset
@@ -116,7 +123,9 @@ def list_courses(request,
                  filter_=None,
                  search_term=None,
                  permissions=None,
-                 active_only=False):
+                 active_only=False,
+                 course_keys=None,
+                 mobile_search=False):
     """
     Yield all available courses.
 
@@ -146,13 +155,21 @@ 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
+        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.
     """
     user = get_effective_user(request.user, username)
-    course_qs = get_courses(user, org=org, filter_=filter_, permissions=permissions, active_only=active_only)
-    course_qs = _filter_by_search(course_qs, search_term)
+    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, mobile_search)
     return course_qs
 
 

lms/djangoapps/course_api/forms.py

@@ -64,6 +64,8 @@ class CourseListGetForm(UsernameValidatorMixin, Form):
     mobile = ExtendedNullBooleanField(required=False)
     active_only = ExtendedNullBooleanField(required=False)
     permissions = MultiValueField(required=False)
+    course_keys = MultiValueField(required=False)
+    mobile_search = ExtendedNullBooleanField(required=False)
 
     def clean(self):
         """
@@ -80,6 +82,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):
     """

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,23 @@ 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 '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_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
 
 

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):
     """

lms/djangoapps/course_api/tests/test_forms.py

@@ -71,6 +71,8 @@ def set_up_data(self, user):
             'filter_': None,
             'permissions': set(),
             'active_only': None,
+            'course_keys': set(),
+            'mobile_search': None,
         }
 
     def test_basic(self):
@@ -100,6 +102,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

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
 

lms/djangoapps/course_api/tests/test_views.py

@@ -449,6 +449,46 @@ 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)]  # pylint: disable=expression-not-assigned
+        response = self.verify_response(params={"search_term": "new"})
+        self.assertEqual(response.status_code, 200)
+        # 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):
+        """
+        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)]  # 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):
     """

lms/djangoapps/course_api/views.py

@@ -286,6 +286,14 @@ 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
+
+        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
@@ -343,7 +351,9 @@ 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'],
+            mobile_search=form.cleaned_data.get('mobile_search', False),
         )
 
 

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(