diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index a23ae9770b..4d2b3ae714 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/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): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index e104e1b0f2..adceb85aa6 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 161899f968..5cf793325b 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/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: diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 2965b2e5ca..be8e6c2f12 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/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') diff --git a/lms/djangoapps/ccx/modulestore.py b/lms/djangoapps/ccx/modulestore.py index e4ac651cf0..6d99f6e797 100644 --- a/lms/djangoapps/ccx/modulestore.py +++ b/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 diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index ecf7419311..be911f91a9 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/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)