Skip to content

E
edx-platform
Commit c04ef4b7 by Adam
Options

Merge pull request #7630 from edx/hotfix/2015-04-08 

Hotfix/2015 04 08
parents 08cb8dd5 33ddecbc
Showing with 188 additions and 199 deletions
cms/djangoapps/contentstore/views/tests/test_assets.py
......@@ -14,9 +14,11 @@ from xmodule.assetstore import AssetMetadata
from xmodule.contentstore.content import StaticContent
from xmodule.contentstore.django import contentstore
from xmodule.modulestore.django import modulestore
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.xml_importer import import_course_from_xml
from django.test.utils import override_settings
from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation
from static_replace import replace_static_urls
import mock
from ddt import ddt
from ddt import data
......@@ -86,6 +88,44 @@ class BasicAssetsTestCase(AssetsTestCase):
# Note: Actual contentType for textbook.pdf in asset.json is 'text/pdf'
self.assertEqual(content.content_type, 'application/pdf')
def test_relative_url_for_split_course(self):
"""
Test relative path for split courses assets
"""
with modulestore().default_store(ModuleStoreEnum.Type.split):
module_store = modulestore()
course_id = module_store.make_course_key('edX', 'toy', '2012_Fall')
import_course_from_xml(
module_store,
self.user.id,
TEST_DATA_DIR,
['toy'],
static_content_store=contentstore(),
target_id=course_id,
create_if_not_present=True
)
course = module_store.get_course(course_id)
filename = 'sample_static.txt'
html_src_attribute = '"/static/{}"'.format(filename)
asset_url = replace_static_urls(html_src_attribute, course_id=course.id)
url = asset_url.replace('"', '')
base_url = url.replace(filename, '')
self.assertTrue("/{}".format(filename) in url)
resp = self.client.get(url)
self.assertEquals(resp.status_code, 200)
# simulation of html page where base_url is up-to asset's main directory
# and relative_path is dom element with its src
relative_path = 'just_a_test.jpg'
# browser append relative_path with base_url
absolute_path = base_url + relative_path
self.assertTrue("/{}".format(relative_path) in absolute_path)
resp = self.client.get(absolute_path)
self.assertEquals(resp.status_code, 200)
class PaginationTestCase(AssetsTestCase):
"""
......
common/djangoapps/contentserver/middleware.py
......@@ -31,6 +31,8 @@ class StaticContentServer(object):
request.path.startswith('/' + XASSET_LOCATION_TAG + '/') or
request.path.startswith('/' + AssetLocator.CANONICAL_NAMESPACE)
):
if AssetLocator.CANONICAL_NAMESPACE in request.path:
request.path = request.path.replace('block/', 'block@', 1)
try:
loc = StaticContent.get_location_from_path(request.path)
except (InvalidLocationError, InvalidKeyError):
......
common/djangoapps/static_replace/__init__.py
......@@ -9,6 +9,8 @@ from xmodule.modulestore.django import modulestore
from xmodule.modulestore import ModuleStoreEnum
from xmodule.contentstore.content import StaticContent
from opaque_keys.edx.locator import AssetLocator
log = logging.getLogger(__name__)
......@@ -152,7 +154,6 @@ def replace_static_urls(text, data_directory=None, course_id=None, static_asset_
"""
Replace a single matched url.
"""
# Don't mess with things that end in '?raw'
if rest.endswith('?raw'):
return original
......@@ -180,6 +181,10 @@ def replace_static_urls(text, data_directory=None, course_id=None, static_asset_
# if not, then assume it's courseware specific content and then look in the
# Mongo-backed database
url = StaticContent.convert_legacy_static_url_with_course_id(rest, course_id)
if AssetLocator.CANONICAL_NAMESPACE in url:
url = url.replace('block@', 'block/', 1)
# Otherwise, look the file up in staticfiles_storage, and append the data directory if needed
else:
course_path = "/".join((static_asset_path or data_directory, rest))
......
common/djangoapps/util/keyword_substitution.py
......@@ -21,26 +21,19 @@ Usage:
"""
from django.contrib.auth.models import User
from student.models import anonymous_id_for_user
from xmodule.modulestore.django import modulestore
KEYWORD_FUNCTION_MAP = {}
def keyword_function_map_is_empty():
"""
Checks if the keyword function map has been filled
"""
return not bool(KEYWORD_FUNCTION_MAP)
def add_keyword_function_map(mapping):
def anonymous_id_from_user_id(user_id):
"""
Attaches the given keyword-function mapping to the existing one
Gets a user's anonymous id from their user id
"""
KEYWORD_FUNCTION_MAP.update(mapping)
user = User.objects.get(id=user_id)
return anonymous_id_for_user(user, None)
def substitute_keywords(string, user=None, course=None):
def substitute_keywords(string, user_id, context):
"""
Replaces all %%-encoded words using KEYWORD_FUNCTION_MAP mapping functions
......@@ -48,35 +41,37 @@ def substitute_keywords(string, user=None, course=None):
them by calling the corresponding functions stored in KEYWORD_FUNCTION_MAP.
Functions stored in KEYWORD_FUNCTION_MAP must return a replacement string.
Also, functions imported from other modules must be wrapped in a
new function if they don't take in user_id and course_id. This simplifies
the loop below, and reduces the need for piling up if elif else statements
when the keyword pool grows.
"""
if user is None or course is None:
# Cannot proceed without course and user information
return string
for key, func in KEYWORD_FUNCTION_MAP.iteritems():
# do this lazily to avoid unneeded database hits
KEYWORD_FUNCTION_MAP = {
'%%USER_ID%%': lambda: anonymous_id_from_user_id(user_id),
'%%USER_FULLNAME%%': lambda: context.get('name'),
'%%COURSE_DISPLAY_NAME%%': lambda: context.get('course_title'),
'%%COURSE_END_DATE%%': lambda: context.get('course_end_date'),
}
for key in KEYWORD_FUNCTION_MAP.keys():
if key in string:
substitutor = func(user, course)
string = string.replace(key, substitutor)
substitutor = KEYWORD_FUNCTION_MAP[key]
string = string.replace(key, substitutor())
return string
def substitute_keywords_with_data(string, user_id=None, course_id=None):
def substitute_keywords_with_data(string, context):
"""
Given user and course ids, replaces all %%-encoded words in the given string
Given an email context, replaces all %%-encoded words in the given string
`context` is a dictionary that should include `user_id` and `course_title`
keys
"""
# Do not proceed without parameters: Compatibility check with existing tests
# that do not supply these parameters
if user_id is None or course_id is None:
return string
user_id = context.get('user_id')
course_title = context.get('course_title')
# Grab user objects
user = User.objects.get(id=user_id)
course = modulestore().get_course(course_id, depth=0)
if user_id is None or course_title is None:
return string
return substitute_keywords(string, user, course)
return substitute_keywords(string, user_id, context)
common/djangoapps/util/tests/fixtures/test_keyword_anonid_sub.json deleted 100644 → 0
{
"standard_test": {
"test_string": "This is a test string. sub this: %%USER_ID%% into anon_id",
"expected": "This is a test string. sub this: 123456789 into anon_id"
},
"multiple_subs": {
"test_string": "There are a lot of anonymous ids here: %%USER_ID%% %%USER_ID%% %%USER_ID%% %%USER_ID%%",
"expected": "There are a lot of anonymous ids here: 123456789 123456789 123456789 123456789"
},
"sub_with_html": {
"test_string": "There is html in this guy <b>%%USER_ID%%</b>",
"expected": "There is html in this guy <b>123456789</b>"
}
}
common/djangoapps/util/tests/fixtures/test_keywordsub_multiple_tags.json
......@@ -5,6 +5,6 @@
},
"invalid_tags": {
"test_string": "The user with id %%user-id%% is named %%USER_FULLNAME%% and is in %%COURSE_DISPLAY_NAME",
"expected": "The user with id %%user-id%% is named Test User and is in test_course"
"expected": "The user with id %%user-id%% is named Test User and is in %%COURSE_DISPLAY_NAME"
}
}
common/djangoapps/util/tests/test_keyword_sub_utils.py
......@@ -30,54 +30,45 @@ class KeywordSubTest(ModuleStoreTestCase):
display_name='test_course'
)
# Mimic monkeypatching done in startup.py
Ks.KEYWORD_FUNCTION_MAP = self.get_keyword_function_map()
def get_keyword_function_map(self):
"""
Generates a mapping from keywords to functions for testing
The keyword sub functions should not return %% encoded strings. This
would lead to ugly and inconsistent behavior due to the way in which
keyword subbing is handled.
"""
def user_fullname_sub(user, course): # pylint: disable=unused-argument
""" Returns the user's name """
return user.profile.name
def course_display_name_sub(user, course): # pylint: disable=unused-argument
""" Returns the course name """
return course.display_name
return {
'%%USER_FULLNAME%%': user_fullname_sub,
'%%COURSE_DISPLAY_NAME%%': course_display_name_sub,
self.context = {
'user_id': self.user.id,
'course_title': self.course.display_name,
'name': self.user.profile.name,
'course_end_date': get_default_time_display(self.course.end),
}
@file_data('fixtures/test_keyword_coursename_sub.json')
def test_course_name_sub(self, test_info):
""" Tests subbing course name in various scenarios """
course_name = self.course.display_name
result = Ks.substitute_keywords_with_data(test_info['test_string'], self.user.id, self.course.id)
result = Ks.substitute_keywords_with_data(
test_info['test_string'], self.context,
)
self.assertIn(course_name, result)
self.assertEqual(result, test_info['expected'])
@file_data('fixtures/test_keyword_anonid_sub.json')
def test_anonymous_id_subs(self, test_info):
""" Tests subbing anon user id in various scenarios """
anon_id = '123456789'
with patch.dict(Ks.KEYWORD_FUNCTION_MAP, {'%%USER_ID%%': lambda x, y: anon_id}):
result = Ks.substitute_keywords_with_data(test_info['test_string'], self.user.id, self.course.id)
self.assertIn(anon_id, result)
self.assertEqual(result, test_info['expected'])
def test_anonymous_id_sub(self):
"""
Test that anonymous_id is subbed
"""
test_string = "Turn %%USER_ID%% into anonymous id"
anonymous_id = Ks.anonymous_id_from_user_id(self.user.id)
result = Ks.substitute_keywords_with_data(
test_string, self.context,
)
self.assertNotIn('%%USER_ID%%', result)
self.assertIn(anonymous_id, result)
def test_name_sub(self):
test_string = "This is the test string. subthis: %%USER_FULLNAME%% into user name"
"""
Test that the user's full name is correctly subbed
"""
test_string = "This is the test string. subthis: %%USER_FULLNAME%% into user name"
user_name = self.user.profile.name
result = Ks.substitute_keywords_with_data(test_string, self.user.id, self.course.id)
result = Ks.substitute_keywords_with_data(
test_string, self.context,
)
self.assertNotIn('%%USER_FULLNAME%%', result)
self.assertIn(user_name, result)
......@@ -87,7 +78,9 @@ class KeywordSubTest(ModuleStoreTestCase):
Test that sub-ing doesn't ocurr with illegal tags
"""
test_string = "%%user_id%%"
result = Ks.substitute_keywords_with_data(test_string, self.user.id, self.course.id)
result = Ks.substitute_keywords_with_data(
test_string, self.context,
)
self.assertEquals(test_string, result)
......@@ -96,58 +89,37 @@ class KeywordSubTest(ModuleStoreTestCase):
Test that sub-ing doesn't work without subtags
"""
test_string = "this string has no subtags"
result = Ks.substitute_keywords_with_data(test_string, self.user.id, self.course.id)
result = Ks.substitute_keywords_with_data(
test_string, self.context,
)
self.assertEquals(test_string, result)
@file_data('fixtures/test_keywordsub_multiple_tags.json')
def test_sub_mutiltple_tags(self, test_info):
def test_sub_multiple_tags(self, test_info):
""" Test that subbing works with multiple subtags """
anon_id = '123456789'
patched_dict = {
'%%USER_ID%%': lambda x, y: anon_id,
'%%USER_FULLNAME%%': lambda x, y: self.user.profile.name,
'%%COURSE_DISPLAY_NAME': lambda x, y: self.course.display_name,
'%%COURSE_END_DATE': lambda x, y: get_default_time_display(self.course.end)
}
with patch.dict(Ks.KEYWORD_FUNCTION_MAP, patched_dict):
result = Ks.substitute_keywords_with_data(test_info['test_string'], self.user.id, self.course.id)
with patch('util.keyword_substitution.anonymous_id_from_user_id', lambda user_id: anon_id):
result = Ks.substitute_keywords_with_data(
test_info['test_string'], self.context,
)
self.assertEqual(result, test_info['expected'])
def test_no_subbing_empty_subtable(self):
"""
Test that no sub-ing occurs when the sub table is empty.
"""
# Set the keyword sub mapping to be empty
Ks.KEYWORD_FUNCTION_MAP = {}
test_string = 'This user\'s name is %%USER_FULLNAME%%'
result = Ks.substitute_keywords_with_data(test_string, self.user.id, self.course.id)
self.assertNotIn(self.user.profile.name, result)
self.assertIn('%%USER_FULLNAME%%', result)
def test_subbing_no_userid_or_courseid(self):
"""
Tests that no subbing occurs if no user_id or no course_id is given.
"""
test_string = 'This string should not be subbed here %%USER_ID%%'
result = Ks.substitute_keywords_with_data(test_string, None, self.course.id)
self.assertEqual(test_string, result)
result = Ks.substitute_keywords_with_data(test_string, self.user.id, None)
self.assertEqual(test_string, result)
def test_subbing_no_user_or_course(self):
"""
Tests that no subbing occurs if no user or no course is given
"""
test_string = "This string should not be subbed here %%USER_ID%%"
result = Ks.substitute_keywords(test_string, course=self.course, user=None)
no_course_context = dict(
(key, value) for key, value in self.context.iteritems() if key != 'course_title'
)
result = Ks.substitute_keywords_with_data(test_string, no_course_context)
self.assertEqual(test_string, result)
result = Ks.substitute_keywords(test_string, self.user, None)
no_user_id_context = dict(
(key, value) for key, value in self.context.iteritems() if key != 'user_id'
)
result = Ks.substitute_keywords_with_data(test_string, no_user_id_context)
self.assertEqual(test_string, result)
common/lib/chem/setup.py
......@@ -8,6 +8,6 @@ setup(
"pyparsing==2.0.1",
"numpy",
"scipy",
"nltk==2.0.4",
"nltk==2.0.5",
],
)
common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py
......@@ -498,6 +498,10 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
draft_course = course_key.for_branch(ModuleStoreEnum.BranchName.draft)
with self.branch_setting(ModuleStoreEnum.Branch.draft_preferred, draft_course):
draft_block = self.import_xblock(user_id, draft_course, block_type, block_id, fields, runtime)
# if block was published once and now it is in draft state then return draft version
# as current state of block is draft state
if self.has_published_version(draft_block) and block_type not in DIRECT_ONLY_CATEGORIES:
return draft_block
return self.publish(draft_block.location.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs)
# do the import
......
common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
......@@ -10,6 +10,8 @@ import itertools
import mimetypes
from unittest import skip
from uuid import uuid4
from shutil import rmtree
from tempfile import mkdtemp
# Mixed modulestore depends on django, so we'll manually configure some django settings
# before importing the module
......@@ -26,6 +28,7 @@ from xmodule.modulestore.tests.test_cross_modulestore_import_export import Mongo
from xmodule.contentstore.content import StaticContent
from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.xml_importer import import_course_from_xml
from xmodule.modulestore.xml_exporter import export_course_to_xml
from xmodule.modulestore.django import SignalHandler
if not settings.configured:
......@@ -151,6 +154,8 @@ class TestMixedModuleStore(CourseComparisonTest):
self.course_locations = {}
self.user_id = ModuleStoreEnum.UserID.test
self.export_dir = mkdtemp()
self.addCleanup(rmtree, self.export_dir, ignore_errors=True)
# pylint: disable=invalid-name
def _create_course(self, course_key):
......@@ -504,6 +509,56 @@ class TestMixedModuleStore(CourseComparisonTest):
component = self.store.publish(component.location, self.user_id)
self.assertFalse(self.store.has_changes(component))
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_has_changes_before_export_and_after_import(self, default_ms):
"""
Tests that an unpublished unit remains with no changes across export and re-import.
"""
with MongoContentstoreBuilder().build() as contentstore:
# initialize the mixed modulestore
self._initialize_mixed(contentstore=contentstore, mappings={})
with self.store.default_store(default_ms):
source_course_key = self.store.make_course_key("org.source", "course.source", "run.source")
self._create_course(source_course_key)
# Create a dummy component to test against
xblock = self.store.create_item(
self.user_id,
self.course.id,
'vertical',
block_id='test_vertical'
)
# Not yet published, so changes are present
self.assertTrue(self.store.has_changes(xblock))
export_course_to_xml(
self.store,
contentstore,
source_course_key,
self.export_dir,
'exported_source_course',
)
import_course_from_xml(
self.store,
'test_user',
self.export_dir,
source_dirs=['exported_source_course'],
static_content_store=contentstore,
target_id=source_course_key,
create_if_not_present=True,
raise_on_failure=True,
)
# Get the xblock from the imported course.
component = self.store.get_item(xblock.location)
# Verify that it still is a draft, i.e. has changes.
self.assertTrue(self.store.has_changes(component))
def _has_changes(self, location):
"""
Helper function that loads the item before calling has_changes
......
docs/shared/requirements.txt
......@@ -39,7 +39,7 @@ mako==0.7.3
Markdown==2.2.1
mock==1.0.1
networkx==1.7
nltk==2.0.4
nltk==2.0.5
oauthlib==0.6.3
paramiko==1.9.0
path.py==3.0.1
......
lms/djangoapps/bulk_email/models.py
......@@ -197,7 +197,7 @@ class CourseEmailTemplate(models.Model):
# Substitute all %%-encoded keywords in the message body
if 'user_id' in context and 'course_id' in context:
message_body = substitute_keywords_with_data(message_body, context['user_id'], context['course_id'])
message_body = substitute_keywords_with_data(message_body, context)
result = format_string.format(**context)
......
lms/djangoapps/bulk_email/tasks.py
......@@ -48,6 +48,7 @@ from instructor_task.subtasks import (
update_subtask_status,
)
from util.query import use_read_replica_if_available
from util.date_utils import get_default_time_display
log = logging.getLogger('edx.celery.task')
......@@ -146,6 +147,7 @@ def _get_course_email_context(course):
"""
course_id = course.id.to_deprecated_string()
course_title = course.display_name
course_end_date = get_default_time_display(course.end)
course_url = 'https://{}{}'.format(
settings.SITE_NAME,
reverse('course_root', kwargs={'course_id': course_id})
......@@ -155,6 +157,7 @@ def _get_course_email_context(course):
'course_title': course_title,
'course_url': course_url,
'course_image_url': image_url,
'course_end_date': course_end_date,
'account_settings_url': 'https://{}{}'.format(settings.SITE_NAME, reverse('dashboard')),
'platform_name': settings.PLATFORM_NAME,
}
......
lms/startup.py
......@@ -14,7 +14,6 @@ import edxmako
import logging
from monkey_patch import django_utils_translation
import analytics
from util import keyword_substitution
log = logging.getLogger(__name__)
......@@ -44,12 +43,6 @@ def run():
if settings.FEATURES.get('SEGMENT_IO_LMS') and hasattr(settings, 'SEGMENT_IO_LMS_KEY'):
analytics.init(settings.SEGMENT_IO_LMS_KEY, flush_at=50)
# Monkey patch the keyword function map
if keyword_substitution.keyword_function_map_is_empty():
keyword_substitution.add_keyword_function_map(get_keyword_function_map())
# Once keyword function map is set, make update function do nothing
keyword_substitution.add_keyword_function_map = lambda x: None
def add_mimetypes():
"""
......@@ -149,51 +142,3 @@ def enable_third_party_auth():
from third_party_auth import settings as auth_settings
auth_settings.apply_settings(settings.THIRD_PARTY_AUTH, settings)
def get_keyword_function_map():
"""
Define the mapping of keywords and filtering functions
The functions are used to filter html, text and email strings
before rendering them.
The generated map will be monkey-patched onto the keyword_substitution
module so that it persists along with the running server.
Each function must take: user & course as parameters
"""
from student.models import anonymous_id_for_user
from util.date_utils import get_default_time_display
def user_id_sub(user, course):
"""
Gives the anonymous id for the given user
For compatibility with the existing anon_ids, return anon_id without course_id
"""
return anonymous_id_for_user(user, None)
def user_fullname_sub(user, course=None):
""" Returns the given user's name """
return user.profile.name
def course_display_name_sub(user, course):
""" Returns the course's display name """
return course.display_name
def course_end_date_sub(user, course):
""" Returns the course end date in the default display """
return get_default_time_display(course.end)
# Define keyword -> function map
# Take care that none of these functions return %% encoded keywords
kf_map = {
'%%USER_ID%%': user_id_sub,
'%%USER_FULLNAME%%': user_fullname_sub,
'%%COURSE_DISPLAY_NAME%%': course_display_name_sub,
'%%COURSE_END_DATE%%': course_end_date_sub
}
return kf_map
lms/tests.py
......@@ -58,21 +58,3 @@ class HelpModalTests(ModuleStoreTestCase):
url = reverse('info', args=[self.course.id.to_deprecated_string()])
resp = self.client.get(url)
self.assertEqual(resp.status_code, 200)
class KeywordSubConfigTests(TestCase):
""" Tests for configuring keyword substitution feature """
def test_keyword_map_not_empty(self):
""" Ensure that the keyword subsitution map is non-empty """
self.assertFalse(keyword_substitution.keyword_function_map_is_empty())
def test_adding_keyword_map_is_noop(self):
""" Test that trying to add a new keyword mapping is a no-op """
existing_map = keyword_substitution.KEYWORD_FUNCTION_MAP
keyword_substitution.add_keyword_function_map({
'%%USER_ID%%': lambda x: x,
'%%USER_FULLNAME%%': lambda x: x,
})
self.assertDictEqual(existing_map, keyword_substitution.KEYWORD_FUNCTION_MAP)
requirements/edx-sandbox/base.txt
......@@ -8,5 +8,5 @@ numpy==1.6.2
networkx==1.7
sympy==0.7.1
pyparsing==2.0.1
nltk==2.0.4
nltk==2.0.5
matplotlib==1.3.1
requirements/edx/base.txt
......@@ -56,7 +56,7 @@ Markdown==2.2.1
meliae==0.4.0
mongoengine==0.7.10
networkx==1.7
nltk==2.0.4
nltk==2.0.5
nose==1.3.3
oauthlib==0.6.3
paramiko==1.9.0
......
requirements/edx/github.txt
......@@ -29,7 +29,7 @@ git+https://github.com/mitocw/django-cas.git@60a5b8e5a62e63e0d5d224a87f0b489201a
-e git+https://github.com/edx/bok-choy.git@82d2f4b72e807b10d112179c0a4abd810a001b82#egg=bok_choy
-e git+https://github.com/edx-solutions/django-splash.git@7579d052afcf474ece1239153cffe1c89935bc4f#egg=django-splash
-e git+https://github.com/edx/acid-block.git@e46f9cda8a03e121a00c7e347084d142d22ebfb7#egg=acid-xblock
-e git+https://github.com/edx/edx-ora2.git@release-2015-03-27T14.32#egg=edx-ora2
-e git+https://github.com/edx/edx-ora2.git@release-2015-04-07T13.05#egg=edx-ora2
-e git+https://github.com/edx/edx-submissions.git@8fb070d2a3087dd7656d27022e550d12e3b85ba3#egg=edx-submissions
-e git+https://github.com/edx/opaque-keys.git@1254ed4d615a428591850656f39f26509b86d30a#egg=opaque-keys
-e git+https://github.com/edx/ease.git@97de68448e5495385ba043d3091f570a699d5b5f#egg=ease
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment