From 7b52d45aa6e44bd995c4d75e3a7e97c13c881805 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 27 Aug 2014 16:03:09 -0400 Subject: [PATCH 1/4] Move edit info into xblock and runtime mixins LMS-11183, LMS-11184 --- cms/envs/common.py | 3 +- .../xmodule/xmodule/modulestore/edit_info.py | 101 ++++++++++++++++++ common/lib/xmodule/xmodule/x_module.py | 2 +- 3 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 common/lib/xmodule/xmodule/modulestore/edit_info.py diff --git a/cms/envs/common.py b/cms/envs/common.py index 5f8ebfdb17..b2e15a9eba 100644 --- a/cms/envs/common.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/modulestore/edit_info.py b/common/lib/xmodule/xmodule/modulestore/edit_info.py new file mode 100644 index 0000000000..4dedccd423 --- /dev/null +++ b/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 diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 3ce77b7b2f..010e2c1fd0 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/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 From 4878f6e83cae5de48ca7391ce3fbad20c67d8bd4 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 27 Aug 2014 16:03:49 -0400 Subject: [PATCH 2/4] Implement edit info as mixin for split LMS-11183, LMS-11184 --- .../split_mongo/caching_descriptor_system.py | 104 ++++++++++++++++-- .../modulestore/split_mongo/split_draft.py | 8 ++ .../tests/test_mixed_modulestore.py | 3 + .../tests/test_split_modulestore.py | 3 +- 4 files changed, 107 insertions(+), 11 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 7c84f5e061..fee8f4ad2b 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/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')) - new_item = self.xblock_from_json(class_, course_key, block_id, json_data, course_entry_override, **kwargs) - return new_item + return json_data # 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_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._edited_by = edit_info.get('edited_by') + 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) @@ -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) + 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 diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index d534e91ac3..4a935bd319 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -387,3 +387,11 @@ 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) + setattr(xblock, '_published_by', published_block['edit_info']['edited_by']) + setattr(xblock, '_published_on', published_block['edit_info']['edited_on']) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 64831faea4..f1498ee63b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/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, diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index cee7f08897..2ed87a592d 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/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 = { From 85fa712fdf4ebd19b922da3f418accf4743995ae Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 27 Aug 2014 16:12:44 -0400 Subject: [PATCH 3/4] Change published_date to published_on to make consistent with other edit info timestamps LMS-11184 --- cms/djangoapps/contentstore/views/item.py | 2 +- .../xmodule/xmodule/modulestore/mongo/base.py | 16 ++++++++-------- .../xmodule/xmodule/modulestore/mongo/draft.py | 7 ++++--- .../xmodule/modulestore/tests/test_mongo.py | 6 +++--- common/lib/xmodule/xmodule/xml_module.py | 2 +- 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 6da18ae47a..76825e9b3e 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -666,7 +666,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, diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 05a3729497..e0e39a28e5 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -235,14 +235,14 @@ class CachingDescriptorSystem(MakoDescriptorSystem): 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: module.edited_by = module.edited_on = module.subtree_edited_on = \ - module.subtree_edited_by = module.published_date = None + module.subtree_edited_by = module.published_on = 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.published_on = datetime(*raw_metadata.get('published_date')[0:6]).replace(tzinfo=UTC) module.published_by = raw_metadata.get('published_by') # otherwise restore the stored editing information else: @@ -250,7 +250,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): 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_on = edit_info.get('published_date') module.published_by = edit_info.get('published_by') # decache any computed pending field settings @@ -1185,12 +1185,12 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): 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_on'): + xblock.published_on = None if not hasattr(xblock, 'published_by'): xblock.published_by = None if isPublish: - xblock.published_date = now + xblock.published_on = now xblock.published_by = user_id # recompute (and update) the metadata inheritance tree which is cached diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 95882cdb3b..fa99775beb 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/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, - SORT_REVISION_FAVOR_DRAFT + MongoModuleStore, MongoRevisionKey, as_draft, as_published, 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 diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 2791a7e33b..84d845eea2 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -783,7 +783,7 @@ class TestMongoModuleStore(unittest.TestCase): def test_update_published_info(self): """ - Tests that published_date and published_by are set correctly + Tests that published_on and published_by are set correctly """ location = Location('edX', 'toy', '2012_Fall', 'html', 'test_html') create_user = 123 @@ -803,7 +803,7 @@ class TestMongoModuleStore(unittest.TestCase): 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.assertLessEqual(old_time, updated_component.published_on) self.assertEqual(updated_component.published_by, publish_user) def test_migrate_published_info(self): @@ -829,7 +829,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): diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index f5117ed223..dc9a3931da 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/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', From a65771df93c9c618bd83dcc487ed23a45dcba951 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 27 Aug 2014 16:36:32 -0400 Subject: [PATCH 4/4] Implement edit info as mixin on base mongo LMS-11183, LMS-11184 --- .../xmodule/xmodule/modulestore/mongo/base.py | 87 +++++++---- .../split_mongo/caching_descriptor_system.py | 2 +- .../modulestore/split_mongo/split_draft.py | 5 +- .../tests/test_mixed_modulestore.py | 143 +++++++++++++++++- .../xmodule/modulestore/tests/test_mongo.py | 105 +------------ 5 files changed, 205 insertions(+), 137 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index e0e39a28e5..c23ffcc6a5 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/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_on if edit_info isn't present - if not edit_info: - module.edited_by = module.edited_on = module.subtree_edited_on = \ - module.subtree_edited_by = module.published_on = None + if module._edit_info is None: + module._edit_info = {} raw_metadata = json_data.get('metadata', {}) # published_on was previously stored as a list of time components instead of a datetime if raw_metadata.get('published_date'): - module.published_on = datetime(*raw_metadata.get('published_date')[0:6]).replace(tzinfo=UTC) - module.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_on = edit_info.get('published_date') - module.published_by = edit_info.get('published_by') + module._edit_info['published_date'] = datetime(*raw_metadata.get('published_date')[0:6]).replace(tzinfo=UTC) + module._edit_info['published_by'] = raw_metadata.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.edited_by': user_id, - 'edit_info.subtree_edited_on': now, - 'edit_info.subtree_edited_by': user_id, + 'edit_info': { + 'edited_on': now, + '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_by'] = user_id + payload['edit_info']['published_date'] = now + 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.edited_by = user_id - xblock.subtree_edited_on = now - xblock.subtree_edited_by = user_id - if not hasattr(xblock, 'published_on'): - xblock.published_on = None - if not hasattr(xblock, 'published_by'): - xblock.published_by = None - if isPublish: - xblock.published_on = now - xblock.published_by = user_id + xblock._edit_info = payload['edit_info'] # 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) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index fee8f4ad2b..ab261dee96 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -276,7 +276,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): 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) + 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'] diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index 4a935bd319..6fd7caf5ec 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -393,5 +393,6 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS 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) - setattr(xblock, '_published_by', published_block['edit_info']['edited_by']) - setattr(xblock, '_published_on', published_block['edit_info']['edited_on']) + 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']) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index f1498ee63b..141f479e9b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -999,7 +999,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( @@ -1123,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): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 84d845eea2..06f4a1ba76 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/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_on 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_on) - 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