Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Tracks last edited date and user, and adds has_changes for blocks.

Browse Source
This commit is contained in:
Ben McMorran
2014-06-09 15:53:40 -04:00
parent b60b7c7e5a
commit ea97dd4508
11 changed files with 222 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
cms/djangoapps/contentstore/course_info_model.py
View File

@@ -239,6 +239,6 @@ def save_course_update_items(location, course_updates, course_update_items, user
course_updates.data = _get_html(course_update_items)
# update db record
modulestore('direct').update_item(course_updates, user)
modulestore('direct').update_item(course_updates, user.id)
return course_updates

2
cms/djangoapps/contentstore/views/tests/test_course_updates.py
View File

@@ -135,7 +135,7 @@ class CourseUpdateTest(CourseTestCase):
update_content = u"Hello world!"
update_data = u"<ol><li><h2>" + update_date + "</h2>" + update_content + "</li></ol>"
course_updates.data = update_data
modulestore('direct').update_item(course_updates, self.user)
modulestore('direct').update_item(course_updates, self.user.id)
# test getting all updates list
course_update_url = self.create_update_url()

4
cms/djangoapps/models/settings/course_metadata.py
View File

@@ -1,7 +1,6 @@
from xblock.fields import Scope
from contentstore.utils import get_modulestore
from cms.lib.xblock.mixin import CmsBlockMixin
class CourseMetadata(object):
@@ -33,9 +32,6 @@ class CourseMetadata(object):
result = {}
for field in descriptor.fields.values():
if field.name in CmsBlockMixin.fields:
continue
if field.scope != Scope.settings:
continue

3
cms/envs/common.py
View File

@@ -33,7 +33,6 @@ from lms.envs.common import (
from path import path
from lms.lib.xblock.mixin import LmsBlockMixin
from cms.lib.xblock.mixin import CmsBlockMixin
from dealer.git import git
############################ FEATURE CONFIGURATION #############################
@@ -238,7 +237,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, CmsBlockMixin, InheritanceMixin, XModuleMixin)
XBLOCK_MIXINS = (LmsBlockMixin, InheritanceMixin, XModuleMixin)
# Allow any XBlock in Studio
# You should also enable the ALLOW_ALL_ADVANCED_COMPONENTS feature flag, so that

8
cms/lib/xblock/mixin.py
View File

@@ -19,11 +19,3 @@ class DateTuple(Field):
return None
return list(value.timetuple())
class CmsBlockMixin(XBlockMixin):
"""
Mixin with fields common to all blocks in Studio
"""
published_date = DateTuple(help="Date when the module was published", scope=Scope.settings)
published_by = Integer(help="Id of the user who published this module", scope=Scope.settings)

3
common/lib/xmodule/xmodule/error_module.py
View File

@@ -13,6 +13,7 @@ from xmodule.x_module import XModule, XModuleDescriptor
from xmodule.errortracker import exc_info_to_str
from xblock.fields import String, Scope, ScopeIds
from xblock.field_data import DictFieldData
from xmodule.modulestore.xml_exporter import EdxJSONEncoder
log = logging.getLogger(__name__)
@@ -121,7 +122,7 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor):
def from_json(cls, json_data, system, location, error_msg='Error not available'):
return cls._construct(
system,
json.dumps(json_data, skipkeys=False, indent=4),
json.dumps(json_data, skipkeys=False, indent=4, cls=EdxJSONEncoder),
error_msg,
location=location
)

37
common/lib/xmodule/xmodule/modulestore/mongo/base.py
View File

@@ -21,6 +21,8 @@ import re
from bson.son import SON
from fs.osfs import OSFS
from path import path
from datetime import datetime
from pytz import UTC
from importlib import import_module
from xmodule.errortracker import null_error_tracker, exc_info_to_str
@@ -206,6 +208,23 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
# to python values
metadata_to_inherit = self.cached_metadata.get(non_draft_loc.to_deprecated_string(), {})
inherit_metadata(module, metadata_to_inherit)
edit_info = json_data.get('edit_info')
# migrate published_by and published_date if edit_info isn't present
if not edit_info:
module.edited_by = module.edited_on = module.published_date = None
# published_date was previously stored as a list of time components instead of a datetime
if metadata.get('published_date'):
module.published_date = datetime(*metadata.get('published_date')[0:6]).replace(tzinfo=UTC)
module.published_by = 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.published_date = edit_info.get('published_date')
module.published_by = edit_info.get('published_by')
# decache any computed pending field settings
module.save()
return module
@@ -826,7 +845,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
return xmodule
def create_and_save_xmodule(self, location, definition_data=None, metadata=None, system=None,
fields={}):
fields={}, user_id=None):
"""
Create the new xmodule and save it. Does not return the new module because if the caller
will insert it as a child, it's inherited metadata will completely change. The difference
@@ -837,12 +856,13 @@ class MongoModuleStore(ModuleStoreWriteBase):
:param definition_data: can be empty. The initial definition_data for the kvs
:param metadata: can be empty, the initial metadata for the kvs
:param system: if you already have an xblock from the course, the xblock.runtime value
:param user_id: the user that created the xblock
"""
# differs from split mongo in that I believe most of this logic should be above the persistence
# layer but added it here to enable quick conversion. I'll need to reconcile these.
new_object = self.create_xmodule(location, definition_data, metadata, system, fields)
location = new_object.scope_ids.usage_id
self.update_item(new_object, allow_not_found=True)
self.update_item(new_object, allow_not_found=True, user_id=user_id)
# VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so
# if we add one then we need to also add it to the policy information (i.e. metadata)
@@ -856,7 +876,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
url_slug=new_object.scope_ids.usage_id.name,
)
)
self.update_item(course)
self.update_item(course, user_id=user_id)
return new_object
@@ -888,7 +908,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
if result['n'] == 0:
raise ItemNotFoundError(location)
def update_item(self, xblock, user_id=None, allow_not_found=False, force=False):
def update_item(self, xblock, user_id=None, allow_not_found=False, force=False, isPublish=False):
"""
Update the persisted version of xblock to reflect its current values.
@@ -902,7 +922,16 @@ class MongoModuleStore(ModuleStoreWriteBase):
payload = {
'definition.data': definition_data,
'metadata': self._convert_reference_fields_to_strings(xblock, own_metadata(xblock)),
'edit_info': {
'edited_on': datetime.now(UTC),
'edited_by': user_id,
}
}
if isPublish:
payload['edit_info']['published_date'] = datetime.now(UTC)
payload['edit_info']['published_by'] = user_id
if xblock.has_children:
children = self._convert_reference_fields_to_strings(xblock, {'children': xblock.children})
payload.update({'definition.children': children['children']})

32
common/lib/xmodule/xmodule/modulestore/mongo/draft.py
View File

@@ -157,7 +157,7 @@ class DraftModuleStore(MongoModuleStore):
return wrap_draft(self._load_items(source_location.course_key, [original])[0])
def update_item(self, xblock, user_id=None, allow_not_found=False, force=False):
def update_item(self, xblock, user_id=None, allow_not_found=False, force=False, isPublish=False):
"""
See superclass doc.
In addition to the superclass's behavior, this method converts the unit to draft if it's not
@@ -175,7 +175,7 @@ class DraftModuleStore(MongoModuleStore):
raise
xblock.location = draft_loc
super(DraftModuleStore, self).update_item(xblock, user_id, allow_not_found)
super(DraftModuleStore, self).update_item(xblock, user_id, allow_not_found, isPublish)
# don't allow locations to truly represent themselves as draft outside of this file
xblock.location = as_published(xblock.location)
@@ -194,6 +194,30 @@ class DraftModuleStore(MongoModuleStore):
return
def has_changes(self, location):
"""
Check if the xblock has been changed since it was last published.
:param location: location to check
:return: True if the draft and published versions differ
"""
# Direct only categories can never have changes because they can't have drafts
if location.category in DIRECT_ONLY_CATEGORIES:
return False
draft = self.get_item(location)
# If the draft was never published, then it clearly has unpublished changes
if not draft.published_date:
return True
# edited_on may be None if the draft was last edited before edit time tracking
# If the draft does not have an edit time, we play it safe and assume there are differences
if draft.edited_on:
return draft.edited_on > draft.published_date
else:
return True
def publish(self, location, published_by_id):
"""
Save a current draft to the underlying modulestore
@@ -209,8 +233,6 @@ class DraftModuleStore(MongoModuleStore):
draft = self.get_item(location)
draft.published_date = datetime.now(UTC)
draft.published_by = published_by_id
if draft.has_children:
if original_published is not None:
# see if children were deleted. 2 reasons for children lists to differ:
@@ -221,7 +243,7 @@ class DraftModuleStore(MongoModuleStore):
rents = self.get_parent_locations(child)
if (len(rents) == 1 and rents[0] == location): # the 1 is this original_published
self.delete_item(child, True)
super(DraftModuleStore, self).update_item(draft, '**replace_user**')
super(DraftModuleStore, self).update_item(draft, published_by_id, isPublish=True)
self.delete_item(location)
def unpublish(self, location):

14
common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
View File

@@ -371,6 +371,20 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
return self._get_block_from_structure(course_structure, usage_key.block_id) is not None
def has_changes(self, usage_key):
"""
Checks if the given block has unpublished changes
:param usage_key: the block to check
:return: True if the draft and published versions differ
"""
draft = self.get_item(usage_key.for_branch("draft"))
try:
published = self.get_item(usage_key.for_branch("published"))
except ItemNotFoundError:
return True
return draft.update_version != published.update_version
def get_item(self, usage_key, depth=0):
"""
depth (int): An argument that some module stores may use to prefetch

117
common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
View File

@@ -8,6 +8,8 @@ import logging
import shutil
from tempfile import mkdtemp
from uuid import uuid4
from datetime import datetime
from pytz import UTC
import unittest
from xblock.core import XBlock
@@ -21,6 +23,7 @@ from opaque_keys.edx.locations import Location
from xmodule.modulestore import MONGO_MODULESTORE_TYPE
from xmodule.modulestore.mongo import MongoModuleStore, MongoKeyValueStore
from xmodule.modulestore.draft import DraftModuleStore
from xmodule.modulestore.mongo.draft import as_draft
from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation
from xmodule.modulestore.xml_exporter import export_to_xml
from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint
@@ -471,6 +474,120 @@ class TestMongoModuleStore(unittest.TestCase):
finally:
shutil.rmtree(root_dir)
def test_has_changes_direct_only(self):
"""
Tests that has_changes() returns false when an xblock in a direct only category is checked
"""
course_location = Location('edx', 'direct', '2012_Fall', 'course', 'test_course')
chapter_location = Location('edx', 'direct', '2012_Fall', 'chapter', 'test_chapter')
dummy_user = 123
# Create dummy direct only xblocks
self.draft_store.create_and_save_xmodule(course_location, user_id=dummy_user)
self.draft_store.create_and_save_xmodule(chapter_location, user_id=dummy_user)
# 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', 'changes', '2012_Fall', 'vertical', 'test_vertical')
dummy_user = 123
# Create a dummy component to test against
self.draft_store.create_and_save_xmodule(location, user_id=dummy_user)
# 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, 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, dummy_user)
self.assertTrue(self.draft_store.has_changes(location))
# Publish and verify again
self.draft_store.publish(location, dummy_user)
self.assertFalse(self.draft_store.has_changes(location))
def test_update_edit_info(self):
"""
Tests that edited_on and edited_by are set correctly during an update
"""
location = Location('edX', 'editInfoTest', '2012_Fall', 'html', 'test_html')
dummy_user = 123
# Create a dummy component to test against
self.draft_store.create_and_save_xmodule(location, user_id=dummy_user)
# 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, dummy_user)
old_edited_on = component.edited_on
# Change the component
component.display_name = component.display_name + ' Changed'
self.draft_store.update_item(component, 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, dummy_user)
def test_update_published_info(self):
"""
Tests that published_date and published_by are set correctly
"""
location = Location('edX', 'publishInfo', '2012_Fall', 'html', 'test_html')
create_user = 123
publish_user = 456
# Create a dummy component to test against
self.draft_store.create_and_save_xmodule(location, user_id=create_user)
# 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
"""
# Insert the test block directly into the module store
location = Location('edX', 'migration', '2012_Fall', 'html', 'test_html')
published_date = datetime(1970, 1, 1, tzinfo=UTC)
published_by = 123
self.store._update_single_item(
as_draft(location),
{
'definition.data': {},
'metadata': {
# published_date was previously stored as a list of time components, not a datetime
'published_date': list(published_date.timetuple()),
'published_by': published_by,
},
},
)
# 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_by, published_by)
class TestMongoKeyValueStore(object):
"""

26
common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py
View File

@@ -805,6 +805,32 @@ class SplitModuleItemTests(SplitModuleTest):
with self.assertRaises(ItemNotFoundError):
modulestore().get_item(course.location.for_branch("published"))
def test_has_changes(self):
"""
Tests that has_changes() only returns true when changes are present
"""
draft_course = CourseLocator(org='testx', offering='GreekHero', branch='draft')
published_course = CourseLocator(org='testx', offering='GreekHero', branch='published')
head = draft_course.make_usage_key('course', 'head12345')
dummy_user = 'testUser'
# Not yet published, so changes are present
self.assertTrue(modulestore().has_changes(head))
# Publish and verify that there are no unpublished changes
modulestore().xblock_publish(dummy_user, draft_course, published_course, [head], None)
self.assertFalse(modulestore().has_changes(head))
# Change the course, then check that there now are changes
course = modulestore().get_item(head)
course.show_calculator = not course.show_calculator
modulestore().update_item(course, dummy_user)
self.assertTrue(modulestore().has_changes(head))
# Publish and verify again
modulestore().xblock_publish(dummy_user, draft_course, published_course, [head], None)
self.assertFalse(modulestore().has_changes(head))
def test_get_non_root(self):
# not a course obj
locator = BlockUsageLocator(