Have urls.py fully parse locator url

cms/djangoapps/contentstore/tests/test_course_index.py

@@ -61,7 +61,7 @@ class TestCourseIndex(CourseTestCase):
         Test the error conditions for the access
         """
         locator = loc_mapper().translate_location(self.course.location.course_id, self.course.location, False, True)
-        outline_url = reverse('contentstore.views.course_handler', kwargs={'course_url': unicode(locator)})
+        outline_url = locator.url_reverse('course/', '')
         # register a non-staff member and try to delete the course branch
         non_staff_client, _ = self.createNonStaffAuthedUserClient()
         response = non_staff_client.delete(outline_url, {}, HTTP_ACCEPT='application/json')

cms/djangoapps/contentstore/views/course.py

@@ -60,7 +60,7 @@ __all__ = ['create_new_course', 'course_info', 'course_handler',
 
 
 @login_required
-def course_handler(request, course_url):
+def course_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None):
     """
     The restful handler for course specific requests.
     It provides the course tree with the necessary information for identifying and labeling the parts. The root
@@ -84,7 +84,10 @@ def course_handler(request, course_url):
     if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'):
         if request.method == 'GET':
             raise NotImplementedError('coming soon')
-        elif not has_access(request.user, BlockUsageLocator(course_url)):
+        elif not has_access(
+            request.user,
+            BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block)
+        ):
             raise PermissionDenied()
         elif request.method == 'POST':
             raise NotImplementedError()
@@ -95,7 +98,7 @@ def course_handler(request, course_url):
         else:
             return HttpResponseBadRequest()
     elif request.method == 'GET':  # assume html
-        return course_index(request, course_url)
+        return course_index(request, course_id, branch, version_guid, block)
     else:
         return HttpResponseNotFound()
 
@@ -108,18 +111,18 @@ def old_course_index_shim(request, org, course, name):
     """
     old_location = Location(['i4x', org, course, 'course', name])
     locator = loc_mapper().translate_location(old_location.course_id, old_location, False, True)
-    return course_index(request, locator)
+    return course_index(request, locator.course_id, locator.branch, locator.version_guid, locator.usage_id)
 
 
 @login_required
 @ensure_csrf_cookie
-def course_index(request, course_url):
+def course_index(request, course_id, branch, version_guid, block):
     """
     Display an editable course overview.
 
     org, course, name: Attributes of the Location for the item to edit
     """
-    location = BlockUsageLocator(course_url)
+    location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block)
     # TODO: when converting to split backend, if location does not have a usage_id,
     # we'll need to get the course's root block_id
     if not has_access(request.user, location):

cms/djangoapps/contentstore/views/user.py

@@ -47,12 +47,13 @@ def index(request):
 
     def format_course_for_view(course):
         # published = false b/c studio manipulates draft versions not b/c the course isn't pub'd
-        course_url = loc_mapper().translate_location(
+        course_loc = loc_mapper().translate_location(
             course.location.course_id, course.location, published=False, add_entry_if_missing=True
         )
         return (
             course.display_name,
-            reverse("contentstore.views.course_handler", kwargs={'course_url': course_url}),
+            # note, couldn't get django reverse to work; so, wrote workaround
+            course_loc.url_reverse('course/', ''),
             get_lms_link_for_item(
                 course.location
             ),

cms/templates/widgets/header.html

@@ -15,10 +15,7 @@
       % if context_course:
       <% 
             ctx_loc = context_course.location
-            index_url = reverse(
-                'contentstore.views.course_handler', 
-                kwargs={'course_url': loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True)}
-            )
+            index_url = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True).url_reverse('course/', '')
       %>
       <h2 class="info-course">
         <span class="sr">${_("Current Course:")}</span>

cms/urls.py

+import re
 from django.conf import settings
 from django.conf.urls import patterns, include, url
 
 # TODO: This should be removed once the CMS is running via wsgi on all production servers
 import cms.startup as startup
+from xmodule.modulestore import parsers
 startup.run()
 
 # There is a course creators admin table.
@@ -136,7 +138,8 @@ urlpatterns += patterns(
     ),
 
     url(r'^course$', 'index'),
-    url(r'^course/(?P<course_url>.*)$', 'course_handler'),
+    # (?ix) == ignore case and verbose (multiline regex)
+    url(r'(?ix)^course/{}$'.format(parsers.URL_RE_SOURCE), 'course_handler'),
 )
 
 js_info_dict = {

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

@@ -266,8 +266,8 @@ class CourseLocator(Locator):
     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 (should begin and end with / if non-empty)
-        :param postfix: the part to append to the url (should begin w/ / if non-empty)
+        :param prefix: the beginning of the url (will be forced to begin and end with / if non-empty)
+        :param postfix: the part to append to the url (will be forced to begin w/ / if non-empty)
         """
         if prefix:
             if not prefix.endswith('/'):
@@ -278,14 +278,10 @@ class CourseLocator(Locator):
             prefix = '/'
         if postfix and not postfix.startswith('/'):
             postfix = '/' + postfix
+        elif postfix is None:
+            postfix = ''
         return prefix + unicode(self) + postfix
 
-    def reverse_kwargs(self):
-        """
-        Get the kwargs list to supply to reverse (basically, a dict of the set properties)
-        """
-        return {key: value for key, value in self.__dict__.iteritems() if value is not None}
-
     def init_from_url(self, url):
         """
         url must be a string beginning with 'edx://' and containing
@@ -452,16 +448,6 @@ class BlockUsageLocator(CourseLocator):
                                      branch=self.branch,
                                      usage_id=self.usage_id)
 
-    def reverse_kwargs(self):
-        """
-        Get the kwargs list to supply to reverse (basically, a dict of the set properties)
-        """
-        result = super(BlockUsageLocator, self).reverse_kwargs()
-        if self.usage_id:
-            del result['usage_id']
-            result['block'] = self.usage_id
-        return result
-
     def set_usage_id(self, new):
         """
         Initialize usage_id to new value.

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

@@ -285,6 +285,28 @@ class LocatorTest(TestCase):
             Locator.to_locator_or_location("hello.world.not.a.url")
         self.assertIsNone(Locator.parse_url("unknown://foo.bar/baz"))
 
+    def test_url_reverse(self):
+        """
+        Test the url_reverse method
+        """
+        locator = CourseLocator(course_id="a.fancy_course-id", branch="branch_1.2-3")
+        self.assertEqual(
+            '/expression/{}/format'.format(unicode(locator)),
+            locator.url_reverse('expression', 'format')
+        )
+        self.assertEqual(
+            '/expression/{}/format'.format(unicode(locator)),
+            locator.url_reverse('/expression', '/format')
+        )
+        self.assertEqual(
+            '/expression/{}'.format(unicode(locator)),
+            locator.url_reverse('expression/', None)
+        )
+        self.assertEqual(
+            '/expression/{}'.format(unicode(locator)),
+            locator.url_reverse('/expression/', '')
+        )
+
     def test_description_locator_url(self):
         object_id = '{:024x}'.format(random.randrange(16 ** 24))
         definition_locator = DefinitionLocator(object_id)