Skip to content

E
edx-platform
Commit de5e8856 by Don Mitchell
Options

Make XmlModuleStore handle asides 

PLAT-221
parent 128d7630
Showing with 297 additions and 126 deletions
cms/djangoapps/contentstore/views/item.py
...@@ -45,7 +45,7 @@ from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primar ...@@ -45,7 +45,7 @@ from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primar
from contentstore.views.preview import get_preview_fragment from contentstore.views.preview import get_preview_fragment
from edxmako.shortcuts import render_to_string from edxmako.shortcuts import render_to_string
from models.settings.course_grading import CourseGradingModel from models.settings.course_grading import CourseGradingModel
from cms.lib.xblock.runtime import handler_url, local_resource_url, get_asides from cms.lib.xblock.runtime import handler_url, local_resource_url, applicable_aside_types
from opaque_keys.edx.keys import UsageKey, CourseKey from opaque_keys.edx.keys import UsageKey, CourseKey
__all__ = ['orphan_handler', 'xblock_handler', 'xblock_view_handler', 'xblock_outline_handler'] __all__ = ['orphan_handler', 'xblock_handler', 'xblock_view_handler', 'xblock_outline_handler']
...@@ -64,7 +64,7 @@ ALWAYS = lambda x: True ...@@ -64,7 +64,7 @@ ALWAYS = lambda x: True
# TODO: Remove this code when Runtimes are no longer created by modulestores # TODO: Remove this code when Runtimes are no longer created by modulestores
xmodule.x_module.descriptor_global_handler_url = handler_url xmodule.x_module.descriptor_global_handler_url = handler_url
xmodule.x_module.descriptor_global_local_resource_url = local_resource_url xmodule.x_module.descriptor_global_local_resource_url = local_resource_url
xmodule.x_module.descriptor_global_get_asides = get_asides xmodule.x_module.descriptor_global_applicable_aside_types = applicable_aside_types
def hash_resource(resource): def hash_resource(resource):
......
cms/djangoapps/contentstore/views/preview.py
...@@ -95,7 +95,7 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method ...@@ -95,7 +95,7 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method
def local_resource_url(self, block, uri): def local_resource_url(self, block, uri):
return local_resource_url(block, uri) return local_resource_url(block, uri)
def get_asides(self, block): def applicable_aside_types(self, block):
# TODO: Implement this to enable XBlockAsides on previews in Studio # TODO: Implement this to enable XBlockAsides on previews in Studio
return [] return []
......
cms/lib/xblock/runtime.py
...@@ -35,12 +35,9 @@ def local_resource_url(block, uri): ...@@ -35,12 +35,9 @@ def local_resource_url(block, uri):
}) })
def get_asides(block): # pylint: disable=unused-argument def applicable_aside_types(block): # pylint: disable=unused-argument
""" """
Return all of the asides which might be decorating this `block`. Get the application-relative list of aside types for this type of block.
Arguments:
block (:class:`.XBlock`): The block to render retrieve asides for.
""" """
# TODO: Implement this method to make XBlockAsides for editing views in Studio # TODO: Implement this method to make XBlockAsides for editing views in Studio
return [] return []
common/lib/xmodule/xmodule/html_module.py
...@@ -136,7 +136,7 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor): ...@@ -136,7 +136,7 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor):
# snippets that will be included in the middle of pages. # snippets that will be included in the middle of pages.
@classmethod @classmethod
def load_definition(cls, xml_object, system, location): def load_definition(cls, xml_object, system, location, id_generator):
'''Load a descriptor from the specified xml_object: '''Load a descriptor from the specified xml_object:
If there is a filename attribute, load it as a string, and If there is a filename attribute, load it as a string, and
...@@ -145,6 +145,12 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor): ...@@ -145,6 +145,12 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor):
If there is not a filename attribute, the definition is the body If there is not a filename attribute, the definition is the body
of the xml_object, without the root tag (do not want <html> in the of the xml_object, without the root tag (do not want <html> in the
middle of a page) middle of a page)
Args:
xml_object: an lxml.etree._Element containing the definition to load
system: the modulestore system or runtime which caches data
location: the usage id for the block--used to compute the filename if none in the xml_object
id_generator: used by other impls of this method to generate the usage_id
''' '''
filename = xml_object.get('filename') filename = xml_object.get('filename')
if filename is None: if filename is None:
......
common/lib/xmodule/xmodule/modulestore/mongo/base.py
...@@ -1275,8 +1275,10 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo ...@@ -1275,8 +1275,10 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
""" """
jsonfields = {} jsonfields = {}
for field_name, field in xblock.fields.iteritems(): for field_name, field in xblock.fields.iteritems():
if (field.scope == scope and field.is_set_on(xblock)): if field.scope == scope and field.is_set_on(xblock):
if isinstance(field, Reference): if field.scope == Scope.parent:
continue
elif isinstance(field, Reference):
jsonfields[field_name] = unicode(field.read_from(xblock)) jsonfields[field_name] = unicode(field.read_from(xblock))
elif isinstance(field, ReferenceList): elif isinstance(field, ReferenceList):
jsonfields[field_name] = [ jsonfields[field_name] = [
......
common/lib/xmodule/xmodule/modulestore/tests/test_asides.py 0 → 100644
"""
Tests for Asides
"""
from xblock.core import XBlockAside
from xblock.fields import Scope, String
from xblock.fragment import Fragment
from unittest import TestCase
from xmodule.modulestore.tests.test_cross_modulestore_import_export import XmlModulestoreBuilder
from mock import patch
class AsideTestType(XBlockAside):
"""
Test Aside type
"""
FRAG_CONTENT = u"<p>Aside rendered</p>"
content = String(default="default_content", scope=Scope.content)
data_field = String(default="default_data", scope=Scope.settings)
@XBlockAside.aside_for('student_view')
def student_view_aside(self, block, context): # pylint: disable=unused-argument
"""Add to the student view"""
return Fragment(self.FRAG_CONTENT)
class TestAsidesXmlStore(TestCase):
"""
Test Asides sourced from xml store
"""
@patch('xmodule.x_module.descriptor_global_applicable_aside_types', lambda block: ['test_aside'])
@XBlockAside.register_temp_plugin(AsideTestType, 'test_aside')
def test_xml_aside(self):
"""
Check that the xml modulestore read in all the asides with their values
"""
with XmlModulestoreBuilder().build(course_ids=['edX/aside_test/2012_Fall']) as store:
def check_block(block):
"""
Check whether block has the expected aside w/ its fields and then recurse to the block's children
"""
asides = block.runtime.get_asides(block)
self.assertEqual(len(asides), 1, "Found {} asides but expected only test_aside".format(asides))
self.assertIsInstance(asides[0], AsideTestType)
category = block.scope_ids.block_type
self.assertEqual(asides[0].data_field, "{} aside data".format(category))
self.assertEqual(asides[0].content, "{} Aside".format(category.capitalize()))
for child in block.get_children():
check_block(child)
check_block(store.get_course(store.make_course_key('edX', "aside_test", "2012_Fall")))
common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py
...@@ -203,6 +203,6 @@ class TestLibraries(MixedSplitTestCase): ...@@ -203,6 +203,6 @@ class TestLibraries(MixedSplitTestCase):
message = u"Hello world" message = u"Hello world"
hello_render = lambda _, context: Fragment(message) hello_render = lambda _, context: Fragment(message)
with patch('xmodule.html_module.HtmlDescriptor.author_view', hello_render, create=True): with patch('xmodule.html_module.HtmlDescriptor.author_view', hello_render, create=True):
with patch('xmodule.x_module.descriptor_global_get_asides', lambda block: []): with patch('xmodule.x_module.descriptor_global_applicable_aside_types', lambda block: []):
result = library.render(AUTHOR_VIEW, context) result = library.render(AUTHOR_VIEW, context)
self.assertIn(message, result.content) self.assertIn(message, result.content)
common/lib/xmodule/xmodule/modulestore/xml.py
...@@ -22,7 +22,6 @@ from xmodule.x_module import XMLParsingSystem, policy_key, OpaqueKeyReader, Asid ...@@ -22,7 +22,6 @@ from xmodule.x_module import XMLParsingSystem, policy_key, OpaqueKeyReader, Asid
from xmodule.modulestore.xml_exporter import DEFAULT_CONTENT_FIELDS from xmodule.modulestore.xml_exporter import DEFAULT_CONTENT_FIELDS
from xmodule.modulestore import ModuleStoreEnum, ModuleStoreReadBase from xmodule.modulestore import ModuleStoreEnum, ModuleStoreReadBase
from xmodule.tabs import CourseTabList from xmodule.tabs import CourseTabList
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
from opaque_keys.edx.locator import CourseLocator from opaque_keys.edx.locator import CourseLocator
...@@ -33,7 +32,7 @@ from xblock.runtime import DictKeyValueStore ...@@ -33,7 +32,7 @@ from xblock.runtime import DictKeyValueStore
from .exceptions import ItemNotFoundError from .exceptions import ItemNotFoundError
from .inheritance import compute_inherited_metadata, inheriting_field_data from .inheritance import compute_inherited_metadata, inheriting_field_data
from xblock.fields import ScopeIds, Reference, ReferenceList, ReferenceValueDict from xblock.fields import ScopeIds
edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False, edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False,
remove_comments=True, remove_blank_text=True) remove_comments=True, remove_blank_text=True)
...@@ -171,9 +170,9 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): ...@@ -171,9 +170,9 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
make_name_unique(xml_data) make_name_unique(xml_data)
descriptor = create_block_from_xml( descriptor = self.xblock_from_node(
etree.tostring(xml_data, encoding='unicode'), xml_data,
self, None, # parent_id
id_manager, id_manager,
) )
except Exception as err: # pylint: disable=broad-except except Exception as err: # pylint: disable=broad-except
...@@ -279,71 +278,6 @@ class CourseLocationManager(OpaqueKeyReader, AsideKeyGenerator): ...@@ -279,71 +278,6 @@ class CourseLocationManager(OpaqueKeyReader, AsideKeyGenerator):
return usage_id return usage_id
def _make_usage_key(course_key, value):
"""
Makes value into a UsageKey inside the specified course.
If value is already a UsageKey, returns that.
"""
if isinstance(value, UsageKey):
return value
return course_key.make_usage_key_from_deprecated_string(value)
def _convert_reference_fields_to_keys(xblock): # pylint: disable=invalid-name
"""
Find all fields of type reference and convert the payload into UsageKeys
"""
course_key = xblock.scope_ids.usage_id.course_key
for field in xblock.fields.itervalues():
if field.is_set_on(xblock):
field_value = getattr(xblock, field.name)
if isinstance(field, Reference):
setattr(xblock, field.name, _make_usage_key(course_key, field_value))
elif isinstance(field, ReferenceList):
setattr(xblock, field.name, [_make_usage_key(course_key, ele) for ele in field_value])
elif isinstance(field, ReferenceValueDict):
for key, subvalue in field_value.iteritems():
assert isinstance(subvalue, basestring)
field_value[key] = _make_usage_key(course_key, subvalue)
setattr(xblock, field.name, field_value)
def create_block_from_xml(xml_data, system, id_generator):
"""
Create an XBlock instance from XML data.
Args:
xml_data (string): A string containing valid xml.
system (XMLParsingSystem): The :class:`.XMLParsingSystem` used to connect the block
to the outside world.
id_generator (IdGenerator): An :class:`~xblock.runtime.IdGenerator` that
will be used to construct the usage_id and definition_id for the block.
Returns:
XBlock: The fully instantiated :class:`~xblock.core.XBlock`.
"""
node = etree.fromstring(xml_data)
raw_class = system.load_block_type(node.tag)
xblock_class = system.mixologist.mix(raw_class)
# leave next line commented out - useful for low-level debugging
# log.debug('[create_block_from_xml] tag=%s, class=%s' % (node.tag, xblock_class))
block_type = node.tag
url_name = node.get('url_name')
def_id = id_generator.create_definition(block_type, url_name)
usage_id = id_generator.create_usage(def_id)
scope_ids = ScopeIds(None, block_type, def_id, usage_id)
xblock = xblock_class.parse_xml(node, system, scope_ids, id_generator)
_convert_reference_fields_to_keys(xblock)
return xblock
class ParentTracker(object): class ParentTracker(object):
"""A simple class to factor out the logic for tracking location parent pointers.""" """A simple class to factor out the logic for tracking location parent pointers."""
def __init__(self): def __init__(self):
......
common/lib/xmodule/xmodule/modulestore/xml_importer.py
...@@ -480,6 +480,8 @@ def _import_module_and_update_references( ...@@ -480,6 +480,8 @@ def _import_module_and_update_references(
fields = {} fields = {}
for field_name, field in module.fields.iteritems(): for field_name, field in module.fields.iteritems():
if field.is_set_on(module): if field.is_set_on(module):
if field.scope == Scope.parent:
continue
if isinstance(field, Reference): if isinstance(field, Reference):
fields[field_name] = _convert_reference_fields_to_new_namespace(field.read_from(module)) fields[field_name] = _convert_reference_fields_to_new_namespace(field.read_from(module))
elif isinstance(field, ReferenceList): elif isinstance(field, ReferenceList):
......
common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py
...@@ -262,7 +262,7 @@ class XBlockWrapperTestMixin(object): ...@@ -262,7 +262,7 @@ class XBlockWrapperTestMixin(object):
This is a mixin for building tests of the implementation of the XBlock This is a mixin for building tests of the implementation of the XBlock
api by wrapping XModule native functions. api by wrapping XModule native functions.
You can creat an actual test case by inheriting from this class and UnitTest, You can create an actual test case by inheriting from this class and UnitTest,
and implement skip_if_invalid and check_property. and implement skip_if_invalid and check_property.
""" """
...@@ -289,6 +289,8 @@ class XBlockWrapperTestMixin(object): ...@@ -289,6 +289,8 @@ class XBlockWrapperTestMixin(object):
mocked_course = Mock() mocked_course = Mock()
modulestore = Mock() modulestore = Mock()
modulestore.get_course.return_value = mocked_course modulestore.get_course.return_value = mocked_course
# pylint: disable=no-member
descriptor.runtime.id_reader.get_definition_id = Mock(return_value='a')
descriptor.runtime.modulestore = modulestore descriptor.runtime.modulestore = modulestore
self.check_property(descriptor) self.check_property(descriptor)
...@@ -299,6 +301,8 @@ class XBlockWrapperTestMixin(object): ...@@ -299,6 +301,8 @@ class XBlockWrapperTestMixin(object):
descriptor_cls, fields = cls_and_fields descriptor_cls, fields = cls_and_fields
self.skip_if_invalid(descriptor_cls) self.skip_if_invalid(descriptor_cls)
descriptor = ContainerModuleFactory(descriptor_cls=descriptor_cls, depth=2, **fields) descriptor = ContainerModuleFactory(descriptor_cls=descriptor_cls, depth=2, **fields)
# pylint: disable=no-member
descriptor.runtime.id_reader.get_definition_id = Mock(return_value='a')
self.check_property(descriptor) self.check_property(descriptor)
# Test that when an xmodule is generated from descriptor_cls # Test that when an xmodule is generated from descriptor_cls
...@@ -347,7 +351,9 @@ class TestStudioView(XBlockWrapperTestMixin, TestCase): ...@@ -347,7 +351,9 @@ class TestStudioView(XBlockWrapperTestMixin, TestCase):
""" """
Assert that studio_view and get_html render the same. Assert that studio_view and get_html render the same.
""" """
self.assertEqual(descriptor.get_html(), descriptor.render(STUDIO_VIEW).content) html = descriptor.get_html()
rendered_content = descriptor.render(STUDIO_VIEW).content
self.assertEqual(html, rendered_content)
class TestXModuleHandler(TestCase): class TestXModuleHandler(TestCase):
......
common/lib/xmodule/xmodule/tests/xml/__init__.py
...@@ -2,12 +2,13 @@ ...@@ -2,12 +2,13 @@
Xml parsing tests for XModules Xml parsing tests for XModules
""" """
import pprint import pprint
from lxml import etree
from mock import Mock from mock import Mock
from unittest import TestCase from unittest import TestCase
from xmodule.x_module import XMLParsingSystem, policy_key from xmodule.x_module import XMLParsingSystem, policy_key
from xmodule.mako_module import MakoDescriptorSystem from xmodule.mako_module import MakoDescriptorSystem
from xmodule.modulestore.xml import create_block_from_xml, CourseLocationManager from xmodule.modulestore.xml import CourseLocationManager
from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
from xblock.runtime import KvsFieldData, DictKeyValueStore from xblock.runtime import KvsFieldData, DictKeyValueStore
...@@ -40,9 +41,9 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable ...@@ -40,9 +41,9 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable
def process_xml(self, xml): # pylint: disable=method-hidden def process_xml(self, xml): # pylint: disable=method-hidden
"""Parse `xml` as an XBlock, and add it to `self._descriptors`""" """Parse `xml` as an XBlock, and add it to `self._descriptors`"""
descriptor = create_block_from_xml( descriptor = self.xblock_from_node(
xml, etree.fromstring(xml),
self, None,
CourseLocationManager(self.course_id), CourseLocationManager(self.course_id),
) )
self._descriptors[descriptor.location.to_deprecated_string()] = descriptor self._descriptors[descriptor.location.to_deprecated_string()] = descriptor
......
common/lib/xmodule/xmodule/video_module/video_module.py
...@@ -18,7 +18,6 @@ Examples of html5 videos for manual testing: ...@@ -18,7 +18,6 @@ Examples of html5 videos for manual testing:
import copy import copy
import json import json
import logging import logging
import os.path
from collections import OrderedDict from collections import OrderedDict
from operator import itemgetter from operator import itemgetter
...@@ -30,14 +29,13 @@ from django.conf import settings ...@@ -30,14 +29,13 @@ from django.conf import settings
from xblock.fields import ScopeIds from xblock.fields import ScopeIds
from xblock.runtime import KvsFieldData from xblock.runtime import KvsFieldData
from xmodule.exceptions import NotFoundError
from xmodule.modulestore.inheritance import InheritanceKeyValueStore, own_metadata from xmodule.modulestore.inheritance import InheritanceKeyValueStore, own_metadata
from xmodule.x_module import XModule, module_attr from xmodule.x_module import XModule, module_attr
from xmodule.editing_module import TabsEditingDescriptor from xmodule.editing_module import TabsEditingDescriptor
from xmodule.raw_module import EmptyDataRawDescriptor from xmodule.raw_module import EmptyDataRawDescriptor
from xmodule.xml_module import is_pointer_tag, name_to_pathname, deserialize_field from xmodule.xml_module import is_pointer_tag, name_to_pathname, deserialize_field
from .transcripts_utils import Transcript, VideoTranscriptsMixin from .transcripts_utils import VideoTranscriptsMixin
from .video_utils import create_youtube_string, get_video_from_cdn from .video_utils import create_youtube_string, get_video_from_cdn
from .video_xfields import VideoFields from .video_xfields import VideoFields
from .video_handlers import VideoStudentViewHandlers, VideoStudioViewHandlers from .video_handlers import VideoStudentViewHandlers, VideoStudioViewHandlers
...@@ -300,7 +298,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler ...@@ -300,7 +298,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
super(VideoDescriptor, self).__init__(*args, **kwargs) super(VideoDescriptor, self).__init__(*args, **kwargs)
# For backwards compatibility -- if we've got XML data, parse it out and set the metadata fields # For backwards compatibility -- if we've got XML data, parse it out and set the metadata fields
if self.data: if self.data:
field_data = self._parse_video_xml(self.data) field_data = self._parse_video_xml(etree.fromstring(self.data))
self._field_data.set_many(self, field_data) self._field_data.set_many(self, field_data)
del self.data del self.data
...@@ -406,8 +404,9 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler ...@@ -406,8 +404,9 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
usage_id = id_generator.create_usage(definition_id) usage_id = id_generator.create_usage(definition_id)
if is_pointer_tag(xml_object): if is_pointer_tag(xml_object):
filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name)) filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name))
xml_data = etree.tostring(cls.load_file(filepath, system.resources_fs, usage_id)) xml_object = cls.load_file(filepath, system.resources_fs, usage_id)
field_data = cls._parse_video_xml(xml_data) system.parse_asides(xml_object, definition_id, usage_id, id_generator)
field_data = cls._parse_video_xml(xml_object)
kvs = InheritanceKeyValueStore(initial_values=field_data) kvs = InheritanceKeyValueStore(initial_values=field_data)
field_data = KvsFieldData(kvs) field_data = KvsFieldData(kvs)
video = system.construct_xblock_from_class( video = system.construct_xblock_from_class(
...@@ -543,12 +542,11 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler ...@@ -543,12 +542,11 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
return ret return ret
@classmethod @classmethod
def _parse_video_xml(cls, xml_data): def _parse_video_xml(cls, xml):
""" """
Parse video fields out of xml_data. The fields are set if they are Parse video fields out of xml_data. The fields are set if they are
present in the XML. present in the XML.
""" """
xml = etree.fromstring(xml_data)
field_data = {} field_data = {}
# Convert between key types for certain attributes -- # Convert between key types for certain attributes --
......
common/lib/xmodule/xmodule/x_module.py
...@@ -15,8 +15,9 @@ from pkg_resources import ( ...@@ -15,8 +15,9 @@ from pkg_resources import (
from webob import Response from webob import Response
from webob.multidict import MultiDict from webob.multidict import MultiDict
from xblock.core import XBlock from xblock.core import XBlock, XBlockAside
from xblock.fields import Scope, Integer, Float, List, XBlockMixin, String, Dict from xblock.fields import Scope, Integer, Float, List, XBlockMixin, String, Dict, ScopeIds, Reference, \
ReferenceList, ReferenceValueDict
from xblock.fragment import Fragment from xblock.fragment import Fragment
from xblock.runtime import Runtime, IdReader, IdGenerator from xblock.runtime import Runtime, IdReader, IdGenerator
from xmodule.fields import RelativeTime from xmodule.fields import RelativeTime
...@@ -852,6 +853,7 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock): ...@@ -852,6 +853,7 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock):
""" """
Interpret the parsed XML in `node`, creating an XModuleDescriptor. Interpret the parsed XML in `node`, creating an XModuleDescriptor.
""" """
# It'd be great to not reserialize and deserialize the xml
xml = etree.tostring(node) xml = etree.tostring(node)
block = cls.from_xml(xml, runtime, id_generator) block = cls.from_xml(xml, runtime, id_generator)
return block return block
...@@ -1131,14 +1133,13 @@ def descriptor_global_local_resource_url(block, uri): # pylint: disable=invalid ...@@ -1131,14 +1133,13 @@ def descriptor_global_local_resource_url(block, uri): # pylint: disable=invalid
raise NotImplementedError("Applications must monkey-patch this function before using local_resource_url for studio_view") raise NotImplementedError("Applications must monkey-patch this function before using local_resource_url for studio_view")
# This function exists to give applications (LMS/CMS) a place to monkey-patch until # pylint: disable=invalid-name
# we can refactor modulestore to split out the FieldData half of its interface from def descriptor_global_applicable_aside_types(block): # pylint: disable=unused-argument
# the Runtime part of its interface. This function matches the Runtime.get_asides interface
def descriptor_global_get_asides(block): # pylint: disable=unused-argument
""" """
See :meth:`xblock.runtime.Runtime.get_asides`. See :meth:`xblock.runtime.Runtime.applicable_aside_types`.
""" """
raise NotImplementedError("Applications must monkey-patch this function before using get_asides from a DescriptorSystem.") raise NotImplementedError("Applications must monkey-patch this function before using applicable_aside_types"
" from a DescriptorSystem.")
class MetricsMixin(object): class MetricsMixin(object):
...@@ -1315,18 +1316,18 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # p ...@@ -1315,18 +1316,18 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # p
# global function that the application can override. # global function that the application can override.
return descriptor_global_local_resource_url(block, uri) return descriptor_global_local_resource_url(block, uri)
def get_asides(self, block): def applicable_aside_types(self, block):
""" """
See :meth:`xblock.runtime.Runtime:get_asides` for documentation. See :meth:`xblock.runtime.Runtime:applicable_aside_types` for documentation.
""" """
if getattr(block, 'xmodule_runtime', None) is not None: if getattr(block, 'xmodule_runtime', None) is not None:
return block.xmodule_runtime.get_asides(block) return block.xmodule_runtime.applicable_aside_types(block)
else: else:
# Currently, Modulestore is responsible for instantiating DescriptorSystems # Currently, Modulestore is responsible for instantiating DescriptorSystems
# This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem
# that implements the correct get_asides. So, for now, instead, we will reference a # that implements the correct get_asides. So, for now, instead, we will reference a
# global function that the application can override. # global function that the application can override.
return descriptor_global_get_asides(block) return descriptor_global_applicable_aside_types(block)
def resource_url(self, resource): def resource_url(self, resource):
""" """
...@@ -1357,6 +1358,103 @@ class XMLParsingSystem(DescriptorSystem): ...@@ -1357,6 +1358,103 @@ class XMLParsingSystem(DescriptorSystem):
super(XMLParsingSystem, self).__init__(**kwargs) super(XMLParsingSystem, self).__init__(**kwargs)
self.process_xml = process_xml self.process_xml = process_xml
def _usage_id_from_node(self, node, parent_id, id_generator=None):
"""Create a new usage id from an XML dom node.
Args:
node (lxml.etree.Element): The DOM node to interpret.
parent_id: The usage ID of the parent block
id_generator (IdGenerator): The :class:`.IdGenerator` to use
for creating ids
Returns:
UsageKey: the usage key for the new xblock
"""
return self.xblock_from_node(node, parent_id, id_generator).scope_ids.usage_id
def xblock_from_node(self, node, parent_id, id_generator=None):
"""
Create an XBlock instance from XML data.
Args:
xml_data (string): A string containing valid xml.
system (XMLParsingSystem): The :class:`.XMLParsingSystem` used to connect the block
to the outside world.
id_generator (IdGenerator): An :class:`~xblock.runtime.IdGenerator` that
will be used to construct the usage_id and definition_id for the block.
Returns:
XBlock: The fully instantiated :class:`~xblock.core.XBlock`.
"""
id_generator = id_generator or self.id_generator
# leave next line commented out - useful for low-level debugging
# log.debug('[_usage_id_from_node] tag=%s, class=%s' % (node.tag, xblock_class))
block_type = node.tag
# remove xblock-family from elements
node.attrib.pop('xblock-family', None)
url_name = node.get('url_name') # difference from XBlock.runtime
def_id = id_generator.create_definition(block_type, url_name)
usage_id = id_generator.create_usage(def_id)
keys = ScopeIds(None, block_type, def_id, usage_id)
block_class = self.mixologist.mix(self.load_block_type(block_type))
self.parse_asides(node, def_id, usage_id, id_generator)
block = block_class.parse_xml(node, self, keys, id_generator)
self._convert_reference_fields_to_keys(block) # difference from XBlock.runtime
block.parent = parent_id
block.save()
return block
def parse_asides(self, node, def_id, usage_id, id_generator):
"""pull the asides out of the xml payload and instantiate them"""
aside_children = []
for child in node.iterchildren():
# get xblock-family from node
xblock_family = child.attrib.pop('xblock-family', None)
if xblock_family:
xblock_family = self._family_id_to_superclass(xblock_family)
if issubclass(xblock_family, XBlockAside):
aside_children.append(child)
# now process them & remove them from the xml payload
for child in aside_children:
self._aside_from_xml(child, def_id, usage_id, id_generator)
node.remove(child)
def _make_usage_key(self, course_key, value):
"""
Makes value into a UsageKey inside the specified course.
If value is already a UsageKey, returns that.
"""
if isinstance(value, UsageKey):
return value
return course_key.make_usage_key_from_deprecated_string(value)
def _convert_reference_fields_to_keys(self, xblock): # pylint: disable=invalid-name
"""
Find all fields of type reference and convert the payload into UsageKeys
"""
course_key = xblock.scope_ids.usage_id.course_key
for field in xblock.fields.itervalues():
if field.is_set_on(xblock):
field_value = getattr(xblock, field.name)
if field_value is None:
continue
elif isinstance(field, Reference):
setattr(xblock, field.name, self._make_usage_key(course_key, field_value))
elif isinstance(field, ReferenceList):
setattr(xblock, field.name, [self._make_usage_key(course_key, ele) for ele in field_value])
elif isinstance(field, ReferenceValueDict):
for key, subvalue in field_value.iteritems():
assert isinstance(subvalue, basestring)
field_value[key] = self._make_usage_key(course_key, subvalue)
setattr(xblock, field.name, field_value)
class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # pylint: disable=abstract-method class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # pylint: disable=abstract-method
""" """
......
common/lib/xmodule/xmodule/xml_module.py
...@@ -181,10 +181,18 @@ class XmlDescriptor(XModuleDescriptor): ...@@ -181,10 +181,18 @@ class XmlDescriptor(XModuleDescriptor):
raise Exception, msg, sys.exc_info()[2] raise Exception, msg, sys.exc_info()[2]
@classmethod @classmethod
def load_definition(cls, xml_object, system, def_id): def load_definition(cls, xml_object, system, def_id, id_generator):
'''Load a descriptor definition from the specified xml_object. '''
Load a descriptor definition from the specified xml_object.
Subclasses should not need to override this except in special Subclasses should not need to override this except in special
cases (e.g. html module)''' cases (e.g. html module)
Args:
xml_object: an lxml.etree._Element containing the definition to load
system: the modulestore system (aka, runtime) which accesses data and provides access to services
def_id: the definition id for the block--used to compute the usage id and asides ids
id_generator: used to generate the usage_id
'''
# VS[compat] -- the filename attr should go away once everything is # VS[compat] -- the filename attr should go away once everything is
# converted. (note: make sure html files still work once this goes away) # converted. (note: make sure html files still work once this goes away)
...@@ -208,6 +216,8 @@ class XmlDescriptor(XModuleDescriptor): ...@@ -208,6 +216,8 @@ class XmlDescriptor(XModuleDescriptor):
break break
definition_xml = cls.load_file(filepath, system.resources_fs, def_id) definition_xml = cls.load_file(filepath, system.resources_fs, def_id)
usage_id = id_generator.create_usage(def_id)
system.parse_asides(definition_xml, def_id, usage_id, id_generator)
# Add the attributes from the pointer node # Add the attributes from the pointer node
definition_xml.attrib.update(xml_object.attrib) definition_xml.attrib.update(xml_object.attrib)
...@@ -281,11 +291,12 @@ class XmlDescriptor(XModuleDescriptor): ...@@ -281,11 +291,12 @@ class XmlDescriptor(XModuleDescriptor):
# read the actual definition file--named using url_name.replace(':','/') # read the actual definition file--named using url_name.replace(':','/')
filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name)) filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name))
definition_xml = cls.load_file(filepath, system.resources_fs, def_id) definition_xml = cls.load_file(filepath, system.resources_fs, def_id)
system.parse_asides(definition_xml, def_id, usage_id, id_generator)
else: else:
definition_xml = xml_object definition_xml = xml_object
filepath = None filepath = None
definition, children = cls.load_definition(definition_xml, system, def_id) # note this removes metadata definition, children = cls.load_definition(definition_xml, system, def_id, id_generator) # note this removes metadata
# VS[compat] -- make Ike's github preview links work in both old and # VS[compat] -- make Ike's github preview links work in both old and
# new file layouts # new file layouts
......
common/test/data/aside/README.md 0 → 100644
This is a very very simple course, useful for initial debugging of processing code.
common/test/data/aside/course.xml 0 → 100644
<course org="edX" course="aside_test" url_name="2012_Fall"/>
\ No newline at end of file
common/test/data/aside/course/2012_Fall.xml 0 → 100644
<course course_image="just_a_test.jpg">
<test_aside xblock-family='xblock_asides.v1' data_field='course aside data'>Course Aside</test_aside>
<textbook title="Textbook" book_url="https://s3.amazonaws.com/edx-textbooks/guttag_computation_v3/"/>
<chapter url_name="Overview">
<test_aside xblock-family='xblock_asides.v1' data_field='chapter aside data'>Chapter Aside</test_aside>
<sequential url_name="Toy_Videos">
<test_aside xblock-family='xblock_asides.v1' data_field='sequential aside data'>Sequential Aside</test_aside>
<html url_name="toyhtml">
<test_aside xblock-family='xblock_asides.v1' data_field='html aside data'>Html Aside</test_aside>
</html>
</sequential>
</chapter>
</course>
common/test/data/aside/html/toyhtml.html 0 → 100644
<a href='/static/handouts/sample_handout.txt'>Sample</a>
\ No newline at end of file
common/test/data/aside/html/toyhtml.xml 0 → 100644
<html filename="toyhtml.html"/>
\ No newline at end of file
common/test/data/aside/info/handouts.html 0 → 100644
<a href='/static/handouts/sample_handout.txt'>Sample</a>
\ No newline at end of file
common/test/data/aside/policies/2012_Fall.json 0 → 100644
{
"course/2012_Fall": {
"graceperiod": "2 days 5 hours 59 minutes 59 seconds",
"start": "2015-07-17T12:00",
"display_name": "Toy Course",
"graded": "true",
},
"tabs": [
{"type": "courseware"},
{"type": "course_info", "name": "Course Info"},
{"type": "static_tab", "url_slug": "syllabus", "name": "Syllabus"},
{"type": "static_tab", "url_slug": "resources", "name": "Resources"},
{"type": "discussion", "name": "Discussion"},
{"type": "wiki", "name": "Wiki"},
{"type": "progress", "name": "Progress"}
],
"chapter/Overview": {
"display_name": "Overview"
},
"html/secret:toylab": {
"display_name": "Toy lab"
},
}
common/test/data/aside/policies/assets.json 0 → 100644
{
"textbook.pdf":{
"contentType":"text/pdf",
"displayname":"textbook.pdf",
"locked":false,
"filename":"/c4x/edx/toy/asset/textbook.pdf",
"import_path":null,
"thumbnail_location":null
}
}
common/test/data/aside/static/another_static.txt 0 → 100644
_
| |
_____ ____ _ _ __ ___ _ __ | | ___
/ _ \ \/ / _` | '_ ` _ \| '_ \| |/ _ \
| __/> < (_| | | | | | | |_) | | __/
\___/_/\_\__,_|_| |_| |_| .__/|_|\___|
| |
|_|
_ _ _
| | | | (_)
___| |_ __ _| |_ _ ___
/ __| __/ _` | __| |/ __|
\__ \ || (_| | |_| | (__
|___/\__\__,_|\__|_|\___|
common/test/data/toy/html/secret/toylab.html
<b>Lab 2A: Superposition Experiment</b> <b>Lab 2A: Superposition Experiment</b>
<<<<<<< Updated upstream
<p>Isn't the toy course great?</p> <p>Isn't the toy course great?</p>
<p>Let's add some markup that uses non-ascii characters. <p>Let's add some markup that uses non-ascii characters.
...@@ -8,6 +7,3 @@ For example, we should be able to write words like encyclop&aelig;dia, or foreig ...@@ -8,6 +7,3 @@ For example, we should be able to write words like encyclop&aelig;dia, or foreig
Looking beyond latin-1, we should handle math symbols: &pi;r&sup2 &le; &#8734. Looking beyond latin-1, we should handle math symbols: &pi;r&sup2 &le; &#8734.
And it shouldn't matter if we use entities or numeric codes &mdash; &Omega; &ne; &pi; &equiv; &#937; &#8800; &#960;. And it shouldn't matter if we use entities or numeric codes &mdash; &Omega; &ne; &pi; &equiv; &#937; &#8800; &#960;.
</p> </p>
=======
<p>Isn't the toy course great? — &le;</p>
>>>>>>> Stashed changes
lms/djangoapps/instructor/views/instructor_dashboard.py
...@@ -324,9 +324,9 @@ def _section_data_download(course, access): ...@@ -324,9 +324,9 @@ def _section_data_download(course, access):
return section_data return section_data
def null_get_asides(block): # pylint: disable=unused-argument def null_applicable_aside_types(block): # pylint: disable=unused-argument
""" """
get_aside method for monkey-patching into descriptor_global_get_asides get_aside method for monkey-patching into descriptor_global_applicable_aside_types
while rendering an HtmlDescriptor for email text editing. This returns while rendering an HtmlDescriptor for email text editing. This returns
an empty list. an empty list.
""" """
...@@ -337,8 +337,8 @@ def _section_send_email(course, access): ...@@ -337,8 +337,8 @@ def _section_send_email(course, access):
""" Provide data for the corresponding bulk email section """ """ Provide data for the corresponding bulk email section """
course_key = course.id course_key = course.id
# Monkey-patch descriptor_global_get_asides to return no asides for the duration of this render # Monkey-patch descriptor_global_applicable_aside_types to return no asides for the duration of this render
with patch('xmodule.x_module.descriptor_global_get_asides', null_get_asides): with patch('xmodule.x_module.descriptor_global_applicable_aside_types', null_applicable_aside_types):
# This HtmlDescriptor is only being used to generate a nice text editor. # This HtmlDescriptor is only being used to generate a nice text editor.
html_module = HtmlDescriptor( html_module = HtmlDescriptor(
course.system, course.system,
......
lms/djangoapps/lms_xblock/runtime.py
...@@ -226,7 +226,7 @@ class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract ...@@ -226,7 +226,7 @@ class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract
extra_data, extra_data,
) )
def get_asides(self, block): def applicable_aside_types(self, block):
""" """
Return all of the asides which might be decorating this `block`. Return all of the asides which might be decorating this `block`.
...@@ -242,8 +242,4 @@ class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract ...@@ -242,8 +242,4 @@ class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract
if block.scope_ids.block_type in config.disabled_blocks.split(): if block.scope_ids.block_type in config.disabled_blocks.split():
return [] return []
return [ return super(LmsModuleSystem, self).applicable_aside_types()
self.get_aside_of_type(block, aside_type)
for aside_type, __
in XBlockAside.load_classes()
]
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment