From e39cc5dfa3006108745ef246f38645329d0aed6b Mon Sep 17 00:00:00 2001 From: Mat Peterson Date: Wed, 2 Jul 2014 17:23:40 +0000 Subject: [PATCH] Switch offering to course and run --- .../contentstore/tests/test_crud.py | 15 +- .../xmodule/xmodule/modulestore/__init__.py | 17 +- .../xmodule/modulestore/loc_mapper_store.py | 57 ++++--- .../lib/xmodule/xmodule/modulestore/mixed.py | 10 +- .../xmodule/xmodule/modulestore/mongo/base.py | 10 +- .../xmodule/modulestore/split_migrator.py | 5 +- .../split_mongo/caching_descriptor_system.py | 8 +- .../split_mongo/mongo_connection.py | 10 +- .../xmodule/modulestore/split_mongo/split.py | 53 ++++--- .../tests/test_mixed_modulestore.py | 10 +- .../xmodule/modulestore/tests/test_orphan.py | 4 +- .../xmodule/modulestore/tests/test_publish.py | 8 +- .../tests/test_split_modulestore.py | 149 ++++++++++-------- .../tests/test_split_w_old_mongo.py | 6 +- .../xmodule/modulestore/xml_importer.py | 2 +- 15 files changed, 206 insertions(+), 158 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_crud.py b/cms/djangoapps/contentstore/tests/test_crud.py index 8910b31fb5..cb1b8f0bfe 100644 --- a/cms/djangoapps/contentstore/tests/test_crud.py +++ b/cms/djangoapps/contentstore/tests/test_crud.py @@ -57,14 +57,15 @@ class TemplateTests(unittest.TestCase): def test_factories(self): test_course = persistent_factories.PersistentCourseFactory.create( - offering='tempcourse', org='testx', + course='course', run='2014', org='testx', display_name='fun test course', user_id='testbot' ) self.assertIsInstance(test_course, CourseDescriptor) self.assertEqual(test_course.display_name, 'fun test course') index_info = self.split_store.get_course_index_info(test_course.id) self.assertEqual(index_info['org'], 'testx') - self.assertEqual(index_info['offering'], 'tempcourse') + self.assertEqual(index_info['course'], 'course') + self.assertEqual(index_info['run'], '2014') test_chapter = persistent_factories.ItemFactory.create(display_name='chapter 1', parent_location=test_course.location) @@ -75,7 +76,7 @@ class TemplateTests(unittest.TestCase): with self.assertRaises(DuplicateCourseError): persistent_factories.PersistentCourseFactory.create( - offering='tempcourse', org='testx', + course='course', run='2014', org='testx', display_name='fun test course', user_id='testbot' ) @@ -84,7 +85,7 @@ class TemplateTests(unittest.TestCase): Test create_xblock to create non persisted xblocks """ test_course = persistent_factories.PersistentCourseFactory.create( - offering='tempcourse', org='testx', + course='course', run='2014', org='testx', display_name='fun test course', user_id='testbot' ) @@ -111,7 +112,7 @@ class TemplateTests(unittest.TestCase): try saving temporary xblocks """ test_course = persistent_factories.PersistentCourseFactory.create( - offering='tempcourse', org='testx', + course='course', run='2014', org='testx', display_name='fun test course', user_id='testbot' ) test_chapter = self.split_store.create_xblock( @@ -150,7 +151,7 @@ class TemplateTests(unittest.TestCase): def test_delete_course(self): test_course = persistent_factories.PersistentCourseFactory.create( - offering='history.doomed', org='edu.harvard', + course='history', run='doomed', org='edu.harvard', display_name='doomed test course', user_id='testbot') persistent_factories.ItemFactory.create(display_name='chapter 1', @@ -173,7 +174,7 @@ class TemplateTests(unittest.TestCase): Test get_block_generations """ test_course = persistent_factories.PersistentCourseFactory.create( - offering='history.hist101', org='edu.harvard', + course='history', run='hist101', org='edu.harvard', display_name='history test course', user_id='testbot' ) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 1d16cafa7a..bc8d2f32e6 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -317,7 +317,7 @@ class ModuleStoreWrite(ModuleStoreRead): :param force: fork the structure and don't update the course draftVersion if there's a version conflict (only applicable to version tracking and conflict detecting persistence stores) - :raises VersionConflictError: if org, offering, and version_guid given and the current + :raises VersionConflictError: if org, course, run, and version_guid given and the current version head != version_guid and force is not True. (only applicable to version tracking stores) """ pass @@ -336,19 +336,20 @@ class ModuleStoreWrite(ModuleStoreRead): :param force: fork the structure and don't update the course draftVersion if there's a version conflict (only applicable to version tracking and conflict detecting persistence stores) - :raises VersionConflictError: if org, offering, and version_guid given and the current + :raises VersionConflictError: if org, course, run, and version_guid given and the current version head != version_guid and force is not True. (only applicable to version tracking stores) """ pass @abstractmethod - def create_course(self, org, offering, user_id, fields=None, **kwargs): + def create_course(self, org, course, run, user_id, fields=None, **kwargs): """ Creates and returns the course. Args: org (str): the organization that owns the course - offering (str): the name of the course offering + course (str): the name of the course + run (str): the name of the run user_id: id of the user creating the course fields (dict): Fields to set on the course at initialization kwargs: Any optional arguments understood by a subset of modulestores to customize instantiation @@ -458,7 +459,9 @@ class ModuleStoreReadBase(ModuleStoreRead): return next( ( c.id for c in self.get_courses() - if c.id.org.lower() == course_id.org.lower() and c.id.offering.lower() == course_id.offering.lower() + if c.id.org.lower() == course_id.org.lower() and \ + c.id.course.lower() == course_id.course.lower() and \ + c.id.run.lower() == course_id.run.lower() ), None ) @@ -542,7 +545,7 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): :param force: fork the structure and don't update the course draftVersion if there's a version conflict (only applicable to version tracking and conflict detecting persistence stores) - :raises VersionConflictError: if org, offering, and version_guid given and the current + :raises VersionConflictError: if org, course, run, and version_guid given and the current version head != version_guid and force is not True. (only applicable to version tracking stores) """ raise NotImplementedError @@ -556,7 +559,7 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): :param force: fork the structure and don't update the course draftVersion if there's a version conflict (only applicable to version tracking and conflict detecting persistence stores) - :raises VersionConflictError: if org, offering, and version_guid given and the current + :raises VersionConflictError: if org, course, run, and version_guid given and the current version head != version_guid and force is not True. (only applicable to version tracking stores) """ raise NotImplementedError diff --git a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py index 42fcc7d763..012971e5f7 100644 --- a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py +++ b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py @@ -55,13 +55,13 @@ class LocMapperStore(object): self.cache = cache # location_map functions - def create_map_entry(self, course_key, org=None, offering=None, + def create_map_entry(self, course_key, org=None, course=None, run=None, draft_branch=ModuleStoreEnum.BranchName.draft, prod_branch=ModuleStoreEnum.BranchName.published, block_map=None): """ - Add a new entry to map this SlashSeparatedCourseKey to the new style CourseLocator.org & offering. If - org and offering are not provided, it defaults them based on course_key. + Add a new entry to map this SlashSeparatedCourseKey to the new style CourseLocator.org & course & run. If + org and course and run are not provided, it defaults them based on course_key. WARNING: Exactly 1 CourseLocator key should index a given SlashSeparatedCourseKey. We provide no mechanism to enforce this assertion. @@ -71,7 +71,8 @@ class LocMapperStore(object): :param course_key (SlashSeparatedCourseKey): a SlashSeparatedCourseKey :param org (string): the CourseLocator style org - :param offering (string): the CourseLocator offering + :param course (string): the CourseLocator course number + :param run (string): the CourseLocator run of this course :param draft_branch: the branch name to assign for drafts. This is hardcoded because old mongo had a fixed notion that there was 2 and only 2 versions for modules: draft and production. The old mongo did not, however, require that a draft version exist. The new one, however, does require a draft to @@ -86,15 +87,16 @@ class LocMapperStore(object): :class:`CourseLocator` representing the new id for the course Raises: - ValueError if one and only one of org and offering is provided. Provide either both or neither. + ValueError if one and only one of org and course and run is provided. Provide all of them or none of them. """ - if org is None and offering is None: - assert(isinstance(course_key, SlashSeparatedCourseKey)) + if org is None and course is None and run is None: + assert(isinstance(course_key, CourseKey)) org = course_key.org - offering = u"{0.course}.{0.run}".format(course_key) - elif org is None or offering is None: + course = course_key.course + run = course_key.run + elif org is None or course is None or run is None: raise ValueError( - u"Either supply both org and offering or neither. Not just one: {}, {}".format(org, offering) + u"Either supply org, course and run or none of them. Not just some of them: {}, {}, {}".format(org, course, run) ) # very like _interpret_location_id but using mongo subdoc lookup (more performant) @@ -103,14 +105,15 @@ class LocMapperStore(object): self.location_map.insert({ '_id': course_son, 'org': org, - 'offering': offering, + 'course': course, + 'run': run, 'draft_branch': draft_branch, 'prod_branch': prod_branch, 'block_map': block_map or {}, 'schema': self.SCHEMA_VERSION, }) - return CourseLocator(org, offering) + return CourseLocator(org, course, run) def translate_location(self, location, published=True, add_entry_if_missing=True, passed_block_id=None): @@ -177,7 +180,8 @@ class LocMapperStore(object): prod_course_locator = CourseLocator( org=entry['org'], - offering=entry['offering'], + course=entry['course'], + run=entry['run'], branch=entry['prod_branch'] ) published_usage = BlockUsageLocator( @@ -223,7 +227,7 @@ class LocMapperStore(object): if cached_value: return cached_value - # migrate any records which don't have the org and offering fields as + # migrate any records which don't have the org and course and run fields as # this won't be able to find what it wants. (only needs to be run once ever per db, # I'm not sure how to control that, but I'm putting some check here for once per launch) if not getattr(self, 'offering_migrated', False): @@ -235,7 +239,8 @@ class LocMapperStore(object): entry = self.location_map.find_one(bson.son.SON([ ('org', locator.org), - ('offering', locator.offering), + ('course', locator.course), + ('run', locator.run), ])) # look for one which maps to this block block_id @@ -257,11 +262,14 @@ class LocMapperStore(object): ) entry_org = "org" - entry_offering = "offering" + entry_course = "course" + entry_run = "run" published_locator = BlockUsageLocator( CourseLocator( - org=entry[entry_org], offering=entry[entry_offering], + org=entry[entry_org], + course=entry[entry_course], + run=entry[entry_run], branch=entry['prod_branch'] ), block_type=category, @@ -269,7 +277,7 @@ class LocMapperStore(object): ) draft_locator = BlockUsageLocator( CourseLocator( - org=entry[entry_org], offering=entry[entry_offering], + org=entry[entry_org], course=entry[entry_course], run=entry[entry_run], branch=entry['draft_branch'] ), block_type=category, @@ -303,10 +311,10 @@ class LocMapperStore(object): raise ItemNotFoundError(course_key) published_course_locator = CourseLocator( - org=entry['org'], offering=entry['offering'], branch=entry['prod_branch'] + org=entry['org'], course=entry['course'], run=entry['run'], branch=entry['prod_branch'] ) draft_course_locator = CourseLocator( - org=entry['org'], offering=entry['offering'], branch=entry['draft_branch'] + org=entry['org'], course=entry['course'], run=entry['run'], branch=entry['draft_branch'] ) self._cache_course_locator(course_key, published_course_locator, draft_course_locator) if published: @@ -441,7 +449,7 @@ class LocMapperStore(object): """ Return the string used to cache the course key """ - return u'{0.org}+{0.offering}'.format(course_key) + return u'{0.org}+{0.course}+{0.run}'.format(course_key) def _cache_course_locator(self, old_course_id, published_course_locator, draft_course_locator): """ @@ -534,7 +542,7 @@ class LocMapperStore(object): """ If entry had an '_id' without a run, remove the whole record. - Add fields: schema, org, offering + Add fields: schema, org, course, run Remove: course_id, lower_course_id :param entry: """ @@ -547,13 +555,14 @@ class LocMapperStore(object): self.location_map.remove({'_id': entry_id}) return None - # add schema, org, offering, etc, remove old fields + # add schema, org, course, run, etc, remove old fields entry['schema'] = 0 entry.pop('course_id', None) entry.pop('lower_course_id', None) old_course_id = SlashSeparatedCourseKey(entry['_id']['org'], entry['_id']['course'], entry['_id']['name']) entry['org'] = old_course_id.org - entry['offering'] = old_course_id.offering.replace('/', '+') + entry['course'] = old_course_id.course + entry['run'] = old_course_id.run return self._migrate_1(entry, True) # insert new migrations just before _migrate_top. _migrate_top sets the schema version and diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 9dc88fa2f6..9bf3690fa4 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -273,13 +273,14 @@ class MixedModuleStore(ModuleStoreWriteBase): errs.update(store.get_errored_courses()) return errs - def create_course(self, org, offering, user_id, fields=None, **kwargs): + def create_course(self, org, course, run, user_id, fields=None, **kwargs): """ Creates and returns the course. Args: org (str): the organization that owns the course - offering (str): the name of the course offering + course (str): the name of the course + run (str): the name of the run user_id: id of the user creating the course fields (dict): Fields to set on the course at initialization kwargs: Any optional arguments understood by a subset of modulestores to customize instantiation @@ -287,7 +288,10 @@ class MixedModuleStore(ModuleStoreWriteBase): Returns: a CourseDescriptor """ store = self._get_modulestore_for_courseid(None) - return store.create_course(org, offering, user_id, fields, **kwargs) + if not hasattr(store, 'create_course'): + raise NotImplementedError(u"Cannot create a course on store {}".format(store)) + + return store.create_course(org, course, run, user_id, fields, **kwargs) def clone_course(self, source_course_id, dest_course_id, user_id): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index b6313dd536..0a969dad3b 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -845,13 +845,18 @@ class MongoModuleStore(ModuleStoreWriteBase): modules = self._load_items(course_id, list(items)) return modules +<<<<<<< HEAD def create_course(self, org, offering, user_id, fields=None, **kwargs): +======= + def create_course(self, org, course, run, user_id=None, fields=None, **kwargs): +>>>>>>> Fix up the signature to use course and run instead of just offering """ Creates and returns the course. Args: org (str): the organization that owns the course - offering (str): the name of the course offering + course (str): the name of the course + run (str): the name of the run user_id: id of the user creating the course fields (dict): Fields to set on the course at initialization kwargs: Any optional arguments understood by a subset of modulestores to customize instantiation @@ -859,9 +864,8 @@ class MongoModuleStore(ModuleStoreWriteBase): Returns: a CourseDescriptor Raises: - InvalidLocationError: If a course with the same org and offering already exists + InvalidLocationError: If a course with the same org, course, and run already exists """ - course, _, run = offering.partition('/') course_id = SlashSeparatedCourseKey(org, course, run) # Check if a course with this org/course has been defined before (case-insensitive) diff --git a/common/lib/xmodule/xmodule/modulestore/split_migrator.py b/common/lib/xmodule/xmodule/modulestore/split_migrator.py index b24e3088ef..f58f6d96d2 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/split_migrator.py @@ -45,7 +45,10 @@ class SplitMigrator(object): original_course = self.draft_modulestore.get_course(course_key) new_course_root_locator = self.loc_mapper.translate_location(original_course.location) new_course = self.split_modulestore.create_course( - new_course_root_locator.org, new_course_root_locator.offering, user.id, + new_course_root_locator.org, + new_course_root_locator.course, + new_course_root_locator.run, + user.id, fields=self._get_json_fields_translate_references(original_course, course_key, True), root_block_id=new_course_root_locator.block_id, master_branch=new_course_root_locator.branch diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 7e92cd2675..b1d1b60396 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -68,7 +68,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): # deeper than initial descendant fetch or doesn't exist course_info = course_entry_override or self.course_entry course_key = CourseLocator( - course_info.get('org'), course_info.get('offering'), course_info.get('branch'), + course_info.get('org'), course_info.get('course'), course_info.get('run'), course_info.get('branch'), course_info['structure']['_id'] ) self.modulestore.cache_items(self, [block_id], course_key, lazy=self.lazy) @@ -97,7 +97,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem): # most recent retrieval is most likely the right one for next caller (see comment above fn) self.course_entry['branch'] = course_entry_override['branch'] self.course_entry['org'] = course_entry_override['org'] - self.course_entry['offering'] = course_entry_override['offering'] + self.course_entry['course'] = course_entry_override['course'] + self.course_entry['run'] = course_entry_override['run'] # most likely a lazy loader or the id directly definition = json_data.get('definition', {}) definition_id = self.modulestore.definition_locator(definition) @@ -110,7 +111,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem): CourseLocator( version_guid=course_entry_override['structure']['_id'], org=course_entry_override.get('org'), - offering=course_entry_override.get('offering'), + course=course_entry_override.get('course'), + run=course_entry_override.get('run'), branch=course_entry_override.get('branch'), ), block_type=json_data.get('category'), diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py index 88d8b26148..f30b82a3d3 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py @@ -84,7 +84,7 @@ class MongoConnection(object): return self.course_index.find_one( son.SON([ (key_attr, re.compile(case_regex.format(getattr(key, key_attr)))) - for key_attr in ('org', 'offering') + for key_attr in ('org', 'course', 'run') ]) ) @@ -106,7 +106,7 @@ class MongoConnection(object): Update the db record for course_index """ self.course_index.update( - son.SON([('org', course_index['org']), ('offering', course_index['offering'])]), + son.SON([('org', course_index['org']), ('course', course_index['course']), ('run', course_index['run'])]), course_index ) @@ -114,7 +114,11 @@ class MongoConnection(object): """ Delete the course_index from the persistence mechanism whose id is the given course_index """ - return self.course_index.remove(son.SON([('org', course_index['org']), ('offering', course_index['offering'])])) + return self.course_index.remove(son.SON([ + ('org', course_index['org']), + ('course', course_index['course']), + ('run', course_index['run']) + ])) def get_definition(self, key): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 5cbf3633ac..0b76fbf3cd 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -5,7 +5,8 @@ Representation: * course_index: a dictionary: ** '_id': a unique id which cannot change, ** 'org': the org's id. Only used for searching not identity, - ** 'offering': the course's catalog number and run id or whatever user decides, + ** 'course': the course's catalog number + ** 'run': the course's run id or whatever user decides, ** 'edited_by': user_id of user who created the original entry, ** 'edited_on': the datetime of the original creation, ** 'versions': versions_dict: {branch_id: structure_id, ...} @@ -215,7 +216,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): course_key = CourseLocator( version_guid=course_entry['structure']['_id'], org=course_entry.get('org'), - offering=course_entry.get('offering'), + course=course_entry.get('course'), + run=course_entry.get('run'), branch=course_entry.get('branch'), ) self.cache_items(system, block_ids, course_key, depth, lazy) @@ -265,7 +267,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): :param course_locator: any subclass of CourseLocator ''' - if course_locator.org and course_locator.offering and course_locator.branch: + if course_locator.org and course_locator.course and course_locator.run and course_locator.branch: # use the course id index = self.db_connection.get_course_index(course_locator) if index is None: @@ -286,12 +288,14 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): version_guid = course_locator.as_object_id(version_guid) entry = self.db_connection.get_structure(version_guid) - # b/c more than one course can use same structure, the 'org', 'offering', and 'branch' are not intrinsic to structure + # b/c more than one course can use same structure, the 'org', 'course', + # 'run', and 'branch' are not intrinsic to structure # and the one assoc'd w/ it by another fetch may not be the one relevant to this fetch; so, # add it in the envelope for the structure. envelope = { 'org': course_locator.org, - 'offering': course_locator.offering, + 'course': course_locator.course, + 'run': course_locator.run, 'branch': course_locator.branch, 'structure': entry, } @@ -331,7 +335,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): course_info = id_version_map[entry['_id']] envelope = { 'org': course_info['org'], - 'offering': course_info['offering'], + 'course': course_info['course'], + 'run': course_info['run'], 'branch': branch, 'structure': entry, } @@ -362,7 +367,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): ''' assert(isinstance(course_id, CourseLocator)) course_index = self.db_connection.get_course_index(course_id, ignore_case) - return CourseLocator(course_index['org'], course_index['offering'], course_id.branch) if course_index else None + return CourseLocator(course_index['org'], course_index['course'], course_index['run'], course_id.branch) if course_index else None def has_item(self, usage_key): """ @@ -514,7 +519,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): The index records the initial creation of the indexed course and tracks the current version heads. This function is primarily for test verification but may serve some more general purpose. - :param course_locator: must have a org and offering set + :param course_locator: must have a org, course, and run set :return {'org': string, versions: {'draft': the head draft version id, 'published': the head published version id if any, @@ -523,7 +528,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): 'edited_on': when the course was originally created } """ - if not (course_locator.offering and course_locator.org): + if not (course_locator.course and course_locator.run and course_locator.org): return None index = self.db_connection.get_course_index(course_locator) return index @@ -749,7 +754,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): :param course_or_parent_locator: If BlockUsageLocator, then it's assumed to be the parent. If it's a CourseLocator, then it's - merely the containing course. If it has a version_guid and a course org + offering + branch, this + merely the containing course. If it has a version_guid and a course org + course + run + branch, this method ensures that the version is the head of the given course branch before making the change. raises InsufficientSpecificationError if there is no course locator. @@ -779,11 +784,11 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): Rules for course locator: - * If the course locator specifies a org and offering and either it doesn't + * If the course locator specifies a org and course and run and either it doesn't specify version_guid or the one it specifies == the current head of the branch, it progresses the course to point to the new head and sets the active version to point to the new head - * If the locator has a org and offering but its version_guid != current head, it raises VersionConflictError. + * If the locator has a org and course and run but its version_guid != current head, it raises VersionConflictError. NOTE: using a version_guid will end up creating a new version of the course. Your new item won't be in the course id'd by version_guid but instead in one w/ a new version_guid. Ensure in this case that you get @@ -886,7 +891,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): ) def create_course( - self, org, offering, user_id, fields=None, + self, org, course, run, user_id, fields=None, master_branch=ModuleStoreEnum.BranchName.draft, versions_dict=None, root_category='course', root_block_id='course', **kwargs ): @@ -897,12 +902,13 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): Arguments: org (str): the organization that owns the course - offering (str): the name of the course offering + course (str): the course number of the course + run (str): the particular run of the course (e.g. 2013_T1) user_id: id of the user creating the course fields (dict): Fields to set on the course at initialization kwargs: Any optional arguments understood by a subset of modulestores to customize instantiation - offering: If it's already taken, this method will raise DuplicateCourseError + course + run: If there are duplicates, this method will raise DuplicateCourseError fields: if scope.settings fields provided, will set the fields of the root course object in the new course. If both @@ -928,8 +934,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): provide any fields overrides, see above). if not provided, will create a mostly empty course structure with just a category course root xblock. """ - # check offering's uniqueness - locator = CourseLocator(org=org, offering=offering, branch=master_branch) + # check course and run's uniqueness + locator = CourseLocator(org=org, course=course, run=run, branch=master_branch) index = self.db_connection.get_course_index(locator) if index is not None: raise DuplicateCourseError(locator, index) @@ -1003,7 +1009,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): index_entry = { '_id': ObjectId(), 'org': org, - 'offering': offering, + 'course': course, + 'run': run, 'edited_by': user_id, 'edited_on': datetime.datetime.now(UTC), 'versions': versions_dict, @@ -1019,7 +1026,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): raises ItemNotFoundError if the location does not exist. - Creates a new course version. If the descriptor's location has a org and offering, it moves the course head + Creates a new course version. If the descriptor's location has a org and course and run, it moves the course head pointer. If the version_guid of the descriptor points to a non-head version and there's been an intervening change to this item, it raises a VersionConflictError unless force is True. In the force case, it forks the course but leaves the head pointer where it is (this change will not be in the course head). @@ -1067,7 +1074,9 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): if index_entry is not None: self._update_head(index_entry, descriptor.location.branch, new_id) course_key = CourseLocator( - org=index_entry['org'], offering=index_entry['offering'], + org=index_entry['org'], + course=index_entry['course'], + run=index_entry['run'], branch=descriptor.location.branch, version_guid=new_id ) @@ -1336,7 +1345,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): raises ItemNotFoundError if the location does not exist. raises ValueError if usage_locator points to the structure root - Creates a new course version. If the descriptor's location has a org and offering, it moves the course head + Creates a new course version. If the descriptor's location has a org, a course, and a run, it moves the course head pointer. If the version_guid of the descriptor points to a non-head version and there's been an intervening change to this item, it raises a VersionConflictError unless force is True. In the force case, it forks the course but leaves the head pointer where it is (this change will not be in the course head). @@ -1560,7 +1569,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): :param continue_version: if True, assumes this operation requires a head version and will not create a new version but instead continue an existing transaction on this version. This flag cannot be True if force is True. """ - if locator.org is None or locator.offering is None or locator.branch is None: + if locator.org is None or locator.course is None or locator. run is None or locator.branch is None: if continue_version: raise InsufficientSpecificationError( "To continue a version, the locator must point to one ({}).".format(locator) 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 8add221bc1..81f03aac8a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -123,11 +123,7 @@ class TestMixedModuleStore(LocMapperSetupSansDjango): """ Create a course w/ one item in the persistence store using the given course & item location. """ - if default == 'split': - offering = course_key.offering.replace('/', '.') - else: - offering = course_key.offering - course = self.store.create_course(course_key.org, offering, self.user_id) + course = self.store.create_course(course_key.org, course_key.course, course_key.run, self.user_id) category = self.writable_chapter_location.category block_id = self.writable_chapter_location.name chapter = self.store.create_item( @@ -367,7 +363,11 @@ class TestMixedModuleStore(LocMapperSetupSansDjango): xml_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.xml) # the important thing is not which exception it raises but that it raises an exception with self.assertRaises(AttributeError): +<<<<<<< HEAD xml_store.create_course("org", "course/run", self.user_id) +======= + xml_store.create_course("org", "course", "run", 999) +>>>>>>> Fix up the signature to use course and run instead of just offering @ddt.data('draft', 'split') def test_get_course(self, default_ms): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py index 5385eeda96..c6cbed8bc1 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py @@ -31,9 +31,9 @@ class TestOrphan(SplitWMongoCourseBoostrapper): """ orphans = self.old_mongo.get_orphans(self.old_course_key) self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans)) - location = self.old_course_key.make_usage_key('chapter', name='OrphanChapter') + location = self.old_course_key.make_usage_key('chapter', 'OrphanChapter') self.assertIn(location.to_deprecated_string(), orphans) - location = self.old_course_key.make_usage_key('vertical', name='OrphanVert') + location = self.old_course_key.make_usage_key('vertical', 'OrphanVert') self.assertIn(location.to_deprecated_string(), orphans) location = self.old_course_key.make_usage_key('html', 'OrphanHtml') self.assertIn(location.to_deprecated_string(), orphans) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py index dafcd0c02b..b6f0171b23 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py @@ -63,7 +63,7 @@ class TestPublish(SplitWMongoCourseBoostrapper): To reproduce a bug (STUD-811) publish a vertical, convert to draft, delete a child, move a child, publish. See if deleted and moved children still is connected or exists in db (bug was disconnected but existed) """ - vert_location = self.old_course_key.make_usage_key('vertical', name='Vert1') + vert_location = self.old_course_key.make_usage_key('vertical', block_id='Vert1') item = self.draft_mongo.get_item(vert_location, 2) # Vert1 has 3 children; so, publishes 4 nodes which may mean 4 inserts & 1 bulk remove # 25-June-2014 find calls are 19. Probably due to inheritance recomputation? @@ -78,16 +78,16 @@ class TestPublish(SplitWMongoCourseBoostrapper): # however, children are still draft, but I'm not sure that's by design # delete the draft version of the discussion - location = self.old_course_key.make_usage_key('discussion', name='Discussion1') + location = self.old_course_key.make_usage_key('discussion', block_id='Discussion1') self.draft_mongo.delete_item(location, self.userid) draft_vert = self.draft_mongo.get_item(vert_location, 0) self.assertTrue(getattr(draft_vert, 'is_draft', False), "Deletion didn't convert parent to draft") self.assertNotIn(location, draft_vert.children) # move the other child - other_child_loc = self.old_course_key.make_usage_key('html', name='Html2') + other_child_loc = self.old_course_key.make_usage_key('html', block_id='Html2') draft_vert.children.remove(other_child_loc) - other_vert = self.draft_mongo.get_item(self.old_course_key.make_usage_key('vertical', name='Vert2'), 0) + other_vert = self.draft_mongo.get_item(self.old_course_key.make_usage_key('vertical', block_id='Vert2'), 0) other_vert.children.append(other_child_loc) self.draft_mongo.update_item(draft_vert, self.userid) self.draft_mongo.update_item(other_vert, self.userid) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 57e5447186..1f024624c5 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -61,7 +61,8 @@ class SplitModuleTest(unittest.TestCase): COURSE_CONTENT = { "testx.GreekHero": { "org": "testx", - "offering": "GreekHero", + "course": "GreekHero", + "run": "run", "root_block_id": "head12345", "user_id": "test@edx.org", "fields": { @@ -281,7 +282,8 @@ class SplitModuleTest(unittest.TestCase): }, "testx.wonderful": { "org": "testx", - "offering": "wonderful", + "course": "wonderful", + "run": "run", "root_block_id": "head23456", "user_id": "test@edx.org", "fields": { @@ -387,7 +389,8 @@ class SplitModuleTest(unittest.TestCase): }, "guestx.contender": { "org": "guestx", - "offering": "contender", + "course": "contender", + "run": "run", "root_block_id": "head345679", "user_id": "test@guestx.edu", "fields": { @@ -450,7 +453,10 @@ class SplitModuleTest(unittest.TestCase): split_store = modulestore() for _course_id, course_spec in SplitModuleTest.COURSE_CONTENT.iteritems(): course = split_store.create_course( - course_spec['org'], course_spec['offering'], course_spec['user_id'], + course_spec['org'], + course_spec['course'], + course_spec['run'], + course_spec['user_id'], fields=course_spec['fields'], root_block_id=course_spec['root_block_id'] ) @@ -483,11 +489,11 @@ class SplitModuleTest(unittest.TestCase): course = split_store.persist_xblock_dag(course, revision['user_id']) # publish "testx.wonderful" to_publish = BlockUsageLocator( - CourseLocator(org="testx", offering="wonderful", branch=BRANCH_NAME_DRAFT), + CourseLocator(org="testx", course="wonderful", run="run", branch=BRANCH_NAME_DRAFT), block_type='course', block_id="head23456" ) - destination = CourseLocator(org="testx", offering="wonderful", branch=BRANCH_NAME_PUBLISHED) + destination = CourseLocator(org="testx", course="wonderful", run="run", branch=BRANCH_NAME_PUBLISHED) split_store.xblock_publish("test@edx.org", to_publish, destination, [to_publish], None) def setUp(self): @@ -520,7 +526,7 @@ class TestHasChildrenAtDepth(SplitModuleTest): def test_has_children_at_depth(self): course_locator = CourseLocator( - org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT + org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT ) block_locator = BlockUsageLocator( course_locator, 'course', 'head12345' @@ -586,7 +592,7 @@ class SplitModuleCourseTests(SplitModuleTest): course = self.findByIdInResult(courses_published, "head23456") self.assertIsNotNone(course, "published courses") self.assertEqual(course.location.course_key.org, "testx") - self.assertEqual(course.location.course_key.offering, "wonderful") + self.assertEqual(course.location.course_key.course, "wonderful") self.assertEqual(course.category, 'course', 'wrong category') self.assertEqual(len(course.tabs), 4, "wrong number of tabs") self.assertEqual(course.display_name, "The most wonderful course", @@ -611,15 +617,15 @@ class SplitModuleCourseTests(SplitModuleTest): check_has_course_method( modulestore(), - CourseLocator(org='testx', offering='wonderful', branch=BRANCH_NAME_DRAFT), - locator_key_fields=['org', 'offering'] + CourseLocator(org='testx', course='wonderful', run="run", branch=BRANCH_NAME_DRAFT), + locator_key_fields=['org', 'course', 'run'] ) def test_get_course(self): ''' Test the various calling forms for get_course ''' - locator = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT) + locator = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT) head_course = modulestore().get_course(locator) self.assertNotEqual(head_course.location.version_guid, head_course.previous_version) locator = CourseLocator(version_guid=head_course.previous_version) @@ -637,10 +643,11 @@ class SplitModuleCourseTests(SplitModuleTest): self.assertEqual(course.edited_by, "testassist@edx.org") self.assertDictEqual(course.grade_cutoffs, {"Pass": 0.55}) - locator = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT) + locator = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT) course = modulestore().get_course(locator) self.assertEqual(course.location.course_key.org, "testx") - self.assertEqual(course.location.course_key.offering, "GreekHero") + self.assertEqual(course.location.course_key.course, "GreekHero") + self.assertEqual(course.location.course_key.run, "run") self.assertEqual(course.category, 'course') self.assertEqual(len(course.tabs), 6) self.assertEqual(course.display_name, "The Ancient Greek Hero") @@ -650,28 +657,28 @@ class SplitModuleCourseTests(SplitModuleTest): self.assertEqual(course.edited_by, "testassist@edx.org") self.assertDictEqual(course.grade_cutoffs, {"Pass": 0.45}) - locator = CourseLocator(org='testx', offering='wonderful', branch=BRANCH_NAME_PUBLISHED) + locator = CourseLocator(org='testx', course='wonderful', run="run", branch=BRANCH_NAME_PUBLISHED) course = modulestore().get_course(locator) published_version = course.location.version_guid - locator = CourseLocator(org='testx', offering='wonderful', branch=BRANCH_NAME_DRAFT) + locator = CourseLocator(org='testx', course='wonderful', run="run", branch=BRANCH_NAME_DRAFT) course = modulestore().get_course(locator) self.assertNotEqual(course.location.version_guid, published_version) def test_get_course_negative(self): # Now negative testing with self.assertRaises(InsufficientSpecificationError): - modulestore().get_course(CourseLocator(org='edu', offering='meh.blah')) + modulestore().get_course(CourseLocator(org='edu', course='meh', run='blah')) with self.assertRaises(ItemNotFoundError): - modulestore().get_course(CourseLocator(org='edu', offering='nosuchthing', branch=BRANCH_NAME_DRAFT)) + modulestore().get_course(CourseLocator(org='edu', course='nosuchthing', run="run", branch=BRANCH_NAME_DRAFT)) with self.assertRaises(ItemNotFoundError): - modulestore().get_course(CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_PUBLISHED)) + modulestore().get_course(CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_PUBLISHED)) def test_cache(self): """ Test that the mechanics of caching work. """ - locator = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT) + locator = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT) course = modulestore().get_course(locator) block_map = modulestore().cache_items( course.system, [child.block_id for child in course.children], course.id, depth=3 @@ -683,7 +690,7 @@ class SplitModuleCourseTests(SplitModuleTest): """ get_course_successors(course_locator, version_history_depth=1) """ - locator = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT) + locator = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT) course = modulestore().get_course(locator) versions = [course.location.version_guid, course.previous_version] locator = CourseLocator(version_guid=course.previous_version) @@ -720,8 +727,9 @@ class SplitModuleItemTests(SplitModuleTest): has_item(BlockUsageLocator) ''' org = 'testx' - offering = 'GreekHero' - course_locator = CourseLocator(org=org, offering=offering, branch=BRANCH_NAME_DRAFT) + course = 'GreekHero' + run = 'run' + course_locator = CourseLocator(org=org, course=course, run=run, branch=BRANCH_NAME_DRAFT) course = modulestore().get_course(course_locator) previous_version = course.previous_version # positive tests of various forms @@ -754,7 +762,7 @@ class SplitModuleItemTests(SplitModuleTest): # in published course locator = BlockUsageLocator( - CourseLocator(org="testx", offering="wonderful", branch=BRANCH_NAME_DRAFT), + CourseLocator(org="testx", course="wonderful", run="run", branch=BRANCH_NAME_DRAFT), block_type="course", block_id="head23456" ) @@ -766,13 +774,13 @@ class SplitModuleItemTests(SplitModuleTest): # negative tests--not found # no such course or block locator = BlockUsageLocator( - CourseLocator(org="foo", offering="doesnotexist", branch=BRANCH_NAME_DRAFT), + CourseLocator(org="foo", course="doesnotexist", run="run", branch=BRANCH_NAME_DRAFT), block_type="course", block_id="head23456" ) self.assertFalse(modulestore().has_item(locator)) locator = BlockUsageLocator( - CourseLocator(org="testx", offering="wonderful", branch=BRANCH_NAME_DRAFT), + CourseLocator(org="testx", course="wonderful", run="run", branch=BRANCH_NAME_DRAFT), block_type="vertical", block_id="doesnotexist" ) @@ -782,7 +790,7 @@ class SplitModuleItemTests(SplitModuleTest): ''' get_item(blocklocator) ''' - hero_locator = CourseLocator(org="testx", offering="GreekHero", branch=BRANCH_NAME_DRAFT) + hero_locator = CourseLocator(org="testx", course="GreekHero", run="run", branch=BRANCH_NAME_DRAFT) course = modulestore().get_course(hero_locator) previous_version = course.previous_version @@ -797,7 +805,8 @@ class SplitModuleItemTests(SplitModuleTest): Check contents of block """ self.assertEqual(block.location.org, "testx") - self.assertEqual(block.location.offering, "GreekHero") + self.assertEqual(block.location.course, "GreekHero") + self.assertEqual(block.location.run, "run") self.assertEqual(len(block.tabs), 6, "wrong number of tabs") self.assertEqual(block.display_name, "The Ancient Greek Hero") self.assertEqual(block.advertised_start, "Fall 2013") @@ -818,8 +827,8 @@ class SplitModuleItemTests(SplitModuleTest): """ Tests that has_changes() only returns true when changes are present """ - draft_course = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT) - published_course = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_PUBLISHED) + draft_course = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT) + published_course = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_PUBLISHED) head = draft_course.make_usage_key('course', 'head12345') dummy_user = ModuleStoreEnum.UserID.test @@ -843,18 +852,18 @@ class SplitModuleItemTests(SplitModuleTest): def test_get_non_root(self): # not a course obj locator = BlockUsageLocator( - CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT), 'chapter', 'chapter1' + CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT), 'chapter', 'chapter1' ) block = modulestore().get_item(locator) self.assertEqual(block.location.org, "testx") - self.assertEqual(block.location.offering, "GreekHero") + self.assertEqual(block.location.course, "GreekHero") self.assertEqual(block.category, 'chapter') self.assertEqual(block.display_name, "Hercules") self.assertEqual(block.edited_by, "testassist@edx.org") # in published course locator = BlockUsageLocator( - CourseLocator(org='testx', offering='wonderful', branch=BRANCH_NAME_PUBLISHED), 'course', 'head23456' + CourseLocator(org='testx', course='wonderful', run="run", branch=BRANCH_NAME_PUBLISHED), 'course', 'head23456' ) self.assertIsInstance( modulestore().get_item(locator), @@ -864,12 +873,12 @@ class SplitModuleItemTests(SplitModuleTest): # negative tests--not found # no such course or block locator = BlockUsageLocator( - CourseLocator(org='doesnotexist', offering='doesnotexist', branch=BRANCH_NAME_DRAFT), 'course', 'head23456' + CourseLocator(org='doesnotexist', course='doesnotexist', run="run", branch=BRANCH_NAME_DRAFT), 'course', 'head23456' ) with self.assertRaises(ItemNotFoundError): modulestore().get_item(locator) locator = BlockUsageLocator( - CourseLocator(org='testx', offering='wonderful', branch=BRANCH_NAME_DRAFT), 'html', 'doesnotexist' + CourseLocator(org='testx', course='wonderful', run="run", branch=BRANCH_NAME_DRAFT), 'html', 'doesnotexist' ) with self.assertRaises(ItemNotFoundError): modulestore().get_item(locator) @@ -903,7 +912,7 @@ class SplitModuleItemTests(SplitModuleTest): ''' get_items(locator, qualifiers, [branch]) ''' - locator = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT) + locator = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT) # get all modules matches = modulestore().get_items(locator) self.assertEqual(len(matches), 6) @@ -929,14 +938,14 @@ class SplitModuleItemTests(SplitModuleTest): get_parent_location(locator): BlockUsageLocator ''' locator = BlockUsageLocator( - CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT), + CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT), 'chapter', block_id='chapter1' ) parent = modulestore().get_parent_location(locator) self.assertIsNotNone(parent) self.assertEqual(parent.block_id, 'head12345') self.assertEqual(parent.org, "testx") - self.assertEqual(parent.offering, "GreekHero") + self.assertEqual(parent.course, "GreekHero") locator = locator.course_key.make_usage_key('Chapter', 'chapter2') parent = modulestore().get_parent_location(locator) self.assertEqual(parent.block_id, 'head12345') @@ -949,7 +958,7 @@ class SplitModuleItemTests(SplitModuleTest): Test the existing get_children method on xdescriptors """ locator = BlockUsageLocator( - CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT), 'course', 'head12345' + CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT), 'course', 'head12345' ) block = modulestore().get_item(locator) children = block.get_children() @@ -996,7 +1005,7 @@ class TestItemCrud(SplitModuleTest): create_item(course_or_parent_locator, category, user, definition_locator=None, fields): new_desciptor """ # grab link to course to ensure new versioning works - locator = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT) + locator = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT) premod_course = modulestore().get_course(locator) premod_history = modulestore().get_course_history_info(premod_course.location) # add minimal one w/o a parent @@ -1006,7 +1015,7 @@ class TestItemCrud(SplitModuleTest): fields={'display_name': 'new sequential'} ) # check that course version changed and course's previous is the other one - self.assertEqual(new_module.location.offering, "GreekHero") + self.assertEqual(new_module.location.course, "GreekHero") self.assertNotEqual(new_module.location.version_guid, premod_course.location.version_guid) self.assertIsNone(locator.version_guid, "Version inadvertently filled in") current_course = modulestore().get_course(locator) @@ -1032,13 +1041,13 @@ class TestItemCrud(SplitModuleTest): Test create_item w/ specifying the parent of the new item """ locator = BlockUsageLocator( - CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT), + CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT), 'chapter', block_id='chapter2' ) original = modulestore().get_item(locator) locator = BlockUsageLocator( - CourseLocator(org='testx', offering='wonderful', branch=BRANCH_NAME_DRAFT), 'course', 'head23456' + CourseLocator(org='testx', course='wonderful', run="run", branch=BRANCH_NAME_DRAFT), 'course', 'head23456' ) premod_course = modulestore().get_course(locator.course_key) category = 'chapter' @@ -1061,13 +1070,13 @@ class TestItemCrud(SplitModuleTest): Actually, this tries to test all create_item features not tested above. """ locator = BlockUsageLocator( - CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT), + CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT), 'problem', block_id='problem1' ) original = modulestore().get_item(locator) locator = BlockUsageLocator( - CourseLocator(org='guestx', offering='contender', branch=BRANCH_NAME_DRAFT), 'course', 'head345679' + CourseLocator(org='guestx', course='contender', run="run", branch=BRANCH_NAME_DRAFT), 'course', 'head345679' ) category = 'problem' new_payload = "empty" @@ -1100,7 +1109,7 @@ class TestItemCrud(SplitModuleTest): """ Check that using odd characters in block id don't break ability to add and retrieve block. """ - course_key = CourseLocator(org='guestx', offering='contender', branch=BRANCH_NAME_DRAFT) + course_key = CourseLocator(org='guestx', course='contender', run="run", branch=BRANCH_NAME_DRAFT) parent_locator = BlockUsageLocator(course_key, 'course', block_id="head345679") chapter_locator = BlockUsageLocator(course_key, 'chapter', block_id="foo.bar_-~:0") modulestore().create_item( @@ -1131,7 +1140,7 @@ class TestItemCrud(SplitModuleTest): """ # start transaction w/ simple creation user = random.getrandbits(32) - new_course = modulestore().create_course('test_org', 'test_transaction', user) + new_course = modulestore().create_course('test_org', 'test_transaction', 'test_run', user) new_course_locator = new_course.id index_history_info = modulestore().get_course_history_info(new_course.location) course_block_prev_version = new_course.previous_version @@ -1209,7 +1218,7 @@ class TestItemCrud(SplitModuleTest): test updating an items metadata ensuring the definition doesn't version but the course does if it should """ locator = BlockUsageLocator( - CourseLocator(org="testx", offering="GreekHero", branch=BRANCH_NAME_DRAFT), + CourseLocator(org="testx", course="GreekHero", run="run", branch=BRANCH_NAME_DRAFT), 'problem', block_id="problem3_2" ) problem = modulestore().get_item(locator) @@ -1243,7 +1252,7 @@ class TestItemCrud(SplitModuleTest): test updating an item's children ensuring the definition doesn't version but the course does if it should """ locator = BlockUsageLocator( - CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT), 'chapter', 'chapter3' + CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT), 'chapter', 'chapter3' ) block = modulestore().get_item(locator) pre_def_id = block.definition_locator.definition_id @@ -1270,7 +1279,7 @@ class TestItemCrud(SplitModuleTest): test updating an item's definition: ensure it gets versioned as well as the course getting versioned """ locator = BlockUsageLocator( - CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT), 'course', 'head12345' + CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT), 'course', 'head12345' ) block = modulestore().get_item(locator) pre_def_id = block.definition_locator.definition_id @@ -1289,13 +1298,13 @@ class TestItemCrud(SplitModuleTest): Test updating metadata, children, and definition in a single call ensuring all the versioning occurs """ locator = BlockUsageLocator( - CourseLocator('testx', 'GreekHero', branch=BRANCH_NAME_DRAFT), + CourseLocator('testx', 'GreekHero', 'run', branch=BRANCH_NAME_DRAFT), 'problem', block_id='problem1' ) original = modulestore().get_item(locator) # first add 2 children to the course for the update to manipulate locator = BlockUsageLocator( - CourseLocator('guestx', 'contender', branch=BRANCH_NAME_DRAFT), + CourseLocator('guestx', 'contender', 'run', branch=BRANCH_NAME_DRAFT), 'course', block_id="head345679" ) category = 'problem' @@ -1373,7 +1382,7 @@ class TestItemCrud(SplitModuleTest): """ Create a course we can delete """ - course = modulestore().create_course('nihilx', 'deletion', 'deleting_user') + course = modulestore().create_course('nihilx', 'deletion', 'run', 'deleting_user') root = course.location.version_agnostic().for_branch(BRANCH_NAME_DRAFT) for _ in range(4): self.create_subtree_for_deletion(root, ['chapter', 'vertical', 'problem']) @@ -1400,7 +1409,7 @@ class TestCourseCreation(SplitModuleTest): The simplest case but probing all expected results from it. """ # Oddly getting differences of 200nsec - new_course = modulestore().create_course('test_org', 'test_course', 'create_user') + new_course = modulestore().create_course('test_org', 'test_course', 'test_run', 'create_user') new_locator = new_course.location # check index entry index_info = modulestore().get_course_index_info(new_locator) @@ -1425,10 +1434,10 @@ class TestCourseCreation(SplitModuleTest): """ Test making a course which points to an existing draft and published but not making any changes to either. """ - original_locator = CourseLocator(org='testx', offering='wonderful', branch=BRANCH_NAME_DRAFT) + original_locator = CourseLocator(org='testx', course='wonderful', run="run", branch=BRANCH_NAME_DRAFT) original_index = modulestore().get_course_index_info(original_locator) new_draft = modulestore().create_course( - 'best', 'leech', 'leech_master', + 'best', 'leech', 'leech_run', 'leech_master', versions_dict=original_index['versions']) new_draft_locator = new_draft.location self.assertRegexpMatches(new_draft_locator.org, 'best') @@ -1467,7 +1476,7 @@ class TestCourseCreation(SplitModuleTest): """ Create a new course which overrides metadata and course_data """ - original_locator = CourseLocator(org='guestx', offering='contender', branch=BRANCH_NAME_DRAFT) + original_locator = CourseLocator(org='guestx', course='contender', run="run", branch=BRANCH_NAME_DRAFT) original = modulestore().get_course(original_locator) original_index = modulestore().get_course_index_info(original_locator) fields = {} @@ -1484,7 +1493,7 @@ class TestCourseCreation(SplitModuleTest): fields['grading_policy']['GRADE_CUTOFFS'] = {'A': .9, 'B': .8, 'C': .65} fields['display_name'] = 'Derivative' new_draft = modulestore().create_course( - 'counter', 'leech', 'leech_master', + 'counter', 'leech', 'leech_run', 'leech_master', versions_dict={BRANCH_NAME_DRAFT: original_index['versions'][BRANCH_NAME_DRAFT]}, fields=fields ) @@ -1504,11 +1513,11 @@ class TestCourseCreation(SplitModuleTest): def test_update_course_index(self): """ - Test the versions pointers. NOTE: you can change the org, offering, or other things, but + Test the versions pointers. NOTE: you can change the org, course, or other things, but it's not clear how you'd find them again or associate them w/ existing student history since we use course_key so many places as immutable. """ - locator = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT) + locator = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT) course_info = modulestore().get_course_index_info(locator) # an allowed but not necessarily recommended way to revert the draft version @@ -1531,7 +1540,7 @@ class TestCourseCreation(SplitModuleTest): """ user = random.getrandbits(32) new_course = modulestore().create_course( - 'test_org', 'test_transaction', user, + 'test_org', 'test_transaction', 'test_run', user, root_block_id='top', root_category='chapter' ) self.assertEqual(new_course.location.block_id, 'top') @@ -1553,7 +1562,7 @@ class TestCourseCreation(SplitModuleTest): courses = modulestore().get_courses() with self.assertRaises(DuplicateCourseError): dupe_course_key = courses[0].location.course_key - modulestore().create_course(dupe_course_key.org, dupe_course_key.offering, user) + modulestore().create_course(dupe_course_key.org, dupe_course_key.course, dupe_course_key.run, user) class TestInheritance(SplitModuleTest): @@ -1567,13 +1576,13 @@ class TestInheritance(SplitModuleTest): # Note, not testing value where defined (course) b/c there's no # defined accessor for it on CourseDescriptor. locator = BlockUsageLocator( - CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT), 'problem', 'problem3_2' + CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT), 'problem', 'problem3_2' ) node = modulestore().get_item(locator) # inherited self.assertEqual(node.graceperiod, datetime.timedelta(hours=2)) locator = BlockUsageLocator( - CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT), 'problem', 'problem1' + CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT), 'problem', 'problem1' ) node = modulestore().get_item(locator) # overridden @@ -1594,8 +1603,8 @@ class TestPublish(SplitModuleTest): """ Test the standard patterns: publish to new branch, revise and publish """ - source_course = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT) - dest_course = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_PUBLISHED) + source_course = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT) + dest_course = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_PUBLISHED) head = source_course.make_usage_key('course', "head12345") chapter1 = source_course.make_usage_key('chapter', 'chapter1') chapter2 = source_course.make_usage_key('chapter', 'chapter2') @@ -1640,16 +1649,16 @@ class TestPublish(SplitModuleTest): """ Test the exceptions which preclude successful publication """ - source_course = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT) + source_course = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT) # destination does not exist - destination_course = CourseLocator(org='fake', offering='Unknown', branch=BRANCH_NAME_PUBLISHED) + destination_course = CourseLocator(org='fake', course='Unknown', run="run", branch=BRANCH_NAME_PUBLISHED) head = source_course.make_usage_key('course', "head12345") chapter3 = source_course.make_usage_key('chapter', 'chapter3') problem1 = source_course.make_usage_key('problem', 'problem1') with self.assertRaises(ItemNotFoundError): modulestore().xblock_publish(self.user_id, source_course, destination_course, [chapter3], None) # publishing into a new branch w/o publishing the root - destination_course = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_PUBLISHED) + destination_course = CourseLocator(org='testx', course='GreekHero', run='run', branch=BRANCH_NAME_PUBLISHED) with self.assertRaises(ItemNotFoundError): modulestore().xblock_publish(self.user_id, source_course, destination_course, [chapter3], None) # publishing a subdag w/o the parent already in course @@ -1661,8 +1670,8 @@ class TestPublish(SplitModuleTest): """ Test publishing moves and deletes. """ - source_course = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT) - dest_course = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_PUBLISHED) + source_course = CourseLocator(org='testx', course='GreekHero', run='run', branch=BRANCH_NAME_DRAFT) + dest_course = CourseLocator(org='testx', course='GreekHero', run='run', branch=BRANCH_NAME_PUBLISHED) head = source_course.make_usage_key('course', "head12345") chapter2 = source_course.make_usage_key('chapter', 'chapter2') problem1 = source_course.make_usage_key('problem', 'problem1') diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py index cc13bd838d..aab59d0463 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py @@ -40,7 +40,7 @@ class SplitWMongoCourseBoostrapper(unittest.TestCase): 'xblock_mixins': (InheritanceMixin,) } - split_course_key = CourseLocator('test_org', 'test_course.runid', branch=ModuleStoreEnum.BranchName.draft) + split_course_key = CourseLocator('test_org', 'test_course', 'runid', branch=ModuleStoreEnum.BranchName.draft) def setUp(self): self.db_config['collection'] = 'modulestore{0}'.format(uuid.uuid4().hex[:5]) @@ -135,8 +135,8 @@ class SplitWMongoCourseBoostrapper(unittest.TestCase): if split: # split requires the course to be created separately from creating items self.split_mongo.create_course( - self.split_course_key.org, self.split_course_key.offering, self.userid, fields=fields, root_block_id='runid' + self.split_course_key.org, self.split_course_key.course, self.split_course_key.run, self.userid, fields=fields, root_block_id='runid' ) - old_course = self.old_mongo.create_course(self.split_course_key.org, 'test_course/runid', self.userid, fields=fields) + old_course = self.old_mongo.create_course(self.split_course_key.org, 'test_course', 'runid', self.user_id, fields=fields) self.old_course_key = old_course.id self.runtime = old_course.runtime diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 61e71b70db..2de6a9b01a 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -172,7 +172,7 @@ def import_from_xml( # Creates a new course if it doesn't already exist if create_new_course_if_not_present and not store.has_course(dest_course_id, ignore_case=True): try: - store.create_course(dest_course_id.org, dest_course_id.offering, user_id) + store.create_course(dest_course_id.org, dest_course_id.course, dest_course_id.run, user_id) except InvalidLocationError: # course w/ same org and course exists log.debug(