more split_draft methods

cms/djangoapps/contentstore/views/item.py

@@ -529,7 +529,7 @@ def orphan_handler(request, course_key_string):
     course_usage_key = CourseKey.from_string(course_key_string)
     if request.method == 'GET':
         if has_course_access(request.user, course_usage_key):
-            return JsonResponse(modulestore().get_orphans(course_usage_key))
+            return JsonResponse([unicode(item) for item in modulestore().get_orphans(course_usage_key)])
         else:
             raise PermissionDenied()
     if request.method == 'DELETE':
@@ -539,7 +539,7 @@ def orphan_handler(request, course_key_string):
             for itemloc in items:
                 # need to delete all versions
                 store.delete_item(itemloc, request.user.id, revision=ModuleStoreEnum.RevisionOption.all)
-            return JsonResponse({'deleted': items})
+            return JsonResponse({'deleted': [unicode(item) for item in items]})
         else:
             raise PermissionDenied()
 

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

@@ -84,7 +84,9 @@ class SplitMigrator(object):
                 # it doesn't need the parent as the first arg. That is, it translates and populates
                 # the 'children' field as it goes.
                 _new_module = self.split_modulestore.create_item(
-                    user.id, new_locator, parent_location=course_version_locator,
+                    user_id,
+                    course_version_locator.make_usage_key(module.location.category, module.location.block_id),
+                    parent_location=course_version_locator,
                     block_id=module.location.block_id,
                     fields=self._get_json_fields_translate_references(
                         module, course_version_locator, new_course.location.block_id
@@ -129,11 +131,7 @@ class SplitMigrator(object):
             else:
                 # only a draft version (aka, 'private').
                 _new_module = self.split_modulestore.create_item(
-<<<<<<< HEAD
-                    new_draft_course_loc, module.category, user_id,
-=======
-                    user.id, new_locator, parent_location=new_draft_course_loc,
->>>>>>> converge create_item.
+                    user_id, new_locator, parent_location=new_draft_course_loc,
                     block_id=new_locator.block_id,
                     fields=self._get_json_fields_translate_references(
                         module, new_draft_course_loc, published_course_usage_key.block_id

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

@@ -41,6 +41,20 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
         for branch in branches_to_delete:
             SplitMongoModuleStore.delete_item(self, location.for_branch(branch), user_id, **kwargs)
 
+    def get_item(self, usage_key, depth=0, revision=None):
+        """
+        Returns the item identified by usage_key and revision.
+        """
+        def for_branch(branch_state):
+            if usage_key.branch is not None and usage_key.branch is not branch_state:
+                raise ValueError('{} already has a branch.'.format(usage_key))
+            return usage_key.for_branch(branch_state)
+        if revision == ModuleStoreEnum.RevisionOption.published_only:
+            usage_key = for_branch(ModuleStoreEnum.BranchName.published)
+        elif revision == ModuleStoreEnum.RevisionOption.draft_only:
+            usage_key = for_branch(ModuleStoreEnum.BranchName.draft)
+        return super(DraftVersioningModuleStore, self).get_item(usage_key, depth=depth)
+
     def get_parent_location(self, location, revision=None, **kwargs):
         # NAATODO - support draft_preferred
         return SplitMongoModuleStore.get_parent_location(self, location, **kwargs)
@@ -77,7 +91,7 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
         Deletes the published version of the item.
         Returns the newly unpublished item.
         """
-        self.delete_item(location.for_branch(ModuleStoreEnum.BranchName.published), user_id)
+        self.delete_item(location, user_id, revision=ModuleStoreEnum.RevisionOption.published_only)
         return self.get_item(location.for_branch(ModuleStoreEnum.BranchName.draft))
 
     def revert_to_published(self, location, user_id):

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

@@ -8,7 +8,7 @@ import unittest
 
 from xmodule.tests import DATA_DIR
 from opaque_keys.edx.locations import Location
-from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore import ModuleStoreEnum, PublishState
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.exceptions import InvalidVersionError
 
@@ -117,22 +117,20 @@ class TestMixedModuleStore(unittest.TestCase):
         """
         Create a course w/ one item in the persistence store using the given course & item location.
         """
-        course = self.store.create_course(course_key.org, course_key.course, course_key.run, self.user_id)
+        self.course = self.store.create_course(course_key.org, course_key.course, course_key.run, self.user_id)
+        self.writable_chapter_location = self.course.id.make_usage_key('chapter', 'Overview').version_agnostic()
         block_id = self.writable_chapter_location.name
         chapter = self.store.create_item(
             # don't use course_location as it may not be the repr
             self.user_id, self.writable_chapter_location,
-            parent_location=course.location, block_id=block_id
+            parent_location=self.course.location, block_id=block_id
         )
-        if isinstance(course.id, CourseLocator):
-            self.course_locations[self.MONGO_COURSEID] = course.location.version_agnostic()
-            self.writable_chapter_location = chapter.location.version_agnostic()
+        if isinstance(self.course.id, CourseLocator):
+            self.course_locations[self.MONGO_COURSEID] = self.course.location.version_agnostic()
         else:
-            self.assertEqual(course.id, course_key)
+            self.assertEqual(self.course.id, course_key)
             self.assertEqual(chapter.location, self.writable_chapter_location)
 
-        self.course = course
-
     def _create_block_hierarchy(self):
         """
         Creates a hierarchy of blocks for testing
@@ -150,6 +148,7 @@ class TestMixedModuleStore(unittest.TestCase):
                                     BlockInfo('problem_x1a_1', 'problem', 'Problem_x1a_1', []),
                                     BlockInfo('problem_x1a_2', 'problem', 'Problem_x1a_2', []),
                                     BlockInfo('problem_x1a_3', 'problem', 'Problem_x1a_3', []),
+                                    BlockInfo('html_x1a_1', 'html', 'HTML_x1a_1', []),
                                 ]
                             )
                         ]
@@ -175,13 +174,13 @@ class TestMixedModuleStore(unittest.TestCase):
 
         def create_sub_tree(parent, block_info):
             block = self.store.create_item(
-                self.user_id, parent_location=parent.location,
+                self.user_id, parent_location=parent.location.version_agnostic(),
                 category=block_info.category, block_id=block_info.display_name
             )
             for tree in block_info.sub_tree:
                 create_sub_tree(block, tree)
             # reload the block to update its children field
-            block = self.store.get_item(block.location)
+            block = self.store.get_item(block.location.version_agnostic())
             setattr(self, block_info.field_name, block)
 
         for tree in trees:
@@ -209,7 +208,7 @@ class TestMixedModuleStore(unittest.TestCase):
 
         # convert to CourseKeys
         self.course_locations = {
-            course_id: SlashSeparatedCourseKey.from_deprecated_string(course_id)
+            course_id: CourseLocator.from_string(course_id)
             for course_id in [self.MONGO_COURSEID, self.XML_COURSEID1, self.XML_COURSEID2]
         }
         # and then to the root UsageKey
@@ -223,9 +222,6 @@ class TestMixedModuleStore(unittest.TestCase):
             ).make_usage_key('vertical', 'baz')
         else:
             self.fake_location = Location('foo', 'bar', 'slowly', 'vertical', 'baz')
-        self.writable_chapter_location = self.course_locations[self.MONGO_COURSEID].replace(
-            category='chapter', name='Overview'
-        )
         self.xml_chapter_location = self.course_locations[self.XML_COURSEID1].replace(
             category='chapter', name='Overview'
         )
@@ -321,7 +317,7 @@ class TestMixedModuleStore(unittest.TestCase):
         self.store.delete_item(self.writable_chapter_location, self.user_id)
         # verify it's gone
         with self.assertRaises(ItemNotFoundError):
-            self.store.get_item(self.writable_chapter_location)
+            self.store.get_item(self.writable_chapter_location.version_agnostic())
 
         # create and delete a private vertical with private children
         private_vert = self.store.create_item(
@@ -520,22 +516,24 @@ class TestMixedModuleStore(unittest.TestCase):
         """
         self.initdb(default_ms)
         self._create_block_hierarchy()
+        vertical_children_num = len(self.vertical_x1a.children)
+
         self.store.publish(self.course.location, self.user_id)
 
         # delete leaf problem (will make parent vertical a draft)
         self.store.delete_item(self.problem_x1a_1.location, self.user_id)
 
         draft_parent = self.store.get_item(self.vertical_x1a.location)
-        self.assertEqual(2, len(draft_parent.children))
+        self.assertEqual(vertical_children_num - 1, len(draft_parent.children))
         published_parent = self.store.get_item(
             self.vertical_x1a.location,
             revision=ModuleStoreEnum.RevisionOption.published_only
         )
-        self.assertEqual(3, len(published_parent.children))
+        self.assertEqual(vertical_children_num, len(published_parent.children))
 
         self.store.revert_to_published(self.vertical_x1a.location, self.user_id)
         reverted_parent = self.store.get_item(self.vertical_x1a.location)
-        self.assertEqual(3, len(published_parent.children))
+        self.assertEqual(vertical_children_num, len(published_parent.children))
         self.assertEqual(reverted_parent, published_parent)
 
     @ddt.data('draft')
@@ -596,12 +594,28 @@ class TestMixedModuleStore(unittest.TestCase):
     @ddt.data('draft', 'split')
     def test_get_orphans(self, default_ms):
         self.initdb(default_ms)
-        # create an orphan
         course_id = self.course_locations[self.MONGO_COURSEID].course_key
-        orphan_location = course_id.make_usage_key('problem', 'orphan')
-        orphan = self.store.create_item(self.user_id, orphan_location)
+
+        # orphans
+        orphan_locations = [
+            course_id.make_usage_key('chapter', 'OrphanChapter'),
+            course_id.make_usage_key('vertical', 'OrphanVertical'),
+            course_id.make_usage_key('problem', 'OrphanProblem'),
+            course_id.make_usage_key('html', 'OrphanHTML'),
+        ]
+
+        # detached items (not considered as orphans)
+        detached_locations = [
+            course_id.make_usage_key('static_tab', 'StaticTab'),
+            course_id.make_usage_key('about', 'overview'),
+            course_id.make_usage_key('course_info', 'updates'),
+        ]
+
+        for location in (orphan_locations + detached_locations):
+            self.store.create_item(self.user_id, location)
+
         found_orphans = self.store.get_orphans(self.course_locations[self.MONGO_COURSEID].course_key)
-        self.assertEqual(found_orphans, [orphan_location])
+        self.assertEqual(set(found_orphans), set(orphan_locations))
 
     @ddt.data('draft')
     def test_create_item_from_parent_location(self, default_ms):
@@ -634,6 +648,38 @@ class TestMixedModuleStore(unittest.TestCase):
         self.assertEqual(len(self.store.get_courses_for_wiki('edX.simple.2012_Fall')), 0)
         self.assertEqual(len(self.store.get_courses_for_wiki('no_such_wiki')), 0)
 
+    @ddt.data('draft', 'split')
+    def test_unpublish(self, default_ms):
+        """
+        Test calling unpublish
+        """
+        self.initdb(default_ms)
+        self._create_block_hierarchy()
+        # publish
+        self.store.publish(self.course.location, self.user_id)
+        published_xblock = self.store.get_item(
+            self.vertical_x1a.location.replace(branch=None),
+            revision=ModuleStoreEnum.RevisionOption.published_only
+        )
+        self.assertIsNotNone(published_xblock)
+
+        # unpublish
+        self.store.unpublish(self.vertical_x1a.location, self.user_id)
+
+        with self.assertRaises(ItemNotFoundError):
+            self.store.get_item(
+                self.vertical_x1a.location.replace(branch=None),
+                revision=ModuleStoreEnum.RevisionOption.published_only
+            )
+
+        draft_xblock_from_get_item = self.store.get_item(
+            self.vertical_x1a.location.replace(branch=None),
+            revision=ModuleStoreEnum.RevisionOption.draft_only
+        )
+        self.assertEquals(
+            published_xblock.location.version_agnostic().replace(branch=None),
+            draft_xblock_from_get_item.location.version_agnostic().replace(branch=None)
+        )
 
 #=============================================================================================================
 # General utils for not using django settings

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

@@ -541,7 +541,7 @@ class TestMongoModuleStore(unittest.TestCase):
         location = Location('edX', 'missing', '2012_Fall', 'sequential', 'parent')
 
         # Create the parent and point it to a fake child
-        parent = self.draft_store.create_and_save_xmodule(location, user_id=self.dummy_user)
+        parent = self.draft_store.create_item(self.dummy_user, location)
         parent.children += [Location('edX', 'missing', '2012_Fall', 'vertical', 'does_not_exist')]
         self.draft_store.update_item(parent, self.dummy_user)
 

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

-from xmodule.modulestore.tests.test_split_w_old_mongo import SplitWMongoCourseBoostrapper
-
-
-class TestOrphan(SplitWMongoCourseBoostrapper):
-    """
-    Test the orphan finding code
-    """
-
-    def _create_course(self):
-        """
-        * some detached items
-        * some attached children
-        * some orphans
-        """
-        super(TestOrphan, self)._create_course()
-
-        self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', 'runid')
-        self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', 'runid')
-        self._create_item('chapter', 'OrphanChapter', {}, {'display_name': 'Orphan Chapter'}, None, None)
-        self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1')
-        self._create_item('vertical', 'OrphanVert', {}, {'display_name': 'Orphan Vertical'}, None, None)
-        self._create_item('html', 'Html1', "<p>Goodbye</p>", {'display_name': 'Parented Html'}, 'vertical', 'Vert1')
-        self._create_item('html', 'OrphanHtml', "<p>Hello</p>", {'display_name': 'Orphan html'}, None, None)
-        self._create_item('static_tab', 'staticuno', "<p>tab</p>", {'display_name': 'Tab uno'}, None, None)
-        self._create_item('about', 'overview', "<p>overview</p>", {}, None, None)
-        self._create_item('course_info', 'updates', "<ol><li><h2>Sep 22</h2><p>test</p></li></ol>", {}, None, None)
-
-    def test_mongo_orphan(self):
-        """
-        Test that old mongo finds the orphans
-        """
-        orphans = self.draft_mongo.get_orphans(self.old_course_key)
-        self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans))
-        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', '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)
-
-    def test_split_orphan(self):
-        """
-        Test that split mongo finds the orphans
-        """
-        orphans = self.split_mongo.get_orphans(self.split_course_key)
-        self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans))
-        location = self.split_course_key.make_usage_key('chapter', 'OrphanChapter')
-        self.assertIn(location, orphans)
-        location = self.split_course_key.make_usage_key('vertical', 'OrphanVert')
-        self.assertIn(location, orphans)
-        location = self.split_course_key.make_usage_key('html', 'OrphanHtml')
-        self.assertIn(location, orphans)

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

@@ -86,8 +86,8 @@ class SplitWMongoCourseBoostrapper(unittest.TestCase):
         existing draft for both the new item and the parent
         """
         location = self.old_course_key.make_usage_key(category, name)
-        self.draft_mongo.create_and_save_xmodule(
-            location, self.user_id, definition_data=data, metadata=metadata, runtime=self.runtime
+        self.draft_mongo.create_item(
+            self.user_id, location, definition_data=data, metadata=metadata, runtime=self.runtime
         )
         if not draft:
             self.draft_mongo.publish(location, self.user_id)

common/lib/xmodule/xmodule/split_test_module.py

@@ -569,7 +569,7 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes
 
         This appends the new vertical to the end of children, and updates group_id_to_child.
         A mutable modulestore is needed to call this method (will need to update after mixed
-        modulestore work, currently relies on mongo's create_and_save_xmodule method).
+        modulestore work, currently relies on mongo's create_item method).
         """
         assert hasattr(self.system, 'modulestore') and hasattr(self.system.modulestore, 'create_item'), \
             "editor_saved should only be called when a mutable modulestore is available"