diff --git a/common/static/coffee/spec/xblock/runtime.v1_spec.coffee b/common/static/coffee/spec/xblock/runtime.v1_spec.coffee
index 266bc7df0eed8e93578332242f01f6f063cef59a..e9c71e9b32fe34d07718eac6cd01051a6f2f043e 100644
--- a/common/static/coffee/spec/xblock/runtime.v1_spec.coffee
+++ b/common/static/coffee/spec/xblock/runtime.v1_spec.coffee
@@ -12,7 +12,7 @@ describe "XBlock.runtime.v1", ->
@runtime = XBlock.runtime.v1(@element, @children)
it "provides a handler url", ->
- expect(@runtime.handlerUrl('foo')).toBe('/xblock/fake-usage-id/handler/foo')
+ expect(@runtime.handlerUrl(@element, 'foo')).toBe('/xblock/fake-usage-id/handler/foo')
it "provides a list of children", ->
expect(@runtime.children).toBe(@children)
diff --git a/common/static/coffee/src/xblock/runtime.v1.coffee b/common/static/coffee/src/xblock/runtime.v1.coffee
index ef05c064722b067c82e0ef4d067dee186fd5acfe..efb9944bd20edac156b5f0f955163de419067900 100644
--- a/common/static/coffee/src/xblock/runtime.v1.coffee
+++ b/common/static/coffee/src/xblock/runtime.v1.coffee
@@ -4,9 +4,21 @@
childMap[child.name] = child
return {
- handlerUrl: (handlerName) ->
+ # Generate the handler url for the specified handler.
+ #
+ # element is the html element containing the xblock requesting the url
+ # handlerName is the name of the handler
+ # suffix is the optional url suffix to include in the handler url
+ # query is an optional query-string (note, this should not include a preceding ? or &)
+ handlerUrl: (element, handlerName, suffix, query) ->
handlerPrefix = $(element).data("handler-prefix")
- "#{handlerPrefix}/#{handlerName}"
+ suffix = if suffix? then "/#{suffix}" else ''
+ query = if query? then "?#{query}" else ''
+ "#{handlerPrefix}/#{handlerName}#{suffix}#{query}"
+
+ # A list of xblock children of this element
children: children
+
+ # A map of name -> child for the xblock children of this element
childMap: childMap
}
diff --git a/lms/lib/xblock/runtime.py b/lms/lib/xblock/runtime.py
index 4fa48be83a83dd074920406a5ef39f6394abf4f1..cb2c0330251b1d4f0fa2512514ac6f8955064a93 100644
--- a/lms/lib/xblock/runtime.py
+++ b/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):
diff --git a/lms/lib/xblock/test/test_runtime.py b/lms/lib/xblock/test/test_runtime.py
index c8c4d006f791a296571e48d8976927015fac9fe2..5b3efc171c05d96d97a6347c198a4e31824308fd 100644
--- a/lms/lib/xblock/test/test_runtime.py
+++ b/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'))