diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 476cab45da..4450031b62 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/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) diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 38d1c6a228..f79d4d0721 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/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() diff --git a/cms/djangoapps/contentstore/views/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py index bb8342cd7a..fde68931a3 100644 --- a/cms/djangoapps/contentstore/views/tests/test_tabs.py +++ b/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('Delete this component', html) self.assertIn('', 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""" diff --git a/cms/djangoapps/contentstore/views/tests/test_user.py b/cms/djangoapps/contentstore/views/tests/test_user.py index da82bdd00e..725858e44d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_user.py +++ b/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") diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index 79208a535d..e5da0a8194 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/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'): diff --git a/common/djangoapps/util/course_key_utils.py b/common/djangoapps/util/course_key_utils.py index d3768e300d..ab58a6558d 100644 --- a/common/djangoapps/util/course_key_utils.py +++ b/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 diff --git a/common/djangoapps/util/tests/test_course_key_utils.py b/common/djangoapps/util/tests/test_course_key_utils.py index 92025715e8..efc88a09ee 100644 --- a/common/djangoapps/util/tests/test_course_key_utils.py +++ b/common/djangoapps/util/tests/test_course_key_utils.py @@ -1,54 +1,32 @@ """ 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") diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index 790b78f858..66323a1968 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/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 diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index ba773e377c..dace54e169 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/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