From 1015b3a974c354d8e2d63c3a0182b92a50304095 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 11 Oct 2012 17:18:02 -0400 Subject: [PATCH 1/3] DraftMongoContentStore needs to both delete the draft *and* the non-draft entities --- common/lib/xmodule/xmodule/modulestore/draft.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/draft.py b/common/lib/xmodule/xmodule/modulestore/draft.py index 78600f1ba0..33ee9eaab2 100644 --- a/common/lib/xmodule/xmodule/modulestore/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/draft.py @@ -157,7 +157,11 @@ class DraftModuleStore(ModuleStoreBase): location: Something that can be passed to Location """ - return super(DraftModuleStore, self).delete_item(as_draft(location)) + + # cdodge: we should always delete both the draft and the original + super(DraftModuleStore, self).delete_item(as_draft(location)) + if (location.revision == None): # did caller request to delete the non-draft, if so be sure to do that + super(DraftModuleStore, self).delete_item(location) def get_parent_locations(self, location): '''Find all locations that are the parents of this location. Needed From 281f194b2a117b738e7db8b0cc66cd63ff3b987c Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 12 Oct 2012 09:25:13 -0400 Subject: [PATCH 2/3] per feedback, change back 'draft-aware' modulestore to not delete the published copy. In delete_item in views.py do the logic to determine whether we should be using a 'draft-aware' modulestore or not, based on the category type --- cms/djangoapps/contentstore/views.py | 10 ++++++++-- common/lib/xmodule/xmodule/modulestore/draft.py | 5 +---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 8911150fbb..7d27db51e2 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -470,10 +470,16 @@ def delete_item(request): item = modulestore().get_item(item_location) + _direct_delete_categories = ['course', 'chapter', 'sequential'] + + # @TODO: this probably leaves draft items dangling. + if delete_children: - _xmodule_recurse(item, lambda i: modulestore().delete_item(i.location)) + _xmodule_recurse(item, lambda i: modulestore('direct' if + i.location.category in _direct_delete_categories else 'direct').delete_item(i.location)) else: - modulestore().delete_item(item.location) + modulestore('direct' if + item.location.category in _direct_delete_categories else 'direct').delete_item(item.location) return HttpResponse() diff --git a/common/lib/xmodule/xmodule/modulestore/draft.py b/common/lib/xmodule/xmodule/modulestore/draft.py index 33ee9eaab2..5fbf05ed9b 100644 --- a/common/lib/xmodule/xmodule/modulestore/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/draft.py @@ -157,11 +157,8 @@ class DraftModuleStore(ModuleStoreBase): location: Something that can be passed to Location """ + return super(DraftModuleStore, self).delete_item(as_draft(location)) - # cdodge: we should always delete both the draft and the original - super(DraftModuleStore, self).delete_item(as_draft(location)) - if (location.revision == None): # did caller request to delete the non-draft, if so be sure to do that - super(DraftModuleStore, self).delete_item(location) def get_parent_locations(self, location): '''Find all locations that are the parents of this location. Needed From 3c8de11f090bd6bc3fb655160935a1f078c8ce09 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 12 Oct 2012 11:10:12 -0400 Subject: [PATCH 3/3] Standardize on how we bypass the draft-aware modulestore --- cms/djangoapps/contentstore/views.py | 37 ++++++++++++++-------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 7d27db51e2..85d0253a7e 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -57,6 +57,18 @@ log = logging.getLogger(__name__) COMPONENT_TYPES = ['customtag', 'discussion', 'html', 'problem', 'video'] +DIRECT_ONLY_CATEGORIES = ['course', 'chapter', 'sequential'] + + +def _modulestore(location): + """ + Returns the correct modulestore to use for modifying the specified location + """ + if location.category in DIRECT_ONLY_CATEGORIES: + return modulestore('direct') + else: + return modulestore() + # ==== Public views ================================================== @@ -470,16 +482,13 @@ def delete_item(request): item = modulestore().get_item(item_location) - _direct_delete_categories = ['course', 'chapter', 'sequential'] # @TODO: this probably leaves draft items dangling. if delete_children: - _xmodule_recurse(item, lambda i: modulestore('direct' if - i.location.category in _direct_delete_categories else 'direct').delete_item(i.location)) + _xmodule_recurse(item, lambda i: _modulestore(i.location).delete_item(i.location)) else: - modulestore('direct' if - item.location.category in _direct_delete_categories else 'direct').delete_item(item.location) + _modulestore(item.location).delete_item(item.location) return HttpResponse() @@ -591,28 +600,20 @@ def clone_item(request): if not has_access(request.user, parent_location): raise PermissionDenied() - # if we are creating a new section or subsection, then we don't want draft awareness - _modulestore = modulestore() if template.category not in ('sequential','chapter') else modulestore('direct') - - parent = _modulestore.get_item(parent_location) + parent = _modulestore(template).get_item(parent_location) dest_location = parent_location._replace(category=template.category, name=uuid4().hex) - new_item = _modulestore.clone_item(template, dest_location) + new_item = _modulestore(template).clone_item(template, dest_location) # TODO: This needs to be deleted when we have proper storage for static content new_item.metadata['data_dir'] = parent.metadata['data_dir'] - + # replace the display name with an optional parameter passed in from the caller if display_name is not None: new_item.metadata['display_name'] = display_name - _modulestore.update_metadata(new_item.location.url(), new_item.own_metadata) - - if parent_location.category not in ('vertical',): - parent_update_modulestore = modulestore('direct') - else: - parent_update_modulestore = modulestore() - parent_update_modulestore.update_children(parent_location, parent.definition.get('children', []) + [new_item.location.url()]) + _modulestore(template).update_metadata(new_item.location.url(), new_item.own_metadata) + _modulestore(parent.location).update_children(parent_location, parent.definition.get('children', []) + [new_item.location.url()]) return HttpResponse(json.dumps({'id': dest_location.url()}))