From 825aaed67f061ab62c297718d4e1d2d387e2cab1 Mon Sep 17 00:00:00 2001 From: Syed Hassan Raza Date: Tue, 31 Mar 2015 17:40:24 +0500 Subject: [PATCH 1/6] Split course's assests can access through '/' in asset url --- .../contentstore/views/tests/test_assets.py | 40 +++++++++++++++++++ common/djangoapps/contentserver/middleware.py | 2 + common/djangoapps/static_replace/__init__.py | 7 +++- 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_assets.py b/cms/djangoapps/contentstore/views/tests/test_assets.py index 8a454d9d06..daa7450f2b 100644 --- a/cms/djangoapps/contentstore/views/tests/test_assets.py +++ b/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): """ diff --git a/common/djangoapps/contentserver/middleware.py b/common/djangoapps/contentserver/middleware.py index 987d4383d4..a5fa3ca2c1 100644 --- a/common/djangoapps/contentserver/middleware.py +++ b/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): diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index d0e617008f..96db5a73b6 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/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)) From d9a21bdf4de74dfe88368f8f08a9f7aa01d99638 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Mon, 6 Apr 2015 09:24:39 -0400 Subject: [PATCH 2/6] use email context for keyword substitution, cutting down on db queries (TNL-1591) --- .../djangoapps/util/keyword_substitution.py | 57 ++++----- .../fixtures/test_keyword_anonid_sub.json | 14 --- .../test_keywordsub_multiple_tags.json | 2 +- .../util/tests/test_keyword_sub_utils.py | 118 +++++++----------- lms/djangoapps/bulk_email/models.py | 2 +- lms/djangoapps/bulk_email/tasks.py | 3 + lms/startup.py | 55 -------- lms/tests.py | 18 --- 8 files changed, 76 insertions(+), 193 deletions(-) delete mode 100644 common/djangoapps/util/tests/fixtures/test_keyword_anonid_sub.json diff --git a/common/djangoapps/util/keyword_substitution.py b/common/djangoapps/util/keyword_substitution.py index 4f206890c7..f9a9aa34e0 100644 --- a/common/djangoapps/util/keyword_substitution.py +++ b/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(): +def anonymous_id_from_user_id(user_id): """ - Checks if the keyword function map has been filled + Gets a user's anonymous id from their user id """ - return not bool(KEYWORD_FUNCTION_MAP) + user = User.objects.get(id=user_id) + return anonymous_id_for_user(user, None) -def add_keyword_function_map(mapping): - """ - Attaches the given keyword-function mapping to the existing one - """ - KEYWORD_FUNCTION_MAP.update(mapping) - - -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: + user_id = context.get('user_id') + course_title = context.get('course_title') + + if user_id is None or course_title is None: return string - # Grab user objects - user = User.objects.get(id=user_id) - course = modulestore().get_course(course_id, depth=0) - - return substitute_keywords(string, user, course) + return substitute_keywords(string, user_id, context) diff --git a/common/djangoapps/util/tests/fixtures/test_keyword_anonid_sub.json b/common/djangoapps/util/tests/fixtures/test_keyword_anonid_sub.json deleted file mode 100644 index 303a028104..0000000000 --- a/common/djangoapps/util/tests/fixtures/test_keyword_anonid_sub.json +++ /dev/null @@ -1,14 +0,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 %%USER_ID%%", - "expected": "There is html in this guy 123456789" - } -} diff --git a/common/djangoapps/util/tests/fixtures/test_keywordsub_multiple_tags.json b/common/djangoapps/util/tests/fixtures/test_keywordsub_multiple_tags.json index dc2f1b0353..2105b9a45a 100644 --- a/common/djangoapps/util/tests/fixtures/test_keywordsub_multiple_tags.json +++ 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" } } diff --git a/common/djangoapps/util/tests/test_keyword_sub_utils.py b/common/djangoapps/util/tests/test_keyword_sub_utils.py index 2cb20aa96f..e9d2381f5c 100644 --- a/common/djangoapps/util/tests/test_keyword_sub_utils.py +++ b/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) + 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_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) - 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) diff --git a/lms/djangoapps/bulk_email/models.py b/lms/djangoapps/bulk_email/models.py index d315db6d15..0842f93958 100644 --- a/lms/djangoapps/bulk_email/models.py +++ b/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) diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 8c53c561d0..fda5b33ae5 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/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, } diff --git a/lms/startup.py b/lms/startup.py index 6d95d629dc..3add027f15 100644 --- a/lms/startup.py +++ b/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 diff --git a/lms/tests.py b/lms/tests.py index 36c0aa05ae..09946a9fbb 100644 --- a/lms/tests.py +++ b/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) From 23e08b2744e1f146f265d2e5f76d8f667f8b581b Mon Sep 17 00:00:00 2001 From: Syed Hassan Raza Date: Tue, 7 Apr 2015 16:58:47 +0500 Subject: [PATCH 3/6] Split courses drafts import issue --- .../modulestore/split_mongo/split_draft.py | 4 ++ .../tests/test_mixed_modulestore.py | 55 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index 7d84e56124..5fb612ca4d 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index cbd6c6762e..2c9b3dfb80 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/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 From c20c99baddcdc4be5ca6332785286e4619150471 Mon Sep 17 00:00:00 2001 From: Waheed Ahmed Date: Tue, 7 Apr 2015 18:11:38 +0500 Subject: [PATCH 4/6] Added ora release tag. --- requirements/edx/github.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 2c3c0ac176..ac1d516f56 100644 --- a/requirements/edx/github.txt +++ b/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 From aac8bf2ebd14f9b78ab85499eafb61b810f72142 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 7 Apr 2015 10:41:02 -0400 Subject: [PATCH 5/6] Update NLTK to 2.0.5 For setuptools compatibility --- requirements/edx/base.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index da620db189..c4e7fba5e2 100644 --- a/requirements/edx/base.txt +++ b/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 From 33ddecbc97749ecffbf670b1c56c53d3a479ca81 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Wed, 8 Apr 2015 11:28:49 -0400 Subject: [PATCH 6/6] update nltk in other parts of the platform --- common/lib/chem/setup.py | 2 +- docs/shared/requirements.txt | 2 +- requirements/edx-sandbox/base.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/chem/setup.py b/common/lib/chem/setup.py index f275ea4d25..dddd484565 100644 --- a/common/lib/chem/setup.py +++ b/common/lib/chem/setup.py @@ -8,6 +8,6 @@ setup( "pyparsing==2.0.1", "numpy", "scipy", - "nltk==2.0.4", + "nltk==2.0.5", ], ) diff --git a/docs/shared/requirements.txt b/docs/shared/requirements.txt index b924ba0c9c..a3e4e2a06a 100644 --- a/docs/shared/requirements.txt +++ b/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 diff --git a/requirements/edx-sandbox/base.txt b/requirements/edx-sandbox/base.txt index cb41b9b7a3..e0a8c46f19 100644 --- a/requirements/edx-sandbox/base.txt +++ b/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