Merge pull request #9202 from edx/waheed/plat776-fix-course-id-regex

Fixed course_id pattern regex.

common/djangoapps/track/tests/test_contexts.py

 # pylint: disable=missing-docstring
 
+import ddt
 from unittest import TestCase
 
 from track import contexts
 
 
+@ddt.ddt
 class TestContexts(TestCase):
 
     COURSE_ID = 'test/course_name/course_run'
+    SPLIT_COURSE_ID = 'course-v1:test+course_name+course_run'
     ORG_ID = 'test'
 
-    def test_course_id_from_url(self):
-        self.assert_parses_course_id_from_url('http://foo.bar.com/courses/{course_id}/more/stuff')
+    @ddt.data(
+        (COURSE_ID, ''),
+        (COURSE_ID, '/more/stuff'),
+        (COURSE_ID, '?format=json'),
+        (SPLIT_COURSE_ID, ''),
+        (SPLIT_COURSE_ID, '/more/stuff'),
+        (SPLIT_COURSE_ID, '?format=json')
+    )
+    @ddt.unpack
+    def test_course_id_from_url(self, course_id, postfix):
+        url = 'http://foo.bar.com/courses/{}{}'.format(course_id, postfix)
+        self.assert_parses_course_id_from_url(url, course_id)
 
-    def assert_parses_course_id_from_url(self, format_string):
+    def assert_parses_course_id_from_url(self, format_string, course_id):
         self.assertEquals(
-            contexts.course_context_from_url(format_string.format(course_id=self.COURSE_ID)),
+            contexts.course_context_from_url(format_string.format(course_id=course_id)),
             {
-                'course_id': self.COURSE_ID,
+                'course_id': course_id,
                 'org_id': self.ORG_ID
             }
         )
@@ -34,11 +47,22 @@ class TestContexts(TestCase):
             }
         )
 
-    def test_malformed_course_id(self):
-        self.assert_empty_context_for_url('http://foo.bar.com/courses/test')
+    @ddt.data('', '/', '/?', '?format=json')
+    def test_malformed_course_id(self, postfix):
+        self.assert_empty_context_for_url('http://foo.bar.com/courses/test/course_name{}'.format(postfix))
 
-    def test_course_id_later_in_url(self):
-        self.assert_parses_course_id_from_url('http://foo.bar.com/x/y/z/courses/{course_id}')
+    @ddt.data(
+        (COURSE_ID, ''),
+        (COURSE_ID, '/more/stuff'),
+        (COURSE_ID, '?format=json'),
+        (SPLIT_COURSE_ID, ''),
+        (SPLIT_COURSE_ID, '/more/stuff'),
+        (SPLIT_COURSE_ID, '?format=json')
+    )
+    @ddt.unpack
+    def test_course_id_later_in_url(self, course_id, postfix):
+        url = 'http://foo.bar.com/x/y/z/courses/{}{}'.format(course_id, postfix)
+        self.assert_parses_course_id_from_url(url, course_id)
 
     def test_no_url(self):
         self.assert_empty_context_for_url(None)

lms/envs/common.py

@@ -593,7 +593,7 @@ LMS_MIGRATION_ALLOWED_IPS = []
 # Note: these intentionally greedily grab all chars up to the next slash including any pluses
 # DHM: I really wanted to ensure the separators were the same (+ or /) but all patts I tried had
 # too many inadvertent side effects :-(
-COURSE_KEY_PATTERN = r'(?P<course_key_string>[^/+]+(/|\+)[^/+]+(/|\+)[^/]+)'
+COURSE_KEY_PATTERN = r'(?P<course_key_string>[^/+]+(/|\+)[^/+]+(/|\+)[^/?]+)'
 COURSE_ID_PATTERN = COURSE_KEY_PATTERN.replace('course_key_string', 'course_id')
 COURSE_KEY_REGEX = COURSE_KEY_PATTERN.replace('P<course_key_string>', ':')