Merge pull request #12293 from edx/revert-12144-SUST-35-course_key_from_string_or_404

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

cms/djangoapps/contentstore/views/course.py

@@ -91,7 +91,7 @@ from util.organizations_helpers import (
     organizations_enabled,
 )
 from util.string_utils import _has_non_ascii_characters
-from util.course_key_utils import course_key_from_string_or_404
+from util.course_key_utils import from_string_or_404
 from xmodule.contentstore.content import StaticContent
 from xmodule.course_module import CourseFields
 from xmodule.course_module import DEFAULT_START_DATE
@@ -875,7 +875,7 @@ def course_info_handler(request, course_key_string):
     GET
         html: return html for editing the course info handouts and updates.
     """
-    course_key = course_key_from_string_or_404(course_key_string)
+    course_key = from_string_or_404(course_key_string)
 
     with modulestore().bulk_operations(course_key):
         course_module = get_course_and_check_access(course_key, request.user)

cms/djangoapps/contentstore/views/tabs.py

@@ -14,8 +14,7 @@ from edxmako.shortcuts import render_to_response
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.tabs import CourseTabList, CourseTab, InvalidTabsException, StaticTab
-from opaque_keys.edx.keys import UsageKey
-from util.course_key_utils import course_key_from_string_or_404
+from opaque_keys.edx.keys import CourseKey, UsageKey
 
 from ..utils import get_lms_link_for_item
 
@@ -40,7 +39,7 @@ def tabs_handler(request, course_key_string):
     Creating a tab, deleting a tab, or changing its contents is not supported through this method.
     Instead use the general xblock URL (see item.xblock_handler).
     """
-    course_key = course_key_from_string_or_404(course_key_string)
+    course_key = CourseKey.from_string(course_key_string)
     if not has_course_author_access(request.user, course_key):
         raise PermissionDenied()
 

cms/djangoapps/contentstore/views/tests/test_tabs.py

@@ -7,12 +7,9 @@ from contentstore.tests.utils import CourseTestCase
 from contentstore.utils import reverse_course_url
 from xmodule.x_module import STUDENT_VIEW
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
-from django.test.client import RequestFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.tabs import CourseTabList
 from xmodule.modulestore.django import modulestore
-from django.http import Http404
-from contentstore.views.tabs import tabs_handler
 
 
 class TabsPageTests(CourseTestCase):
@@ -194,14 +191,6 @@ class TabsPageTests(CourseTestCase):
         self.assertIn('<span class="sr">Delete this component</span>', html)
         self.assertIn('<span data-tooltip="Drag to reorder" class="drag-handle action"></span>', html)
 
-    def test_invalid_course_id(self):
-        """ Asserts that Http404 is raised when the course id is not valid. """
-        request_factory = RequestFactory()
-        request = request_factory.get('/dummy-url')
-        request.user = self.user
-        with self.assertRaises(Http404):
-            tabs_handler(request, "/some.invalid.key/course-v1:TTT+CS01+2015_T0")
-
 
 class PrimitiveTabEdit(ModuleStoreTestCase):
     """Tests for the primitive tab edit data manipulations"""

cms/djangoapps/contentstore/views/tests/test_user.py

@@ -9,9 +9,6 @@ from django.contrib.auth.models import User
 from student.models import CourseEnrollment
 from student.roles import CourseStaffRole, CourseInstructorRole
 from student import auth
-from django.http import Http404
-from contentstore.views.user import course_team_handler
-from django.test.client import RequestFactory
 
 
 class UsersTestCase(CourseTestCase):
@@ -318,11 +315,3 @@ class UsersTestCase(CourseTestCase):
             CourseEnrollment.is_enrolled(self.ext_user, self.course.id),
             'User ext_user should have been enrolled in the course'
         )
-
-    def test_invalid_course_id(self):
-        """ Asserts that Http404 is raised when the course id is not valid. """
-        request_factory = RequestFactory()
-        request = request_factory.get('/dummy-url')
-        request.user = self.user
-        with self.assertRaises(Http404):
-            course_team_handler(request, "/some.invalid.key/course-v1:TTT+CS01+2015_T0")

cms/djangoapps/contentstore/views/user.py

@@ -8,7 +8,7 @@ from django.views.decorators.csrf import ensure_csrf_cookie
 from edxmako.shortcuts import render_to_response
 
 from xmodule.modulestore.django import modulestore
-from util.course_key_utils import course_key_from_string_or_404
+from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locator import LibraryLocator
 from util.json_request import JsonResponse, expect_json
 from student.roles import CourseInstructorRole, CourseStaffRole, LibraryUserRole
@@ -49,7 +49,7 @@ def course_team_handler(request, course_key_string=None, email=None):
     DELETE:
         json: remove a particular course team member from the course team (email is required).
     """
-    course_key = course_key_from_string_or_404(course_key_string) if course_key_string else None
+    course_key = CourseKey.from_string(course_key_string) if course_key_string else None
     # No permissions check here - each helper method does its own check.
 
     if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'):

common/djangoapps/util/course_key_utils.py

@@ -6,15 +6,14 @@ from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
 
 
-def course_key_from_string_or_404(course_key_string, message=None):
+def from_string_or_404(course_key_string):
     """
     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 contains the course key
-        message(str): It contains the exception message
+        course_key_string(str): It is string containing the course key
 
     Returns:
         CourseKey: A key that uniquely identifies a course
@@ -26,6 +25,6 @@ def course_key_from_string_or_404(course_key_string, message=None):
     try:
         course_key = CourseKey.from_string(course_key_string)
     except InvalidKeyError:
-        raise Http404(message)
+        raise Http404
 
     return course_key

common/djangoapps/util/tests/test_course_key_utils.py

 """
 Tests for util.course_key_utils
 """
-import ddt
-import unittest
-from util.course_key_utils import course_key_from_string_or_404
+from nose.tools import assert_equals, assert_raises  # pylint: disable=no-name-in-module
+from util.course_key_utils import from_string_or_404
 from opaque_keys.edx.keys import CourseKey
 from django.http import Http404
 
 
-@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)
-        )
+def test_from_string_or_404():
 
-    @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
+    #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")
     )
-    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,
-        )
 
-    @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
+    #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")
     )
-    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")

lms/djangoapps/django_comment_client/forum/tests.py

@@ -1312,15 +1312,6 @@ class InlineDiscussionUnicodeTestCase(SharedModuleStoreTestCase, UnicodeTestMixi
         self.assertEqual(response_data["discussion_data"][0]["title"], text)
         self.assertEqual(response_data["discussion_data"][0]["body"], text)
 
-    def _test_invalid_course_id(self):
-        """ Asserts that Http404 is raised when the course id is not valid. """
-        request = RequestFactory().get("dummy_url")
-        request.user = self.student
-        with self.assertRaises(Http404):
-            views.inline_discussion(
-                request, "/some.invalid.key/course-v1:TTT+CS01+2015_T0", self.course.discussion_topics['General']['id']
-            )
-
 
 class ForumFormDiscussionUnicodeTestCase(SharedModuleStoreTestCase, UnicodeTestMixin):
     @classmethod

lms/djangoapps/django_comment_client/forum/views.py

@@ -40,7 +40,7 @@ from django_comment_client.utils import (
 import django_comment_client.utils as utils
 import lms.lib.comment_client as cc
 
-from util.course_key_utils import course_key_from_string_or_404
+from opaque_keys.edx.keys import CourseKey
 
 THREADS_PER_PAGE = 20
 INLINE_THREADS_PER_PAGE = 20
@@ -178,7 +178,7 @@ def use_bulk_ops(view_func):
     """
     @wraps(view_func)
     def wrapped_view(request, course_id, *args, **kwargs):  # pylint: disable=missing-docstring
-        course_key = course_key_from_string_or_404(course_id)
+        course_key = CourseKey.from_string(course_id)
         with modulestore().bulk_operations(course_key):
             return view_func(request, course_key, *args, **kwargs)
     return wrapped_view