Split: add schema version and drop pretty_id (was placeholder looking for purpose)

STUD-1359

cms/djangoapps/contentstore/tests/test_crud.py

@@ -57,14 +57,14 @@ class TemplateTests(unittest.TestCase):
 
     def test_factories(self):
         test_course = persistent_factories.PersistentCourseFactory.create(
-            course_id='testx.tempcourse', org='testx', prettyid='tempcourse',
+            course_id='testx.tempcourse', 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 = modulestore('split').get_course_index_info(test_course.location)
         self.assertEqual(index_info['org'], 'testx')
-        self.assertEqual(index_info['prettyid'], 'tempcourse')
+        self.assertEqual(index_info['_id'], 'testx.tempcourse')
 
         test_chapter = persistent_factories.ItemFactory.create(display_name='chapter 1',
             parent_location=test_course.location)
@@ -75,7 +75,7 @@ class TemplateTests(unittest.TestCase):
 
         with self.assertRaises(DuplicateCourseError):
             persistent_factories.PersistentCourseFactory.create(
-                course_id='testx.tempcourse', org='testx', prettyid='tempcourse',
+                course_id='testx.tempcourse', org='testx', 
                 display_name='fun test course', user_id='testbot'
             )
 
@@ -84,7 +84,7 @@ class TemplateTests(unittest.TestCase):
         Test using load_from_json to create non persisted xblocks
         """
         test_course = persistent_factories.PersistentCourseFactory.create(
-            course_id='testx.tempcourse', org='testx', prettyid='tempcourse',
+            course_id='testx.tempcourse', org='testx', 
             display_name='fun test course', user_id='testbot'
         )
 
@@ -111,8 +111,9 @@ class TemplateTests(unittest.TestCase):
         try saving temporary xblocks
         """
         test_course = persistent_factories.PersistentCourseFactory.create(
-            course_id='testx.tempcourse', org='testx', prettyid='tempcourse',
-            display_name='fun test course', user_id='testbot')
+            course_id='testx.tempcourse', org='testx', 
+            display_name='fun test course', user_id='testbot'
+        )
         test_chapter = self.load_from_json({'category': 'chapter',
             'fields': {'display_name': 'chapter n'}},
             test_course.system, parent_xblock=test_course)
@@ -149,7 +150,6 @@ class TemplateTests(unittest.TestCase):
     def test_delete_course(self):
         test_course = persistent_factories.PersistentCourseFactory.create(
             course_id='edu.harvard.history.doomed', org='testx',
-            prettyid='edu.harvard.history.doomed',
             display_name='doomed test course',
             user_id='testbot')
         persistent_factories.ItemFactory.create(display_name='chapter 1',
@@ -173,9 +173,9 @@ class TemplateTests(unittest.TestCase):
         """
         test_course = persistent_factories.PersistentCourseFactory.create(
             course_id='edu.harvard.history.hist101', org='testx',
-            prettyid='edu.harvard.history.hist101',
             display_name='history test course',
-            user_id='testbot')
+            user_id='testbot'
+        )
         chapter = persistent_factories.ItemFactory.create(display_name='chapter 1',
             parent_location=test_course.location, user_id='testbot')
         sub = persistent_factories.ItemFactory.create(display_name='subsection 1',

cms/djangoapps/contentstore/views/course.py

@@ -97,7 +97,7 @@ def course_handler(request, tag=None, package_id=None, branch=None, version_guid
         index entry.
     PUT
         json: update this course (index entry not xblock) such as repointing head, changing display name, org,
-        package_id, prettyid. Return same json as above.
+        package_id. Return same json as above.
     DELETE
         json: delete this branch from this course (leaving off /branch/draft would imply delete the course)
     """

common/lib/xmodule/xmodule/modulestore/locator.py

@@ -358,8 +358,7 @@ class CourseLocator(Locator):
         Generate a discussion group id based on course
 
         To make compatible with old Location object functionality. I don't believe this behavior fits at this
-        place, but I have no way to override. If this is really needed, it should probably use the pretty_id to seed
-        the name although that's mutable. We should also clearly define the purpose and restrictions of this
+        place, but I have no way to override. We should clearly define the purpose and restrictions of this
         (e.g., I'm assuming periods are fine).
         """
         return self.package_id

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

@@ -282,7 +282,6 @@ class MixedModuleStore(ModuleStoreWriteBase):
         :param fields: a dict of xblock field name - value pairs for the course module.
         :param metadata: the old way of setting fields by knowing which ones are scope.settings v scope.content
         :param definition_data: the complement to metadata which is also a subset of fields
-        :param pretty_id: a field split.create_course uses and may quit using
         :returns: course xblock
         """
         store = self.modulestores[store_name]
@@ -297,11 +296,10 @@ class MixedModuleStore(ModuleStoreWriteBase):
                 org = None
 
             org = kwargs.pop('org', org)
-            pretty_id = kwargs.pop('pretty_id', course_id)
             fields = kwargs.pop('fields', {})
             fields.update(kwargs.pop('metadata', {}))
             fields.update(kwargs.pop('definition_data', {}))
-            course = store.create_course(course_id, org, pretty_id, user_id, fields=fields, **kwargs)
+            course = store.create_course(course_id, org, user_id, fields=fields, **kwargs)
         else:  # assume mongo
             course = store.create_course(course_id, **kwargs)
 

common/lib/xmodule/xmodule/modulestore/split_migrator.py

@@ -47,7 +47,7 @@ class SplitMigrator(object):
         original_course = self.direct_modulestore.get_item(course_location)
         new_course_root_locator = self.loc_mapper.translate_location(old_course_id, course_location)
         new_course = self.split_modulestore.create_course(
-            new_package_id, course_location.org, original_course.display_name,
+            new_package_id, course_location.org, 
             user.id,
             fields=self._get_json_fields_translate_children(original_course, old_course_id, True),
             root_block_id=new_course_root_locator.block_id,

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -5,7 +5,6 @@ Representation:
 * course_index: a dictionary:
     ** '_id': package_id (e.g., myu.mydept.mycourse.myrun),
     ** 'org': the org's id. Only used for searching not identity,
-    ** 'prettyid': a vague to-be-determined field probably more useful to storing searchable tags,
     ** '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, ...}
@@ -99,6 +98,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
     A Mongodb backed ModuleStore supporting versions, inheritance,
     and sharing.
     """
+
+    SCHEMA_VERSION = 1
     reference_type = Locator
     def __init__(self, doc_store_config, fs_root, render_template,
                  default_class=None,
@@ -468,7 +469,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         heads. This function is primarily for test verification but may serve some
         more general purpose.
         :param course_locator: must have a package_id set
-        :return {'org': , 'prettyid': ,
+        :return {'org': string,
             versions: {'draft': the head draft version id,
                 'published': the head published version id if any,
             },
@@ -618,7 +619,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
                 "edited_on": datetime.datetime.now(UTC),
                 "previous_version": None,
                 "original_version": new_id,
-            }
+            },
+            'schema_version': self.SCHEMA_VERSION,
         }
         self.db_connection.insert_definition(document)
         definition_locator = DefinitionLocator(new_id)
@@ -654,6 +656,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
             old_definition['edit_info']['edited_on'] = datetime.datetime.now(UTC)
             # previous version id
             old_definition['edit_info']['previous_version'] = definition_locator.definition_id
+            old_definition['schema_version'] = self.SCHEMA_VERSION
             self.db_connection.insert_definition(old_definition)
             return DefinitionLocator(old_definition['_id']), True
         else:
@@ -811,7 +814,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         return self.get_item(item_loc)
 
     def create_course(
-        self, course_id, org, prettyid, user_id, fields=None,
+        self, course_id, org, user_id, fields=None,
         master_branch='draft', versions_dict=None, root_category='course',
         root_block_id='course'
     ):
@@ -870,7 +873,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
                     'edited_on': datetime.datetime.now(UTC),
                     'previous_version': None,
                     'original_version': definition_id,
-                }
+                },
+                'schema_version': self.SCHEMA_VERSION,
             }
             self.db_connection.insert_definition(definition_entry)
 
@@ -904,6 +908,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
                     definition['edit_info']['edited_by'] = user_id
                     definition['edit_info']['edited_on'] = datetime.datetime.now(UTC)
                     definition['_id'] = ObjectId()
+                    definition['schema_version'] = self.SCHEMA_VERSION
                     self.db_connection.insert_definition(definition)
                     root_block['definition'] = definition['_id']
                     root_block['edit_info']['edited_on'] = datetime.datetime.now(UTC)
@@ -917,10 +922,11 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         index_entry = {
             '_id': course_id,
             'org': org,
-            'prettyid': prettyid,
             'edited_by': user_id,
             'edited_on': datetime.datetime.now(UTC),
-            'versions': versions_dict}
+            'versions': versions_dict,
+            'schema_version': self.SCHEMA_VERSION,
+        }
         self.db_connection.insert_course_index(index_entry)
         return self.get_course(CourseLocator(package_id=course_id, branch=master_branch))
 
@@ -1448,6 +1454,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         new_structure['previous_version'] = structure['_id']
         new_structure['edited_by'] = user_id
         new_structure['edited_on'] = datetime.datetime.now(UTC)
+        new_structure['schema_version'] = self.SCHEMA_VERSION
         return new_structure
 
     def _find_local_root(self, element_to_find, possibility, tree):
@@ -1508,7 +1515,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
             'original_version': new_id,
             'edited_by': user_id,
             'edited_on': datetime.datetime.now(UTC),
-            'blocks': blocks
+            'blocks': blocks,
+            'schema_version': self.SCHEMA_VERSION,
         }
 
     def _get_parents_from_structure(self, block_id, structure):

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

@@ -24,7 +24,6 @@ class PersistentCourseFactory(SplitFactory):
     keywords: any xblock field plus (note, the below are filtered out; so, if they
     become legitimate xblock fields, they won't be settable via this factory)
     * org: defaults to textX
-    * prettyid: defaults to 999
     * master_branch: (optional) defaults to 'draft'
     * user_id: (optional) defaults to 'test_user'
     * display_name (xblock field): will default to 'Robot Super Course' unless provided
@@ -33,14 +32,14 @@ class PersistentCourseFactory(SplitFactory):
 
     # pylint: disable=W0613
     @classmethod
-    def _create(cls, target_class, course_id='testX.999', org='testX', prettyid='999', user_id='test_user',
+    def _create(cls, target_class, course_id='testX.999', org='testX', user_id='test_user',
                 master_branch='draft', **kwargs):
 
         modulestore = kwargs.pop('modulestore')
         root_block_id = kwargs.pop('root_block_id', 'course')
         # Write the data to the mongo datastore
         new_course = modulestore.create_course(
-            course_id, org, prettyid, user_id, fields=kwargs,
+            course_id, org, user_id, fields=kwargs,
             master_branch=master_branch, root_block_id=root_block_id
         )
 

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

@@ -114,7 +114,7 @@ class TestOrphan(unittest.TestCase):
         fields.update(data)
         # split requires the course to be created separately from creating items
         self.split_mongo.create_course(
-            self.split_package_id, 'test_org', 'my course', self.userid, fields=fields, root_block_id='runid'
+            self.split_package_id, 'test_org', self.userid, fields=fields, root_block_id='runid'
         )
         self.course_location = Location('i4x', 'test_org', 'test_course', 'course', 'runid')
         self.old_mongo.create_and_save_xmodule(self.course_location, data, metadata)

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

@@ -176,12 +176,6 @@ class SplitModuleCourseTests(SplitModuleTest):
             qualifiers={'edited_on': {"$lt": datetime.datetime(2013, 3, 28, 15)}})
         self.assertEqual(len(courses), 2)
 
-        courses = modulestore().get_courses(
-            branch='draft',
-            qualifiers={'org': 'testx', "prettyid": "test_course"})
-        self.assertEqual(len(courses), 1)
-        self.assertIsNotNone(self.findByIdInResult(courses, "head12345"))
-
     def test_get_course(self):
         '''
         Test the various calling forms for get_course
@@ -652,7 +646,7 @@ class TestItemCrud(SplitModuleTest):
         """
         # start transaction w/ simple creation
         user = random.getrandbits(32)
-        new_course = modulestore().create_course('test_org.test_transaction', 'test_org', 'test_transaction', user)
+        new_course = modulestore().create_course('test_org.test_transaction', 'test_org', user)
         new_course_locator = new_course.location.as_course_locator()
         index_history_info = modulestore().get_course_history_info(new_course.location)
         course_block_prev_version = new_course.previous_version
@@ -901,7 +895,7 @@ class TestItemCrud(SplitModuleTest):
         check_subtree(nodes[0])
 
     def create_course_for_deletion(self):
-        course = modulestore().create_course('nihilx.deletion', 'nihilx', 'deletion', 'deleting_user')
+        course = modulestore().create_course('nihilx.deletion', 'nihilx', 'deleting_user')
         root = BlockUsageLocator(
             package_id=course.location.package_id,
             block_id=course.location.block_id,
@@ -929,12 +923,11 @@ class TestCourseCreation(SplitModuleTest):
         """
         # Oddly getting differences of 200nsec
         pre_time = datetime.datetime.now(UTC) - datetime.timedelta(milliseconds=1)
-        new_course = modulestore().create_course('test_org.test_course', 'test_org', 'test_course', 'create_user')
+        new_course = modulestore().create_course('test_org.test_course', 'test_org', 'create_user')
         new_locator = new_course.location
         # check index entry
         index_info = modulestore().get_course_index_info(new_locator)
         self.assertEqual(index_info['org'], 'test_org')
-        self.assertEqual(index_info['prettyid'], 'test_course')
         self.assertGreaterEqual(index_info["edited_on"], pre_time)
         self.assertLessEqual(index_info["edited_on"], datetime.datetime.now(UTC))
         self.assertEqual(index_info['edited_by'], 'create_user')
@@ -963,7 +956,7 @@ class TestCourseCreation(SplitModuleTest):
         original_locator = CourseLocator(package_id="wonderful", branch='draft')
         original_index = modulestore().get_course_index_info(original_locator)
         new_draft = modulestore().create_course(
-            'best', 'leech', 'best_course', 'leech_master',
+            'best', 'leech', 'leech_master',
             versions_dict=original_index['versions'])
         new_draft_locator = new_draft.location
         self.assertRegexpMatches(new_draft_locator.package_id, 'best')
@@ -1028,7 +1021,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', 'derivative', 'leech_master',
+            'counter', 'leech', 'leech_master',
             versions_dict={'draft': original_index['versions']['draft']},
             fields=fields
         )
@@ -1061,11 +1054,9 @@ class TestCourseCreation(SplitModuleTest):
         self.assertEqual(course_info['org'], 'funkyU')
 
         course_info['org'] = 'moreFunky'
-        course_info['prettyid'] = 'Ancient Greek Demagods'
         modulestore().update_course_index(course_info)
         course_info = modulestore().get_course_index_info(locator)
         self.assertEqual(course_info['org'], 'moreFunky')
-        self.assertEqual(course_info['prettyid'], 'Ancient Greek Demagods')
 
         # an allowed but not necessarily recommended way to revert the draft version
         versions = course_info['versions']
@@ -1086,7 +1077,7 @@ class TestCourseCreation(SplitModuleTest):
         """
         user = random.getrandbits(32)
         new_course = modulestore().create_course(
-            'test_org.test_transaction', 'test_org', 'test_transaction', user,
+            'test_org.test_transaction', 'test_org', user,
             root_block_id='top', root_category='chapter'
         )
         self.assertEqual(new_course.location.block_id, 'top')

common/test/data/splitmongo_json/active_versions.json

 [{"_id" : "GreekHero", 
   "org" : "testx", 
-  "prettyid" : "test_course", 
   "versions" : {
     "draft" : { "$oid" : "1d00000000000000dddd0000" }
    },
@@ -9,7 +8,6 @@
 
  {"_id" : "wonderful", 
   "org" : "testx", 
-  "prettyid" : "another_course", 
   "versions" : {
     "draft" : { "$oid" : "1d00000000000000dddd2222" },
     "published" : { "$oid" : "1d00000000000000eeee0000" }
@@ -19,7 +17,6 @@
   
  {"_id" : "contender", 
   "org" : "guestx", 
-  "prettyid" : "test_course", 
   "versions" : {
         "draft" : { "$oid" : "1d00000000000000dddd5555" }},
   "edited_on" : {"$date" : 1364491313238},