Change delete_item to RESTful URL.

Part of STUD-847.

cms/djangoapps/auth/authz.py

@@ -11,6 +11,7 @@ from django.conf import settings
 from xmodule.modulestore import Location
 from xmodule.modulestore.locator import CourseLocator, Locator
 from xmodule.modulestore.django import loc_mapper
+from xmodule.modulestore.exceptions import InvalidLocationError
 
 
 # define a couple of simple roles, we just need ADMIN and EDITOR now for our purposes
@@ -32,11 +33,14 @@ def get_course_groupname_for_role(location, role):
     # if it exists, then use that one, otherwise use a <role>_<course_id> which contains
     # more information
     groupnames = []
-    groupnames.append('{0}_{1}'.format(role, location.course_id))
+    try:
+        groupnames.append('{0}_{1}'.format(role, location.course_id))
+    except InvalidLocationError:  # will occur on old locations where location is not of category course
+        pass
     if isinstance(location, Location):
         groupnames.append('{0}_{1}'.format(role, location.course))
     elif isinstance(location, CourseLocator):
-        old_location = loc_mapper().translate_locator_to_location(location)
+        old_location = loc_mapper().translate_locator_to_location(location, get_course=True)
         if old_location:
             groupnames.append('{0}_{1}'.format(role, old_location.course_id))
 

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -488,11 +488,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         # make sure the parent points to the child object which is to be deleted
         self.assertTrue(sequential.location.url() in chapter.children)
 
-        self.client.post(
-            reverse('delete_item'),
-            json.dumps({'id': sequential.location.url(), 'delete_children': 'true', 'delete_all_versions': 'true'}),
-            "application/json"
-        )
+        location = loc_mapper().translate_location(course_location.course_id, sequential.location, False, True)
+        self.client.delete(location.url_reverse('xblock'), {'recurse': True, 'all_versions': True})
 
         found = False
         try:
@@ -1640,29 +1637,24 @@ class ContentStoreTest(ModuleStoreTestCase):
                                        kwargs={'location': unit_location.url()}))
         self.assertEqual(resp.status_code, 200)
 
+        def delete_item(category, name):
+            """ Helper method for testing the deletion of an xblock item. """
+            del_loc = loc.replace(category=category, name=name)
+            del_location = loc_mapper().translate_location(loc.course_id, del_loc, False, True)
+            resp = self.client.delete(del_location.url_reverse('xblock'))
+            self.assertEqual(resp.status_code, 204)
+
         # delete a component
-        del_loc = loc.replace(category='html', name='test_html')
-        resp = self.client.post(reverse('delete_item'),
-                                json.dumps({'id': del_loc.url()}), "application/json")
-        self.assertEqual(resp.status_code, 204)
+        delete_item(category='html', name='test_html')
 
         # delete a unit
-        del_loc = loc.replace(category='vertical', name='test_vertical')
-        resp = self.client.post(reverse('delete_item'),
-                                json.dumps({'id': del_loc.url()}), "application/json")
-        self.assertEqual(resp.status_code, 204)
+        delete_item(category='vertical', name='test_vertical')
 
         # delete a unit
-        del_loc = loc.replace(category='sequential', name='test_sequence')
-        resp = self.client.post(reverse('delete_item'),
-                                json.dumps({'id': del_loc.url()}), "application/json")
-        self.assertEqual(resp.status_code, 204)
+        delete_item(category='sequential', name='test_sequence')
 
         # delete a chapter
-        del_loc = loc.replace(category='chapter', name='chapter_2')
-        resp = self.client.post(reverse('delete_item'),
-                                json.dumps({'id': del_loc.url()}), "application/json")
-        self.assertEqual(resp.status_code, 204)
+        delete_item(category='chapter', name='chapter_2')
 
     def test_import_into_new_course_id(self):
         module_store = modulestore('direct')

cms/djangoapps/contentstore/tests/test_item.py

@@ -12,7 +12,7 @@ from xmodule.modulestore.django import modulestore
 
 
 class DeleteItem(CourseTestCase):
-    """Tests for '/delete_item' url."""
+    """Tests for '/xblock' DELETE url."""
     def setUp(self):
         """ Creates the test course with a static page in it. """
         super(DeleteItem, self).setUp()
@@ -33,11 +33,8 @@ class DeleteItem(CourseTestCase):
         self.assertEqual(resp.status_code, 200)
 
         # Now delete it. There was a bug that the delete was failing (static tabs do not exist in draft modulestore).
-        resp = self.client.post(
-            reverse('delete_item'),
-            resp.content,
-            "application/json"
-        )
+        resp_content = json.loads(resp.content)
+        resp = self.client.delete(resp_content['update_url'])
         self.assertEqual(resp.status_code, 204)
 
 

cms/djangoapps/contentstore/views/component.py

@@ -16,6 +16,7 @@ from mitxmako.shortcuts import render_to_response
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
 from xmodule.util.date_utils import get_default_time_display
+from xmodule.modulestore.django import loc_mapper
 
 from xblock.fields import Scope
 from util.json_request import expect_json, JsonResponse
@@ -174,6 +175,12 @@ def edit_unit(request, location):
             course_id=course.location.course_id
     )
 
+    # Note that the unit_state (draft, public, private) does not match up with the published value
+    # passed to translate_location. The two concepts are different at this point.
+    unit_update_url = loc_mapper().translate_location(
+        course.location.course_id, Location(location), False, True
+    ).url_reverse("xblock", "")
+
     component_templates = defaultdict(list)
     for category in COMPONENT_TYPES:
         component_class = load_mixed_class(category)
@@ -238,7 +245,12 @@ def edit_unit(request, location):
         )
 
     components = [
-        component.location.url()
+        [
+            component.location.url(),
+            loc_mapper().translate_location(
+                course.location.course_id, component.location, False, True
+            ).url_reverse("xblock")
+        ]
         for component
         in item.get_children()
     ]
@@ -283,12 +295,11 @@ def edit_unit(request, location):
             index=index
         )
 
-    unit_state = compute_unit_state(item)
-
     return render_to_response('unit.html', {
         'context_course': course,
         'unit': item,
         'unit_location': location,
+        'unit_update_url': unit_update_url,
         'components': components,
         'component_templates': component_templates,
         'draft_preview_link': preview_lms_link,
@@ -300,7 +311,7 @@ def edit_unit(request, location):
         ),
         'section': containing_section,
         'new_unit_category': 'vertical',
-        'unit_state': unit_state,
+        'unit_state': compute_unit_state(item),
         'published_date': (
             get_default_time_display(item.published_date)
             if item.published_date is not None else None

cms/djangoapps/contentstore/views/item.py

@@ -16,16 +16,16 @@ from util.json_request import expect_json, JsonResponse
 
 from ..transcripts_utils import manage_video_subtitles_save
 
-from ..utils import get_modulestore
+from ..utils import get_modulestore, get_course_for_item
 
 from .access import has_access
 from .helpers import _xmodule_recurse
 from xmodule.x_module import XModuleDescriptor
 from django.views.decorators.http import require_http_methods
-from xmodule.modulestore.locator import CourseLocator, BlockUsageLocator
+from xmodule.modulestore.locator import BlockUsageLocator
 from student.models import CourseEnrollment
 
-__all__ = ['save_item', 'create_item', 'delete_item', 'orphan']
+__all__ = ['save_item', 'create_item', 'orphan', 'xblock_handler']
 
 log = logging.getLogger(__name__)
 
@@ -33,6 +33,31 @@ log = logging.getLogger(__name__)
 DETACHED_CATEGORIES = ['about', 'static_tab', 'course_info']
 
 
+@require_http_methods(("DELETE"))
+@login_required
+@expect_json
+def xblock_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None):
+    """
+    The restful handler for xblock requests.
+
+    DELETE
+        json: delete this xblock instance from the course. Supports query parameters "recurse" to delete
+        all children and "all_versions" to delete from all (mongo) versions.
+    """
+    if request.method == 'DELETE':
+        location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block)
+        if not has_access(request.user, location):
+            raise PermissionDenied()
+
+        old_location = loc_mapper().translate_locator_to_location(location)
+
+        delete_children = bool(request.REQUEST.get('recurse', False))
+        delete_all_versions = bool(request.REQUEST.get('all_versions', False))
+
+        _delete_item_at_location(old_location, delete_children, delete_all_versions)
+        return JsonResponse()
+
+
 @login_required
 @expect_json
 def save_item(request):
@@ -171,24 +196,18 @@ def create_item(request):
     if category not in DETACHED_CATEGORIES:
         get_modulestore(parent.location).update_children(parent_location, parent.children + [dest_location.url()])
 
-    return JsonResponse({'id': dest_location.url()})
+    locator = loc_mapper().translate_location(
+        get_course_for_item(parent_location).location.course_id, dest_location, False, True
+    )
+    return JsonResponse({'id': dest_location.url(), "update_url": locator.url_reverse("xblock")})
 
 
-@login_required
-@expect_json
-def delete_item(request):
-    """View for removing items."""
-    item_location = request.json['id']
-    item_location = Location(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 = request.json.get('delete_children', False)
-    delete_all_versions = request.json.get('delete_all_versions', False)
+def _delete_item_at_location(item_location, delete_children=False, delete_all_versions=False):
+    """
+    Deletes the item at with the given Location.
 
+    It is assumed that course permissions have already been checked.
+    """
     store = get_modulestore(item_location)
 
     item = store.get_item(item_location)
@@ -211,7 +230,6 @@ def delete_item(request):
                 parent.children = children
                 modulestore('direct').update_children(parent.location, parent.children)
 
-    return JsonResponse()
 
 # pylint: disable=W0613
 @login_required

cms/djangoapps/contentstore/views/tabs.py

@@ -12,6 +12,7 @@ from mitxmako.shortcuts import render_to_response
 from xmodule.modulestore import Location
 from xmodule.modulestore.inheritance import own_metadata
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.django import loc_mapper
 
 from ..utils import get_course_for_item, get_modulestore
 
@@ -117,7 +118,12 @@ def edit_tabs(request, org, course, coursename):
         static_tabs.append(modulestore('direct').get_item(static_tab_loc))
 
     components = [
-        static_tab.location.url()
+        [
+            static_tab.location.url(),
+            loc_mapper().translate_location(
+                course_item.location.course_id, static_tab.location, False, True
+            ).url_reverse("xblock")
+        ]
         for static_tab
         in static_tabs
     ]

cms/static/coffee/src/views/module_edit.coffee

@@ -70,6 +70,7 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1",
           (data) =>
               @model.set(id: data.id)
               @$el.data('id', data.id)
+              @$el.data('update_url', data.update_url)
               @render()
       )
 

cms/static/coffee/src/views/tabs.coffee

@@ -82,9 +82,10 @@ define ["jquery", "jquery.ui", "backbone", "js/views/feedback_prompt", "js/views
               deleting = new NotificationView.Mini
                 title: gettext('Deleting…')
               deleting.show()
-              $.postJSON('/delete_item', {
-                id: $component.data('id')
-              }, =>
+              $.ajax({
+                type: 'DELETE',
+                url: $component.data('update_url')
+              }).success(=>
                 $component.remove()
                 deleting.hide()
               )

cms/static/coffee/src/views/unit.coffee

@@ -134,9 +134,10 @@ define ["jquery", "jquery.ui", "gettext", "backbone",
                 title: gettext('Deleting…'),
               deleting.show()
               $component = $(event.currentTarget).parents('.component')
-              $.postJSON('/delete_item', {
-                id: $component.data('id')
-              }, =>
+              $.ajax({
+                type: 'DELETE',
+                url: $component.data('update_url')
+              }).success(=>
                 deleting.hide()
                 analytics.track "Deleted a Component",
                   course: course_location_analytics
@@ -162,16 +163,16 @@ define ["jquery", "jquery.ui", "gettext", "backbone",
 
     deleteDraft: (event) ->
       @wait(true)
+      $.ajax({
+          type: 'DELETE',
+          url: @$el.data('update_url') + "?" + $.param({recurse: true})
+      }).success(=>
 
-      $.postJSON('/delete_item', {
-        id: @$el.data('id')
-        delete_children: true
-      }, =>
-        analytics.track "Deleted Draft",
-          course: course_location_analytics
-          unit_id: unit_location_analytics
+          analytics.track "Deleted Draft",
+              course: course_location_analytics
+              unit_id: unit_location_analytics
 
-        window.location.reload()
+          window.location.reload()
       )
 
     createDraft: (event) ->

cms/static/js/base.js

@@ -279,15 +279,14 @@ function _deleteItem($el, type) {
                     });
                     deleting.show();
 
-                    $.postJSON('/delete_item',
-                           {'id': id,
-                            'delete_children': true,
-                            'delete_all_versions': true},
-                           function(data) {
-                               $el.remove();
-                               deleting.hide();
-                           }
-                          );
+                    $.ajax({
+                        type: 'DELETE',
+                        url: $el.data('update_url')+'?'+ $.param({recurse: true, all_versions: true}),
+                        success: function () {
+                            $el.remove();
+                            deleting.hide();
+                        }
+                    });
                 }
             },
             secondary: {

cms/templates/edit-tabs.html

@@ -61,8 +61,8 @@ require(["coffee/src/views/tabs", "coffee/src/models/module"], function(TabsEdit
 
       <div class="tab-list">
         <ol class='components'>
-          % for id in components:
-          <li class="component" data-id="${id}"/>
+          % for id, update_url in components:
+          <li class="component" data-id="${id}" data-update_url="${update_url}"/>
           % endfor
 
           <li class="new-component-item">

cms/templates/overview.html

@@ -4,6 +4,7 @@
   from xmodule.util import date_utils
   from django.utils.translation import ugettext as _
   from django.core.urlresolvers import reverse
+  from xmodule.modulestore.django import loc_mapper
 %>
 <%block name="title">${_("Course Outline")}</%block>
 <%block name="bodyclass">is-signedin course view-outline</%block>
@@ -141,7 +142,13 @@ require(["domReady!", "jquery", "js/models/location", "js/models/section", "js/v
       <div class="wrapper-dnd">
         <article class="courseware-overview" data-id="${context_course.location.url()}">
           % for section in sections:
-          <section class="courseware-section branch is-draggable" data-id="${section.location}" data-parent-id="${context_course.location.url()}">
+          <%
+            section_update_url = loc_mapper().translate_location(
+              context_course.location.course_id, section.location, False, True
+            ).url_reverse('xblock')
+          %>
+          <section class="courseware-section branch is-draggable" data-id="${section.location}"
+                   data-parent-id="${context_course.location.url()}" data-update_url="${section_update_url}">
 
             <%include file="widgets/_ui-dnd-indicator-before.html" />
 
@@ -184,7 +191,13 @@ require(["domReady!", "jquery", "js/models/location", "js/models/section", "js/v
               </div>
               <ol class="sortable-subsection-list" data-id="${section.location.url()}">
                 % for subsection in section.get_children():
-                <li class="courseware-subsection branch collapsed id-holder is-draggable" data-id="${subsection.location}" data-parent-id="${section.location.url()}">
+                  <%
+                    subsection_update_url = loc_mapper().translate_location(
+                      context_course.location.course_id, subsection.location, False, True
+                    ).url_reverse('xblock')
+                  %>
+                <li class="courseware-subsection branch collapsed id-holder is-draggable" data-id="${subsection.location}"
+                    data-parent-id="${section.location.url()}" data-update_url="${subsection_update_url}">
 
                   <%include file="widgets/_ui-dnd-indicator-before.html" />
 

cms/templates/unit.html

@@ -34,7 +34,7 @@ require(["domReady!", "jquery", "coffee/src/models/module", "coffee/src/views/un
 
 </%block>
 <%block name="content">
-  <div class="main-wrapper edit-state-${unit_state}" data-id="${unit_location}">
+  <div class="main-wrapper edit-state-${unit_state}" data-id="${unit_location}" data-update_url="${unit_update_url}">
     <div class="inner-wrapper">
       <div class="alert editing-draft-alert">
         <p class="alert-message"><strong>${_("You are editing a draft.")}</strong>
@@ -48,8 +48,8 @@ require(["domReady!", "jquery", "coffee/src/models/module", "coffee/src/views/un
         <article class="unit-body window">
         <p class="unit-name-input"><label>${_("Display Name:")}</label><input type="text" value="${unit.display_name_with_default | h}" class="unit-display-name-input" /></p>
           <ol class="components">
-            % for id in components:
-            <li class="component" data-id="${id}"/>
+            % for id, component_update_url in components:
+            <li class="component" data-id="${id}" data-update_url="${component_update_url}"/>
             % endfor
             <li class="new-component-item adding">
               <div class="new-component">

cms/templates/widgets/units.html

@@ -1,5 +1,6 @@
 <%! from django.core.urlresolvers import reverse %>
 <%! from contentstore.utils import compute_unit_state %>
+<%! from xmodule.modulestore.django import loc_mapper %>
 
 <!--
 This def will enumerate through a passed in subsection and list all of the units
@@ -11,7 +12,11 @@ This def will enumerate through a passed in subsection and list all of the units
       subsection_units = subsection.get_children()
   %>
   % for unit in subsection_units:
-  <li class="courseware-unit leaf unit is-draggable" data-id="${unit.location}" data-parent-id="${subsection.location.url()}">
+  <%
+    unit_update_url = loc_mapper().translate_location(context_course.location.course_id, unit.location, False, True).url_reverse('xblock')
+  %>
+  <li class="courseware-unit leaf unit is-draggable" data-id="${unit.location}" data-parent-id="${subsection.location.url()}"
+      data-update_url="${unit_update_url}" >
 
     <%include file="_ui-dnd-indicator-before.html" />
 
@@ -29,7 +34,9 @@ This def will enumerate through a passed in subsection and list all of the units
       </a>
       % if actions:
       <div class="item-actions">
-	<a href="#" data-tooltip="Delete this unit" class="delete-button" data-id="${unit.location}"><span class="delete-icon"></span></a>
+	<a href="#" data-tooltip="Delete this unit" class="delete-button" data-id="${unit.location}"
+       data-update_url="${unit_update_url}">
+        <span class="delete-icon"></span></a>
 	<span data-tooltip="Drag to sort" class="drag-handle unit-drag-handle"></span>
       </div>
       % endif

cms/urls.py

@@ -1,4 +1,3 @@
-import re
 from django.conf import settings
 from django.conf.urls import patterns, include, url
 
@@ -17,7 +16,6 @@ urlpatterns = patterns('',  # nopep8
     url(r'^subsection/(?P<location>.*?)$', 'contentstore.views.edit_subsection', name='edit_subsection'),
     url(r'^preview_component/(?P<location>.*?)$', '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'),
     url(r'^create_item$', 'contentstore.views.create_item', name='create_item'),
 
     url(r'^transcripts/upload$', 'contentstore.views.upload_transcripts', name='upload_transcripts'),
@@ -121,6 +119,7 @@ urlpatterns += patterns(
     url(r'(?ix)^import/{}$'.format(parsers.URL_RE_SOURCE), 'import_handler'),
     url(r'(?ix)^import_status/{}/(?P<filename>.+)$'.format(parsers.URL_RE_SOURCE), 'import_status_handler'),
     url(r'(?ix)^export/{}$'.format(parsers.URL_RE_SOURCE), 'export_handler'),
+    url(r'(?ix)^xblock/{}$'.format(parsers.URL_RE_SOURCE), 'xblock_handler'),
 )
 
 js_info_dict = {