Fix sequence navigation issues for CCX courses

MA-2258

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -38,6 +38,11 @@ new_contract('XBlock', XBlock)
 LIBRARY_ROOT = 'library.xml'
 COURSE_ROOT = 'course.xml'
 
+# List of names of computed fields on xmodules that are of type usage keys.
+# This list can be used to determine which fields need to be stripped of
+# extraneous usage key data when entering/exiting modulestores.
+XMODULE_FIELDS_WITH_USAGE_KEYS = ['location', 'parent']
+
 
 class ModuleStoreEnum(object):
     """

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -17,8 +17,7 @@ from opaque_keys.edx.locator import LibraryLocator
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from xmodule.assetstore import AssetMetadata
 
-from . import ModuleStoreWriteBase
-from . import ModuleStoreEnum
+from . import ModuleStoreWriteBase, ModuleStoreEnum, XMODULE_FIELDS_WITH_USAGE_KEYS
 from .exceptions import ItemNotFoundError, DuplicateCourseError
 from .draft_and_published import ModuleStoreDraftAndPublished
 from .split_migrator import SplitMigrator
@@ -67,8 +66,9 @@ def strip_key(func):
                 retval = retval.version_agnostic()
             if rem_branch and hasattr(retval, 'for_branch'):
                 retval = retval.for_branch(None)
-            if hasattr(retval, 'location'):
-                retval.location = strip_key_func(retval.location)
+            for field_name in XMODULE_FIELDS_WITH_USAGE_KEYS:
+                if hasattr(retval, field_name):
+                    setattr(retval, field_name, strip_key_func(getattr(retval, field_name)))
             return retval
 
         # function for stripping both, collection of, and individual, values

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -127,6 +127,8 @@ class MongoKeyValueStore(InheritanceKeyValueStore):
     def set(self, key, value):
         if key.scope == Scope.children:
             self._children = value
+        elif key.scope == Scope.parent:
+            self._parent = value
         elif key.scope == Scope.settings:
             self._metadata[key.field_name] = value
         elif key.scope == Scope.content:

common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py

@@ -767,15 +767,15 @@ class TestMongoKeyValueStore(unittest.TestCase):
     def test_write(self):
         yield (self._check_write, KeyValueStore.Key(Scope.content, None, None, 'foo'), 'new_data')
         yield (self._check_write, KeyValueStore.Key(Scope.children, None, None, 'children'), [])
+        yield (self._check_write, KeyValueStore.Key(Scope.children, None, None, 'parent'), None)
         yield (self._check_write, KeyValueStore.Key(Scope.settings, None, None, 'meta'), 'new_settings')
-        # write Scope.parent raises InvalidScope, which is covered in test_write_invalid_scope
 
     def test_write_non_dict_data(self):
         self.kvs = MongoKeyValueStore('xml_data', self.parent, self.children, self.metadata)
         self._check_write(KeyValueStore.Key(Scope.content, None, None, 'data'), 'new_data')
 
     def test_write_invalid_scope(self):
-        for scope in (Scope.preferences, Scope.user_info, Scope.user_state, Scope.parent):
+        for scope in (Scope.preferences, Scope.user_info, Scope.user_state):
             with assert_raises(InvalidScopeError):
                 self.kvs.set(KeyValueStore.Key(scope, None, None, 'foo'), 'new_value')
 

lms/djangoapps/ccx/modulestore.py

@@ -13,6 +13,7 @@ from contextlib import contextmanager
 from functools import partial
 from ccx_keys.locator import CCXLocator, CCXBlockUsageLocator
 from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator
+from xmodule.modulestore import XMODULE_FIELDS_WITH_USAGE_KEYS
 
 
 def strip_ccx(val):
@@ -28,8 +29,11 @@ def strip_ccx(val):
     elif isinstance(retval, CCXBlockUsageLocator):
         ccx_id = retval.course_key.ccx
         retval = retval.to_block_locator()
-    elif hasattr(retval, 'location'):
-        retval.location, ccx_id = strip_ccx(retval.location)
+    else:
+        for field_name in XMODULE_FIELDS_WITH_USAGE_KEYS:
+            if hasattr(retval, field_name):
+                stripped_field_value, ccx_id = strip_ccx(getattr(retval, field_name))
+                setattr(retval, field_name, stripped_field_value)
     return retval, ccx_id
 
 
@@ -43,8 +47,9 @@ def restore_ccx(val, ccx_id):
     elif isinstance(val, BlockUsageLocator):
         ccx_key = restore_ccx(val.course_key, ccx_id)
         val = CCXBlockUsageLocator(ccx_key, val.block_type, val.block_id)
-    if hasattr(val, 'location'):
-        val.location = restore_ccx(val.location, ccx_id)
+    for field_name in XMODULE_FIELDS_WITH_USAGE_KEYS:
+        if hasattr(val, field_name):
+            setattr(val, field_name, restore_ccx(getattr(val, field_name), ccx_id))
     if hasattr(val, 'children'):
         val.children = restore_ccx_collection(val.children, ccx_id)
     return val

lms/djangoapps/ccx/tests/test_views.py

@@ -53,6 +53,7 @@ from xmodule.modulestore.tests.django_utils import (
 from xmodule.modulestore.tests.factories import (
     CourseFactory,
     ItemFactory,
+    SampleCourseFactory,
 )
 from ccx_keys.locator import CCXLocator
 
@@ -1049,9 +1050,9 @@ class CCXCoachTabTestCase(SharedModuleStoreTestCase):
             )
 
 
-class TestStudentDashboardWithCCX(ModuleStoreTestCase):
+class TestStudentViewsWithCCX(ModuleStoreTestCase):
     """
-    Test to ensure that the student dashboard works for users enrolled in CCX
+    Test to ensure that the student dashboard and courseware works for users enrolled in CCX
     courses.
     """
 
@@ -1059,13 +1060,13 @@ class TestStudentDashboardWithCCX(ModuleStoreTestCase):
         """
         Set up courses and enrollments.
         """
-        super(TestStudentDashboardWithCCX, self).setUp()
+        super(TestStudentViewsWithCCX, self).setUp()
 
         # Create a Draft Mongo and a Split Mongo course and enroll a student user in them.
         self.student_password = "foobar"
         self.student = UserFactory.create(username="test", password=self.student_password, is_staff=False)
-        self.draft_course = CourseFactory.create(default_store=ModuleStoreEnum.Type.mongo)
-        self.split_course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split)
+        self.draft_course = SampleCourseFactory.create(default_store=ModuleStoreEnum.Type.mongo)
+        self.split_course = SampleCourseFactory.create(default_store=ModuleStoreEnum.Type.split)
         CourseEnrollment.enroll(self.student, self.draft_course.id)
         CourseEnrollment.enroll(self.student, self.split_course.id)
 
@@ -1078,11 +1079,16 @@ class TestStudentDashboardWithCCX(ModuleStoreTestCase):
         self.ccx = CcxFactory(course_id=self.split_course.id, coach=self.coach)
         last_week = datetime.datetime.now(UTC()) - datetime.timedelta(days=7)
         override_field_for_ccx(self.ccx, self.split_course, 'start', last_week)  # Required by self.ccx.has_started().
-        course_key = CCXLocator.from_course_locator(self.split_course.id, self.ccx.id)
-        CourseEnrollment.enroll(self.student, course_key)
+        self.ccx_course_key = CCXLocator.from_course_locator(self.split_course.id, self.ccx.id)
+        CourseEnrollment.enroll(self.student, self.ccx_course_key)
 
     def test_load_student_dashboard(self):
         self.client.login(username=self.student.username, password=self.student_password)
         response = self.client.get(reverse('dashboard'))
         self.assertEqual(response.status_code, 200)
         self.assertTrue(re.search('Test CCX', response.content))
+
+    def test_load_courseware(self):
+        self.client.login(username=self.student.username, password=self.student_password)
+        response = self.client.get(reverse('courseware', kwargs={'course_id': unicode(self.ccx_course_key)}))
+        self.assertEqual(response.status_code, 200)