From 9616ac99f8648ca5c89c06c7d49fc26ad81b1106 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Sun, 24 Jul 2016 21:20:16 -0400 Subject: [PATCH] Expose course_version from Split Modulestore; Support subtree_edited_on and course_version fields in BlockStructures TNL-5010 --- .../xmodule/xmodule/modulestore/mongo/base.py | 9 ++++++++- .../xmodule/modulestore/split_mongo/split.py | 10 +++++++++- .../tests/test_mixed_modulestore.py | 19 +++++++++++++++++++ .../transformers/tests/helpers.py | 2 +- .../grades/tests/test_transformer.py | 15 +++++++++++++++ lms/djangoapps/grades/transformer.py | 4 ++-- 6 files changed, 54 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 7000e1e777..6b97d7e47d 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -953,7 +953,14 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo system.module_data.update(data_cache) system.cached_metadata.update(cached_metadata) - return system.load_item(location, for_parent=for_parent) + item = system.load_item(location, for_parent=for_parent) + + # TODO Once PLAT-1055 is implemented, we can remove the following line + # of code. Until then, set the course_version field on the block to be + # consistent with the Split modulestore. Since Mongo modulestore doesn't + # maintain course versions set it to None. + item.course_version = None + return item def _load_items(self, course_key, items, depth=0, using_descriptor_system=None, for_parent=None): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 1a9a7ccb2c..2718fc793d 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -779,7 +779,15 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): self._add_cache(course_entry.structure['_id'], runtime) self.cache_items(runtime, block_keys, course_entry.course_key, depth, lazy) - return [runtime.load_item(block_key, course_entry, **kwargs) for block_key in block_keys] + blocks = [runtime.load_item(block_key, course_entry, **kwargs) for block_key in block_keys] + + # TODO Once PLAT-1055 is implemented, we can expose the course version + # information within the key identifier of the block. Until then, set + # the course_version as a field on each returned block so higher layers + # can use it when needed. + for block in blocks: + block.course_version = course_entry.course_key.version_guid + return blocks def _get_cache(self, course_version_guid): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 826c89b0c2..8b08d009a8 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -425,6 +425,25 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): revision=ModuleStoreEnum.RevisionOption.draft_preferred ) + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_course_version_on_block(self, default_ms): + self.initdb(default_ms) + self._create_block_hierarchy() + + course = self.store.get_course(self.course.id) + course_version = course.course_version + + if default_ms == ModuleStoreEnum.Type.split: + self.assertIsNotNone(course_version) + else: + self.assertIsNone(course_version) + + blocks = self.store.get_items(self.course.id, qualifiers={'category': 'problem'}) + blocks.append(self.store.get_item(self.problem_x1a_1)) + self.assertEquals(len(blocks), 7) + for block in blocks: + self.assertEquals(block.course_version, course_version) + @ddt.data((ModuleStoreEnum.Type.split, 2, False), (ModuleStoreEnum.Type.mongo, 3, True)) @ddt.unpack def test_get_items_include_orphans(self, default_ms, expected_items_in_tree, orphan_in_items): diff --git a/lms/djangoapps/course_blocks/transformers/tests/helpers.py b/lms/djangoapps/course_blocks/transformers/tests/helpers.py index 2ce251f6d1..fd304aad5b 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/helpers.py +++ b/lms/djangoapps/course_blocks/transformers/tests/helpers.py @@ -73,12 +73,12 @@ class CourseStructureTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase) if block_type != 'course': kwargs['category'] = block_type + kwargs['publish_item'] = True, if parent: kwargs['parent'] = parent xblock = factory.create( display_name=self.create_block_id(block_type, block_ref), - publish_item=True, **kwargs ) block_map[block_ref] = xblock diff --git a/lms/djangoapps/grades/tests/test_transformer.py b/lms/djangoapps/grades/tests/test_transformer.py index 5575c1b90d..1fc8f7d2a7 100644 --- a/lms/djangoapps/grades/tests/test_transformer.py +++ b/lms/djangoapps/grades/tests/test_transformer.py @@ -7,6 +7,7 @@ import pytz import random from student.tests.factories import UserFactory +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import check_mongo_calls @@ -50,6 +51,9 @@ class GradesTransformerTestCase(CourseStructureTestCase): block_structure.get_xblock_field(usage_key, field), msg=u'in field {},'.format(repr(field)), ) + self.assertIsNotNone( + block_structure.get_xblock_field(usage_key, u'subtree_edited_on'), + ) def assert_collected_transformer_block_fields(self, block_structure, usage_key, transformer_class, **expectations): """ @@ -196,6 +200,17 @@ class GradesTransformerTestCase(CourseStructureTestCase): max_score=2, ) + def test_course_version_not_collected_in_old_mongo(self): + blocks = self.build_course_with_problems() + block_structure = get_course_blocks(self.student, blocks[u'course'].location, self.transformers) + self.assertIsNone(block_structure.get_xblock_field(blocks[u'course'].location, u'course_version')) + + def test_course_version_collected_in_split(self): + with self.store.default_store(ModuleStoreEnum.Type.split): + blocks = self.build_course_with_problems() + block_structure = get_course_blocks(self.student, blocks[u'course'].location, self.transformers) + self.assertIsNotNone(block_structure.get_xblock_field(blocks[u'course'].location, u'course_version')) + class MultiProblemModulestoreAccessTestCase(CourseStructureTestCase, SharedModuleStoreTestCase): """ diff --git a/lms/djangoapps/grades/transformer.py b/lms/djangoapps/grades/transformer.py index d7ec17485e..82c7f92c90 100644 --- a/lms/djangoapps/grades/transformer.py +++ b/lms/djangoapps/grades/transformer.py @@ -30,8 +30,8 @@ class GradesTransformer(BlockStructureTransformer): max_score: (numeric) """ - VERSION = 1 - FIELDS_TO_COLLECT = [u'due', u'format', u'graded', u'has_score', u'weight'] + VERSION = 2 + FIELDS_TO_COLLECT = [u'due', u'format', u'graded', u'has_score', u'weight', u'course_version', u'subtree_edited_on'] @classmethod def name(cls):