diff --git a/cms/djangoapps/contentstore/module_info_model.py b/cms/djangoapps/contentstore/module_info_model.py index 91f722a699..f7d1bbd8fe 100644 --- a/cms/djangoapps/contentstore/module_info_model.py +++ b/cms/djangoapps/contentstore/module_info_model.py @@ -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] diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index caf3901e03..824d2119f1 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -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] diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 4429e35692..708e79f0a3 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -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): diff --git a/cms/static/coffee/spec/views/module_edit_spec.coffee b/cms/static/coffee/spec/views/module_edit_spec.coffee index 5e83ecb42d..baf9ee9c20 100644 --- a/cms/static/coffee/spec/views/module_edit_spec.coffee +++ b/cms/static/coffee/spec/views/module_edit_spec.coffee @@ -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'}) diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index 3cb3b1703f..bf56807f66 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -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) diff --git a/cms/templates/widgets/metadata-edit.html b/cms/templates/widgets/metadata-edit.html index 51fe400f88..9693c18e9c 100644 --- a/cms/templates/widgets/metadata-edit.html +++ b/cms/templates/widgets/metadata-edit.html @@ -1,5 +1,6 @@ <% import hashlib + from xmodule.fields import StringyInteger, StringyFloat hlskey = hashlib.md5(module.location.url()).hexdigest() %>
@@ -7,17 +8,40 @@ % for field_name, field_value in editable_metadata_fields.items():
  • % if field_name == 'source_code': - Edit High Level Source + % if field_value['is_default'] is False: + Edit High Level Source + % endif % else: - - + + + ## Change to True to see all the information being passed through. + % if False: + + + + + % if field_value['field'].values: + + % for value in field_value['field'].values: + + % endfor + % endif + % endif % endif
  • % endfor - % 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
    diff --git a/cms/templates/widgets/source-edit.html b/cms/templates/widgets/source-edit.html index 883190d6b3..b7ee6c9db9 100644 --- a/cms/templates/widgets/source-edit.html +++ b/cms/templates/widgets/source-edit.html @@ -12,7 +12,7 @@
    - +
    diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 4143345196..479cd5a759 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -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 diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py index 8968e221b2..98082ddea2 100644 --- a/common/lib/xmodule/xmodule/discussion_module.py +++ b/common/lib/xmodule/xmodule/discussion_module.py @@ -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 diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index d901fc5fbe..bbf24a6320 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -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): """ diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py index 84db6ad779..8abb1d7777 100644 --- a/common/lib/xmodule/xmodule/mako_module.py +++ b/common/lib/xmodule/xmodule/mako_module.py @@ -1,5 +1,4 @@ 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 diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py new file mode 100644 index 0000000000..06d5b0b0a3 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -0,0 +1,79 @@ +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="
    Test Template HTML
    ") + 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="
    Test Template HTML
    ") + 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']) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 1fd0b8e138..749ca66258 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -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): diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index f9de929c05..7480cda0c5 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -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