Skip to content

E
edx-platform
Commit e39cc5df by Mat Peterson Committed by Diana Huang
Options

Switch offering to course and run

parent 78a29595
Showing with 127 additions and 88 deletions
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'
)
......
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
......
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
......
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):
"""
......
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)
......
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
......
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'),
......
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):
"""
......
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):
......
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)
......
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)
......
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
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(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment