diff --git a/cms/templates/widgets/metadata-edit.html b/cms/templates/widgets/metadata-edit.html
index 9693c18e9c..aada438f38 100644
--- a/cms/templates/widgets/metadata-edit.html
+++ b/cms/templates/widgets/metadata-edit.html
@@ -8,7 +8,7 @@
% for field_name, field_value in editable_metadata_fields.items():
% if field_name == 'source_code':
- % if field_value['is_default'] is False:
+ % if field_value['explicitly_set'] is True:
Edit High Level Source
% endif
% else:
@@ -26,8 +26,10 @@
% if False:
-
-
+
+
+
+
% if field_value['field'].values:
% for value in field_value['field'].values:
@@ -40,7 +42,7 @@
% endfor
- % if 'source_code' in editable_metadata_fields and not editable_metadata_fields['source_code']['is_default']:
+ % if 'source_code' in editable_metadata_fields and editable_metadata_fields['source_code']['explicitly_set']:
<%include file="source-edit.html" />
% endif
diff --git a/common/lib/xmodule/xmodule/modulestore/inheritance.py b/common/lib/xmodule/xmodule/modulestore/inheritance.py
index 62b351999d..a816aa9776 100644
--- a/common/lib/xmodule/xmodule/modulestore/inheritance.py
+++ b/common/lib/xmodule/xmodule/modulestore/inheritance.py
@@ -31,15 +31,22 @@ def inherit_metadata(descriptor, model_data):
Only metadata specified in self.inheritable_metadata will
be inherited
"""
+ # The inherited values that are actually being used.
if not hasattr(descriptor, '_inherited_metadata'):
setattr(descriptor, '_inherited_metadata', {})
+ # All inheritable metadata values (for which a value exists in model_data).
+ if not hasattr(descriptor, '_inheritable_metadata'):
+ setattr(descriptor, '_inheritable_metadata', {})
+
# Set all inheritable metadata from kwargs that are
# in self.inheritable_metadata and aren't already set in metadata
for attr in INHERITABLE_METADATA:
- if attr not in descriptor._model_data and attr in model_data:
- descriptor._inherited_metadata[attr] = model_data[attr]
- descriptor._model_data[attr] = model_data[attr]
+ if attr in model_data:
+ descriptor._inheritable_metadata[attr] = model_data[attr]
+ if attr not in descriptor._model_data:
+ descriptor._inherited_metadata[attr] = model_data[attr]
+ descriptor._model_data[attr] = model_data[attr]
def own_metadata(module):
diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py
index 9d73fdcc17..aedca1d058 100644
--- a/common/lib/xmodule/xmodule/tests/test_import.py
+++ b/common/lib/xmodule/xmodule/tests/test_import.py
@@ -151,6 +151,10 @@ class ImportTestCase(BaseCourseTestCase):
# Check that the child inherits due correctly
child = descriptor.get_children()[0]
self.assertEqual(child.lms.due, Date().from_json(v))
+ self.assertEqual(child._inheritable_metadata, child._inherited_metadata)
+ self.assertEqual(2, len(child._inherited_metadata))
+ self.assertEqual('1970-01-01T00:00:00Z', child._inherited_metadata['start'])
+ self.assertEqual(v, child._inherited_metadata['due'])
# Now export and check things
resource_fs = MemoryFS()
@@ -184,6 +188,60 @@ class ImportTestCase(BaseCourseTestCase):
self.assertEqual(chapter_xml.tag, 'chapter')
self.assertFalse('due' in chapter_xml.attrib)
+ def test_metadata_no_inheritance(self):
+ """
+ Checks that default value of None (for due) does not get marked as inherited.
+ """
+ system = self.get_system()
+ url_name = 'test1'
+ start_xml = '''
+
+
+ Two houses, ...
+
+ '''.format(org=ORG, course=COURSE, url_name=url_name)
+ descriptor = system.process_xml(start_xml)
+ compute_inherited_metadata(descriptor)
+
+ self.assertEqual(descriptor.lms.due, None)
+
+ # Check that the child does not inherit a value for due
+ child = descriptor.get_children()[0]
+ self.assertEqual(child.lms.due, None)
+ self.assertEqual(child._inheritable_metadata, child._inherited_metadata)
+ self.assertEqual(1, len(child._inherited_metadata))
+ self.assertEqual('1970-01-01T00:00:00Z', child._inherited_metadata['start'])
+
+ def test_metadata_override_default(self):
+ """
+ Checks that due date can be overriden at child level.
+ """
+ system = self.get_system()
+ course_due = 'March 20 17:00'
+ child_due = 'April 10 00:00'
+ url_name = 'test1'
+ start_xml = '''
+
+
+ Two houses, ...
+
+ '''.format(due=course_due, org=ORG, course=COURSE, url_name=url_name)
+ descriptor = system.process_xml(start_xml)
+ child = descriptor.get_children()[0]
+ child._model_data['due'] = child_due
+ compute_inherited_metadata(descriptor)
+
+ self.assertEqual(descriptor.lms.due, Date().from_json(course_due))
+ self.assertEqual(child.lms.due, Date().from_json(child_due))
+ # Test inherited metadata. Due does not appear here (because explicitly set on child).
+ self.assertEqual(1, len(child._inherited_metadata))
+ self.assertEqual('1970-01-01T00:00:00Z', child._inherited_metadata['start'])
+ # Test inheritable metadata. This has the course inheritable value for due.
+ self.assertEqual(2, len(child._inheritable_metadata))
+ self.assertEqual(course_due, child._inheritable_metadata['due'])
+
def test_is_pointer_tag(self):
"""
Check that is_pointer_tag works properly.
diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py
index 06d5b0b0a3..e41bcdd73a 100644
--- a/common/lib/xmodule/xmodule/tests/test_xml_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py
@@ -9,13 +9,13 @@ from mock import Mock
class TestFields(object):
# Will be returned by editable_metadata_fields.
- max_attempts = StringyInteger(scope=Scope.settings)
+ max_attempts = StringyInteger(scope=Scope.settings, default=1000)
# Will not be returned by editable_metadata_fields because filtered out by non_editable_metadata_fields.
due = Date(scope=Scope.settings)
# Will not be returned by editable_metadata_fields because is not Scope.settings.
student_answers = Object(scope=Scope.user_state)
# Will be returned, and can override the inherited value from XModule.
- display_name = String(scope=Scope.settings)
+ display_name = String(scope=Scope.settings, default='local default')
class EditableMetadataFieldsTest(unittest.TestCase):
@@ -25,27 +25,45 @@ class EditableMetadataFieldsTest(unittest.TestCase):
# Tests that the xblock fields (currently tags and name) get filtered out.
# Also tests that xml_attributes is filtered out of XmlDescriptor.
self.assertEqual(1, len(editable_fields), "Expected only 1 editable field for xml descriptor.")
- self.assert_display_name_default(editable_fields)
+ self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name,
+ explicitly_set=False, inheritable=False, value=None, default_value=None)
def test_override_default(self):
- # Tests that is_default is correct when a value overrides the default.
+ # Tests that explicitly_set is correct when a value overrides the default (not inheritable).
editable_fields = self.get_xml_editable_fields({'display_name': 'foo'})
- display_name = editable_fields['display_name']
- self.assertFalse(display_name['is_default'])
- self.assertEqual('foo', display_name['value'])
+ self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name,
+ explicitly_set=True, inheritable=False, value='foo', default_value=None)
def test_additional_field(self):
- editable_fields = self.get_module_editable_fields({'max_attempts' : '7'})
+ descriptor = self.get_descriptor({'max_attempts' : '7'})
+ editable_fields = descriptor.editable_metadata_fields
self.assertEqual(2, len(editable_fields))
- self.assert_field_values(editable_fields, 'max_attempts', TestFields.max_attempts, False, False, 7)
- self.assert_display_name_default(editable_fields)
+ self.assert_field_values(editable_fields, 'max_attempts', TestFields.max_attempts,
+ explicitly_set=True, inheritable=False, value=7, default_value=1000)
+ self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name,
+ explicitly_set=False, inheritable=False, value='local default', default_value='local default')
- editable_fields = self.get_module_editable_fields({})
- self.assert_field_values(editable_fields, 'max_attempts', TestFields.max_attempts, True, False, None)
+ editable_fields = self.get_descriptor({}).editable_metadata_fields
+ self.assert_field_values(editable_fields, 'max_attempts', TestFields.max_attempts,
+ explicitly_set=False, inheritable=False, value=1000, default_value=1000)
def test_inherited_field(self):
- editable_fields = self.get_module_editable_fields({'display_name' : 'inherited'})
- self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name, False, True, 'inherited')
+ model_val = {'display_name' : 'inherited'}
+ descriptor = self.get_descriptor(model_val)
+ # Mimic an inherited value for display_name (inherited and inheritable are the same in this case).
+ descriptor._inherited_metadata = model_val
+ descriptor._inheritable_metadata = model_val
+ editable_fields = descriptor.editable_metadata_fields
+ self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name,
+ explicitly_set=False, inheritable=True, value='inherited', default_value='inherited')
+
+ descriptor = self.get_descriptor({'display_name' : 'explicit'})
+ # Mimic the case where display_name WOULD have been inherited, except we explicitly set it.
+ descriptor._inheritable_metadata = {'display_name' : 'inheritable value'}
+ descriptor._inherited_metadata = {}
+ editable_fields = descriptor.editable_metadata_fields
+ self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name,
+ explicitly_set=True, inheritable=True, value='explicit', default_value='inheritable value')
# Start of helper methods
def get_xml_editable_fields(self, model_data):
@@ -53,7 +71,7 @@ class EditableMetadataFieldsTest(unittest.TestCase):
system.render_template = Mock(return_value="
Test Template HTML
")
return XmlDescriptor(system=system, location=None, model_data=model_data).editable_metadata_fields
- def get_module_editable_fields(self, model_data):
+ def get_descriptor(self, model_data):
class TestModuleDescriptor(TestFields, XmlDescriptor):
@property
@@ -64,16 +82,12 @@ class EditableMetadataFieldsTest(unittest.TestCase):
system = test_system()
system.render_template = Mock(return_value="
Test Template HTML
")
- descriptor = TestModuleDescriptor(system=system, location=None, model_data=model_data)
- descriptor._inherited_metadata = {'display_name' : 'inherited'}
- return descriptor.editable_metadata_fields
+ return TestModuleDescriptor(system=system, location=None, model_data=model_data)
- def assert_display_name_default(self, editable_fields):
- self.assert_field_values(editable_fields, 'display_name', XModuleFields.display_name, True, False, None)
-
- def assert_field_values(self, editable_fields, name, field, is_default, is_inherited, value):
+ def assert_field_values(self, editable_fields, name, field, explicitly_set, inheritable, value, default_value):
test_field = editable_fields[name]
self.assertEqual(field, test_field['field'])
- self.assertEqual(is_default, test_field['is_default'])
- self.assertEqual(is_inherited, test_field['is_inherited'])
+ self.assertEqual(explicitly_set, test_field['explicitly_set'])
+ self.assertEqual(inheritable, test_field['inheritable'])
self.assertEqual(value, test_field['value'])
+ self.assertEqual(default_value, test_field['default_value'])
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 749ca66258..7c24d593e3 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -624,27 +624,28 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
Can be limited by extending `non_editable_metadata_fields`.
"""
inherited_metadata = getattr(self, '_inherited_metadata', {})
+ inheritable_metadata = getattr(self, '_inheritable_metadata', {})
metadata = {}
for field in self.fields:
if field.scope != Scope.settings or field in self.non_editable_metadata_fields:
continue
- inherited = False
- default = False
+ inheritable = False
value = getattr(self, field.name)
- if field.name in self._model_data:
- default = False
+ default_value = field.default
+ explicitly_set = field.name in self._model_data
+ if field.name in inheritable_metadata:
+ inheritable = True
+ default_value = field.from_json(inheritable_metadata.get(field.name))
if field.name in inherited_metadata:
- if self._model_data.get(field.name) == inherited_metadata.get(field.name):
- inherited = True
- else:
- default = True
+ explicitly_set = False
metadata[field.name] = {'field': field,
'value': value,
- 'is_inherited': inherited,
- 'is_default': default}
+ 'default_value': default_value,
+ 'inheritable': inheritable,
+ 'explicitly_set': explicitly_set }
return metadata
diff --git a/github-requirements.txt b/github-requirements.txt
index 048f3cee68..35ad8af027 100644
--- a/github-requirements.txt
+++ b/github-requirements.txt
@@ -8,4 +8,4 @@
-e git://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk
# Our libraries:
--e git+https://github.com/edx/XBlock.git@5ce6f70a#egg=XBlock
+-e git+https://github.com/edx/XBlock.git@483e0cb1#egg=XBlock
diff --git a/lms/lib/comment_client/utils.py b/lms/lib/comment_client/utils.py
index 860035dc06..53bdd462ad 100644
--- a/lms/lib/comment_client/utils.py
+++ b/lms/lib/comment_client/utils.py
@@ -37,6 +37,10 @@ def perform_request(method, url, data_or_params=None, *args, **kwargs):
else:
response = requests.request(method, url, params=data_or_params, timeout=5)
except Exception as err:
+ # remove API key if it is in the params
+ if 'api_key' in data_or_params:
+ log.info('Deleting API key from params')
+ del data_or_params['api_key']
log.exception("Trying to call {method} on {url} with params {params}".format(
method=method, url=url, params=data_or_params))
# Reraise with a single exception type