Merge pull request #1580 from edx/christina/item

Change delete_item to RESTful URL.

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

 <%! 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

-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 = {

common/djangoapps/util/json_request.py

@@ -17,7 +17,7 @@ def expect_json(view_function):
     def parse_json_into_request(request, *args, **kwargs):
         # 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 "application/json" in request.META.get('CONTENT_TYPE', ''):
+        if "application/json" in request.META.get('CONTENT_TYPE', '') and request.body:
             request.json = json.loads(request.body)
         else:
             request.json = {}

common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py

@@ -167,7 +167,7 @@ class LocMapperStore(object):
 
         return BlockUsageLocator(course_id=entry['course_id'], branch=branch, usage_id=usage_id)
 
-    def translate_locator_to_location(self, locator):
+    def translate_locator_to_location(self, locator, get_course=False):
         """
         Returns an old style Location for the given Locator if there's an appropriate entry in the
         mapping collection. Note, it requires that the course was previously mapped (a side effect of
@@ -175,6 +175,9 @@ class LocMapperStore(object):
         the block's usage_id was previously stored in the
         map (a side effect of translate_location or via add|update_block_location).
 
+        If get_course, then rather than finding the map for this locator, it finds the 'course' root
+        for the mapped course.
+
         If there are no matches, it returns None.
 
         If there's more than one location to locator mapping to the same course_id, it looks for the first
@@ -191,7 +194,16 @@ class LocMapperStore(object):
         for candidate in maps:
             for old_name, cat_to_usage in candidate['block_map'].iteritems():
                 for category, usage_id in cat_to_usage.iteritems():
-                    if usage_id == locator.usage_id:
+                    if get_course:
+                        if category == 'course':
+                            return Location(
+                                'i4x',
+                                candidate['_id']['org'],
+                                candidate['_id']['course'],
+                                'course',
+                                self._decode_from_mongo(old_name),
+                                None)
+                    elif usage_id == locator.usage_id:
                         # figure out revision
                         # enforce the draft only if category in [..] logic
                         if category in draft.DIRECT_ONLY_CATEGORIES:

common/lib/xmodule/xmodule/modulestore/locator.py

@@ -263,7 +263,7 @@ class CourseLocator(Locator):
                              version_guid=self.version_guid,
                              branch=self.branch)
 
-    def url_reverse(self, prefix, postfix):
+    def url_reverse(self, prefix, postfix=''):
         """
         Do what reverse is supposed to do but seems unable to do. Generate a url using prefix unicode(self) postfix
         :param prefix: the beginning of the url (will be forced to begin and end with / if non-empty)

common/lib/xmodule/xmodule/modulestore/mongo/draft.py

@@ -81,7 +81,7 @@ class DraftModuleStore(MongoModuleStore):
         try:
             return wrap_draft(super(DraftModuleStore, self).get_item(as_draft(location), depth=depth))
         except ItemNotFoundError:
-            return wrap_draft(super(DraftModuleStore, self).get_item(location, depth=depth))
+            return wrap_draft(super(DraftModuleStore, self).get_item(as_published(location), depth=depth))
 
     def get_instance(self, course_id, location, depth=0):
         """
@@ -169,7 +169,7 @@ class DraftModuleStore(MongoModuleStore):
         try:
             draft_item = self.get_item(location)
             if not getattr(draft_item, 'is_draft', False):
-                self.convert_to_draft(location)
+                self.convert_to_draft(as_published(location))
         except ItemNotFoundError, e:
             if not allow_not_found:
                 raise e
@@ -187,7 +187,7 @@ class DraftModuleStore(MongoModuleStore):
         draft_loc = as_draft(location)
         draft_item = self.get_item(location)
         if not getattr(draft_item, 'is_draft', False):
-            self.convert_to_draft(location)
+            self.convert_to_draft(as_published(location))
 
         return super(DraftModuleStore, self).update_children(draft_loc, children)
 
@@ -203,7 +203,7 @@ class DraftModuleStore(MongoModuleStore):
         draft_item = self.get_item(location)
 
         if not getattr(draft_item, 'is_draft', False):
-            self.convert_to_draft(location)
+            self.convert_to_draft(as_published(location))
 
         if 'is_draft' in metadata:
             del metadata['is_draft']
@@ -262,7 +262,7 @@ class DraftModuleStore(MongoModuleStore):
         """
         Turn the published version into a draft, removing the published version
         """
-        self.convert_to_draft(location)
+        self.convert_to_draft(as_published(location))
         super(DraftModuleStore, self).delete_item(location)
 
     def _query_children_for_cache_children(self, items):

common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py

@@ -258,13 +258,17 @@ class TestLocationMapper(unittest.TestCase):
             new_style_course_id,
             block_map={
                 'abc123': {'problem': 'problem2'},
-                '48f23a10395384929234': {'chapter': 'chapter48f'}
+                '48f23a10395384929234': {'chapter': 'chapter48f'},
+                'baz_run': {'course': 'root'},
             }
         )
         # only one course matches
         prob_location = loc_mapper().translate_locator_to_location(prob_locator)
         # default branch
         self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', None))
+        # test get_course keyword
+        prob_location = loc_mapper().translate_locator_to_location(prob_locator, get_course=True)
+        self.assertEqual(prob_location, Location('i4x', org, course, 'course', 'baz_run', None))
         # explicit branch
         prob_locator = BlockUsageLocator(
             course_id=prob_locator.course_id, branch='draft', usage_id=prob_locator.usage_id

common/lib/xmodule/xmodule/modulestore/xml_exporter.py

@@ -97,15 +97,16 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
         if len(draft_verticals) > 0:
             draft_course_dir = export_fs.makeopendir('drafts')
             for draft_vertical in draft_verticals:
-                parent_locs = draft_modulestore.get_parent_locations(draft_vertical.location, course.location.course_id)
-                # Don't try to export orphaned items.
-                if len(parent_locs) > 0:
-                    logging.debug('parent_locs = {0}'.format(parent_locs))
-                    draft_vertical.xml_attributes['parent_sequential_url'] = Location(parent_locs[0]).url()
-                    sequential = modulestore.get_item(Location(parent_locs[0]))
-                    index = sequential.children.index(draft_vertical.location.url())
-                    draft_vertical.xml_attributes['index_in_children_list'] = str(index)
-                    draft_vertical.export_to_xml(draft_course_dir)
+                if getattr(draft_vertical, 'is_draft', False):
+                    parent_locs = draft_modulestore.get_parent_locations(draft_vertical.location, course.location.course_id)
+                    # Don't try to export orphaned items.
+                    if len(parent_locs) > 0:
+                        logging.debug('parent_locs = {0}'.format(parent_locs))
+                        draft_vertical.xml_attributes['parent_sequential_url'] = Location(parent_locs[0]).url()
+                        sequential = modulestore.get_item(Location(parent_locs[0]))
+                        index = sequential.children.index(draft_vertical.location.url())
+                        draft_vertical.xml_attributes['index_in_children_list'] = str(index)
+                        draft_vertical.export_to_xml(draft_course_dir)
 
 
 def export_extra_content(export_fs, modulestore, course_id, course_location, category_type, dirname, file_suffix=''):