Merge pull request #6134 from edx/hotfix/2014-12-03

Make static URLs absolute in handouts, updates, and about for mobile API

common/djangoapps/static_replace/__init__.py

@@ -92,7 +92,50 @@ def replace_course_urls(text, course_key):
     return re.sub(_url_replace_regex('/course/'), replace_course_url, text)
 
 
-def replace_static_urls(text, data_directory, course_id=None, static_asset_path=''):
+def process_static_urls(text, replacement_function, data_dir=None):
+    """
+    Run an arbitrary replacement function on any urls matching the static file
+    directory
+    """
+    def wrap_part_extraction(match):
+        """
+        Unwraps a match group for the captures specified in _url_replace_regex
+        and forward them on as function arguments
+        """
+        original = match.group(0)
+        prefix = match.group('prefix')
+        quote = match.group('quote')
+        rest = match.group('rest')
+        return replacement_function(original, prefix, quote, rest)
+
+    return re.sub(
+        _url_replace_regex(u'(?:{static_url}|/static/)(?!{data_dir})'.format(
+            static_url=settings.STATIC_URL,
+            data_dir=data_dir
+        )),
+        wrap_part_extraction,
+        text
+    )
+
+
+def make_static_urls_absolute(request, html):
+    """
+    Converts relative URLs referencing static assets to absolute URLs
+    """
+    def replace(__, prefix, quote, rest):
+        """
+        Function to actually do a single relative -> absolute url replacement
+        """
+        processed = request.build_absolute_uri(prefix + rest)
+        return quote + processed + quote
+
+    return process_static_urls(
+        html,
+        replace
+    )
+
+
+def replace_static_urls(text, data_directory=None, course_id=None, static_asset_path=''):
     """
     Replace /static/$stuff urls either with their correct url as generated by collectstatic,
     (/static/$md5_hashed_stuff) or by the course-specific content static url
@@ -105,11 +148,10 @@ def replace_static_urls(text, data_directory, course_id=None, static_asset_path=
     static_asset_path: Path for static assets, which overrides data_directory and course_namespace, if nonempty
     """
 
-    def replace_static_url(match):
-        original = match.group(0)
-        prefix = match.group('prefix')
-        quote = match.group('quote')
-        rest = match.group('rest')
+    def replace_static_url(original, prefix, quote, rest):
+        """
+        Replace a single matched url.
+        """
 
         # Don't mess with things that end in '?raw'
         if rest.endswith('?raw'):
@@ -155,11 +197,4 @@ def replace_static_urls(text, data_directory, course_id=None, static_asset_path=
 
         return "".join([quote, url, quote])
 
-    return re.sub(
-        _url_replace_regex(u'(?:{static_url}|/static/)(?!{data_dir})'.format(
-            static_url=settings.STATIC_URL,
-            data_dir=static_asset_path or data_directory
-        )),
-        replace_static_url,
-        text
-    )
+    return process_static_urls(text, replace_static_url, data_dir=static_asset_path or data_directory)

common/djangoapps/static_replace/test/test_static_replace.py

 import re
 
 from nose.tools import assert_equals, assert_true, assert_false  # pylint: disable=no-name-in-module
-from static_replace import (replace_static_urls, replace_course_urls,
-                            _url_replace_regex)
+from static_replace import (
+    replace_static_urls,
+    replace_course_urls,
+    _url_replace_regex,
+    process_static_urls,
+    make_static_urls_absolute
+)
 from mock import patch, Mock
 
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
@@ -27,6 +32,37 @@ def test_multi_replace():
     )
 
 
+def test_process_url():
+    def processor(__, prefix, quote, rest):  # pylint: disable=missing-docstring
+        return quote + 'test' + prefix + rest + quote
+
+    assert_equals('"test/static/file.png"', process_static_urls(STATIC_SOURCE, processor))
+
+
+def test_process_url_data_dir_exists():
+    base = '"/static/{data_dir}/file.png"'.format(data_dir=DATA_DIRECTORY)
+
+    def processor(original, prefix, quote, rest):  # pylint: disable=unused-argument,missing-docstring
+        return quote + 'test' + rest + quote
+
+    assert_equals(base, process_static_urls(base, processor, data_dir=DATA_DIRECTORY))
+
+
+def test_process_url_no_match():
+
+    def processor(__, prefix, quote, rest):  # pylint: disable=missing-docstring
+        return quote + 'test' + prefix + rest + quote
+
+    assert_equals('"test/static/file.png"', process_static_urls(STATIC_SOURCE, processor))
+
+
+@patch('django.http.HttpRequest')
+def test_static_urls(mock_request):
+    mock_request.build_absolute_uri = lambda url: 'http://' + url
+    result = make_static_urls_absolute(mock_request, STATIC_SOURCE)
+    assert_equals(result, '\"http:///static/file.png\"')
+
+
 @patch('static_replace.staticfiles_storage')
 def test_storage_url_exists(mock_storage):
     mock_storage.exists.return_value = True

lms/djangoapps/mobile_api/course_info/tests.py

 """
 Tests for course_info
 """
-from django.test.utils import override_settings
+import json
+
+from django.conf import settings
 from django.core.urlresolvers import reverse
 from rest_framework.test import APITestCase
+
+from courseware.tests.factories import UserFactory
+from xmodule.html_module import CourseInfoModule
+from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
-from courseware.tests.factories import UserFactory
-from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
+from xmodule.modulestore.xml_importer import import_from_xml
 
 
-@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
-class TestVideoOutline(ModuleStoreTestCase, APITestCase):
+class TestCourseInfo(APITestCase):
     """
     Tests for /api/mobile/v0.5/course_info/...
     """
     def setUp(self):
-        super(TestVideoOutline, self).setUp()
+        super(TestCourseInfo, self).setUp()
         self.user = UserFactory.create()
         self.course = CourseFactory.create(mobile_available=True)
         self.client.login(username=self.user.username, password='test')
@@ -27,14 +31,91 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase):
         self.assertEqual(response.status_code, 200)
         self.assertTrue('overview' in response.data)  # pylint: disable=maybe-no-member
 
-    def test_handouts(self):
-        url = reverse('course-handouts-list', kwargs={'course_id': unicode(self.course.id)})
-        response = self.client.get(url)
-        self.assertEqual(response.status_code, 404)
-
     def test_updates(self):
         url = reverse('course-updates-list', kwargs={'course_id': unicode(self.course.id)})
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, [])  # pylint: disable=maybe-no-member
-        # TODO: add handouts and updates, somehow
+
+    def test_about_static_rewrites(self):
+        about_usage_key = self.course.id.make_usage_key('about', 'overview')
+        about_module = modulestore().get_item(about_usage_key)
+        underlying_about_html = about_module.data
+
+        # check that we start with relative static assets
+        self.assertIn('\"/static/', underlying_about_html)
+
+        url = reverse('course-about-detail', kwargs={'course_id': unicode(self.course.id)})
+        response = self.client.get(url)
+        json_data = json.loads(response.content)
+        about_html = json_data['overview']
+
+        # but shouldn't finish with any
+        self.assertEqual(response.status_code, 200)
+        self.assertNotIn('\"/static/', about_html)
+
+    def test_updates_rewrite(self):
+        updates_usage_key = self.course.id.make_usage_key('course_info', 'updates')
+        course_updates = modulestore().create_item(
+            self.user.id,
+            updates_usage_key.course_key,
+            updates_usage_key.block_type,
+            block_id=updates_usage_key.block_id
+        )
+        course_update_data = {
+            "id": 1,
+            "date": "Some date",
+            "content": "<a href=\"/static/\">foo</a>",
+            "status": CourseInfoModule.STATUS_VISIBLE
+        }
+
+        course_updates.items = [course_update_data]
+        modulestore().update_item(course_updates, self.user.id)
+
+        url = reverse('course-updates-list', kwargs={'course_id': unicode(self.course.id)})
+        response = self.client.get(url)
+        content = response.data[0]["content"]  # pylint: disable=maybe-no-member
+        self.assertEqual(response.status_code, 200)
+        self.assertNotIn("\"/static/", content)
+
+        underlying_updates_module = modulestore().get_item(updates_usage_key)
+        self.assertIn("\"/static/", underlying_updates_module.items[0]['content'])
+
+
+class TestHandoutInfo(ModuleStoreTestCase, APITestCase):
+    """
+    Tests for /api/mobile/v0.5/course_info/{course_id}/handouts
+    """
+    def setUp(self):
+        super(TestHandoutInfo, self).setUp()
+        self.user = UserFactory.create()
+        self.client.login(username=self.user.username, password='test')
+        course_items = import_from_xml(self.store, self.user.id, settings.COMMON_TEST_DATA_ROOT, ['toy'])
+        self.course = course_items[0]
+
+    def test_no_handouts(self):
+        empty_course = CourseFactory.create(mobile_available=True)
+        url = reverse('course-handouts-list', kwargs={'course_id': unicode(empty_course.id)})
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
+
+    def test_handout_exists(self):
+        url = reverse('course-handouts-list', kwargs={'course_id': unicode(self.course.id)})
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+
+    def test_handout_static_rewrites(self):
+        # 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)
+
+        url = reverse('course-handouts-list', kwargs={'course_id': unicode(self.course.id)})
+        response = self.client.get(url)
+
+        json_data = json.loads(response.content)
+        handouts_html = json_data['handouts_html']
+
+        # but shouldn't finish with any
+        self.assertNotIn('\'/static/', handouts_html)
+        self.assertEqual(response.status_code, 200)

lms/djangoapps/mobile_api/course_info/views.py

@@ -10,6 +10,7 @@ from courseware.courses import get_course_about_section, get_course_info_section
 from opaque_keys.edx.keys import CourseKey
 
 from xmodule.modulestore.django import modulestore
+from static_replace import make_static_urls_absolute, replace_static_urls
 
 
 class CourseUpdatesList(generics.ListAPIView):
@@ -28,8 +29,7 @@ class CourseUpdatesList(generics.ListAPIView):
 
             * date: The date of the course update.
 
-            * content: The content, as a string, of the course update. HTML tags
-              are not included in the string.
+            * content: The content, as an HTML string, of the course update.
 
             * status: Whether the update is visible or not.
 
@@ -42,10 +42,21 @@ class CourseUpdatesList(generics.ListAPIView):
         course_id = CourseKey.from_string(kwargs['course_id'])
         course = modulestore().get_course(course_id)
         course_updates_module = get_course_info_section_module(request, course, 'updates')
+        update_items = reversed(getattr(course_updates_module, 'items', []))
+
         updates_to_show = [
-            update for update in reversed(getattr(course_updates_module, 'items', []))
+            update for update in update_items
             if update.get("status") != "deleted"
         ]
+
+        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)
+
         return Response(updates_to_show)
 
 
@@ -71,7 +82,13 @@ class CourseHandoutsList(generics.ListAPIView):
         course = modulestore().get_course(course_id)
         course_handouts_module = get_course_info_section_module(request, course, 'handouts')
         if course_handouts_module:
-            return Response({'handouts_html': course_handouts_module.data})
+            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)
+            return Response({'handouts_html': handouts_html})
         else:
             # course_handouts_module could be None if there are no handouts
             # (such as while running tests)
@@ -104,6 +121,8 @@ class CourseAboutDetail(generics.RetrieveAPIView):
         #
         # This can also return None, so check for that before calling strip()
         about_section_html = get_course_about_section(course, "overview")
+        about_section_html = make_static_urls_absolute(self.request, about_section_html)
+
         return Response(
             {"overview": about_section_html.strip() if about_section_html else ""}
         )