From 2d35e48b68ff51fae09369b4a1a00d7599c454c1 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 20 Sep 2012 11:07:11 -0400 Subject: [PATCH 1/9] Fix JSON postback error where the content-type header line can contain more info than just the application/json descriptor. Now we just to a compare on the start of the header value. --- common/djangoapps/util/json_request.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/util/json_request.py b/common/djangoapps/util/json_request.py index 391905e574..169a7e3fb4 100644 --- a/common/djangoapps/util/json_request.py +++ b/common/djangoapps/util/json_request.py @@ -6,7 +6,9 @@ import json def expect_json(view_function): @wraps(view_function) def expect_json_with_cloned_request(request, *args, **kwargs): - if request.META['CONTENT_TYPE'] == "application/json": + # cdodge: fix postback errors in CMS. The POST 'content-type' header can include additional information + # e.g. 'charset', so we can't do a direct string compare + if request.META['CONTENT_TYPE'].lower().startswith("application/json"): cloned_request = copy.copy(request) cloned_request.POST = cloned_request.POST.copy() cloned_request.POST.update(json.loads(request.body)) From 1cd81a2a4fd6db9c46c0cc697e14470c5bd6b998 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 21 Sep 2012 09:14:28 -0400 Subject: [PATCH 2/9] work-in-progress: As a problem author, I would like to be able to edit module metadata --- cms/djangoapps/contentstore/views.py | 6 ++++++ cms/static/coffee/src/models/module.coffee | 12 ++++++++++++ cms/templates/widgets/html-edit.html | 1 + cms/templates/widgets/metadata-edit.html | 8 ++++++++ cms/templates/widgets/raw-edit.html | 1 + common/lib/xmodule/xmodule/editing_module.py | 1 + 6 files changed, 29 insertions(+) create mode 100644 cms/templates/widgets/metadata-edit.html diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 505b897497..bffadcf666 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -110,6 +110,7 @@ def edit_item(request): 'category': item.category, 'url_name': item.url_name, 'previews': get_module_previews(request, item), + 'metadata': item.metadata }) @@ -279,6 +280,11 @@ def save_item(request): data = json.loads(request.POST['data']) modulestore().update_item(item_location, data) + # cdodge: also commit any metadata which might have been passed along in the + # POST from the client + if request.POST['metadata']: + modulestore().update_metadata(item_location, request.POST['metadata']) + # Export the course back to github # This uses wildcarding to find the course, which requires handling # multiple courses returned, but there should only ever be one diff --git a/cms/static/coffee/src/models/module.coffee b/cms/static/coffee/src/models/module.coffee index 159172e852..b1bbb3bb72 100644 --- a/cms/static/coffee/src/models/module.coffee +++ b/cms/static/coffee/src/models/module.coffee @@ -2,14 +2,26 @@ class CMS.Models.Module extends Backbone.Model url: '/save_item' defaults: data: '' + metadata: {} loadModule: (element) -> elt = $(element).find('.xmodule_edit').first() @module = XModule.loadModule(elt) + # find the metadata edit region which should be setup server side, + # so that we can wire up posting back those changes + @metadata_elt = $(element).find('.metadata_edit') editUrl: -> "/edit_item?#{$.param(id: @get('id'))}" save: (args...) -> @set(data: JSON.stringify(@module.save())) if @module + # cdodge: package up metadata which is separated into a number of input fields + # there's probably a better way to do this, but at least this lets me continue to move onwards + if @metadata_elt + _metadata = {} + # walk through the set of elments which have the 'xmetadata_name' attribute and + # build up a object to pass back to the server on the subsequent POST + _metadata[el.getAttribute("xmetadata_name")]=el.value for el in $('[xmetadata_name]') + @set(metadata: _metadata) super(args...) diff --git a/cms/templates/widgets/html-edit.html b/cms/templates/widgets/html-edit.html index 1e86c6c734..9f7196b6e4 100644 --- a/cms/templates/widgets/html-edit.html +++ b/cms/templates/widgets/html-edit.html @@ -1,3 +1,4 @@ +<%include file="metadata-edit.html" />
diff --git a/cms/templates/widgets/metadata-edit.html b/cms/templates/widgets/metadata-edit.html new file mode 100644 index 0000000000..a130436e37 --- /dev/null +++ b/cms/templates/widgets/metadata-edit.html @@ -0,0 +1,8 @@ + diff --git a/cms/templates/widgets/raw-edit.html b/cms/templates/widgets/raw-edit.html index fbd1757be5..9488552be5 100644 --- a/cms/templates/widgets/raw-edit.html +++ b/cms/templates/widgets/raw-edit.html @@ -1,3 +1,4 @@ +<%include file="metadata-edit.html" />
diff --git a/common/lib/xmodule/xmodule/editing_module.py b/common/lib/xmodule/xmodule/editing_module.py index 67a4d66dad..c562617c98 100644 --- a/common/lib/xmodule/xmodule/editing_module.py +++ b/common/lib/xmodule/xmodule/editing_module.py @@ -21,6 +21,7 @@ class EditingDescriptor(MakoModuleDescriptor): return { 'module': self, 'data': self.definition.get('data', ''), + 'metadata': self.metadata # TODO (vshnayder): allow children and metadata to be edited. #'children' : self.definition.get('children, ''), From a7b7fd52d97786cbc08680975632b11d60d1fbe2 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 21 Sep 2012 13:56:53 -0400 Subject: [PATCH 3/9] Support editing of metadata on Sequences --- cms/templates/widgets/metadata-edit.html | 2 ++ cms/templates/widgets/sequence-edit.html | 1 + common/lib/xmodule/xmodule/seq_module.py | 6 ++++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/cms/templates/widgets/metadata-edit.html b/cms/templates/widgets/metadata-edit.html index a130436e37..368959022a 100644 --- a/cms/templates/widgets/metadata-edit.html +++ b/cms/templates/widgets/metadata-edit.html @@ -1,3 +1,4 @@ +% if metadata: +% endif diff --git a/cms/templates/widgets/sequence-edit.html b/cms/templates/widgets/sequence-edit.html index d92ccbb7a7..dba6627bb2 100644 --- a/cms/templates/widgets/sequence-edit.html +++ b/cms/templates/widgets/sequence-edit.html @@ -29,6 +29,7 @@ + <%include file="metadata-edit.html" />
    diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index b05ea36e50..cc2bfe2bc1 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -9,6 +9,7 @@ from xmodule.x_module import XModule from xmodule.progress import Progress from xmodule.exceptions import NotFoundError from pkg_resources import resource_string +from .editing_module import EditingDescriptor log = logging.getLogger("mitx.common.lib.seq_module") @@ -94,7 +95,8 @@ class SequenceModule(XModule): 'element_id': self.location.html_id(), 'item_id': self.id, 'position': self.position, - 'tag': self.location.category} + 'tag': self.location.category + } self.content = self.system.render_template('seq_module.html', params) self.rendered = True @@ -109,7 +111,7 @@ class SequenceModule(XModule): return new_class -class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): +class SequenceDescriptor(EditingDescriptor, XmlDescriptor): mako_template = 'widgets/sequence-edit.html' module_class = SequenceModule From 0a7dfca0e4783abc24a6ec9d0bd9b84219593a1f Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 20 Sep 2012 11:07:11 -0400 Subject: [PATCH 4/9] Fix JSON postback error where the content-type header line can contain more info than just the application/json descriptor. Now we just to a compare on the start of the header value. --- common/djangoapps/util/json_request.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/util/json_request.py b/common/djangoapps/util/json_request.py index 391905e574..169a7e3fb4 100644 --- a/common/djangoapps/util/json_request.py +++ b/common/djangoapps/util/json_request.py @@ -6,7 +6,9 @@ import json def expect_json(view_function): @wraps(view_function) def expect_json_with_cloned_request(request, *args, **kwargs): - if request.META['CONTENT_TYPE'] == "application/json": + # cdodge: fix postback errors in CMS. The POST 'content-type' header can include additional information + # e.g. 'charset', so we can't do a direct string compare + if request.META['CONTENT_TYPE'].lower().startswith("application/json"): cloned_request = copy.copy(request) cloned_request.POST = cloned_request.POST.copy() cloned_request.POST.update(json.loads(request.body)) From d51f127afa6db598b8065fde650322cbec4142e3 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 21 Sep 2012 09:14:28 -0400 Subject: [PATCH 5/9] work-in-progress: As a problem author, I would like to be able to edit module metadata --- cms/djangoapps/contentstore/views.py | 6 ++++++ cms/static/coffee/src/models/module.coffee | 12 ++++++++++++ cms/templates/widgets/html-edit.html | 1 + cms/templates/widgets/metadata-edit.html | 8 ++++++++ cms/templates/widgets/raw-edit.html | 1 + common/lib/xmodule/xmodule/editing_module.py | 1 + 6 files changed, 29 insertions(+) create mode 100644 cms/templates/widgets/metadata-edit.html diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 6f428ee3e8..14a11a2db1 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -110,6 +110,7 @@ def edit_item(request): 'category': item.category, 'url_name': item.url_name, 'previews': get_module_previews(request, item), + 'metadata': item.metadata }) @@ -284,6 +285,11 @@ def save_item(request): children = request.POST['children'] modulestore().update_children(item_location, children) + # cdodge: also commit any metadata which might have been passed along in the + # POST from the client + if request.POST['metadata']: + modulestore().update_metadata(item_location, request.POST['metadata']) + # Export the course back to github # This uses wildcarding to find the course, which requires handling # multiple courses returned, but there should only ever be one diff --git a/cms/static/coffee/src/models/module.coffee b/cms/static/coffee/src/models/module.coffee index e2f911e9bd..4e1a926445 100644 --- a/cms/static/coffee/src/models/module.coffee +++ b/cms/static/coffee/src/models/module.coffee @@ -3,14 +3,26 @@ class CMS.Models.Module extends Backbone.Model defaults: data: '' children: '' + metadata: {} loadModule: (element) -> elt = $(element).find('.xmodule_edit').first() @module = XModule.loadModule(elt) + # find the metadata edit region which should be setup server side, + # so that we can wire up posting back those changes + @metadata_elt = $(element).find('.metadata_edit') editUrl: -> "/edit_item?#{$.param(id: @get('id'))}" save: (args...) -> @set(data: @module.save()) if @module + # cdodge: package up metadata which is separated into a number of input fields + # there's probably a better way to do this, but at least this lets me continue to move onwards + if @metadata_elt + _metadata = {} + # walk through the set of elments which have the 'xmetadata_name' attribute and + # build up a object to pass back to the server on the subsequent POST + _metadata[el.getAttribute("xmetadata_name")]=el.value for el in $('[xmetadata_name]') + @set(metadata: _metadata) super(args...) diff --git a/cms/templates/widgets/html-edit.html b/cms/templates/widgets/html-edit.html index 1e86c6c734..9f7196b6e4 100644 --- a/cms/templates/widgets/html-edit.html +++ b/cms/templates/widgets/html-edit.html @@ -1,3 +1,4 @@ +<%include file="metadata-edit.html" />
    diff --git a/cms/templates/widgets/metadata-edit.html b/cms/templates/widgets/metadata-edit.html new file mode 100644 index 0000000000..a130436e37 --- /dev/null +++ b/cms/templates/widgets/metadata-edit.html @@ -0,0 +1,8 @@ + diff --git a/cms/templates/widgets/raw-edit.html b/cms/templates/widgets/raw-edit.html index fbd1757be5..9488552be5 100644 --- a/cms/templates/widgets/raw-edit.html +++ b/cms/templates/widgets/raw-edit.html @@ -1,3 +1,4 @@ +<%include file="metadata-edit.html" />
    diff --git a/common/lib/xmodule/xmodule/editing_module.py b/common/lib/xmodule/xmodule/editing_module.py index 67a4d66dad..c562617c98 100644 --- a/common/lib/xmodule/xmodule/editing_module.py +++ b/common/lib/xmodule/xmodule/editing_module.py @@ -21,6 +21,7 @@ class EditingDescriptor(MakoModuleDescriptor): return { 'module': self, 'data': self.definition.get('data', ''), + 'metadata': self.metadata # TODO (vshnayder): allow children and metadata to be edited. #'children' : self.definition.get('children, ''), From 4b6eabec5f739d339bc533907aedb3e2c48b1f4e Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 21 Sep 2012 13:56:53 -0400 Subject: [PATCH 6/9] Support editing of metadata on Sequences --- cms/templates/widgets/metadata-edit.html | 2 ++ cms/templates/widgets/sequence-edit.html | 1 + common/lib/xmodule/xmodule/seq_module.py | 6 ++++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/cms/templates/widgets/metadata-edit.html b/cms/templates/widgets/metadata-edit.html index a130436e37..368959022a 100644 --- a/cms/templates/widgets/metadata-edit.html +++ b/cms/templates/widgets/metadata-edit.html @@ -1,3 +1,4 @@ +% if metadata: +% endif diff --git a/cms/templates/widgets/sequence-edit.html b/cms/templates/widgets/sequence-edit.html index 242bfeae21..e9d796784d 100644 --- a/cms/templates/widgets/sequence-edit.html +++ b/cms/templates/widgets/sequence-edit.html @@ -29,6 +29,7 @@
+ <%include file="metadata-edit.html" />
    diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 841936cf17..0e0fd10fa4 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -9,6 +9,7 @@ from xmodule.x_module import XModule from xmodule.progress import Progress from xmodule.exceptions import NotFoundError from pkg_resources import resource_string +from .editing_module import EditingDescriptor log = logging.getLogger("mitx.common.lib.seq_module") @@ -95,7 +96,8 @@ class SequenceModule(XModule): 'element_id': self.location.html_id(), 'item_id': self.id, 'position': self.position, - 'tag': self.location.category} + 'tag': self.location.category + } self.content = self.system.render_template('seq_module.html', params) self.rendered = True @@ -110,7 +112,7 @@ class SequenceModule(XModule): return new_class -class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): +class SequenceDescriptor(EditingDescriptor, XmlDescriptor): mako_template = 'widgets/sequence-edit.html' module_class = SequenceModule From a3bf3fb1ca34e454f51e093126271296e5cae075 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 21 Sep 2012 15:49:07 -0400 Subject: [PATCH 7/9] Put SequenceDescriptor back as a subclass of MakeModuleDescriptor. THe previous change to have it derive from EditorDescriptor was not safe as EditorDescriptor presumes that there is a edit box in the JS save flows. So we need to edit get_context() on MakoModuleDescriptor to also pass along the metadata to the Mako rendering templates --- common/lib/xmodule/xmodule/mako_module.py | 4 +++- common/lib/xmodule/xmodule/seq_module.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py index eedac99aa8..87c2c7d5f9 100644 --- a/common/lib/xmodule/xmodule/mako_module.py +++ b/common/lib/xmodule/xmodule/mako_module.py @@ -31,7 +31,9 @@ class MakoModuleDescriptor(XModuleDescriptor): """ Return the context to render the mako template with """ - return {'module': self} + return {'module': self, + 'metadata': self.metadata + } def get_html(self): return self.system.render_template( diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 0e0fd10fa4..8592d4b38c 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -112,7 +112,7 @@ class SequenceModule(XModule): return new_class -class SequenceDescriptor(EditingDescriptor, XmlDescriptor): +class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): mako_template = 'widgets/sequence-edit.html' module_class = SequenceModule From e6445ceaa0ef21c1b190623ecdb6a859147cc6d2 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 24 Sep 2012 13:03:34 -0400 Subject: [PATCH 8/9] adjust implementation to reflect feedback from Calen. Also limit the jQuery selection to be within the metadata-edit region, just to avoid picking up other elements decorated with that attribute --- cms/static/coffee/src/models/module.coffee | 2 +- cms/templates/widgets/metadata-edit.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/static/coffee/src/models/module.coffee b/cms/static/coffee/src/models/module.coffee index 4e1a926445..52357795ed 100644 --- a/cms/static/coffee/src/models/module.coffee +++ b/cms/static/coffee/src/models/module.coffee @@ -23,6 +23,6 @@ class CMS.Models.Module extends Backbone.Model _metadata = {} # walk through the set of elments which have the 'xmetadata_name' attribute and # build up a object to pass back to the server on the subsequent POST - _metadata[el.getAttribute("xmetadata_name")]=el.value for el in $('[xmetadata_name]') + _metadata[$(el).data("metadata-name")]=el.value for el in $('[data-metadata-name]', @metadata_elt) @set(metadata: _metadata) super(args...) diff --git a/cms/templates/widgets/metadata-edit.html b/cms/templates/widgets/metadata-edit.html index 368959022a..296b16e9c0 100644 --- a/cms/templates/widgets/metadata-edit.html +++ b/cms/templates/widgets/metadata-edit.html @@ -3,7 +3,7 @@

    Metadata

      % for keyname in metadata.keys(): -
    • ${keyname}:
    • +
    • ${keyname}:
    • % endfor
From a85b255271e8ed46c7ae9ae3ccc5fa610ae347fa Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 24 Sep 2012 16:54:46 -0400 Subject: [PATCH 9/9] Define 'system metadata' which should not be editable by the end user. When posting back metadata edits, we need to fetch a copy of the existing metadata and apply the changes. --- cms/djangoapps/contentstore/views.py | 11 ++++++++++- cms/templates/widgets/metadata-edit.html | 2 +- common/lib/xmodule/xmodule/editing_module.py | 19 +++++++++---------- common/lib/xmodule/xmodule/mako_module.py | 11 ++++++++++- common/lib/xmodule/xmodule/x_module.py | 4 ++++ 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 02625eb93f..74ff6657ea 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -104,6 +104,7 @@ def edit_item(request): item = modulestore().get_item(item_location) item.get_html = wrap_xmodule(item.get_html, item, "xmodule_edit.html") + return render_to_response('unit.html', { 'contents': item.get_html(), 'js_module': item.js_module_name, @@ -287,8 +288,16 @@ def save_item(request): # cdodge: also commit any metadata which might have been passed along in the # POST from the client, if it is there + # note, that the postback is not the complete metadata, as there's system metadata which is + # not presented to the end-user for editing. So let's fetch the original and + # 'apply' the submitted metadata, so we don't end up deleting system metadata if request.POST['metadata']: - modulestore().update_metadata(item_location, request.POST['metadata']) + posted_metadata = request.POST['metadata'] + # fetch original + existing_item = modulestore().get_item(item_location) + # update existing metadata with submitted metadata (which can be partial) + existing_item.metadata.update(posted_metadata) + modulestore().update_metadata(item_location, existing_item.metadata) # Export the course back to github # This uses wildcarding to find the course, which requires handling diff --git a/cms/templates/widgets/metadata-edit.html b/cms/templates/widgets/metadata-edit.html index 296b16e9c0..62d5563047 100644 --- a/cms/templates/widgets/metadata-edit.html +++ b/cms/templates/widgets/metadata-edit.html @@ -2,7 +2,7 @@