diff --git a/lms/djangoapps/course_structure_api/v0/tests.py b/lms/djangoapps/course_structure_api/v0/tests.py index 88f4e02ecd..663f2f4bf5 100644 --- a/lms/djangoapps/course_structure_api/v0/tests.py +++ b/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( + '', + 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' diff --git a/lms/djangoapps/course_structure_api/v0/views.py b/lms/djangoapps/course_structure_api/v0/views.py index 73b7083d27..e7bc54537f 100644 --- a/lms/djangoapps/course_structure_api/v0/views.py +++ b/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') + + # 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)) # Sort the results in a predictable manner. - results.sort(key=attrgetter('id')) - - return results + return sorted(results, key=attrgetter('id')) class CourseDetail(CourseViewMixin, RetrieveAPIView):