Use new restful pattern instead

cms/djangoapps/contentstore/tests/test_orphan.py

@@ -3,7 +3,7 @@ Test finding orphans via the view and django config
 """
 import json
 from contentstore.tests.utils import CourseTestCase
-from xmodule.modulestore.django import editable_modulestore
+from xmodule.modulestore.django import editable_modulestore, loc_mapper
 from django.core.urlresolvers import reverse
 from student.models import CourseEnrollment
 
@@ -41,12 +41,12 @@ class TestOrphan(CourseTestCase):
         """
         Test that old mongo finds the orphans
         """
+        locator = loc_mapper().translate_location(self.course.location.course_id, self.course.location, False, True)
+        orphan_url = locator.url_reverse('orphan/', '')
+
         orphans = json.loads(
             self.client.get(
-                reverse(
-                    'orphan',
-                    kwargs={'course_id': '{}.{}'.format(self.course.location.org, self.course.location.course)}
-                ),
+                orphan_url,
                 HTTP_ACCEPT='application/json'
             ).content
         )
@@ -62,13 +62,11 @@ class TestOrphan(CourseTestCase):
         """
         Test that old mongo deletes the orphans
         """
-        url = reverse(
-            'orphan',
-            kwargs={'course_id': '{}.{}'.format(self.course.location.org, self.course.location.course)}
-        )
-        self.client.delete(url)
+        locator = loc_mapper().translate_location(self.course.location.course_id, self.course.location, False, True)
+        orphan_url = locator.url_reverse('orphan/', '')
+        self.client.delete(orphan_url)
         orphans = json.loads(
-            self.client.get(url, HTTP_ACCEPT='application/json').content
+            self.client.get(orphan_url, HTTP_ACCEPT='application/json').content
         )
         self.assertEqual(len(orphans), 0, "Orphans not deleted {}".format(orphans))
 
@@ -78,11 +76,9 @@ class TestOrphan(CourseTestCase):
         """
         test_user_client, test_user = self.createNonStaffAuthedUserClient()
         CourseEnrollment.enroll(test_user, self.course.location.course_id)
-        url = reverse(
-            'orphan',
-            kwargs={'course_id': '{}.{}'.format(self.course.location.org, self.course.location.course)}
-        )
-        response = test_user_client.get(url)
+        locator = loc_mapper().translate_location(self.course.location.course_id, self.course.location, False, True)
+        orphan_url = locator.url_reverse('orphan/', '')
+        response = test_user_client.get(orphan_url)
         self.assertEqual(response.status_code, 403)
-        response = test_user_client.delete(url)
+        response = test_user_client.delete(orphan_url)
         self.assertEqual(response.status_code, 403)

cms/djangoapps/contentstore/views/item.py

@@ -21,7 +21,7 @@ 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
+from xmodule.modulestore.locator import CourseLocator, BlockUsageLocator
 from student.models import CourseEnrollment
 
 __all__ = ['save_item', 'create_item', 'delete_item', 'orphan']
@@ -204,13 +204,13 @@ def delete_item(request):
 
     return JsonResponse()
 
-
+# pylint: disable=W0613
 @login_required
 @require_http_methods(("GET", "DELETE"))
-def orphan(request, course_id):
+def orphan(request, tag=None, course_id=None, branch=None, version_guid=None, block=None):
     """
-    View for handling orphan related requests. A get gets all of the current orphans.
-    DELETE, PUT and POST are meaningless for now.
+    View for handling orphan related requests. GET gets all of the current orphans.
+    DELETE removes all orphans (requires is_staff access)
 
     An orphan is a block whose category is not in the DETACHED_CATEGORY list, is not the root, and is not reachable
     from the root via children
@@ -218,15 +218,17 @@ def orphan(request, course_id):
     :param request:
     :param course_id: Locator syntax course_id
     """
-    course_loc = CourseLocator(course_id=course_id)
+    location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block)
+    # DHM: when split becomes back-end, move or conditionalize this conversion
+    old_location = loc_mapper().translate_locator_to_location(location)
     if request.method == 'GET':
-        if has_access(request.user, course_loc):
-            return JsonResponse(modulestore().get_orphans(course_id, DETACHED_CATEGORIES, 'draft'))
+        if has_access(request.user, old_location):
+            return JsonResponse(modulestore().get_orphans(old_location, DETACHED_CATEGORIES, 'draft'))
         else:
             raise PermissionDenied()
     if request.method == 'DELETE':
         if request.user.is_staff:
-            items = modulestore().get_orphans(course_id, DETACHED_CATEGORIES, 'draft')
+            items = modulestore().get_orphans(old_location, DETACHED_CATEGORIES, 'draft')
             for item in items:
                 modulestore('draft').delete_item(item, True)
             return JsonResponse({'deleted': items})

cms/urls.py

@@ -131,7 +131,6 @@ urlpatterns += patterns(
 
     url(r'^logout$', 'student.views.logout_user', name='logout'),
 
-    url(r'^(?P<course_id>[^/]+)/orphan', 'contentstore.views.orphan', name='orphan')
 )
 
 # restful api
@@ -142,6 +141,7 @@ urlpatterns += patterns(
     # (?ix) == ignore case and verbose (multiline regex)
     url(r'(?ix)^course/{}$'.format(parsers.URL_RE_SOURCE), 'course_handler'),
     url(r'(?ix)^checklists/{}(/)?(?P<checklist_index>\d+)?$'.format(parsers.URL_RE_SOURCE), 'checklists_handler'),
+    url(r'(?ix)^orphan/{}$'.format(parsers.URL_RE_SOURCE), 'orphan')
 )
 
 js_info_dict = {

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

@@ -855,27 +855,13 @@ class MongoModuleStore(ModuleStoreBase):
         """
         return MONGO_MODULESTORE_TYPE
 
-    COURSE_ID_RE = re.compile(r'(?P<org>[^.]+)\.(?P<course_id>.+)')
-    def parse_course_id(self, course_id):
+    def get_orphans(self, course_location, detached_categories, _branch):
         """
-        Parse a Locator style course_id into a dict w/ the org and course_id
-        :param course_id: a string looking like 'org.course.id.part'
+        Return an array all of the locations for orphans in the course.
         """
-        match = self.COURSE_ID_RE.match(course_id)
-        if match is None:
-            raise ValueError(course_id)
-        return match.groupdict()
-
-    def get_orphans(self, course_id, detached_categories, _branch):
-        """
-        Return a dict of all of the orphans in the course.
-
-        :param course_id:
-        """
-        locator_dict = self.parse_course_id(course_id)
         all_items = self.collection.find({
-            '_id.org': locator_dict['org'],
-            '_id.course': locator_dict['course_id'],
+            '_id.org': course_location.org,
+            '_id.course': course_location.course,
             '_id.category': {'$nin': detached_categories}
         })
         all_reachable = set()

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

@@ -23,12 +23,12 @@ class TestOrphan(unittest.TestCase):
         'db': 'test_xmodule',
     }
 
-    modulestore_options = dict({
+    modulestore_options = {
         'default_class': 'xmodule.raw_module.RawDescriptor',
         'fs_root': '',
         'render_template': mock.Mock(return_value=""),
         'xblock_mixins': (InheritanceMixin,)
-    })
+    }
 
     split_course_id = 'test_org.test_course.runid'
 
@@ -41,25 +41,35 @@ class TestOrphan(unittest.TestCase):
             self.db_config,
             **self.modulestore_options
         )
-        self.addCleanup(self.tearDownSplit)
+        self.addCleanup(self.tear_down_split)
         self.old_mongo = MongoModuleStore(self.db_config, **self.modulestore_options)
-        self.addCleanup(self.tearDownMongo)
+        self.addCleanup(self.tear_down_mongo)
         self.course_location = None
         self._create_course()
 
-    def tearDownSplit(self):
+    def tear_down_split(self):
+        """
+        Remove the test collections, close the db connection
+        """
         split_db = self.split_mongo.db
         split_db.drop_collection(split_db.course_index)
         split_db.drop_collection(split_db.structures)
         split_db.drop_collection(split_db.definitions)
         split_db.connection.close()
 
-    def tearDownMongo(self):
+    def tear_down_mongo(self):
+        """
+        Remove the test collections, close the db connection
+        """
         split_db = self.split_mongo.db
         # old_mongo doesn't give a db attr, but all of the dbs are the same
         split_db.drop_collection(self.old_mongo.collection)
 
     def _create_item(self, category, name, data, metadata, parent_category, parent_name, runtime):
+        """
+        Create the item of the given category and block id in split and old mongo, add it to the optional
+        parent. The parent category is only needed because old mongo requires it for the id.
+        """
         location = Location('i4x', 'test_org', 'test_course', category, name)
         self.old_mongo.create_and_save_xmodule(location, data, metadata, runtime)
         if isinstance(data, basestring):
@@ -125,7 +135,7 @@ class TestOrphan(unittest.TestCase):
         """
         Test that old mongo finds the orphans
         """
-        orphans = self.old_mongo.get_orphans('test_org.test_course', ['static_tab', 'about', 'course_info'], None)
+        orphans = self.old_mongo.get_orphans(self.course_location, ['static_tab', 'about', 'course_info'], None)
         self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans))
         location = self.course_location.replace(category='chapter', name='OrphanChapter')
         self.assertIn(location.url(), orphans)