Commit 729ca900 by Christina Roberts

Merge pull request #1920 from edx/feature/christina/fields

Pass additional field information from editable_metadata_fields, remove system_metadata_fields variable.
parents 29a5d3b4 5c3719b9
......@@ -75,11 +75,7 @@ def set_module_info(store, location, post_data):
# IMPORTANT NOTE: if the client passed pack 'null' (None) for a piece of metadata that means 'remove it'
for metadata_key, value in posted_metadata.items():
# let's strip out any metadata fields from the postback which have been identified as system metadata
# and therefore should not be user-editable, so we should accept them back from the client
if metadata_key in module.system_metadata_fields:
del posted_metadata[metadata_key]
elif posted_metadata[metadata_key] is None:
if posted_metadata[metadata_key] is None:
# remove both from passed in collection as well as the collection read in from the modulestore
if metadata_key in module._model_data:
del module._model_data[metadata_key]
......
......@@ -676,11 +676,7 @@ def save_item(request):
# IMPORTANT NOTE: if the client passed pack 'null' (None) for a piece of metadata that means 'remove it'
for metadata_key, value in posted_metadata.items():
# let's strip out any metadata fields from the postback which have been identified as system metadata
# and therefore should not be user-editable, so we should accept them back from the client
if metadata_key in existing_item.system_metadata_fields:
del posted_metadata[metadata_key]
elif posted_metadata[metadata_key] is None:
if posted_metadata[metadata_key] is None:
# remove both from passed in collection as well as the collection read in from the modulestore
if metadata_key in existing_item._model_data:
del existing_item._model_data[metadata_key]
......
......@@ -14,13 +14,14 @@ class CourseMetadata(object):
The objects have no predefined attrs but instead are obj encodings of the
editable metadata.
'''
FILTERED_LIST = XModuleDescriptor.system_metadata_fields + ['start',
'end',
'enrollment_start',
'enrollment_end',
'tabs',
'graceperiod',
'checklists']
FILTERED_LIST = ['xml_attributes',
'start',
'end',
'enrollment_start',
'enrollment_end',
'tabs',
'graceperiod',
'checklists']
@classmethod
def fetch(cls, course_location):
......
......@@ -72,3 +72,14 @@ describe "CMS.Views.ModuleEdit", ->
it "loads the .xmodule-display inside the module editor", ->
expect(XModule.loadModule).toHaveBeenCalled()
expect(XModule.loadModule.mostRecentCall.args[0]).toBe($('.xmodule_display'))
describe "changedMetadata", ->
it "returns empty if no metadata loaded", ->
expect(@moduleEdit.changedMetadata()).toEqual({})
it "returns only changed values", ->
@moduleEdit.originalMetadata = {'foo', 'bar'}
spyOn(@moduleEdit, 'metadata').andReturn({'a': '', 'b': 'before', 'c': ''})
@moduleEdit.loadEdit()
@moduleEdit.metadata.andReturn({'a': '', 'b': 'after', 'd': 'only_after'})
expect(@moduleEdit.changedMetadata()).toEqual({'b' : 'after', 'd' : 'only_after'})
......@@ -20,6 +20,7 @@ class CMS.Views.ModuleEdit extends Backbone.View
loadEdit: ->
if not @module
@module = XModule.loadModule(@$el.find('.xmodule_edit'))
@originalMetadata = @metadata()
metadata: ->
# cdodge: package up metadata which is separated into a number of input fields
......@@ -35,6 +36,14 @@ class CMS.Views.ModuleEdit extends Backbone.View
return _metadata
changedMetadata: ->
currentMetadata = @metadata()
changedMetadata = {}
for key of currentMetadata
if currentMetadata[key] != @originalMetadata[key]
changedMetadata[key] = currentMetadata[key]
return changedMetadata
cloneTemplate: (parent, template) ->
$.post("/clone_item", {
parent_location: parent
......@@ -60,7 +69,7 @@ class CMS.Views.ModuleEdit extends Backbone.View
course: course_location_analytics
id: _this.model.id
data.metadata = _.extend(data.metadata || {}, @metadata())
data.metadata = _.extend(data.metadata || {}, @changedMetadata())
@hideModal()
@model.save(data).done( =>
# # showToastMessage("Your changes have been saved.", null, 3)
......
<%
import hashlib
from xmodule.fields import StringyInteger, StringyFloat
hlskey = hashlib.md5(module.location.url()).hexdigest()
%>
<section class="metadata_edit">
......@@ -7,17 +8,40 @@
% for field_name, field_value in editable_metadata_fields.items():
<li>
% if field_name == 'source_code':
<a href="#hls-modal-${hlskey}" style="color:yellow;" id="hls-trig-${hlskey}" >Edit High Level Source</a>
% if field_value['is_default'] is False:
<a href="#hls-modal-${hlskey}" style="color:yellow;" id="hls-trig-${hlskey}" >Edit High Level Source</a>
% endif
% else:
<label>${field_name}:</label>
<input type='text' data-metadata-name='${field_name}' value='${field_value}' size='60' />
<label>${field_value['field'].display_name}:</label>
<input type='text' data-metadata-name='${field_value["field"].display_name}'
## This is a hack to keep current behavior for weight and attempts (empty will parse OK as unset).
## This hack will go away with our custom editors.
% if field_value["value"] == None and (isinstance(field_value["field"], StringyFloat) or isinstance(field_value["field"], StringyInteger)):
value = ''
% else:
value='${field_value["field"].to_json(field_value["value"])}'
% endif
size='60' />
## Change to True to see all the information being passed through.
% if False:
<label>Help: ${field_value['field'].help}</label>
<label>Type: ${type(field_value['field']).__name__}</label>
<label>Inherited: ${field_value['is_inherited']}</label>
<label>Default: ${field_value['is_default']}</label>
% if field_value['field'].values:
<label>Possible values:</label>
% for value in field_value['field'].values:
<label>${value}</label>
% endfor
% endif
% endif
% endif
</li>
% endfor
</ul>
% if 'source_code' in editable_metadata_fields:
<%include file="source-edit.html" />
% if 'source_code' in editable_metadata_fields and not editable_metadata_fields['source_code']['is_default']:
<%include file="source-edit.html" />
% endif
</section>
......@@ -12,7 +12,7 @@
<form id="hls-form" enctype="multipart/form-data">
<section class="source-edit">
<textarea name="" data-metadata-name="source_code" class="source-edit-box hls-data" rows="8" cols="40">${editable_metadata_fields['source_code']|h}</textarea>
<textarea name="" data-metadata-name="source_code" class="source-edit-box hls-data" rows="8" cols="40">${editable_metadata_fields['source_code']['value']|h}</textarea>
</section>
<div class="submit">
<button type="reset" class="hls-compile">Save &amp; Compile to edX XML</button>
......
......@@ -65,7 +65,8 @@ class CapaFields(object):
max_attempts = StringyInteger(help="Maximum number of attempts that a student is allowed", scope=Scope.settings)
due = Date(help="Date that this problem is due by", scope=Scope.settings)
graceperiod = Timedelta(help="Amount of time after the due date that submissions will be accepted", scope=Scope.settings)
showanswer = String(help="When to show the problem answer to the student", scope=Scope.settings, default="closed")
showanswer = String(help="When to show the problem answer to the student", scope=Scope.settings, default="closed",
values=["answered", "always", "attempted", "closed", "never"])
force_save_button = Boolean(help="Whether to force the save button to appear on the page", scope=Scope.settings, default=False)
rerandomize = Randomization(help="When to rerandomize the problem", default="always", scope=Scope.settings)
data = String(help="XML data for the problem", scope=Scope.content)
......@@ -882,16 +883,6 @@ class CapaDescriptor(CapaFields, RawDescriptor):
'enable_markdown': self.markdown is not None})
return _context
@property
def editable_metadata_fields(self):
"""Remove metadata from the editable fields since it has its own editor"""
subset = super(CapaDescriptor, self).editable_metadata_fields
if 'markdown' in subset:
del subset['markdown']
if 'empty' in subset:
del subset['empty']
return subset
# VS[compat]
# TODO (cpennington): Delete this method once all fall 2012 course are being
# edited in the cms
......@@ -901,3 +892,10 @@ class CapaDescriptor(CapaFields, RawDescriptor):
'problems/' + path[8:],
path[8:],
]
@property
def non_editable_metadata_fields(self):
non_editable_fields = super(CapaDescriptor, self).non_editable_metadata_fields
non_editable_fields.extend([CapaDescriptor.due, CapaDescriptor.graceperiod,
CapaDescriptor.force_save_button, CapaDescriptor.markdown])
return non_editable_fields
......@@ -37,3 +37,10 @@ class DiscussionDescriptor(DiscussionFields, MetadataOnlyEditingDescriptor, RawD
metadata_translations = dict(RawDescriptor.metadata_translations)
metadata_translations['id'] = 'discussion_id'
metadata_translations['for'] = 'discussion_target'
@property
def non_editable_metadata_fields(self):
non_editable_fields = super(DiscussionDescriptor, self).non_editable_metadata_fields
# We may choose to enable sort_keys in the future, but while Kevin is investigating....
non_editable_fields.extend([DiscussionDescriptor.discussion_id, DiscussionDescriptor.sort_key])
return non_editable_fields
......@@ -19,6 +19,7 @@ log = logging.getLogger("mitx.courseware")
class HtmlFields(object):
data = String(help="Html contents to display for this module", scope=Scope.content)
source_code = String(help="Source code for LaTeX documents. This feature is not well-supported.", scope=Scope.settings)
class HtmlModule(HtmlFields, XModule):
......@@ -166,16 +167,6 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor):
elt.set("filename", relname)
return elt
@property
def editable_metadata_fields(self):
"""Remove any metadata from the editable fields which have their own editor or shouldn't be edited by user."""
subset = super(HtmlDescriptor, self).editable_metadata_fields
if 'empty' in subset:
del subset['empty']
return subset
class AboutDescriptor(HtmlDescriptor):
"""
......
from .x_module import XModuleDescriptor, DescriptorSystem
from .modulestore.inheritance import own_metadata
class MakoDescriptorSystem(DescriptorSystem):
......@@ -34,20 +33,10 @@ class MakoModuleDescriptor(XModuleDescriptor):
"""
return {
'module': self,
'editable_metadata_fields': self.editable_metadata_fields,
'editable_metadata_fields': self.editable_metadata_fields
}
def get_html(self):
return self.system.render_template(
self.mako_template, self.get_context())
# cdodge: encapsulate a means to expose "editable" metadata fields (i.e. not internal system metadata)
@property
def editable_metadata_fields(self):
fields = {}
for field, value in own_metadata(self).items():
if field in self.system_metadata_fields:
continue
fields[field] = value
return fields
from xmodule.x_module import XModuleFields
from xblock.core import Scope, String, Object
from xmodule.fields import Date, StringyInteger
from xmodule.xml_module import XmlDescriptor
import unittest
from . import test_system
from mock import Mock
class TestFields(object):
# Will be returned by editable_metadata_fields.
max_attempts = StringyInteger(scope=Scope.settings)
# 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)
class EditableMetadataFieldsTest(unittest.TestCase):
def test_display_name_field(self):
editable_fields = self.get_xml_editable_fields({})
# 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)
def test_override_default(self):
# Tests that is_default is correct when a value overrides the default.
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'])
def test_additional_field(self):
editable_fields = self.get_module_editable_fields({'max_attempts' : '7'})
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)
editable_fields = self.get_module_editable_fields({})
self.assert_field_values(editable_fields, 'max_attempts', TestFields.max_attempts, True, False, None)
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')
# Start of helper methods
def get_xml_editable_fields(self, model_data):
system = test_system()
system.render_template = Mock(return_value="<div>Test Template HTML</div>")
return XmlDescriptor(system=system, location=None, model_data=model_data).editable_metadata_fields
def get_module_editable_fields(self, model_data):
class TestModuleDescriptor(TestFields, XmlDescriptor):
@property
def non_editable_metadata_fields(self):
non_editable_fields = super(TestModuleDescriptor, self).non_editable_metadata_fields
non_editable_fields.append(TestModuleDescriptor.due)
return non_editable_fields
system = test_system()
system.render_template = Mock(return_value="<div>Test Template HTML</div>")
descriptor = TestModuleDescriptor(system=system, location=None, model_data=model_data)
descriptor._inherited_metadata = {'display_name' : 'inherited'}
return descriptor.editable_metadata_fields
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):
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(value, test_field['value'])
......@@ -82,7 +82,7 @@ class XModuleFields(object):
display_name = String(
help="Display name for this module",
scope=Scope.settings,
default=None,
default=None
)
......@@ -334,12 +334,6 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
# (like a practice problem).
has_score = False
# cdodge: this is a list of metadata names which are 'system' metadata
# and should not be edited by an end-user
system_metadata_fields = ['data_dir', 'published_date', 'published_by', 'is_draft',
'discussion_id', 'xml_attributes']
# A list of descriptor attributes that must be equal for the descriptors to
# be equal
equality_attributes = ('_model_data', 'location')
......@@ -612,6 +606,48 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
model_data=self._model_data,
))
@property
def non_editable_metadata_fields(self):
"""
Return the list of fields that should not be editable in Studio.
When overriding, be sure to append to the superclasses' list.
"""
# We are not allowing editing of xblock tag and name fields at this time (for any component).
return [XBlock.tags, XBlock.name]
@property
def editable_metadata_fields(self):
"""
Returns the metadata fields to be edited in Studio. These are fields with scope `Scope.settings`.
Can be limited by extending `non_editable_metadata_fields`.
"""
inherited_metadata = getattr(self, '_inherited_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
value = getattr(self, field.name)
if field.name in self._model_data:
default = False
if field.name in inherited_metadata:
if self._model_data.get(field.name) == inherited_metadata.get(field.name):
inherited = True
else:
default = True
metadata[field.name] = {'field': field,
'value': value,
'is_inherited': inherited,
'is_default': default}
return metadata
class DescriptorSystem(object):
def __init__(self, load_item, resources_fs, error_tracker, **kwargs):
......
......@@ -84,7 +84,8 @@ class XmlDescriptor(XModuleDescriptor):
Mixin class for standardized parsing of from xml
"""
xml_attributes = Object(help="Map of unhandled xml attributes, used only for storage between import and export", default={}, scope=Scope.settings)
xml_attributes = Object(help="Map of unhandled xml attributes, used only for storage between import and export",
default={}, scope=Scope.settings)
# Extension to append to filename paths
filename_extension = 'xml'
......@@ -418,3 +419,9 @@ class XmlDescriptor(XModuleDescriptor):
"""
raise NotImplementedError(
"%s does not implement definition_to_xml" % self.__class__.__name__)
@property
def non_editable_metadata_fields(self):
non_editable_fields = super(XmlDescriptor, self).non_editable_metadata_fields
non_editable_fields.append(XmlDescriptor.xml_attributes)
return non_editable_fields
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