Update handler_url support to match the latest in the xblock repo, for both js and python runtimes

[LMS-1613]

lms/lib/xblock/runtime.py

@@ -82,12 +82,22 @@ def handler_url(course_id, block, handler, suffix='', query='', thirdparty=False
     if thirdparty:
         view_name = 'xblock_handler_noauth'
 
-    return reverse(view_name, kwargs={
+    url = reverse(view_name, kwargs={
         'course_id': course_id,
         'usage_id': quote_slashes(str(block.scope_ids.usage_id)),
         'handler': handler,
         'suffix': suffix,
-    }) + '?' + query
+    })
+
+    # If suffix is an empty string, remove the trailing '/'
+    if not suffix:
+        url = url.rstrip('/')
+
+    # If there is a query string, append it
+    if query:
+        url += '?' + query
+
+    return url
 
 
 def handler_prefix(course_id, block):
@@ -96,9 +106,13 @@ def handler_prefix(course_id, block):
 
     The prefix is a valid handler url after the handler name is slash-appended
     to it.
-
     """
-    return handler_url(course_id, block, '').rstrip('/')
+    # This depends on handler url having the handler_name as the final piece of the url
+    # so that leaving an empty handler_name really does leave the opportunity to append
+    # the handler_name on the frontend
+
+    # This is relied on by the xblock/runtime.v1.coffee frontend handlerUrl function
+    return handler_url(course_id, block, '').rstrip('/?')
 
 
 class LmsHandlerUrls(object):

lms/lib/xblock/test/test_runtime.py

@@ -3,8 +3,10 @@ Tests of the LMS XBlock Runtime and associated utilities
 """
 
 from ddt import ddt, data
+from mock import Mock
 from unittest import TestCase
-from lms.lib.xblock.runtime import quote_slashes, unquote_slashes
+from urlparse import urlparse
+from lms.lib.xblock.runtime import quote_slashes, unquote_slashes, handler_url
 
 TEST_STRINGS = [
     '',
@@ -31,3 +33,46 @@ class TestQuoteSlashes(TestCase):
     @data(*TEST_STRINGS)
     def test_escaped(self, test_string):
         self.assertNotIn('/', quote_slashes(test_string))
+
+
+class TestHandlerUrl(TestCase):
+    """Test the LMS handler_url"""
+
+    def setUp(self):
+        self.block = Mock()
+        self.course_id = "org/course/run"
+
+    def test_trailing_characters(self):
+        self.assertFalse(handler_url(self.course_id, self.block, 'handler').endswith('?'))
+        self.assertFalse(handler_url(self.course_id, self.block, 'handler').endswith('/'))
+
+        self.assertFalse(handler_url(self.course_id, self.block, 'handler', 'suffix').endswith('?'))
+        self.assertFalse(handler_url(self.course_id, self.block, 'handler', 'suffix').endswith('/'))
+
+        self.assertFalse(handler_url(self.course_id, self.block, 'handler', 'suffix', 'query').endswith('?'))
+        self.assertFalse(handler_url(self.course_id, self.block, 'handler', 'suffix', 'query').endswith('/'))
+
+        self.assertFalse(handler_url(self.course_id, self.block, 'handler', query='query').endswith('?'))
+        self.assertFalse(handler_url(self.course_id, self.block, 'handler', query='query').endswith('/'))
+
+    def _parsed_query(self, query_string):
+        """Return the parsed query string from a handler_url generated with the supplied query_string"""
+        return urlparse(handler_url(self.course_id, self.block, 'handler', query=query_string)).query
+
+    def test_query_string(self):
+        self.assertIn('foo=bar', self._parsed_query('foo=bar'))
+        self.assertIn('foo=bar&baz=true', self._parsed_query('foo=bar&baz=true'))
+        self.assertIn('foo&bar&baz', self._parsed_query('foo&bar&baz'))
+
+    def _parsed_path(self, handler_name='handler', suffix=''):
+        """Return the parsed path from a handler_url with the supplied handler_name and suffix"""
+        return urlparse(handler_url(self.course_id, self.block, handler_name, suffix=suffix)).path
+
+    def test_suffix(self):
+        self.assertTrue(self._parsed_path(suffix="foo").endswith('foo'))
+        self.assertTrue(self._parsed_path(suffix="foo/bar").endswith('foo/bar'))
+        self.assertTrue(self._parsed_path(suffix="/foo/bar").endswith('/foo/bar'))
+
+    def test_handler_name(self):
+        self.assertIn('handler1', self._parsed_path('handler1'))
+        self.assertIn('handler_a', self._parsed_path('handler_a'))