Merge pull request #2434 from edx/christina/preview_handler

preview_handler changes to support Locators

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)
+        )

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'

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),
     )
 

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

+"""
+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': "<html>foobar</html>"}
+        )
+
+        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, '<html>foobar</html>')
+        self.assertNotRegexpMatches(html, 'i4x')

common/lib/xmodule/xmodule/contentstore/content.py

@@ -112,15 +112,6 @@ class StaticContent(object):
         return Location(path.split('/'))
 
     @staticmethod
-    def get_id_from_path(path):
-        return get_id_from_location(get_location_from_path(path))
-
-    @staticmethod
-    def convert_legacy_static_url(path, course_namespace):
-        loc = StaticContent.compute_location(course_namespace.org, course_namespace.course, path)
-        return StaticContent.get_url_path_from_location(loc)
-
-    @staticmethod
     def convert_legacy_static_url_with_course_id(path, course_id):
         """
         Returns a path to a piece of static content when we are provided with a filepath and