From f500b72290e3edf1e3c60b87bced55316d1466ee Mon Sep 17 00:00:00 2001
From: Calen Pennington <cale@edx.org>
Date: Tue, 17 Sep 2013 11:55:34 -0400
Subject: [PATCH] Make sure that we have the right set of fields available
during xml parsing
We had a bug where mixins weren't being applied before `load_from_xml`
was called. This meant that not all of the fields were being loaded
correctly. To fix it, we used the mixoligist from the runtime to apply
the mixins earlier in the process. However, that caused the mixins to be
applied twice.
The included fixes to xblock resolved the multiply-applied mixins, and
the fixes to the parsing code make it simpler to understand, and add
some unit tests of the parsing to boot.
---
.../lib/xmodule/xmodule/tests/test_import.py | 1 -
.../xmodule/xmodule/tests/test_xml_module.py | 97 ++++++++++++-
.../lib/xmodule/xmodule/tests/xml/__init__.py | 46 +++++++
.../xmodule/xmodule/tests/xml/factories.py | 128 ++++++++++++++++++
.../xmodule/tests/xml/test_inheritance.py | 29 ++++
.../xmodule/xmodule/tests/xml/test_policy.py | 30 ++++
common/lib/xmodule/xmodule/video_module.py | 35 ++---
common/lib/xmodule/xmodule/xml_module.py | 81 +++--------
lms/djangoapps/courseware/module_render.py | 2 +-
pylintrc | 8 +-
requirements/edx/base.txt | 2 +-
requirements/edx/github.txt | 4 +-
12 files changed, 370 insertions(+), 93 deletions(-)
create mode 100644 common/lib/xmodule/xmodule/tests/xml/__init__.py
create mode 100644 common/lib/xmodule/xmodule/tests/xml/factories.py
create mode 100644 common/lib/xmodule/xmodule/tests/xml/test_inheritance.py
create mode 100644 common/lib/xmodule/xmodule/tests/xml/test_policy.py
diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py
index cd7749dc09d..19232e383fc 100644
--- a/common/lib/xmodule/xmodule/tests/test_import.py
+++ b/common/lib/xmodule/xmodule/tests/test_import.py
@@ -91,7 +91,6 @@ class ImportTestCase(BaseCourseTestCase):
self.assertNotEqual(descriptor1.location, descriptor2.location)
- @unittest.skip('Temporarily disabled')
def test_reimport(self):
'''Make sure an already-exported error xml tag loads properly'''
diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py
index 1515c76237d..9d11b2b168f 100644
--- a/common/lib/xmodule/xmodule/tests/test_xml_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py
@@ -1,17 +1,25 @@
# disable missing docstring
#pylint: disable=C0111
-from xmodule.x_module import XModuleFields
-from xblock.fields import Scope, String, Dict, Boolean, Integer, Float, Any, List
-from xblock.field_data import DictFieldData
-from xmodule.fields import Date, Timedelta
-from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field
import unittest
-from nose.tools import assert_equals # pylint: disable=E0611
+
from mock import Mock
-from xmodule.modulestore.inheritance import InheritanceKeyValueStore, InheritanceMixin
+from nose.tools import assert_equals, assert_not_equals, assert_true, assert_false, assert_in, assert_not_in # pylint: disable=E0611
+
+from xblock.field_data import DictFieldData
+from xblock.fields import Scope, String, Dict, Boolean, Integer, Float, Any, List
from xblock.runtime import DbModel
+
+from xmodule.fields import Date, Timedelta
+from xmodule.modulestore.inheritance import InheritanceKeyValueStore, InheritanceMixin
+from xmodule.x_module import XModuleFields
+from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field
+from xmodule.course_module import CourseDescriptor
+from xmodule.seq_module import SequenceDescriptor
+
from xmodule.tests import get_test_descriptor_system
+from xmodule.tests.xml import XModuleXmlImportTest
+from xmodule.tests.xml.factories import CourseFactory, SequenceFactory, ProblemFactory
class CrazyJsonString(String):
@@ -379,3 +387,78 @@ class TestDeserializeTimedelta(TestDeserialize):
self.assertDeserializeEqual('1 day 12 hours 59 minutes 59 seconds',
'"1 day 12 hours 59 minutes 59 seconds"')
self.assertDeserializeNonString()
+
+
+class TestXmlAttributes(XModuleXmlImportTest):
+
+ def test_unknown_attribute(self):
+ assert_false(hasattr(CourseDescriptor, 'unknown_attr'))
+ course = self.process_xml(CourseFactory.build(unknown_attr='value'))
+ assert_false(hasattr(course, 'unknown_attr'))
+ assert_equals('value', course.xml_attributes['unknown_attr'])
+
+ def test_known_attribute(self):
+ assert_true(hasattr(CourseDescriptor, 'show_chat'))
+ course = self.process_xml(CourseFactory.build(show_chat='true'))
+ assert_true(course.show_chat)
+ assert_not_in('show_chat', course.xml_attributes)
+
+ def test_rerandomize_in_policy(self):
+ # Rerandomize isn't a basic attribute of Sequence
+ assert_false(hasattr(SequenceDescriptor, 'rerandomize'))
+
+ root = SequenceFactory.build(policy={'rerandomize': 'never'})
+ ProblemFactory.build(parent=root)
+
+ seq = self.process_xml(root)
+
+ # Rerandomize is added to the constructed sequence via the InheritanceMixin
+ assert_equals('never', seq.rerandomize)
+
+ # Rerandomize is a known value coming from policy, and shouldn't appear
+ # in xml_attributes
+ assert_not_in('rerandomize', seq.xml_attributes)
+
+ def test_attempts_in_policy(self):
+ # attempts isn't a basic attribute of Sequence
+ assert_false(hasattr(SequenceDescriptor, 'attempts'))
+
+ root = SequenceFactory.build(policy={'attempts': '1'})
+ ProblemFactory.build(parent=root)
+
+ seq = self.process_xml(root)
+
+ # attempts isn't added to the constructed sequence, because
+ # it's not in the InheritanceMixin
+ assert_false(hasattr(seq, 'attempts'))
+
+ # attempts is an unknown attribute, so we should include it
+ # in xml_attributes so that it gets written out (despite the misleading
+ # name)
+ assert_in('attempts', seq.xml_attributes)
+
+ def test_inheritable_attribute(self):
+ # days_early_for_beta isn't a basic attribute of Sequence
+ assert_false(hasattr(SequenceDescriptor, 'days_early_for_beta'))
+
+ # days_early_for_beta is added by InheritanceMixin
+ assert_true(hasattr(InheritanceMixin, 'days_early_for_beta'))
+
+ root = SequenceFactory.build(policy={'days_early_for_beta': '2'})
+ ProblemFactory.build(parent=root)
+
+ # InheritanceMixin will be used when processing the XML
+ assert_in(InheritanceMixin, root.xblock_mixins)
+
+ seq = self.process_xml(root)
+
+ assert_equals(seq.unmixed_class, SequenceDescriptor)
+ assert_not_equals(type(seq), SequenceDescriptor)
+
+ # days_early_for_beta is added to the constructed sequence, because
+ # it's in the InheritanceMixin
+ assert_equals(2, seq.days_early_for_beta)
+
+ # days_early_for_beta is a known attribute, so we shouldn't include it
+ # in xml_attributes
+ assert_not_in('days_early_for_beta', seq.xml_attributes)
diff --git a/common/lib/xmodule/xmodule/tests/xml/__init__.py b/common/lib/xmodule/xmodule/tests/xml/__init__.py
new file mode 100644
index 00000000000..32cefadca72
--- /dev/null
+++ b/common/lib/xmodule/xmodule/tests/xml/__init__.py
@@ -0,0 +1,46 @@
+"""
+Xml parsing tests for XModules
+"""
+import pprint
+from mock import Mock
+
+from xmodule.x_module import XMLParsingSystem, XModuleDescriptor
+from xmodule.mako_module import MakoDescriptorSystem
+
+
+class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable=abstract-method
+ """
+ The simplest possible XMLParsingSystem
+ """
+ def __init__(self, xml_import_data):
+ self.org = xml_import_data.org
+ self.course = xml_import_data.course
+ self.default_class = xml_import_data.default_class
+ self._descriptors = {}
+ super(InMemorySystem, self).__init__(
+ policy=xml_import_data.policy,
+ process_xml=self.process_xml,
+ load_item=self.load_item,
+ error_tracker=Mock(),
+ resources_fs=xml_import_data.filesystem,
+ mixins=xml_import_data.xblock_mixins,
+ render_template=lambda template, context: pprint.pformat((template, context))
+ )
+
+ def process_xml(self, xml): # pylint: disable=method-hidden
+ """Parse `xml` as an XModuleDescriptor, and add it to `self._descriptors`"""
+ descriptor = XModuleDescriptor.load_from_xml(xml, self, self.org, self.course, self.default_class)
+ self._descriptors[descriptor.location.url()] = descriptor
+ return descriptor
+
+ def load_item(self, location): # pylint: disable=method-hidden
+ """Return the descriptor loaded for `location`"""
+ return self._descriptors[location]
+
+
+class XModuleXmlImportTest(object):
+ """Base class for tests that use basic `XModuleDescriptor.load_from_xml` xml parsing"""
+ def process_xml(self, xml_import_data):
+ """Use the `xml_import_data` to import an :class:`XModuleDescriptor` from xml"""
+ system = InMemorySystem(xml_import_data)
+ return system.process_xml(xml_import_data.xml_string)
diff --git a/common/lib/xmodule/xmodule/tests/xml/factories.py b/common/lib/xmodule/xmodule/tests/xml/factories.py
new file mode 100644
index 00000000000..168dcd579ba
--- /dev/null
+++ b/common/lib/xmodule/xmodule/tests/xml/factories.py
@@ -0,0 +1,128 @@
+"""
+Factories for generating edXML for testing XModule import
+"""
+
+import inspect
+
+from fs.memoryfs import MemoryFS
+from factory import Factory, lazy_attribute, post_generation, Sequence
+from lxml import etree
+
+from xmodule.modulestore.inheritance import InheritanceMixin
+
+
+class XmlImportData(object):
+ """
+ Class to capture all of the data needed to actually run an XML import,
+ so that the Factories have something to generate
+ """
+ def __init__(self, xml_node, xml=None, org=None, course=None,
+ default_class=None, policy=None,
+ filesystem=None, parent=None,
+ xblock_mixins=()):
+
+ self._xml_node = xml_node
+ self._xml_string = xml
+ self.org = org
+ self.course = course
+ self.default_class = default_class
+ self.filesystem = filesystem
+ self.xblock_mixins = xblock_mixins
+ self.parent = parent
+
+ if policy is None:
+ self.policy = {}
+ else:
+ self.policy = policy
+
+ @property
+ def xml_string(self):
+ """Return the stringified version of the generated xml"""
+ if self._xml_string is not None:
+ return self._xml_string
+
+ return etree.tostring(self._xml_node)
+
+ def __repr__(self):
+ return u"XmlImportData{!r}".format((
+ self._xml_node, self._xml_string, self.org,
+ self.course, self.default_class, self.policy,
+ self.filesystem, self.parent, self.xblock_mixins
+ ))
+
+
+# Extract all argument names used to construct XmlImportData objects,
+# so that the factory doesn't treat them as XML attributes
+XML_IMPORT_ARGS = inspect.getargspec(XmlImportData.__init__).args
+
+
+class XmlImportFactory(Factory):
+ """
+ Factory for generating XmlImportData's, which can hold all the data needed
+ to run an XModule XML import
+ """
+ FACTORY_FOR = XmlImportData
+
+ filesystem = MemoryFS()
+ xblock_mixins = (InheritanceMixin,)
+ url_name = Sequence(str)
+ attribs = {}
+ policy = {}
+ tag = 'unknown'
+
+ @classmethod
+ def _adjust_kwargs(cls, **kwargs):
+ """
+ Adjust the kwargs to be passed to the generated class.
+
+ Any kwargs that match :fun:`XmlImportData.__init__` will be passed
+ through. Any other unknown `kwargs` will be treated as XML attributes
+
+ :param tag: xml tag for the generated :class:`Element` node
+ :param text: (Optional) specifies the text of the generated :class:`Element`.
+ :param policy: (Optional) specifies data for the policy json file for this node
+ :type policy: dict
+ :param attribs: (Optional) specify attributes for the XML node
+ :type attribs: dict
+ """
+ tag = kwargs.pop('tag', 'unknown')
+ kwargs['policy'] = {'{tag}/{url_name}'.format(tag=tag, url_name=kwargs['url_name']): kwargs['policy']}
+
+ kwargs['xml_node'].text = kwargs.pop('text', None)
+
+ kwargs['xml_node'].attrib.update(kwargs.pop('attribs', {}))
+ for key in kwargs.keys():
+ if key not in XML_IMPORT_ARGS:
+ kwargs['xml_node'].set(key, kwargs.pop(key))
+
+ return kwargs
+
+ @lazy_attribute
+ def xml_node(self):
+ """An :class:`xml.etree.Element`"""
+ return etree.Element(self.tag)
+
+ @post_generation
+ def parent(self, _create, extracted, **_):
+ """Hook to merge this xml into a parent xml node"""
+ if extracted is None:
+ return
+
+ extracted._xml_node.append(self._xml_node) # pylint: disable=no-member, protected-access
+ extracted.policy.update(self.policy)
+
+
+class CourseFactory(XmlImportFactory):
+ """Factory for <course> nodes"""
+ tag = 'course'
+
+
+class SequenceFactory(XmlImportFactory):
+ """Factory for <sequential> nodes"""
+ tag = 'sequential'
+
+
+class ProblemFactory(XmlImportFactory):
+ """Factory for <problem> nodes"""
+ tag = 'problem'
+ text = '<h1>Empty Problem!</h1>'
diff --git a/common/lib/xmodule/xmodule/tests/xml/test_inheritance.py b/common/lib/xmodule/xmodule/tests/xml/test_inheritance.py
new file mode 100644
index 00000000000..dc27f0c7920
--- /dev/null
+++ b/common/lib/xmodule/xmodule/tests/xml/test_inheritance.py
@@ -0,0 +1,29 @@
+"""
+Test that inherited fields work correctly when parsing XML
+"""
+from nose.tools import assert_equals # pylint: disable=no-name-in-module
+
+from xmodule.tests.xml import XModuleXmlImportTest
+from xmodule.tests.xml.factories import CourseFactory, SequenceFactory, ProblemFactory
+
+
+class TestInheritedFieldParsing(XModuleXmlImportTest):
+ """
+ Test that inherited fields work correctly when parsing XML
+
+ """
+ def test_null_string(self):
+ # Test that the string inherited fields are passed through 'deserialize_field',
+ # which converts the string "null" to the python value None
+ root = CourseFactory.build(days_early_for_beta="null")
+ sequence = SequenceFactory.build(parent=root)
+ ProblemFactory.build(parent=sequence)
+
+ course = self.process_xml(root)
+ assert_equals(None, course.days_early_for_beta)
+
+ sequence = course.get_children()[0]
+ assert_equals(None, sequence.days_early_for_beta)
+
+ problem = sequence.get_children()[0]
+ assert_equals(None, problem.days_early_for_beta)
diff --git a/common/lib/xmodule/xmodule/tests/xml/test_policy.py b/common/lib/xmodule/xmodule/tests/xml/test_policy.py
new file mode 100644
index 00000000000..2e702cc82d1
--- /dev/null
+++ b/common/lib/xmodule/xmodule/tests/xml/test_policy.py
@@ -0,0 +1,30 @@
+"""
+Tests that policy json files import correctly when loading XML
+"""
+
+from nose.tools import assert_equals, assert_raises # pylint: disable=no-name-in-module
+
+from xmodule.tests.xml.factories import CourseFactory
+from xmodule.tests.xml import XModuleXmlImportTest
+
+
+class TestPolicy(XModuleXmlImportTest):
+ """
+ Tests that policy json files import correctly when loading xml
+ """
+ def test_no_attribute_mapping(self):
+ # Policy files are json, and thus the values aren't passed through 'deserialize_field'
+ # Therefor, the string 'null' is passed unchanged to the Float field, which will trigger
+ # a ValueError
+ with assert_raises(ValueError):
+ course = self.process_xml(CourseFactory.build(policy={'days_early_for_beta': 'null'}))
+
+ # Trigger the exception by looking at the imported data
+ course.days_early_for_beta # pylint: disable=pointless-statement
+
+ def test_course_policy(self):
+ course = self.process_xml(CourseFactory.build(policy={'days_early_for_beta': None}))
+ assert_equals(None, course.days_early_for_beta)
+
+ course = self.process_xml(CourseFactory.build(policy={'days_early_for_beta': 9}))
+ assert_equals(9, course.days_early_for_beta)
diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py
index d97d5aa144a..febfe5961f8 100644
--- a/common/lib/xmodule/xmodule/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module.py
@@ -24,7 +24,7 @@ from django.conf import settings
from xmodule.x_module import XModule
from xmodule.editing_module import TabsEditingDescriptor
from xmodule.raw_module import EmptyDataRawDescriptor
-from xmodule.xml_module import is_pointer_tag, name_to_pathname
+from xmodule.xml_module import is_pointer_tag, name_to_pathname, deserialize_field
from xmodule.modulestore import Location
from xblock.fields import Scope, String, Boolean, Float, List, Integer, ScopeIds
@@ -217,7 +217,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
# For backwards compatibility -- if we've got XML data, parse
# it out and set the metadata fields
if self.data:
- field_data = VideoDescriptor._parse_video_xml(self.data)
+ field_data = self._parse_video_xml(self.data)
self._field_data.set_many(self, field_data)
del self.data
@@ -241,7 +241,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
if is_pointer_tag(xml_object):
filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name))
xml_data = etree.tostring(cls.load_file(filepath, system.resources_fs, location))
- field_data = VideoDescriptor._parse_video_xml(xml_data)
+ field_data = cls._parse_video_xml(xml_data)
field_data['location'] = location
kvs = InheritanceKeyValueStore(initial_values=field_data)
field_data = DbModel(kvs)
@@ -292,8 +292,8 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
xml.append(ele)
return xml
- @staticmethod
- def _parse_youtube(data):
+ @classmethod
+ def _parse_youtube(cls, data):
"""
Parses a string of Youtube IDs such as "1.0:AXdE34_U,1.5:VO3SxfeD"
into a dictionary. Necessary for backwards compatibility with
@@ -310,14 +310,14 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
# Handle the fact that youtube IDs got double-quoted for a period of time.
# Note: we pass in "VideoFields.youtube_id_1_0" so we deserialize as a String--
# it doesn't matter what the actual speed is for the purposes of deserializing.
- youtube_id = VideoDescriptor._deserialize(VideoFields.youtube_id_1_0.name, pieces[1])
+ youtube_id = deserialize_field(cls.youtube_id_1_0, pieces[1])
ret[speed] = youtube_id
except (ValueError, IndexError):
log.warning('Invalid YouTube ID: %s' % video)
return ret
- @staticmethod
- def _parse_video_xml(xml_data):
+ @classmethod
+ def _parse_video_xml(cls, xml_data):
"""
Parse video fields out of xml_data. The fields are set if they are
present in the XML.
@@ -326,8 +326,8 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
field_data = {}
conversions = {
- 'start_time': VideoDescriptor._parse_time,
- 'end_time': VideoDescriptor._parse_time
+ 'start_time': cls._parse_time,
+ 'end_time': cls._parse_time
}
# Convert between key names for certain attributes --
@@ -349,10 +349,10 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
for attr, value in xml.items():
if attr in compat_keys:
attr = compat_keys[attr]
- if attr in VideoDescriptor.metadata_to_strip + ('url_name', 'name'):
+ if attr in cls.metadata_to_strip + ('url_name', 'name'):
continue
if attr == 'youtube':
- speeds = VideoDescriptor._parse_youtube(value)
+ speeds = cls._parse_youtube(value)
for speed, youtube_id in speeds.items():
# should have made these youtube_id_1_00 for
# cleanliness, but hindsight doesn't need glasses
@@ -367,20 +367,13 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
else:
# We export values with json.dumps (well, except for Strings, but
# for about a month we did it for Strings also).
- value = VideoDescriptor._deserialize(attr, value)
+ value = deserialize_field(cls.fields[attr], value)
field_data[attr] = value
return field_data
@classmethod
- def _deserialize(cls, attr, value):
- """
- Handles deserializing values that may have been encoded with json.dumps.
- """
- return cls.get_map_for_field(attr).from_xml(value)
-
- @staticmethod
- def _parse_time(str_time):
+ def _parse_time(cls, str_time):
"""Converts s in '12:34:45' format to seconds. If s is
None, returns empty string"""
if not str_time:
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 5fa9ff3260f..83bb6faffdc 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -62,23 +62,6 @@ def get_metadata_from_xml(xml_object, remove=True):
xml_object.remove(meta)
return dmdata
-_AttrMapBase = namedtuple('_AttrMap', 'from_xml to_xml')
-
-
-class AttrMap(_AttrMapBase):
- """
- A class that specifies two functions:
-
- from_xml: convert value from the xml representation into
- an internal python representation
-
- to_xml: convert the internal python representation into
- the value to store in the xml.
- """
- def __new__(_cls, from_xml=lambda x: x,
- to_xml=lambda x: x):
- return _AttrMapBase.__new__(_cls, from_xml, to_xml)
-
def serialize_field(value):
"""
@@ -166,20 +149,6 @@ class XmlDescriptor(XModuleDescriptor):
metadata_to_export_to_policy = ('discussion_topics', 'checklists')
- @classmethod
- def get_map_for_field(cls, attr):
- """
- Returns a serialize/deserialize AttrMap for the given field of a class.
-
- Searches through fields defined by cls to find one named attr.
- """
- if attr in cls.fields:
- from_xml = lambda val: deserialize_field(cls.fields[attr], val)
- to_xml = lambda val: serialize_field(val)
- return AttrMap(from_xml, to_xml)
- else:
- return AttrMap()
-
@classmethod
def definition_from_xml(cls, xml_object, system):
"""
@@ -274,19 +243,19 @@ class XmlDescriptor(XModuleDescriptor):
Returns a dictionary {key: value}.
"""
- metadata = {}
- for attr in xml_object.attrib:
- val = xml_object.get(attr)
- if val is not None:
- # VS[compat]. Remove after all key translations done
- attr = cls._translate(attr)
-
- if attr in cls.metadata_to_strip:
- # don't load these
- continue
-
- attr_map = cls.get_map_for_field(attr)
- metadata[attr] = attr_map.from_xml(val)
+ metadata = {'xml_attributes': {}}
+ for attr, val in xml_object.attrib.iteritems():
+ # VS[compat]. Remove after all key translations done
+ attr = cls._translate(attr)
+
+ if attr in cls.metadata_to_strip:
+ # don't load these
+ continue
+
+ if attr not in cls.fields:
+ metadata['xml_attributes'][attr] = val
+ else:
+ metadata[attr] = deserialize_field(cls.fields[attr], val)
return metadata
@classmethod
@@ -295,9 +264,14 @@ class XmlDescriptor(XModuleDescriptor):
Add the keys in policy to metadata, after processing them
through the attrmap. Updates the metadata dict in place.
"""
- for attr in policy:
- attr_map = cls.get_map_for_field(attr)
- metadata[cls._translate(attr)] = attr_map.from_xml(policy[attr])
+ for attr, value in policy.iteritems():
+ attr = cls._translate(attr)
+ if attr not in cls.fields:
+ # Store unknown attributes coming from policy.json
+ # in such a way that they will export to xml unchanged
+ metadata['xml_attributes'][attr] = value
+ else:
+ metadata[attr] = value
@classmethod
def from_xml(cls, xml_data, system, org=None, course=None):
@@ -357,11 +331,7 @@ class XmlDescriptor(XModuleDescriptor):
field_data.update(definition)
field_data['children'] = children
- field_data['xml_attributes'] = {}
field_data['xml_attributes']['filename'] = definition.get('filename', ['', None]) # for git link
- for key, value in metadata.items():
- if key not in cls.fields:
- field_data['xml_attributes'][key] = value
field_data['location'] = location
field_data['category'] = xml_object.tag
kvs = InheritanceKeyValueStore(initial_values=field_data)
@@ -415,18 +385,11 @@ class XmlDescriptor(XModuleDescriptor):
# Set the tag so we get the file path right
xml_object.tag = self.category
- def val_for_xml(attr):
- """Get the value for this attribute that we want to store.
- (Possible format conversion through an AttrMap).
- """
- attr_map = self.get_map_for_field(attr)
- return attr_map.to_xml(self._field_data.get(self, attr))
-
# Add the non-inherited metadata
for attr in sorted(own_metadata(self)):
# don't want e.g. data_dir
if attr not in self.metadata_to_strip and attr not in self.metadata_to_export_to_policy:
- val = val_for_xml(attr)
+ val = serialize_field(self._field_data.get(self, attr))
try:
xml_object.set(attr, val)
except Exception, e:
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index acdd4f0aa0c..f86bef29ade 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -306,7 +306,7 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
# Construct the key for the module
key = KeyValueStore.Key(
scope=Scope.user_state,
- student_id=user.id,
+ user_id=user.id,
block_scope_id=descriptor.location,
field_name='grade'
)
diff --git a/pylintrc b/pylintrc
index 9525f04362e..a3c84c15558 100644
--- a/pylintrc
+++ b/pylintrc
@@ -89,6 +89,9 @@ evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / stateme
# evaluation report (RP0004).
comment=no
+# Display symbolic names of messages in reports
+symbols=yes
+
[TYPECHECK]
@@ -120,7 +123,10 @@ generated-members=
content,
status_code,
# For factory_boy factories
- create
+ create,
+ build,
+# For xblocks
+ fields,
[BASIC]
diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt
index addef7b629c..52fe3ce44f0 100644
--- a/requirements/edx/base.txt
+++ b/requirements/edx/base.txt
@@ -47,7 +47,7 @@ Pillow==1.7.8
pip>=1.4
polib==1.0.3
pycrypto>=2.6
-pygments==1.5
+pygments==1.6
pygraphviz==1.1
pymongo==2.4.1
pyparsing==1.5.6
diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt
index add9bda1d1c..e0c88b217e8 100644
--- a/requirements/edx/github.txt
+++ b/requirements/edx/github.txt
@@ -14,8 +14,8 @@
-e git+https://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk
# Our libraries:
--e git+https://github.com/edx/XBlock.git@aa0d60627#egg=XBlock
+-e git+https://github.com/edx/XBlock.git@a8de02c0#egg=XBlock
-e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail
--e git+https://github.com/edx/diff-cover.git@v0.2.3#egg=diff_cover
+-e git+https://github.com/edx/diff-cover.git@v0.2.4#egg=diff_cover
-e git+https://github.com/edx/js-test-tool.git@v0.0.7#egg=js_test_tool
-e git+https://github.com/edx/django-waffle.git@823a102e48#egg=django-waffle
--
GitLab