Merge pull request #5001 from edx/split/edit_info

Split/edit info

cms/djangoapps/contentstore/views/item.py

@@ -669,7 +669,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F
         "category": xblock.category,
         "edited_on": get_default_time_display(xblock.subtree_edited_on) if xblock.subtree_edited_on else None,
         "published": published,
-        "published_on": get_default_time_display(xblock.published_date) if xblock.published_date else None,
+        "published_on": get_default_time_display(xblock.published_on) if xblock.published_on else None,
         "studio_url": xblock_studio_url(xblock, parent_xblock),
         "released_to_students": datetime.now(UTC) > xblock.start,
         "release_date": release_date,

cms/envs/common.py

@@ -38,6 +38,7 @@ from warnings import simplefilter
 
 from lms.lib.xblock.mixin import LmsBlockMixin
 from dealer.git import git
+from xmodule.modulestore.edit_info import EditInfoMixin
 
 ############################ FEATURE CONFIGURATION #############################
 
@@ -254,7 +255,7 @@ from xmodule.x_module import XModuleMixin
 
 # This should be moved into an XBlock Runtime/Application object
 # once the responsibility of XBlock creation is moved out of modulestore - cpennington
-XBLOCK_MIXINS = (LmsBlockMixin, InheritanceMixin, XModuleMixin)
+XBLOCK_MIXINS = (LmsBlockMixin, InheritanceMixin, XModuleMixin, EditInfoMixin)
 
 # Allow any XBlock in Studio
 # You should also enable the ALLOW_ALL_ADVANCED_COMPONENTS feature flag, so that

common/lib/xmodule/xmodule/modulestore/edit_info.py

@@ -0,0 +1,101 @@
+"""
+Access methods to get EditInfo for xblocks
+"""
+from xblock.fields import XBlockMixin
+from abc import ABCMeta, abstractmethod
+
+
+class EditInfoMixin(XBlockMixin):
+    """
+    Provides the interfaces for getting the edit info from XBlocks
+    """
+    @property
+    def edited_by(self):
+        """
+        The user id of the last user to change this xblock content, children, or settings.
+        """
+        return self.runtime.get_edited_by(self)
+
+    @property
+    def edited_on(self):
+        """
+        The datetime of the last change to this xblock content, children, or settings.
+        """
+        return self.runtime.get_edited_on(self)
+
+    @property
+    def subtree_edited_by(self):
+        """
+        The user id of the last user to change content, children, or settings in this xblock's subtree
+        """
+        return self.runtime.get_subtree_edited_by(self)
+
+    @property
+    def subtree_edited_on(self):
+        """
+        The datetime of the last change content, children, or settings in this xblock's subtree
+        """
+        return self.runtime.get_subtree_edited_on(self)
+
+    @property
+    def published_by(self):
+        """
+        The user id of the last user to publish this specific xblock (or a previous version of it).
+        """
+        return self.runtime.get_published_by(self)
+
+    @property
+    def published_on(self):
+        """
+        The datetime of the last time this specific xblock was published.
+        """
+        return self.runtime.get_published_on(self)
+
+
+class EditInfoRuntimeMixin(object):
+    """
+    An abstract mixin class for the functions which the :class: `EditInfoMixin` methods call on the runtime
+    """
+    __metaclass__ = ABCMeta
+
+    @abstractmethod
+    def get_edited_by(self, xblock):
+        """
+        The datetime of the last change to this xblock content, children, or settings.
+        """
+        pass
+
+    @abstractmethod
+    def get_edited_on(self, xblock):
+        """
+        The datetime of the last change to this xblock content, children, or settings.
+        """
+        pass
+
+    @abstractmethod
+    def get_subtree_edited_by(self, xblock):
+        """
+        The user id of the last user to change content, children, or settings in this xblock's subtree
+        """
+        pass
+
+    @abstractmethod
+    def get_subtree_edited_on(self, xblock):
+        """
+        The datetime of the last change content, children, or settings in this xblock's subtree
+        """
+        pass
+
+    @abstractmethod
+    def get_published_by(self, xblock):
+        """
+        The user id of the last user to publish this specific xblock (or a previous version of it).
+        """
+        pass
+
+    @abstractmethod
+    def get_published_on(self, xblock):
+        """
+        The datetime of the last time this specific xblock was published.
+        """
+        pass

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

@@ -44,6 +44,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from opaque_keys.edx.locator import CourseLocator
 from opaque_keys.edx.keys import UsageKey, CourseKey
 from xmodule.exceptions import HeartbeatFailure
+from xmodule.modulestore.edit_info import EditInfoRuntimeMixin
 
 log = logging.getLogger(__name__)
 
@@ -137,7 +138,7 @@ class MongoKeyValueStore(InheritanceKeyValueStore):
             return False
 
 
-class CachingDescriptorSystem(MakoDescriptorSystem):
+class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
     """
     A system that has a cache of module json that it will use to load modules
     from, with a backup of calling to the underlying modulestore for more data
@@ -233,25 +234,16 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
                     metadata_to_inherit = self.cached_metadata.get(unicode(non_draft_loc), {})
                     inherit_metadata(module, metadata_to_inherit)
 
-                edit_info = json_data.get('edit_info')
+                module._edit_info = json_data.get('edit_info')
 
-                # migrate published_by and published_date if edit_info isn't present
+                # migrate published_by and published_on if edit_info isn't present
-                if not edit_info:
+                if module._edit_info is None:
-                    module.edited_by = module.edited_on = module.subtree_edited_on = \
+                    module._edit_info = {}
-                        module.subtree_edited_by = module.published_date = None
                     raw_metadata = json_data.get('metadata', {})
-                    # published_date was previously stored as a list of time components instead of a datetime
+                    # published_on was previously stored as a list of time components instead of a datetime
                     if raw_metadata.get('published_date'):
-                        module.published_date = datetime(*raw_metadata.get('published_date')[0:6]).replace(tzinfo=UTC)
+                        module._edit_info['published_date'] = datetime(*raw_metadata.get('published_date')[0:6]).replace(tzinfo=UTC)
-                    module.published_by = raw_metadata.get('published_by')
+                    module._edit_info['published_by'] = raw_metadata.get('published_by')
-                # otherwise restore the stored editing information
-                else:
-                    module.edited_by = edit_info.get('edited_by')
-                    module.edited_on = edit_info.get('edited_on')
-                    module.subtree_edited_on = edit_info.get('subtree_edited_on')
-                    module.subtree_edited_by = edit_info.get('subtree_edited_by')
-                    module.published_date = edit_info.get('published_date')
-                    module.published_by = edit_info.get('published_by')
 
                 # decache any computed pending field settings
                 module.save()
@@ -316,6 +308,42 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
 
         return json
 
+    def get_edited_by(self, xblock):
+        """
+        See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
+        """
+        return xblock._edit_info.get('edited_by')
+
+    def get_edited_on(self, xblock):
+        """
+        See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
+        """
+        return xblock._edit_info.get('edited_on')
+
+    def get_subtree_edited_by(self, xblock):
+        """
+        See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
+        """
+        return xblock._edit_info.get('subtree_edited_by')
+
+    def get_subtree_edited_on(self, xblock):
+        """
+        See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
+        """
+        return xblock._edit_info.get('subtree_edited_on')
+
+    def get_published_by(self, xblock):
+        """
+        See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
+        """
+        return xblock._edit_info.get('published_by')
+
+    def get_published_on(self, xblock):
+        """
+        See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
+        """
+        return xblock._edit_info.get('published_date')
+
 
 # The only thing using this w/ wildcards is contentstore.mongo for asset retrieval
 def location_to_query(location, wildcard=True, tag='i4x'):
@@ -1153,15 +1181,20 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
             payload = {
                 'definition.data': definition_data,
                 'metadata': self._serialize_scope(xblock, Scope.settings),
-                'edit_info.edited_on': now,
+                'edit_info': {
-                'edit_info.edited_by': user_id,
+                    'edited_on': now,
-                'edit_info.subtree_edited_on': now,
+                    'edited_by': user_id,
-                'edit_info.subtree_edited_by': user_id,
+                    'subtree_edited_on': now,
+                    'subtree_edited_by': user_id,
+                }
             }
 
             if isPublish:
-                payload['edit_info.published_date'] = now
+                payload['edit_info']['published_date'] = now
-                payload['edit_info.published_by'] = user_id
+                payload['edit_info']['published_by'] = user_id
+            elif 'published_date' in getattr(xblock, '_edit_info', {}):
+                payload['edit_info']['published_date'] = xblock._edit_info['published_date']
+                payload['edit_info']['published_by'] = xblock._edit_info['published_by']
 
             if xblock.has_children:
                 children = self._serialize_scope(xblock, Scope.children)
@@ -1181,17 +1214,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
                 self._update_ancestors(xblock.scope_ids.usage_id, ancestor_payload)
 
             # update the edit info of the instantiated xblock
-            xblock.edited_on = now
+            xblock._edit_info = payload['edit_info']
-            xblock.edited_by = user_id
-            xblock.subtree_edited_on = now
-            xblock.subtree_edited_by = user_id
-            if not hasattr(xblock, 'published_date'):
-                xblock.published_date = None
-            if not hasattr(xblock, 'published_by'):
-                xblock.published_by = None
-            if isPublish:
-                xblock.published_date = now
-                xblock.published_by = user_id
 
             # recompute (and update) the metadata inheritance tree which is cached
             self.refresh_cached_metadata_inheritance_tree(xblock.scope_ids.usage_id.course_key, xblock.runtime)

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

@@ -12,10 +12,11 @@ import logging
 from opaque_keys.edx.locations import Location
 from xmodule.exceptions import InvalidVersionError
 from xmodule.modulestore import ModuleStoreEnum
-from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateItemError, DuplicateCourseError
+from xmodule.modulestore.exceptions import (
+    ItemNotFoundError, DuplicateItemError, DuplicateCourseError, InvalidBranchSetting
+)
 from xmodule.modulestore.mongo.base import (
-    MongoModuleStore, MongoRevisionKey, as_draft, as_published,
+    MongoModuleStore, MongoRevisionKey, as_draft, as_published, 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, DIRECT_ONLY_CATEGORIES

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

@@ -11,11 +11,12 @@ from ..exceptions import ItemNotFoundError
 from .split_mongo_kvs import SplitMongoKVS
 from fs.osfs import OSFS
 from .definition_lazy_loader import DefinitionLazyLoader
+from xmodule.modulestore.edit_info import EditInfoRuntimeMixin
 
 log = logging.getLogger(__name__)
 
 
-class CachingDescriptorSystem(MakoDescriptorSystem):
+class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
     """
     A system that has a cache of a course version's json that it will use to load modules
     from, with a backup of calling to the underlying modulestore for more data.
@@ -89,6 +90,19 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
                 run=course_info.get('run'),
                 branch=course_info.get('branch'),
             )
+        json_data = self.get_module_data(block_id, course_key)
+
+        class_ = self.load_block_type(json_data.get('category'))
+        new_item = self.xblock_from_json(class_, course_key, block_id, json_data, course_entry_override, **kwargs)
+        return new_item
+
+    def get_module_data(self, block_id, course_key):
+        """
+        Get block from module_data adding it to module_data if it's not already there but is in the structure
+
+        Raises:
+            ItemNotFoundError if block is not in the structure
+        """
         json_data = self.module_data.get(block_id)
         if json_data is None:
             # deeper than initial descendant fetch or doesn't exist
@@ -97,9 +111,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
             if json_data is None:
                 raise ItemNotFoundError(block_id)
 
-        class_ = self.load_block_type(json_data.get('category'))
+        return json_data
-        new_item = self.xblock_from_json(class_, course_key, block_id, json_data, course_entry_override, **kwargs)
-        return new_item
 
     # xblock's runtime does not always pass enough contextual information to figure out
     # which named container (course x branch) or which parent is requesting an item. Because split allows
@@ -181,12 +193,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
             )
 
         edit_info = json_data.get('edit_info', {})
-        module.edited_by = edit_info.get('edited_by')
+        module._edited_by = edit_info.get('edited_by')
-        module.edited_on = edit_info.get('edited_on')
+        module._edited_on = edit_info.get('edited_on')
-        module.subtree_edited_by = None  # TODO - addressed with LMS-11183
-        module.subtree_edited_on = None  # TODO - addressed with LMS-11183
-        module.published_by = None  # TODO - addressed with LMS-11184
-        module.published_date = None  # TODO - addressed with LMS-11184
         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)
@@ -199,3 +207,79 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
             self.local_modules[block_locator] = module
 
         return module
+
+    def get_edited_by(self, xblock):
+        """
+        See :meth: cms.lib.xblock.runtime.EditInfoRuntimeMixin.get_edited_by
+        """
+        return xblock._edited_by
+
+    def get_edited_on(self, xblock):
+        """
+        See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
+        """
+        return xblock._edited_on
+
+    def get_subtree_edited_by(self, xblock):
+        """
+        See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
+        """
+        if not hasattr(xblock, '_subtree_edited_by'):
+            json_data = self.module_data[xblock.location.block_id]
+            if '_subtree_edited_by' not in json_data.setdefault('edit_info', {}):
+                self._compute_subtree_edited_internal(
+                    xblock.location.block_id, json_data, xblock.location.course_key
+                )
+            setattr(xblock, '_subtree_edited_by', json_data['edit_info']['_subtree_edited_by'])
+
+        return getattr(xblock, '_subtree_edited_by')
+
+    def get_subtree_edited_on(self, xblock):
+        """
+        See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
+        """
+        if not hasattr(xblock, '_subtree_edited_on'):
+            json_data = self.module_data[xblock.location.block_id]
+            if '_subtree_edited_on' not in json_data.setdefault('edit_info', {}):
+                self._compute_subtree_edited_internal(
+                    xblock.location.block_id, json_data, xblock.location.course_key
+                )
+            setattr(xblock, '_subtree_edited_on', json_data['edit_info']['_subtree_edited_on'])
+
+        return getattr(xblock, '_subtree_edited_on')
+
+    def get_published_by(self, xblock):
+        """
+        See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
+        """
+        if not hasattr(xblock, '_published_by'):
+            self.modulestore.compute_published_info_internal(xblock)
+
+        return getattr(xblock, '_published_by', None)
+
+    def get_published_on(self, xblock):
+        """
+        See :class: cms.lib.xblock.runtime.EditInfoRuntimeMixin
+        """
+        if not hasattr(xblock, '_published_on'):
+            self.modulestore.compute_published_info_internal(xblock)
+
+        return getattr(xblock, '_published_on', None)
+
+    def _compute_subtree_edited_internal(self, block_id, json_data, course_key):
+        """
+        Recurse the subtree finding the max edited_on date and its concomitant edited_by. Cache it
+        """
+        max_date = json_data['edit_info']['edited_on']
+        max_by = json_data['edit_info']['edited_by']
+
+        for child in json_data.get('fields', {}).get('children', []):
+            child_data = self.get_module_data(child, course_key)
+            if '_subtree_edited_on' not in json_data.setdefault('edit_info', {}):
+                self._compute_subtree_edited_internal(child, child_data, course_key)
+            if child_data['edit_info']['_subtree_edited_on'] > max_date:
+                max_date = child_data['edit_info']['_subtree_edited_on']
+                max_by = child_data['edit_info']['_subtree_edited_by']
+
+        json_data['edit_info']['_subtree_edited_on'] = max_date
+        json_data['edit_info']['_subtree_edited_by'] = max_by

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

@@ -387,3 +387,12 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
             return self._update_item_from_fields(
                 user_id, course_key, block_type, block_id, partitioned_fields, None, allow_not_found=True, force=True
             )
+
+    def compute_published_info_internal(self, xblock):
+        """
+        Get the published branch and find when it was published if it was. Cache the results in the xblock
+        """
+        published_block = self._get_head(xblock, ModuleStoreEnum.BranchName.published)
+        if published_block is not None:
+            setattr(xblock, '_published_by', published_block['edit_info']['edited_by'])
+            setattr(xblock, '_published_on', published_block['edit_info']['edited_on'])

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

@@ -13,6 +13,8 @@ from uuid import uuid4
 # before importing the module
 # TODO remove this import and the configuration -- xmodule should not depend on django!
 from django.conf import settings
+from xmodule.modulestore.edit_info import EditInfoMixin
+
 if not settings.configured:
     settings.configure()
 
@@ -52,6 +54,7 @@ class TestMixedModuleStore(unittest.TestCase):
         'default_class': DEFAULT_CLASS,
         'fs_root': DATA_DIR,
         'render_template': RENDER_TEMPLATE,
+        'xblock_mixins': (EditInfoMixin,)
     }
     DOC_STORE_CONFIG = {
         'host': HOST,
@@ -995,7 +998,7 @@ class TestMixedModuleStore(unittest.TestCase):
         self.assertEqual(self.user_id, block.edited_by)
         self.assertGreater(datetime.datetime.now(UTC), block.edited_on)
 
-    @ddt.data('draft')
+    @ddt.data('draft', 'split')
     def test_create_item_populates_subtree_edited_info(self, default_ms):
         self.initdb(default_ms)
         block = self.store.create_item(
@@ -1120,6 +1123,147 @@ class TestMixedModuleStore(unittest.TestCase):
         self.assertTrue(self.store.has_changes(item))
         self.assertTrue(self.store.has_published_version(item))
 
+    @ddt.data('draft', 'split')
+    def test_update_edit_info_ancestors(self, default_ms):
+        """
+        Tests that edited_on, edited_by, subtree_edited_on, and subtree_edited_by are set correctly during update
+        """
+        self.initdb(default_ms)
+
+        test_course = self.store.create_course('testx', 'GreekHero', 'test_run', self.user_id)
+
+        def check_node(location_key, after, before, edited_by, subtree_after, subtree_before, subtree_by):
+            """
+            Checks that the node given by location_key matches the given edit_info constraints.
+            """
+            node = self.store.get_item(location_key)
+            if after:
+                self.assertLess(after, node.edited_on)
+            self.assertLess(node.edited_on, before)
+            self.assertEqual(node.edited_by, edited_by)
+            if subtree_after:
+                self.assertLess(subtree_after, node.subtree_edited_on)
+            self.assertLess(node.subtree_edited_on, subtree_before)
+            self.assertEqual(node.subtree_edited_by, subtree_by)
+
+        # Create a dummy vertical & html to test against
+        component = self.store.create_child(
+            self.user_id,
+            test_course.location,
+            'vertical',
+            block_id='test_vertical'
+        )
+        child = self.store.create_child(
+            self.user_id,
+            component.location,
+            'html',
+            block_id='test_html'
+        )
+        sibling = self.store.create_child(
+            self.user_id,
+            component.location,
+            'html',
+            block_id='test_html_no_change'
+        )
+
+        after_create = datetime.datetime.now(UTC)
+        # Verify that all nodes were last edited in the past by create_user
+        [
+            check_node(block.location, None, after_create, self.user_id, None, after_create, self.user_id)
+            for block in [component, child, sibling]
+        ]
+
+        # Change the component, then check that there now are changes
+        component.display_name = 'Changed Display Name'
+
+        editing_user = self.user_id - 2
+        component = self.store.update_item(component, editing_user)
+        after_edit = datetime.datetime.now(UTC)
+        check_node(component.location, after_create, after_edit, editing_user, after_create, after_edit, editing_user)
+        # but child didn't change
+        check_node(child.location, None, after_create, self.user_id, None, after_create, self.user_id)
+
+
+        # Change the child
+        child = self.store.get_item(child.location)
+        child.display_name = 'Changed Display Name'
+        self.store.update_item(child, user_id=editing_user)
+
+        after_edit = datetime.datetime.now(UTC)
+
+        # Verify that child was last edited between after_create and after_edit by edit_user
+        check_node(child.location, after_create, after_edit, editing_user, after_create, after_edit, editing_user)
+
+        # Verify that ancestors edit info is unchanged, but their subtree edit info matches child
+        check_node(test_course.location, None, after_create, self.user_id, after_create, after_edit, editing_user)
+
+        # Verify that others have unchanged edit info
+        check_node(sibling.location, None, after_create, self.user_id, None, after_create, self.user_id)
+
+    @ddt.data('draft', 'split')
+    def test_update_edit_info(self, default_ms):
+        """
+        Tests that edited_on and edited_by are set correctly during an update
+        """
+        self.initdb(default_ms)
+
+        test_course = self.store.create_course('testx', 'GreekHero', 'test_run', self.user_id)
+
+        # Create a dummy component to test against
+        component = self.store.create_child(
+            self.user_id,
+            test_course.location,
+            'vertical',
+        )
+
+        # Store the current edit time and verify that user created the component
+        self.assertEqual(component.edited_by, self.user_id)
+        old_edited_on = component.edited_on
+
+        edit_user = self.user_id - 2
+        # Change the component
+        component.display_name = 'Changed'
+        self.store.update_item(component, edit_user)
+        updated_component = self.store.get_item(component.location)
+
+        # Verify the ordering of edit times and that dummy_user made the edit
+        self.assertLess(old_edited_on, updated_component.edited_on)
+        self.assertEqual(updated_component.edited_by, edit_user)
+
+    @ddt.data('draft', 'split')
+    def test_update_published_info(self, default_ms):
+        """
+        Tests that published_on and published_by are set correctly
+        """
+        self.initdb(default_ms)
+
+        test_course = self.store.create_course('testx', 'GreekHero', 'test_run', self.user_id)
+
+        publish_user = 456
+
+        # Create a dummy component to test against
+        component = self.store.create_child(
+            self.user_id,
+            test_course.location,
+            'vertical',
+        )
+
+        # Store the current time, then publish
+        old_time = datetime.datetime.now(UTC)
+        self.store.publish(component.location, publish_user)
+        updated_component = self.store.get_item(component.location)
+
+        # Verify the time order and that publish_user caused publication
+        self.assertLessEqual(old_time, updated_component.published_on)
+        self.assertEqual(updated_component.published_by, publish_user)
+
+        # Verify that changing the item doesn't unset the published info
+        updated_component.display_name = 'changed'
+        self.store.update_item(updated_component, self.user_id)
+        updated_component = self.store.get_item(updated_component.location)
+        self.assertLessEqual(old_time, updated_component.published_on)
+        self.assertEqual(updated_component.published_by, publish_user)
+
     @ddt.data('draft', 'split')
     def test_auto_publish(self, default_ms):
         """

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

@@ -37,6 +37,7 @@ from git.test.lib.asserts import assert_not_none
 from xmodule.x_module import XModuleMixin
 from xmodule.modulestore.mongo.base import as_draft
 from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST
+from xmodule.modulestore.edit_info import EditInfoMixin
 
 log = logging.getLogger(__name__)
 
@@ -105,7 +106,9 @@ class TestMongoModuleStore(unittest.TestCase):
             content_store,
             doc_store_config, FS_ROOT, RENDER_TEMPLATE,
             default_class=DEFAULT_CLASS,
-            branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred
+            branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred,
+            xblock_mixins=(EditInfoMixin,)
+
         )
         import_from_xml(
             draft_store,
@@ -706,106 +709,6 @@ class TestMongoModuleStore(unittest.TestCase):
         self.assertTrue(self._has_changes(parent_location))
         self.assertTrue(self._has_changes(child_location))
 
-    def test_update_edit_info_ancestors(self):
-        """
-        Tests that edited_on, edited_by, subtree_edited_on, and subtree_edited_by are set correctly during update
-        """
-        create_user = 123
-        edit_user = 456
-        locations =self._create_test_tree('update_edit_info_ancestors', create_user)
-
-        def check_node(location_key, after, before, edited_by, subtree_after, subtree_before, subtree_by):
-            """
-            Checks that the node given by location_key matches the given edit_info constraints.
-            """
-            node = self.draft_store.get_item(locations[location_key])
-            if after:
-                self.assertLess(after, node.edited_on)
-            self.assertLess(node.edited_on, before)
-            self.assertEqual(node.edited_by, edited_by)
-            if subtree_after:
-                self.assertLess(subtree_after, node.subtree_edited_on)
-            self.assertLess(node.subtree_edited_on, subtree_before)
-            self.assertEqual(node.subtree_edited_by, subtree_by)
-
-        after_create = datetime.now(UTC)
-        # Verify that all nodes were last edited in the past by create_user
-        for key in locations:
-            check_node(key, None, after_create, create_user, None, after_create, create_user)
-
-        # Change the child
-        child = self.draft_store.get_item(locations['child'])
-        child.display_name = 'Changed Display Name'
-        self.draft_store.update_item(child, user_id=edit_user)
-
-        after_edit = datetime.now(UTC)
-        ancestors = ['parent', 'grandparent']
-        others = ['child_sibling', 'parent_sibling']
-
-        # Verify that child was last edited between after_create and after_edit by edit_user
-        check_node('child', after_create, after_edit, edit_user, after_create, after_edit, edit_user)
-
-        # Verify that ancestors edit info is unchanged, but their subtree edit info matches child
-        for key in ancestors:
-            check_node(key, None, after_create, create_user, after_create, after_edit, edit_user)
-
-        # Verify that others have unchanged edit info
-        for key in others:
-            check_node(key, None, after_create, create_user, None, after_create, create_user)
-
-    def test_update_edit_info(self):
-        """
-        Tests that edited_on and edited_by are set correctly during an update
-        """
-        location = Location('edX', 'toy', '2012_Fall', 'html', 'test_html')
-
-        # 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
-        )
-
-        # Store the current edit time and verify that dummy_user created the component
-        component = self.draft_store.get_item(location)
-        self.assertEqual(component.edited_by, self.dummy_user)
-        old_edited_on = component.edited_on
-
-        # Change the component
-        component.display_name = component.display_name + ' Changed'
-        self.draft_store.update_item(component, self.dummy_user)
-        updated_component = self.draft_store.get_item(location)
-
-        # Verify the ordering of edit times and that dummy_user made the edit
-        self.assertLess(old_edited_on, updated_component.edited_on)
-        self.assertEqual(updated_component.edited_by, self.dummy_user)
-
-    def test_update_published_info(self):
-        """
-        Tests that published_date and published_by are set correctly
-        """
-        location = Location('edX', 'toy', '2012_Fall', 'html', 'test_html')
-        create_user = 123
-        publish_user = 456
-
-        # Create a dummy component to test against
-        self.draft_store.create_item(
-            create_user,
-            location.course_key,
-            location.block_type,
-            block_id=location.block_id
-        )
-
-        # Store the current time, then publish
-        old_time = datetime.now(UTC)
-        self.draft_store.publish(location, publish_user)
-        updated_component = self.draft_store.get_item(location)
-
-        # Verify the time order and that publish_user caused publication
-        self.assertLessEqual(old_time, updated_component.published_date)
-        self.assertEqual(updated_component.published_by, publish_user)
-
     def test_migrate_published_info(self):
         """
         Tests that blocks that were storing published_date and published_by through CMSBlockMixin are loaded correctly
@@ -829,7 +732,7 @@ class TestMongoModuleStore(unittest.TestCase):
 
         # Retrieve the block and verify its fields
         component = self.draft_store.get_item(location)
-        self.assertEqual(component.published_date, published_date)
+        self.assertEqual(component.published_on, published_date)
         self.assertEqual(component.published_by, published_by)
 
     def test_export_course_with_peer_component(self):

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

@@ -23,6 +23,7 @@ from xmodule.fields import Date, Timedelta
 from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore
 from xmodule.modulestore.tests.test_modulestore import check_has_course_method
 from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST
+from xmodule.modulestore.edit_info import EditInfoMixin
 
 
 BRANCH_NAME_DRAFT = ModuleStoreEnum.BranchName.draft
@@ -45,7 +46,7 @@ class SplitModuleTest(unittest.TestCase):
     modulestore_options = {
         'default_class': 'xmodule.raw_module.RawDescriptor',
         'fs_root': '',
-        'xblock_mixins': (InheritanceMixin, XModuleMixin)
+        'xblock_mixins': (InheritanceMixin, XModuleMixin, EditInfoMixin)
     }
 
     MODULESTORE = {

common/lib/xmodule/xmodule/x_module.py

@@ -724,7 +724,7 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock):
         # leaving off original_version since it complicates creation w/o any obv value yet and is computable
         # by following previous until None
         # definition_locator is only used by mongostores which separate definitions from blocks
-        self.edited_by = self.edited_on = self.previous_version = self.update_version = self.definition_locator = None
+        self.previous_version = self.update_version = self.definition_locator = None
         self.xmodule_runtime = None
 
     @classmethod

common/lib/xmodule/xmodule/xml_module.py

@@ -124,7 +124,7 @@ class XmlDescriptor(XModuleDescriptor):
     # import and export.
 
     metadata_to_strip = ('data_dir',
-                         'tabs', 'grading_policy', 'published_by', 'published_date',
+                         'tabs', 'grading_policy',
                          'discussion_blackouts',
                          # VS[compat] -- remove the below attrs once everything is in the CMS
                          'course', 'org', 'url_name', 'filename',