Merge pull request #7089 from edx/clintonb/cs-api-fix

Fixed Course List Endpoint

lms/djangoapps/course_structure_api/v0/tests.py

@@ -7,12 +7,17 @@ from datetime import datetime
 
 from django.core.urlresolvers import reverse
 from django.test.utils import override_settings
+from mock import patch, Mock
 from oauth2_provider.tests.factories import AccessTokenFactory, ClientFactory
-from openedx.core.djangoapps.content.course_structures.models import CourseStructure, update_course_structure
+from opaque_keys.edx.locator import CourseLocator
+from xmodule.error_module import ErrorDescriptor
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.modulestore.xml import CourseLocationManager
+from xmodule.tests import get_test_system
 
 from courseware.tests.factories import GlobalStaffFactory, StaffFactory
+from openedx.core.djangoapps.content.course_structures.models import CourseStructure, update_course_structure
 
 
 TEST_SERVER_HOST = 'http://testserver'
@@ -260,6 +265,24 @@ class CourseListTests(CourseViewTestsMixin, ModuleStoreTestCase):
         self.assertEqual(response.status_code, 200)
         self.assertDictContainsSubset({'count': 0, u'results': []}, response.data)
 
+    def test_course_error(self):
+        """
+        Ensure the view still returns results even if get_courses() returns an ErrorDescriptor. The ErrorDescriptor
+        should be filtered out.
+        """
+
+        error_descriptor = ErrorDescriptor.from_xml(
+            '<course></course>',
+            get_test_system(),
+            CourseLocationManager(CourseLocator(org='org', course='course', run='run')),
+            None
+        )
+
+        descriptors = [error_descriptor, self.empty_course, self.course]
+
+        with patch('xmodule.modulestore.mixed.MixedModuleStore.get_courses', Mock(return_value=descriptors)):
+            self.test_get()
+
 
 class CourseDetailTests(CourseDetailMixin, CourseViewTestsMixin, ModuleStoreTestCase):
     view = 'course_structure_api:v0:detail'

lms/djangoapps/course_structure_api/v0/views.py

@@ -12,12 +12,12 @@ from rest_framework.response import Response
 from xmodule.modulestore.django import modulestore
 from opaque_keys.edx.keys import CourseKey
 
+from course_structure_api.v0 import serializers
+from courseware import courses
 from courseware.access import has_access
 from openedx.core.djangoapps.content.course_structures import models
 from openedx.core.lib.api.permissions import IsAuthenticatedOrDebug
 from openedx.core.lib.api.serializers import PaginationSerializer
-from courseware import courses
-from course_structure_api.v0 import serializers
 from student.roles import CourseInstructorRole, CourseStaffRole
 
 
@@ -115,22 +115,24 @@ class CourseList(CourseViewMixin, ListAPIView):
     def get_queryset(self):
         course_ids = self.request.QUERY_PARAMS.get('course_id', None)
 
-        course_descriptors = []
+        results = []
         if course_ids:
             course_ids = course_ids.split(',')
             for course_id in course_ids:
                 course_key = CourseKey.from_string(course_id)
                 course_descriptor = courses.get_course(course_key)
-                course_descriptors.append(course_descriptor)
+                results.append(course_descriptor)
         else:
-            course_descriptors = modulestore().get_courses()
+            results = modulestore().get_courses()
 
-        results = [course for course in course_descriptors if self.user_can_access_course(self.request.user, course)]
+        # Ensure only course descriptors are returned.
+        results = (course for course in results if course.scope_ids.block_type == 'course')
 
-        # Sort the results in a predictable manner.
-        results.sort(key=attrgetter('id'))
+        # Ensure only courses accessible by the user are returned.
+        results = (course for course in results if self.user_can_access_course(self.request.user, course))
 
-        return results
+        # Sort the results in a predictable manner.
+        return sorted(results, key=attrgetter('id'))
 
 
 class CourseDetail(CourseViewMixin, RetrieveAPIView):