Merge pull request #11602 from edx/sstudent/MA-1919

MA-1919 making mobile handout links accommodate jump to id's and cour…

lms/djangoapps/mobile_api/course_info/tests.py

@@ -84,60 +84,98 @@ class TestUpdates(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTest
             self.assertIn("Update" + str(num), update_data['content'])
 
 
+@ddt.ddt
 class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin, MilestonesTestCaseMixin):
     """
     Tests for /api/mobile/v0.5/course_info/{course_id}/handouts
     """
     REVERSE_INFO = {'name': 'course-handouts-list', 'params': ['course_id']}
 
-    def setUp(self):
-        super(TestHandouts, self).setUp()
-
-        # Deleting handouts fails with split modulestore because the handout has no parent.
-        # This needs further investigation to determine if it is a bug in the split modulestore.
-        # pylint: disable=protected-access
-        self.store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo)
-
-        # use toy course with handouts, and make it mobile_available
-        course_items = import_course_from_xml(self.store, self.user.id, settings.COMMON_TEST_DATA_ROOT, ['toy'])
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_handouts(self, default_ms):
+        with self.store.default_store(default_ms):
+            self.add_mobile_available_toy_course()
+            response = self.api_response(expected_response_code=200)
+            self.assertIn("Sample", response.data['handouts_html'])
+
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_no_handouts(self, default_ms):
+        with self.store.default_store(default_ms):
+            self.add_mobile_available_toy_course()
+
+            # delete handouts in course
+            handouts_usage_key = self.course.id.make_usage_key('course_info', 'handouts')
+            with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id):
+                self.store.delete_item(handouts_usage_key, self.user.id)
+
+            response = self.api_response(expected_response_code=200)
+            self.assertIsNone(response.data['handouts_html'])
+
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_empty_handouts(self, default_ms):
+        with self.store.default_store(default_ms):
+            self.add_mobile_available_toy_course()
+
+            # set handouts to empty tags
+            handouts_usage_key = self.course.id.make_usage_key('course_info', 'handouts')
+            underlying_handouts = self.store.get_item(handouts_usage_key)
+            underlying_handouts.data = "<ol></ol>"
+            self.store.update_item(underlying_handouts, self.user.id)
+            response = self.api_response(expected_response_code=200)
+            self.assertIsNone(response.data['handouts_html'])
+
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_handouts_static_rewrites(self, default_ms):
+        with self.store.default_store(default_ms):
+            self.add_mobile_available_toy_course()
+
+            # check that we start with relative static assets
+            handouts_usage_key = self.course.id.make_usage_key('course_info', 'handouts')
+            underlying_handouts = self.store.get_item(handouts_usage_key)
+            self.assertIn('\'/static/', underlying_handouts.data)
+
+            # but shouldn't finish with any
+            response = self.api_response()
+            self.assertNotIn('\'/static/', response.data['handouts_html'])
+
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_jump_to_id_handout_href(self, default_ms):
+        with self.store.default_store(default_ms):
+            self.add_mobile_available_toy_course()
+
+            # check that we start with relative static assets
+            handouts_usage_key = self.course.id.make_usage_key('course_info', 'handouts')
+            underlying_handouts = self.store.get_item(handouts_usage_key)
+            underlying_handouts.data = "<a href=\"/jump_to_id/identifier\">Intracourse Link</a>"
+            self.store.update_item(underlying_handouts, self.user.id)
+
+            # but shouldn't finish with any
+            response = self.api_response()
+            self.assertIn("/courses/{}/jump_to_id/".format(self.course.id), response.data['handouts_html'])
+
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_course_url_handout_href(self, default_ms):
+        with self.store.default_store(default_ms):
+            self.add_mobile_available_toy_course()
+
+            # check that we start with relative static assets
+            handouts_usage_key = self.course.id.make_usage_key('course_info', 'handouts')
+            underlying_handouts = self.store.get_item(handouts_usage_key)
+            underlying_handouts.data = "<a href=\"/course/identifier\">Linked Content</a>"
+            self.store.update_item(underlying_handouts, self.user.id)
+
+            # but shouldn't finish with any
+            response = self.api_response()
+            self.assertIn("/courses/{}/".format(self.course.id), response.data['handouts_html'])
+
+    def add_mobile_available_toy_course(self):
+        """ use toy course with handouts, and make it mobile_available """
+        course_items = import_course_from_xml(
+            self.store, self.user.id,
+            settings.COMMON_TEST_DATA_ROOT, ['toy'],
+            create_if_not_present=True
+        )
         self.course = course_items[0]
         self.course.mobile_available = True
         self.store.update_item(self.course, self.user.id)
-
-    def verify_success(self, response):
-        super(TestHandouts, self).verify_success(response)
-        self.assertIn('Sample', response.data['handouts_html'])
-
-    def test_no_handouts(self):
         self.login_and_enroll()
-
-        # delete handouts in course
-        handouts_usage_key = self.course.id.make_usage_key('course_info', 'handouts')
-        with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id):
-            self.store.delete_item(handouts_usage_key, self.user.id)
-
-        response = self.api_response(expected_response_code=200)
-        self.assertIsNone(response.data['handouts_html'])
-
-    def test_empty_handouts(self):
-        self.login_and_enroll()
-
-        # set handouts to empty tags
-        handouts_usage_key = self.course.id.make_usage_key('course_info', 'handouts')
-        underlying_handouts = self.store.get_item(handouts_usage_key)
-        underlying_handouts.data = "<ol></ol>"
-        self.store.update_item(underlying_handouts, self.user.id)
-        response = self.api_response(expected_response_code=200)
-        self.assertIsNone(response.data['handouts_html'])
-
-    def test_handouts_static_rewrites(self):
-        self.login_and_enroll()
-
-        # check that we start with relative static assets
-        handouts_usage_key = self.course.id.make_usage_key('course_info', 'handouts')
-        underlying_handouts = self.store.get_item(handouts_usage_key)
-        self.assertIn('\'/static/', underlying_handouts.data)
-
-        # but shouldn't finish with any
-        response = self.api_response()
-        self.assertNotIn('\'/static/', response.data['handouts_html'])

lms/djangoapps/mobile_api/course_info/views.py

@@ -6,7 +6,7 @@ from rest_framework import generics
 from rest_framework.response import Response
 
 from courseware.courses import get_course_info_section_module
-from static_replace import make_static_urls_absolute, replace_static_urls
+from static_replace import make_static_urls_absolute
 from openedx.core.lib.xblock_utils import get_course_update_items
 
 from ..utils import mobile_view, mobile_course_access
@@ -46,12 +46,7 @@ class CourseUpdatesList(generics.ListAPIView):
         ]
 
         for item in updates_to_show:
-            content = item['content']
-            content = replace_static_urls(
-                content,
-                course_id=course.id,
-                static_asset_path=course.static_asset_path)
-            item['content'] = make_static_urls_absolute(request, content)
+            item['content'] = apply_wrappers_to_content(item['content'], course_updates_module, request)
 
         return Response(updates_to_show)
 
@@ -82,14 +77,32 @@ class CourseHandoutsList(generics.ListAPIView):
             if course_handouts_module.data == "<ol></ol>":
                 handouts_html = None
             else:
-                handouts_html = course_handouts_module.data
-                handouts_html = replace_static_urls(
-                    handouts_html,
-                    course_id=course.id,
-                    static_asset_path=course.static_asset_path
-                )
-                handouts_html = make_static_urls_absolute(self.request, handouts_html)
+                handouts_html = apply_wrappers_to_content(course_handouts_module.data, course_handouts_module, request)
             return Response({'handouts_html': handouts_html})
         else:
             # course_handouts_module could be None if there are no handouts
             return Response({'handouts_html': None})
+
+
+def apply_wrappers_to_content(content, module, request):
+    """
+    Updates a piece of html content with the filter functions stored in its module system, then replaces any
+    static urls with absolute urls.
+
+    Args:
+        content: The html content to which to apply the content wrappers generated for this module system.
+        module: The module containing a reference to the module system which contains functions to apply to the
+        content. These functions include:
+            * Replacing static url's
+            * Replacing course url's
+            * Replacing jump to id url's
+        request: The request, used to replace static URLs with absolute URLs.
+
+    Returns: A piece of html content containing the original content updated by each wrapper.
+
+    """
+    content = module.system.replace_urls(content)
+    content = module.system.replace_course_urls(content)
+    content = module.system.replace_jump_to_id_urls(content)
+
+    return make_static_urls_absolute(request, content)