diff --git a/lms/djangoapps/ccx/api/v0/tests/test_views.py b/lms/djangoapps/ccx/api/v0/tests/test_views.py index 0543b172d1..b9b369917e 100644 --- a/lms/djangoapps/ccx/api/v0/tests/test_views.py +++ b/lms/djangoapps/ccx/api/v0/tests/test_views.py @@ -30,7 +30,6 @@ from lms.djangoapps.ccx.models import CcxFieldOverride, CustomCourseForEdX from lms.djangoapps.ccx.overrides import override_field_for_ccx from lms.djangoapps.ccx.tests.utils import CcxTestCase from lms.djangoapps.ccx.utils import ccx_course as ccx_course_cm -from lms.djangoapps.ccx.utils import get_course_chapters from lms.djangoapps.instructor.access import allow_access, list_with_level from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params from student.models import CourseEnrollment @@ -78,7 +77,7 @@ class CcxRestApiTest(CcxTestCase, APITestCase): self.course.enable_ccx = True self.mstore.update_item(self.course, self.coach.id) # making the master course chapters easily available - self.master_course_chapters = get_course_chapters(self.master_course_key) + self.master_course_chapters = courses.get_course_chapters(self.master_course_key) def get_auth_token(self, app_grant, app_client): """ diff --git a/lms/djangoapps/ccx/api/v0/views.py b/lms/djangoapps/ccx/api/v0/views.py index e543f4c75f..9177282f89 100644 --- a/lms/djangoapps/ccx/api/v0/views.py +++ b/lms/djangoapps/ccx/api/v0/views.py @@ -23,7 +23,6 @@ from lms.djangoapps.ccx.overrides import override_field_for_ccx from lms.djangoapps.ccx.utils import ( add_master_course_staff_to_ccx, assign_staff_role_to_ccx, - get_course_chapters, is_email ) from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params @@ -187,9 +186,7 @@ def valid_course_modules(course_module_list, master_course_key): Returns: bool: whether or not all the course module strings belong to the master course """ - course_chapters = get_course_chapters(master_course_key) - if course_chapters is None: - return False + course_chapters = courses.get_course_chapters(master_course_key) return set(course_module_list).intersection(set(course_chapters)) == set(course_module_list) diff --git a/lms/djangoapps/ccx/tests/test_utils.py b/lms/djangoapps/ccx/tests/test_utils.py index 2eef54c79d..13e7b39bed 100644 --- a/lms/djangoapps/ccx/tests/test_utils.py +++ b/lms/djangoapps/ccx/tests/test_utils.py @@ -55,50 +55,6 @@ class TestGetCCXFromCCXLocator(ModuleStoreTestCase): self.assertEqual(result, ccx) -@attr(shard=1) -class TestGetCourseChapters(CcxTestCase): - """ - Tests for the `get_course_chapters` util function - """ - ENABLED_SIGNALS = ['course_published'] - - def setUp(self): - """ - Set up tests - """ - super(TestGetCourseChapters, self).setUp() - self.course_key = self.course.location.course_key - - def test_get_structure_non_existing_key(self): - """ - Test to get the course structure - """ - self.assertEqual(utils.get_course_chapters(None), None) - # build a fake key - fake_course_key = CourseKey.from_string('course-v1:FakeOrg+CN1+CR-FALLNEVER1') - self.assertEqual(utils.get_course_chapters(fake_course_key), None) - - @mock.patch('openedx.core.djangoapps.content.course_structures.models.CourseStructure.structure', - new_callable=mock.PropertyMock) - def test_wrong_course_structure(self, mocked_attr): - """ - Test the case where the course has an unexpected structure. - """ - mocked_attr.return_value = {'foo': 'bar'} - self.assertEqual(utils.get_course_chapters(self.course_key), []) - - def test_get_chapters(self): - """ - Happy path - """ - course_chapters = utils.get_course_chapters(self.course_key) - self.assertEqual(len(course_chapters), 2) - self.assertEqual( - sorted(course_chapters), - sorted([unicode(child) for child in self.course.children]) - ) - - class TestStaffOnCCX(CcxTestCase): """ Tests for staff on ccx courses. diff --git a/lms/djangoapps/ccx/utils.py b/lms/djangoapps/ccx/utils.py index f940ce7ebd..f45336ac34 100644 --- a/lms/djangoapps/ccx/utils.py +++ b/lms/djangoapps/ccx/utils.py @@ -321,32 +321,6 @@ def is_email(identifier): return True -def get_course_chapters(course_key): - """ - Extracts the chapters from a course structure. - If the course does not exist returns None. - If the structure does not contain 1st level children, - it returns an empty list. - - Args: - course_key (CourseLocator): the course key - Returns: - list (string): a list of string representing the chapters modules - of the course - """ - if course_key is None: - return - try: - course_obj = CourseStructure.objects.get(course_id=course_key) - except CourseStructure.DoesNotExist: - return - course_struct = course_obj.structure - try: - return course_struct['blocks'][course_struct['root']].get('children', []) - except KeyError: - return [] - - def add_master_course_staff_to_ccx(master_course, ccx_key, display_name, send_email=True): """ Add staff and instructor roles on ccx to all the staff and instructors members of master course. diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index f5232047d4..95ec1d6484 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -593,3 +593,19 @@ def get_current_child(xmodule, min_depth=None, requested_child=None): child = _get_default_child_module(children) return child + + +def get_course_chapters(course_key): + """ + Extracts the chapter block keys from a course structure. + + Arguments: + course_key (CourseLocator): The course key + Returns: + list (string): The list of string representations of the chapter block keys in the course. + """ + try: + chapters = modulestore().get_items(course_key, qualifiers={'category': 'chapter'}) + except Exception: # pylint: disable=broad-except + return [] + return [unicode(chapter.location) for chapter in chapters] diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index de5b7c5d66..bc0b23e33a 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/lms/djangoapps/courseware/tests/test_courses.py @@ -13,6 +13,7 @@ from django.core.urlresolvers import reverse from django.test.client import RequestFactory from django.test.utils import override_settings from nose.plugins.attrib import attr +from opaque_keys.edx.keys import CourseKey from six import text_type from courseware.courses import ( @@ -21,11 +22,12 @@ from courseware.courses import ( get_cms_course_link, get_course_about_section, get_course_by_id, + get_course_chapters, get_course_info_section, get_course_overview_with_access, get_course_with_access, get_courses, - get_current_child + get_current_child, ) from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor @@ -424,3 +426,30 @@ class CourseInstantiationTests(ModuleStoreTestCase): for section in chapter.get_children(): for item in section.get_children(): self.assertTrue(item.graded) + + +@attr(shard=1) +class TestGetCourseChapters(ModuleStoreTestCase): + """ + Tests for the `get_course_chapters` function. + """ + + def test_get_non_existant_course(self): + """ + Test non-existant course returns empty list. + """ + self.assertEqual(get_course_chapters(None), []) + # build a fake key + fake_course_key = CourseKey.from_string('course-v1:FakeOrg+CN1+CR-FALLNEVER1') + self.assertEqual(get_course_chapters(fake_course_key), []) + + def test_get_chapters(self): + """ + Test get_course_chapters returns expected result. + """ + course = CourseFactory() + ItemFactory(parent=course, category='chapter') + self.assertEqual( + get_course_chapters(course.location.course_key), + [unicode(child) for child in course.children] + )