From c2199279e77f106a91de41bcbade6131197b01a0 Mon Sep 17 00:00:00 2001 From: cahrens Date: Mon, 3 Feb 2014 13:07:34 -0500 Subject: [PATCH] Changes to preview to support xblocks using Locators intead of Locatins. STUD-967 --- .../contentstore/tests/test_access.py | 22 ++++++++ cms/djangoapps/contentstore/views/access.py | 6 +- cms/djangoapps/contentstore/views/preview.py | 22 +++++--- .../contentstore/views/tests/__init__.py | 0 .../contentstore/views/tests/test_preview.py | 55 +++++++++++++++++++ 5 files changed, 93 insertions(+), 12 deletions(-) create mode 100644 cms/djangoapps/contentstore/views/tests/__init__.py create mode 100644 cms/djangoapps/contentstore/views/tests/test_preview.py diff --git a/cms/djangoapps/contentstore/tests/test_access.py b/cms/djangoapps/contentstore/tests/test_access.py index 63441d60f9..1fbdbdf848 100644 --- a/cms/djangoapps/contentstore/tests/test_access.py +++ b/cms/djangoapps/contentstore/tests/test_access.py @@ -4,6 +4,7 @@ Tests access.py from django.test import TestCase from django.contrib.auth.models import User from xmodule.modulestore import Location +from xmodule.modulestore.locator import CourseLocator from student.roles import CourseInstructorRole, CourseStaffRole from student.tests.factories import AdminFactory @@ -20,6 +21,7 @@ class RolesTest(TestCase): self.instructor = User.objects.create_user('testinstructor', 'testinstructor+courses@edx.org', 'foo') self.staff = User.objects.create_user('teststaff', 'teststaff+courses@edx.org', 'foo') self.location = Location('i4x', 'mitX', '101', 'course', 'test') + self.locator = CourseLocator(url='edx://mitX.101.test') def test_get_user_role_instructor(self): """ @@ -31,6 +33,16 @@ class RolesTest(TestCase): get_user_role(self.instructor, self.location, self.location.course_id) ) + def test_get_user_role_instructor_locator(self): + """ + Verifies if user is instructor, using a CourseLocator. + """ + add_users(self.global_admin, CourseInstructorRole(self.locator), self.instructor) + self.assertEqual( + 'instructor', + get_user_role(self.instructor, self.locator) + ) + def test_get_user_role_staff(self): """ Verifies if user is staff. @@ -40,3 +52,13 @@ class RolesTest(TestCase): 'staff', get_user_role(self.staff, self.location, self.location.course_id) ) + + def test_get_user_role_staff_locator(self): + """ + Verifies if user is staff, using a CourseLocator. + """ + add_users(self.global_admin, CourseStaffRole(self.locator), self.staff) + self.assertEqual( + 'staff', + get_user_role(self.staff, self.locator) + ) diff --git a/cms/djangoapps/contentstore/views/access.py b/cms/djangoapps/contentstore/views/access.py index 9ff328a898..fb40105d42 100644 --- a/cms/djangoapps/contentstore/views/access.py +++ b/cms/djangoapps/contentstore/views/access.py @@ -22,13 +22,13 @@ def has_course_access(user, location, role=CourseStaffRole): return auth.has_access(user, role(location)) -def get_user_role(user, location, context): +def get_user_role(user, location, context=None): """ Return corresponding string if user has staff or instructor role in Studio. This will not return student role because its purpose for using in Studio. - :param location: a descriptor.location - :param context: a course_id + :param location: a descriptor.location (which may be a Location or a CourseLocator) + :param context: a course_id. This is not used if location is a CourseLocator. """ if auth.has_access(user, CourseInstructorRole(location, context)): return 'instructor' diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 53de39dcf2..28b81316ad 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -11,7 +11,8 @@ from edxmako.shortcuts import render_to_string from xmodule_modifiers import replace_static_urls, wrap_xblock from xmodule.error_module import ErrorDescriptor from xmodule.exceptions import NotFoundError, ProcessingError -from xmodule.modulestore.django import modulestore +from xmodule.modulestore.django import modulestore, loc_mapper +from xmodule.modulestore.locator import Locator from xmodule.x_module import ModuleSystem from xblock.runtime import KvsFieldData from xblock.django.request import webob_to_django_response, django_to_webob_request @@ -44,7 +45,8 @@ def preview_handler(request, usage_id, handler, suffix=''): handler: The handler to execute suffix: The remainder of the url to be passed to the handler """ - + # Note: usage_id is currently the string form of a Location, but in the + # future it will be the string representation of a Locator. location = unquote_slashes(usage_id) descriptor = modulestore().get_item(location) @@ -95,7 +97,11 @@ def _preview_module_system(request, descriptor): descriptor: An XModuleDescriptor """ - course_id = get_course_for_item(descriptor.location).location.course_id + if isinstance(descriptor.location, Locator): + course_location = loc_mapper().translate_locator_to_location(descriptor.location, get_course=True) + course_id = course_location.course_id + else: + course_id = get_course_for_item(descriptor.location).location.course_id return PreviewModuleSystem( static_url=settings.STATIC_URL, @@ -115,17 +121,15 @@ def _preview_module_system(request, descriptor): # Set up functions to modify the fragment produced by student_view wrappers=( # This wrapper wraps the module in the template specified above - partial(wrap_xblock, 'PreviewRuntime', display_name_only=descriptor.location.category == 'static_tab'), + partial(wrap_xblock, 'PreviewRuntime', display_name_only=descriptor.category == 'static_tab'), # This wrapper replaces urls in the output that start with /static # with the correct course-specific url for the static content - partial( - replace_static_urls, - getattr(descriptor, 'data_dir', descriptor.location.course), - course_id=descriptor.location.org + '/' + descriptor.location.course + '/BOGUS_RUN_REPLACE_WHEN_AVAILABLE', - ), + partial(replace_static_urls, None, course_id=course_id), ), error_descriptor_class=ErrorDescriptor, + # get_user_role accepts a location or a CourseLocator. + # If descriptor.location is a CourseLocator, course_id is unused. get_user_role=lambda: get_user_role(request.user, descriptor.location, course_id), ) diff --git a/cms/djangoapps/contentstore/views/tests/__init__.py b/cms/djangoapps/contentstore/views/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py new file mode 100644 index 0000000000..cb5b62c67f --- /dev/null +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -0,0 +1,55 @@ +""" +Tests for contentstore.views.preview.py +""" +from django.test import TestCase +from django.test.client import RequestFactory + +from student.tests.factories import UserFactory + +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.django import loc_mapper + +from contentstore.views.preview import get_preview_fragment + + +class GetPreviewHtmlTestCase(TestCase): + """ + Tests for get_preview_html. + + Note that there are other existing test cases in test_contentstore that indirectly execute + get_preview_html via the xblock RESTful API. + """ + + def test_preview_handler_locator(self): + """ + Test for calling get_preview_html when descriptor.location is a Locator. + """ + course = CourseFactory.create() + html = ItemFactory.create( + parent_location=course.location, + category="html", + data={'data': "foobar"} + ) + + locator = loc_mapper().translate_location( + course.location.course_id, html.location, True, True + ) + + # Change the stored location to a locator. + html.location = locator + html.save() + + request = RequestFactory().get('/dummy-url') + request.user = UserFactory() + request.session = {} + + # Must call get_preview_fragment directly, as going through xblock RESTful API will attempt + # to use item.location as a Location. + html = get_preview_fragment(request, html).content + # Verify student view html is returned, and there are no old locations in it. + self.assertRegexpMatches( + html, + 'data-usage-id="MITx.999.Robot_Super_Course;_branch;_published;_block;_html_[0-9]*"' + ) + self.assertRegexpMatches(html, 'foobar') + self.assertNotRegexpMatches(html, 'i4x')