Merge pull request #4513 from edx/split/auto_publish

Split/auto publish

cms/djangoapps/contentstore/tests/utils.py

@@ -9,10 +9,11 @@ from django.test.client import Client
 from django.contrib.auth.models import User
 
 from xmodule.contentstore.django import contentstore
-from xmodule.modulestore import PublishState, ModuleStoreEnum, mongo
+from xmodule.modulestore import PublishState, ModuleStoreEnum
 from xmodule.modulestore.inheritance import own_metadata
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES
 from xmodule.modulestore.xml_importer import import_from_xml
 from student.models import Registration
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation
@@ -262,23 +263,15 @@ class CourseTestCase(ModuleStoreTestCase):
                     self.store.compute_publish_state(course2_item)
                 )
             except AssertionError:
-                # TODO LMS-11017 "Studio auto-publish course-wide features and settings"
-                # Temporary hack until autopublish implemented - right now, because we call
-                # update_item within create_course to set the wiki & other course-wide settings,
-                # the publish version does not necessarily equal the draft version in split.
-                # So if either item is in Split, just continue on
-                if not isinstance(course1_item.runtime.modulestore, SplitMongoModuleStore) and \
-                   not isinstance(course2_item.runtime.modulestore, SplitMongoModuleStore):
-                    # old mongo calls things draft if draft exists even if it's != published; so, do more work
-                    c1_state = self.compute_real_state(course1_item)
-                    c2_state = self.compute_real_state(course2_item)
-                    self.assertEqual(
-                        c1_state,
-                        c2_state,
-                        "Course item {} in state {} != course item {} in state {}".format(
-                            course1_item, c1_state, course2_item, c2_state
-                        )
+                c1_state = self.compute_real_state(course1_item)
+                c2_state = self.compute_real_state(course2_item)
+                self.assertEqual(
+                    c1_state,
+                    c2_state,
+                    "Publish states not equal: course item {} in state {} != course item {} in state {}".format(
+                        course1_item.location, c1_state, course2_item.location, c2_state
                     )
+                )
 
             # compare data
             self.assertEqual(hasattr(course1_item, 'data'), hasattr(course2_item, 'data'))
@@ -351,7 +344,7 @@ class CourseTestCase(ModuleStoreTestCase):
                 return supposed_state
             # published == item in all respects, so return public
             return PublishState.public
-        elif supposed_state == PublishState.public and item.location.category in mongo.base.DIRECT_ONLY_CATEGORIES:
+        elif supposed_state == PublishState.public and item.location.category in DIRECT_ONLY_CATEGORIES:
             if not all([
                 self.store.has_item(child_loc, revision=ModuleStoreEnum.RevisionOption.draft_only)
                 for child_loc in item.children

cms/djangoapps/contentstore/views/item.py

@@ -23,7 +23,7 @@ import xmodule
 from xmodule.tabs import StaticTab, CourseTabList
 from xmodule.modulestore import PublishState, ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.draft import DIRECT_ONLY_CATEGORIES
+from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES
 from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, DuplicateItemError
 from xmodule.modulestore.inheritance import own_metadata
 from xmodule.x_module import PREVIEW_VIEWS, STUDIO_VIEW

common/lib/xmodule/xmodule/modulestore/draft.py

@@ -4,4 +4,4 @@ Backwards compatibility for old pointers to draft module store
 This modulestore has been moved to xmodule.modulestore.mongo.draft
 """
 
-from xmodule.modulestore.mongo.draft import DIRECT_ONLY_CATEGORIES, DraftModuleStore
+from xmodule.modulestore.mongo.draft import DraftModuleStore

common/lib/xmodule/xmodule/modulestore/draft_and_published.py

@@ -7,6 +7,8 @@ from abc import ABCMeta, abstractmethod
 from contextlib import contextmanager
 from . import ModuleStoreEnum
 
+# Things w/ these categories should never be marked as version=DRAFT
+DIRECT_ONLY_CATEGORIES = ['course', 'chapter', 'sequential', 'about', 'static_tab', 'course_info']
 
 class BranchSettingMixin(object):
     """

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -35,7 +35,7 @@ from xblock.exceptions import InvalidScopeError
 from xblock.fields import Scope, ScopeIds, Reference, ReferenceList, ReferenceValueDict
 
 from xmodule.modulestore import ModuleStoreWriteBase, ModuleStoreEnum
-from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished
+from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished, DIRECT_ONLY_CATEGORIES
 from opaque_keys.edx.locations import Location
 from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, ReferentialIntegrityError
 from xmodule.modulestore.inheritance import own_metadata, InheritanceMixin, inherit_metadata, InheritanceKeyValueStore
@@ -46,10 +46,6 @@ from xmodule.exceptions import HeartbeatFailure
 
 log = logging.getLogger(__name__)
 
-
-# Things w/ these categories should never be marked as version=DRAFT
-DIRECT_ONLY_CATEGORIES = ['course', 'chapter', 'sequential', 'about', 'static_tab', 'course_info']
-
 # sort order that returns DRAFT items first
 SORT_REVISION_FAVOR_DRAFT = ('_id.revision', pymongo.DESCENDING)
 

common/lib/xmodule/xmodule/modulestore/mongo/draft.py

@@ -17,10 +17,10 @@ from xmodule.modulestore.exceptions import (
 )
 from xmodule.modulestore.mongo.base import (
     MongoModuleStore, MongoRevisionKey, as_draft, as_published,
-    DIRECT_ONLY_CATEGORIES, SORT_REVISION_FAVOR_DRAFT
+    SORT_REVISION_FAVOR_DRAFT
 )
 from xmodule.modulestore.store_utilities import rewrite_nonportable_content_links
-from xmodule.modulestore.draft_and_published import UnsupportedRevisionError
+from xmodule.modulestore.draft_and_published import UnsupportedRevisionError, DIRECT_ONLY_CATEGORIES
 
 log = logging.getLogger(__name__)
 

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

@@ -153,6 +153,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         module.edited_on = edit_info.get('edited_on')
         module.previous_version = edit_info.get('previous_version')
         module.update_version = edit_info.get('update_version')
+        module.source_version = edit_info.get('source_version', None)
         module.definition_locator = definition_id
         # decache any pending field settings
         module.save()

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

@@ -3,11 +3,10 @@ Module for the dual-branch fall-back Draft->Published Versioning ModuleStore
 """
 
 from ..exceptions import ItemNotFoundError
-from split import SplitMongoModuleStore
+from split import SplitMongoModuleStore, EXCLUDE_ALL
 from xmodule.modulestore import ModuleStoreEnum, PublishState
-from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished, UnsupportedRevisionError
-from xmodule.modulestore.draft import DIRECT_ONLY_CATEGORIES
 from xmodule.modulestore.exceptions import InsufficientSpecificationError
+from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished, DIRECT_ONLY_CATEGORIES, UnsupportedRevisionError
 
 
 class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleStore):
@@ -46,9 +45,11 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
         Returns: a CourseDescriptor
         """
         master_branch = kwargs.pop('master_branch', ModuleStoreEnum.BranchName.draft)
-        return super(DraftVersioningModuleStore, self).create_course(
+        item = super(DraftVersioningModuleStore, self).create_course(
             org, course, run, user_id, master_branch=master_branch, **kwargs
         )
+        self._auto_publish_no_children(item.location, item.location.category, user_id)
+        return item
 
     def get_courses(self):
         """
@@ -56,6 +57,51 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
         """
         return super(DraftVersioningModuleStore, self).get_courses(ModuleStoreEnum.BranchName.draft)
 
+    def _auto_publish_no_children(self, location, category, user_id):
+        """
+        Publishes item if the category is DIRECT_ONLY. This assumes another method has checked that
+        location points to the head of the branch and ignores the version. If you call this in any
+        other context, you may blow away another user's changes.
+        NOTE: only publishes the item at location: no children get published.
+        """
+        if location.branch == ModuleStoreEnum.BranchName.draft and category in DIRECT_ONLY_CATEGORIES:
+            # version_agnostic b/c of above assumption in docstring
+            self.publish(location.version_agnostic(), user_id, blacklist=EXCLUDE_ALL)
+
+    def update_item(self, descriptor, user_id, allow_not_found=False, force=False):
+        item = super(DraftVersioningModuleStore, self).update_item(
+            descriptor,
+            user_id,
+            allow_not_found=allow_not_found,
+            force=force
+        )
+        self._auto_publish_no_children(item.location, item.location.category, user_id)
+        return item
+
+    def create_item(
+        self, user_id, course_key, block_type, block_id=None,
+        definition_locator=None, fields=None,
+        force=False, continue_version=False, **kwargs
+    ):
+        item = super(DraftVersioningModuleStore, self).create_item(
+            user_id, course_key, block_type, block_id=block_id,
+            definition_locator=definition_locator, fields=fields,
+            force=force, continue_version=continue_version, **kwargs
+        )
+        self._auto_publish_no_children(item.location, item.location.category, user_id)
+        return item
+
+    def create_child(
+            self, user_id, parent_usage_key, block_type, block_id=None,
+            fields=None, continue_version=False, **kwargs
+    ):
+        item = super(DraftVersioningModuleStore, self).create_child(
+            user_id, parent_usage_key, block_type, block_id=block_id,
+            fields=fields, continue_version=continue_version, **kwargs
+        )
+        self._auto_publish_no_children(parent_usage_key, item.location.category, user_id)
+        return item
+
     def delete_item(self, location, user_id, revision=None, **kwargs):
         """
         Delete the given item from persistence. kwargs allow modulestore specific parameters.
@@ -86,7 +132,10 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
             )
 
         for branch in branches_to_delete:
-            SplitMongoModuleStore.delete_item(self, location.for_branch(branch), user_id, **kwargs)
+            branched_location = location.for_branch(branch)
+            parent_loc = self.get_parent_location(branched_location)
+            SplitMongoModuleStore.delete_item(self, branched_location, user_id, **kwargs)
+            self._auto_publish_no_children(parent_loc, parent_loc.category, user_id)
 
     def _map_revision_to_branch(self, key, revision=None):
         """
@@ -158,20 +207,24 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
         except ItemNotFoundError:
             return True
 
-        return draft.update_version != published.update_version
+        return draft.update_version != published.source_version
 
-    def publish(self, location, user_id, **kwargs):
+    def publish(self, location, user_id, blacklist=None, **kwargs):
         """
-        Save a current draft to the underlying modulestore.
+        Publishes the subtree under location from the draft branch to the published branch
         Returns the newly published item.
         """
         SplitMongoModuleStore.copy(
             self,
             user_id,
-            location.course_key.for_branch(ModuleStoreEnum.BranchName.draft),
+            # Directly using the replace function rather than the for_branch function
+            # because for_branch obliterates the version_guid and will lead to missed version conflicts.
+            location.course_key.replace(branch=ModuleStoreEnum.BranchName.draft),
             location.course_key.for_branch(ModuleStoreEnum.BranchName.published),
             [location],
+            blacklist=blacklist
         )
+        return self.get_item(location.for_branch(ModuleStoreEnum.BranchName.published))
 
     def unpublish(self, location, user_id):
         """
@@ -211,7 +264,7 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
             Return the version of the given database representation of a block.
             """
             #TODO: make this method a more generic helper
-            return block['edit_info']['update_version']
+            return block['edit_info'].get('source_version', block['edit_info']['update_version'])
 
         draft_head = get_head(ModuleStoreEnum.BranchName.draft)
         published_head = get_head(ModuleStoreEnum.BranchName.published)

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

@@ -508,56 +508,6 @@ class TestMongoModuleStore(unittest.TestCase):
         finally:
             shutil.rmtree(root_dir)
 
-    def test_has_changes_direct_only(self):
-        """
-        Tests that has_changes() returns false when a new xblock in a direct only category is checked
-        """
-        course_location = Location('edX', 'toy', '2012_Fall', 'course', '2012_Fall')
-        chapter_location = Location('edX', 'toy', '2012_Fall', 'chapter', 'vertical_container')
-
-        # Create dummy direct only xblocks
-        self.draft_store.create_item(
-            self.dummy_user,
-            chapter_location.course_key,
-            chapter_location.block_type,
-            block_id=chapter_location.block_id
-        )
-
-        # Check that neither xblock has changes
-        self.assertFalse(self.draft_store.has_changes(course_location))
-        self.assertFalse(self.draft_store.has_changes(chapter_location))
-
-    def test_has_changes(self):
-        """
-        Tests that has_changes() only returns true when changes are present
-        """
-        location = Location('edX', 'toy', '2012_Fall', 'vertical', 'test_vertical')
-
-        # Create a dummy component to test against
-        self.draft_store.create_item(
-            self.dummy_user,
-            location.course_key,
-            location.block_type,
-            block_id=location.block_id
-        )
-
-        # Not yet published, so changes are present
-        self.assertTrue(self.draft_store.has_changes(location))
-
-        # Publish and verify that there are no unpublished changes
-        self.draft_store.publish(location, self.dummy_user)
-        self.assertFalse(self.draft_store.has_changes(location))
-
-        # Change the component, then check that there now are changes
-        component = self.draft_store.get_item(location)
-        component.display_name = 'Changed Display Name'
-        self.draft_store.update_item(component, self.dummy_user)
-        self.assertTrue(self.draft_store.has_changes(location))
-
-        # Publish and verify again
-        self.draft_store.publish(location, self.dummy_user)
-        self.assertFalse(self.draft_store.has_changes(location))
-
     def test_has_changes_missing_child(self):
         """
         Tests that has_changes() returns False when a published parent points to a child that doesn't exist.

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

-"""
-    Test split_draft modulestore
-"""
-import unittest
-import uuid
-from xmodule.modulestore.split_mongo.split_draft import DraftVersioningModuleStore
-from xmodule.modulestore import ModuleStoreEnum
-from opaque_keys.edx.locator import CourseLocator
-from xmodule.modulestore.inheritance import InheritanceMixin
-from xmodule.x_module import XModuleMixin
-from xmodule.modulestore.tests.test_split_modulestore import SplitModuleTest
-
-# pylint: disable=W0613
-def render_to_template_mock(*args):
-    pass
-
-
-class TestDraftVersioningModuleStore(unittest.TestCase):
-    def setUp(self):
-        super(TestDraftVersioningModuleStore, self).setUp()
-        self.module_store = DraftVersioningModuleStore(
-            contentstore=None,
-            doc_store_config={
-                'host': 'localhost',
-                'db': 'test_xmodule',
-                'collection': 'modulestore{0}'.format(uuid.uuid4().hex[:5]),
-            },
-            fs_root='',
-            default_class='xmodule.raw_module.RawDescriptor',
-            render_template=render_to_template_mock,
-            xblock_mixins=(InheritanceMixin, XModuleMixin),
-        )
-        self.addCleanup(self.module_store._drop_database)
-
-        SplitModuleTest.bootstrapDB(self.module_store)
-
-    def test_has_changes(self):
-        """
-        Tests that has_changes() only returns true when changes are present
-        """
-        draft_course = CourseLocator(
-            org='testx', course='GreekHero', run='run', branch=ModuleStoreEnum.BranchName.draft
-        )
-        head = draft_course.make_usage_key('course', 'head12345')
-        dummy_user = ModuleStoreEnum.UserID.test
-
-        # Not yet published, so changes are present
-        self.assertTrue(self.module_store.has_changes(head))
-
-        # Publish and verify that there are no unpublished changes
-        self.module_store.publish(head, dummy_user)
-        self.assertFalse(self.module_store.has_changes(head))
-
-        # Change the course, then check that there now are changes
-        course = self.module_store.get_item(head)
-        course.show_calculator = not course.show_calculator
-        self.module_store.update_item(course, dummy_user)
-        self.assertTrue(self.module_store.has_changes(head))
-
-        # Publish and verify again
-        self.module_store.publish(head, dummy_user)
-        self.assertFalse(self.module_store.has_changes(head))

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

@@ -1687,7 +1687,12 @@ class TestPublish(SplitModuleTest):
             pub_copy = modulestore().get_item(dest_course_loc.make_usage_key("", expected))
             # everything except previous_version & children should be the same
             self.assertEqual(source.category, pub_copy.category)
-            self.assertEqual(source.update_version, pub_copy.update_version)
+            self.assertEqual(
+                source.update_version, pub_copy.source_version,
+                u"Versions don't match for {}: {} != {}".format(
+                    expected, source.update_version, pub_copy.update_version
+                )
+            )
             self.assertEqual(
                 self.user_id, pub_copy.edited_by,
                 "{} edited_by {} not {}".format(pub_copy.location, pub_copy.edited_by, self.user_id)

common/lib/xmodule/xmodule/modulestore/xml_exporter.py

@@ -15,7 +15,7 @@ import json
 import os
 from path import path
 import shutil
-from xmodule.modulestore.mongo.base import DIRECT_ONLY_CATEGORIES
+from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES
 
 DRAFT_DIR = "drafts"
 PUBLISHED_DIR = "published"