Merge pull request #12144 from Ayub-Khan/SUST-35-course_key_from_string_or_404

SUST-35 Implementation of course_key_from_string_or_404 to return course_key or raise a 404

common/djangoapps/util/course_key_utils.py

@@ -6,14 +6,15 @@ from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
 
 
-def from_string_or_404(course_key_string):
+def course_key_from_string_or_404(course_key_string, message=None):
     """
     Gets CourseKey from the string passed as parameter.
 
     Parses course key from string(containing course key) or raises 404 if the string's format is invalid.
 
     Arguments:
-        course_key_string(str): It is string containing the course key
+        course_key_string(str): It contains the course key
+        message(str): It contains the exception message
 
     Returns:
         CourseKey: A key that uniquely identifies a course
@@ -25,6 +26,6 @@ def from_string_or_404(course_key_string):
     try:
         course_key = CourseKey.from_string(course_key_string)
     except InvalidKeyError:
-        raise Http404
+        raise Http404(message)
 
     return course_key

common/djangoapps/util/tests/test_course_key_utils.py

@@ -1,32 +1,54 @@
 """
 Tests for util.course_key_utils
 """
-from nose.tools import assert_equals, assert_raises  # pylint: disable=no-name-in-module
-from util.course_key_utils import from_string_or_404
+import ddt
+import unittest
+from util.course_key_utils import course_key_from_string_or_404
 from opaque_keys.edx.keys import CourseKey
 from django.http import Http404
 
 
-def test_from_string_or_404():
+@ddt.ddt
+class TestFromStringOr404(unittest.TestCase):
+    """
+    Base Test class for course_key_from_string_or_404 utility tests
+    """
+    @ddt.data(
+        "course-v1:TTT+CS01+2015_T0",  # split style course keys
+        "TTT/CS01/2015_T0"  # mongo style course keys
+    )
+    def test_from_string_or_404_for_valid_course_key(self, valid_course_key):
+        """
+        Tests course_key_from_string_or_404 for valid split style course keys and mongo style course keys.
+        """
+        self.assertEquals(
+            CourseKey.from_string(valid_course_key),
+            course_key_from_string_or_404(valid_course_key)
+        )
 
-    #testing with split style course keys
-    assert_raises(
-        Http404,
-        from_string_or_404,
-        "/some.invalid.key/course-v1:TTT+CS01+2015_T0"
-    )
-    assert_equals(
-        CourseKey.from_string("course-v1:TTT+CS01+2015_T0"),
-        from_string_or_404("course-v1:TTT+CS01+2015_T0")
+    @ddt.data(
+        "/some.invalid.key/course-v1:TTT+CS01+2015_T0",  # split style course keys
+        "/some.invalid.key/TTT/CS01/2015_T0"  # mongo style course keys
     )
+    def test_from_string_or_404_for_invalid_course_key(self, invalid_course_key):
+        """
+        Tests course_key_from_string_or_404 for valid split style course keys and mongo style course keys.
+        """
+        self.assertRaises(
+            Http404,
+            course_key_from_string_or_404,
+            invalid_course_key,
+        )
 
-    #testing with mongo style course keys
-    assert_raises(
-        Http404,
-        from_string_or_404,
-        "/some.invalid.key/TTT/CS01/2015_T0"
-    )
-    assert_equals(
-        CourseKey.from_string("TTT/CS01/2015_T0"),
-        from_string_or_404("TTT/CS01/2015_T0")
+    @ddt.data(
+        "/some.invalid.key/course-v1:TTT+CS01+2015_T0",  # split style invalid course key
+        "/some.invalid.key/TTT/CS01/2015_T0"  # mongo style invalid course key
     )
+    def test_from_string_or_404_with_message(self, course_string):
+        """
+        Tests course_key_from_string_or_404 with exception message for split style and monog style invalid course keys.
+        :return:
+        """
+        with self.assertRaises(Http404) as context:
+            course_key_from_string_or_404(course_string, message="Invalid Keys")
+        self.assertEquals(str(context.exception), "Invalid Keys")