From 5390a19fc067fbb7d85e7b5b29132fe4ffd35634 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 3 Oct 2012 16:19:14 -0400 Subject: [PATCH 1/6] delete unit in overview and subsection_edit pages --- cms/djangoapps/contentstore/views.py | 27 ++++++++++++++++++++++----- cms/static/js/base.js | 16 ++++++++++++++++ cms/templates/widgets/units.html | 7 +++++-- cms/urls.py | 1 - 4 files changed, 43 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 1df2c44ff7..78a1a41bc1 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -209,10 +209,6 @@ def preview_component(request, location): }) -@login_required -def delete_unit(request, location): - pass - def user_author_string(user): '''Get an author string for commits by this user. Format: @@ -382,12 +378,33 @@ def get_module_previews(request, descriptor): preview_html.append(module.get_html()) return preview_html +def _delete_item(item, recurse=False): + if recurse: + children = item.get_children() + for child in children: + _delete_item(child) + + modulestore().delete_item(item.location); + @login_required @expect_json def delete_item(request): item_location = request.POST['id'] - modulestore().delete_item(item_location) + + # check permissions for this user within this course + if not has_access(request.user, item_location): + raise PermissionDenied() + + # optional parameter to delete all children (default False) + delete_children = False + if 'delete_children' in request.POST: + delete_children = request.POST['delete_children'] in ['true', 'True'] + + item = modulestore().get_item(item_location) + + _delete_item(item) + return HttpResponse() diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 16a7f87202..86846ee5e4 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -20,8 +20,24 @@ $(document).ready(function() { $('.unit-history ol a').bind('click', showHistoryModal); $modal.bind('click', hideHistoryModal); $modalCover.bind('click', hideHistoryModal); + + $('.unit .item-actions .delete-button').bind('click', deleteUnit); }); +function deleteUnit(e) { + e.preventDefault(); + var id = $(this).data('id'); + var _this = $(this); + + $.post('/delete_item', + {'id': id, 'delete_children' : 'true'}, + function(data) { + // remove 'leaf' class containing
  • element + var parent = _this.parents('li.leaf'); + parent.remove(); + }); +} + function toggleSubmodules(e) { e.preventDefault(); $(this).toggleClass('expand').toggleClass('collapse'); diff --git a/cms/templates/widgets/units.html b/cms/templates/widgets/units.html index 12b0d33039..2605277e71 100644 --- a/cms/templates/widgets/units.html +++ b/cms/templates/widgets/units.html @@ -6,13 +6,13 @@ This def will enumerate through a passed in subsection and list all of the units <%def name="enum_units(subsection)">
      % for unit in subsection.get_children(): -
    1. +
    2. ${unit.display_name} - private
      - +
      @@ -25,3 +25,6 @@ This def will enumerate through a passed in subsection and list all of the units
    + + + diff --git a/cms/urls.py b/cms/urls.py index 7cbb912b06..b57f894acb 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -11,7 +11,6 @@ urlpatterns = ('', url(r'^$', 'contentstore.views.index', name='index'), url(r'^edit/(?P.*?)$', 'contentstore.views.edit_unit', name='edit_unit'), url(r'^subsection/(?P.*?)$', 'contentstore.views.edit_subsection', name='edit_subsection'), - url(r'^delete/(?P.*?)$', 'contentstore.views.delete_unit', name='delete_unit'), url(r'^preview_component/(?P.*?)$', 'contentstore.views.preview_component', name='preview_component'), url(r'^save_item$', 'contentstore.views.save_item', name='save_item'), url(r'^delete_item$', 'contentstore.views.delete_item', name='delete_item'), From 45b82f3a451be468fd83fce1d8b9a32765d6a755 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 4 Oct 2012 10:58:48 -0400 Subject: [PATCH 2/6] update creating a new unit to use an empty template. Also twiddle the units.html and base.js where we put the unit Location on the
  • element --- cms/djangoapps/contentstore/views.py | 39 ++++++++++++++++++- cms/static/js/base.js | 30 +++++++++++--- cms/templates/widgets/units.html | 4 +- cms/urls.py | 1 + common/lib/xmodule/xmodule/vertical_module.py | 4 ++ 5 files changed, 69 insertions(+), 9 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index b6aeebd602..53923e5c6b 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -45,6 +45,8 @@ from auth.authz import get_user_by_email, add_user_to_course_group, remove_user_ from auth.authz import ADMIN_ROLE_NAME, EDITOR_ROLE_NAME from .utils import get_course_location_for_item +from xmodule.templates import all_templates + log = logging.getLogger(__name__) @@ -126,9 +128,14 @@ def course_index(request, org, course, name): course = modulestore().get_item(location) sections = course.get_children() + # This knowledge of what the 'new template' should be seems like it needs to be kept deeper down in the + # code. We should probably refactor + template = modulestore().get_item(Location('i4x', 'edx', 'templates', 'vertical', 'Empty')) + return render_to_response('overview.html', { 'sections': sections, - 'upload_asset_callback_url': upload_asset_callback_url + 'upload_asset_callback_url': upload_asset_callback_url, + 'create_new_unit_template': template.location }) @@ -144,8 +151,14 @@ def edit_subsection(request, location): if item.location.category != 'sequential': return HttpResponseBadRequest + # This knowledge of what the 'new template' should be seems like it needs to be kept deeper down in the + # code. We should probably refactor + template = modulestore().get_item(Location('i4x', 'edx', 'templates', 'vertical', 'Empty')) + return render_to_response('edit_subsection.html', - {'subsection':item}) + {'subsection':item, + 'create_new_unit_template' : template.location + }) @login_required def edit_unit(request, location): @@ -407,6 +420,20 @@ def delete_item(request): return HttpResponse() +@login_required +@expect_json +def create_item(request): + # parent_location should be the location of the parent container + parent_location = request.POST['parent_id'] + + # which type of item to create + category = request.POST['category'] + + # check permissions for this user within this course + if not has_access(request.user, parent_location): + raise PermissionDenied() + + @login_required @expect_json @@ -446,6 +473,10 @@ def save_item(request): def clone_item(request): parent_location = Location(request.POST['parent_location']) template = Location(request.POST['template']) + + display_name = None + if 'display_name' in request.POST: + display_name = request.POST['display_name'] if not has_access(request.user, parent_location): raise PermissionDenied() @@ -457,6 +488,10 @@ def clone_item(request): # 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) modulestore().update_children(parent_location, parent.definition.get('children', []) + [new_item.location.url()]) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 8d1af795b3..364cb61f3d 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -24,19 +24,39 @@ $(document).ready(function() { $('.assets .upload-button').bind('click', showUploadModal); $('.upload-modal .close-button').bind('click', hideModal); $('.unit .item-actions .delete-button').bind('click', deleteUnit); + $('.new-unit-item').bind('click', createNewUnit); }); +function createNewUnit(e) { + e.preventDefault(); + + parent = $(this).data('parent'); + template = $(this).data('template'); + + $.post('/clone_item', + {'parent_location' : parent, + 'template' : template, + 'display_name': 'New Unit', + }, + function(data) { + // redirect to the edit page + window.location = "/edit/" + data['id']; + }); +} + function deleteUnit(e) { e.preventDefault(); - var id = $(this).data('id'); - var _this = $(this); + + if(!confirm('Are you sure you wish to delete this item. It cannot be reversed!')) + return; + + var _li_el = $(this).parents('li.leaf'); + var id = _li_el.data('id'); $.post('/delete_item', {'id': id, 'delete_children' : 'true'}, function(data) { - // remove 'leaf' class containing
  • element - var parent = _this.parents('li.leaf'); - parent.remove(); + _li_el.remove(); }); } diff --git a/cms/templates/widgets/units.html b/cms/templates/widgets/units.html index 2605277e71..9d9c7943da 100644 --- a/cms/templates/widgets/units.html +++ b/cms/templates/widgets/units.html @@ -6,7 +6,7 @@ This def will enumerate through a passed in subsection and list all of the units <%def name="enum_units(subsection)">
      % for unit in subsection.get_children(): -
    1. +
    2. ${unit.display_name} - private @@ -19,7 +19,7 @@ This def will enumerate through a passed in subsection and list all of the units
      3. % endfor
    3. - + New Unit
      4. diff --git a/cms/urls.py b/cms/urls.py index ae22220511..f57f8d91d2 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -13,6 +13,7 @@ urlpatterns = ('', url(r'^subsection/(?P.*?)$', 'contentstore.views.edit_subsection', name='edit_subsection'), url(r'^preview_component/(?P.*?)$', 'contentstore.views.preview_component', name='preview_component'), url(r'^save_item$', 'contentstore.views.save_item', name='save_item'), + url(r'^create_item$', 'contentstore.views.create_item', name='create_item'), url(r'^delete_item$', 'contentstore.views.delete_item', name='delete_item'), url(r'^clone_item$', 'contentstore.views.clone_item', name='clone_item'), url(r'^(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', diff --git a/common/lib/xmodule/xmodule/vertical_module.py b/common/lib/xmodule/xmodule/vertical_module.py index f0c26e045f..738ee9ac5f 100644 --- a/common/lib/xmodule/xmodule/vertical_module.py +++ b/common/lib/xmodule/xmodule/vertical_module.py @@ -45,5 +45,9 @@ class VerticalModule(XModule): class VerticalDescriptor(SequenceDescriptor): module_class = VerticalModule + # cdodge: override the SequenceDescript's template_dir_name to point to default template directory + template_dir_name = "default" + js = {'coffee': [resource_string(__name__, 'js/src/vertical/edit.coffee')]} js_module_name = "VerticalDescriptor" + From 29c142e3dcf974373e8fb943b49b284cd5e996d0 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 4 Oct 2012 11:09:28 -0400 Subject: [PATCH 3/6] need to wire through the new unit template location to the edit unit page --- cms/djangoapps/contentstore/views.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 1ac82bc213..e044227f43 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -210,6 +210,10 @@ def edit_unit(request, location): containing_section_locs = modulestore().get_parent_locations(containing_subsection.location) containing_section = modulestore().get_item(containing_section_locs[0]) + # This knowledge of what the 'new template' should be seems like it needs to be kept deeper down in the + # code. We should probably refactor + template = modulestore().get_item(Location('i4x', 'edx', 'templates', 'vertical', 'Empty')) + return render_to_response('unit.html', { 'unit': item, 'components': components, @@ -217,6 +221,7 @@ def edit_unit(request, location): 'lms_link': lms_link, 'subsection': containing_subsection, 'section': containing_section, + 'create_new_unit_template' : template.location }) From d93bf63dff756ac67721b47e02cb5a02d8d7e416 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 4 Oct 2012 13:05:45 -0400 Subject: [PATCH 4/6] save display_name and subtitle metadata --- cms/static/js/base.js | 32 ++++++++++++++++++++++++++++++ cms/templates/edit_subsection.html | 6 +++--- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 364cb61f3d..59466d3b96 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -25,8 +25,40 @@ $(document).ready(function() { $('.upload-modal .close-button').bind('click', hideModal); $('.unit .item-actions .delete-button').bind('click', deleteUnit); $('.new-unit-item').bind('click', createNewUnit); + $('.save-subsection').bind('click', saveSubsection); }); +function saveSubsection(e) { + e.preventDefault(); + + var id = $(this).data('id'); + + // pull all metadata editable fields on page + var metadata_fields = $('input[data-metadata-name]'); + + metadata = {}; + for(var i=0; i< metadata_fields.length;i++) { + el = metadata_fields[i]; + metadata[$(el).data("metadata-name")] = el.value; + } + + children =[]; + + $.ajax({ + url: "/save_item", + type: "POST", + dataType: "json", + contentType: "application/json", + data:JSON.stringify({ 'id' : id, 'metadata' : metadata, 'data': null, 'children' : children}), + success: function() { + alert('Your changes have been saved.'); + }, + error: function() { + alert('There has been an error while saving your changes.'); + } + }); +} + function createNewUnit(e) { e.preventDefault(); diff --git a/cms/templates/edit_subsection.html b/cms/templates/edit_subsection.html index c9fe9fbda8..578e2a9ceb 100644 --- a/cms/templates/edit_subsection.html +++ b/cms/templates/edit_subsection.html @@ -12,11 +12,11 @@
      - +
      - +
      @@ -55,7 +55,7 @@ hideshow
      - Save + Save Preview
      From 48d84c2e3d2d2d7aadc865e2d58227edb0873096 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 4 Oct 2012 13:44:46 -0400 Subject: [PATCH 5/6] support reordering of units inside subsection list --- cms/static/js/base.js | 27 +++++++++++++++++++++++++++ cms/templates/widgets/units.html | 4 ++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 59466d3b96..48789bbae8 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -26,8 +26,35 @@ $(document).ready(function() { $('.unit .item-actions .delete-button').bind('click', deleteUnit); $('.new-unit-item').bind('click', createNewUnit); $('.save-subsection').bind('click', saveSubsection); + + // making the unit list sortable + $('.sortable-unit-list').sortable(); + $('.sortable-unit-list').disableSelection(); + $('.sortable-unit-list').bind('sortstop', onUnitReordered); }); +// This method only changes the ordering of the child objects in a subsection +function onUnitReordered() { + var subsection_id = $(this).data('subsection-id'); + + var _els = $(this).children('li:.leaf'); + + var children = new Array(); + for(var i=0;i<_els.length;i++) { + el = _els[i]; + children[i] = $(el).data('id'); + } + + // call into server to commit the new order + $.ajax({ + url: "/save_item", + type: "POST", + dataType: "json", + contentType: "application/json", + data:JSON.stringify({ 'id' : subsection_id, 'metadata' : null, 'data': null, 'children' : children}) + }); +} + function saveSubsection(e) { e.preventDefault(); diff --git a/cms/templates/widgets/units.html b/cms/templates/widgets/units.html index cb9d06db4c..ee7c02dedb 100644 --- a/cms/templates/widgets/units.html +++ b/cms/templates/widgets/units.html @@ -3,8 +3,8 @@ -<%def name="enum_units(subsection, actions=True, selected=None)"> -
        +<%def name="enum_units(subsection, actions=True, selected=None, sortable=True)"> +
          % for unit in subsection.get_children():
        1. <% From fe42f6fd22bd2570b58661157a99356eeacc1451 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 4 Oct 2012 14:00:25 -0400 Subject: [PATCH 6/6] address pull request feedback --- cms/djangoapps/contentstore/views.py | 44 +++++----------------------- cms/static/js/base.js | 2 +- cms/templates/widgets/units.html | 4 +-- cms/urls.py | 1 - 4 files changed, 10 insertions(+), 41 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index e044227f43..b8da38f2c7 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -128,14 +128,10 @@ def course_index(request, org, course, name): course = modulestore().get_item(location) sections = course.get_children() - # This knowledge of what the 'new template' should be seems like it needs to be kept deeper down in the - # code. We should probably refactor - template = modulestore().get_item(Location('i4x', 'edx', 'templates', 'vertical', 'Empty')) - return render_to_response('overview.html', { 'sections': sections, 'upload_asset_callback_url': upload_asset_callback_url, - 'create_new_unit_template': template.location + 'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty') }) @@ -151,13 +147,9 @@ def edit_subsection(request, location): if item.location.category != 'sequential': return HttpResponseBadRequest - # This knowledge of what the 'new template' should be seems like it needs to be kept deeper down in the - # code. We should probably refactor - template = modulestore().get_item(Location('i4x', 'edx', 'templates', 'vertical', 'Empty')) - return render_to_response('edit_subsection.html', {'subsection': item, - 'create_new_unit_template' : template.location + 'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty') }) @login_required @@ -210,10 +202,6 @@ def edit_unit(request, location): containing_section_locs = modulestore().get_parent_locations(containing_subsection.location) containing_section = modulestore().get_item(containing_section_locs[0]) - # This knowledge of what the 'new template' should be seems like it needs to be kept deeper down in the - # code. We should probably refactor - template = modulestore().get_item(Location('i4x', 'edx', 'templates', 'vertical', 'Empty')) - return render_to_response('unit.html', { 'unit': item, 'components': components, @@ -221,7 +209,7 @@ def edit_unit(request, location): 'lms_link': lms_link, 'subsection': containing_subsection, 'section': containing_section, - 'create_new_unit_template' : template.location + 'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty') }) @@ -412,7 +400,7 @@ def _delete_item(item, recurse=False): if recurse: children = item.get_children() for child in children: - _delete_item(child) + _delete_item(child, recurse) modulestore().delete_item(item.location); @@ -427,30 +415,14 @@ def delete_item(request): raise PermissionDenied() # optional parameter to delete all children (default False) - delete_children = False - if 'delete_children' in request.POST: - delete_children = request.POST['delete_children'] in ['true', 'True'] + delete_children = request.POST.get('delete_children', False) item = modulestore().get_item(item_location) - _delete_item(item) + _delete_item(item, delete_children) return HttpResponse() -@login_required -@expect_json -def create_item(request): - # parent_location should be the location of the parent container - parent_location = request.POST['parent_id'] - - # which type of item to create - category = request.POST['category'] - - # check permissions for this user within this course - if not has_access(request.user, parent_location): - raise PermissionDenied() - - @login_required @expect_json @@ -491,9 +463,7 @@ def clone_item(request): parent_location = Location(request.POST['parent_location']) template = Location(request.POST['template']) - display_name = None - if 'display_name' in request.POST: - display_name = request.POST['display_name'] + display_name = request.POST.get('display_name') if not has_access(request.user, parent_location): raise PermissionDenied() diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 48789bbae8..66f76c265f 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -113,7 +113,7 @@ function deleteUnit(e) { var id = _li_el.data('id'); $.post('/delete_item', - {'id': id, 'delete_children' : 'true'}, + {'id': id, 'delete_children' : true}, function(data) { _li_el.remove(); }); diff --git a/cms/templates/widgets/units.html b/cms/templates/widgets/units.html index ee7c02dedb..8207b485a7 100644 --- a/cms/templates/widgets/units.html +++ b/cms/templates/widgets/units.html @@ -22,14 +22,14 @@ This def will enumerate through a passed in subsection and list all of the units % if actions:
          - +
          % endif
          2. % endfor
        2. - + New Unit
          3. diff --git a/cms/urls.py b/cms/urls.py index 0caecd654a..44f42343f3 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -13,7 +13,6 @@ urlpatterns = ('', url(r'^subsection/(?P.*?)$', 'contentstore.views.edit_subsection', name='edit_subsection'), url(r'^preview_component/(?P.*?)$', 'contentstore.views.preview_component', name='preview_component'), url(r'^save_item$', 'contentstore.views.save_item', name='save_item'), - url(r'^create_item$', 'contentstore.views.create_item', name='create_item'), url(r'^delete_item$', 'contentstore.views.delete_item', name='delete_item'), url(r'^clone_item$', 'contentstore.views.clone_item', name='clone_item'), url(r'^(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$',