diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index ff347a2878..bbd95dba84 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -58,14 +58,12 @@ def save_item(request): # 'apply' the submitted metadata, so we don't end up deleting system metadata existing_item = modulestore().get_item(item_location) for metadata_key in request.POST.get('nullout', []): - # [dhm] see comment on _get_xblock_field _get_xblock_field(existing_item, metadata_key).write_to(existing_item, None) # update existing metadata with submitted metadata (which can be partial) # IMPORTANT NOTE: if the client passed 'null' (None) for a piece of metadata that means 'remove it'. If # the intent is to make it None, use the nullout field for metadata_key, value in request.POST.get('metadata', {}).items(): - # [dhm] see comment on _get_xblock_field field = _get_xblock_field(existing_item, metadata_key) if value is None: @@ -82,32 +80,15 @@ def save_item(request): return JsonResponse() -# [DHM] A hack until we implement a permanent soln. Proposed perm solution is to make namespace fields also top level -# fields in xblocks rather than requiring dereference through namespace but we'll need to consider whether there are -# plausible use cases for distinct fields w/ same name in different namespaces on the same blocks. -# The idea is that consumers of the xblock, and particularly the web client, shouldn't know about our internal -# representation (namespaces as means of decorating all modules). -# Given top-level access, the calls can simply be setattr(existing_item, field, value) ... -# Really, this method should be elsewhere (e.g., xblock). We also need methods for has_value (v is_default)... def _get_xblock_field(xblock, field_name): """ A temporary function to get the xblock field either from the xblock or one of its namespaces by name. :param xblock: :param field_name: """ - def find_field(fields): - for field in fields: - if field.name == field_name: - return field - - found = find_field(xblock.fields) - if found: - return found - for namespace in xblock.namespaces: - found = find_field(getattr(xblock, namespace).fields) - if found: - return found - + for field in xblock.iterfields(): + if field.name == field_name: + return field @login_required @expect_json diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index ba1626c1b2..cc8c39dca3 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -657,6 +657,17 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): ) ) + def iterfields(self): + """ + A generator for iterate over the fields of this xblock (including the ones in namespaces). + Example usage: [field.name for field in module.iterfields()] + """ + for field in self.fields: + yield field + for namespace in self.namespaces: + for field in getattr(self, namespace).fields: + yield field + @property def non_editable_metadata_fields(self): """ @@ -675,17 +686,17 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): if scope == Scope.settings and hasattr(self, '_inherited_metadata'): inherited_metadata = getattr(self, '_inherited_metadata') result = {} - for field in self.fields: + for field in self.iterfields(): if (field.scope == scope and field.name in self._model_data and field.name not in inherited_metadata): - result[field.name] = getattr(self, field.name) + result[field.name] = self._model_data[field.name] return result else: result = {} - for field in self.fields: + for field in self.iterfields(): if (field.scope == scope and field.name in self._model_data): - result[field.name] = getattr(self, field.name) + result[field.name] = self._model_data[field.name] return result @property