diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index 8396940f8e..d0e617008f 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/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) diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py index b74965277f..9a9191ab8c 100644 --- a/common/djangoapps/static_replace/test/test_static_replace.py +++ b/common/djangoapps/static_replace/test/test_static_replace.py @@ -1,8 +1,13 @@ 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 diff --git a/lms/djangoapps/mobile_api/course_info/tests.py b/lms/djangoapps/mobile_api/course_info/tests.py index d1a6f7c9cc..68d4f11d87 100644 --- a/lms/djangoapps/mobile_api/course_info/tests.py +++ b/lms/djangoapps/mobile_api/course_info/tests.py @@ -1,23 +1,26 @@ """ 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.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE +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 xmodule.modulestore.xml_importer import import_from_xml -@override_settings(MODULESTORE=TEST_DATA_MOCK_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') @@ -28,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": "foo", + "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) diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index 153d1a84d7..85f90d1225 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/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 ""} )